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

Move semantic tokens to LSP implementation #2000

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

0dinD
Copy link
Contributor

@0dinD 0dinD commented Jun 24, 2021

Requires eclipse-jdtls/eclipse.jdt.ls#1806

This PR simply removes the old client-side code for semantic tokens, which is no longer required when the implementation is moved to LSP in eclipse-jdtls/eclipse.jdt.ls#1806.

I also removed the java.semanticHighlighting.enabled setting, since the extension no longer has control over disabling the semantic tokens provider. But this setting can be substituted with editor.semanticHighlighting.enabled via language-specific editor settings for Java, which should provide the same exact result:

{
    "[java]": {
        "editor.semanticHighlighting.enabled": false
    }
}

Also, I noticed that the wiki page for semantic highlighting is a bit outdated now. Not only is java.semanticHighlighting.enabled not a setting anymore, the prompt to enable or disable semantic highlighting was also removed a while ago. Perhaps it's time to update it? While you're at it, a quick tip is to change the language id for the code block from json to jsonc to get rid of the annoying error highlight on the comments.

@0dinD
Copy link
Contributor Author

0dinD commented Jul 25, 2021

Updated vscode-languageclient to 7.1.0-next.5 to grab microsoft/vscode-languageserver-node@dae62de. For more information, see this comment.

@0dinD
Copy link
Contributor Author

0dinD commented Sep 6, 2021

Not sure why the bot didn't remove the label, because the conflict is fixed now.

OK maybe it just takes a while for it to update.

@Eskibear
Copy link
Contributor

Eskibear commented Sep 8, 2021

LGTM.

  • 7.1.0-next.5 is latest version with several fixes, compared to 7.0.0. And is required to completely resolve flickering issue.
  • java.semanticHighlighting.enabled was introduced when semantic highlighting was unstable, allowing users to disable it. Now since vscode have language-specific setting to control it, it's reasonable to directly leverage it.

@Eskibear
Copy link
Contributor

Eskibear commented Sep 8, 2021

After this PR get merged, we should also update Wiki page.
https://github.com/redhat-developer/vscode-java/wiki/Semantic-Highlighting

@0dinD
Copy link
Contributor Author

0dinD commented Sep 8, 2021

Yes, and also https://github.com/eclipse/eclipse.jdt.ls/wiki/Language-Server-Protocol-support.

I'm also wondering if maybe there should be a wiki page about semantic highlighting in eclipse.jdt.ls as well, now that other clients could start using it over LSP. I was thinking supported token types/modifiers, what they correspond to in Java etc. especially since we have some non-standard ones. The wiki page in this repo could be more VS Code-specific with themeing examples, but also link back to the wiki in eclipse.jdt.ls. What do you think?

It seems like anyone can edit the eclipse.jdt.ls wiki (not sure if that's intentional), so I think I could create a page like that when I have time.

@fbricon
Copy link
Collaborator

fbricon commented Sep 8, 2021

It seems like anyone can edit the eclipse.jdt.ls wiki (not sure if that's intentional), so I think I could create a page like that when I have time.

It's intentional. Please knock yourself out :-)

@fbricon fbricon added this to the Mid September 2021 milestone Sep 8, 2021
@fbricon fbricon merged commit 83eff21 into redhat-developer:master Sep 8, 2021
@fbricon
Copy link
Collaborator

fbricon commented Sep 8, 2021

@0dinD thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants