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

Add Tree-sitter CSS grammar #156

Merged
merged 25 commits into from Oct 30, 2018

Conversation

Projects
None yet
4 participants
@maxbrunsfeld
Contributor

maxbrunsfeld commented Oct 19, 2018

Description of the Change

This PR adds a new CSS grammar that uses the tree-sitter-css parser instead of first-mate. This takes advantage of Atom's built-in support for Tree-sitter which allows for more consistent syntax highlighting, more reliable code folding, the Editor: Select Larger Syntax Node editing command, and better performance.

I tried to make the highlighting pretty similar to the TextMate grammar's highlighting, but there are some differences. Here's a screenshot:

screen shot 2018-10-18 at 8 37 19 pm

Alternate Designs

We could not add a new CSS parser. The strongest reason why I wanted to do this now is that we've already added HTML and JavaScript parsers based on Tree-sitter, and we need a Tree-sitter CSS grammar for highlighting the contents of style tags in HTML. But aside from that, this should make for a better experience editing CSS files.

Benefits

  • Large CSS files will highlight much faster
  • Code folding will work better for CSS
  • CSS inside of <style> tags will be syntax highlighted with Tree-sitter enabled.

Possible Drawbacks

I haven't yet added a less parser, so there might now be a bigger difference between editing .css and .less files in Atom. A less parser would be pretty easy to add. In fact, this parser might already work pretty well for Less, because I already implemented support for nested rule sets, since that feature looks like it's going to be standardized.

TODO

  • A lot of things are given the unstyled class support.constant.property-value.css in textmate. In tree-sitter they are given keyword.other which is styled. Example auto and bold in this css.
.class
{
  margin: 0 auto;
  font-weight:bold;
}
  • svg in the following is given entity.name.function in textmate. Nothing in tree-sitter:
@namespace svg url(http://www.w3.org/2000/svg);
  • The # symbol in a color is given the class punctuation.definition.constant.css which is styled differently in one-dark-ui
p {color: #F2DEDE;}
  • // is parsed as comment. From what I (@Ben3eeE) can tell // is ignored by some browsers but not valid as a comment. And the textmate grammar does not parse // as a comment. See textmate image below:
    image

  • the final semicolon is not nessecary in declaration blocks

div p, #id:first-line {background-color: red; background-style: none}
div p {margin: 0; padding: 1em;}
  • @media only screen parses screen as a syntax error. It should be keyword_query
@media only screen and (min-width : /* 40 */
	320px),
	not print and (max-width: 480px)  /* kek */ and (-webkit-min-device-pixel-ratio /*:*/ : 2),
only speech and (min-width: 10em),  /* wat */     (-webkit-min-device-pixel-ratio: 2) { }
  • unit in shapes are not parsed correctly. All but the last are plain_value. The comment is also plain_value
a{
	shape-outside: circle(20em/*=*/at 50% 50%);
	shape-outside: inset(1em, 1em, 1em, 1em);
}
  • The calc from #147 parses with syntax errors
p {
  font-size: calc(10px + (56 - 10) * ((100vw - 320px) / (1920 - 320))); 
}
  • Negative numbers are not parsed as numbers
.number    { margin:    -456.8px   } /* Negative non-integer <number> */
.number    { margin:    -0.0px     } /* Zero, with a leading - (Though strange, this is an allowed value) */
  • css variables are parsed as property_name. textmate scopes them as variable.css.
element {
  --main-bg-color: brown;
}
element {
  background-color: var(--main-bg-color);
}
  • pseudo selectors are parsed as tag_name. textmate scopes them as entity.other.attribute-name.psuedo-element.css or meta.selector.css
.blockquote-footer::before {
  content: "\2014 \00A0";
}
  • odd is parsed as tag_name. textmate scopes it as support.constant.parity.css
.table-striped tbody tr:nth-of-type(odd) {
  background-color: rgba(0, 0, 0, 0.05);
}

Applicable Issues

Refs atom/atom#18327

/cc @Arcanemagus since you pointed out the issue with <style> tags.
/cc @simurai since you code some CSS in Atom.

@simurai

Noticed some differences in coloring. Maybe ok, considering all the other benefits and something themes might could address.

@maxbrunsfeld

This comment was marked as outdated.

Contributor

maxbrunsfeld commented Oct 25, 2018

/cc @Ben3eeE would you be interested in taking a look at this one, and seeing if you can improve the scopes that I have?

I think there are probably some issues; I didn't spend too much time trying to get them to match.

@Ben3eeE

This comment was marked as resolved.

Member

Ben3eeE commented Oct 26, 2018

@maxbrunsfeld Yeah I'm interested. I will add the issues I find to this comment. It seems like we also need to fix issues in the parser. After I have a list of things to do I will start the work on addressing the issues.

Syntax Errors

  • tag_names and class_names can have numbers. Adding a rule for h1 parses as syntax error
h1   {color: blue;}
@media (max-aspect-ratio: 2/1) {
  div {
    border: 2px solid blue;
  }
}
.class {
  margin: 5%
}
.number 
{
  margin: 10E3px
}
a:hover {
  background-color: gold;
}

Scopes

  • The # symbol in a color is given the class punctuation.definition.constant.css which is styled differently in one-dark-ui
p {color: #F2DEDE;}
  • !important is styled as keyword.other.important in textmate. Now it's just keyword.operator which is unstyled in one-dark-ui
.class {width: 100% !important;}
  • A lot of things are given the unstyled class support.constant.property-value.css in textmate. In tree-sitter they are given keyword.other which is styled. Example auto and bold in this css.
.class
{
  margin: 0 auto;
  font-weight:bold;
}
  • Functions are styled with support.function.[name] in textmate. entity.name.function in tree-sitter
@font-face {
    font-family: 'open_sansextrabold';
    src: url('OpenSans-ExtraBold-webfont.eot');
}
  • The number and unit in 3px are given different classes in textmate. But they are styled the same with one-dark-ui
p {border: 3px solid red;}
  • Keyframes from and to are given the class entity.name.tag when they are not tags
@keyframes important1 {
  from { margin-top: 50px; }
  50%  { margin-top: 150px !important; } /* ignored */
  to   { margin-top: 100px; }
}
@thomasjo

This comment has been minimized.

Member

thomasjo commented Oct 26, 2018

@maxbrunsfeld

This comment was marked as resolved.

Contributor

maxbrunsfeld commented Oct 26, 2018

@Ben3eeE Thanks so much for that checklist of examples! I believe all of the parsing problems have been fixed.

@maxbrunsfeld

This comment was marked as resolved.

Contributor

maxbrunsfeld commented Oct 26, 2018

A lot of things are given the unstyled class support.constant.property-value.css in textmate. In tree-sitter they are given keyword.other which is styled. Example auto and bold in this css.

We should probably roll that back. I'm not sure what I was thinking there.

@Ben3eeE

This comment was marked as resolved.

Member

Ben3eeE commented Oct 26, 2018

Working on the scopes and found some more syntax errors

@namespace url(http://www.w3.org/1999/xhtml);
@namespace svg url(http://www.w3.org/2000/svg);
  • // is being parsed as a comment
@namespace url(http://www.w3.org/1999/xhtml);
@namespace svg url(http://www.w3.org/2000/svg);
  • !important is giving me a syntax error now. It didn't on the previous tree-sitter-css version
.class {width: 100% !important;}
div {
  width: 300px;
  height: 300px;
  background: repeating-linear-gradient(red, orange 50px);
  clip-path: polygon(50% 0%, 60% 40%, 100% 50%, 60% 60%, 50% 100%, 40% 60%, 0% 50%, 40% 40%);
  animation: 4s poly infinite alternate ease-in-out;
  margin: 10px auto;
}

@keyframes poly {
  from {
    clip-path: polygon(50% 0%, 60% 40%, 100% 50%, 60% 60%, 50% 100%, 40% 60%, 0% 50%, 40% 40%);
  }

  to {
    clip-path: polygon(50% 30%, 100% 0%, 70% 50%, 100% 100%, 50% 70%, 0% 100%, 30% 50%, 0% 0%);
  }
}
@Ben3eeE

This comment was marked as resolved.

Member

Ben3eeE commented Oct 26, 2018

We should probably roll that back. I'm not sure what I was thinking there.

I'm not sure how the textmate grammar works with this. Everything is given support.constant.[whatitis].css and some of them are styled some are not (one-dark-syntax).

@Ben3eeE

This comment was marked as resolved.

Member

Ben3eeE commented Oct 26, 2018

Moved the list of things to do to the body of the PR instead of multiple comments and minimized some resolved comments.

@maxbrunsfeld

This comment was marked as resolved.

Contributor

maxbrunsfeld commented Oct 28, 2018

Ok, addressed a couple more issues.

Ben3eeE added some commits Oct 28, 2018

@Ben3eeE

This comment was marked as outdated.

Member

Ben3eeE commented Oct 29, 2018

Added a few commits to fix inconsistencies in scopes compared to the textmate grammar. We are getting very close to looking like textmate now.

I also added a few more issues to the body of the PR.

Side by side comparison using atom-material-syntax:
(Left is this Branch, Right is textmate)
image

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Oct 29, 2018

From what I can this fixes all the open issues related to syntax highlighting in css if we fix the calc issue.

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented Oct 29, 2018

Ok, pushed some more parser improvements and checked some more boxes.

Ben3eeE added some commits Oct 30, 2018

Scope `keyword.operator` in binary_expressions
This prevents a conflict with `nesting_selector` and `universal_selector` that should be scoped as `entity.name.tag` and not `entity.name.tag` + `keyword.operator`.

/cc: @maxbrunsfeld A bit unsure if it's valid syntax to nest anonymous nodes like this but it seems to work.
@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Oct 30, 2018

  • Updated scopes to match the latest tree-sitter-css
  • Confirmed the fixes in the latest tree-sitter-css
  • Added scopes for anonymous nodes that make sense. I skipped a few which have inconsistencies in textmate like ( and left them completely unscoped here.
    image
  • Fixed inconsistent scope of attribute_selectors
  • Fixed so keyword.operator is only scoped in binary_expression because otherwise it would conflict with selectors like * in binary_expression and * the all elements selector

Ben3eeE added some commits Oct 30, 2018

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Oct 30, 2018

svg in the following is given entity.name.function in textmate.

I scoped this as entity.namespace.name because entity.name.function feels wrong. It doesn't match textmate but I didn't want to scope the namespace name as a function name.

Maybe it should be entity.name.namespace though 🤔

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented Oct 30, 2018

Ok, pushed a change to highlight variables. We're now getting into differences that are pretty minor. Do you think this is good to 🚢 ?

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Oct 30, 2018

Yeah :shipit: It's important to get this out to fix highlighting of css in html and javascript.

I'll do another quick check on master after this is merged and open a PR with some minor tweaks if I find anything but I feel I have been quite thorough already so not sure that I will.

@Ben3eeE

Awesome 💯

@@ -1,4 +1,6 @@
'.source.css':
'core':
'useTreeSitterParsers': false

This comment has been minimized.

@maxbrunsfeld

maxbrunsfeld Oct 30, 2018

Contributor

/cc @Ben3eeE - I'm a bit hesitant to release this new parser in a hotfix release, so I've turned it off by default for now, just for CSS. This way, we'll get proper syntax highlighting in style tags in HTML and in tagged template literals with the word styled in JavaScript, which will address some regression that have been reported. But we won't risk introducing any more breakage, since this is a patch release.

This comment has been minimized.

@Ben3eeE

Ben3eeE Oct 30, 2018

Member

Sounds good to me 👍

@maxbrunsfeld maxbrunsfeld merged commit 605a5e0 into master Oct 30, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@maxbrunsfeld maxbrunsfeld deleted the mb-add-tree-sitter-grammar branch Oct 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment