Fix for CSS unicode-range support, issue #4253. #4254
Conversation
|
Resolves issue #4253 (Adding "Resolves" again because the link didn't function completely the first time.) |
src/languages/lib/css-shared.js
Outdated
| begin: /#(([0-9a-fA-F]{3,4})|(([0-9a-fA-F]{2}){3,4}))\b/ | ||
| begin: /#(([0-9A-F]{3,4})|(([0-9A-F]{2}){3,4}))\b/i | ||
| }, | ||
| UNICODE_RANGE: { | ||
| scope: 'number', | ||
| begin: /\bU\+[0-9A-F][0-9A-F?]{0,4}(-[0-9A-F][0-9A-F]{0,4})?/i |
There was a problem hiding this comment.
We can't use /i... the entire grammar is either case insensitive or not (the engine handles this) - the tags on any individual regex are irrelevent... right now 3 or our CSS grammars are marked case in-sensitive while the 4th is not. So we can either:
- Revert the first, and add case variants to the second
- You could make the argument that Stylus should also be case-insensitive, then we'd get this for free... but AI says that Stylus isn't 100% case insensative... :-/
There was a problem hiding this comment.
Aren't all of these languages case-insensitive on the particular issue of parsing hexadecimal numbers? I'm not familiar with Stylus, but since the existing regex that I modified contained 0-9a-fA-F that made it just as case-insensitive as adding the i flag does.
I can back out the change to the regex that had been there, and make my own regex /\b[Uu]\+[0-9a-fA-F][0-9a-fA-F?]{0,4}(-[0-9a-fA-F][0-9a-fA-F]{0,4})?/ if you prefer, but the functional end result is the same, and doesn't somehow spread case-insensitivity to any other language parsing that occurs elsewhere.
There was a problem hiding this comment.
but the functional end result is the same
It's not.
The /i flag you've added will have no effect. In Stylus right now your /[A-F]/i would be broken. It would NOT match lowercase hex digits. If a grammar is marked insensitive EVERY regex will have the i flag (no need to specify it per regex), but if not NONE of the regex will have the i flag (even if specified, it'll be removed). This is because of how the parsing engine combines rules into one giant regex.
make my own regex...
Yes, because Stylus is still case-sensitive you'll need to add a-f to your regex to make it insensitive.
There was a problem hiding this comment.
...because of how the parsing engine combines rules into one giant regex.
Whoa. That's unexpected. As a suggestion, perhaps a warning in the contributor documentation about that?
I've now fixed this PR to take the case handling/regex combining issue into account.
Added support for CSS unicode-range keyword and associate range arguments.
Resolves issue #4253
Checklist
CHANGES.md