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.

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

Add static modifier to static variables #16933

Conversation

maxwase
Copy link

@maxwase maxwase commented Mar 24, 2024

Fixes #16894

It seems that there is a name collision between SymbolKind::Static and HlMod::Static resulting in a wired looking test diff (static mutable static unsafe), that's why I decided to rename SymbolKind::Static. I don't think that any of these are exposed to a user, however, not fully sure, I was not able to break my configuration with either of these name changes.

Tested with the following config static colors configuration:

    "editor.semanticTokenColorCustomizations": {
        "enabled": true,
        "rules": {
            "variable": "#FFFFFF",
            "variable.static": {
                "bold": true,
                "foreground": "#cc711d",
            },
            "variable.constant": {
                "bold": true,
                "foreground": "#F0F080",
            },
        }
    }

image

// semantic token type : modifiers

// variable : declaration constant
const A: u32 = 0;
// variable : declaration static
static B: u32 = 0;

fn foo() {
    // variable : declaration
    let _c = 0;
}

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 24, 2024
@Veykril
Copy link
Member

Veykril commented Mar 25, 2024

The name collision is fine. I remove the use of the static modifier for static items because I assumed they are used for static members usually (think of static methods in languages like java). It felt somewhat wrong to me to use it for static (as in the item) but arguably its not clear what it should be used for as the LSP spec (given its a standard modifiers) doesn't specify it.

@maxwase
Copy link
Author

maxwase commented Mar 26, 2024

@Veykril I see your point, however I think it is probably ok to have it here, at least for the sake of diversification, without it there is no way to semantically distinguish those

@Veykril
Copy link
Member

Veykril commented Apr 15, 2024

I'm still not too convinced by this being used for static items 😅 I opened #17074 which will add it as an item type instead (and const as well) as that seems more correct to me

@Veykril Veykril closed this Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

static variables do not have static modifier
3 participants