spring-projects / sts4

The next generation of tooling for Spring Boot, including support for Cloud Foundry manifest files, Concourse CI pipeline definitions, BOSH deployment manifests, and more... - Available for Eclipse, Visual Studio Code, and Theia
https://spring.io/tools
Eclipse Public License 1.0
882 stars 206 forks source link

regular Java document highlighting broken in VSCode when extension is active #1380

Closed martinlippert closed 1 month ago

martinlippert commented 1 month ago

This is the follow-up from https://github.com/redhat-developer/vscode-java/issues/3787 Looks like regular document highlighting in Java source code is broken when the Spring Boot Tools extension is active.

martinlippert commented 1 month ago

Maybe a similar issue as we had with semantic tokens not being merged when coming from different language servers? (just guessing here, came to my mind then looking into this)

martinlippert commented 1 month ago

I looked into this and it seems like returning an empty list of document highlights causing the client to take only that empty list from the spring tools language server into account and ignoring other document highlight results (e.g. from the Java language server).

Fixed with https://github.com/spring-projects/sts4/commit/16f4d0c0140d3b1d063e963ed1e17df088c1b4b2.

But probably worth to report this to VSCode itself and propose that document highlights coming from multiple sources are merged instead of "pick one and ignore the others".

@BoykoAlex, what do you think? Interested in reporting this to VSCode (and maybe proposing a PR for that)?

martinlippert commented 1 month ago

Closing this here already, since the fix seems to work nicely. Nevertheless, we should look into the VSCode side as well @BoykoAlex.

BoykoAlex commented 1 month ago

@martinlippert I agree, yes, will report to VSCode... If I'm to work on the PR I'd start with semantic tokens though. It's been reported a while ago, it has been idle for a long time and it affects us more as we have rather ugly workaround ;-)

BoykoAlex commented 1 month ago

Raised https://github.com/microsoft/vscode/issues/230946 but I would not say we are affected much by this issue... The range is the parameter. If range is inside the query unlikely Java would provide anything. If the cursor is at Java method parameter and we'd like to highlight that parameter in the query then this would be a problem... I feel semantic tokens not merged is far more painful then this...

martinlippert commented 1 month ago

@BoykoAlex Thanks for raising this with the VSCode team. And I agree that the fix that we have in place for now (returning null instead of an empty list) is good enough here. Once the VSCode piece got fixed, we can think about the special case that you mentioned.

And yes, getting the semantic tokens merged is far more important, totally agree.