Cannot distinguish between semantic token types and modifiers with the same name

[oxalica]

Result from CocInfo

## versions

vim version: NVIM v0.9.0
node version: v18.16.0
coc.nvim version: 0.0.82-b7375d5f 2023-01-30 05:09:03 +0800
coc.nvim directory: /nix/store/maqb95048mnnn5kl13wl3r01c9bgwk9c-vimplugin-coc.nvim-2023-01-29
term: tmux
platform: linux

Describe the bug

See: rust-lang/rust-analyzer#14795 (comment)

rust-analyzer recently added a macro modifier to all tokens inside macro invocations, which cause them to be all displayed as the same color of the macro itself, because CocSemMacro is defined by default. But this name is originally for token type macro, not modifier macro.

Reproduce the bug

• Install coc-rust-analyzer and a recent rust-analyzer like 2023-05-22 or later

• Create a rust library project with the following content in lib.rs

macro_rules! foo {
    ($item:item) => {
        $item
    };
}

foo! {
    pub struct Foo;
}

• All tokens inside the macro invocation (line 8) are displayed as CocSemMacro highlighting. But should be colored as CocSemKeyword and CocSemStruct.

Output of CocCommand semanticTokens.inspect on the pub token inside macro invocation.

Type: keyword
Modifiers: macro
Highlight group: CocSemMacro

[fannheyward]

Reproduced and got your point. I got some questions about semanticToken highlighting, and I'd like to see your option as you're language server author :D.

From LSP, a token is composed with one token type and n token modifiers, n can be zero.

By default, coc.nvim defined some hi group like CocSemKeyword, here Keyword is token type, coc.nvim didn't handles modifier in default hi groups.

After language server responses tokens, coc.nvim parsed and converted to vim's hi group, in following rules:

1. CocSem + modifier + type
2. CocSem + modifier (I think this is incorrect because a token should must have type
3. CocSem + type

For line-8 pub keyword

1. first goes to CocSemMacroKeyword, no such hi group found in default groups, fail
2. in early version of RA, no modifiers returns, no hi group, in newer release, goes to CocSemMacro
3. in early version, goes to CocSemKeyword, with latest release, we finally got CocSemMacro, this is the issue.

Solutions:

1. defines more hi groups by default. Cause a token can has n modifiers, I think this is hard to define all hi groups
2. with a limit count hi groups defined by default, and change the rules to CocSem + modifier + type for all modifiers, fallback to CocSem + type if no modifiers found, didn't check found or not in default hi groups, leave them for user to define.

In this case, line-8 Foo goes to CocSemDeclarationStruct, CocSemMacroStruct and CocSemPublicStruct, you can define want as you like.

What do you think?

[oxalica]

2. with a limit count hi groups defined by default, and change the rules to CocSem + modifier + type for all modifiers, fallback to CocSem + type if no modifiers found, didn't check found or not in default hi groups, leave them for user to define.

So that's basically removing the rule 2 CocSem + modifier, yes? So this will no longer work:

coc.nvim/doc/coc.txt

Line 3330 in edcc376

hi link CocSemDefaultLibrary TSOtherDefaultLibrary

To me, removing this support and always requiring a token type in hlgroups sound reasonable, since token type is more primary while modifiers are secondary and kinda optional. But not sure how many ones are relying on this feature.

[fannheyward]

removing the rule 2 CocSem + modifier

We should keep this, CocSem + modifier is still useful. We can keep our 3-rules, key point is: don't limit default groups must have the highlight exist, and allow multiple highlight for same token.

• CocSem + type
• CocSem + modifier
• CocSem + modifier + type

[oxalica]

We should keep this, CocSem + modifier is still useful. We can keep our 3-rules, key point is: don't limit default groups must have the highlight exist.

Sorry I don't get it. What do you mean by "don't limit default groups must have the highlight exist"? Making default groups non-existent?

, and allow multiple highlight for same token

If there are multiple highlights, we still need to define priorities yeah? We cannot apply all colors to a single token.

My basic expectation is:

• Tokens with type=Keyword and modifier=[macro], that are macro arguments, should apply CocSemKeyword. Don't use CocSemMacro -- this is coc.nvim currently using.
• Tokens with type=Macro and modifiers=[whatever], that is the macro itself, should apply CocSemMacro.

Thus my suggestion is to either remove rule 2, or swap the priority of rule 2&3.

[fannheyward]

What do you mean by "don't limit default groups must have the highlight exist"?

In current implementation, coc.nvim defined default groups in vim file as you see (CocSemKeyword/CocSemFunction etc). After LS returns tokens, coc.nvim creates hi group with rules 1/2/3, then checks the group exists in highlightGroups or not, only use it as found in default groups, otherwise, no hi group returns for the token.

Removing this limit, a hi group can be created even if not defined, user can define the missing group.

nvim-lsp's rule:

• @lsp.type.<type>.<ft> for the type
• @lsp.mod.<mod>.<ft> for each modifier
• @lsp.typemod.<type>.<mod>.<ft> for each modifier

suggestion is to either remove rule 2, or swap the priority of rule 2&3.

New rules:

• CocSem + type
• CocSem + modifier
• CocSem + modifier + type

[oxalica]

Removing this limit, a hi group can be created even if not defined, user can define the missing group.

My issue is not about user customization. Sending all groups instead of one does NOT help here. It's only causing more unnecessary computation and updates.

New rules:

* `CocSem + type`

* `CocSem + modifier`

* `CocSem + modifier + type`

This sill collides then type==modifier. I only want to define for type=macro, NOT modifier=macro. What should I write? highlight CocSemMacro ... defines both type=macro and modifier=macro, which is wrong.

[fannheyward]

@oxalica sorry but I was completely forcing my attention on the default hi groups limitation, and totally ignored the priority issue you already pointed out.

I checked nvim-lsp's rules, how about this?

• CocSemType + type
• CocSemMod + modifier
• CocSemTypeMod + type + modifier

The CocSemTypeMacro is what you want.

My preview fix removed the limitation to allow to create hi groups, one or multiple signs for one token. Yes, this may bring more updating.

[oxalica]

I checked nvim-lsp's rules, how about this?

* `CocSemType + type`

* `CocSemMod + modifier`

* `CocSemTypeMod + type + modifier`

The CocSemTypeMacro is what you want.

This should work. 👍 Just that it breaks users' current configurations. If coc.nvim allows such a breaking change, it's okay to me.

[fannheyward]

one or multiple signs for one token

This bring some flicking on CursorMoved:

1. applies multiple signs to one token
2. onCursorMoved, coc will highlight visible range/regions
3. requests range or full tokens from LS
4. diffHighlights with current visible start/end
5. applies diff highlights

step 4 is the issue: there will still be new added highlights even the document is not modified, this makes the flicking.

Checking on the diffHighlights logic.

[oxalica]

What's the status of this issue? I saw 9ae8651 is in dev branch but still not reach master.

[fannheyward]

@oxalica dev branch is just rebase master and some unmerged PRs, for unable to merge to master usage.

#4667 brought flicking issue #4659 (comment), maybe this is not a good solution or needs more improvement.

sorry for the slow response.

[oxalica]

#4667 brought flicking issue #4659 (comment), maybe this is not a good solution or needs more improvement.

I don't really think it's blocking hlgroup renaming. For this particular issue, I just want CocSemType*, CocSemMod*, CocSemTypeMod* hlgroups so that I can highlight them differently.

I guess the flicking issue is about 0cb6074#diff-610be46d48af22f1bc73dff66ee66e288671c57f1130353bc88d3fc65379eadbR241 which add multiple highlights on the same token. Will it work if we keep the old code (if + for find the first one) to only push one highlight, just rename the hlgroup names?

[fannheyward]

@oxalica thank you for your help, will check #4667 later this week, a little busy recently.

[fannheyward]

Will it work if we keep the old code (if + for find the first one) to only push one highlight

I have a new thought:

1. has tokenModifiers => CocSemTypeMod*, for multiple modifiers, only use first one to avoid flicking
2. if not modifiers => CocSemType*, as the fallback one
3. no more CocSemMod*

back to your code, struct goes to CocSemTypeModKeywordMacro. What do you think?

#4667 has updated, in my tests, no more flicking.

I'm digging vscode source to find how vscode handle this...

[oxalica]

Thanks for the update.

I have a new thought:

1. has `tokenModifiers` => `CocSemTypeMod*`, for multiple modifiers, only use first one to avoid flicking

2. if not modifiers => `CocSemType*`, as the fallback one

3. no more `CocSemMod*`

I personally think CocSemMod* is still useful in some situations, but mostly can be replaced with TypeMod* by listing all types using that modifier. I do use builtins modifier in nil LSP and want to highlight all token types with that modifier with the same color. I don't have a strong opinion on this.

back to your code, struct goes to CocSemTypeModKeywordMacro. What do you think?

What I want is CocSemTypeMacro, which is already included in the PR. It works for me.

[fannheyward]

What I want is CocSemTypeMacro

struct won't go to this, RA returns this:

tokenType: keyword
tokenModifiers: [ 'macro' ]

[oxalica]

What I want is CocSemTypeMacro

struct won't go to this, RA returns this:

tokenType: keyword
tokenModifiers: [ 'macro' ]

No. I DO NOT want the struct in screenshot to be highlighted as macro, it should be CocSemTypeKeyword. I only expect the foo! (type="macro") to be highlighted CocSemTypeMacro.

[fannheyward]

@oxalica fixed