Skip to content

feat(terminal): link URLs and paths that wrap across rows or sit in table cells - #686

Merged
Ark0N merged 8 commits into
Ark0N:masterfrom
aakhter:pr/terminal-wrapped-links
Oct 27, 2026
Merged

Ark0N merged 8 commits into
Ark0N:masterfrom
aakhter:pr/terminal-wrapped-links

Conversation

@aakhter

@aakhter aakhter commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Problem

Terminal links often opened only part of their target:

  • A CLI that word-wraps inside a markdown table cell or an indented paragraph (codex tables, for one) breaks a long URL at /, ? or - short of the right edge and indents the rest. Nothing is flagged isWrapped and no row reaches the last column, so each row linked a fragment.
  • The URL pattern ended every URL at ), so .../Team-Space(QA)Archive/... linked only as far as (QA.
  • A full-width table divider counted as a hard-wrapped line and was glued onto the row below it.
  • An absolute path wrapped inside a table cell matched nothing: the first piece has no extension and the second has no root. In a two-column table both continuations sit on the same next row, so a linear stitch cannot follow them.
  • OSC 8 hyperlinks used xterm's default handler instead of the terminal's own opener.

Approach

All detection is pure and lives in constants.js; the xterm link provider in terminal-ui.js consumes it.

  • findTerminalUrls() is the URL grammar: a ) that closes a ( inside the URL is kept, an unmatched one still ends it, so (https://...) and [text](https://...) stay clean.
  • terminalUrlContinuationExtent() joins rows a CLI wrapped a URL across, only when the fragment ends on a wrap-break character, the next row is indented no further right than the URL started, and its first token looks like URL material (not prose, not a new scheme://). Table columns after the first fragment are left out. terminalLinkLine() widens terminalPathLine() to that extent.
  • A URL that spans rows is emitted as one link per row, each opening the whole URL, so the indent and other columns between the pieces are not underlined.
  • terminalCellWrappedPaths() follows an absolute path down its own cell (space-separated or box-drawn tables). The join is accepted only when the joined text matches the file path pattern from its first character. These links displace any fragment link on the same cells.
  • terminalLogicalLine() treats a row made only of divider characters as a boundary.
  • OSC 8 links and text URL links both open through _openTerminalUrl(), which keeps the loopback web-tab routing. OSC 8 accepts web URLs only. Grid tiles and the split pane use the same handler.

Tests

  • New: test/terminal-url-continuation.test.ts (grammar, wrap joining, divider, OSC 8 handler, provider ranges), test/terminal-cell-wrapped-paths.test.ts (two-column and box-drawn cells, negative alignment cases, provider).
  • test/webview-loopback-links.test.ts now reads _openTerminalUrl() for the loopback check.
  • npm run typecheck, lint, format:check, check:frontend-syntax, check:public-assets and npm run test:ci pass.
  • Checked in Chromium on an isolated instance: a shell printed a URL wrapped across three table rows and a two-column box-drawn table with wrapped paths. The link provider returned the full URL on each of the three rows (each range limited to that row's URL cells) and both full paths on both table rows.

Paths that contain spaces are a separate PR.

…able cells

A CLI that word-wraps inside a markdown table cell or an indented paragraph
breaks a long URL or path short of the right edge and indents the rest, so
nothing is flagged isWrapped and no row is full width. Each row linked only
a fragment, and the fragment opened the wrong target.

- findTerminalUrls() (constants.js) is the URL grammar for the link provider:
  a ")" that closes a "(" inside the URL is kept, an unmatched one still ends
  it, so (https://...) and [text](https://...) stay clean.
- terminalUrlContinuationExtent() joins rows the CLI wrapped a URL across,
  only when the fragment ends on a wrap-break character, the next row is
  indented no further right than the URL started, and its first token looks
  like URL material. Table columns after the first fragment are left out.
- terminalLinkLine() widens terminalPathLine() to that extent; a URL that
  spans rows is emitted as one link per row, each opening the whole URL, so
  the indent and other columns between the pieces are not underlined.
- terminalCellWrappedPaths() follows an absolute path down its own table
  cell (space-separated or box-drawn), so two-column tables link both paths.
- A full-width table divider no longer counts as a hard-wrapped line.
- OSC 8 hyperlinks open through the same opener, web URLs only.
@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thank you, aakhter, for this one. It teaches the terminal link provider to follow URLs and absolute paths that a CLI word-wrapped inside table cells or indented paragraphs, keeps balanced parens inside URLs, treats full-width dividers as row boundaries, and routes OSC 8 links through our own opener.

The paren grammar, the divider boundary and the single _openTerminalUrl() opener are good. Two of the new detectors turn links that are correct on master into wrong links in common output, though, so this needs a round of changes before it can merge. I ran the shipped provider from both trees side by side on the same fake-buffer shape your tests use.

1. Cell-wrapped paths join the next listing entry onto a directory row (must fix)

src/web/public/constants.js:2779: the only test for a cell continuation is that the joined string matches the path pattern from index 0, and src/web/public/terminal-ui.js:2364 then removes every other link on those cells. Any directory row followed by a file row in the same column now links a concatenation, and the correct link on that row is gone:

Output master this PR
find: /tmp/x, /tmp/x/dir, /tmp/x/dir/a.png each row its own path /tmp/x/tmp/x/dir/tmp/x/dir/a.png on all three rows
Claude tool output: ⎿ /home/u/proj/src, /home/u/proj/src/a.ts each row its own path /home/u/proj/src/home/u/proj/src/a.ts
du: 4.0K /tmp/foo/bar/, 8.0K /tmp/foo/baz.png each row its own path /tmp/foo/bar//tmp/foo/baz.png
ls -R: /home/u/proj/docs:, guide.md notes.md guide.md linked /home/u/proj/docs:guide.md
Created /tmp/out, report.md written next to it two links /tmp/outreport.md

This breaks the rule in docs/architecture-invariants.md (File-path links) that a row starting a new path is never joined and that the broken row must be at least as long as the continuing one. Please gate each cell continuation through the existing pathContinuation(joined, token) instead of the pattern match alone (I checked it: it answers join/chain for all five fragment pairs in test/terminal-cell-wrapped-paths.test.ts and null for the find, du and ls -R pairs above), add the per-cell form of the row-length rule for the Created /tmp/out case, and add those listings as negative tests.

2. URL continuation glues list numbers, versions and status codes onto a URL ending in / (must fix)

src/web/public/constants.js:2612 and :2624:

Output master this PR
1. https://example.com/one/, 2. https://example.com/two/ each URL whole https://example.com/one/2
Docs: https://docs.example.com/guide/, 1. Install the package .../guide/ .../guide/1
Fetching https://registry.example.com/pkg/, v1.2.3 is the latest .../pkg/ .../pkg/v1.2.3
GET http://localhost:3000/api/, 200 OK in 12ms .../api/ .../api/200

indent <= urlStartCol admits an unindented row under any URL that does not start at column 0, the token test accepts any token with a digit or a dot, and the numbered-row guard at line 2614 only runs when other columns follow the URL. Please apply the list-marker refusal in both branches, tighten the token rule so a bare number or version word does not qualify on its own, and add these four shapes as negative tests.

3. OSC 8 links open with no confirmation

src/web/public/terminal-ui.js:2413: xterm's default handler shows the real target in a confirm dialog before opening, because an OSC 8 link's visible text can differ from where it goes. The new handler opens immediately (and a loopback target saves a web tab). Please keep a confirmation that shows the target URI, or move the OSC 8 change into its own PR so we can decide it separately.

4. Smaller things

  • docs/architecture-invariants.md:410 and CLAUDE.md:225 still describe terminalPathLine() as the provider's stitch. Please add a short paragraph for terminalLinkLine, the URL continuation, the cell-wrapped paths (and that they displace other links), and the divider rule.
  • src/web/public/constants.js:2341: the divider rule is in the shared terminalLogicalLine(), so touch selection changes too. Please add a case to test/terminal-logical-line.test.ts.
  • test/link-provider-regex.test.ts:103: the local strip still trims ), which the shipped trim no longer does. Pointing those cases at findTerminalUrls() would test what ships.

Once 1 and 2 are fixed with their negative tests and we have settled 3, I will review again and merge. If you would rather land the clean parts first, the paren grammar, the divider boundary and the _openTerminalUrl() routing can go in their own PR right away.

@aakhter

aakhter commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed the requested changes in 40a9528.

  • Cell-wrapped paths now require pathContinuation() and a per-cell length check. Added all five negative listing shapes, including provider-level checks that their original links survive.
  • URL continuation rejects list markers in both branches and bare number/version tokens; added the four reported negative cases.
  • OSC 8 keeps target confirmation before the shared opener, including loopback links. Tests cover accept, cancel and unsafe schemes; a Chromium smoke check exercised the actual confirmation dialog.
  • Updated both architecture docs, added the shared logical-line divider regression, and changed URL punctuation cases to call shipped findTerminalUrls().

Validation: 88 targeted tests pass; typecheck, lint, frontend syntax and format checks pass. Both PR branches combine without conflicts and pass all 121 combined link tests. CI is green on this head, including 12,325 unit/integration tests and 261 xterm package tests. The shared working checkout was not modified.

@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thank you, aakhter, for the quick and thorough second round. This PR teaches the terminal link provider to follow URLs and absolute paths that a CLI word-wrapped inside table cells or indented paragraphs, keeps balanced parens inside URLs, treats full-width dividers as row boundaries, and routes OSC 8 links through our own opener.

Everything from the first round landed. I re-ran the side-by-side comparison against master: the find, Claude tool output, du, ls -R and Created listings all keep their own links again, list markers, versions and status codes no longer continue a URL, the OSC 8 handler confirms the real target (and xterm still drops non-http schemes before it is called), and the docs paragraph, the divider test and the findTerminalUrls() test rewrite are in. Thank you for the negative tests and the provider-level checks.

One thing still needs a change before merge, and two small ones are optional.

1. Prose words with -, _ or . still continue a URL (must fix)

src/web/public/constants.js:2625: the token rule accepts any first word that contains -, _ or ., and those are ordinary prose characters. In an indented paragraph the URL starts well right of the paragraph indent, so indent <= urlStartCol at :2613 admits every following line and the token rule is the only guard. Same fake-buffer shape your tests use, master vs this branch:

Output (two rows) master this PR
⏺ The guide lives at https://example.com/guide/ / self-hosted runners need a token first. https://example.com/guide/ https://example.com/guide/self-hosted
Docs: https://example.com/guide/ / Self-hosting is covered in chapter 3. https://example.com/guide/ https://example.com/guide/Self-hosting
Open http://localhost:3000/ / e.g. to check the dashboard http://localhost:3000/ http://localhost:3000/e.g
Endpoint https://api.example.com/v1/ / user_id is required https://api.example.com/v1/ https://api.example.com/v1/user_id
Repo: https://github.com/org/repo/ / well-known issues are tracked https://github.com/org/repo/ https://github.com/org/repo/well-known
Callback https://example.com/cb?code= / Re-run the login https://example.com/cb?code= https://example.com/cb?code=Re-run
Branch https://github.com/o/r/tree/feat- / x.y happened https://github.com/o/r/tree/feat- https://github.com/o/r/tree/feat-x.y

The first row is the shape of a Claude Code reply paragraph, so this shows up in everyday output.

One rule that clears all seven: a continuation token with none of % & = / ? must end its cell (the row ends, or a gap of two or more spaces follows it). In chainFrom, right after the version check:

const rest = nextText.slice(indent + token.length);
if (!/[%&=/?]/.test(token) && rest.trim() !== '' && !/^ {2,}/.test(rest)) break;

I ran it on a copy of this branch: all 56 tests in your three test files still pass, and all seven shapes go back to master's link. The trade-off is that a URL broken at / whose next piece is a plain slug (my-post-title) followed by more prose on the same row falls back to the fragment link master shows today, a missing link rather than a wrong one. A different rule is fine too; please add these seven shapes as negative tests either way.

2. Links in the first row's other columns disappear (small)

src/web/public/constants.js:2683: when a URL continues and other table columns follow its first fragment, terminalLinkLine() cuts the first row at firstRowUrlEnd, so those columns are no longer scanned. With

  Docs     https://example.com/a/very/long/    /tmp/report.txt
           path/x.html

master links /tmp/report.txt on the first row and this branch does not (on touch too, since taps use the same provider). Keeping the cut-off remainder as its own segment after the joined rows (separated by a space, mapped back to its original cells) keeps it; a test on the TABLE_WITH_TRAILING_COLUMNS shape with a path in a trailing column would pin it.

3. Hover underline on soft-wrapped URLs (nit)

src/web/public/terminal-ui.js:2204: a URL that xterm soft-wrapped (isWrapped) is now split into one link per row as well, so hovering underlines only the row under the pointer, where master underlined the whole URL. Splitting only at hard continuations, where an indent or other columns sit between the pieces, keeps the old look for soft wraps.

Once 1 is in with its negative tests, I will merge. 2 and 3 are welcome in this PR; if they are not in by then, I will pick them up as a follow-up.

@aakhter

aakhter commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed required item 1 in 27c93b2 using the suggested conservative cell-end guard.

A continuation with none of % & = / ? must finish its cell (row end or a gap of at least two spaces). All seven reported prose shapes are covered both at detector level and through the shipped provider, including activation of the unchanged original URL and absence of a URL link on the prose row. Positive cases cover weak-punctuation slugs at cell boundaries and stronger URL punctuation; existing table/paragraph continuation tests still pass. The JSDoc and architecture documentation describe the rule and its trade-off: a real plain slug followed by prose falls back to the original fragment link.

Validation: 111 targeted tests passed; typecheck, lint, frontend syntax and formatting checks passed; Chromium exercised the shipped detector. The branch combines cleanly with #687 at 74a107c, with all 188 combined link/viewer tests passing. CI for this exact head is green, including the full unit/integration suite and xterm package tests.

Local full-suite result: 12,347 passed, 12 skipped, one failure in the untouched quick-start skill-injection test (missing injected SKILL.md). The same failure reproduces on the pre-fix head 40a9528 as well as this head; no unrelated source was changed to work around it.

Optional items 2 and 3 (links in trailing table columns and whole-URL soft-wrap hover underlining) are unchanged for your offered follow-up. No shared checkout or unrelated PR was modified.

@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thank you, aakhter, for the third round. This PR makes terminal links follow URLs and absolute paths that a CLI word-wrapped inside table cells or indented paragraphs, keeps balanced parens inside URLs, treats full-width dividers as row boundaries, and routes OSC 8 links through our own opener with a confirmation.

The cell-end rule landed exactly as discussed: all seven prose shapes from the last round keep master's link again, at detector and provider level, and the positive cases on both sides of the rule make the trade-off easy to see. Thank you for that.

1. Shell prompts, HTTP status lines and home paths still continue a URL (must fix)

src/web/public/constants.js:2630: the cell-end rule exempts any token that contains /, = or ?, and indent > urlStartCol at :2614 admits every unindented row. So a URL that ends a row on / takes the next row's first word whenever that word carries URL punctuation, and shell prompts and curl output do. Same fake-buffer shape your tests use, master vs this branch:

Output (two rows) master this PR
https://registry.npmjs.org/ / arkon@box:~/proj$ https://registry.npmjs.org/, and ~/proj on the prompt https://registry.npmjs.org/arkon@box:~/proj$ on both rows, ~/proj link gone
https://pypi.org/simple/ / dev@laptop:/srv/app$ URL, and /srv/app https://pypi.org/simple/dev@laptop:/srv/app$, /srv/app link gone
$ curl -sI https://example.com/ / HTTP/2 200 https://example.com/ https://example.com/HTTP/2
$ curl -i http://localhost:3000/api/ / HTTP/1.1 200 OK http://localhost:3000/api/ http://localhost:3000/api/HTTP/1.1
Docs: https://example.com/guide/ / ~/notes.md has more URL, and ~/notes.md https://example.com/guide/~/notes.md, ~/notes.md link gone
fetch https://api.example.com/items/ / status=200 took=12ms URL https://api.example.com/items/status=200
Status page: https://status.example.com/ / 24/7 monitoring is on. URL https://status.example.com/24/7
⏺ The docs are at https://example.com/guide/ / and/or the API reference covers it. URL https://example.com/guide/and/or
Did you mean https://example.com/search?q= / really? no. URL https://example.com/search?q=really

The first two rows are npm config get registry and pip config get global.index-url followed by the stock Debian/Ubuntu prompt, and the prompt's own folder link disappears as well (on taps too, since they use the same provider).

A guard right after the scheme check at :2625 clears the first five rows and keeps all 79 tests in your three link test files green (I ran them against a copy with this applied):

// A shell prompt (`user@host:~/dir$`), a home path and an HTTP status line
// are never the rest of a URL, however much URL punctuation they carry.
if (/^[\w.-]+@[\w.-]+:|^~\/|^HTTP\/\d/i.test(token)) break;

Optional, and your call on the fixtures: refusing an unindented hard continuation (add !indent to the condition at :2614, which the trailing-columns branch already requires at :2619) also clears status=200, 24/7 and really?. Every real continuation in your fixtures is indented (codex cells, Claude's two-space paragraph indent); the cost is that the column-0 positives at test/terminal-url-continuation.test.ts:199 and :208 fall back to the fragment link and would need indented fixtures.

Please add these rows as negative tests at detector and provider level, as you did for the last round.

2. Smaller things (optional)

  • src/web/public/constants.js:2630: ⏺ Docs live at https://example.com/guide/ / self-hosted. still links https://example.com/guide/self-hosted, because the end of a row counts as the end of the cell. That is the trade-off of the rule I suggested, so one sentence next to the existing trade-off note in docs/architecture-invariants.md is enough.
  • src/web/public/constants.js:2341: the divider check only looks at the previous row, so a URL that reaches the right edge still absorbs a full-width ──── row below it. Master does the same; returning false when row r itself is a divider row closes it.

Once 1 is in with its negative tests, I will merge. Round-2 items 2 and 3 (trailing-column links on the first row, soft-wrap hover underline) stay on my follow-up list, together with anything from 2 you leave out.

@Ark0N

Ark0N commented Oct 24, 2026

Copy link
Copy Markdown
Owner

@aakhter, one correction to my last review, so you don't chase a test that cannot pass. I asked for all nine rows as negative tests, but the indented and/or row (⏺ The docs are at https://example.com/guide/ then and/or the API reference covers it.) still joins with the required guard, and with the optional !indent refusal too: it is indented and and/or carries a /, so to the code it looks the same as the path/slug more text positive. Leave that row out of the required negatives. If you want it on record, add it as a known trade-off with a comment, like self-hosted.. The other eight rows and the guard stand as written.

A URL ending a row on `/` took the next row's first word whenever it
carried URL punctuation, so a shell prompt, a home path or an HTTP
status line could be appended to it. Refuse those tokens, and refuse a
column-0 row as a hard continuation (real continuations are indented),
which also keeps `status=200`, `24/7` and `really?` out.

A divider row no longer continues the row above it, so a URL reaching
the right edge does not absorb a full-width rule below it.

Pin the new negatives at detector and provider level, re-indent the
positive continuation fixtures, and pin the `self-hosted.` and `and/or`
trade-offs.
@aakhter

aakhter commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. Pushed a follow-up commit (fix(terminal): keep shell prompts and status lines out of wrapped URLs) that covers all of it:

  1. Prompt / home path / status line guard: added right after the scheme check, as you suggested. A token matching user@host:, ~/ or HTTP/<digit> never continues a URL.

  2. !indent: applied. A column-0 row is no longer accepted as a hard continuation, which clears status=200, 24/7 and really?. The two column-0 positive fixtures (the weak-punctuation "ends its cell" set and the stronger-punctuation-followed-by-prose set) are now indented by two spaces, so the positive behaviour stays pinned. No other positive needed changing; the table and list fixtures were already indented.

  3. All eight rows from your table are negatives at both levels:

    • detector: terminalUrlContinuationExtent returns null for both rows
    • provider: row 1 links only the original URL, it opens that URL, row 2 has no http link
    • for the prompt and home-path rows, the provider test also asserts the row 2 path link is still there (~/proj, /srv/app, ~/notes.md)
      Because the column-0 prompt rows are now also refused by !indent, there is a separate detector test with the prompt, home-path and HTTP status tokens indented, so the guard itself is pinned on its own.
  4. The indented and/or row is pinned as a known trade-off next to self-hosted. (both still link .../guide/and/or and .../guide/self-hosted), at detector and provider level, with a comment explaining that a row end counts as a cell end.

  5. Optional items, both done:

    • docs/architecture-invariants.md now describes the two trade-offs, the indent requirement and the prompt/status-line rule next to the existing cell-end note.
    • terminalLogicalLine now returns false from the continuation check when row r itself is a divider row, so a URL reaching the right edge no longer absorbs a full-width ──── (or ----) row below it. Tested at the logical-line level and through the provider.

Mutation check (terminal-url-continuation.test.ts, 76 tests): removing only the guard fails 5 tests, removing only !indent fails 6, removing both fails 21, removing the divider check fails 2. All 76 pass with the change. The wider set of tests that load constants.js (106 files, 2284 tests) passes, and lint, format check, frontend syntax, public assets and typecheck are clean.

@Ark0N

Ark0N commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thank you, aakhter, for the fourth round. This PR makes terminal links follow URLs and absolute paths that a CLI word-wrapped inside table cells or indented paragraphs, keeps balanced parens inside URLs, treats full-width dividers as row boundaries, and routes OSC 8 links through our own opener with a confirmation.

Everything from the last round landed: the prompt, home path and status line guard, the !indent refusal, the eight negative rows at detector and provider level (with the ~/proj, /srv/app and ~/notes.md links checked on the second row), the two trade-offs pinned with a comment, the docs sentence and the divider fix in terminalLogicalLine(). I re-ran the side-by-side comparison against master and all of those shapes keep master's links. Thank you for the mutation check, too.

1. A URL in the last column of a space-aligned table takes the next row's first cell (must fix)

src/web/public/constants.js:2620: the alignment test at :2629 (no column gap between the next row's indent and the URL on the first row) only runs when other columns follow the URL. When the URL is the last thing on its row, nothing checks that the next row starts in the URL's own column, so the first cell of the next record continues the URL whenever it is a word with -, _ or .: the column gap after that word counts as the end of its cell at :2640. Same fake-buffer shape your tests use, each row indented two spaces, master vs this branch:

Output (rows) master this PR
auth-api https://auth.example.com/ / billing-api https://billing.example.com/ each URL whole https://auth.example.com/billing-api
docs.example.com https://docs.example.com/ / api.example.com https://api.example.com/ each URL whole https://docs.example.com/api.example.com
Homepage: https://example.com/ / Bug-reports: https://github.com/org/repo/issues/ each URL whole https://example.com/Bug-reports
user_service https://users.example.com/v1/ / order_service https://orders.example.com/v1/ each URL whole https://users.example.com/v1/order_service
README.md https://example.com/docs/ / CHANGELOG.md https://example.com/changes/ each URL whole, CHANGELOG.md opens its preview https://example.com/docs/CHANGELOG.md, CHANGELOG.md link gone
my-service- https://svc.example.com/api/ / long-name https://svc2.example.com/ each URL whole https://svc.example.com/api/long-name

The joined link also covers the next row's first cell, so clicking a service name opens the row above's URL. The last row is a codex table whose first column wrapped.

The rule that clears them: the next row must start inside the URL's own column. A gap of two or more spaces between where the next row starts and where the URL starts on the first row means it opens an earlier column, the next record of a table. That is the test the trailing-columns branch already makes at :2629, applied to every row. Right before the scheme check at :2632:

// The next row must start inside the URL's own cell: a column gap between
// its indent and the URL on the first row means it opens an earlier
// column (the next record of a table), not the rest of the URL.
if (/ {2,}/.test(first.text.slice(indent, last.startIndex).trimStart())) break;

The trimStart() is there for your TABLE fixture, whose continuation rows start at column 26, inside the gap before the Quarterly cell. I ran it on a copy of this branch: all 208 tests in the seven link test files pass, and the six shapes above go back to master's links. A different rule is fine too. Please add these shapes as negative tests at detector and provider level, and one sentence for the rule in the paragraph at docs/architecture-invariants.md:412 and CLAUDE.md:227.

2. An indented email address continues a URL (optional)

src/web/public/constants.js:2635: https://example.com/ followed by user@example.com links https://example.com/user@example.com on both rows, where master links https://example.com/. The prompt guard needs a : after the host. One more line after it refuses a token that is a whole email address, and the same 208 tests pass with it:

if (/^[\w.+-]+@[\w-]+(?:\.[\w-]+)+$/.test(token)) break;

Once 1 is in with its negative tests, I will review again and merge.

A URL in the last column of a space-aligned table ended on a slash, so
the first cell of the next record (billing-api, CHANGELOG.md, ...)
passed every continuation test and was joined onto it. The alignment
check that refused this only ran when other columns followed the URL.

A continuation row must now start inside the URL's own cell: a gap of
two or more spaces between its indent and where the URL starts on the
first row means it opens an earlier column. An indented token that is
a whole email address is also refused.

Adds the six table shapes and the email case as negative tests at
detector and provider level, and documents the rule.
A URL ending in `/` still took the next indented row when that row was
code, a label, a log or config field, a measurement or the end of a
sentence. A side-by-side sweep against master turned up shapes such as
`ready.` under a codex `Listening on` line, `expect(x).toBe(y)` under a
jest title, `Failure/Error:` under an rspec title, `API_KEY=...` under
`BASE_URL=...`, `level=info` log fields, `45%|####` tqdm bars,
`12.3MB/45.0MB`, `12.3ms` and `10:00:00.123`.

A continuation is now refused when it ends with a colon, when it is a
measurement or clock time, or when it is a `name=value` token under a
URL ending in `/` (a query needs `?` first). A token with only `-`, `_`
or `.` is also refused when it carries a call paren or ends a sentence.
Wiki titles such as `Mercury_(planet)`, a file name before its period,
query parameters after `?`, `&` or `_`, and percent-encoding that starts
with digits still join.

`self-hosted.` moves from the pinned trade-offs to the negatives; the
trade-off is now a lone word without sentence punctuation.
@aakhter

aakhter commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful round again, and for the exact table of shapes.

Both items are in 2a90fb8, and a few more shapes are in 1bdc341.

  1. Last-column URL taking the next record's first cell: I applied your rule as written, right before the scheme check in terminalUrlContinuationExtent. A continuation row has to 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 first row ends the chain. It runs for every row now, not only when trailing columns follow the URL. The trimStart() keeps the TABLE fixture joining as before. All six shapes from your table are now negative tests at detector and provider level, each row indented two spaces: the detector returns null for both rows, and the provider links each URL whole on its own row and opens it. The README.md / CHANGELOG.md row also checks that CHANGELOG.md still gets its own file link on row 2. The rule is one sentence in the paragraph in docs/architecture-invariants.md and in CLAUDE.md, and the function's doc comment lists it.

  2. Indented email address: added your check after the prompt guard, with a negative test at both levels ( https://example.com/ then user@example.com links only https://example.com/). The docs sentences and the doc comment mention it too.

  3. Extra shapes I found and fixed before pushing (1bdc341). I ran a side-by-side sweep of 321 realistic two-to-four-row samples (npm, pip, cargo, go, git, gh, docker, kubectl, curl, dev servers, test runners, compiler errors, stack traces, Claude and Codex tables, lists, code blocks, logs, JSON/YAML, diffs, listings, progress bars) through master and this branch, in the same fake-buffer shape. Under a URL ending in /, these indented rows still continued it: ready. and Done., code such as expect(received).toBe(expected) and client.get(), labels such as Failure/Error: and prometheus.io/scrape:, fields such as API_KEY=..., level=info and request_id=abc-123, and measurements such as 45%|####, 12.3MB/45.0MB, 12.3ms and 10:00:00.123. A token ending in :, a measurement or clock time, and a name=value token under a URL ending in / (a query needs ? first) no longer continue a URL, and a token with only -, _ or . is also refused when it has a call paren or ends a sentence. Wiki titles (Mercury_(planet)), install.html., query parameters after ?, & or _, and 2026%20Plan.xlsx still join. 18 negative shapes and 6 positives are tests at detector and provider level. self-hosted. moved from the trade-offs to the negatives (the pinned trade-off is now self-hosted with no period), and the x=1&y=2 positive now sits under a URL ending in ?. After this, the only differences from master in the sweep are intended joins, plus the pinned trade-off of a lone word at a row end (get_user_by_id, npm-run-all), which is the same shape as my-post-title, and the path/slug more text positive (github.com/x/y v1.2.3 under https://proxy.golang.org/).

Mutation check: with the column guard removed, the 12 new table tests (6 detector, 6 provider) fail and the rest pass. With the email guard removed, the 2 email tests fail. Both are restored in the commit. For the new guards: colon 4 tests fail, name=value 10, measurement 8, call paren 6, sentence end 8; dropping the wiki-paren, file-name, hex and /-only exceptions fails 2, 1, 1 and 13.

Gate: the link test files plus every non-browser test that reads constants.js pass under the CI config (107 files, 2352 tests). lint, format:check, check:frontend-syntax, check:public-assets and typecheck are clean.

@Ark0N

Ark0N commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thank you, aakhter, for the fifth round. This PR makes terminal links follow URLs and absolute paths that a CLI word-wrapped inside table cells or indented paragraphs, keeps balanced parens inside URLs, treats full-width dividers as row boundaries, and routes OSC 8 links through our own opener with a confirmation.

Everything from the last round landed: the last-column rule with all six table shapes at detector and provider level (CHANGELOG.md keeps its own link), the email guard, and the docs sentences. The extra shapes from your sweep (ready., client.get(), Failure/Error:, API_KEY=..., 45%|####, 12.3ms, 10:00:00.123 and the rest) keep master's links too, and the positives on the other side of each rule still join. Thank you for the sweep and the mutation check.

1. A directory value in a table cell takes the next row's file name (must fix)

src/web/public/constants.js:2828: in a key/value or two-column table, an absolute directory that is the whole content of its cell is joined onto the file name in the same column on the next row, which is the next record, and src/web/public/terminal-ui.js:2364 then removes the correct links on both cells. Same fake-buffer shape your tests use, master vs this branch:

Output (rows) master this PR
Repo /home/u/proj / Config config.yaml /home/u/proj, and config.yaml /home/u/projconfig.yaml on both rows
Workspace /home/u/proj/app / Entry main.py /home/u/proj/app, and main.py /home/u/proj/appmain.py on both rows
│ Workspace │ /home/u/proj/app │ / │ Entry │ main.py │ /home/u/proj/app, and main.py /home/u/proj/appmain.py on both rows
Output dir /tmp/build/out / Log build.log /tmp/build/out, and build.log /tmp/build/outbuild.log on both rows
Path Purpose / /home/u/proj/src sources / index.ts entry /home/u/proj/src, and index.ts /home/u/proj/srcindex.ts on both rows

Tapping main.py opens /home/u/proj/appmain.py, which does not exist, and the folder link is gone as well. The third row is Claude Code's box-drawn table, so this shows up in ordinary agent summaries.

On master, terminalPathLine() leaves these alone because the directory is not at the end of its row and pathContinuation() sees the next row's first token (Entry). The cell detector treats the end of the cell as the end of the row and passes the cell's own token (main.py), and pathContinuation() answers join for any extensionless tail with a / followed by a name with an extension. The per-cell length rule does not separate them, since the file name in each of these shapes is shorter than the directory above it.

One rule that clears all five: a wrapper breaks a path after / or -, and every fragment in your positive fixtures ends on one (.../review/, .../BLDMESH-, .../BLD-, .../very/, .../nested/). A fragment that ends inside a name is a whole value. Right before line 2828:

// A wrapper breaks a path after `/` or `-`; a fragment that ends inside a
// name (`/home/u/proj` over `config.yaml`) is a whole value, not a piece.
if (!/[/-]$/.test(joined)) break;

I ran it on a copy of this branch: all 276 tests in the eight link test files pass (your two new files, file-path-link-forms, terminal-logical-line, link-provider-regex, file-link-click, terminal-link-provider-target and terminal-touch-tap), the five shapes go back to master's links, and the two-column BLDMESH table still links both whole paths. Add _ to the class if you have seen codex break a path there; a different rule is fine too. What still joins with it: a directory with a trailing slash over a relative path in the same column (/home/u/proj/src/ over lib/util.ts), which looks the same as a real wrap at /. A known trade-off note for that one is enough.

Please add these shapes as negative tests at detector and provider level (checking that config.yaml, main.py, build.log and index.ts keep their own links), and update the terminalCellWrappedPaths() sentence in the paragraph at docs/architecture-invariants.md:412 and CLAUDE.md:227, which still says it is gated by pathContinuation() and the per-cell row-length rule.

Once 1 is in with its negative tests, I will review again and merge.

…name

A table cell holding a whole directory (`/home/u/proj` over `config.yaml`)
was joined onto the file name in the same column on the next row, which is
the next record, and the joined link then replaced the correct links on both
cells. A wrapper breaks a path after `/` or `-` (codex wraps cells on
Unicode line-break opportunities plus a hyphen splitter, which never break at
`_`), so a cell fragment must end on one of those before the next row can
continue it.

A directory written with its trailing slash over a relative path in the same
column still joins, since it reads exactly like a wrap at `/`; pinned as a
known trade-off.
A wrapper wraps every column of one table row together, so the row that
carries a cell's continuation is blank to the left of that cell unless a
path in a left column continued on it too. A label there (`Entry`,
`│ Helper │`) opens the next record, so a directory ending in `/` or `-`
(`Workspace  /home/u/proj/app/` over `Entry  main.py`) no longer takes
the next record's file name.

What still joins is a trailing-slash directory over a relative path in the
same column with nothing to its left, which reads exactly like a wrap at
`/`; that trade-off stays pinned.
@aakhter

aakhter commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for round 6. Two commits on top of 1bdc341.

1. A directory cell taking the next row's file name (8ad60fa)

Your rule is in, right before the pathContinuation() check in terminalCellWrappedPaths(): a fragment has to end on / or - before the next row can carry it on. All five shapes from your table are negative tests at detector level (no hit on any row) and provider level. The provider tests check that config.yaml, main.py, build.log and index.ts keep their own links on their own row, that the directory keeps its folder link (/home/u/proj, /home/u/proj/app, /tmp/build/out, /home/u/proj/src), and that nothing links the joined string. With the rule removed, those 10 tests fail.

_: not added. codex wraps table cells with RtOptions::new(width) (wrap_cell in codex-rs/tui/src/markdown_render.rs and table_key_value.rs), which uses textwrap 0.16 with its default features: WordSeparator::UnicodeBreakProperties plus WordSplitter::HyphenSplitter. Under UAX #14, / is class SY, so a break is allowed after it before a letter. - is split by the hyphen splitter. _ is class AL, which gives no break opportunity, so codex never breaks a path at _. A test pins that a fragment ending in _ does not join. (break_words can still split a single path segment wider than the whole cell at any character. Your rule refuses those, and so does master.)

Trade-off. The comment at the rule and a test pin the shape that still joins. While sweeping I narrowed that shape (next section), so the pinned version is the unlabelled one: /home/u/proj/src/ over lib/util.ts in the same column with nothing to its left.

Docs. I rewrote the terminalCellWrappedPaths() sentence in docs/architecture-invariants.md and CLAUDE.md. It now covers the break-character rule, the blank-left rule below, pathContinuation(), the row-length rule and the trade-off.

Path-in-table sweep (ae3d314)

I ran the side-by-side harness, master vs this branch, on 188 path-focused samples, at provider and detector level. They cover:

  • key/value summaries (Repo, Workspace, Config, Output, Log, Branch, Commit)
  • box-drawn tables (│, ┃, ║, |, with ┌─┬─┐ frames) and raw markdown pipe tables
  • space-aligned two- and three-column tables
  • git status, diff --stat and worktree output; ls, du, df, tree, find, rg output
  • test-runner summaries (vitest, jest coverage, pytest, cargo, go, playwright)
  • narrow codex tables that really wrap a path cell
  • relative, ~/, WSL, Windows and UNC paths
  • - or _ at a row end, trailing-slash directories, files listed under a directory row, and paths mixed with URLs

With the first commit alone, 17 samples differed from master. 13 were intended joins. One was a regression of the same kind as yours: Prefix /tmp/run- over Suffix log.txt became /tmp/run-log.txt. Three were your trade-off in its common form, a labelled record: Workspace /home/u/proj/app/ over Entry main.py, and the same thing box-drawn as │ Source │ /home/u/proj/src/ │ over │ Helper │ lib/util.ts │.

One narrow rule fixes all four. A wrapper wraps every column of a table row together, so the row that carries a cell's continuation is blank to the left of that cell. The one exception is when a path in a left column also continued on that row, as in the two-column BLDMESH table. So a label there means the next record.

There are three negative tests for it at each level. Without the rule, 6 tests fail. If I keep the rule but drop the left-column exception, the 5 two-column BLDMESH and box tests fail. If I remove both rules, 17 tests fail.

After the fix, 13 samples differ from master. 12 are intended joins (box, pipe, codex narrow and two-column wraps at / and -). The 13th is the unlabelled trade-off. The URL corpus from the last round (321 samples) gives the same result as before.

One thing I left alone because master does it too: if a left-column path is wrapped and a URL ending in / sits at the end of the same row, master's terminalPathLine() appends the next row's file name to the URL. The branch keeps that URL on the first row but gives the continuation row's cell to the path.

Checks: the eight link test files pass (294 tests). Every non-browser test that reads constants.js or terminal-ui.js passes (130 files, 2748 tests). typecheck, lint, format:check, check:frontend-syntax, check:public-assets and check:browser-excludes are all clean.

@Ark0N

Ark0N commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thank you, aakhter, for the sixth round. This PR makes terminal links follow URLs and absolute paths that a CLI word-wrapped inside table cells or indented paragraphs, keeps balanced parens inside URLs, treats full-width dividers as row boundaries, and routes OSC 8 links through our own opener with a confirmation.

Round 6 landed. I re-ran the side-by-side comparison against master. The five whole-value shapes, the three labelled records (Workspace /home/u/proj/app/ over Entry main.py, the box-drawn Source/Helper row, Prefix /tmp/run- over Suffix log.txt) and the five listings from round 1 keep master's links on every row. The two-column BLDMESH and box-drawn tables still link both whole paths. Thank you for tracing codex's textwrap behaviour for _ and for the path-in-table sweep.

Two small things are left. I will apply both at merge time, so no new round is needed:

  1. src/web/public/constants.js:2614: firstRowUrlEnd is an offset into the first logical line, but terminalLinkLine() (:2727) uses it to cut only the first physical row. When the URL's row soft-wraps and a trailing column sits on the wrapped part, that column stays in the scanned text, and the continuation row gets glued onto it. At 41 columns, with the second row a soft wrap of the first:
  Doc  https://example.com/aaaaaaaaaaaaaa
bbb/    /tmp/report.txt
       ccc/x.html

Master links /tmp/report.txt and ccc/x.html. This branch links /tmp/report.txtccc/x.html on all three rows. Adding first.startRow === first.endRow && to the trailingColumns condition gives this shape master's links back, and all 278 tests in the six link test files still pass with it. I will add that guard with a negative test.

  1. src/web/public/constants.js:2840: the comment says the continuation row is blank "unless a left column wrapped on that row too", but the exception covers only a left-column path. A left column of wrapped prose (a Description | Path table) also fills that row, so a path there keeps master's fragment links. That is the safe direction. I will reword the comment to match the JSDoc and add one clause about it to the paragraph in docs/architecture-invariants.md and CLAUDE.md.

The static checks and the 349 tests in the twelve link test files pass locally, and CI is green on this head. I will merge with those two changes on top.

@Ark0N
Ark0N merged commit ae3d314 into Ark0N:master Oct 27, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
- constants.js:2618 terminalUrlContinuationExtent: trailingColumns now needs a
  one-row first line (first.startRow === first.endRow), so a URL row that
  soft-wraps with a column on its wrapped part no longer glues the next row
  onto that column (`/tmp/report.txtccc/x.html` at 41 columns); master's links
  come back. Negative tests for the extent and the link provider.
- constants.js:2847 terminalCellWrappedPaths: comment reworded to match the
  JSDoc (the exception is a left-column PATH), plus the wrapped-prose case.
- docs/architecture-invariants.md, CLAUDE.md: one clause that a left column of
  wrapped prose (a Description | Path table) also fills the continuation row,
  so the path beside it keeps its per-row fragment links, the safe direction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
…p across rows or sit in table cells

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Ark0N

Ark0N commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Merged in 1.46.0 (https://github.com/Ark0N/Codeman/releases/tag/codeman%401.46.0). Thank you for six rounds and for the side-by-side corpus runs; they are why this could land. I applied the two items from the last review: the first.startRow === first.endRow guard on trailing columns with the 41-column negative test, and the reworded comment plus one clause in the architecture notes and CLAUDE.md. The release review then found one more shape: in a table where a URL wraps inside its cell and a path sits in the next column, the path lost its link on the URL's first row, because that row was cut at the URL's end before the detectors ran. That is fixed on top (cc08016) with a regression test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants