Skip to content

fix: Send a single hover request every time the user hovers#1544

Merged
rubenporras merged 1 commit into
eclipse-lsp4e:mainfrom
FlorianKroiss:single-hover-request
Jun 9, 2026
Merged

fix: Send a single hover request every time the user hovers#1544
rubenporras merged 1 commit into
eclipse-lsp4e:mainfrom
FlorianKroiss:single-hover-request

Conversation

@FlorianKroiss

@FlorianKroiss FlorianKroiss commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

I looked at the Eclipse Platform code and the protocol for getHoverRegion and getHoverInfo2 seems to be:

IRegion region = hover.getHoverRegion(viewer, offset);
// [... setup some stuff with region ...]
hover.getHoverInfo2(viewer, region);

Most importantly, the region returned from getHoverRegion is also the one passed back to getHoverInfo2. We can use this to return a custom Region object which holds the request sent to the LS. This way, getHoverInfo2 can simply use that stored request instead of having to do its own bookkeeping to know if the last stored request is the correct one to use.
This also makes sure that exactly one request is sent per getHoverRegion/getHoverInfo2 cycle.

I also added another fast-path to avoid async loading, if the result is already present.

Fixes #1514

…fferent part of the document

I looked at the Eclipse Platform code and the protocol for `getHoverRegion` and `getHoverInfo2` seems to be:
```java
IRegion region = hover.getHoverRegion(viewer, offset);
// setup some stuff with region
hover.getHoverInfo2(viewer, region);
```
So the region returned from `getHoverRegion` is also the one passed back to `getHoverInfo2`.
We can use this to return a custom Region object which holds the request sent to the LS.
This way, `getHoverInfo2` can simply use that stored request instead of having to do its own bookkeeping to know if the last stored request is the correct one to use.

Fixes eclipse-lsp4e#1514
@FlorianKroiss FlorianKroiss requested a review from rubenporras June 7, 2026 08:43
@FlorianKroiss

FlorianKroiss commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

This is the alternative fix for #1514 that I mentioned in #1533 (comment)

@betamaxbandit I incorporated the test cases from your PR which also pass with these changes.

I prefer the changes of this PR because it greatly reduces the complexity / state of LSPTextHover

@FlorianKroiss FlorianKroiss marked this pull request as ready for review June 8, 2026 09:31

@rubenporras rubenporras left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Florian,

thanks for taking a look. Let us try this approach.

Regards

@rubenporras rubenporras merged commit 725ca4e into eclipse-lsp4e:main Jun 9, 2026
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LSPTextHover reuses completed hover result for unchanged region and does not issue a new textDocument/hover request

2 participants