redhat-developer / intellij-quarkus

IntelliJ Quarkus Tools
Eclipse Public License 2.0
120 stars 50 forks source link

fix: set languageId to have highlight with LSP4IJ 0.7.0 #1394

Closed angelozerr closed 1 month ago

angelozerr commented 1 month ago

fix: set languageId to have highlight with LSP4IJ 0.7.0

LSP4IJ 0.7.0 will support documentSelector and dynamic capabilities with PR https://github.com/redhat-developer/lsp4ij/pull/566

MicroProfile LS supports textDocument/documentHighlight with dynamic capability and set the documentSelector for the microprofile-properties languageId:

[Trace - 20:07:06] Received request 'client/registerCapability - (9)'
Params: {
  "registrations": [
    {
      "id": "04793b8a-d455-49f8-8c45-d932f15a1714",
      "method": "textDocument/documentHighlight",
      "registerOptions": {
        "documentSelector": [
          {
            "language": "microprofile-properties"
          }
        ]
      }
    }
  ]
}

It means that textDocument/documentHighlight LSP request must be only sent for "microprofile-properties" language and not for java files which works like this in vscode.

As LSP4IJ 0.7.0 will support "documentSelector", this PR associate the microprofile-config.properties to the "microprofile-properties" to enable highlight for this file. Without this languageId, we will loose highlight when LSP4IJ 0.7.0 will be released.

@aparnamichael @turkeylurkey @mrglavas please do the same think with your Liberty Tools.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

mrglavas commented 1 month ago

@angelozerr Thanks for the heads-up. This is helpful for our future development, but I'm concerned that this is a breaking change in LSP4IJ. We released Liberty Tools for IntelliJ 24.0.9 less than two weeks ago when the current version of LSP4IJ was still 0.5.0 and sounds like in the near future this function will be regressing. Our main concern about adoption of LSP4IJ as a plugin has been stability, particularly with existing releases of our plugin working with the current version of LSP4IJ that will get installed by default from the Marketplace. Is there a way that the change planned for LSP4IJ 0.7.0 can be updated so that it does not have an impact on adopters’ existing plugin releases?

angelozerr commented 1 month ago

@angelozerr Thanks for the heads-up. This is helpful for our future development, but I'm concerned that this is a breaking change in LSP4IJ. We released Liberty Tools for IntelliJ 24.0.9 less than two weeks ago when the current version of LSP4IJ was still 0.5.0 and sounds like in the near future this function will be regressing. Our main concern about adoption of LSP4IJ as a plugin has been stability, particularly with existing releases of our plugin working with the current version of LSP4IJ that will get installed by default from the Marketplace. Is there a way that the change planned for LSP4IJ 0.7.0 can be updated so that it does not have an impact on adopters’ existing plugin releases?

I understand but it is not a regression from LSP4IJ. It is a bug from intellij quarkus and liberty tools.

vscode has this behaviour and LSP4IJ implements now the full spec of LSP registration options.

I have played with other ls like go. Rust and other ls used by adapters and they dont use registerCapabilty to register some lsp feature.

The problem is only for us with mp ls.

The impact is not very big since it just disable highlight which is not very important.

So developping some hack just for this minor feature and just for us is very shame I think.

I wonder if you create on your side a bug fixes release like I did in intellij quarkus, it os doable for you?

You could release now ,(you dont need to wait for 0.7.0 release) since this languageId is used only on IJ client side