Skip to content

Commit 569eff5

Browse files
committed
Disarm stale URL handlers, match uppercase schemes, hedge OSC-8 docs
A retext that dropped the last URL left the old hit ranges armed, so Ctrl+clicking the old columns still opened. The armer now clears its handlers when no line holds a URL.
1 parent 3e89d22 commit 569eff5

5 files changed

Lines changed: 108 additions & 10 deletions

File tree

‎docs/TUI.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -823,12 +823,17 @@ running its own selection. Two chords cover remaining copy needs:
823823
`http(s)` URL in a plain or structured-text transcript row underlines it;
824824
pressing and releasing on the same URL opens it in the default browser,
825825
while a press that releases anywhere else stays a selection gesture:
826-
- **macOS: Cmd+click.** The terminal itself owns this chord: every link
827-
span carries OSC-8 metadata, so the emulator opens the URL and the app
828-
never sees the press.
826+
- **macOS: Cmd+click.** The terminal itself owns this chord: link spans
827+
carry OSC-8 metadata, so an emulator with OSC-8 support opens the URL
828+
and the app never sees the press. Per-terminal: Terminal.app does not
829+
support OSC-8, so Cmd+click does nothing there; iTerm2 3.5+, Ghostty,
830+
WezTerm, Kitty, and VS Code support it.
829831
- **Linux/Windows: Ctrl+click.** The app opens the URL through the
830832
platform opener (`open` on macOS as fallback, `xdg-open`, `cmd /c
831833
start`).
834+
- **Right-click safety.** The open gesture requires a left (button-0)
835+
press with the modifier held, so Ctrl+right-click never opens a URL —
836+
context menus stay safe.
832837
- Without the modifier, nothing changes: clicks still expand rows and
833838
drags still select-and-copy. With mouse capture off (Alt+M), the
834839
terminal owns every click and the app sees none, so there is nothing to
@@ -915,7 +920,8 @@ terminal. It cannot observe:
915920
and headless suites pin the gating (Ctrl+press opens, plain click and
916921
Ctrl+drag do not, non-`http(s)` never opens) with a mocked opener, but
917922
only a real terminal can show whether it delivers the held Ctrl on motion
918-
and press events, or resolves the `open`/`xdg-open` spawn into a browser.
923+
and press events, whether it honors OSC-8 for Cmd+click, or resolves the
924+
`open`/`xdg-open` spawn into a browser.
919925
- **The system clipboard.** `system-clipboard.ts`'s helper-binary spawns and
920926
OSC 52 fallback are exercised with mocked spawn functions in tests; no
921927
test round-trips through a real `pbcopy`/`xclip`/terminal clipboard.

‎src/tui/shell/transcript.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import { stringWidth } from "../view/height.js";
1414
import { viewToTableContent, type McpStructuredView } from "../mcp-view.js";
1515
import {
16+
armLinkLine,
1617
buildLinkLine,
1718
findLinks,
1819
paintLinkLine,
@@ -407,6 +408,9 @@ function paintPlainRowNode(
407408
if (!lines.some((line) => findLinks(line).length > 0)) {
408409
node.content = painted.content;
409410
node.fg = painted.fg;
411+
// Route through the armer so a retext that drops the last URL disarms
412+
// the handlers a previous arming installed (armLinkLine clears them).
413+
armLinkLine(node, []);
410414
return;
411415
}
412416
paintLinkLine(

‎src/tui/url-click.test.ts‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,19 @@
77
* spot (docs/TUI.md); headless, the mock delivers it like any click.
88
*/
99
import { describe, expect, test } from "bun:test";
10+
import { TextRenderable } from "@opentui/core";
1011

1112
import { defined } from "../../tests/helpers/defined.js";
1213
import { withTestRenderer } from "./harness";
1314
import { appendStreamRow } from "./shell/chrome";
1415
import { createAppShell } from "./shell/index";
15-
import { isUnderlined, resetUrlOpener, setUrlOpener } from "./url-links";
16+
import {
17+
isUnderlined,
18+
paintLinkLine,
19+
resetUrlOpener,
20+
setUrlOpener,
21+
splitLinkSpans,
22+
} from "./url-links";
1623
import { type StreamRow } from "./stream";
1724

1825
const CALL: StreamRow = {
@@ -123,4 +130,62 @@ describe("Ctrl+clicking a transcript URL", () => {
123130
{ width: 80, height: 24 },
124131
);
125132
});
133+
134+
test("retexting the URL away disarms the node: old columns open nothing", async () => {
135+
await withTestRenderer(
136+
async (h) => {
137+
const opened: string[] = [];
138+
setUrlOpener((url) => {
139+
opened.push(url);
140+
});
141+
try {
142+
const line = "see https://example.com/x ok";
143+
const node = new TextRenderable(h.renderer, { content: line });
144+
h.root.add(node);
145+
paintLinkLine(node, [splitLinkSpans([{ text: line, fg: "#fff" }])]);
146+
await h.renderOnce();
147+
148+
const link = findCell(h.captureCharFrame(), "example.com");
149+
expect(link).not.toBeNull();
150+
const at = defined(link);
151+
152+
await h.mockMouse.click(at.x, at.y, 0, {
153+
modifiers: { ctrl: true },
154+
});
155+
await h.renderOnce();
156+
expect(opened).toEqual(["https://example.com/x"]);
157+
158+
// Retext the URL away, exactly as the row retext path does. The
159+
// handlers are setter-only (no getter to assert on), so pin the
160+
// disarm behaviorally: the old columns open nothing and hover
161+
// leaves the painted text intact.
162+
const retexted = "see nothing here";
163+
paintLinkLine(node, [
164+
splitLinkSpans([{ text: retexted, fg: "#fff" }]),
165+
]);
166+
await h.renderOnce();
167+
const frame = h.captureCharFrame();
168+
expect(frame).toContain("nothing here");
169+
expect(frame).not.toContain("example.com");
170+
171+
opened.length = 0;
172+
await h.mockMouse.click(at.x, at.y, 0, {
173+
modifiers: { ctrl: true },
174+
});
175+
await h.renderOnce();
176+
expect(opened).toEqual([]);
177+
178+
const before = h.captureCharFrame();
179+
await h.mockMouse.moveTo(at.x, at.y, {
180+
modifiers: { ctrl: true },
181+
});
182+
await h.renderOnce();
183+
expect(h.captureCharFrame()).toBe(before);
184+
} finally {
185+
resetUrlOpener();
186+
}
187+
},
188+
{ width: 80, height: 24 },
189+
);
190+
});
126191
});

‎src/tui/url-links.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ interface LinkHit {
4141
readonly end: number;
4242
}
4343

44-
const URL_PATTERN = /https?:\/\/[^\s<>"'`\]]+/g;
44+
const URL_PATTERN = /https?:\/\/[^\s<>"'`\]]+/gi;
4545
const TRAILING_PUNCTUATION = new Set([
4646
".",
4747
",",
@@ -296,9 +296,10 @@ function linkLinesChunks(
296296
/**
297297
* Arm a text node as a link hit target over caller-built per-line spans:
298298
* Ctrl+hover highlights the URL under the pointer, Ctrl+press and release on
299-
* the same URL opens it. A no-op when no line holds a URL, so URL-free nodes
300-
* carry no handlers at all; handler assignment replaces, so re-arming after
301-
* a retext never stacks.
299+
* the same URL opens it. When no line holds a URL the node is disarmed — any
300+
* handlers a previous arming installed are cleared — so a retext that drops
301+
* the last URL leaves no stale hit target behind; handler assignment
302+
* replaces, so re-arming after a retext never stacks.
302303
*
303304
* The press deliberately keeps bubbling — stopping it would break drag-select
304305
* starting on a URL — and the open fires on release only when the pointer
@@ -312,7 +313,14 @@ export function armLinkLine(
312313
lines: readonly (readonly LinkSpan[])[],
313314
): void {
314315
const hits = lines.map(linkColumnHits);
315-
if (!hits.some((line) => line.length > 0)) return;
316+
if (!hits.some((line) => line.length > 0)) {
317+
node.onMouseDown = undefined;
318+
node.onMouseUp = undefined;
319+
node.onMouseOver = undefined;
320+
node.onMouseMove = undefined;
321+
node.onMouseOut = undefined;
322+
return;
323+
}
316324
const at = (event: MouseEvent): string | null => {
317325
// Events carry terminal-absolute coordinates with no per-node transform,
318326
// so map through the node's own screen position (scroll-aware through the

‎tests/unit/tui/url-links.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ describe("findLinks", () => {
8080
[],
8181
);
8282
});
83+
84+
test("matches uppercase schemes", () => {
85+
expect(findLinks("see HTTP://EXAMPLE.COM/x ok").map((h) => h.url)).toEqual([
86+
"HTTP://EXAMPLE.COM/x",
87+
]);
88+
});
8389
});
8490

8591
describe("splitLinkSpans", () => {
@@ -143,4 +149,13 @@ describe("openUrl", () => {
143149
});
144150
expect(() => openUrl("https://example.com")).not.toThrow();
145151
});
152+
153+
test("an uppercase URL round-trips through the opener", () => {
154+
const calls: string[] = [];
155+
setUrlOpener((url) => {
156+
calls.push(url);
157+
});
158+
openUrl("HTTP://EXAMPLE.COM/x");
159+
expect(calls).toEqual(["HTTP://EXAMPLE.COM/x"]);
160+
});
146161
});

0 commit comments

Comments
 (0)