diff --git a/src/editor/lsp/hover.ts b/src/editor/lsp/hover.ts index e9706558a..955efc28e 100644 --- a/src/editor/lsp/hover.ts +++ b/src/editor/lsp/hover.ts @@ -4,7 +4,8 @@ * history, `docs.ts`) and code.quickDefinition (⌥Space, definition source popup). */ import { EditorSelection } from "@codemirror/state"; -import { hoverTooltip, type EditorView } from "@codemirror/view"; +import { closeHoverTooltips, hasHoverTooltips, hoverTooltip, ViewPlugin, type EditorView } from "@codemirror/view"; +import type { Extension } from "@codemirror/state"; import { commands, isTauri, @@ -32,6 +33,7 @@ import { basename } from "@/platform/project"; import { iconNode } from "./iconMarkup"; import { editorPrefs } from "@/platform/editorSettings"; import { CONTRIBUTED_HOVER, contributedAt, type ContributedHover } from "@/editor/api"; +import { navigationModifierHeld, onNavigationModifier, pointerTarget } from "@/platform/navigationModifier"; let hoverRequest: RequestId | null = null; @@ -98,43 +100,78 @@ function renderDiagnostics(view: EditorView, diags: PlacedDiagnostic[]): HTMLEle return box; } -/** Hover: problems always; quick documentation per Settings › Editor › Code Editing. */ -export function hoverExtension() { - return hoverTooltip( - async (view, pos) => { - const diags = diagnosticsAt(view, pos); - let hover: Hover | null = null; - if (editorPrefs().codeEditing.quickDocOnHover) - try { - hover = await requestHover(view, pos); - } catch (e) { - reportLspError(view, e, false); - } - if (!diags.length && !hover) return null; - let from = pos, - to = pos; - if (hover?.range) ({ from, to } = rangeToOffsets(view.state.doc, hover.range)); - else if (diags.length) ({ from, to } = diags[0]!); - else { - const word = view.state.wordAt(pos); - if (word) ({ from, to } = word); - } - const lang = docOf(view)?.uiMode ?? null; - return { - pos: from, - end: to, - above: true, - create: () => { - const dom = document.createElement("div"); - dom.className = `cm-lsp-hover ${selectableText}`; - if (diags.length) dom.appendChild(renderDiagnostics(view, diags)); - if (hover) dom.appendChild(renderMarkup(hover.contents, lang)); - return { dom }; - }, - }; +/** + * Whether the plain hover (problems + quick documentation) stays out: while + * ⌘/Ctrl is held the ⌘-hover link and its brief info (`navigation.ts`) own the + * pointer, as in IDEA; and over an inlay hint, which has its own tooltip. + */ +export function hoverYields(): boolean { + return navigationModifierHeld() || !!pointerTarget()?.closest?.(".cm-lsp-inlay"); +} + +/** Problems always; quick documentation per Settings › Editor › Code Editing. */ +export async function hoverSource(view: EditorView, pos: number) { + if (hoverYields()) return null; + const diags = diagnosticsAt(view, pos); + let hover: Hover | null = null; + if (editorPrefs().codeEditing.quickDocOnHover) + try { + hover = await requestHover(view, pos); + } catch (e) { + reportLspError(view, e, false); + } + // ⌘/Ctrl pressed while the server answered. + if (hoverYields()) return null; + if (!diags.length && !hover) return null; + let from = pos, + to = pos; + if (hover?.range) ({ from, to } = rangeToOffsets(view.state.doc, hover.range)); + else if (diags.length) ({ from, to } = diags[0]!); + else { + const word = view.state.wordAt(pos); + if (word) ({ from, to } = word); + } + const lang = docOf(view)?.uiMode ?? null; + return { + pos: from, + end: to, + above: true, + create: () => { + const dom = document.createElement("div"); + dom.className = `cm-lsp-hover ${selectableText}`; + if (diags.length) dom.appendChild(renderDiagnostics(view, diags)); + if (hover) dom.appendChild(renderMarkup(hover.contents, lang)); + return { dom }; }, - { hoverTime: Math.max(100, editorPrefs().codeEditing.quickDocDelay) }, - ); + }; +} + +/** + * Pressing ⌘/Ctrl closes the open hover popups (documentation, problems, + * debugger values) so the ⌘-hover info replaces them instead of stacking; + * a popup the pointer is in stays (selecting text in it and copying). + */ +const hoverYieldsToNavigation = ViewPlugin.fromClass( + class { + private off: () => void; + constructor(view: EditorView) { + this.off = onNavigationModifier((held) => { + if (!held || !hasHoverTooltips(view.state)) return; + if (pointerTarget()?.closest?.(".cm-tooltip")) return; + view.dispatch({ effects: closeHoverTooltips }); + }); + } + destroy() { + this.off(); + } + }, +); + +export function hoverExtension(): Extension { + return [ + hoverTooltip(hoverSource, { hoverTime: Math.max(100, editorPrefs().codeEditing.quickDocDelay) }), + hoverYieldsToNavigation, + ]; } /** The documentation popup has the keyboard (F1 pressed twice, or clicked into). */ diff --git a/src/editor/lsp/hoverPrecedence.test.ts b/src/editor/lsp/hoverPrecedence.test.ts new file mode 100644 index 000000000..1c42150b8 --- /dev/null +++ b/src/editor/lsp/hoverPrecedence.test.ts @@ -0,0 +1,127 @@ +/** Issue #57: ⌘/Ctrl-hover info and the plain hover popups never stack. */ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { EditorState } from "@codemirror/state"; +import { EditorView, activateHover, hasHoverTooltips } from "@codemirror/view"; +import { CONTRIBUTED_HOVER, type ContributedHover } from "@/editor/api"; +import { usePlatform } from "@/platform/platformInfo"; +import { + isNavigationGesture, + navigationModifierHeld, + pointerTarget, + resetNavigationModifier, +} from "@/platform/navigationModifier"; +import { hoverExtension, hoverSource } from "./hover"; + +const docHover: ContributedHover = (state, pos) => { + const w = state.wordAt(pos); + return w ? { from: w.from, to: w.to, markdown: "package query" } : null; +}; + +function editor() { + const state = EditorState.create({ + doc: "query.FlightPlan", + extensions: [hoverExtension(), EditorState.languageData.of(() => [{ [CONTRIBUTED_HOVER]: docHover }])], + }); + const parent = document.body.appendChild(document.createElement("div")); + return new EditorView({ state, parent }); +} + +const key = (type: "keydown" | "keyup", k: string, init: KeyboardEventInit = {}) => + window.dispatchEvent(new KeyboardEvent(type, { key: k, bubbles: true, ...init })); +const settle = () => new Promise((r) => setTimeout(r, 0)); + +describe.each([ + { os: "windows", mod: "Control", flag: "ctrlKey", other: "metaKey" }, + { os: "macos", mod: "Meta", flag: "metaKey", other: "ctrlKey" }, +] as const)("navigation modifier on $os", ({ os, mod, flag, other }) => { + let view: EditorView | null = null; + beforeEach(() => { + usePlatform.setState({ os }); + resetNavigationModifier(); + }); + afterEach(() => { + view?.destroy(); + view = null; + document.body.replaceChildren(); + resetNavigationModifier(); + }); + + it("tracks the platform's modifier alone, pressed and released", () => { + expect(navigationModifierHeld()).toBe(false); + key("keydown", mod, { [flag]: true }); + expect(navigationModifierHeld()).toBe(true); + key("keyup", mod); + expect(navigationModifierHeld()).toBe(false); + // With ⇧/⌥ it is another gesture; the other platform's Ctrl/⌘ is not it. + key("keydown", "Shift", { [flag]: true, shiftKey: true }); + expect(navigationModifierHeld()).toBe(false); + key("keydown", mod === "Meta" ? "Control" : "Meta", { [other]: true }); + expect(navigationModifierHeld()).toBe(false); + const both = new MouseEvent("mousemove", { [flag]: true, [other]: true }); + expect(isNavigationGesture(both)).toBe(false); + expect(isNavigationGesture(new MouseEvent("mousemove", { [flag]: true }))).toBe(true); + }); + + it("learns the state from mouse moves and forgets it when the window loses focus", () => { + expect(navigationModifierHeld()).toBe(false); + window.dispatchEvent(new MouseEvent("mousemove", { [flag]: true })); + expect(navigationModifierHeld()).toBe(true); + window.dispatchEvent(new Event("blur")); + expect(navigationModifierHeld()).toBe(false); + }); + + it("modifier held before hovering: no documentation popup", async () => { + view = editor(); + expect(await hoverSource(view, 2)).not.toBeNull(); + key("keydown", mod, { [flag]: true }); + expect(await hoverSource(view, 2)).toBeNull(); + }); + + it("modifier pressed while the documentation popup is open: it closes", async () => { + view = editor(); + activateHover(view, 2, 1); + await settle(); + expect(hasHoverTooltips(view.state)).toBe(true); + key("keydown", mod, { [flag]: true }); + expect(hasHoverTooltips(view.state)).toBe(false); + }); + + it("modifier pressed while the request is in flight: the late answer is dropped", async () => { + view = editor(); + const pending = hoverSource(view, 2); + key("keydown", mod, { [flag]: true }); + expect(await pending).toBeNull(); + }); + + it("modifier released: nothing stale is left and the next hover shows documentation again", async () => { + view = editor(); + key("keydown", mod, { [flag]: true }); + activateHover(view, 2, 1); + await settle(); + expect(hasHoverTooltips(view.state)).toBe(false); + key("keyup", mod); + expect(hasHoverTooltips(view.state)).toBe(false); + activateHover(view, 2, 1); + await settle(); + expect(hasHoverTooltips(view.state)).toBe(true); + }); + + it("keeps a popup the pointer is in (select + copy with the modifier)", async () => { + view = editor(); + activateHover(view, 2, 1); + await settle(); + const tooltip = view.dom.querySelector(".cm-tooltip")!; + tooltip.dispatchEvent(new MouseEvent("mousemove", { bubbles: true })); + expect(pointerTarget()).toBe(tooltip); + key("keydown", mod, { [flag]: true }); + expect(hasHoverTooltips(view.state)).toBe(true); + }); + + it("no documentation popup over an inlay hint (it has its own tooltip)", async () => { + view = editor(); + const inlay = view.contentDOM.appendChild(document.createElement("span")); + inlay.className = "cm-lsp-inlay"; + inlay.dispatchEvent(new MouseEvent("mousemove", { bubbles: true })); + expect(await hoverSource(view, 2)).toBeNull(); + }); +}); diff --git a/src/editor/lsp/inlay.test.ts b/src/editor/lsp/inlay.test.ts index 1b0deb774..510943758 100644 --- a/src/editor/lsp/inlay.test.ts +++ b/src/editor/lsp/inlay.test.ts @@ -10,6 +10,7 @@ vi.mock("@/workbench/contributions", async (orig) => { }); const { inlayDecorations, inlayMenuItems } = await import("./inlay"); +const { usePlatform } = await import("@/platform/platformInfo"); const { useContextMenuStore } = await import("@/ui"); const hint: InlayHint = { @@ -51,7 +52,11 @@ describe("inlay hints", () => { expect(view.dom.querySelector(".cm-lsp-inlay")!.textContent).toBe(": Foo"); part.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true, button: 0 })); expect(openFile).not.toHaveBeenCalled(); - part.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true, button: 0, metaKey: true, ctrlKey: true })); + usePlatform.setState({ os: "windows" }); + // ⌘ is not the navigation modifier off macOS. + part.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true, button: 0, metaKey: true })); + expect(openFile).not.toHaveBeenCalled(); + part.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true, button: 0, ctrlKey: true })); expect(openFile).toHaveBeenCalledWith("/p/foo.rs", { selection: { line: 4, col: 7 }, focus: true }); }); diff --git a/src/editor/lsp/inlay.ts b/src/editor/lsp/inlay.ts index f97ee238d..9896aed3e 100644 --- a/src/editor/lsp/inlay.ts +++ b/src/editor/lsp/inlay.ts @@ -8,7 +8,7 @@ import { StateEffect, StateField, type Extension } from "@codemirror/state"; import { Decoration, EditorView, ViewPlugin, WidgetType, type DecorationSet, type ViewUpdate } from "@codemirror/view"; import { call, commands, isTauri, type InlayHint } from "@/platform/ipc"; import { executeAction } from "@/platform/actions"; -import { hasPrimaryModifier } from "@/platform/keymap"; +import { isNavigationGesture } from "@/platform/navigationModifier"; import { maybeEditorService } from "@/workbench/contributions"; import { showContextMenu, type MenuItem } from "@/ui"; import { useProject } from "@/platform/project"; @@ -69,7 +69,7 @@ class InlayWidget extends WidgetType { const loc = part.location; // ⌘-click (Ctrl elsewhere) navigates to the declaration the part names, as in IDEA. span.addEventListener("mousedown", (e) => { - if (e.button !== 0 || !hasPrimaryModifier(e)) return; + if (e.button !== 0 || !isNavigationGesture(e)) return; e.preventDefault(); e.stopPropagation(); void maybeEditorService()?.openFile(loc.path, { diff --git a/src/editor/lsp/navigation.ts b/src/editor/lsp/navigation.ts index 8d1705a28..55ff239ab 100644 --- a/src/editor/lsp/navigation.ts +++ b/src/editor/lsp/navigation.ts @@ -49,7 +49,8 @@ import { requestHover } from "./hover"; import { declarationSummary, renderCode } from "./markdown"; import { editorPrefs } from "@/platform/editorSettings"; import { isVirtualUri, virtualContainer } from "@/platform/virtualDocs"; -import { hasPrimaryModifier, isMacHost, shortcutLabel } from "@/platform/keymap"; +import { shortcutLabel } from "@/platform/keymap"; +import { isNavigationGesture, onNavigationModifier } from "@/platform/navigationModifier"; import { create } from "zustand"; export async function navigateTo(loc: Location) { @@ -337,7 +338,16 @@ const linkPlugin = ViewPlugin.fromClass( private probed = new Map(); private probedDoc: Text | null = null; private probing: string | null = null; - constructor(readonly view: EditorView) {} + private off: () => void; + constructor(readonly view: EditorView) { + // Window-wide, so ⌘/Ctrl over an unfocused editor underlines too; released anywhere clears it. + this.off = onNavigationModifier((held) => { + if (!held || this.last) this.show(held); + }); + } + destroy() { + this.off(); + } 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)) @@ -391,25 +401,17 @@ const linkPlugin = ViewPlugin.fromClass( { eventHandlers: { mousemove(e) { - this.last = { x: e.clientX, y: e.clientY }; - this.show(hasPrimaryModifier(e) && !e.altKey && !e.shiftKey); + // Not through a popup or an inlay hint (which navigates on its own ⌘-click) to the text under it. + const over = e.target instanceof Element ? e.target : null; + this.last = over?.closest(".cm-tooltip, .cm-lsp-inlay") ? null : { x: e.clientX, y: e.clientY }; + this.show(isNavigationGesture(e)); }, mouseleave() { this.last = null; this.show(false); }, - keydown(e) { - if (e.key === (isMacHost() ? "Meta" : "Control")) this.show(true); - }, - keyup(e) { - if (e.key === (isMacHost() ? "Meta" : "Control")) this.show(false); - }, - blur() { - this.show(false); - }, mousedown(e, view) { - if (e.button !== 0 || !hasPrimaryModifier(e) || e.altKey || e.shiftKey) return false; - if (isMacHost() ? e.ctrlKey : e.metaKey) return false; + if (e.button !== 0 || !isNavigationGesture(e)) return false; const pos = view.posAtCoords({ x: e.clientX, y: e.clientY }, false); if (pos == null) return false; e.preventDefault(); diff --git a/src/features/debugger/editor/valueTooltip.ts b/src/features/debugger/editor/valueTooltip.ts index 6bca28473..6fafe8cdd 100644 --- a/src/features/debugger/editor/valueTooltip.ts +++ b/src/features/debugger/editor/valueTooltip.ts @@ -6,6 +6,7 @@ import type { Extension } from "@codemirror/state"; import { hoverTooltip, type EditorView } from "@codemirror/view"; import { workflowSettings } from "@/platform/workflowSettings"; +import { navigationModifierHeld } from "@/platform/navigationModifier"; import { activeSession, evaluate } from "../state"; import { inlineValue } from "../values"; import { tx } from "@/platform/i18n"; @@ -40,7 +41,8 @@ export function valueTooltip(): Extension { return hoverTooltip( async (view, pos) => { const dbg = workflowSettings().debugger; - if (!dbg.valueTooltip) return null; + // ⌘/Ctrl-hover shows the navigation info instead (closed on press by the LSP hover). + if (!dbg.valueTooltip || navigationModifierHeld()) return null; const session = activeSession(); if (!session || session.status !== "paused") return null; const expr = expressionAt(view, pos); @@ -53,6 +55,7 @@ export function valueTooltip(): Extension { } catch { return null; } + if (navigationModifierHeld()) return null; return { pos: expr.from, end: expr.to, diff --git a/src/platform/navigationModifier.ts b/src/platform/navigationModifier.ts new file mode 100644 index 000000000..b592e9e29 --- /dev/null +++ b/src/platform/navigationModifier.ts @@ -0,0 +1,77 @@ +/** + * The navigation modifier (⌘ on macOS, Ctrl elsewhere) as a live, window-wide + * state: ⌘/Ctrl-hover shows the navigation link and its brief info, and every + * plain hover popup (quick documentation, problems, debugger values) yields to + * it. Tracked on the window so an editor sees the key even when it isn't focused. + */ +import { hasPrimaryModifier, isMacHost } from "./keymap"; + +type ModifierEvent = Pick; + +/** ⌘/Ctrl alone: no ⌥/⇧ (other gestures) and not the other platform's Ctrl/⌘. */ +export function isNavigationGesture(e: ModifierEvent): boolean { + return hasPrimaryModifier(e) && !e.altKey && !e.shiftKey && !(isMacHost() ? e.ctrlKey : e.metaKey); +} + +let held = false; +let pointer: Element | null = null; +const listeners = new Set<(held: boolean) => void>(); +let installed: (() => void) | null = null; + +function setHeld(value: boolean) { + if (held === value) return; + held = value; + for (const fn of [...listeners]) fn(value); +} + +function install() { + if (installed || typeof window === "undefined") return; + const onKey = (e: KeyboardEvent) => setHeld(isNavigationGesture(e)); + // Also from pointer events: a key released outside the window never reports its keyup. + const onMouse = (e: MouseEvent) => { + pointer = e.target instanceof Element ? e.target : null; + setHeld(isNavigationGesture(e)); + }; + const onBlur = () => setHeld(false); + const opts = { capture: true, passive: true } as const; + window.addEventListener("keydown", onKey, opts); + window.addEventListener("keyup", onKey, opts); + window.addEventListener("mousemove", onMouse, opts); + window.addEventListener("mousedown", onMouse, opts); + window.addEventListener("blur", onBlur); + installed = () => { + window.removeEventListener("keydown", onKey, opts); + window.removeEventListener("keyup", onKey, opts); + window.removeEventListener("mousemove", onMouse, opts); + window.removeEventListener("mousedown", onMouse, opts); + window.removeEventListener("blur", onBlur); + installed = null; + }; +} + +/** Whether ⌘ (macOS) / Ctrl is held on its own right now. */ +export function navigationModifierHeld(): boolean { + install(); + return held; +} + +/** Element under the pointer at the last mouse move (null before any). */ +export function pointerTarget(): Element | null { + install(); + return pointer; +} + +/** Called with the new state whenever the navigation modifier is pressed or released. */ +export function onNavigationModifier(fn: (held: boolean) => void): () => void { + install(); + listeners.add(fn); + return () => listeners.delete(fn); +} + +/** Tests: forget the tracked state and listeners. */ +export function resetNavigationModifier() { + installed?.(); + held = false; + pointer = null; + listeners.clear(); +}