From 8ee284a85b513c6ccc3680181eb7b4161c253dc8 Mon Sep 17 00:00:00 2001 From: Serhii Vecherenko Date: Tue, 16 Jun 2026 12:52:14 -0700 Subject: [PATCH] fix(terminal): only stitch URLs split at terminal wrap edge - Gate hard-break URL continuation on `_isWrapEdge` using column cell width - Handle wide/CJK glyphs, shrink-resize overrun rows, and unknown `cols` - Add faithful xterm buffer mocks and stitch/skip regression tests --- .../terminal/TerminalLinkProvider.test.ts | 143 +++++++++++++++++- .../terminal/TerminalLinkProvider.ts | 42 ++++- 2 files changed, 175 insertions(+), 10 deletions(-) diff --git a/src/renderer/components/terminal/TerminalLinkProvider.test.ts b/src/renderer/components/terminal/TerminalLinkProvider.test.ts index 21f020ca..37bfb66d 100644 --- a/src/renderer/components/terminal/TerminalLinkProvider.test.ts +++ b/src/renderer/components/terminal/TerminalLinkProvider.test.ts @@ -5,20 +5,69 @@ type LinkProviderProbe = { _getWindowedLineStrings(lineIndex: number): [string[], number]; }; -function makeLine(text: string, isWrapped = false) { +type Cell = { chars: string; width: number }; + +/** One width-1 cell per character — models a plain ASCII row. */ +function ascii(text: string): Cell[] { + return [...text].map((ch) => ({ chars: ch, width: 1 })); +} + +/** A wide/CJK glyph: a width-2 lead cell followed by a width-0 spacer cell. */ +function wide(ch: string): Cell[] { + return [ + { chars: ch, width: 2 }, + { chars: "", width: 0 }, + ]; +} + +/** + * A faithful xterm buffer-line mock: backed by an explicit cell array so the + * provider's column-accurate width measurement (getCell / getWidth) and its + * string view (translateToString) both behave like the real terminal. + */ +function makeLine(cells: Cell[] | string, isWrapped = false) { + const arr = typeof cells === "string" ? ascii(cells) : cells; return { isWrapped, - translateToString: () => text, + get length() { + return arr.length; + }, + translateToString(trim = false) { + const s = arr.map((c) => (c.width === 0 ? "" : c.chars || " ")).join(""); + return trim ? s.replace(/[ ]+$/u, "") : s; + }, + getCell(i: number, cell: { _chars: string; _width: number }) { + const c = arr[i] ?? { chars: "", width: 0 }; + cell._chars = c.chars; + cell._width = c.width; + return cell; + }, }; } -function makeProvider(lines: ReturnType[]): LinkProviderProbe { +// `cols` defaults to 80, the conventional terminal width. The hard-break +// continuation only fires when the line ending in the URL fills the row to +// exactly `cols`, so the stitching tests set `cols` to that line's width. +function makeProvider(lines: ReturnType[], cols = 80): LinkProviderProbe { return new TerminalLinkProvider( { + cols, buffer: { active: { getLine: (index: number) => lines[index], - getNullCell: () => ({}), + getNullCell: () => { + const cell = { + _chars: "", + _width: 0, + getChars() { + return cell._chars; + }, + getWidth() { + return cell._width; + }, + }; + return cell; + }, }, }, } as never, @@ -39,14 +88,92 @@ describe("TerminalLinkProvider", () => { }); it("keeps stitching hard-wrapped URL continuation lines", () => { - const provider = makeProvider([ - makeLine("https://auth.x.ai/oauth2/authorize?response_type=code&clie"), - makeLine("nt_id=grok-build&redirect_uri=http%3A%2F%2F127.0.0.1%3A3000%2Fcallback"), - ]); + // The first line is filled to the right edge (width === cols), modelling a + // soft-wrap whose `isWrapped` flag was dropped by ConPTY / scrollback replay. + const head = "https://auth.x.ai/oauth2/authorize?response_type=code&client_id=grok"; + const provider = makeProvider( + [makeLine(head), makeLine("-build&redirect_uri=http%3A%2F%2F127.0.0.1%3A3000%2Fcallback")], + head.length, + ); const [lines] = provider._getWindowedLineStrings(0); expect(lines.join("")).toContain("client_id=grok-build"); expect(lines.join("")).toContain("redirect_uri=http%3A%2F%2F127.0.0.1%3A3000%2Fcallback"); }); + + it("stitches a full-width wrapped localhost URL whose wrap flag was lost", () => { + const head = "http://localhost:5173/some/deep/path/abc"; + const provider = makeProvider([makeLine(head), makeLine("def/more?query=value")], head.length); + + const [lines] = provider._getWindowedLineStrings(0); + + expect(lines.join("")).toContain("path/abcdef/more?query=value"); + }); + + it("stitches a wrapped URL even when wide glyphs make the row's width exceed its string length", () => { + // Three wide glyphs (6 columns, 3 code units) + a 14-column URL fragment = + // 20 columns === cols, but the string length is only 17. A code-unit check + // would wrongly treat this full-width row as short and refuse to stitch. + const head = [...wide("本"), ...wide("地"), ...wide("址"), ...ascii("http://localho")]; + const provider = makeProvider([makeLine(head), makeLine("st:5173/page")], 20); + + const [lines] = provider._getWindowedLineStrings(0); + + expect(lines.join("")).toContain("http://localhost:5173/page"); + }); + + it("does not glue the next concurrently-prefixed line onto a short URL (WSL/Vite)", () => { + // Reproduces the WSL bug: Vite prints a short `Local:` URL line, and the + // very next line is another process's output (concurrently's `[1]` prefix). + // The URL ends well short of the right edge, so nothing must be stitched. + const provider = makeProvider([ + makeLine("[1] ➜ Local: http://localhost:5173/"), + makeLine("[1] ➜ Network: http://10.255.255.254:5173/"), + ]); + + const [lines] = provider._getWindowedLineStrings(0); + + expect(lines.join("")).toBe("[1] ➜ Local: http://localhost:5173/"); + // The URL target must not become `http://localhost:5173/[1`. + expect(lines.join("")).not.toContain("5173/["); + }); + + it("does not glue a bare following line onto a short URL", () => { + const provider = makeProvider([ + makeLine(" Local: http://localhost:5173/"), + makeLine("README.md"), + ]); + + const [lines] = provider._getWindowedLineStrings(0); + + expect(lines.join("")).toBe(" Local: http://localhost:5173/"); + }); + + it("does not stitch an over-long un-trimmed row left by a shrink-resize", () => { + // The URL line's content (33 cols) overruns the shrunk grid (cols 12). A + // real wrap would have reflowed the overflow onto a wrapped line, so an + // over-wide single row is not a join point. + const provider = makeProvider( + [makeLine("Local: http://localhost:5173/"), makeLine("next-unrelated-output")], + 12, + ); + + const [lines] = provider._getWindowedLineStrings(0); + + expect(lines.join("")).toBe("Local: http://localhost:5173/"); + expect(lines.join("")).not.toContain("5173/next"); + }); + + it("fails safe and does not stitch when the terminal width is unknown (cols=0)", () => { + const provider = makeProvider( + [makeLine("Local: http://localhost:5173/"), makeLine("next-unrelated-output")], + 0, + ); + + const [lines] = provider._getWindowedLineStrings(0); + + expect(lines.join("")).toBe("Local: http://localhost:5173/"); + expect(lines.join("")).not.toContain("5173/next"); + }); }); diff --git a/src/renderer/components/terminal/TerminalLinkProvider.ts b/src/renderer/components/terminal/TerminalLinkProvider.ts index 3b9e5185..ddfddd6a 100644 --- a/src/renderer/components/terminal/TerminalLinkProvider.ts +++ b/src/renderer/components/terminal/TerminalLinkProvider.ts @@ -121,12 +121,21 @@ export class TerminalLinkProvider implements ILinkProvider { } // ── Continue past hard breaks when the text ends mid-URL ───── - // Only extend if a URL regex match reaches the very end of the - // accumulated text (i.e. the URL was cut off by a hard newline). + // Only extend when a URL regex match reaches the very end of the + // accumulated text (the URL was cut off mid-token) AND the line holding it + // is a wrap edge (filled to the terminal's right edge). A URL only + // "continues" across a newline when the break is really a soft-wrap whose + // `isWrapped` flag was lost — Windows ConPTY repaints and sliced scrollback + // replay both drop that flag. A URL that stops short of the edge ended on + // purpose: the following line is unrelated output (e.g. a `concurrently` + // "[1]" prefix, or Vite's next "Network:" URL) and must not be glued on. while (length < 2048) { const joined = lines.join(""); if (!this._endsWithPartialUrl(joined)) break; + const edgeLine = buf.getLine(bottomIdx - 1); + if (!edgeLine || !this._isWrapEdge(edgeLine)) break; + const nextLine = buf.getLine(bottomIdx); if (!nextLine) break; @@ -149,6 +158,35 @@ export class TerminalLinkProvider implements ILinkProvider { return [lines, topIdx]; } + /** + * Whether `line` was filled to the terminal's right edge — i.e. the newline + * after it is a soft-wrap whose `isWrapped` flag was lost, not a deliberate + * line ending. The visible content width (one past the rightmost non-blank + * column) is measured in COLUMNS from the buffer cells, so a wide/CJK glyph + * counts as its cell width rather than its UTF-16 length. The whole cell + * array is scanned — including any cells past the live `cols`, which xterm + * leaves un-trimmed after a shrink-resize (see `IBufferLine.length`) — so an + * overrunning row measures wider than `cols` and is rejected (a real wrap + * would have reflowed the overflow onto a wrapped line). Unknown width + * (cols <= 0) fails safe to `false`. + */ + private _isWrapEdge(line: IBufferLine): boolean { + const cols = this._terminal.cols; + if (cols <= 0) return false; + const cell = this._terminal.buffer.active.getNullCell(); + let edge = 0; + for (let i = 0; i < line.length; i++) { + line.getCell(i, cell); + const width = cell.getWidth(); + if (width === 0) continue; // spacer cell trailing a wide glyph + const chars = cell.getChars(); + if (chars !== "" && chars !== " ") { + edge = i + width; // content reaches through this cell to column i+width + } + } + return edge === cols; + } + /** Returns true when the text ends with an in-progress URL (http(s)://...). */ private _endsWithPartialUrl(text: string): boolean { // Quick check: does a URL regex match reach the very end of the string?