Skip to content

Commit 5d52490

Browse files
sebthomsebthom
committed
fix: eliminate UI freezes in LSPTextHover via async updates
LSPTextHover now implements ITextHoverExtension2 and returns an async input that shows a placeholder immediately and updates when the language server responds. The UI no longer waits for LS replies; getHoverRegion avoids blocking and falls back to a heuristic word-like region when data isn't ready. FocusableBrowserInformationControl swaps in the final HTML and hides/disposes the hover when the server returns no content, errors, or stalls, preventing lingering "Loading..." popups.
1 parent 4ec874a commit 5d52490

4 files changed

Lines changed: 183 additions & 73 deletions

File tree

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.io.IOException;
2222
import java.lang.reflect.Field;
2323
import java.util.Collections;
24+
import java.util.concurrent.TimeUnit;
2425
import java.util.List;
2526
import java.util.concurrent.atomic.AtomicBoolean;
2627

@@ -65,7 +66,8 @@ public void setUp() {
6566

6667
@Test
6768
public void testHoverRegion() throws CoreException {
68-
final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), new Range(new Position(0, 0), new Position(0, 10)));
69+
final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")),
70+
new Range(new Position(0, 0), new Position(0, 10)));
6971
MockLanguageServer.INSTANCE.setHover(hoverResponse);
7072

7173
IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text");
@@ -81,24 +83,30 @@ public void testHoverRegionInvalidOffset() throws CoreException {
8183
IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text");
8284
ITextViewer viewer = TestUtils.openTextViewer(file);
8385

84-
assertEquals(new Region(15, 0), hover.getHoverRegion(viewer, 15));
86+
var region = hover.getHoverRegion(viewer, 15);
87+
assertNotNull(region);
88+
assertTrue("region should include the hover offset",
89+
region.getOffset() <= 15 && (region.getOffset() + region.getLength()) >= 15);
8590
}
8691

8792
@Test
88-
public void testHoverInfo() throws CoreException {
89-
final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), new Range(new Position(0, 0), new Position(0, 10)));
93+
public void testHoverInfo() throws Exception {
94+
final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")),
95+
new Range(new Position(0, 0), new Position(0, 10)));
9096
MockLanguageServer.INSTANCE.setHover(hoverResponse);
9197

9298
IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text");
9399
ITextViewer viewer = TestUtils.openTextViewer(file);
94100

95-
// TODO update test when MARKDOWN to HTML will be finished
96-
assertTrue(hover.getHoverInfo(viewer, new Region(0, 10)).contains("HoverContent"));
101+
String html = hover.getHoverInfoFuture(viewer, new Region(0, 10)).get(2, TimeUnit.SECONDS);
102+
assertNotNull(html);
103+
assertTrue(html.contains("HoverContent"));
97104
}
98105

99106
@Test
100107
public void testHoverInfoEmptyContentList() throws CoreException {
101-
final var hoverResponse = new Hover(Collections.emptyList(), new Range(new Position(0, 0), new Position(0, 10)));
108+
final var hoverResponse = new Hover(Collections.emptyList(),
109+
new Range(new Position(0, 0), new Position(0, 10)));
102110
MockLanguageServer.INSTANCE.setHover(hoverResponse);
103111

104112
IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text");
@@ -119,7 +127,8 @@ public void testHoverInfoInvalidOffset() throws CoreException {
119127

120128
@Test
121129
public void testHoverEmptyContentItem() throws CoreException {
122-
final var hoverResponse = new Hover(List.of(Either.forLeft("")), new Range(new Position(0, 0), new Position(0, 10)));
130+
final var hoverResponse = new Hover(List.of(Either.forLeft("")),
131+
new Range(new Position(0, 0), new Position(0, 10)));
123132
MockLanguageServer.INSTANCE.setHover(hoverResponse);
124133

125134
IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text");
@@ -129,27 +138,28 @@ public void testHoverEmptyContentItem() throws CoreException {
129138
}
130139

131140
@Test
132-
public void testHoverOnExternalFile() throws CoreException, IOException {
141+
public void testHoverOnExternalFile() throws Exception {
133142
final var hoverResponse = new Hover(List.of(Either.forLeft("blah")),
134143
new Range(new Position(0, 0), new Position(0, 0)));
135144
MockLanguageServer.INSTANCE.setHover(hoverResponse);
136145

137146
File file = TestUtils.createTempFile("testHoverOnExternalfile", ".lspt");
138-
ITextViewer viewer = LSPEclipseUtils.getTextViewer(IDE.openInternalEditorOnFileStore(
139-
UI.getActivePage(), EFS.getStore(file.toURI())));
140-
Assert.assertTrue(hover.getHoverInfo(viewer, new Region(0, 0)).contains("blah"));
147+
ITextViewer viewer = LSPEclipseUtils
148+
.getTextViewer(IDE.openInternalEditorOnFileStore(UI.getActivePage(), EFS.getStore(file.toURI())));
149+
String html = hover.getHoverInfoFuture(viewer, new Region(0, 0)).get(2, TimeUnit.SECONDS);
150+
Assert.assertTrue(html != null && html.contains("blah"));
141151
}
142152

143153
@Test
144154
public void testMultipleHovers() throws Exception {
145-
final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), new Range(new Position(0, 0), new Position(0, 10)));
155+
final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")),
156+
new Range(new Position(0, 0), new Position(0, 10)));
146157
MockLanguageServer.INSTANCE.setHover(hoverResponse);
147158

148159
IFile file = TestUtils.createUniqueTestFileMultiLS(project, "HoverRange Other Text");
149160
ITextViewer viewer = TestUtils.openTextViewer(file);
150161

151-
// TODO update test when MARKDOWN to HTML will be finished
152-
String hoverInfo = hover.getHoverInfo(viewer, new Region(0, 10));
162+
String hoverInfo = hover.getHoverInfoFuture(viewer, new Region(0, 10)).get(2, TimeUnit.SECONDS);
153163
int index = hoverInfo.indexOf("HoverContent");
154164
assertNotEquals("Hover content not found", -1, index);
155165
index += "HoverContent".length();
@@ -167,12 +177,12 @@ public void testIntroUrlLink() throws Exception {
167177

168178
IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text");
169179
IEditorPart editorPart = TestUtils.openEditor(file);
170-
180+
171181
waitForAndAssertCondition(5_000, () -> LSPEclipseUtils.getTextViewer(editorPart) != null);
172182
ITextViewer viewer = LSPEclipseUtils.getTextViewer(editorPart);
173183
assertEquals(UI.getActivePart(), editorPart);
174184

175-
String hoverContent = hover.getHoverInfo(viewer, new Region(0, 10));
185+
String hoverContent = hover.getHoverInfoFuture(viewer, new Region(0, 10)).get(2, TimeUnit.SECONDS);
176186

177187
final var hoverManager = new LSPTextHover();
178188

@@ -188,8 +198,8 @@ public void testIntroUrlLink() throws Exception {
188198

189199
wrapperControl = (BrowserInformationControl) hoverManager.getHoverControlCreator()
190200
.createInformationControl(shell);
191-
control = (BrowserInformationControl) wrapperControl
192-
.getInformationPresenterControlCreator().createInformationControl(shell);
201+
control = (BrowserInformationControl) wrapperControl.getInformationPresenterControlCreator()
202+
.createInformationControl(shell);
193203
Field f = BrowserInformationControl.class.getDeclaredField("fBrowser"); //
194204
f.setAccessible(true);
195205

@@ -209,7 +219,7 @@ public void completed(ProgressEvent event) {
209219
});
210220

211221
assertNotNull("Editor should be opened", viewer.getTextWidget());
212-
222+
213223
UI.getActivePage().activate(editorPart);
214224
browser.setText(hoverContent);
215225

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/AsyncHtmlHoverInput.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation.
11+
*******************************************************************************/
12+
package org.eclipse.lsp4e.operations.hover;
13+
14+
import java.util.UUID;
15+
import java.util.concurrent.CompletableFuture;
16+
17+
import org.eclipse.jdt.annotation.Nullable;
18+
19+
/**
20+
* Lightweight carrier for asynchronous hover HTML content.
21+
*
22+
* Holds a placeholder HTML to show immediately and a future that will
23+
* eventually provide the final HTML. The {@link #token} acts as an identity to
24+
* guard UI updates against races when the control input changes quickly.
25+
*/
26+
@SuppressWarnings("javadoc")
27+
final class AsyncHtmlHoverInput {
28+
29+
final UUID token = UUID.randomUUID();
30+
final CompletableFuture<@Nullable String> future;
31+
final String placeholderHtml;
32+
33+
AsyncHtmlHoverInput(CompletableFuture<@Nullable String> future, String placeholderHtml) {
34+
this.future = future;
35+
this.placeholderHtml = placeholderHtml;
36+
}
37+
}

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/FocusableBrowserInformationControl.java‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
package org.eclipse.lsp4e.operations.hover;
1313

1414
import java.net.URL;
15+
import java.util.UUID;
1516

1617
import org.eclipse.core.runtime.FileLocator;
1718
import org.eclipse.core.runtime.Platform;
@@ -62,6 +63,8 @@ public void changed(LocationEvent event) {
6263
}
6364
};
6465

66+
private @Nullable UUID currentAsyncToken;
67+
6568
public FocusableBrowserInformationControl(Shell parent, String symbolicFontName, boolean resizable) {
6669
super(parent, JFaceResources.DEFAULT_FONT, resizable);
6770
}
@@ -155,6 +158,41 @@ private static boolean safeExecute(Browser browser, String expression) {
155158

156159
@Override
157160
public void setInput(@Nullable Object input) {
161+
if (input instanceof AsyncHtmlHoverInput async) {
162+
this.currentAsyncToken = async.token;
163+
super.setInput(styleHtml(async.placeholderHtml));
164+
// Hide automatically if the LS doesn't reply within a reasonable time
165+
UI.getDisplay().timerExec(2_000, () -> {
166+
if (getShell() == null || getShell().isDisposed()) {
167+
return;
168+
}
169+
if (this.currentAsyncToken != null && this.currentAsyncToken.equals(async.token)
170+
&& !async.future.isDone()) {
171+
dispose();
172+
}
173+
});
174+
async.future.whenComplete((html, ex) -> UI.getDisplay().asyncExec(() -> {
175+
if (getShell() == null || getShell().isDisposed()) {
176+
return;
177+
}
178+
final var currentAsyncToken = this.currentAsyncToken;
179+
if (currentAsyncToken != null && !currentAsyncToken.equals(async.token)) {
180+
return; // input changed; ignore stale update
181+
}
182+
if (ex != null) {
183+
LanguageServerPlugin.logError(ex);
184+
dispose();
185+
return;
186+
}
187+
if (html != null && !html.isBlank()) {
188+
super.setInput(styleHtml(html));
189+
} else {
190+
// No content from LS; hide placeholder
191+
dispose();
192+
}
193+
}));
194+
return;
195+
}
158196
if (input instanceof String html) {
159197
input = styleHtml(html);
160198
}
@@ -239,5 +277,4 @@ private static void appendAsHexString(StringBuilder buffer, int intValue) {
239277
}
240278
};
241279
}
242-
243-
}
280+
}

0 commit comments

Comments
 (0)