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

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

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

angelozerr
Copy link
Contributor

fix: set languageId to have highlight with LSP4IJ 0.7.0

LSP4IJ 0.7.0 will support documentSelector and dynamic capabilities with PR redhat-developer/lsp4ij#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.

Copy link

sonarqubecloud bot commented Oct 7, 2024

@angelozerr angelozerr added this to the 2.1.0 milestone Oct 7, 2024
@angelozerr angelozerr added the bug Something isn't working label Oct 7, 2024
@angelozerr angelozerr self-assigned this Oct 7, 2024
@angelozerr angelozerr merged commit f3946a0 into redhat-developer:main Oct 7, 2024
10 checks passed
@mrglavas
Copy link

mrglavas commented Oct 7, 2024

@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
Copy link
Contributor Author

angelozerr commented Oct 7, 2024

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants