Skip to content
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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 21, 2025
+1 −0
Merged

Improve Rust syntax highlighting #25333

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fd3d1b0
Improve Rust syntax highlighting
chbk Feb 21, 2025
4cbe4c2
Merge branch 'main' into pr/25333
maxdeviant Feb 21, 2025
ae605f9
Remove `@constant.variable`
chbk Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions crates/languages/src/rust/highlights.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
(identifier) @variable
(type_identifier) @type
(primitive_type) @type.builtin
(self) @variable.special
Expand Down Expand Up @@ -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
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

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?

Copy link
Contributor Author

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 and constant.variable
  • or we add literal and literal.builtin scopes for literal values

Copy link
Member

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.

Copy link
Contributor Author

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!

(#match? @constant.variable "^_*[A-Z][A-Z\\d_]*$"))

[
"("
Expand Down
Loading