-
Notifications
You must be signed in to change notification settings - Fork 5.9k
languages: Separate control flow keywords for JS/TS/TSX #39801
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
|
We require contributors to sign our Contributor License Agreement, and we don't have @azeier on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
0297bd9 to
87dc78d
Compare
87dc78d to
2f8a9a2
Compare
|
|
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.
Thanks for this!
Regarding yield/await: I think I am leaning against await and somewhat against yield, because both do not really change the control flow in a "classical" way like e.g. return would.
That said, Helix highlights both: https://github.com/helix-editor/helix/blob/97aee4950fd9a08a78415cd8992354ae5cf3aaf0/runtime/queries/rust/highlights.scm#L233-L234
but then, just because they do does not mean we have to.
Given that I merged the other PR as is, I think it is fine to go with this one as is as well and iterate on it as needed.
Hence, thank you for this PR, your added input, big thank you for the very well crafted release notes and - finally - congratulations to your first contribution! 🎉
|
Any theme supporting this as of now? |
|
I suspect it will take a while for themes to pick up on changes like this, but you can override it in your "experimental.theme_overrides": {
"syntax": {
"keyword.control": {
"color": "#ffffff"
}
}
}, |
|
@azeier , hello. If you have a free time, maybe you could create another PR with adding ability to highligh JSX-components besides JSX tags? |
Related to #9461, inspired by #39683
awaitandyieldboth seem somewhat debatable on whether they should be considered the be control flow keywords.For now I went with:
await: no – The control flow effect ofawaitis at a level does not seem relevant for syntax highlighting.yield: yes –yielddirectly affects the output of a generator, and is also included for consistency with Rust (Highlight control flow in Rust/C/C++ #39683).Happy to change these either direction.
Release Notes:
keyword.controlfor control flow keywords likeif,else,return, etc.