diff --git a/src/editor/lsp/navigation.ts b/src/editor/lsp/navigation.ts index 55ff239ab..ee774825a 100644 --- a/src/editor/lsp/navigation.ts +++ b/src/editor/lsp/navigation.ts @@ -320,15 +320,14 @@ async function probeNavigation(view: EditorView, offset: number): Promise<{ info try { navigable = (await lspRequest(() => commands.lspGoto(doc.doc, pos, "definition"))).length > 0; } catch (e) { - if (isCancelled(e)) return null; - if (isUnavailable(e)) navigable = !!(await gotoFallback(view, doc, pos))?.length; + if (!isUnavailable(e)) throw e; + navigable = !!(await gotoFallback(view, doc, pos))?.length; } if (!navigable && WEB_LANGUAGES.has(doc.language) && !doc.virtual) navigable = !!(await lspRequest(() => commands.webGoto(doc.doc, pos)).catch(() => null))?.targets.length; } if (!navigable) return null; - const hover = await requestHover(view, offset).catch(() => null); - return { info: hover ? declarationSummary(hover.contents) : null }; + return { info: null }; } const linkPlugin = ViewPlugin.fromClass( @@ -339,6 +338,10 @@ const linkPlugin = ViewPlugin.fromClass( private probedDoc: Text | null = null; private probing: string | null = null; private off: () => void; + private disposed = false; + private hovered: string | null = null; + private probeVersion = 0; + private probeTimer: ReturnType | undefined; constructor(readonly view: EditorView) { // Window-wide, so ⌘/Ctrl over an unfocused editor underlines too; released anywhere clears it. this.off = onNavigationModifier((held) => { @@ -347,10 +350,16 @@ const linkPlugin = ViewPlugin.fromClass( } destroy() { this.off(); + this.disposed = true; + this.probeVersion++; + clearTimeout(this.probeTimer); } private set(value: LinkState | null) { const current = this.view.state.field(linkField); - if (current === value || (current && value && current.from === value.from && current.to === value.to && current.info === value.info)) + if ( + current === value || + (current && value && current.from === value.from && current.to === value.to && current.info === value.info) + ) return; if (!current && !value) return; this.view.dispatch({ effects: setLink.of(value) }); @@ -361,10 +370,19 @@ const linkPlugin = ViewPlugin.fromClass( // Underline whole links once the server's links are in. if (doc && !doc.virtual && !knownLinks(view)) void documentLinks(view, doc).then(() => { - if (view.state.field(linkField)) this.show(true); + if (!this.disposed && view.state.field(linkField)) this.show(true); }); const target = meta && this.last ? linkAt(view, this.last.x, this.last.y) : null; + const hovered = target ? `${target.from}:${target.to}` : null; + if (hovered !== this.hovered) { + this.hovered = hovered; + this.probeVersion++; + clearTimeout(this.probeTimer); + this.probing = null; + } if (!target) { + this.probeVersion++; + clearTimeout(this.probeTimer); this.probing = null; this.set(null); return; @@ -388,13 +406,41 @@ const linkPlugin = ViewPlugin.fromClass( if (current && (current.from !== target.from || current.to !== target.to)) this.set(null); if (this.probing === key) return; this.probing = key; + const version = ++this.probeVersion; + clearTimeout(this.probeTimer); + // A timed-out probe must be retryable, rather than cached as "no target". + this.probeTimer = setTimeout(() => { + if (this.probeVersion === version) { + this.probeVersion++; + this.probing = null; + } + }, 3000); const docAt = view.state.doc; - void probeNavigation(view, target.from).then((result) => { - if (view.state.doc !== docAt) return; - this.probed.set(key, result); - if (this.probing !== key) return; - this.probing = null; - if (result) this.set({ from: target.from, to: target.to, info: result.info }); + void probeNavigation(view, target.from) + .then((result) => { + if (this.disposed || view.state.doc !== docAt || this.probeVersion !== version) return; + clearTimeout(this.probeTimer); + this.probed.set(key, result); + if (this.probing !== key) return; + this.probing = null; + if (result) { + this.set({ from: target.from, to: target.to, info: null }); + // Documentation must never delay the link cursor or underline. + void requestHover(view, target.from) + .then((hover) => { + if (this.disposed || view.state.doc !== docAt || this.probeVersion !== version || !hover) return; + const info = declarationSummary(hover.contents); + this.probed.set(key, { info }); + this.set({ from: target.from, to: target.to, info }); + }) + .catch(() => {}); + } + }) + .catch(() => { + if (this.disposed || this.probeVersion !== version) return; + clearTimeout(this.probeTimer); + this.probing = null; + // Cancellation and server errors are transient, not negative results. }); } }, diff --git a/src/editor/lsp/syntaxFallback.test.ts b/src/editor/lsp/syntaxFallback.test.ts index 5524891c1..903e5438b 100644 --- a/src/editor/lsp/syntaxFallback.test.ts +++ b/src/editor/lsp/syntaxFallback.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { EditorState, Text } from "@codemirror/state"; import { EditorView } from "@codemirror/view"; import type { IdeError, Locals, NodeAt, Range, SymbolMatch, SyntaxNode } from "@/bindings"; @@ -8,6 +8,9 @@ const ok = (data: T) => Promise.resolve({ status: "ok" as const, data }); const unavailable = () => Promise.resolve({ status: "error" as const, error: { kind: "unavailable", message: "no server" } as IdeError }); const cmd = { + lspHover: vi.fn((): R => ok(null)), + lspDocumentLinks: vi.fn(() => ok([])), + cancelRequest: vi.fn(() => ok(null)), lspGoto: vi.fn((): R => unavailable()), lspDocumentHighlights: vi.fn((): R => unavailable()), lspPrepareRename: vi.fn((): R => unavailable()), @@ -390,3 +393,84 @@ describe("rename wiring", () => { expect(useLspPopup.getState().popup?.kind).toBe("rename"); }); }); + +describe("modifier-hover navigation recovery", () => { + const target = { path: "/p/target.ts", range: r(0, 0, 1), uri: null }; + let mac = false; + beforeAll(async () => { + mac = (await import("@/platform/keymap")).isMacHost(); + }); + function move(view: EditorView, pos = 0, primary = true) { + vi.spyOn(view, "posAtCoords").mockReturnValue(pos); + view.contentDOM.dispatchEvent( + new MouseEvent("mousemove", { + bubbles: true, + metaKey: primary && mac, + ctrlKey: primary && !mac, + }), + ); + } + + it("shows the link without waiting for documentation", async () => { + cmd.lspGoto.mockReturnValueOnce(ok([target])); + cmd.lspHover.mockReturnValueOnce(new Promise(() => {})); + const view = editor("foo"); + move(view); + await vi.waitFor(() => expect(view.dom.querySelector(".cm-lsp-link")?.textContent).toBe("foo")); + expect(cmd.lspHover).toHaveBeenCalled(); + }); + + it("retries a transient server error instead of caching no target", async () => { + cmd.lspGoto.mockReturnValueOnce( + Promise.resolve({ status: "error", error: { kind: "internal", message: "timeout" } }), + ); + const view = editor("foo"); + move(view); + await new Promise((resolve) => setTimeout(resolve, 20)); + cmd.lspGoto.mockReturnValueOnce(ok([target])); + move(view); + await vi.waitFor(() => expect(view.dom.querySelector(".cm-lsp-link")?.textContent).toBe("foo")); + expect(cmd.lspGoto).toHaveBeenCalledTimes(2); + }); + + it("retries a stuck probe and ignores its eventual response", async () => { + let finish!: (value: Awaited>) => void; + cmd.lspGoto.mockReturnValueOnce( + new Promise((resolve) => { + finish = resolve; + }), + ); + const view = editor("foo"); + vi.useFakeTimers(); + try { + move(view); + await vi.advanceTimersByTimeAsync(3001); + cmd.lspGoto.mockReturnValueOnce(ok([target])); + move(view); + expect(cmd.lspGoto).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + await vi.waitFor(() => expect(view.dom.querySelector(".cm-lsp-link")?.textContent).toBe("foo")); + finish({ status: "ok", data: [] }); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(view.dom.querySelector(".cm-lsp-link")?.textContent).toBe("foo"); + }); + + it("ignores a late probe after moving to a different word", async () => { + let finish!: (value: Awaited>) => void; + cmd.lspGoto.mockReturnValueOnce( + new Promise((resolve) => { + finish = resolve; + }), + ); + cmd.lspGoto.mockReturnValueOnce(ok([target])); + const view = editor("foo bar"); + move(view); + move(view, 4); + await vi.waitFor(() => expect(view.dom.querySelector(".cm-lsp-link")?.textContent).toBe("bar")); + finish({ status: "ok", data: [target] }); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(view.dom.querySelector(".cm-lsp-link")?.textContent).toBe("bar"); + }); +});