fix(editor): Show the ⌘/Ctrl-hover link without waiting for documentation

The navigation link waited for the hover request before it underlined the
symbol, so a slow or stuck documentation request held back the link cursor.
Probe results were also cached too eagerly: a cancelled or failed definition
request was stored as "no target", and a probe that never answered blocked
further probes for that word.

- Underline the link as soon as a definition is found; fill in the brief
  info when hover returns
- Treat cancellation and server errors as transient and retry on the next
  hover instead of caching them
- Expire a probe after 3 seconds so it can be retried, and ignore its late
  response
- Drop responses for a previously hovered word and after the plugin is
  destroyed

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<typeof setTimeout> | 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.
       });
     }
   },

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 = <T>(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<unknown> => ok(null)),
+  lspDocumentLinks: vi.fn(() => ok([])),
+  cancelRequest: vi.fn(() => ok(null)),
   lspGoto: vi.fn((): R<unknown> => unavailable()),
   lspDocumentHighlights: vi.fn((): R<unknown> => unavailable()),
   lspPrepareRename: vi.fn((): R<unknown> => 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<R<unknown>>) => 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<R<unknown>>) => 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");
+  });
+});