Neovim client: fix semantic tokens

[aspeddro]

Neovim 0.9 add support for semantic tokens.

When opening a file and editing I get the following error: E5248: Invalid character in group name

I saw that the server registers tokens that are not in the specification: support-type-primitive, jsx-tag and jsx-lowercase.

This PR change name of tokens:

• support-type-primitive -> type
• jsx-tag -> class (class was used as module name, now it's namespace)
• jsx-lowercase -> interface (div, h1)

One question: Why emit a token for jsx-tag, the < and > in <div>? Couldn't this be done via regex?

Forum post: https://forum.rescript-lang.org/t/neovim-vim-rescript-lsp-semantic-tokens-issue-and-maintenance/4387

[cristianoc]

I don't remember all the details on top of my head, but the names were selected after trying many themes.
Now we'd need to know that these changes have no effect on those themes.

[cristianoc]

Can semantic tokens be turned off in neovim until all works?

[aspeddro]

Can semantic tokens be turned off in neovim until all works?

yes

[cristianoc]

I'm afraid this PR will (from memory but I could be wrong) break some themes.

[aspeddro]

It seems not, because semanticTokenScopes maps to the same TextMate Scope.

This changes the name of the token the server emit, but the semanticTokenScopes field didn't change the TextMate scope target.

Example: previously the token jsx-lowercase was mapped to entity.name.tag, I changed jsx-lowercase to interface but it still gets mapped to entity.name.tag

VSCode Semantic token map

Edit 1:

This PR can change colors.

If a theme applied a color to the class token it would highlight the module name, with this PR the color will be different because the module name now uses the namespace token.

[aspeddro]

I'm going to do some tests.

[aspeddro]

Dark (default dark)

Before:

After:

Light (default light)

Before:

After:

Solarized Dark

Before:

After:

Solarized Light

Before:

After:

Monokai Dimmed ⚠️

Before:

After:

Tomorrow Night Blue ⚠️

Before:

After:

One Dark Pro ⚠️

Before:

After:

[aspeddro]

Three themes have changed: Monokai Dimmed, Tomorrow Night Blue and One Dark Pro.

The changes were in the token < and >

Curious that the token </ is not emited.

[cristianoc]

Three themes have changed: Monokai Dimmed, Tomorrow Night Blue and One Dark Pro.

The changes were in the token < and >

Curious that the token </ is not emited.

The token </, if I remember correctly, is handled via regexp, as it's easy to recognise. While < and > are really overloaded and need the help of semantic highlighting.

So based on these findings, is it possible to handle the issue on the neovim side?

[cristianoc]

In general, I just don't know what the constraints are with neovim. One could also do everything via grammar, which is less precise but avoids breaking things. As long as that extra complexity does not affect the main extension.

[aspeddro]

So based on these findings, is it possible to handle the issue on the neovim side?

No, because it is necessary to change the name of the token emited by the server. Ideally, the specification would have a name for the token < and >, but it doesn't.

The best approach is to not emit < and > as jsx_tag.

[cristianoc]

So based on these findings, is it possible to handle the issue on the neovim side?

No, because it is necessary to change the name of the token emited by the server. Ideally, the specification would have a name for the token < and >, but it doesn't.

The best approach is to not emit < and > as jsx_tag.

That breaks things for 98% of users to benefit 2%? Or maybe I don't understand the suggestion.

[aspeddro]

Yes, you are correct.

What I'm saying is that the LSP specification should have a token to jsx.

I will continue emitting the token, but I will change the token type.

[aspeddro]

I changed the token from class to modifier.

One Dark Pro

After:

Tomorrow Night Blue

After:

[cristianoc]

So this seems to solve the issue.
Anything else pending, or can this go ahead?

[cristianoc]

I guess a quick check is needed that the other styles still work.

[aspeddro]

Yes, I tested it on all themes supported by the extension.

[cristianoc]

Yes, I tested it on all themes supported by the extension.

Fantastic. Thank you so much for sorting this out. Pretty subtle stuff.