LSP API in 0.7.0, please try it with nighthy build!

[angelozerr]

With @fbricon we decided to merge my big PR #543

You can try by installing nightly-builds and read doc at:

• https://github.com/redhat-developer/lsp4ij/blob/main/docs/LSPApi.md
• https://github.com/redhat-developer/lsp4ij/blob/main/docs/DeveloperGuide.md#get-language-server
• https://github.com/redhat-developer/lsp4ij/blob/main/docs/DeveloperGuide.md#execute-a-command

I had to do a very big refactoring (but if you use LSP4IJ without using LanguageServerAccessor, you should not be impacted for the code compatibility), but I hope I have not broken some LSP features.

Please apologize if there are some regression and please create issues if you find some regression that I will fix it ASAP.

You can find documentation of LSP API at https://github.com/redhat-developer/lsp4ij/blob/main/docs/LSPApi.md
Please note the API could change during the 0.7.0, so if you have some suggestion (rename of some method, etc), we can do that now, but after the release it will harder.

I need to revisit LSP completion feature API because the prefix code completion is very uggly. I need to revisit that, but you can start to override basic method at https://github.com/redhat-developer/lsp4ij/issues?q=is%3Aissue+is%3Aopen+label%3Aapi-customization

I have tried to add proper Javadoc, documentation but it is not perfect, so please create PRs to fix that.

This LSP API should fix almost issues https://github.com/redhat-developer/lsp4ij/issues?q=is%3Aissue+is%3Aopen+label%3Aapi-customization

There are too new methods which helps to get language server and execute command if you want to get your language server orexecute command in a custom view for instance. See:

• https://github.com/redhat-developer/lsp4ij/blob/main/docs/DeveloperGuide.md#get-language-server
• https://github.com/redhat-developer/lsp4ij/blob/main/docs/DeveloperGuide.md#execute-a-command

In other words, please install nightly-builds, play with LSP API and don't hesitate to make PRs to fix Javadoc, documentation, your suggestions for API improvements, Thanks!

[angelozerr]

@CppCXY @InSyncWithFoo @seachicken @KUGDev @AlexWeinstein92 @aparnamichael @MituuZ @turkeylurkey @mrglavas @koxudaxi @FalsePattern @onriv @kristofvb @themartdev please follow this discussion

[InSyncWithFoo]

Congratulations! I revised my review: now it has 5 stars.

[angelozerr]

Thanks so much @InSyncWithFoo but you should try it before :)

Hope you will like the api and hope you will not have some regression.

[FalsePattern]

I've removed the formatter proxy workaround from ZigBrains dev branch, LSPFormattingFeature.isExistingFormatterOverrideable is exactly what I needed for it.
FalsePattern/ZigBrains@948d566

[angelozerr]

Thanks so much for your feedback!

What do you think about this name? Is the javadoc is enough?

[FalsePattern]

The name is good, and I've opened a PR at #562 that makes the javadoc explain the underlying logic a bit more in-depth, the original javadoc was a bit vague and I had to double check the implementation to see what it was used for to know what to actually override and when to return true.
One small nitpick I have is that there should be some sort information on the "preferred way of overriding" on these classes, or just the example code from the documentation as a class-level javadoc so that it's immediately evident what's the "officially preferred" way to use the API without having to look for it in separate documentation files.

[angelozerr]

The name is good, and I've opened a PR at #562 that makes the javadoc explain the underlying logic a bit more in-depth, the original javadoc was a bit vague and I had to double check the implementation to see what it was used for to know what to actually override and when to return true.
One small nitpick I have is that there should be some sort information on the "preferred way of overriding" on these classes, or just the example code from the documentation as a class-level javadoc so that it's immediately evident what's the "officially preferred" way to use the API without having to look for it in separate documentation files.

Is there any chance that you improve your PR to add example like you are suggesting?

[FalsePattern]

Done, I simplified the example from the documentation a bit to make it take up less space, but it still gets the point across.

[angelozerr]

Your PR has been merged. Thanks!

[kdvolder]

Q: How can I add a dependency on the nightly build from my plugin?

I think this question was already answered elsewhere (I saw/read it somewhere)... but it took me a while to find this info again: #543 (comment)

@fbricon did a great work in our IJ Quarkus to use the local LSP4IJ with https://github.com/redhat-developer/intellij-quarkus/blob/main/build.gradle.kts#L149

Might be something to add to the instructions on consuming knightly builds (in case someone wants to compile/test their own plugin code against changed/new apis... they will probably need to know how to do this)

[FalsePattern]

If you use the jetbrains platform plugin version 2.X, you can try doing something similar to how I did it in ZB:
https://github.com/FalsePattern/ZigBrains/blob/18747eca39220dc3178c7d21c682fc34002e2c38/build.gradle.kts
lines 42-53, 86-91, 195-198
tl;dr: com.jetbrains.plugins:com.redhat.devtools.lsp4ij:VERSION for regular builds, nightly.com.jetbrains.plugins:com.redhat.devtools.lsp4ij:VERSION for nightlies, both sourced from the marketplace {} platform repository.
This way, running it in dev also automatically pulls it into the test instance IDE directly from the marketplace.

[InSyncWithFoo]

The comment you are referring to is about using unpublished releases. Published nightly releases can be specified as com.redhat.devtools.lsp4ij:<version>@nightly in gradle.properties (example). A list of such versions can be found here.

[InSyncWithFoo]

Minor nits:

• "Tooltip" is one word. LSPDiagnosticFeature.getToolTip should thus be renamed to getTooltip.
• LSPDiagnosticFeature.createAnnotation's fixes is not annotated with @NotNull. This is probably not intentional, since it is subsequently used in a for loop.
• isEnabled and isSupported's names are a bit confusing.

Overall, good job.

[angelozerr]

Minor nits:

• "Tooltip" is one word. LSPDiagnosticFeature.getToolTip should thus be renamed to getTooltip.

Thanks fixed, please install nighty build

• LSPDiagnosticFeature.createAnnotation's fixes is not annotated with @NotNull. This is probably not intentional, since it is subsequently used in a for loop.

fixed

• isEnabled and isSupported's names are a bit confusing.

I think more and more than isSupported should be never overrided. Internally I need to have an isEnabled which is called before starting the server and isSupported which is called after the start of language server to use the LSP server capability received from the initialize LSP request.

Perhaps a better name, should be hasServerCapability ? @fbricon what do you think about that?

What is your suggestion @InSyncWithFoo ?

Overall, good job.

Thanks! Your sample code helped me a lot to have this kind of API.

[InSyncWithFoo]

Ideally, the names should make it clear when it is called. isEnabled's name is fine, though I'm not sure why it should be passed a PsiFile if all it does is to determine whether a feature of the entire server is enabled. isSupported, on the other hand, should certainly be renamed, perhaps to shouldActOnFile or something like that.

[angelozerr]

Ideally, the names should make it clear when it is called. isEnabled's name is fine, though I'm not sure why it should be passed a PsiFile

It is because a language server can defines several mappings (we have this use in Intellij Quarkus where our language servers map Java and properties file). Having a PsiFile provides the capability to enable the language server for Java files and not for properties file by using the language.

isSupported, on the other hand, should certainly be renamed, perhaps to shouldActOnFile or something like that.

isSupported checks the server capability, so I'm not sure shouldActOnFile is a good name.

[mrglavas]

@angelozerr I have been testing 0.7.0-20241004-071208 today and haven't encountered any new issues so far. As you might be aware, we released Liberty Tools for IntelliJ 24.0.9 last week and that is the first version of our plugin to use LSP4IJ from the Marketplace. Now that we have a release in the field we're now especially interested in a stable API so that it continues to work as new versions of LSP4IJ are released. Thanks.

[angelozerr]

I have been testing 0.7.0-20241004-071208 today and haven't encountered any new issues so far.

Great! Thanks for your feedback.

As you might be aware, we released Liberty Tools for IntelliJ 24.0.9 last week and that is the first version of our plugin to use LSP4IJ from the Marketplace.

It is a very nice news. Congrats!

Could you please create a PR to add Liberty Tools at https://github.com/redhat-developer/lsp4ij?tab=readme-ov-file#who-is-using-lsp4ij please and to help us to promote LSP4IJ add a review at https://plugins.jetbrains.com/plugin/23257-lsp4ij/edit/reviews Thanks!

[angelozerr]

Thanks all for your feedback. I launched the 0.7.0 release, we need to wait for few day for JetBrains approval.