From add1778e4ad72c162bc209eed8a1a200852e2fe2 Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Fri, 30 Oct 2026 22:47:25 +0000 Subject: [PATCH] fix(terminal): link a URL wrapped inside a table cell on every row (#36482) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(terminal): link a URL wrapped inside a table cell A markdown table rendered by an agent TUI wraps a long URL onto extra rows of its own cell, leaving the sibling cells blank. Two gates in the hard-wrapped HTTP builder rejected that layout, so the link stayed cut at the first row and opened a truncated URL: - a changed prefix under a second left frame was always read as an adjacent table cell, and - only the padding up to the URL's own right frame is cell-local, so a populated sibling cell to the right killed the row. Treat a blank gutter between the frames as the same cell continuing, and only require the URL's own cell padding to be blank. A blank-gutter continuation is also enough evidence to join a two-row URL, which the previous three-row minimum ruled out. * fix(terminal): join a two-row URL in the first table column A URL that starts in the left table column continues through the unchanged-prefix branch, so no blank sibling cell marks its continuation row and the row-count guard dropped the second row. Count the blank cells right of the URL as continuation evidence for that layout too, and keep single-column tables on the three-row minimum so a complete URL that happens to end at the cell border is not glued onto the next table row. * fix(terminal): join a table-cell URL only when its row's other cells are blank A wrapped table cell leaves every other cell on its continuation rows blank, so that one rule now decides a two-row join in any column. This replaces the adjacent-cell gutter count and the first-column 0.95 border threshold. Checking only the adjacent cell glued a next table row like `│ 2 │ │ notes │` onto the URL above it. * fix(terminal): end a table-cell URL at a row whose right-hand cells are filled Below the URL's first row, a filled cell to its right starts a new table row. Text beside an overlay with no table frame is not a sibling cell either. * fix(terminal): end a table-cell URL where the cells left of it change Every continuation row of a wrapped cell shows the same cells to its left, so a row that fills them again (a repeated label) starts a new table row. * fix(terminal): apply table-cell rules only to rows that keep the table frame An overlay's gutter can show a frame beside its first row only; those rows vary by design and keep the overlay join. * fix(terminal): count cells right of a URL as a table only when they close with a frame A split pane or screen text beside an overlay ends without a closing frame and is not a sibling cell. --------- Co-authored-by: ZheNing Hu --- ...rapped-terminal-http-link-provider.test.ts | 50 ++++++++++++++ .../hard-wrapped-terminal-http-links.test.ts | 67 +++++++++++++++++++ .../hard-wrapped-terminal-http-links.ts | 41 ++++++++++-- 3 files changed, 152 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/wrapped-terminal-http-link-provider.test.ts b/src/renderer/src/components/terminal-pane/wrapped-terminal-http-link-provider.test.ts index ebde32ea15d..095aa0298f7 100644 --- a/src/renderer/src/components/terminal-pane/wrapped-terminal-http-link-provider.test.ts +++ b/src/renderer/src/components/terminal-pane/wrapped-terminal-http-link-provider.test.ts @@ -426,6 +426,56 @@ describe('wrapped terminal HTTP link provider', () => { } }) + it.each([ + [ + 'the last', + [ + '│ 1 │ doc │ https://example.com/a/very/lon │', + '│ │ │ g/path/that/keeps/going/and/go │', + '│ │ │ ing │', + '│ 2 │ api │ https://example.com/docs/refer │', + '│ │ │ ence/terminal-links?v=2#anchor │' + ] + ], + [ + 'a middle', + [ + '│ 1 │ https://example.com/a/very/lon │ done │', + '│ │ g/path/that/keeps/going/and/go │ │', + '│ │ ing │ │', + '│ 2 │ https://example.com/docs/refer │ todo │', + '│ │ ence/terminal-links?v=2#anchor │ │' + ] + ], + [ + 'the first', + [ + '│ https://example.com/a/very/lon │ done │', + '│ g/path/that/keeps/going/and/go │ │', + '│ ing │ │', + '│ https://example.com/docs/refer │ todo │', + '│ ence/terminal-links?v=2#anchor │ │' + ] + ] + ])('links a URL wrapped inside %s table column on every row of its cell', async (_, rows) => { + const urls = [ + 'https://example.com/a/very/long/path/that/keeps/going/and/going', + 'https://example.com/docs/reference/terminal-links?v=2#anchor' + ] + const x = rows[0]!.indexOf('https') + 1 + const { terminal, linksAt } = await terminalWithRows(rows, 60) + try { + for (const [index] of rows.entries()) { + expect((await linksAt(index + 1))?.[0]).toMatchObject({ + text: urls[index < 3 ? 0 : 1], + range: { start: { x, y: index + 1 } } + }) + } + } finally { + terminal.dispose() + } + }) + it('maps a padded wide-character continuation to its terminal cells', async () => { const firstRow = `https://example.com/${'a'.repeat(18)}` const continuation = '界文档/路径' diff --git a/src/shared/hard-wrapped-terminal-http-links.test.ts b/src/shared/hard-wrapped-terminal-http-links.test.ts index 691843c8fe0..2de9f80c935 100644 --- a/src/shared/hard-wrapped-terminal-http-links.test.ts +++ b/src/shared/hard-wrapped-terminal-http-links.test.ts @@ -125,6 +125,25 @@ describe('hard-wrapped terminal HTTP candidate bounds', () => { ) }) + it('reconstructs an overlay URL when only its first row shows a frame in the gutter', () => { + const inner = 'http://example.com/aaaa' + const rows = [ + `ls | g|${inner}|`, + `todo |${'b'.repeat(inner.length)}|`, + ` in |${'c'.repeat(inner.length)}|`, + `Final |${'dddd'.padEnd(inner.length, ' ')}|` + ].map(bufferLineWithCellColumns) + + const candidates = buildHardWrappedHttpLogicalLineCandidates( + { getLine: (y) => rows[y] }, + rows.length + ) + + expect(candidates[0]?.text).toBe( + `${inner}${'b'.repeat(inner.length)}${'c'.repeat(inner.length)}dddd` + ) + }) + it('reconstructs a URL in a nested frame whose left label repeats on every row', () => { const inner = 'http://example.com/aaaa' const rows = [ @@ -163,6 +182,54 @@ describe('hard-wrapped terminal HTTP candidate bounds', () => { ) }) + it.each([ + [ + 'a single-column table', + ['│ https://example.com/aaaaaaaaaa │', '│ README.md │'] + ], + [ + 'rows with filled cells right of it', + [ + '│ https://example.com/aaaaaaaaaa │ ok │', + '│ src/renderer/terminal-pane.tsx │ todo │', + '│ README.md │ done │' + ] + ], + [ + 'a row labelled further left', + [ + '│ 1 │ doc │ https://example.com/aaaaaaaaaa │', + '│ 2 │ │ notes │' + ] + ], + [ + 'an overlay over screen text', + ['todo |http://example.com/aaaa|', ' |README.md |'] + ], + [ + 'a box beside an open split pane', + ['│ https://example.com/aaaaaaaaaa │ │ $ ls', '│ README.md │ │'] + ] + ])('does not join a URL that fills its cell onto the next rows in %s', (_, rowTexts) => { + const rows = rowTexts.map(bufferLineWithCellColumns) + + expect( + buildHardWrappedHttpLogicalLineCandidates({ getLine: (y) => rows[y] }, rows.length) + ).toEqual([]) + }) + + it('ends a wrapped cell URL at the next row that fills a cell left of it again', () => { + const rows = [ + '│ doc │ https://example.com/aaaaaaaaaa │', + '│ │ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb │', + '│ doc │ README.md │' + ].map(bufferLineWithCellColumns) + + expect(buildHardWrappedHttpLogicalLineCandidates({ getLine: (y) => rows[y] }, 2)[0]?.text).toBe( + `https://example.com/aaaaaaaaaa${'b'.repeat(30)}` + ) + }) + it('does not join adjacent cells from a shared framed table', () => { const rows = [ '│ Label │ https://example.com/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa │', diff --git a/src/shared/hard-wrapped-terminal-http-links.ts b/src/shared/hard-wrapped-terminal-http-links.ts index bf0d13c2de9..0609e03078e 100644 --- a/src/shared/hard-wrapped-terminal-http-links.ts +++ b/src/shared/hard-wrapped-terminal-http-links.ts @@ -9,6 +9,7 @@ const HTTP_SCHEME_PATTERN = /https?:\/\//i const HTTP_SCHEME_START_PATTERN = /^https?:\/\//i const HTTP_FRAGMENT_PATTERN = /^[^\s"'!*(){}|\\^<>`│┃║╎╏┆┇┊┋]*/ const VERTICAL_LAYOUT_FRAME_PATTERN = /[│┃║╎╏┆┇┊┋|]/ +const CLOSING_LAYOUT_FRAME_PATTERN = /[│┃║╎╏┆┇┊┋|]\s*$/ const NON_LAYOUT_SUFFIX_PATTERN = /[^\s│┃║╎╏┆┇┊┋|]/ const HARD_WRAP_CONTINUATION_SUFFIX_PATTERN = /[/?&=#%+:-]$/ const LAYOUT_SUFFIX_CHAR_PATTERN = /[\s│┃║╎╏┆┇┊┋|]/ @@ -54,14 +55,18 @@ function findOverlayFragmentStart( outerFrameColumn: number | null, framedPrefix: string ): number { - // A second stable left frame under a changed prefix denotes adjacent table cells. - if (outerFrameColumn !== null && hasFrameAtColumn(translated, outerFrameColumn)) { - return -1 - } const frameIndex = translated.columns.indexOf(frameColumn) if (frameIndex === -1) { return -1 } + // Table cells left of the URL: blank ones continue its wrapped cell, filled ones start a new row. + if ( + outerFrameColumn !== null && + hasFrameAtColumn(translated, outerFrameColumn) && + NON_LAYOUT_SUFFIX_PATTERN.test(translated.text.slice(0, frameIndex)) + ) { + return -1 + } const prefixEnd = frameIndex + framedPrefix.length return translated.text.slice(frameIndex, prefixEnd) === framedPrefix ? prefixEnd : -1 } @@ -101,6 +106,8 @@ function buildCandidateFromStart( let rightFrameColumn: number | null = null let previousRowCanContinue = true let startRowFilled = false + let isWrappedTableCell = false + let continuationLeftCells: string | null = null const rows: WrappedLogicalLine['rows'] = [] for (let rowY = startY; rowY < startY + TERMINAL_HTTP_URL_MAX_HARD_WRAPPED_ROWS; rowY++) { @@ -127,6 +134,12 @@ function buildCandidateFromStart( if (fragmentStartColumn === undefined) { break } + const leftCells = translated.text.slice(0, fragmentStart - framedPrefix.length) + const isTableRow = outerFrameColumn !== null && hasFrameAtColumn(translated, outerFrameColumn) + // A wrapped table cell shows the same cells to its left on every continuation row; a change starts a new row. + if (rowY > startY + 1 && isTableRow && leftCells !== continuationLeftCells) { + break + } const fragment = translated.text.slice(fragmentStart).match(HTTP_FRAGMENT_PATTERN)?.[0] ?? '' if (!fragment || (rowY > startY && HTTP_SCHEME_START_PATTERN.test(fragment))) { @@ -137,11 +150,13 @@ function buildCandidateFromStart( const rightFrameOffset = layoutSuffix.search(VERTICAL_LAYOUT_FRAME_PATTERN) const currentRightFrameIndex = rightFrameOffset === -1 ? -1 : fragmentEnd + rightFrameOffset const currentRightFrameColumn = translated.columns[currentRightFrameIndex] + // Cells right of the URL may be filled on its first row only; below it they start a new table row. + const rowSuffix = rowY === startY ? layoutSuffix.slice(0, rightFrameOffset) : layoutSuffix if ( currentRightFrameIndex === -1 || currentRightFrameColumn === undefined || (rightFrameColumn !== null && currentRightFrameColumn !== rightFrameColumn) || - NON_LAYOUT_SUFFIX_PATTERN.test(layoutSuffix) + NON_LAYOUT_SUFFIX_PATTERN.test(rowSuffix) ) { break } @@ -166,12 +181,26 @@ function buildCandidateFromStart( const fragmentWidth = translated.columns[fragmentEnd]! - fragmentStartColumn const fillsRow = contentWidth > 0 && fragmentWidth / contentWidth >= MIN_HARD_WRAP_FILL_RATIO if (rowY === startY) { + const rightCells = translated.text.slice(currentRightFrameIndex + 1) startRowFilled = fillsRow + // Only framed cells count; screen text around an overlay is not a table row. + isWrappedTableCell = + (isTableRow && NON_LAYOUT_SUFFIX_PATTERN.test(leftCells)) || + (CLOSING_LAYOUT_FRAME_PATTERN.test(rightCells) && + NON_LAYOUT_SUFFIX_PATTERN.test(rightCells)) + } else { + continuationLeftCells = leftCells + isWrappedTableCell &&= + (outerFrameColumn === null || isTableRow) && !NON_LAYOUT_SUFFIX_PATTERN.test(leftCells) } previousRowCanContinue = HARD_WRAP_CONTINUATION_SUFFIX_PATTERN.test(fragment) || fillsRow } - if (rows.length > 1 && (rows.length < MIN_HARD_WRAPPED_HTTP_ROWS || !startRowFilled)) { + // A wrapped table cell blanks its row's other cells, which proves continuation without a third row. + if ( + rows.length > 1 && + (!startRowFilled || (rows.length < MIN_HARD_WRAPPED_HTTP_ROWS && !isWrappedTableCell)) + ) { // A complete URL may end in /; one adjacent framed token alone cannot prove continuation. rows.splice(1) text = rows[0]?.text ?? ''