From a7c40cd89b2fde220c04abbc24289bb38456da3b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:21:09 -0700 Subject: [PATCH] fix(terminal): stop gluing the next line onto a modifier-clicked URL (#9100) Fixes #8832. HTTP hit-testing no longer consumes path hard-wrap candidates that poisoned mid-row URLs with next-line labels. Uses soft-wrap, framed hard-wrap HTTP, and dedicated edge-wrap reconstruction instead. Supersedes #8852 (edge-wrap approach credit: @xianjianlf2). --- .../edge-wrapped-terminal-http-links.test.ts | 425 ++++++++++++++++++ .../edge-wrapped-terminal-http-links.ts | 183 ++++++++ .../repro-8832-url-next-line.test.ts | 203 +++++++++ .../terminal-url-link-click.test.ts | 38 ++ .../terminal-url-link-hit-testing.ts | 33 +- 5 files changed, 866 insertions(+), 16 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/edge-wrapped-terminal-http-links.test.ts create mode 100644 src/renderer/src/components/terminal-pane/edge-wrapped-terminal-http-links.ts create mode 100644 src/renderer/src/components/terminal-pane/repro-8832-url-next-line.test.ts diff --git a/src/renderer/src/components/terminal-pane/edge-wrapped-terminal-http-links.test.ts b/src/renderer/src/components/terminal-pane/edge-wrapped-terminal-http-links.test.ts new file mode 100644 index 00000000000..9289f67b7b2 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/edge-wrapped-terminal-http-links.test.ts @@ -0,0 +1,425 @@ +import type { IBufferLine } from '@xterm/xterm' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { registerHttpLinkStoreAccessor } from '@/lib/http-link-routing' +import { openHttpLinkAtBufferPosition } from './terminal-url-link-hit-testing' + +const COLS = 40 +const WIDE_GLYPH = 'η•Œ' +const openUrlMock = vi.fn() + +function makeBufferLine(content: string, cols = COLS): IBufferLine { + return makeBufferLineFromCells( + Array.from(content, (text) => ({ text, width: text === WIDE_GLYPH ? 2 : 1 })), + cols + ) +} + +function makeBufferLineFromCells( + cells: { text: string; width: number }[], + cols = COLS +): IBufferLine { + const columns: number[] = [] + let column = 0 + let text = '' + for (const cell of cells) { + text += cell.text + for (let index = 0; index < cell.text.length; index++) { + columns.push(column) + } + column += cell.width + } + while (column < cols) { + text += ' ' + columns.push(column) + column++ + } + columns.push(column) + + return { + isWrapped: false, + length: cols, + translateToString: ( + _trimRight?: boolean, + startColumn = 0, + endColumn = text.length, + outColumns?: number[] + ) => { + outColumns?.splice(0, outColumns.length, ...columns.slice(startColumn, endColumn + 1)) + return text.slice(startColumn, endColumn) + } + } as IBufferLine +} + +describe('edge-wrapped terminal HTTP links', () => { + beforeEach(() => { + vi.stubGlobal('window', { api: { shell: { openUrl: openUrlMock } } }) + registerHttpLinkStoreAccessor(() => ({ + settings: { openLinksInApp: false }, + setActiveWorktree: vi.fn(), + createBrowserTab: vi.fn() + })) + openUrlMock.mockReset() + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('opens a URL when a wide glyph starts the continuation row', () => { + const prefix = 'https://example.com/wide/' + const firstRow = `${prefix}${'c'.repeat(COLS - 1 - prefix.length)}` + const continuationRow = `${WIDE_GLYPH}tail` + const expectedUrl = new URL(`${firstRow}${continuationRow}`).toString() + const rows = [makeBufferLine(firstRow), makeBufferLine(continuationRow)] + const buffer = { getLine: (y: number) => rows[y] } + + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(expectedUrl) + + openUrlMock.mockReset() + expect( + openHttpLinkAtBufferPosition(buffer, { x: 3, y: 2 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(expectedUrl) + }) + + it('opens a URL when a multi-code-point wide cell starts the continuation row', () => { + const wideCell = 'πŸ‘©β€πŸ’»' + const prefix = 'https://example.com/wide/' + const firstRow = `${prefix}${'c'.repeat(COLS - 1 - prefix.length)}` + const continuationRow = `${wideCell}tail` + const expectedUrl = new URL(`${firstRow}${continuationRow}`).toString() + const rows = [ + makeBufferLine(firstRow), + makeBufferLineFromCells([ + { text: wideCell, width: 2 }, + ...Array.from('tail', (text) => ({ text, width: 1 })) + ]) + ] + const buffer = { getLine: (y: number) => rows[y] } + + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(expectedUrl) + + openUrlMock.mockReset() + expect( + openHttpLinkAtBufferPosition(buffer, { x: 1, y: 2 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(expectedUrl) + }) + + it('does not use the wide-glyph tolerance for an ASCII continuation row', () => { + const firstRow = `https://example.com/${'a'.repeat(COLS - 1 - 'https://example.com/'.length)}` + const rows = [makeBufferLine(firstRow), makeBufferLine('next')] + const buffer = { getLine: (y: number) => rows[y] } + + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(firstRow) + + openUrlMock.mockReset() + expect( + openHttpLinkAtBufferPosition(buffer, { x: 2, y: 2 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(false) + expect(openUrlMock).not.toHaveBeenCalled() + }) + + it.each(['Description: 123', 'Description:', '404: Not Found', 'HTTP/2: 200', 'HTTP/2:200'])( + 'does not append a %s label when a complete URL ends at the terminal edge', + (labelRow) => { + const firstRow = `https://example.com/${'a'.repeat(COLS - 1 - 'https://example.com/'.length)}/` + const rows = [makeBufferLine(firstRow), makeBufferLine(labelRow)] + const buffer = { getLine: (y: number) => rows[y] } + + expect(firstRow).toHaveLength(COLS) + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(firstRow) + + openUrlMock.mockReset() + expect( + openHttpLinkAtBufferPosition(buffer, { x: 2, y: 2 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(false) + expect(openUrlMock).not.toHaveBeenCalled() + } + ) + + it('does not append an adjacent URL that starts on the next row', () => { + const firstRow = `https://example.com/${'a'.repeat(COLS - 'https://example.com/'.length)}` + const secondRow = 'https://two.test/path' + const rows = [makeBufferLine(firstRow), makeBufferLine(secondRow)] + const buffer = { getLine: (y: number) => rows[y] } + + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(firstRow) + + openUrlMock.mockReset() + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 2 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(secondRow) + }) + + it('keeps a full-width continuation row that ends in a colon', () => { + const prefix = 'https://example.com/' + const firstRow = `${prefix}${'a'.repeat(COLS - prefix.length)}` + const secondRow = `${'b'.repeat(COLS - 1)}:` + const thirdRow = 'tail' + const expectedUrl = new URL(`${firstRow}${secondRow}${thirdRow}`).toString() + const rows = [makeBufferLine(firstRow), makeBufferLine(secondRow), makeBufferLine(thirdRow)] + const buffer = { getLine: (y: number) => rows[y] } + + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(expectedUrl) + + openUrlMock.mockReset() + expect( + openHttpLinkAtBufferPosition(buffer, { x: 20, y: 2 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(expectedUrl) + }) + + it('keeps a short colon-bearing URL continuation', () => { + const firstRow = `https://example.com/${'a'.repeat(COLS - 'https://example.com/'.length)}` + const secondRow = 'urn:abc' + const expectedUrl = `${firstRow}${secondRow}` + const rows = [makeBufferLine(firstRow), makeBufferLine(secondRow)] + const buffer = { getLine: (y: number) => rows[y] } + + expect( + openHttpLinkAtBufferPosition(buffer, { x: 4, y: 2 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(expectedUrl) + }) + + it('does not count text before the URL against the URL length limit', () => { + const cols = 200 + const prefix = `${'P'.repeat(179)} ` + const url = `https://example.com/${'a'.repeat(1_880)}` + const displayed = `${prefix}${url}` + const rowTexts = Array.from({ length: Math.ceil(displayed.length / cols) }, (_value, index) => + displayed.slice(index * cols, (index + 1) * cols) + ) + const rows = rowTexts.map((row) => makeBufferLine(row, cols)) + const buffer = { getLine: (y: number) => rows[y] } + + expect(url).toHaveLength(1_900) + expect(displayed.length).toBeGreaterThan(2_048) + expect( + openHttpLinkAtBufferPosition(buffer, { x: 191, y: 1 }, cols, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(url) + + openUrlMock.mockReset() + expect( + openHttpLinkAtBufferPosition(buffer, { x: 10, y: rows.length }, cols, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(url) + }) + + it('reconstructs a narrow-terminal URL spanning more than 20 rows from either end', () => { + const url = `https://example.com/${'a'.repeat(1_000)}` + const rowTexts = Array.from({ length: Math.ceil(url.length / COLS) }, (_value, index) => + url.slice(index * COLS, (index + 1) * COLS) + ) + const rows = rowTexts.map((row) => makeBufferLine(row)) + const buffer = { getLine: (y: number) => rows[y] } + + expect(url.length).toBeLessThanOrEqual(2_048) + expect(rowTexts.length).toBeGreaterThan(20) + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(url) + + openUrlMock.mockReset() + expect( + openHttpLinkAtBufferPosition(buffer, { x: 5, y: rowTexts.length }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(url) + }) + + it('reconstructs a wide-glyph URL whose row span exceeds the single-width column bound', () => { + const wideCount = 1_100 + const url = `https://example.com/${WIDE_GLYPH.repeat(wideCount)}` + const rowTexts: string[] = [] + let row = '' + let width = 0 + for (const char of url) { + const charWidth = char === WIDE_GLYPH ? 2 : 1 + if (width + charWidth > COLS) { + rowTexts.push(row) + row = '' + width = 0 + } + row += char + width += charWidth + } + if (row.length > 0) { + rowTexts.push(row) + } + const rows = rowTexts.map((rowText) => makeBufferLine(rowText)) + const buffer = { getLine: (y: number) => rows[y] } + + expect(url.length).toBeLessThanOrEqual(2_048) + expect(rowTexts.length).toBeGreaterThan(Math.ceil(2_048 / COLS) + 1) + expect( + openHttpLinkAtBufferPosition(buffer, { x: 5, y: rowTexts.length }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(new URL(url).toString()) + }) + + it('joins a real multi-line URL whose continuation is a Chinese path segment', () => { + const prefix = 'https://example.com/' + const firstRow = `${prefix}${'a'.repeat(COLS - prefix.length)}` + const continuation = `${WIDE_GLYPH}ζ–‡ζ‘£/θ·―εΎ„` + const expectedUrl = new URL(`${firstRow}${continuation}`).toString() + const rows = [makeBufferLine(firstRow), makeBufferLine(continuation)] + const buffer = { getLine: (y: number) => rows[y] } + + expect(firstRow).toHaveLength(COLS) + expect( + openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(expectedUrl) + }) + + it('does not glue a mid-row URL to a next-line Windows path', () => { + const firstRow = 'Repo: https://example.com/repo/' + const secondRow = 'C:\\Users\\demo\\project\\file.ts' + const rows = [makeBufferLine(firstRow), makeBufferLine(secondRow)] + const buffer = { getLine: (y: number) => rows[y] } + + expect( + openHttpLinkAtBufferPosition(buffer, { x: firstRow.indexOf('https') + 8, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith('https://example.com/repo/') + }) + + it('does not create an edge candidate for a scheme embedded in a word', () => { + const embeddedUrl = `abchttps://example.com/${'a'.repeat( + COLS - 'abchttps://example.com/'.length + )}` + const rows = [makeBufferLine(embeddedUrl), makeBufferLine('tail')] + const buffer = { getLine: (y: number) => rows[y] } + + expect( + openHttpLinkAtBufferPosition(buffer, { x: 10, y: 1 }, COLS, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(false) + expect(openUrlMock).not.toHaveBeenCalled() + }) + + it('keeps a URL under the length limit when its final row also contains a label', () => { + const cols = 200 + const url = `https://example.com/${'a'.repeat(2_020)}` + const displayed = `${url} ${'L'.repeat(159)}` + const rowTexts = Array.from({ length: Math.ceil(displayed.length / cols) }, (_value, index) => + displayed.slice(index * cols, (index + 1) * cols) + ) + const rows = rowTexts.map((row) => makeBufferLine(row, cols)) + const buffer = { getLine: (y: number) => rows[y] } + + expect(url).toHaveLength(2_040) + expect(rowTexts).toHaveLength(11) + expect(rowTexts.at(-1)).toHaveLength(cols) + expect( + openHttpLinkAtBufferPosition(buffer, { x: 10, y: 1 }, cols, { + worktreeId: 'wt-1', + forceSystemBrowser: true + }) + ).toBe(true) + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(url) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/edge-wrapped-terminal-http-links.ts b/src/renderer/src/components/terminal-pane/edge-wrapped-terminal-http-links.ts new file mode 100644 index 00000000000..48e10fabedd --- /dev/null +++ b/src/renderer/src/components/terminal-pane/edge-wrapped-terminal-http-links.ts @@ -0,0 +1,183 @@ +import type { IBufferLine } from '@xterm/xterm' +import { TERMINAL_HTTP_URL_MAX_LENGTH } from './terminal-http-link-limits' +import { translateLineWithColumns, type WrappedLogicalLine } from './wrapped-terminal-link-ranges' + +const HTTP_SCHEME_PATTERN = /https?:\/\//i +const HTTP_SCHEME_START_PATTERN = /^https?:\/\//i +const HTTP_STATUS_ROW_PATTERN = /^HTTP\/\d(?:\.\d)?:\d{3}(?:\s|$)/i +const VERTICAL_LAYOUT_FRAME_PATTERN = /[β”‚β”ƒβ•‘β•Žβ•β”†β”‡β”Šβ”‹|]/ +const EMPTY_LABEL_ROW_PATTERN = /^[^\s:][^:]*:$/ +const LABEL_WITH_SPACING_ROW_PATTERN = /^[^\s:][^:]*:\s/ + +function maxEdgeWrappedHttpRows(lineLength: number): number { + // Why: every continued row reaches the terminal edge, so even all-width-2 + // cells carry at least floor(columns / 2) characters per row; the URL length + // limit then bounds the scan without truncating URLs on narrow terminals. + // The +2 covers the start and tail rows, which may hold only a fragment. + const minCharsPerFullRow = Math.max(1, Math.floor(lineLength / 2)) + return Math.ceil(TERMINAL_HTTP_URL_MAX_LENGTH / minCharsPerFullRow) + 2 +} + +type TrimmedTranslatedLine = { + text: string + sourceText: string + columns: number[] + lineLength: number +} + +function trimRightTranslatedLine(line: IBufferLine): TrimmedTranslatedLine | null { + const translated = translateLineWithColumns(line) + let endIndex = translated.text.length + while (endIndex > 0 && /\s/.test(translated.text[endIndex - 1])) { + endIndex-- + } + if (endIndex === 0) { + return null + } + return { + text: translated.text.slice(0, endIndex), + sourceText: translated.text, + columns: translated.columns.slice(0, endIndex + 1), + lineLength: line.length + } +} + +function firstCellWidth(row: TrimmedTranslatedLine): number { + const firstColumn = row.columns[0] + if (firstColumn === undefined) { + return 0 + } + const nextColumn = row.columns.find((column) => column > firstColumn) + return nextColumn === undefined ? 0 : nextColumn - firstColumn +} + +function isAsciiWordCode(code: number): boolean { + return ( + (code >= 48 && code <= 57) || + (code >= 65 && code <= 90) || + code === 95 || + (code >= 97 && code <= 122) + ) +} + +function findHttpSchemeIndex(text: string): number { + let searchStart = 0 + while (searchStart < text.length) { + const relativeIndex = text.slice(searchStart).search(HTTP_SCHEME_PATTERN) + if (relativeIndex === -1) { + return -1 + } + const schemeIndex = searchStart + relativeIndex + if (schemeIndex === 0 || !isAsciiWordCode(text.charCodeAt(schemeIndex - 1))) { + return schemeIndex + } + searchStart = schemeIndex + 1 + } + return -1 +} + +function rowsCanContinueAtTerminalEdge( + row: WrappedLogicalLine['rows'][number], + nextRow: TrimmedTranslatedLine +): boolean { + // Why: a cursor-positioned label is not URL continuation evidence even when + // the preceding URL happens to end at the terminal edge (#8832). + const nextColumnAfterText = nextRow.columns.at(-1) + const nextRowReachesEdge = + nextColumnAfterText !== undefined && nextColumnAfterText >= nextRow.lineLength + const isLabelRow = + LABEL_WITH_SPACING_ROW_PATTERN.test(nextRow.text) || + ((EMPTY_LABEL_ROW_PATTERN.test(nextRow.text) || HTTP_STATUS_ROW_PATTERN.test(nextRow.text)) && + !nextRowReachesEdge) + if (isLabelRow || HTTP_SCHEME_START_PATTERN.test(nextRow.text)) { + return false + } + const columnAfterText = row.columns.at(-1) + if (columnAfterText === undefined || columnAfterText < row.lineLength - 1) { + return false + } + return columnAfterText >= row.lineLength || firstCellWidth(nextRow) > 1 +} + +export function buildEdgeWrappedHttpLogicalLineCandidates( + buffer: { getLine(y: number): IBufferLine | undefined }, + bufferLineNumber: number +): WrappedLogicalLine[] { + // Why: cursor-positioned output lacks xterm wrap metadata, but an HTTP URL + // may still continue when each earlier row reaches the terminal edge. + const currentY = bufferLineNumber - 1 + const currentLine = buffer.getLine(currentY) + if (!currentLine) { + return [] + } + const maxRows = maxEdgeWrappedHttpRows(currentLine.length) + + const candidates: WrappedLogicalLine[] = [] + const translatedLines = new Map() + const getTranslatedLine = (rowY: number): TrimmedTranslatedLine | null => { + if (translatedLines.has(rowY)) { + return translatedLines.get(rowY) ?? null + } + const line = buffer.getLine(rowY) + const translated = line ? trimRightTranslatedLine(line) : null + translatedLines.set(rowY, translated) + return translated + } + const minY = Math.max(0, currentY - maxRows + 1) + for (let startY = currentY; startY >= minY; startY--) { + const start = getTranslatedLine(startY) + const schemeIndex = start ? findHttpSchemeIndex(start.text) : -1 + if (!start || schemeIndex === -1 || VERTICAL_LAYOUT_FRAME_PATTERN.test(start.text)) { + continue + } + + let text = '' + const rows: WrappedLogicalLine['rows'] = [] + for (let rowY = startY; rowY < startY + maxRows; rowY++) { + const line = buffer.getLine(rowY) + const translated = getTranslatedLine(rowY) + if (!line || !translated) { + break + } + if (VERTICAL_LAYOUT_FRAME_PATTERN.test(translated.text)) { + break + } + if ( + rows.length > 0 && + !line.isWrapped && + !rowsCanContinueAtTerminalEdge(rows.at(-1)!, translated) + ) { + break + } + const fragmentStartIndex = rowY === startY ? schemeIndex : 0 + const fragment = translated.text.slice(fragmentStartIndex) + const whitespaceIndex = fragment.search(/\s/) + const possibleUrlFragmentLength = whitespaceIndex === -1 ? fragment.length : whitespaceIndex + if (text.length + possibleUrlFragmentLength > TERMINAL_HTTP_URL_MAX_LENGTH) { + break + } + + rows.push({ + y: rowY, + text: fragment, + sourceText: translated.sourceText, + columns: translated.columns.slice(fragmentStartIndex), + startIndex: text.length, + isWrapped: line.isWrapped, + lineLength: line.length + }) + text += fragment + if (rows.length > 1 && rowY >= currentY) { + candidates.push({ + text, + rows: [...rows], + fingerprint: `edge-http:${rows.map((row) => `${row.y}:${row.sourceText}`).join('\0')}` + }) + } + } + } + + return candidates.sort( + (left, right) => right.rows.length - left.rows.length || right.text.length - left.text.length + ) +} diff --git a/src/renderer/src/components/terminal-pane/repro-8832-url-next-line.test.ts b/src/renderer/src/components/terminal-pane/repro-8832-url-next-line.test.ts new file mode 100644 index 00000000000..14fbe14aa81 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/repro-8832-url-next-line.test.ts @@ -0,0 +1,203 @@ +/** + * Issue #8832 β€” Cmd-click URL must not glue the next logical line. + * + * Root cause: path hard-wrap reconstruction (from #8339) still joins a URL + * suffix ending in `/` with the next row's path-like prefix. HTTP hit-testing + * must not consume those candidates; it uses soft-wrap, framed hard-wrap HTTP, + * and edge-wrap reconstruction only. + * + * Re-run: + * pnpm exec vitest run --config config/vitest.config.ts \ + * src/renderer/src/components/terminal-pane/repro-8832-url-next-line.test.ts + */ +import type { IBufferLine } from '@xterm/xterm' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { registerHttpLinkStoreAccessor } from '@/lib/http-link-routing' +import { buildCandidateLogicalLinesForBufferPosition } from './terminal-file-link-hit-testing' +import { + extractTerminalHttpLinks, + openHttpLinkAtBufferPosition +} from './terminal-url-link-hit-testing' +import { buildHardWrappedPathLogicalLineCandidates } from './wrapped-terminal-link-ranges' + +const LINE_1 = 'Repo: https://github.com/stablyai/orca/' +const LINE_2 = 'Description: 123' +const EXPECTED_URL = 'https://github.com/stablyai/orca/' +const BUGGY_URL = 'https://github.com/stablyai/orca/Description' + +const openUrlMock = vi.fn() + +function makeBufferLine( + text: string, + options: { cols?: number; isWrapped?: boolean } = {} +): IBufferLine { + const cols = options.cols ?? Math.max(text.length, 80) + const padded = text.padEnd(cols) + return { + isWrapped: options.isWrapped ?? false, + length: cols, + getCell: () => undefined, + translateToString: ( + _trimRight?: boolean, + startColumn = 0, + endColumn = padded.length, + outColumns?: number[] + ) => { + if (outColumns) { + outColumns.length = 0 + for (let index = startColumn; index <= endColumn; index++) { + outColumns.push(index) + } + } + return padded.slice(startColumn, endColumn) + } + } as IBufferLine +} + +function twoRowBuffer( + row0: string, + row1: string, + options: { cols?: number; softWrapped?: boolean } = {} +): { getLine(y: number): IBufferLine | undefined } { + const cols = options.cols ?? 120 + const rows = [ + makeBufferLine(row0, { cols, isWrapped: false }), + makeBufferLine(row1, { cols, isWrapped: options.softWrapped === true }) + ] + return { getLine: (y: number) => rows[y] } +} + +function issueBuffer(): { getLine(y: number): IBufferLine | undefined } { + return twoRowBuffer(LINE_1, LINE_2, { cols: 120 }) +} + +function openUrlAt( + buffer: { getLine(y: number): IBufferLine | undefined }, + x: number, + y: number, + cols = 120 +) { + openUrlMock.mockReset() + const opened = openHttpLinkAtBufferPosition(buffer, { x, y }, cols, { + worktreeId: 'wt-repro-8832', + forceSystemBrowser: true + }) + return { opened, url: openUrlMock.mock.calls[0]?.[0] as string | undefined } +} + +describe('#8832 hard-wrapped path candidates glue next-line text into URLs', () => { + beforeEach(() => { + vi.stubGlobal('navigator', { userAgent: 'Macintosh' }) + vi.stubGlobal('window', { api: { shell: { openUrl: openUrlMock } } }) + registerHttpLinkStoreAccessor(() => ({ + settings: { openLinksInApp: false }, + setActiveWorktree: vi.fn(), + createBrowserTab: vi.fn() + })) + openUrlMock.mockReset() + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('still builds a multi-row path candidate that concatenates Description onto the URL', () => { + // Why: documents the path-reconstruction poison that HTTP hit-testing must + // not consume. File links remain protected by existence checks. + const buffer = issueBuffer() + const candidates = buildHardWrappedPathLogicalLineCandidates(buffer, 1) + const multiRow = candidates.filter((candidate) => candidate.rows.length > 1) + + expect(multiRow.some((candidate) => candidate.text.includes('Description'))).toBe(true) + + const glued = multiRow.find((candidate) => candidate.text.includes('https://')) + expect(glued).toBeDefined() + expect(glued!.text).toContain('https://github.com/stablyai/orca/Description') + }) + + it('HTTP extraction on a path logical line still yields the glued URL', () => { + const buffer = issueBuffer() + const candidates = buildCandidateLogicalLinesForBufferPosition(buffer, 1) + const extracted = candidates.flatMap((line) => extractTerminalHttpLinks(line.text)) + + expect(extracted.map((link) => link.url)).toContain(BUGGY_URL) + const singleLineUrls = extractTerminalHttpLinks(LINE_1).map((link) => link.url) + expect(singleLineUrls).toEqual([EXPECTED_URL]) + }) + + it('openHttpLinkAtBufferPosition opens the unglued URL when Cmd-clicking the URL row', () => { + const buffer = issueBuffer() + const urlStart = LINE_1.indexOf('https://') + const { opened, url } = openUrlAt(buffer, urlStart + 10, 1) + + expect(opened).toBe(true) + expect(url).toBe(EXPECTED_URL) + expect(url).not.toBe(BUGGY_URL) + }) + + it.each([ + ['Chinese label', '说明: δΈ­ζ–‡θ·―εΎ„/ζ–‡δ»Ά.ts'], + ['Windows path', 'C:\\Users\\demo\\project\\README.md'], + ['POSIX path', '/usr/local/bin/orca'], + ['relative path', './src/main/index.ts'], + ['UNC-ish path', '\\\\server\\share\\file.txt'] + ])('does not glue a mid-row URL to a next-line %s', (_label, nextLine) => { + const row0 = 'See https://example.com/docs/' + const buffer = twoRowBuffer(row0, nextLine, { cols: 100 }) + const urlStart = row0.indexOf('https://') + const { opened, url } = openUrlAt(buffer, urlStart + 8, 1) + + expect(opened).toBe(true) + expect(url).toBe('https://example.com/docs/') + expect(url).not.toMatch(/说明|Users|usr|src|server/) + }) + + it('still joins a soft-wrapped multi-line URL via native wrap metadata', () => { + const cols = 40 + const url = 'https://example.com/very/long/path/segments/that/continue/more' + const row0 = url.slice(0, cols) + const row1 = url.slice(cols) + const buffer = twoRowBuffer(row0, row1, { cols, softWrapped: true }) + const first = openUrlAt(buffer, 11, 1, cols) + const second = openUrlAt(buffer, 5, 2, cols) + + expect(first.opened).toBe(true) + expect(first.url).toBe(url) + expect(second.opened).toBe(true) + expect(second.url).toBe(url) + }) + + it('still joins an edge-wrapped multi-line URL without wrap metadata', () => { + const cols = 40 + const url = 'https://example.com/very/long/path/segments/that/continue/more' + const row0 = url.slice(0, cols) + const row1 = url.slice(cols) + expect(row0).toHaveLength(cols) + const buffer = twoRowBuffer(row0, row1, { cols, softWrapped: false }) + const first = openUrlAt(buffer, 11, 1, cols) + const second = openUrlAt(buffer, 5, 2, cols) + + expect(first.opened).toBe(true) + expect(first.url).toBe(url) + expect(second.opened).toBe(true) + expect(second.url).toBe(url) + }) + + it('does not treat file:// as an HTTP link (file routing stays separate)', () => { + const row0 = 'open file:///Users/demo/project/README.md' + const buffer = twoRowBuffer(row0, 'next: line', { cols: 100 }) + const { opened, url } = openUrlAt(buffer, row0.indexOf('file://') + 4, 1) + + expect(opened).toBe(false) + expect(url).toBeUndefined() + }) + + it('opens only the HTTP URL when a Windows path shares the same row', () => { + const row0 = 'https://example.com/a C:\\Users\\demo\\file.txt' + const buffer = twoRowBuffer(row0, 'Description: more', { cols: 100 }) + const { opened, url } = openUrlAt(buffer, 8, 1) + + expect(opened).toBe(true) + expect(url).toBe('https://example.com/a') + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-url-link-click.test.ts b/src/renderer/src/components/terminal-pane/terminal-url-link-click.test.ts index c8e85950da3..36b38c09a5e 100644 --- a/src/renderer/src/components/terminal-pane/terminal-url-link-click.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-url-link-click.test.ts @@ -379,6 +379,44 @@ describe('hard-wrapped terminal HTTP clicks', () => { expect(openUrlMock).toHaveBeenCalledWith('http://example.com/') }) + it('does not glue the next logical line onto a URL that ends mid-row (#8832)', () => { + const { terminal, registrations } = makeTerminal({ + cols: 80, + urlRows: ['Repo: https://github.com/stablyai/orca/', 'Description: 123'], + softWrapped: false + }) + const disposable = installHttpLinkClickFallback(terminal, { worktreeId: 'wt-1' }) + + const fallback = registrations.find( + ([name, _listener, options]) => name === 'mouseup' && options === undefined + )?.[1] + fallback!(mouseEventForRow(0)) + + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith('https://github.com/stablyai/orca/') + disposable.dispose() + }) + + it('still joins a URL hard-wrapped at the row edge without native wrap metadata', () => { + const cols = 40 + const url = 'https://example.com/very/long/path/segments/that/continue/more' + const { terminal, registrations } = makeTerminal({ + cols, + urlRows: [url.slice(0, cols), url.slice(cols)], + softWrapped: false + }) + const disposable = installHttpLinkClickFallback(terminal, { worktreeId: 'wt-1' }) + + const fallback = registrations.find( + ([name, _listener, options]) => name === 'mouseup' && options === undefined + )?.[1] + fallback!(mouseEventForRow(0)) + + expect(openUrlMock).toHaveBeenCalledOnce() + expect(openUrlMock).toHaveBeenCalledWith(url) + disposable.dispose() + }) + it('does not suppress a modifier-click when the buffer position is not an HTTP link', () => { const { terminal, registrations } = makeTerminal({ urlRows: ['not-a-link'] }) const disposable = installHttpLinkClickFallback(terminal, { worktreeId: 'wt-1' }) diff --git a/src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.ts b/src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.ts index bfbbe29f723..3a6e2c1f274 100644 --- a/src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.ts +++ b/src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.ts @@ -1,10 +1,8 @@ import type { IBufferLine, IBufferRange, IDisposable, Terminal } from '@xterm/xterm' import { openHttpLink } from '@/lib/http-link-routing' -import { - buildCandidateLogicalLinesForBufferPosition, - dedupeLogicalLines -} from './terminal-file-link-hit-testing' +import { buildEdgeWrappedHttpLogicalLineCandidates } from './edge-wrapped-terminal-http-links' import { buildHardWrappedHttpLogicalLineCandidates } from './hard-wrapped-terminal-http-links' +import { dedupeLogicalLines } from './terminal-file-link-hit-testing' import { isTerminalHttpLinkActivation } from './terminal-http-link-activation' import { installTerminalLinkPtyMouseSuppression } from './terminal-link-pty-mouse-suppression' import { getTerminalBufferPositionForMouseEvent } from './terminal-mouse-buffer-position' @@ -271,7 +269,10 @@ function findHttpLinkAtBufferPosition( ? [nativeWrappedLogicalLine] : []), ...buildHardWrappedHttpLogicalLineCandidates(buffer, position.y), - ...buildCandidateLogicalLinesForBufferPosition(buffer, position.y) + ...buildEdgeWrappedHttpLogicalLineCandidates(buffer, position.y), + ...(nativeWrappedLogicalLine && nativeWrappedLogicalLine.rows.length === 1 + ? [nativeWrappedLogicalLine] + : []) ]) if (logicalLines.length === 0) { return null @@ -290,6 +291,17 @@ function findHttpLinkAtBufferPosition( return null } +function rangeContainsBufferPosition( + range: IBufferRange, + position: { x: number; y: number }, + terminalColumns: number +): boolean { + const lower = range.start.y * terminalColumns + range.start.x + const upper = range.end.y * terminalColumns + range.end.x + const current = position.y * terminalColumns + position.x + return lower <= current && current <= upper +} + export function openTerminalHttpLink(url: string, deps: UrlLinkHitTestDeps): void { if (deps.forceSystemBrowser) { openHttpLink(url, { worktreeId: deps.worktreeId, forceSystemBrowser: true }) @@ -316,14 +328,3 @@ export function openTerminalHttpLink(url: string, deps: UrlLinkHitTestDeps): voi openHttpLink(url, { worktreeId: deps.worktreeId, forceSystemBrowser: true }) }) } - -function rangeContainsBufferPosition( - range: IBufferRange, - position: { x: number; y: number }, - terminalColumns: number -): boolean { - const lower = range.start.y * terminalColumns + range.start.x - const upper = range.end.y * terminalColumns + range.end.x - const current = position.y * terminalColumns + position.x - return lower <= current && current <= upper -}