Fix vscode #142516 [css] support unicode-range wildcard#264
Fix vscode #142516 [css] support unicode-range wildcard#264aeschli merged 6 commits intomicrosoft:mainfrom
Conversation
|
Thanks a lot @marknn3. I think the proper solution would be to deal with the unicode range in the scanner. Right now it is parsed as an expression Surprisingly, we already have a scanner token type for unicode-range (I don't remember why) |
|
The fact that the current CSS expression parser works at all on unicode-ranges is pure coincidence/luck! For sure the best solution is to create actual unicode-range type tokens. I will look into this to do it properly. |
|
@marknn3 Cool, no rush, and thanks again! |
Having a What I have implemented is a |
|
From looking at https://www.w3.org/TR/CSS21/syndata.html#tokenization I still think this should be done in the scanner. I don't think context is needed |
|
Ok, now I see https://www.w3.org/TR/css-syntax-3/#urange What I don't like with the current proposal is the special handling for I suggest to handle unicode range as a valid term in _parseTermExpression Alternative is to do this in |
I agree, this makes more sense! Will refactor this. Do you agree with the UnicodeRange Node implementation? |
|
Sounds good. |
- In _parseDeclaration we first call _tryParseUnicodeRangeExpr, and when this fails to parse then fallback to _parseExpr. According to the spec (https://www.w3.org/TR/css-syntax-3/#urange-syntax) anything that looks like a unicode-range should be interpreted as a unicode-range. - Same change applied in scssParser.ts. - Added new @font-face tests in scss/parser.tests.ts to cover the changes in scssParser.ts.
|
The introduction of a dedicated _parseFontFaceDeclaration parser would result in a lot of duplicate and error prone code. Instead, I introduced _tryParseUnicodeRangeExpr which is used in _parseDeclaration as follows (in cssParser.ts and scssParser.ts): node.setValue(this._tryParseUnicodeRangeExpr() || this._parseExpr())I think this will address your valid concerns and stays close to the <urange> parsing specs Also added a new @font-face test in scss/parser.test.ts to cover the change in scssParser.ts. A dedicated linter for the misuse of unicode-ranges is IMHO overkill. It would be extremely rare to trigger and quite expensive to detect. |
|
My idea was that But yes, just allowing it a valid expression for all properties is fine too. Just add |
I already did try and implement this idea, but it did break the scss parser, surprisingly. A fix would require a similar (and less obvious) duplication in scssParser. |
|
I pushed a commit that uses the does Unicode range parsing in a special method in the scanner. That's much simpler than trying to do it in the parser. E.g. what was missing before was a check that there are no whitespaces between the tokens. I hope that's ok for you. If you see a problem, please speak up Thanks a lot for your help! |
This will fix microsoft/vscode#142516 : [css] support unicode-range wildcard.
Wildcard characters in @font-face unicode-range did generate a css language syntax error.