diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx index d9ddf9225bdf..91c7cc855596 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.tsx +++ b/apps/web/src/components/ThreadTerminalDrawer.tsx @@ -57,7 +57,7 @@ import { } from "~/terminal/ghostty/surface"; import { type GhosttyColor, type GhosttyTheme } from "~/terminal/ghostty/core"; import { useOpenInPreferredEditor } from "../editorPreferences"; -import { isTerminalLinkActivation, isTerminalUrl, resolvePathLinkTarget } from "../terminal-links"; +import { isTerminalUrl, resolvePathLinkTarget } from "../terminal-links"; import { isDiffToggleShortcut, isTerminalClearShortcut, @@ -782,7 +782,6 @@ export function TerminalViewport({ } function handleLinkActivate(text: string, event: MouseEvent): void { - if (!isTerminalLinkActivation(event)) return; const latestTerminal = terminalRef.current; if (!latestTerminal) return; if (isTerminalUrl(text)) { @@ -803,6 +802,7 @@ export function TerminalViewport({ threadRef, openPreview, fallbackToBrowser, + forceBrowser: event.metaKey || event.ctrlKey, }).catch((error: unknown) => { toastManager.add( stackedThreadToast({ diff --git a/apps/web/src/components/preview/openTerminalLinkInPreview.test.ts b/apps/web/src/components/preview/openTerminalLinkInPreview.test.ts index 2ce81cb06af2..96629b5c9b02 100644 --- a/apps/web/src/components/preview/openTerminalLinkInPreview.test.ts +++ b/apps/web/src/components/preview/openTerminalLinkInPreview.test.ts @@ -88,6 +88,7 @@ describe("openTerminalLinkInPreview", () => { threadRef, openPreview, fallbackToBrowser, + forceBrowser: false, }), ).rejects.toBe(failure); expect(fallbackToBrowser).not.toHaveBeenCalled(); @@ -105,6 +106,7 @@ describe("openTerminalLinkInPreview", () => { threadRef, openPreview, fallbackToBrowser, + forceBrowser: false, }); expect(fallbackToBrowser).toHaveBeenCalledOnce(); @@ -120,6 +122,7 @@ describe("openTerminalLinkInPreview", () => { threadRef, openPreview, fallbackToBrowser, + forceBrowser: false, }); expect(openPreview).toHaveBeenCalledOnce(); @@ -141,6 +144,7 @@ describe("openTerminalLinkInPreview", () => { threadRef, openPreview, fallbackToBrowser: vi.fn(), + forceBrowser: false, }); await vi.waitFor(() => expect(browserDefaultsMocks.resolve).toHaveBeenCalledOnce()); @@ -170,6 +174,7 @@ describe("openTerminalLinkInPreview", () => { threadRef, openPreview: async () => AsyncResult.failure(cause), fallbackToBrowser, + forceBrowser: false, }); expect(fallbackToBrowser).toHaveBeenCalledOnce(); @@ -194,9 +199,26 @@ describe("openTerminalLinkInPreview", () => { threadRef, openPreview: async () => AsyncResult.failure(Cause.interrupt()), fallbackToBrowser, + forceBrowser: false, }); expect(reportError).not.toHaveBeenCalled(); expect(fallbackToBrowser).not.toHaveBeenCalled(); }); + + it("opens in the system browser when Ctrl or Command is held", async () => { + const fallbackToBrowser = vi.fn(); + const openPreview = vi.fn(async () => AsyncResult.success(snapshot)); + + await openTerminalLinkInPreview({ + url: "https://example.com/docs", + threadRef, + openPreview, + fallbackToBrowser, + forceBrowser: true, + }); + + expect(fallbackToBrowser).toHaveBeenCalledOnce(); + expect(openPreview).not.toHaveBeenCalled(); + }); }); diff --git a/apps/web/src/components/preview/openTerminalLinkInPreview.ts b/apps/web/src/components/preview/openTerminalLinkInPreview.ts index 4a48403c7e54..c81eeee3e1f1 100644 --- a/apps/web/src/components/preview/openTerminalLinkInPreview.ts +++ b/apps/web/src/components/preview/openTerminalLinkInPreview.ts @@ -34,19 +34,19 @@ interface OpenTerminalLinkInPreviewInput { readonly threadRef: ScopedThreadRef; readonly openPreview: OpenPreviewMutation; readonly fallbackToBrowser: () => void; + /** Cmd/Ctrl-click bypasses the preference and opens in the system browser. */ + readonly forceBrowser: boolean; } /** - * Opens a terminal hyperlink where the "Open links in" setting says. Terminal - * links are activated with the platform modifier already held, so unlike chat - * links the modifier cannot double as the system-browser override; the setting - * alone decides, and the system browser is the fallback whenever the in-app - * one cannot take the URL. + * Opens a terminal hyperlink where the "Open links in" setting says, unless a + * Cmd/Ctrl-click explicitly requests the system browser. */ export async function openTerminalLinkInPreview( input: OpenTerminalLinkInPreviewInput, ): Promise { const supportsPreview = + !input.forceBrowser && isWebUrl(input.url) && isPreviewSupportedInRuntime() && input.threadRef.threadId.length > 0 && diff --git a/apps/web/src/components/settings/IntegrationsSettings.tsx b/apps/web/src/components/settings/IntegrationsSettings.tsx index a3e96107b29c..26c407e9b5e8 100644 --- a/apps/web/src/components/settings/IntegrationsSettings.tsx +++ b/apps/web/src/components/settings/IntegrationsSettings.tsx @@ -516,7 +516,7 @@ function BrowserLinkTargetSetting({ disabled }: { readonly disabled: boolean }) return ( { resize() { for (const callback of resizeCallbacks) callback(); }, - pointer(type: string, clientX: number, buttons: number) { + pointer(type: string, clientX: number, buttons: number, shiftKey = false) { canvas.dispatchEvent( Object.assign(new Event(type, { cancelable: true }), { clientX, @@ -176,6 +175,7 @@ describe("GhosttyTerminalSurface visibility", () => { pointerId: 1, button: 0, buttons, + shiftKey, }), ); }, @@ -280,6 +280,84 @@ describe("GhosttyTerminalSurface visibility", () => { expect(harness.renderedSnapshot.rowData[0]?.cells.some((cell) => cell.selected)).toBe(false); }); + it("starts a selection when dragging from a link", async () => { + const harness = createHarness(); + const onLinkActivate = vi.fn(); + const surface = await harness.create({ onLinkActivate }); + surface.write("https://example.com"); + harness.flushFrame(); + + harness.pointer("pointerdown", 5, 1); + harness.pointer("pointermove", 37, 1); + harness.pointer("pointerup", 37, 0); + + expect(onLinkActivate).not.toHaveBeenCalled(); + expect(surface.getSelection()).toBe("https"); + }); + + it("keeps a link click active through slight pointer movement", async () => { + const harness = createHarness(); + const onLinkActivate = vi.fn(); + const surface = await harness.create({ onLinkActivate }); + surface.write("https://example.com"); + harness.flushFrame(); + + harness.pointer("pointerdown", 5, 1); + harness.pointer("pointermove", 6, 1); + harness.pointer("pointerup", 6, 0); + + expect(onLinkActivate).toHaveBeenCalledOnce(); + }); + + it("uses repeated link clicks for word and line selection", async () => { + const harness = createHarness(); + const onLinkActivate = vi.fn(); + const surface = await harness.create({ onLinkActivate }); + surface.write("https://example.com tail"); + harness.flushFrame(); + + harness.pointer("pointerdown", 5, 1); + harness.pointer("pointerup", 5, 0); + harness.pointer("pointerdown", 5, 1); + harness.pointer("pointerup", 5, 0); + expect(onLinkActivate).toHaveBeenCalledOnce(); + expect(surface.getSelection()).not.toBe(""); + + harness.pointer("pointerdown", 5, 1); + harness.pointer("pointerup", 5, 0); + expect(onLinkActivate).toHaveBeenCalledOnce(); + expect(surface.getSelection()).toBe("https://example.com tail"); + }); + + it("uses Shift drags over links for selection", async () => { + const harness = createHarness(); + const onLinkActivate = vi.fn(); + const surface = await harness.create({ onLinkActivate }); + surface.write("https://example.com"); + harness.flushFrame(); + + harness.pointer("pointerdown", 5, 1, true); + harness.pointer("pointermove", 37, 1, true); + harness.pointer("pointerup", 37, 0, true); + expect(onLinkActivate).not.toHaveBeenCalled(); + expect(surface.getSelection()).toBe("https"); + }); + + it("does not activate a link replaced before pointer release", async () => { + const harness = createHarness(); + const onLinkActivate = vi.fn(); + const surface = await harness.create({ onLinkActivate }); + surface.write("https://first.example"); + harness.flushFrame(); + + harness.pointer("pointerdown", 5, 1); + surface.write("\x1b[2J\x1b[Hhttps://second.example"); + harness.flushFrame(); + harness.pointer("pointerup", 5, 0); + + expect(onLinkActivate).not.toHaveBeenCalled(); + }); + it("stops zero-size mounts and repaints when the same size returns", async () => { const harness = createHarness(); const surface = await harness.create(); @@ -876,19 +954,6 @@ describe("terminalWheelArrowData", () => { }); }); -describe("isTerminalLinkPointerGesture", () => { - it("uses Command on macOS and Control elsewhere", () => { - expect(isTerminalLinkPointerGesture({ ctrlKey: false, metaKey: true }, "MacIntel")).toBe(true); - expect(isTerminalLinkPointerGesture({ ctrlKey: true, metaKey: false }, "MacIntel")).toBe(false); - expect(isTerminalLinkPointerGesture({ ctrlKey: true, metaKey: false }, "Linux x86_64")).toBe( - true, - ); - expect(isTerminalLinkPointerGesture({ ctrlKey: false, metaKey: true }, "Linux x86_64")).toBe( - false, - ); - }); -}); - describe("advanceTerminalSelectionClickSequence", () => { it("recognizes stationary double and triple pointer presses without PointerEvent.detail", () => { const first = advanceTerminalSelectionClickSequence(null, { diff --git a/apps/web/src/terminal/ghostty/surface.ts b/apps/web/src/terminal/ghostty/surface.ts index 29aaac6f6abd..7892d3200dcd 100644 --- a/apps/web/src/terminal/ghostty/surface.ts +++ b/apps/web/src/terminal/ghostty/surface.ts @@ -249,6 +249,19 @@ export interface TerminalLinkWithRange { readonly range: GhosttyCellRange; } +function isSameTerminalLink( + left: TerminalLinkWithRange, + right: TerminalLinkWithRange | null, +): boolean { + return ( + right?.text === left.text && + right.range.start.x === left.range.start.x && + right.range.start.y === left.range.start.y && + right.range.end.x === left.range.end.x && + right.range.end.y === left.range.end.y + ); +} + function terminalColumnAtOffset(row: GhosttySnapshot["rowData"][number], offset: number): number { for (let column = 0; column < row.cells.length; column += 1) { const nextOffset = terminalColumnOffset(row, column + 1); @@ -463,15 +476,6 @@ export function terminalWheelArrowData(rows: number, applicationCursorKeys: bool return sequence.repeat(Math.abs(rows)); } -export function isTerminalLinkPointerGesture( - event: Pick, - platform = navigator.platform, -): boolean { - return isMacPlatform(platform) - ? event.metaKey && !event.ctrlKey - : event.ctrlKey && !event.metaKey; -} - export function ghosttyMouseButton(button: number): number | null { switch (button) { case 0: @@ -496,6 +500,8 @@ export interface TerminalSelectionClickSequence { readonly y: number; } +const TERMINAL_LINK_DRAG_THRESHOLD_PX = 4; + export function advanceTerminalSelectionClickSequence( previous: TerminalSelectionClickSequence | null, event: Pick, @@ -503,7 +509,8 @@ export function advanceTerminalSelectionClickSequence( const repeats = previous !== null && event.timeStamp - previous.time <= SELECTION_MULTI_CLICK_INTERVAL_MS && - Math.hypot(event.clientX - previous.x, event.clientY - previous.y) <= 4; + Math.hypot(event.clientX - previous.x, event.clientY - previous.y) <= + TERMINAL_LINK_DRAG_THRESHOLD_PX; return { count: repeats ? (previous.count >= 3 ? 1 : previous.count + 1) : 1, time: event.timeStamp, @@ -588,9 +595,14 @@ export class GhosttyTerminalSurface { private mouseReportingPointerId: number | null = null; private mouseReportingButton: number | null = null; private linkActivationPointerId: number | null = null; + private linkActivationLink: TerminalLinkWithRange | null = null; + private linkActivationOrigin: { + x: number; + y: number; + clickCount: number; + } | null = null; private hoveredLink: TerminalLinkWithRange | null = null; private hoverPointer: { x: number; y: number } | null = null; - private linkModifierActive = false; private selectionClickSequence: TerminalSelectionClickSequence | null = null; private selectionMoved = false; private composing = false; @@ -1014,7 +1026,6 @@ export class GhosttyTerminalSurface { } private readonly onKeyDown = (event: KeyboardEvent) => { - this.updateLinkModifier(event); // Presses handled outside the terminal must also swallow their release: // beforeKey runs side effects (keybindings, navigation sends), so it cannot // be consulted again on keyup, and Kitty report-event-types sessions would @@ -1117,7 +1128,6 @@ export class GhosttyTerminalSurface { }; private readonly onKeyUp = (event: KeyboardEvent) => { - this.updateLinkModifier(event); if (this.suppressedKeyCodes.delete(event.code)) return; if (isTerminalCompositionKey(event, this.composing)) { return; @@ -1139,7 +1149,6 @@ export class GhosttyTerminalSurface { private readonly onBlur = () => { this.focused = false; - this.linkModifierActive = false; this.refreshHoveredLink(); // Suppressions survive blur deliberately: a shortcut that moves focus (for // example terminal-toggle) must still swallow its own keyup if focus comes @@ -1260,21 +1269,39 @@ export class GhosttyTerminalSurface { return; } if (event.button !== 0) return; - if (isTerminalLinkPointerGesture(event)) { + const clickCount = this.recordSelectionClick(event); + const link = this.linkAt(event.clientX, event.clientY); + if (link && !event.shiftKey && clickCount === 1) { event.preventDefault(); event.stopPropagation(); this.linkActivationPointerId = event.pointerId; + this.linkActivationLink = link; + this.linkActivationOrigin = { + x: event.clientX, + y: event.clientY, + clickCount, + }; this.canvas.setPointerCapture(event.pointerId); return; } this.clearHoveredLink(); - const cell = this.cellAt(event.clientX, event.clientY); - this.selectionMoved = false; + this.beginSelection(event, clickCount); + this.canvas.setPointerCapture(event.pointerId); + }; + + private recordSelectionClick( + event: Pick, + ): number { this.selectionClickSequence = advanceTerminalSelectionClickSequence( this.selectionClickSequence, event, ); - const clickCount = this.selectionClickSequence.count; + return this.selectionClickSequence.count; + } + + private beginSelection(event: { clientX: number; clientY: number }, clickCount: number): void { + const cell = this.cellAt(event.clientX, event.clientY); + this.selectionMoved = false; this.selectionMode = clickCount >= 3 ? "line" : clickCount === 2 ? "word" : "cell"; const range = this.selectionMode === "line" @@ -1302,12 +1329,31 @@ export class GhosttyTerminalSurface { } } this.forceFullRender = true; - this.canvas.setPointerCapture(event.pointerId); this.requestRender(); - }; + } private readonly onPointerMove = (event: PointerEvent) => { - if (this.linkActivationPointerId === event.pointerId) return; + if (this.linkActivationPointerId === event.pointerId) { + const origin = this.linkActivationOrigin; + if ( + origin === null || + Math.hypot(event.clientX - origin.x, event.clientY - origin.y) <= + TERMINAL_LINK_DRAG_THRESHOLD_PX + ) { + return; + } + this.linkActivationPointerId = null; + this.linkActivationLink = null; + this.linkActivationOrigin = null; + this.clearHoveredLink(); + this.beginSelection( + { + clientX: origin.x, + clientY: origin.y, + }, + origin.clickCount, + ); + } // Hover motion is only reportable in any-event tracking (DEC 1003); normal and // button-event tracking never report motion without a captured pressed button. const anyEventTracking = this.synchronizeMouseTrackingState(); @@ -1317,7 +1363,6 @@ export class GhosttyTerminalSurface { ) { event.preventDefault(); this.hoverPointer = { x: event.clientX, y: event.clientY }; - this.linkModifierActive = isTerminalLinkPointerGesture(event); // A drag whose press was already sent to the terminal application cannot // turn into link activation midway through, so link feedback would lie. this.setHoveredLink(null); @@ -1392,14 +1437,6 @@ export class GhosttyTerminalSurface { private updateHoverCursor(event: PointerEvent): void { this.hoverPointer = { x: event.clientX, y: event.clientY }; - this.linkModifierActive = isTerminalLinkPointerGesture(event); - this.refreshHoveredLink(); - } - - private updateLinkModifier(event: Pick): void { - const active = isTerminalLinkPointerGesture(event); - if (active === this.linkModifierActive) return; - this.linkModifierActive = active; this.refreshHoveredLink(); } @@ -1416,7 +1453,7 @@ export class GhosttyTerminalSurface { private refreshHoveredLink(): void { const pointer = this.hoverPointer; - const link = pointer && this.linkModifierActive ? this.linkAt(pointer.x, pointer.y) : null; + const link = pointer ? this.linkAt(pointer.x, pointer.y) : null; this.setHoveredLink(link); } @@ -1440,13 +1477,17 @@ export class GhosttyTerminalSurface { if (this.linkActivationPointerId === event.pointerId) { event.preventDefault(); event.stopPropagation(); + const link = this.linkActivationLink; this.linkActivationPointerId = null; + this.linkActivationLink = null; + this.linkActivationOrigin = null; if (this.canvas.hasPointerCapture(event.pointerId)) { this.canvas.releasePointerCapture(event.pointerId); } if (event.type !== "pointercancel") { - const link = this.linkAt(event.clientX, event.clientY); - if (link) this.options.onLinkActivate(link.text, event); + if (link && isSameTerminalLink(link, this.linkAt(event.clientX, event.clientY))) { + this.options.onLinkActivate(link.text, event); + } } return; } @@ -1463,7 +1504,6 @@ export class GhosttyTerminalSurface { this.clearHoveredLink(); } else { this.hoverPointer = { x: event.clientX, y: event.clientY }; - this.linkModifierActive = isTerminalLinkPointerGesture(event); this.refreshHoveredLink(); } return;