Enhanced coloring

[georgewfraser]

This PR builds on #4397 to enhance the existing syntax coloring.

Underline mutable variables

The textmate scope markup.underline underlines identifiers, which is a nice way to make mutable vars stand out:

Italicize static variables

The textmate scope markup.italic italicizes identifiers. Italic = static is a common convention in IDEs like IntelliJ:

[CryZe]

Coincidentally I ran into a similar issue myself in #4399. Is there a reason we don't use field though? That seems more appropriate for Rust. It seems like there's like 10 different terms for the same thing in the protocol and every language server does something completely different.

[georgewfraser]

@CryZe method and field aren't in the list of recommended semantic scopes published by VSCode. That doesn't mean we can't use them, but there needs to be some alignment between language-server-implementers and color-theme-creators. The community needs to align around some common set of scope names, and that list is the main thing we have to go on so far.

[aloucks]

This matches how C++ colors preprocessor directives:

I don't think they need to match. Attributes are annotations, not macros. The only thing they have in common with C pre-processor directives is the # symbol. I think attributes should use meta.attribute.rust.

Wouldn't this PR remove the ability to color attributes differently from keywords? I certainly don't want attributes to look like keywords.

I hope that neither this PR nor the other one are making changes to the TM scopes that reduce the ability to customize the highlighting. You can always build a theme that gives multiple scopes the same color, but you can't use multiple colors if the items have the same scope.

Apologies if I'm coming across a little defensively, but I'm pretty happy with my current syntax coloring and I don't want to see it become impossible due to the changes here.

[georgewfraser]

@aloucks Let's focus on #4397 for the moment, which is designed to be a non-controversial fix that prevents semantic coloring from "fighting" with textmate coloring. I agree with your point that if we do make any changes to scopes, we should make sure to use distinct scopes so it doesn't take away the ability to customize. You might think that rust-analyzer should just not concern itself with VSCode's default rust syntax coloring, but now that it's doing semantic coloring, it is arguably necessary to take "ownership" of the default theme to keep it in sync with semantic colors, or users will see weird changes when they open Rust files.

[matklad]

@georgewfraser could you rebase? Seems like GH is having trouble picking up the correct diff here

[georgewfraser]

@matklad I rebased. I also removed coloring attributes for now. It's potentially controversial, I'll make a new PR where we can have a more extensive discussion about what we could do or not do there.

[matklad]

I definitively love the approach here of lowering semantic scopes to completely wrong text mate scopes to get basically a css class :D

I do think we should do this for .*mutable. I am unsure about static. In Rust, I feel that statics & const are already more the sufficiently called out by SCREAMING_SNAKE_CASE naming convention, so it feels like the case where highlighting just creates meaningless extra difference.

[georgewfraser]

Think of TextMate as a giant mountain of cruft, upon which we are carefully perching an elegant foundation of semantic coloring 😄 The key idea is that we must not let the cruftiness of TextMate infect the semantic scopes---the semantic scopes should all be sensible, and then there's this layer of fallback scopes that translates down to the TextMate craziness.

I think your point is reasonable about statics. Plus, I think it makes sense to make additions slowly and see how people react.

If we're not "using" italic for statics, what do you think about underline vs italic for mutables? Italic is a lot more subtle, which is good or bad depending on your POV.

[matklad]

Definitely unreling italics is too subtle. I've been using myself (I run a cusom theme) and that's what is used in IntellijJ as well.

bors d+

[bors]

✌️ georgewfraser can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

[georgewfraser]

bors r+

[bors]

Merge conflict.

[lnicola]

@georgewfraser do you want to merge this? :-)

[georgewfraser]

bors r+

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript

[Xanewok]

It might be worth doing the same distinction for const/let (let/(let) mut here) as it's done by default in the newest VSCode April update: https://code.visualstudio.com/updates/v1_45#_new-color-for-constants-in-the-default-dark-theme

[georgewfraser]

@Xanewok Right now we're using the const coloring for:

• const
• Enum members

We're using variable coloring for:

• static
• let / let mut
• Parameters
• Fields

[anderejd]

Can the underlines be deactivated?

[georgewfraser]

@anderejd It should be possible...wasn't able to figure it out in a few minutes but will take another look as soon as I can.

[anderejd]

Ok thanks for looking into it. For now I edited the package.json for the vscode extension locally and that seems to work fine for now.

[woody77]

You should be able to override the theme locally. But this particular PR is doing that, by specifying an explicit way of formatting vs. just the semantic markup, and letting the theme do the formatting. I think the better change here would have been for the theme to apply the underlining for mutable vars.

e.g. using this in settings.json until the themes do the same:

"editor.semanticTokenColorCustomizations": {
        "enabled": true,
        "rules": {
            // All-themes colors
            "*.mutable": {
                "fontStyle": "underline"
            },
        },

[anderejd]

Thanks @woody77 ! This works for me and deactivates underlines:

    "editor.semanticTokenColorCustomizations": {
        "enabled": true,
        "rules": {
            "*.mutable": {
                "fontStyle": ""
            },
        }
    }

[georgewfraser]

@woody77 I was trying to follow the guidance from VSCode, where they say if you introduce a semantic token, you should provide a fallback textmate scope:

https://code.visualstudio.com/api/language-extensions/semantic-highlight-guide#semantic-token-scope-map

[woody77]

I agree it should be a fallback scope, but one that doesn't imply formatting, and instead is a more structural (maybe keyword.control.rust?)