atom / language-css

CSS package for Atom
Other
59 stars 77 forks source link

Add Tree-sitter CSS grammar #156

Closed maxbrunsfeld closed 6 years ago

maxbrunsfeld commented 6 years ago

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

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

@namespace svg url(http://www.w3.org/2000/svg);

Applicable Issues

Refs https://github.com/atom/atom/issues/18327

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

maxbrunsfeld commented 6 years ago

/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 commented 6 years ago

@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

thomasjo commented 6 years ago

Potentially useful CSS grammar resources:

maxbrunsfeld commented 6 years ago

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

maxbrunsfeld commented 6 years ago

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 commented 6 years ago

Working on the scopes and found some more syntax errors

@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 commented 6 years ago

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 commented 6 years ago

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

maxbrunsfeld commented 6 years ago

Ok, addressed a couple more issues.

Ben3eeE commented 6 years ago

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 commented 6 years ago

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

maxbrunsfeld commented 6 years ago

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

Ben3eeE commented 6 years ago
Ben3eeE commented 6 years ago

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 :thinking:

maxbrunsfeld commented 6 years ago

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 commented 6 years ago

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.