-
Notifications
You must be signed in to change notification settings - Fork 743
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
Improve robustness of Crystal lexer #1644
Improve robustness of Crystal lexer #1644
Conversation
@rymiel I'm not sure if you're in a position to do so but if you could have a peek at this, it'd be much appreciated. I've added a more comprehensive visual sample for the Crystal lexer and tried to fix some of the areas in which the lexer was deficient. |
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.
I am in no way in any authoritative position on this, I simply was annoyed that half of my files were turning bland, though, I read through it from a simple user's perspective, I can't immediately think of any language constructs missing, though I'll hopefully continue to participate on this topic.
@rymiel Cool. Well, I think I'll merge it in sometime tomorrow and if there are any issues, I'm sure they'll be reported soon enough. Thanks for taking a look! |
@pyrmont I tried really hard to break it but the lexer is very robust as of now. Syntax that breaks other lexers like the one used in github doesn't break rouge, however, after trying really hard and going back to what broke it before, regex literals, I've found that nested regex interpolation I haven't investigated the cause yet but mostly I just wanted to say how much I struggled to find any problems. |
@rymiel That's great to hear. I can't really take any credit, the Crystal lexer is very closely based on Rouge's Ruby lexer and I think it's the best around (probably not a surprise for a Ruby lexing library). I'll probably leave the nested regular expressions for now and see if it comes up naturally as an issue. Thanks for giving it such a thorough test! It was really helpful :) |
…#1644) The Crystal lexer does not include a very extensive visual sample. This commit adds a sample based on the one included in the Pygments lexer. As a result of testing with the sample, issues were identified with the lexing of macros and the `{%` and `%}` conditionals. These were fixed.
Crystal's visual sample was not very comprehensive and meant that the lexer did not cover elements of the syntax (notably macros and
%
conditionals).This PR adds a more comprehensive visual sample (based on the one in Pygments) and makes changes to the lexer to fix coverage errors.