-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Improve Rust syntax highlighting #25333
Conversation
I think I have made some of these changes in another PR #25338 |
@@ -47,8 +48,8 @@ | |||
(#match? @type "^[A-Z]")) | |||
|
|||
; Assume all-caps names are constants | |||
((identifier) @constant | |||
(#match? @constant "^_*[A-Z][A-Z\\d_]*$")) | |||
((identifier) @constant.variable |
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'm not sure I understand the intent of this change.
Rust only has one kind of constants: constants.
So why do we need to denote them as @constant.variable
and not just @constant
?
This is the only place in the Rust highlights we use @constant
, so what does @constant.variable
give us?
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.
The issue is that we only have two scopes to differentiate three different elements:
constant.builtin
: for builtin constants, or builtin literal values, as in Java.
constant
: for constant variables, as in C, but the concept is similar in Rust.
Then how do we scope other literal values?
My solution to this is to use constant.variable
for constants and constant
for literals.
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.
The issue is that we only have two scopes to differentiate three different elements:
constant.builtin
: for builtin constants, or builtin literal values, as in Java.constant
: for constant variables, as in C, but the concept is similar in Rust.Then how do we scope other literal values?
My solution to this is to use
constant.variable
for constants andconstant
for literals.
Let's get concrete here and provide examples of what would fall into each case.
We already have @constant.builtin
for built-in constants:
undefined
null
And right now in Rust we use @constant
for constants:
const MESSAGE: &str = "Hello";
What is the third type of constant?
I know in TypeScript there are two kinds of "constants" that have different semantic meaning:
const FIVE = 5;
const foo = bar();
Is this what you're referring to?
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 is the conflict I am trying to solve. In Java:
final int x = 3; // x is a constant -> scoped constant
String y = null; // null is a literal -> scoped constant.builtin
Then let's jump to CSS:
bold, red /* literal values -> scoped constant, get same color as constant variables... */
As a solution:
- we differentiate literals from constants respectively with
constant
andconstant.variable
- or we add
literal
andliteral.builtin
scopes for literal values
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.
It seems like that should just be something that is fixed in the CSS highlights?
I don't see why we need to introduce a new @constant.variable
everywhere and use it as opposed to just changing the CSS selectors to use @constant.builtin
(which we already have precedent for), or maybe some other selector that is CSS-specific.
The name constant.variable
just doesn't seem like a good fit either, as if something is constant it is, by definition, not variable. But naming aside, it just seems like we don't need to make this change to the Rust highlights at all.
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 agree the naming is odd, but they are called constant variables in C, so this was my attempt at homogenizing the language highlights. In that case, I will rework the CSS PR. Thank you for your feedback!
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!
This reverts commit 3e75a66.
This PR changes the color used for `@variable` syntax highlights in the Gruvbox themes to be less intense. We now use the same color as `editor.foreground`. | Language | Before | After | | -------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | | Rust | <img width="1410" alt="Screenshot 2025-02-24 at 10 08 41 AM" src="https://github.com/user-attachments/assets/9a34964d-9fdc-4deb-ac30-4a1c9e6fb531" /> | <img width="1410" alt="Screenshot 2025-02-24 at 10 55 18 AM" src="https://github.com/user-attachments/assets/c245d0fd-28af-42b8-93f6-48cb14671d94" /> | | Python | <img width="1410" alt="Screenshot 2025-02-24 at 10 08 38 AM" src="https://github.com/user-attachments/assets/8f8d111e-1d50-4229-a333-eb29b6ce9f4f" /> | <img width="1410" alt="Screenshot 2025-02-24 at 10 55 20 AM" src="https://github.com/user-attachments/assets/010b661e-dc9e-4ccb-8e52-ee10c8eb8342" /> | In #25333 and #25331 the highlight used for identifiers in Rust and Python, respectively, was changed to `@variable`, which resulted in the intense colors you see in the "Before" screenshots above. We considered reverting the highlight query changes to those languages, but after taking a look at our other languages, they already use similar queries. Instead we're adjusting the theme to make these cases less visually intense. Release Notes: - Gruvbox themes: Changed the color used for `@variable` syntax highlights to be less intense.
#25333 added broader highlighting for identifiers, which broke the generic query for attribute queries, resulting in these being highlighted the same as identifiers. To accomodate for this change, this PR updates the attribute matches to be more specific. Additionally, path matches in scoped identifiers are no longer highlighted as attributes, as seen in the comparison screenshot. Can revert this if requested. | Zed Preview | <img width="750" alt="preview" src="https://github.com/user-attachments/assets/2cd2e830-f510-4adf-8ce9-c41ed6fb157c" /> | | --- | --- | | `main` | <img width="750" alt="main" src="https://github.com/user-attachments/assets/cbe93186-9afd-4515-bc06-e519fd4ee6af" /> | | This PR | <img width="750" alt="pr" src="https://github.com/user-attachments/assets/68270de8-e083-4fc6-a45e-25d3151acd87" /> | The generic match for `token_tree` is needed to recursively match patterns like `#[cfg(any(test, feature = "test-support"))]` (or at least I was unable to find a better query here). I tried to validate that this does not break any other highlights and I believe it does not. However, I might have still missed something. Release Notes: - N/A
Release Notes:
identifier
:variable