-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Support template string inline CSS in JavaScript and TypeScript #32478
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
Conversation
| tree-sitter-c = "0.23" | ||
| tree-sitter-cpp = "0.23" | ||
| tree-sitter-css = "0.23" | ||
| tree-sitter-css = { git = "https://github.com/mantou132/tree-sitter-css", rev = "62bb39a376ee8aa0ceb6cab27474b7c7136913b9" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little question, don't you intend to contribute this change to the upstream? 🤔
tree-sitter/tree-sitter-css@master...mantou132:tree-sitter-css:master
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes:
- Supports ``` at the beginning and end of the document
- Recognize
${}content as comments and avoid recognizing it as a pseudo-class
These are not part of the CSS spec, so I don't think they should be merged upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And due to that, Zed is supposed to rely on someone's upstream forever and maintain it?
Does not sound exciting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any better solution besides modifying tree-sitter-css? Although I've tried creating a PR for tree-sitter-css
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I have thought of it more and more and could not find anything clever than "move it to zed-industries org".
I do no think it's a good solution either, maybe @notpeter have ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, not a tree-sitter or CSS/JS/CSS-in-JS expert and haven't looked at the tree-sitter changes, so don't have any immediate suggestion. I do recall that tree-sitter-markdown has wonky two pass approach where they first parse it with the block grammar, then they specify all the inline injections (think ```json and similar). dunno if that's helpful here, please ignore if not.
I just pruned the zed-industries/tree-sitter-* forks and we're down to 6 (was 11). I'd obviously much rather not fork, especially for something as complex as css/js, but it is always a potential option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a more robust solution would be to make a PR to the js/ts tree sitter to separate the quotes ` from the contents like so (element names made up):
(template-string
"`"
(template-string-contents ... )
"`"
)
That way you can have a more precise injection (this is probably generally useful). Then separately make a PR against the css grammar to play nice with ${} or see if you can make it work with tree-sitter predicates instead
|
It seems that html has the same problem
I submitted pr to support more valid attribute values and has not been merged to the official tree-sitter-html yet |
|
Closing for now based on feedback of not wanting to support a fork of the tree-sitter grammar. |


Closes #29044
Before:
After:
Release Notes: