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

Semantic string tokens vs injected syntax #7111

Closed
mitsuhiko opened this issue Dec 31, 2020 · 5 comments · Fixed by #8795
Closed

Semantic string tokens vs injected syntax #7111

mitsuhiko opened this issue Dec 31, 2020 · 5 comments · Fixed by #8795
Labels
E-easy E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now

Comments

@mitsuhiko
Copy link

I'm currently trying to inject some syntax into string literals by the use of the injectTo grammar feature. The idea is that I match on some specifically formed strings to provide custom syntax highlighting for specific strings in macros (mitsuhiko/insta#149).

Configuration wise my extension does something like this:

    "grammars": [
      {
        "language": "insta-snapshots",
        "scopeName": "source.insta-snapshots",
        "path": "./syntaxes/insta-snapshots.tmLanguage.json"
      },
      {
        "scopeName": "source.inline-insta-snapshots",
        "injectTo": [
          "source.rust"
        ],
        "path": "./syntaxes/inline-insta-snapshots.tmLanguage.json",
        "embeddedLanguages": {
          "meta.embedded.inline-insta-snapshot": "insta-snapshots"
        }
      }
    ]

Then in the grammar I'm matching on something. This all works and eventually I end up injecting a custom sub syntax into strings that look like @r"""...""". The issue here now is that once RLS runs a "string" semantic token is put over the entire region which I just parsed which completely removes my custom syntax again.

I was looking into how this is supposed to be solved or how other languages are doing it but the only thing I found is that apparently rust analyzer is the only language server which emits semantic string tokens? At least I do not see similar things in typescript and some other languages I tried.

Not sure if filing this here makes any sense but considering there are many things working together I figured I start filing something here.

The following screenshot shows the issue:

image

The token under the cursor is correctly determined to be keyword.insta but the styling in the theme is discarded because of the string semantic token which takes precedence.

@mitsuhiko
Copy link
Author

I filed this against vscode now since I think this is more likely to be an issue there: microsoft/vscode#113640

@mitsuhiko
Copy link
Author

The response in the vscode issue is effectively that this would require coordination with RLS:

For this particular case, I think you could try to coordinate with the rust extension / rust semantic tokens provider implementation to give you an option or an API that would prevent the semantic tokens provider from covering those strings. Also, if the semantic tokens provider just creates string tokens, (which would be equal with the tokens created by TM), then they could simply stop creating them altogether without any visible effects.

@lnicola lnicola added C-Architecture Big architectural things which we need to figure up-front (or suggestions for rewrites :0) ) E-hard S-unactionable Issue requires feedback, design decisions or is blocked on other work labels Jan 18, 2021
@matklad
Copy link
Member

matklad commented Jan 18, 2021

Yeah, I think it makes sense to enable a config flag here, to suppress semantic tokens for strings or all tokens. Config value should be declared here:

https://github.com/rust-analyzer/rust-analyzer/blob/9daba961f236750c3a5d831c9775606271b37eff/crates/rust-analyzer/src/config.rs#L28-L184

Rather than non-producing the tokens, we should filter them out in the lsp layer, over here:

https://github.com/rust-analyzer/rust-analyzer/blob/c72d3a7c0989e63a9b063fed445cbbaf3e40a29f/crates/rust-analyzer/src/to_proto.rs#L336-L363

See this parameter to learn how to thread config between two places.

@matklad matklad added E-easy E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now and removed C-Architecture Big architectural things which we need to figure up-front (or suggestions for rewrites :0) ) E-hard S-unactionable Issue requires feedback, design decisions or is blocked on other work labels Jan 18, 2021
@djrenren
Copy link
Contributor

djrenren commented May 7, 2021

Hey @matklad, that "this parameter" link seems to be another link to the semantic_tokens function. Did you mean to point to something else?

@matklad
Copy link
Member

matklad commented May 7, 2021

Yeah, I think https://github.com/rust-analyzer/rust-analyzer/blob/c72d3a7c0989e63a9b063fed445cbbaf3e40a29f/crates/rust-analyzer/src/to_proto.rs#L462 this is what I wanted to link here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants