Organize hover responses in a predictable way for ease of use

[turkeylurkey]

I have a file called microprofile-config.properties and it contains

injected.value2=Injected value2
value=lookup value

com.example.demo.client.Service/mp-rest/url=http://localhost:9081/data/client/service
serviceb.url=http://localhost:9081/data/protected

When I hover over the word "Service" I can get two different results in the IDE. Two different language servers respond and it seems random which result is displayed first. Sometimes an empty page is displayed as page 1 and other content on page 2. Sometimes the page with all the content is first and the blank one is second.

I think the responses should be sorted in some deterministic way so the user can rely on seeing things a certain way. Even if it is as simple as alphabetical order by name of language server it should be consistent over a reasonable period of time. Obviously as language servers start and stop the order could change but it should be consistent while a certain collection of LS are running.

Also, one language server responded with

[Trace - 16:48:07] Received response 'textDocument/hover - (9)' in 6ms.
Result: {
  "contents": {
    "kind": "plaintext",
    "value": ""
  }
}

Fixed ~I think we should check value for null or empty string and not even return a message in that case.~

[angelozerr]

I think we should check value for null or empty string and not even return a message in that case.

It should work like this by using https://github.com/redhat-developer/lsp4ij/blob/c521ff9ca41ae9064be83c4661a1b7b6917df914/src/main/java/com/redhat/devtools/lsp4ij/features/documentation/LSPDocumentationHelper.java#L59

and there is a test about this usecase https://github.com/redhat-developer/lsp4ij/blob/c521ff9ca41ae9064be83c4661a1b7b6917df914/src/test/java/com/redhat/devtools/lsp4ij/features/documentation/TypeScriptHoverTest.java#L115

Are you sure the empty hover comes from an another language server? You mean that your Jakarta LS contribuet to microprofile-config.properties?

Coud youshare your 2 LSP traces withe the two hover.

[aparnamichael]

@angelozerr

These are the traces from the language servers.

Liberty config LS:

[Trace - 15:17:10] Sending request 'textDocument/hover - (13)'. Params: { "textDocument": { "uri": "file:///Users/aparnacmichael/Documents/Devex/Codebase/liberty-tools-intellij/src/test/resources/projects/gradle/sampleGradleMPLSApp/src/main/resources/META-INF/microprofile-config.properties" }, "position": { "line": 5, "character": 9 } }

[Trace - 15:17:10] Received response 'textDocument/hover - (13)' in 1ms. Result: { "contents": { "kind": "plaintext", "value": "" } }

Microprofile LS:

[Trace - 15:17:10] Sending request 'textDocument/hover - (21)'. Params: { "textDocument": { "uri": "file:///Users/aparnacmichael/Documents/Devex/Codebase/liberty-tools-intellij/src/test/resources/projects/gradle/sampleGradleMPLSApp/src/main/resources/META-INF/microprofile-config.properties" }, "position": { "line": 5, "character": 9 } }

[Trace - 15:17:10] Received response 'textDocument/hover - (21)' in 2ms. Result: { "contents": { "kind": "markdown", "value": "io.openliberty.mp.sample.client.Service/mp-rest/url\n\nThe base URL to use for this service, the equivalent of the baseUrl method.\r\nThis property (or /mp-rest/uri) is considered required, however implementations may have other ways to define these URLs/URIs.\n\n Type: java.lang.String\n * Value: http://localhost:9081/data/client/service" }, "range": { "start": { "line": 5, "character": 0 }, "end": { "line": 5, "character": 51 } } }

Video for your reference.

https://github.com/redhat-developer/lsp4ij/assets/114654953/37a7f544-ea5d-4751-9f51-9eefefbf416a

[angelozerr]

Thanks @aparnamichael for your answer.

I don't know when I will have time to study this issue since now I'm very busy with our IJ Quarkus to create a release as soon as some bug fixes will be done.

[turkeylurkey]

It looks like this line should catch it. https://github.com/redhat-developer/lsp4ij/blob/c521ff9ca41ae9064be83c4661a1b7b6917df914/src/main/java/com/redhat/devtools/lsp4ij/features/documentation/LSPDocumentationHelper.java#L76

The test you refer to is for "kind": "markdown", so it is not applicable in this case. Your plaintext test case has a non-empty string.

[angelozerr]

Any PR with test are welcome!

[turkeylurkey]

I think your tests are unit tests and don't hit LSPDocumentationHelper.

Also, we have been using 06/15 and I believe the code from 06/18 fixes the problem with the empty message.

This means we don't have a use case that shows multiple messages for a hover situation but if we did the messages would be in a random order. So the first part of this issue is still in play.

[angelozerr]

If I understand correctly your comment if there are several ls which provide non empty hover, LSP4IJ need to sort them.

[turkeylurkey]

That's right, the user will be more comfortable seeing them in a predictable order.

[angelozerr]

It means that we need to provide an extension point to sort.

I think we should wait for having a concrete usecase to implement that

[turkeylurkey]

That sounds like a good idea but I'd hate for it to become so complex that the basic function is not implemented.

That said, we no longer have a use case so this is not high priority for us.

[angelozerr]

An another simple idea is to sort hover by server name.

That said, we no longer have a use case so this is not high priority for us.

Indeed let's waiting for a concrete usecase to discuss again about that.