diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java index a76badf64..17bab197f 100644 --- a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java +++ b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.lang.reflect.Field; import java.util.Collections; +import java.util.concurrent.TimeUnit; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -65,7 +66,8 @@ public void setUp() { @Test public void testHoverRegion() throws CoreException { - final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), new Range(new Position(0, 0), new Position(0, 10))); + final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), + new Range(new Position(0, 0), new Position(0, 10))); MockLanguageServer.INSTANCE.setHover(hoverResponse); IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text"); @@ -81,24 +83,30 @@ public void testHoverRegionInvalidOffset() throws CoreException { IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text"); ITextViewer viewer = TestUtils.openTextViewer(file); - assertEquals(new Region(15, 0), hover.getHoverRegion(viewer, 15)); + var region = hover.getHoverRegion(viewer, 15); + assertNotNull(region); + assertTrue("region should include the hover offset", + region.getOffset() <= 15 && (region.getOffset() + region.getLength()) >= 15); } @Test - public void testHoverInfo() throws CoreException { - final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), new Range(new Position(0, 0), new Position(0, 10))); + public void testHoverInfo() throws Exception { + final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), + new Range(new Position(0, 0), new Position(0, 10))); MockLanguageServer.INSTANCE.setHover(hoverResponse); IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text"); ITextViewer viewer = TestUtils.openTextViewer(file); - // TODO update test when MARKDOWN to HTML will be finished - assertTrue(hover.getHoverInfo(viewer, new Region(0, 10)).contains("HoverContent")); + String html = hover.getHoverInfoFuture(viewer, new Region(0, 10)).get(2, TimeUnit.SECONDS); + assertNotNull(html); + assertTrue(html.contains("HoverContent")); } @Test public void testHoverInfoEmptyContentList() throws CoreException { - final var hoverResponse = new Hover(Collections.emptyList(), new Range(new Position(0, 0), new Position(0, 10))); + final var hoverResponse = new Hover(Collections.emptyList(), + new Range(new Position(0, 0), new Position(0, 10))); MockLanguageServer.INSTANCE.setHover(hoverResponse); IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text"); @@ -119,7 +127,8 @@ public void testHoverInfoInvalidOffset() throws CoreException { @Test public void testHoverEmptyContentItem() throws CoreException { - final var hoverResponse = new Hover(List.of(Either.forLeft("")), new Range(new Position(0, 0), new Position(0, 10))); + final var hoverResponse = new Hover(List.of(Either.forLeft("")), + new Range(new Position(0, 0), new Position(0, 10))); MockLanguageServer.INSTANCE.setHover(hoverResponse); IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text"); @@ -129,27 +138,28 @@ public void testHoverEmptyContentItem() throws CoreException { } @Test - public void testHoverOnExternalFile() throws CoreException, IOException { + public void testHoverOnExternalFile() throws Exception { final var hoverResponse = new Hover(List.of(Either.forLeft("blah")), new Range(new Position(0, 0), new Position(0, 0))); MockLanguageServer.INSTANCE.setHover(hoverResponse); File file = TestUtils.createTempFile("testHoverOnExternalfile", ".lspt"); - ITextViewer viewer = LSPEclipseUtils.getTextViewer(IDE.openInternalEditorOnFileStore( - UI.getActivePage(), EFS.getStore(file.toURI()))); - Assert.assertTrue(hover.getHoverInfo(viewer, new Region(0, 0)).contains("blah")); + ITextViewer viewer = LSPEclipseUtils + .getTextViewer(IDE.openInternalEditorOnFileStore(UI.getActivePage(), EFS.getStore(file.toURI()))); + String html = hover.getHoverInfoFuture(viewer, new Region(0, 0)).get(2, TimeUnit.SECONDS); + Assert.assertTrue(html != null && html.contains("blah")); } @Test public void testMultipleHovers() throws Exception { - final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), new Range(new Position(0, 0), new Position(0, 10))); + final var hoverResponse = new Hover(List.of(Either.forLeft("HoverContent")), + new Range(new Position(0, 0), new Position(0, 10))); MockLanguageServer.INSTANCE.setHover(hoverResponse); IFile file = TestUtils.createUniqueTestFileMultiLS(project, "HoverRange Other Text"); ITextViewer viewer = TestUtils.openTextViewer(file); - // TODO update test when MARKDOWN to HTML will be finished - String hoverInfo = hover.getHoverInfo(viewer, new Region(0, 10)); + String hoverInfo = hover.getHoverInfoFuture(viewer, new Region(0, 10)).get(2, TimeUnit.SECONDS); int index = hoverInfo.indexOf("HoverContent"); assertNotEquals("Hover content not found", -1, index); index += "HoverContent".length(); @@ -167,12 +177,12 @@ public void testIntroUrlLink() throws Exception { IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text"); IEditorPart editorPart = TestUtils.openEditor(file); - + waitForAndAssertCondition(5_000, () -> LSPEclipseUtils.getTextViewer(editorPart) != null); ITextViewer viewer = LSPEclipseUtils.getTextViewer(editorPart); assertEquals(UI.getActivePart(), editorPart); - String hoverContent = hover.getHoverInfo(viewer, new Region(0, 10)); + String hoverContent = hover.getHoverInfoFuture(viewer, new Region(0, 10)).get(2, TimeUnit.SECONDS); final var hoverManager = new LSPTextHover(); @@ -188,8 +198,8 @@ public void testIntroUrlLink() throws Exception { wrapperControl = (BrowserInformationControl) hoverManager.getHoverControlCreator() .createInformationControl(shell); - control = (BrowserInformationControl) wrapperControl - .getInformationPresenterControlCreator().createInformationControl(shell); + control = (BrowserInformationControl) wrapperControl.getInformationPresenterControlCreator() + .createInformationControl(shell); Field f = BrowserInformationControl.class.getDeclaredField("fBrowser"); // f.setAccessible(true); @@ -209,7 +219,7 @@ public void completed(ProgressEvent event) { }); assertNotNull("Editor should be opened", viewer.getTextWidget()); - + UI.getActivePage().activate(editorPart); browser.setText(hoverContent); diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/AsyncHtmlHoverInput.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/AsyncHtmlHoverInput.java new file mode 100644 index 000000000..09d2fd65d --- /dev/null +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/AsyncHtmlHoverInput.java @@ -0,0 +1,37 @@ +/******************************************************************************* + * Copyright (c) 2025 Vegard IT GmbH and others. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Sebastian Thomschke (Vegard IT GmbH) - initial implementation. + *******************************************************************************/ +package org.eclipse.lsp4e.operations.hover; + +import java.util.UUID; +import java.util.concurrent.CompletableFuture; + +import org.eclipse.jdt.annotation.Nullable; + +/** + * Lightweight carrier for asynchronous hover HTML content. + * + * Holds a placeholder HTML to show immediately and a future that will + * eventually provide the final HTML. The {@link #token} acts as an identity to + * guard UI updates against races when the control input changes quickly. + */ +@SuppressWarnings("javadoc") +final class AsyncHtmlHoverInput { + + final UUID token = UUID.randomUUID(); + final CompletableFuture<@Nullable String> future; + final String placeholderHtml; + + AsyncHtmlHoverInput(CompletableFuture<@Nullable String> future, String placeholderHtml) { + this.future = future; + this.placeholderHtml = placeholderHtml; + } +} diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/FocusableBrowserInformationControl.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/FocusableBrowserInformationControl.java index 4a391922c..5dbf80526 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/FocusableBrowserInformationControl.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/FocusableBrowserInformationControl.java @@ -8,10 +8,12 @@ * * Contributors: * Mickael Istria (Red Hat Inc.) - initial implementation + * Sebastian Thomschke (Vegard IT GmbH) - Prevent UI freezes through non-blocking hover rendering *******************************************************************************/ package org.eclipse.lsp4e.operations.hover; import java.net.URL; +import java.util.UUID; import org.eclipse.core.runtime.FileLocator; import org.eclipse.core.runtime.Platform; @@ -62,6 +64,8 @@ public void changed(LocationEvent event) { } }; + private @Nullable UUID currentAsyncToken; + public FocusableBrowserInformationControl(Shell parent, String symbolicFontName, boolean resizable) { super(parent, JFaceResources.DEFAULT_FONT, resizable); } @@ -155,6 +159,31 @@ private static boolean safeExecute(Browser browser, String expression) { @Override public void setInput(@Nullable Object input) { + if (input instanceof AsyncHtmlHoverInput async) { + this.currentAsyncToken = async.token; + super.setInput(styleHtml(async.placeholderHtml)); + async.future.whenComplete((html, ex) -> UI.getDisplay().asyncExec(() -> { + if (getShell() == null || getShell().isDisposed()) { + return; + } + final var currentAsyncToken = this.currentAsyncToken; + if (currentAsyncToken != null && !currentAsyncToken.equals(async.token)) { + return; // input changed; ignore stale update + } + if (ex != null) { + LanguageServerPlugin.logError(ex); + dispose(); + return; + } + if (html != null && !html.isBlank()) { + super.setInput(styleHtml(html)); + } else { + // No content from LS; hide placeholder + dispose(); + } + })); + return; + } if (input instanceof String html) { input = styleHtml(html); } @@ -239,5 +268,4 @@ private static void appendAsHexString(StringBuilder buffer, int intValue) { } }; } - -} \ No newline at end of file +} diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java index d338b62d1..9470f4c18 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java @@ -12,6 +12,7 @@ * Angelo Zerr - Bug 525602 - LSBasedHover must check if LS have codelens capability * Lucas Bullen (Red Hat Inc.) - [Bug 517428] Requests sent before initialization * Alex Boyko (VMware) - [Bug 566164] fix for NPE in LSPTextHover + * Sebastian Thomschke (Vegard IT GmbH) - Prevent UI freezes through non-blocking hover rendering *******************************************************************************/ package org.eclipse.lsp4e.operations.hover; @@ -40,12 +41,12 @@ import org.eclipse.jface.text.IRegion; import org.eclipse.jface.text.ITextHover; import org.eclipse.jface.text.ITextHoverExtension; +import org.eclipse.jface.text.ITextHoverExtension2; import org.eclipse.jface.text.ITextViewer; import org.eclipse.jface.text.Region; import org.eclipse.lsp4e.LSPEclipseUtils; import org.eclipse.lsp4e.LanguageServerPlugin; import org.eclipse.lsp4e.LanguageServers; -import org.eclipse.lsp4e.internal.CancellationUtil; import org.eclipse.lsp4j.Hover; import org.eclipse.lsp4j.HoverParams; import org.eclipse.lsp4j.MarkedString; @@ -57,12 +58,11 @@ /** * LSP implementation of {@link org.eclipse.jface.text.ITextHover} - * */ @SuppressWarnings("restriction") -public class LSPTextHover implements ITextHover, ITextHoverExtension { +public class LSPTextHover implements ITextHover, ITextHoverExtension, ITextHoverExtension2 { - private static final int GET_TIMEOUT_MS = 1000; + private static final int GET_HOVER_REGION_TIMEOUT_MS = 100; private @Nullable IRegion lastRegion; private @Nullable ITextViewer lastViewer; @@ -71,32 +71,36 @@ public class LSPTextHover implements ITextHover, ITextHoverExtension { @Override public @Nullable String getHoverInfo(ITextViewer textViewer, IRegion hoverRegion) { - hoverInfoFuture = getHoverInfoFuture(textViewer, hoverRegion); - try { - return hoverInfoFuture.get(GET_TIMEOUT_MS, TimeUnit.MILLISECONDS); - } catch (ExecutionException e) { - if (!CancellationUtil.isRequestCancelledException(e)) { + // Non-blocking: only return immediately available content. + final var hoverInfoRequest_ = this.hoverInfoFuture = getHoverInfoFuture(textViewer, hoverRegion); + if (hoverInfoRequest_.isDone()) { + try { + return hoverInfoRequest_.getNow(null); + } catch (Exception e) { LanguageServerPlugin.logError(e); } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } catch (TimeoutException e) { - LanguageServerPlugin.logWarning("Could not get hover information due to timeout after " + GET_TIMEOUT_MS + " milliseconds"); //$NON-NLS-1$ //$NON-NLS-2$ } return null; } + @Override + public @Nullable Object getHoverInfo2(ITextViewer textViewer, IRegion hoverRegion) { + final var hoverInfoRequest_ = this.hoverInfoFuture = getHoverInfoFuture(textViewer, hoverRegion); + final String placeholder = "Loading…"; //$NON-NLS-1$ + return new AsyncHtmlHoverInput(hoverInfoRequest_, placeholder); + } + public CompletableFuture<@Nullable String> getHoverInfoFuture(ITextViewer textViewer, IRegion hoverRegion) { if (this.request == null || !textViewer.equals(this.lastViewer) || !hoverRegion.equals(this.lastRegion)) { initiateHoverRequest(textViewer, hoverRegion.getOffset()); } return castNonNull(request).thenApply(hoversList -> { - String result = hoversList.stream() - .filter(Objects::nonNull) - .map(LSPTextHover::getHoverString) - .filter(Objects::nonNull) - .collect(Collectors.joining("\n\n")) //$NON-NLS-1$ - .trim(); + String result = hoversList.stream() // + .filter(Objects::nonNull) // + .map(LSPTextHover::getHoverString) // + .filter(Objects::nonNull) // + .collect(Collectors.joining("\n\n")) //$NON-NLS-1$ + .trim(); if (!result.isEmpty()) { Parser parser = Parser.builder().build(); Node document = parser.parse(result); @@ -145,29 +149,32 @@ public class LSPTextHover implements ITextHover, ITextHoverExtension { || offset < lastRegion.getOffset() || offset > lastRegion.getOffset() + lastRegion.getLength()) { initiateHoverRequest(textViewer, offset); } + + final IDocument document = textViewer.getDocument(); + if (document == null) { + return null; + } + try { - final IDocument document = textViewer.getDocument(); - if (document == null) { - return null; - } final var oneHoverAtLeast = new boolean[] { false }; final var regionStartOffset = new int[] { 0 }; final var regionEndOffset = new int[] { document.getLength() }; - castNonNull(this.request).get(GET_TIMEOUT_MS, TimeUnit.MILLISECONDS).stream() - .filter(Objects::nonNull) - .map(Hover::getRange) - .filter(Objects::nonNull) - .forEach(range -> { - try { + // Wait shortly for hover region result, fallback to heuristics if LS is laggy + castNonNull(this.request).get(GET_HOVER_REGION_TIMEOUT_MS, TimeUnit.MILLISECONDS).stream() // + .filter(Objects::nonNull) // + .map(Hover::getRange) // + .filter(Objects::nonNull) // + .forEach(range -> { + try { regionStartOffset[0] = Math.max(regionStartOffset[0], LSPEclipseUtils.toOffset(range.getStart(), document)); regionEndOffset[0] = Math.min(regionEndOffset[0], LSPEclipseUtils.toOffset(range.getEnd(), document)); - oneHoverAtLeast[0] = true; - } catch (BadLocationException e) { - LanguageServerPlugin.logError(e); - } - }); + oneHoverAtLeast[0] = true; + } catch (BadLocationException e) { + LanguageServerPlugin.logError(e); + } + }); if (oneHoverAtLeast[0]) { this.lastRegion = new Region(regionStartOffset[0], regionEndOffset[0] - regionStartOffset[0]); return this.lastRegion; @@ -177,10 +184,38 @@ public class LSPTextHover implements ITextHover, ITextHoverExtension { } catch (InterruptedException e) { Thread.currentThread().interrupt(); } catch (TimeoutException e) { - LanguageServerPlugin.logWarning("Could not get hover region due to timeout after " + GET_TIMEOUT_MS + " milliseconds"); //$NON-NLS-1$ //$NON-NLS-2$ + LanguageServerPlugin.logWarning( + "Could not get hover region due to timeout after " + GET_HOVER_REGION_TIMEOUT_MS + " milliseconds"); //$NON-NLS-1$ //$NON-NLS-2$ } - this.lastRegion = new Region(offset, 0); - return this.lastRegion; + + // Fallback to heuristic region without blocking. + final Region heuristic = computeHeuristicRegion(document, offset); + this.lastRegion = heuristic; + return heuristic; + } + + private static Region computeHeuristicRegion(final IDocument document, final int offset) { + try { + final int length = document.getLength(); + if (offset < 0 || offset > length) { + return new Region(Math.max(0, Math.min(offset, length)), 0); + } + int start = offset; + int end = offset; + while (start > 0 && isWordPart(document.getChar(start - 1))) { + start--; + } + while (end < length && isWordPart(document.getChar(end))) { + end++; + } + return new Region(start, Math.max(0, end - start)); + } catch (final BadLocationException ex) { + return new Region(offset, 0); + } + } + + private static boolean isWordPart(char c) { + return Character.isLetterOrDigit(c) || c == '_' || c == '$'; } /** @@ -215,9 +250,9 @@ private void initiateHoverRequest(ITextViewer viewer, int offset) { try { HoverParams params = LSPEclipseUtils.toHoverParams(offset, document); - this.request = LanguageServers.forDocument(document) - .withCapability(ServerCapabilities::getHoverProvider) - .collectAll(server -> server.getTextDocumentService().hover(params)); + this.request = LanguageServers.forDocument(document) // + .withCapability(ServerCapabilities::getHoverProvider) // + .collectAll(server -> server.getTextDocumentService().hover(params)); } catch (BadLocationException e) { LanguageServerPlugin.logError(e); } @@ -236,5 +271,4 @@ protected IInformationControl doCreateInformationControl(Shell parent) { } }; } - }