Skip to content
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

Gracefully Handle Unknown At-Rules #51

Closed
amiller-gh opened this issue Nov 11, 2017 · 7 comments
Closed

Gracefully Handle Unknown At-Rules #51

amiller-gh opened this issue Nov 11, 2017 · 7 comments
Assignees
Labels
feature-request Request for new features or functionality
Milestone

Comments

@amiller-gh
Copy link
Contributor

Currently, the parser just gives up when it encounters an at-rule it does not understand. This puts the parser into "recovery mode" and can cause some very wonky syntax highlighting until it successfully recovers.

It should be possible to build a generic unknown at-rule parser that will mark the error and gracefully handle any valid at-rule syntax if all other at-rule parsers fail.

@aeschli
Copy link
Contributor

aeschli commented Nov 23, 2017

http://www.w3.org/TR/css-syntax-3/#at-rule has the syntax of atrules

@aeschli aeschli added the feature-request Request for new features or functionality label Nov 23, 2017
@aeschli aeschli added this to the Backlog milestone Nov 23, 2017
@aeschli aeschli added the help wanted Issues identified as good community contribution opportunities label Nov 23, 2017
@aeschli
Copy link
Contributor

aeschli commented Jun 4, 2018

@octref
Copy link
Contributor

octref commented Jun 11, 2018

@aeschli How can this co-exist with validation against wrong at-rules, such as @charse (typo, without the t)?

@octref
Copy link
Contributor

octref commented Jun 11, 2018

It seems the original issue is for being able to use https://github.com/css-modules/postcss-icss-values

css-modules is not a part of W3C CSS spec (not even a proposal). It just adds a bunch of custom syntaxes to standard CSS. I don't think we should support it in css-language-features. If we do, we open doors to supporting the hundreds of postcss plugins that add John Doe's favorite new syntaxes to CSS.

The original issue is about syntax highlighting. @amiller-gh We don't use parser but a TextMate grammar to support syntax highlighting, see: https://code.visualstudio.com/docs/extensions/themes-snippets-colorizers

TextMate grammars are static and cannot highlight incorrect at values, since each at value has its own syntax.

IMO the current support is good enough:

image

@amiller-gh
Copy link
Contributor Author

amiller-gh commented Jun 11, 2018

Current implementation works well enough for single line at-rules (although the prelude highlighting could be a little more fallback-friendly), but it starts getting even little less ideal when you begin using block custom-at-rules:

screen shot 2018-06-11 at 10 29 53 pm

It fails to highlight the prelude, and some of the contents of the block. As far as the CSS grammar parser spec is concerned, it doesn't look like it defines specific keyword identification for at-rules parsing, the CSSOM is what determines what is and isn't a valid at-rule. So I would expect that the grammar to have a nicer fallback for unknown keywords...

Either way – its not the best user experience for those of us using (or writing!) pre-processors that define custom at-rules 😢

@octref
Copy link
Contributor

octref commented Jun 11, 2018

@amiller-gh From https://www.w3.org/TR/css-syntax-3/#consume-an-at-rule it seems possible to update the grammar to highlight @custom-at-rule as keyword.

@aeschli What's your opinion? Adding a new config that toggles semantic checking for all at-rules?

@aeschli
Copy link
Contributor

aeschli commented Jun 12, 2018

Yes, let's add a new validation setting css.lint.unknownAtRules. If set, there should be only single problem reported on the at-keyword.
At any case, the css parser should read over the rest of the rule. If the rule has a body the whole body should be ignored (we shouldn't make any assumptions that it contains declarations or other rules).

For improving the TextMate grammar we can file an issue against https://github.com/atom/language-css

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

3 participants