From a5733953b83eaae6a69be098cd3b0e0366d783e5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 24 Aug 2026 06:01:05 -0700 Subject: [PATCH] fix(tui): keep sentence punctuation out of terminal hyperlink targets (#128727) --- src/tui/osc8-hyperlinks.test.ts | 55 +++++++++++++++++++++++++++++++-- src/tui/osc8-hyperlinks.ts | 34 ++++++++++---------- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/src/tui/osc8-hyperlinks.test.ts b/src/tui/osc8-hyperlinks.test.ts index 653eb7b1e55d..78fee334cc6d 100644 --- a/src/tui/osc8-hyperlinks.test.ts +++ b/src/tui/osc8-hyperlinks.test.ts @@ -8,6 +8,15 @@ describe("extractUrls", () => { expect(urls).toEqual(["https://example.com"]); }); + it.each([".", ",", ";", "!", "?", ":", "*", "_", "~", ".?!"])( + "excludes trailing GFM punctuation %s from bare URLs", + (punctuation) => { + expect(extractUrls(`Visit https://example.com/path${punctuation}`)).toEqual([ + "https://example.com/path", + ]); + }, + ); + it("stops bare URLs before bidi formatting controls", () => { const url = "https://example.com/path"; expect(extractUrls(`مرحبا ${url}\u2069`)).toEqual([url]); @@ -21,9 +30,9 @@ describe("extractUrls", () => { expect(urls).toHaveLength(2); }); - it("extracts markdown link hrefs", () => { - const urls = extractUrls("[Click here](https://example.com/path)"); - expect(urls).toEqual(["https://example.com/path"]); + it.each(["", ".", "?", ";"])("preserves authored markdown href suffix %s", (suffix) => { + const url = `https://example.com/path${suffix}`; + expect(extractUrls(`[Click here](${url})`)).toEqual([url]); }); it("extracts markdown links with angle brackets and title text", () => { @@ -64,6 +73,11 @@ describe("extractUrls", () => { expect(extractUrls(`[Wikipedia](${url})`)).toEqual([url]); }); + it("keeps balanced URL parentheses and internal query punctuation", () => { + const url = "https://example.com/v1.0/URL_(disambiguation)?a=1&b=2#section"; + expect(extractUrls(`Visit ${url}.`)).toEqual([url]); + }); + it("does not extract an incomplete markdown link destination", () => { expect(extractUrls("[broken](https://)")).toEqual([]); }); @@ -108,6 +122,41 @@ describe("addOsc8Hyperlinks", () => { expect(result[0]).toBe(`Visit \x1b]8;;${url}\x07${url}\x1b]8;;\x07 for info`); }); + it.each([".", ",", ";", "!", "?", ":", "*", "_", "~", ".?!"])( + "keeps trailing GFM punctuation %s outside terminal hyperlink targets", + (punctuation) => { + const url = "https://example.com/path"; + const line = `Visit ${url}${punctuation}`; + + expect(addOsc8Hyperlinks([line], extractUrls(line))).toEqual([ + `Visit \x1b]8;;${url}\x07${url}\x1b]8;;\x07${punctuation}`, + ]); + }, + ); + + it("preserves punctuation explicitly authored in markdown hyperlink targets", () => { + const url = "https://example.com/path."; + const line = `Docs (${url})`; + + expect(addOsc8Hyperlinks([line], extractUrls(`[Docs](${url})`))).toEqual([ + `Docs (\x1b]8;;${url}\x07${url}\x1b]8;;\x07)`, + ]); + }); + + it.each([".", ","])( + "keeps authored and bare hyperlink occurrences distinct before %s", + (punctuation) => { + const bareUrl = "https://example.com/path"; + const authoredUrl = `${bareUrl}${punctuation}`; + const markdown = `[Docs](${authoredUrl}) and ${bareUrl}${punctuation}`; + const rendered = `Docs (${authoredUrl}) and ${bareUrl}${punctuation}`; + + expect(addOsc8Hyperlinks([rendered], extractUrls(markdown))).toEqual([ + `Docs (\x1b]8;;${authoredUrl}\x07${authoredUrl}\x1b]8;;\x07) and \x1b]8;;${bareUrl}\x07${bareUrl}\x1b]8;;\x07${punctuation}`, + ]); + }, + ); + it("keeps bidi isolation outside the exact OSC 8 target", () => { const url = "https://example.com/path"; expect(addOsc8Hyperlinks([`\u2067مرحبا ${url}\u2069`], [url])).toEqual([ diff --git a/src/tui/osc8-hyperlinks.ts b/src/tui/osc8-hyperlinks.ts index a0ad0c68930f..5eebb0f530de 100644 --- a/src/tui/osc8-hyperlinks.ts +++ b/src/tui/osc8-hyperlinks.ts @@ -13,11 +13,11 @@ const OSC8_START_RE = new RegExp(`^${OSC8_PATTERN}`); const URL_PATH_WITH_PARENS = /https?:\/\/[^()\s<>\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]+(?:\([^()\s<>\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]*\)[^()\s<>\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]*)*/g; -/** Strip the suffix starting at a `)` without a matching `(` in the URL. - * Bare URLs in prose can pick up a trailing `)` that belongs to surrounding - * punctuation, e.g. `(see https://example.com/path)` — the `)` after `path` - * and anything after it are sentence punctuation, not part of the URL. */ -function trimUnbalancedTrailingParens(url: string): string { +/** Strip GFM sentence punctuation and unmatched closing parentheses from bare + * URLs, while preserving balanced parentheses and exact authored Markdown + * destinations. `(see https://example.com/path).` must link only the URL, + * but `[label](https://example.com/path.)` must retain its authored dot. */ +function trimUrlTrailingPunctuation(url: string, knownUrls?: string[]): string { let open = 0; for (let index = 0; index < url.length; index++) { const ch = url[index]; @@ -25,12 +25,16 @@ function trimUnbalancedTrailingParens(url: string): string { open++; } else if (ch === ")") { if (open === 0) { - return url.slice(0, index); + const authoredUrl = url.slice(0, index); + return knownUrls?.includes(authoredUrl) + ? authoredUrl + : trimUrlTrailingPunctuation(authoredUrl, knownUrls); } open--; } } - return url; + const trimmed = url.replace(/[?!.,:;*_~]+$/u, ""); + return knownUrls?.includes(url) && !knownUrls.includes(trimmed) ? url : trimmed; } function hasUrlContent(url: string): boolean { @@ -65,7 +69,7 @@ export function extractUrls(markdown: string): string[] { const bareRe = /https?:\/\/(?:\[[0-9a-f:.]+\](?::\d+)?[^\s\]>\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]*|[^\s[\]>\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]+)/gi; while ((m = bareRe.exec(stripped)) !== null) { - const url = trimUnbalancedTrailingParens(m[0]); + const url = trimUrlTrailingPunctuation(m[0]); if (hasUrlContent(url)) { urls.add(url); } @@ -134,7 +138,7 @@ function findUrlRanges( let match: RegExpExecArray | null; while ((match = urlRe.exec(visibleText)) !== null) { - const fragment = trimUnbalancedTrailingParens(match[0]); + const fragment = trimUrlTrailingPunctuation(match[0], knownUrls); const start = match.index; // Resolve fragment to a known URL (exact > prefix > superstring) @@ -154,7 +158,7 @@ function findUrlRanges( nextVisibleText ?.trimStart() .match(/^[^\s\]>\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]+/)?.[0] ?? ""; - const nextFragment = trimUnbalancedTrailingParens(nextToken); + const nextFragment = trimUrlTrailingPunctuation(nextToken); for (const known of knownUrls) { if (!known.startsWith(fragment)) { continue; @@ -171,14 +175,8 @@ function findUrlRanges( } } - if (!found) { - for (const known of knownUrls) { - if (known === fragment) { - resolvedUrl = known; - found = true; - break; - } - } + if (!found && knownUrls.includes(fragment)) { + found = true; } if (!found) { let bestLen = 0;