From 5ce8213726d748020b03a637e54a7a287fb74fb4 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Wed, 28 Oct 2026 20:30:10 +0800 Subject: [PATCH] fix(links): preserve wrapped configuration filenames --- .changeset/Repair-FilePathLinks.md | 5 ++ BUGS.md | 1 + config/test-suites.ts | 1 + src/web/public/constants.js | 53 +++++++++++----- test/file-path-link-forms.test.ts | 42 +++++++++++-- test/response-viewer-file-links.test.ts | 9 +++ test/spaced-file-path-links.test.ts | 14 +++++ test/wrapped-file-paths.browser.test.ts | 82 +++++++++++++++++++++++++ 8 files changed, 189 insertions(+), 18 deletions(-) create mode 100644 .changeset/Repair-FilePathLinks.md create mode 100644 BUGS.md create mode 100644 test/wrapped-file-paths.browser.test.ts diff --git a/.changeset/Repair-FilePathLinks.md b/.changeset/Repair-FilePathLinks.md new file mode 100644 index 00000000..d187e66b --- /dev/null +++ b/.changeset/Repair-FilePathLinks.md @@ -0,0 +1,5 @@ +--- +'aicodeman': patch +--- + +Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. Preserve the filename's space and clickable row positions when a CLI wraps the path onto another line, instead of opening a truncated folder name. The same detector serves the terminal and Response Viewer, with the existing path guards retained. diff --git a/BUGS.md b/BUGS.md new file mode 100644 index 00000000..9264185e --- /dev/null +++ b/BUGS.md @@ -0,0 +1 @@ +Bug tracker: [Codeman issues](https://github.com/Ark0N/Codeman/issues). diff --git a/config/test-suites.ts b/config/test-suites.ts index ae974542..4760e091 100644 --- a/config/test-suites.ts +++ b/config/test-suites.ts @@ -19,6 +19,7 @@ * why these are a runnable suite (`npm run test:browser`) rather than skipped. */ export const BROWSER_TEST_GLOBS = [ + 'test/wrapped-file-paths.browser.test.ts', 'test/command-palette-agent-arrows.browser.test.ts', 'test/tmux-capture-sgr.browser.test.ts', 'test/tab-rail-resize.browser.test.ts', diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 742ac363..62c3dade 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2106,7 +2106,7 @@ function computeRewriteScrollLine(input) { * a `/g` regex, so {@link absoluteFilePathPattern} mints a fresh one per call. */ const FILE_PATH_LINK_PATTERN = - /(\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|markdown|md|ya?ml|csv|tsv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html?|toml|ini|sql|diff|patch|png|jpe?g|gif|webp|avif|bmp|ico|svg|pdf|docx|pptx|xlsx|zip|mp4|webm|mov|m4v|ogv|mp3|wav))\b/g; + /(\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|markdown|md|ya?ml|csv|tsv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html?|toml|ini|conf|sql|diff|patch|png|jpe?g|gif|webp|avif|bmp|ico|svg|pdf|docx|pptx|xlsx|zip|mp4|webm|mov|m4v|ogv|mp3|wav))\b/g; /** A fresh, zero-state instance of {@link FILE_PATH_LINK_PATTERN}. */ function absoluteFilePathPattern() { @@ -2159,7 +2159,7 @@ function previewsInFileViewer(filePath) { * {@link findFilePathLinks}. */ const RELATIVE_FILE_PATH_LINK_PATTERN = - /(^|[\s"'`(\[{<>=,])((?![^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*:\/\/)(?:~\/|[^\s"'`<>|;&()[\]{}=,\/~\n\x00-\x1f])[^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*\.(?:log|txt|json|markdown|md|ya?ml|csv|tsv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html?|toml|ini|sql|diff|patch|png|jpe?g|gif|webp|avif|bmp|ico|svg|pdf|docx|pptx|xlsx|zip|mp4|webm|mov|m4v|ogv|mp3|wav))\b/g; + /(^|[\s"'`(\[{<>=,])((?![^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*:\/\/)(?:~\/|[^\s"'`<>|;&()[\]{}=,\/~\n\x00-\x1f])[^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*\.(?:log|txt|json|markdown|md|ya?ml|csv|tsv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html?|toml|ini|conf|sql|diff|patch|png|jpe?g|gif|webp|avif|bmp|ico|svg|pdf|docx|pptx|xlsx|zip|mp4|webm|mov|m4v|ogv|mp3|wav))\b/g; /** The `(?:log|txt|…)` extension group, read off the rooted pattern so it cannot drift. */ const FILE_LINK_EXTENSION_GROUP = /\\\.(\(\?:[^()]*\))\)\\b$/.exec(FILE_PATH_LINK_PATTERN.source)[1]; @@ -2195,19 +2195,25 @@ function findFilePathLinks(text) { // A path with spaces is taken whole, or the span it covers links nothing // (spacedFilePathOnLine). Either way no detector below may cut a fragment out of it. const reserved = []; + const isReserved = (index, length) => + reserved.some((r) => index < r.index + r.path.length && r.index < index + length); + const reserve = (spaced, index) => { + if (!spaced) return; + const start = index + spaced.index; + if (inUrl(start, spaced.path.length) || isReserved(start, spaced.path.length)) return; + reserved.push({ index: start, path: spaced.path }); + if (spaced.link) found.push({ path: spaced.path, index: start }); + }; let lineStart = 0; for (const line of value.split('\n')) { - const spaced = spacedFilePathOnLine(line); - if (spaced && !inUrl(lineStart + spaced.index, spaced.path.length)) { - const index = lineStart + spaced.index; - reserved.push({ index, path: spaced.path }); - if (spaced.link) found.push({ path: spaced.path, index }); + reserve(spacedFilePathOnLine(line), lineStart); + // Parentheses bound an inline path just as a whole line bounds a standalone path. + const parenthesised = /\(([^()\r\n]{1,1000})\)/g; + while ((match = parenthesised.exec(line)) !== null) { + reserve(spacedFilePathOnLine(match[1]), lineStart + match.index + 1); } lineStart += line.length + 1; } - const isReserved = (index, length) => - reserved.some((r) => index < r.index + r.path.length && r.index < index + length); - const absolute = absoluteFilePathPattern(); while ((match = absolute.exec(value)) !== null) { if (!inUrl(match.index, match[1].length) && !isReserved(match.index, match[1].length)) { @@ -2548,7 +2554,8 @@ const pathFirstSegment = (path) => path.replace(/^(?:\.?\/)+/, '').split('/')[0] * Whether the line `nextText` carries on a file path `prevText` broke off. * * Answers `'join'` when the path ends (reaches a known extension) on the next - * line, `'chain'` when the next line is nothing but more of the path (a path + * line, `'join-spaced'` for a filename word wrapped inside parentheses, and + * `'chain'` when the next line is nothing but more of the path (a path * spanning three or more lines; only worth keeping once a later line ends it), * and `null` otherwise. The broken-off token must already look like a path in * progress: it contains a `/`, ends on a path character and does not yet end @@ -2557,9 +2564,25 @@ const pathFirstSegment = (path) => path.replace(/^(?:\.?\/)+/, '').split('/')[0] * top directory the broken token did (the next entry of a listing). */ function pathContinuation(prevText, nextText) { - const tail = /[^\s"'<>|;&]+$/.exec(String(prevText || '').replace(/\s+$/, ''))?.[0] || ''; - if (!tail.includes('/') || !/[\w\-.~+@%/]$/.test(tail) || PATH_TOKEN_ENDS_WITH_EXTENSION.test(tail)) return null; + const previous = String(prevText || '').trimEnd(); const next = String(nextText || '').replace(/^\s+/, ''); + const opening = previous.lastIndexOf('('); + const closing = next.indexOf(')'); + if (opening >= 0 && closing > 0) { + const before = previous.slice(opening + 1); + const after = next.slice(0, closing); + // A word wrap in a parenthesised filename replaces its separating space. + if ( + !before.includes(')') && + !/[/-]$/.test(before) && + !after.includes('/') && + spacedFilePathOnLine(`${before} ${after}`)?.link + ) { + return 'join-spaced'; + } + } + const tail = /[^\s"'<>|;&]+$/.exec(previous)?.[0] || ''; + if (!tail.includes('/') || !/[\w\-.~+@%/]$/.test(tail) || PATH_TOKEN_ENDS_WITH_EXTENSION.test(tail)) return null; const head = /^[^\s"'<>|;&]+/.exec(next)?.[0] || ''; if (!head || PATH_TOKEN_STARTS_NEW_PATH.test(head) || PATH_TOKEN_STARTS_EXPLICIT_RELATIVE.test(head)) return null; // A listing (`git ls-files`, `find`, `ls -R`) puts one path per row, so `src/components` @@ -2616,9 +2639,10 @@ function terminalPathLine(buffer, row, cols, maxRows) { const kind = continues(parts[parts.length - 1], next, text); if (!kind) break; parts.push(next); + if (kind === 'join-spaced') text += ' '; text += next.text.replace(/^\s+/, ''); rows += rowsOf(next); - if (kind === 'join') confirmed = parts.length; + if (kind !== 'chain') confirmed = parts.length; } return parts.slice(0, confirmed); }; @@ -2640,6 +2664,7 @@ function terminalPathLine(buffer, row, cols, maxRows) { const segments = []; let text = ''; for (const [i, part] of parts.entries()) { + if (i > 0 && continues(parts[i - 1], part, text) === 'join-spaced') text += ' '; const skip = i === 0 ? 0 : part.text.length - part.text.replace(/^\s+/, '').length; segments.push({ part, textStart: text.length, skip }); text += part.text.slice(skip); diff --git a/test/file-path-link-forms.test.ts b/test/file-path-link-forms.test.ts index e601bc9e..0383459c 100644 --- a/test/file-path-link-forms.test.ts +++ b/test/file-path-link-forms.test.ts @@ -52,13 +52,13 @@ const { findFilePathLinks, resolveLinkedFilePath, terminalPathLine } = loadHelpe const paths = (text: string) => findFilePathLinks(text).map((f) => f.path); /** An xterm buffer stand-in: hard-broken rows (no `isWrapped`), as Claude Code emits them. */ -function buffer(lines: string[], cols = 80) { +function buffer(lines: string[], cols = 80, wrappedRows = new Set()) { return { length: lines.length, getLine: (row: number) => row >= 0 && row < lines.length ? { - isWrapped: false, + isWrapped: wrappedRows.has(row), translateToString: (trim?: boolean) => (trim ? lines[row].replace(/\s+$/, '') : lines[row].padEnd(cols)), } : undefined, @@ -157,6 +157,10 @@ describe('terminalPathLine', () => { // A path that already ended, and a next line opening a new rooted path. expect(terminalPathLine(buffer(['see /tmp/a/b.png', 'c/d.png']), 0, 80)).toMatchObject({ endRow: 0 }); expect(terminalPathLine(buffer(['/home/joe/dir/', '/home/x.png']), 0, 80)).toMatchObject({ endRow: 0 }); + expect(terminalPathLine(buffer(['see (/mnt/raid1/folder)', ' file.conf)']), 0, 80)).toMatchObject({ endRow: 0 }); + expect(terminalPathLine(buffer(['see (/mnt/raid1/folder', ' /mnt/other/file.conf)']), 0, 80)).toMatchObject({ + endRow: 0, + }); }); // `git ls-files`, `find` and `ls -R` print one path per row. An extensionless @@ -241,9 +245,9 @@ function loadApp() { return app; } -function linksOn(lines: string[], row: number) { +function linksOn(lines: string[], row: number, cols = 80, wrappedRows = new Set()) { const app = loadApp(); - app.terminal = { cols: 80, registerLinkProvider: vi.fn(), buffer: { active: buffer(lines) } }; + app.terminal = { cols, registerLinkProvider: vi.fn(), buffer: { active: buffer(lines, cols, wrappedRows) } }; const provider = app.registerFilePathLinkProvider() as Provider; let out: Link[] = []; provider.provideLinks(row, (links) => { @@ -253,6 +257,36 @@ function linksOn(lines: string[], row: number) { } describe('terminal link provider', () => { + it('retains the filename space when the CLI word-wraps a parenthesised path', () => { + const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + const rows = [ + 'Found it: I prepared a corrected configuration (/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo', + ' eReader.conf).', + ]; + for (const row of [1, 2]) { + const { app, links } = linksOn(rows, row, 120); + expect(links.map((link) => link.text)).toEqual([path]); + expect(links[0].range.end).toEqual({ x: ' eReader.conf'.length + 1, y: 2 }); + links[0].activate({}, links[0].text); + expect(app.openLogViewerWindow).toHaveBeenCalledWith(path, 's1'); + } + }); + + it('opens the complete parenthesised configuration path from either wrapped row', () => { + const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + const text = `Found it: I prepared a corrected configuration (${path}).`; + const split = text.indexOf(' eReader') + 1; + const rows = [text.slice(0, split), text.slice(split)]; + for (const row of [1, 2]) { + const { app, links } = linksOn(rows, row, split, new Set([1])); + expect(links.map((link) => link.text)).toEqual([path]); + expect(links[0].range.start).toEqual({ x: text.indexOf(path) + 1, y: 1 }); + expect(links[0].range.end).toEqual({ x: 'eReader.conf'.length + 1, y: 2 }); + links[0].activate({}, links[0].text); + expect(app.openLogViewerWindow).toHaveBeenCalledWith(path, 's1'); + } + }); + it('links a broken path once, across both rows, and opens the whole path', () => { const { app, links } = linksOn(BROKEN, 2); expect(links.map((l) => l.text)).toEqual([BROKEN_PATH]); diff --git a/test/response-viewer-file-links.test.ts b/test/response-viewer-file-links.test.ts index 0eff0b35..723d233b 100644 --- a/test/response-viewer-file-links.test.ts +++ b/test/response-viewer-file-links.test.ts @@ -78,6 +78,15 @@ function linkify(html: string): HTMLElement { const paths = (root: HTMLElement) => Array.from(root.querySelectorAll('a.rv-path')); describe('response viewer file-path linkifier', () => { + it('links a parenthesised configuration filename with spaces without changing the prose', () => { + const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + const text = `Prepared a corrected configuration (${path}).`; + const root = linkify(`

${text}

`); + expect(paths(root).map((a) => a.dataset.path)).toEqual([path]); + expect(paths(root)[0].dataset.folder).toBeUndefined(); + expect(root.textContent).toBe(text); + }); + it.each([ ['- 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx', ['Design/Build-SIPOC.pptx']], ['- notes/a b/c.md', ['b/c.md']], diff --git a/test/spaced-file-path-links.test.ts b/test/spaced-file-path-links.test.ts index ead0aaf9..5b08e185 100644 --- a/test/spaced-file-path-links.test.ts +++ b/test/spaced-file-path-links.test.ts @@ -28,6 +28,20 @@ const LONG = '/mnt/c/Users/someone/OneDrive - Contoso/01_Projects/Other/kb/out/deliverables/fy27-messaging-market-story.docx'; describe('spaced file paths', () => { + it('keeps a parenthesised configuration filename whole inside a sentence', () => { + const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + const text = `Prepared a corrected configuration (${path}).`; + expect(findFilePathLinks(text)).toEqual([{ path, index: text.indexOf(path) }]); + }); + + it('keeps separate parenthesised paths and the surrounding prose separate', () => { + const first = '/mnt/data/Kobo/Kobo eReader.conf'; + const second = '/home/me/My Docs/notes.md'; + expect(paths(`Updated (${first}) and (${second}).`)).toEqual([first, second]); + expect(paths('Do not link (/etc/private files/settings.conf).')).toEqual([]); + expect(paths('Do not link (/mnt/data/../private files/settings.conf).')).toEqual([]); + }); + it('links an own-line absolute path with spaces whole', () => { expect(paths(LONG)).toEqual([LONG]); expect(paths('/mnt/c/proj/My Big Docs/report.docx')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']); diff --git a/test/wrapped-file-paths.browser.test.ts b/test/wrapped-file-paths.browser.test.ts new file mode 100644 index 00000000..1a0e9f19 --- /dev/null +++ b/test/wrapped-file-paths.browser.test.ts @@ -0,0 +1,82 @@ +/** Real xterm clicks must open the complete spaced filename from either wrapped row. */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { chromium, type Browser } from 'playwright'; + +const publicDir = resolve(import.meta.dirname, '../src/web/public'); +const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + +describe('wrapped configuration file links (Chromium)', () => { + let browser: Browser; + beforeAll(async () => { + browser = await chromium.launch({ headless: true }); + }); + afterAll(async () => { + await browser.close(); + }); + + it.each(['soft', 'hard'])('opens the complete file from both rows of a %s wrap', async (wrap) => { + const page = await browser.newPage({ viewport: { width: 1920, height: 600 } }); + try { + await page.setContent('
'); + await page.addStyleTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/css/xterm.css') }); + await page.addScriptTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/lib/xterm.js') }); + await page.addScriptTag({ content: 'window.CodemanApp = class CodemanApp {};' }); + for (const file of ['constants.js', 'terminal-ui.js']) { + await page.addScriptTag({ content: readFileSync(resolve(publicDir, file), 'utf8') }); + } + await page.evaluate( + async ({ path, wrap }) => { + const w = window as any; + const first = `Found it: prepared a corrected configuration (${path.slice(0, -' eReader.conf'.length)}`; + const terminal = new w.Terminal({ cols: first.length + (wrap === 'soft' ? 1 : 20), rows: 6 }); + terminal.open(document.getElementById('terminal')); + w.opened = []; + w.app = Object.assign(new w.CodemanApp(), { + terminal, + activeSessionId: 'fixture', + sessions: new Map([['fixture', { workingDir: '/workspace' }]]), + _isExternalPreviewPath: () => true, + openFilePreview: (file: string) => w.opened.push(file), + openLogViewerWindow: (file: string) => w.opened.push(`log:${file}`), + openFolderGrid: (file: string) => w.opened.push(`folder:${file}`), + }); + w.app.registerFilePathLinkProvider(); + const text = `${first}${wrap === 'soft' ? ' ' : '\r\n '}eReader.conf).`; + await new Promise((done) => terminal.write(text, done)); + await new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done()))); + }, + { path, wrap } + ); + + for (const row of [0, 1]) { + const point = await page.evaluate( + async ({ path, row }) => { + const app = (window as any).app; + const terminal = app.terminal; + const links = await new Promise((done) => app._terminalLinkProvider.provideLinks(row + 1, done)); + const link = links.find((candidate) => candidate.text === path); + if (!link) throw new Error(`The complete filename is not linked on row ${row + 1}`); + const text = terminal.buffer.active.getLine(row).translateToString(true); + const col = row === 0 ? link.range.start.x : text.indexOf('eReader') + 1; + const box = terminal.element.querySelector('.xterm-screen').getBoundingClientRect(); + return { + x: box.left + (col - 0.5) * (box.width / terminal.cols), + y: box.top + (row + 0.5) * (box.height / terminal.rows), + }; + }, + { path, row } + ); + await page.mouse.move(point.x, point.y); + await page.waitForFunction(() => (window as any).app._linkHovered === true); + await page.mouse.click(point.x, point.y); + await page.waitForFunction((count) => (window as any).opened.length === count, row + 1); + expect(await page.evaluate(() => (window as any).opened)).toEqual(Array(row + 1).fill(path)); + await page.mouse.move(0, 0); + } + } finally { + await page.close(); + } + }); +});