diff --git a/.changeset/Repair-FilePathLinks.md b/.changeset/Repair-FilePathLinks.md index 3418dccc..70c3e620 100644 --- a/.changeset/Repair-FilePathLinks.md +++ b/.changeset/Repair-FilePathLinks.md @@ -5,3 +5,5 @@ Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. For an ambiguous hard wrap, check the original concatenated filename through the existing guarded attachment route when clicked, and try the spaced filename only after a not-found response. Every original response other than not-found retains the normal opener, including workspace files whose extensions the attachment registry rejects. A spaced alternative opens only after successful registration. Preserve clickable row positions and share the spaced-path detector between the terminal and Response Viewer. Restrict comma-separated-list detection to parenthesised candidates so existing own-line, list-marker and labelled paths with commas remain whole. A rejected workspace path is named in the attachment-lookup error without describing it as outside the workspace. + +Keep the normal opener for the original filename when both hard-wrap candidates are missing, and exclude path labels and list markers inside parentheses from the lookup key. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index cf823d05..1fdafa96 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -415,7 +415,7 @@ Spaced paths are considered first by `findFilePathLinks()`: accepted candidates Parentheses also bound an inline spaced path, such as `Updated (/mnt/c/My Docs/report.conf)`. The whole candidate is reserved before fragment detectors run; existing root, prose and traversal guards still apply. Inside parentheses only, a comma at the end of an earlier word marks a list, not one filename. Own-line, list-marker and labelled paths retain commas in filenames and directory names. -Hard-wrapped filenames keep the existing concatenation without inserted spaces. A parenthesised final filename broken between words also carries a spaced alternative without changing its cell mapping. On activation only, the terminal checks the original path through the existing guarded attachment registration route (`notify: false`), tries the alternative only after HTTP 404, and retains the clicked session throughout. If both exist, the original wins. Every original result other than HTTP 404 goes to the normal opener, preserving workspace previews and log viewing even for extensions the attachment registry rejects; for an external path that opener retains the same attachment guards and errors. After an original 404, a spaced alternative opens only if it registers successfully. Hover performs no lookup. Text alone cannot distinguish the two filenames, so this uses the filesystem result rather than a spacing guess. +Hard-wrapped filenames keep the existing concatenation without inserted spaces. A parenthesised final filename broken between words also carries a spaced alternative without changing its cell mapping. On activation only, the terminal checks the original path through the existing guarded attachment registration route (`notify: false`), tries the alternative only after HTTP 404, and retains the clicked session throughout. If both exist, the original wins. Every original result other than HTTP 404 goes to the normal opener, preserving workspace previews and log viewing even for extensions the attachment registry rejects; for an external path that opener retains the same attachment guards and errors. After an original 404, a spaced alternative opens only if it registers successfully; if it also returns 404, the original goes to the normal opener. Path labels, list markers and leading indentation inside parentheses are excluded from the alternative’s lookup key. Hover performs no lookup. Text alone cannot distinguish the two filenames, so this uses the filesystem result rather than a spacing guess. The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers and version tokens never continue a URL. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. Such a token is still refused when it is code with a call paren (`client.get()`, `expect(x).toBe(y)`; a wiki title like `Mercury_(planet)` keeps its paren) or a word ending a sentence (`ready.`, `Done.`, `self-hosted.`; a file name keeps its period, so `install.html.` still joins). Whatever its punctuation, a token never continues a URL when it ends with a colon (a label such as `Failure/Error:` or `prometheus.io/scrape:`), when it is a measurement or clock time (`45%`, `45%|####`, `12.3MB/45.0MB`, `12.3ms`, `10:00:00`; percent-encoding such as `2026%20Plan` still joins), or when it is a `name=value` field (`API_KEY=...`, `level=info`) under a URL ending in `/`, since a query needs a `?` first (after `?`, `&` or `_` it is a query parameter and joins). A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. When other columns follow a wrapped URL on its first row (a codex table's path or date column), the URL is stitched without them, and they still link on that row: `terminalLinkLine()` scans them after the stitched rows as a segment of their own, mapped back to their cells. A URL in such a column is the row's last URL, and only the last URL on a row is continued, so that row is never stitched. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the continuation row is blank to the left of the cell (a wrapper wraps every column of a table row together, so a label there such as `Entry` or `│ Log │` is the next record, unless a left column's path continued on that row too; a left column of wrapped prose, as in a `Description | Path` table, fills that row as well, so the path beside it keeps its per-row fragment links, the safe direction), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`, or `/home/u/proj/app/` over a labelled `main.py`) keeps its folder link and the next row keeps its own file link. With no label to tell them apart, a directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. Accepted paths displace other links on their cells. Full-width divider rows stop `terminalLogicalLine()` stitching for both links and touch selection. OSC 8 links confirm the real target URI before the shared opener runs. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 8a645941..551fb234 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2637,7 +2637,7 @@ function terminalPathLine(buffer, row, cols, maxRows) { const after = next.text.slice(0, closing).trimStart(); const spaced = spacedFilePathOnLine(`${before} ${after}`, { parenthesised: true }); if (!before.includes(')') && !/[/-]$/.test(before) && !after.includes('/') && spaced?.link) { - alternatives.push({ path: before + after, spacedPath: spaced.path }); + alternatives.push({ path: before.slice(spaced.index) + after, spacedPath: spaced.path }); } } } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index d52e6bba..993c6b67 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -2347,6 +2347,7 @@ Object.assign(CodemanApp.prototype, { if (original.status !== 404) return activateFilePath(target, false, sessionId); const path = self._resolveLinkedFilePath(alternative.spacedPath, sessionId); const registered = await self._registerExternalPreview(path, sessionId); + if (registered.status === 404) return activateFilePath(target, false, sessionId); if (!registered.attachmentId) { self.showToast(registered.error || 'Cannot open this file', 'error'); return; diff --git a/test/file-path-link-forms.test.ts b/test/file-path-link-forms.test.ts index b80615d5..ea77a3d6 100644 --- a/test/file-path-link-forms.test.ts +++ b/test/file-path-link-forms.test.ts @@ -343,24 +343,54 @@ describe('terminal link provider', () => { expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1'); }); - it.each([400, 403, 404, 500, 503])( - 'does not open an unregistered spaced alternative after HTTP %s', - async (status) => { - const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); - app._registerExternalPreview - .mockResolvedValueOnce({ status: 404, error: 'File not found' }) - .mockResolvedValueOnce({ status, error: 'Alternative unavailable' }); + it('uses the original opener when both hard-wrap filenames are missing', async () => { + for (const row of [1, 2]) { + const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], row, 120); + app._registerExternalPreview.mockResolvedValue({ status: 404, error: 'Attachment file not found' }); await links[0].activate({}, links[0].text); expect(app._registerExternalPreview.mock.calls).toEqual([ ['/mnt/data/KoboeReader.conf', 's1'], ['/mnt/data/Kobo eReader.conf', 's1'], ]); - expect(app.openFilePreview).not.toHaveBeenCalled(); - expect(app.openLogViewerWindow).not.toHaveBeenCalled(); - expect(app.showToast).toHaveBeenCalledWith('Alternative unavailable', 'error'); + expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1'); + expect(app.showToast).not.toHaveBeenCalled(); + } + }); + + it.each(['Path: ', '- ', '- Path: ', ' Path: '])( + 'resolves the filename without its parenthesised prefix %j', + async (prefix) => { + for (const row of [1, 2]) { + const { app, links } = linksOn([`Saved the notes (${prefix}/home/me/proj/out/My`, ' Notes.md).'], row, 120); + app._registerExternalPreview + .mockResolvedValueOnce({ status: 404, error: 'Attachment file not found' }) + .mockResolvedValueOnce({ attachmentId: 'att-spaced' }); + expect(links.map((link) => link.text)).toEqual(['/home/me/proj/out/MyNotes.md']); + await links[0].activate({}, links[0].text); + expect(app._registerExternalPreview.mock.calls).toEqual([ + ['/home/me/proj/out/MyNotes.md', 's1'], + ['/home/me/proj/out/My Notes.md', 's1'], + ]); + expect(app.openFilePreview).toHaveBeenCalledWith('/home/me/proj/out/My Notes.md', 's1'); + } } ); + it.each([400, 403, 500, 503])('does not open an unregistered spaced alternative after HTTP %s', async (status) => { + const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); + app._registerExternalPreview + .mockResolvedValueOnce({ status: 404, error: 'File not found' }) + .mockResolvedValueOnce({ status, error: 'Alternative unavailable' }); + await links[0].activate({}, links[0].text); + expect(app._registerExternalPreview.mock.calls).toEqual([ + ['/mnt/data/KoboeReader.conf', 's1'], + ['/mnt/data/Kobo eReader.conf', 's1'], + ]); + expect(app.openFilePreview).not.toHaveBeenCalled(); + expect(app.openLogViewerWindow).not.toHaveBeenCalled(); + expect(app.showToast).toHaveBeenCalledWith('Alternative unavailable', 'error'); + }); + it('keeps the session selected at click time while a path lookup is pending', async () => { const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); const pending = links[0].activate({}, links[0].text); diff --git a/test/wrapped-file-paths.browser.test.ts b/test/wrapped-file-paths.browser.test.ts index 14dddf4f..10b5d4c7 100644 --- a/test/wrapped-file-paths.browser.test.ts +++ b/test/wrapped-file-paths.browser.test.ts @@ -17,18 +17,20 @@ describe('wrapped configuration file links (Chromium)', () => { }); it.each([ - { wrap: 'soft', originalExists: false, extension: 'conf', registrationStatus: 200 }, - { wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 200 }, - { wrap: 'hard', originalExists: true, extension: 'conf', registrationStatus: 200 }, - { wrap: 'hard', originalExists: true, extension: 'svg', registrationStatus: 400 }, - { wrap: 'hard', originalExists: true, extension: 'markdown', registrationStatus: 400 }, - { wrap: 'hard', originalExists: false, extension: 'svg', registrationStatus: 400 }, + { wrap: 'soft', originalExists: false, extension: 'conf', registrationStatus: 200, prefix: '' }, + { wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 200, prefix: '' }, + { wrap: 'hard', originalExists: true, extension: 'conf', registrationStatus: 200, prefix: '' }, + { wrap: 'hard', originalExists: true, extension: 'svg', registrationStatus: 400, prefix: '' }, + { wrap: 'hard', originalExists: true, extension: 'markdown', registrationStatus: 400, prefix: '' }, + { wrap: 'hard', originalExists: false, extension: 'svg', registrationStatus: 400, prefix: '' }, + { wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 404, prefix: '' }, + { wrap: 'hard', originalExists: false, extension: 'markdown', registrationStatus: 200, prefix: 'Path: ' }, ])( - 'handles .$extension links on both rows: $wrap, original exists: $originalExists', - async ({ wrap, originalExists, extension, registrationStatus }) => { + 'handles .$extension links on both rows: $wrap, original exists: $originalExists, prefix: $prefix', + async ({ wrap, originalExists, extension, registrationStatus, prefix }) => { const path = basePath.replace('.conf', `.${extension}`); const original = path.replace('Kobo eReader', 'KoboeReader'); - const expected = wrap === 'hard' && originalExists ? original : path; + const expected = wrap === 'hard' && (originalExists || registrationStatus === 404) ? original : path; const lookupFails = registrationStatus === 400 && !originalExists; const lookups: string[] = []; const page = await browser.newPage({ viewport: { width: 1920, height: 600 } }); @@ -61,10 +63,10 @@ describe('wrapped configuration file links (Chromium)', () => { await page.addScriptTag({ content: readFileSync(resolve(publicDir, file), 'utf8') }); } await page.evaluate( - async ({ path, wrap, workingDir }) => { + async ({ path, wrap, workingDir, prefix }) => { const w = window as any; const boundary = path.lastIndexOf(' '); - const first = `Found it: prepared a corrected configuration (${path.slice(0, boundary)}`; + const first = `Found it: prepared a corrected configuration (${prefix}${path.slice(0, boundary)}`; const terminal = new w.Terminal({ cols: first.length + (wrap === 'soft' ? 1 : 20), rows: 6 }); terminal.open(document.getElementById('terminal')); w.opened = []; @@ -82,7 +84,7 @@ describe('wrapped configuration file links (Chromium)', () => { await new Promise((done) => terminal.write(text, done)); await new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done()))); }, - { path, wrap, workingDir: registrationStatus === 400 ? '/mnt/user/Scratch/Kobo' : '/workspace' } + { path, wrap, workingDir: registrationStatus === 400 ? '/mnt/user/Scratch/Kobo' : '/workspace', prefix } ); expect(lookups).toEqual([]);