-
Notifications
You must be signed in to change notification settings - Fork 447
Tokenizer for syntax highlighting using Prism #1478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
cf98f2b to
870b68f
Compare
870b68f to
5db5b5d
Compare
5db5b5d to
0c393a5
Compare
0c393a5 to
46cc1f8
Compare
|
🚀 Preview deployment available at: https://1dae1d57.rdoc-6cd.pages.dev (commit: 46cc1f8) |
st0012
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we make Prism a dependency and just remove RipperStateLex?
RipperStateLex is still used in This pull request makes
We can change this, but it will also change syntax highlight result. It may also be a relatively large change. |
|
So at the moment we have
And this PR will add another tokenizer using Prism. Is this correct?
Will we avoid this if we fully migrate to Prism parser? I think my main concern is that after this we'll have 2 tokenizers and 2 parsers but it's not clear when we'll be able to drop the old ones.
|
Yes. And two of the Ripper based parser/tokenizer will be unmaintaind/keep unchanged until we drop it.
Yes. I reconsidered this pull request, it's better to avoid it.
It should be, but the current pull req is not the straight way for this. |
|
WDYT about moving coloring to the frontend, after we deprecated darkfish? It'll simplify RDoc's Ruby implementation quite a bit and provide significant speedup to generation (with YJIT enabled in core, it went from 50s to 23s). |
|
I think syntax highlight is not a bottleneck.
RDoc's C-tokenzier/parser is not a complete parser, so using JS syntax highlighter makes sense to me. |
To replace RDoc::Parser::RipperStateLex