mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(terminal): merge-time fixes for the copy gutter strip (#469)
- stock.ts: claude is no longer the only entry declaring transcriptGutter; codex declares it too. - architecture-invariants: the strip applies when the session's CLI declares a margin (not detection), and a note that it keys on the session's launch mode, not on what is running in the pane (a claude pane dropped to a shell still loses up to two columns; copyStripMargin is the escape hatch). - render-index-html test: the gutter map is injected for a solo /session/:id render as well. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -320,9 +320,9 @@ Invariants:
|
|||||||
|
|
||||||
Copy goes through `_copyText()` (Clipboard API, then hidden-textarea + `execCommand`), not raw `navigator.clipboard`, because `install.sh`'s LAN option serves plain HTTP where `navigator.clipboard` is undefined; the fallback steals focus, so the terminal is refocused afterwards. Related: xterm registers its own `copy` listener on the terminal element gated on `hasSelection()`, which is why right-click → Copy has always worked. Selection itself is unavailable on touch devices by design (`user-select: none` on the terminal subtree), and in `shell`/`opencode`/`antigravity` tabs the TUI owns the mouse, so selecting there needs Shift+drag. Tests: `test/terminal-copy-selection.test.ts` (gate + wiring invariants), `test/terminal-copy-shortcut.test.ts` (browser, real key presses).
|
Copy goes through `_copyText()` (Clipboard API, then hidden-textarea + `execCommand`), not raw `navigator.clipboard`, because `install.sh`'s LAN option serves plain HTTP where `navigator.clipboard` is undefined; the fallback steals focus, so the terminal is refocused afterwards. Related: xterm registers its own `copy` listener on the terminal element gated on `hasSelection()`, which is why right-click → Copy has always worked. Selection itself is unavailable on touch devices by design (`user-select: none` on the terminal subtree), and in `shell`/`opencode`/`antigravity` tabs the TUI owns the mouse, so selecting there needs Shift+drag. Tests: `test/terminal-copy-selection.test.ts` (gate + wiring invariants), `test/terminal-copy-shortcut.test.ts` (browser, real key presses).
|
||||||
|
|
||||||
**The main terminal's four copy paths clean the selection first** (`CodemanCopySelection.clean` in constants.js, pure; `cleanedTerminalSelection()` in terminal-ui.js is the half that reads the live terminal). Those four are the `Ctrl+C` chord, right-click, the phone selection button and Auto Copy. ⚠️ Three routes still copy the RAW padded rows, all of them predating the clean: the browser's own Edit → Copy, which xterm's own `copy` listener on the terminal element serves with `selectionText` directly; a `copy-selection` shortcut the user disabled in App Settings, where nothing calls `preventDefault()` and that native listener runs; and the subagent/teammate windows, which build their own `Terminal` in panels-ui.js with no copy wiring at all. xterm hands back whole screen ROWS and its own trim drops only cells that were never written to, so the real spaces a full-screen TUI paints across the unused part of a row count as content and reach the clipboard. Measured against Claude Code in a 282-column pane, single lines arrived carrying 138 trailing spaces on top of the two-space transcript indent. The clean drops each line's trailing run, and strips a LEADING margin when the pane behind the selection turns out to have one. Four rules keep it honest:
|
**The main terminal's four copy paths clean the selection first** (`CodemanCopySelection.clean` in constants.js, pure; `cleanedTerminalSelection()` in terminal-ui.js is the half that reads the live terminal). Those four are the `Ctrl+C` chord, right-click, the phone selection button and Auto Copy. ⚠️ Three routes still copy the RAW padded rows, all of them predating the clean: the browser's own Edit → Copy, which xterm's own `copy` listener on the terminal element serves with `selectionText` directly; a `copy-selection` shortcut the user disabled in App Settings, where nothing calls `preventDefault()` and that native listener runs; and the subagent/teammate windows, which build their own `Terminal` in panels-ui.js with no copy wiring at all. xterm hands back whole screen ROWS and its own trim drops only cells that were never written to, so the real spaces a full-screen TUI paints across the unused part of a row count as content and reach the clipboard. Measured against Claude Code in a 282-column pane, single lines arrived carrying 138 trailing spaces on top of the two-space transcript indent. The clean drops each line's trailing run, and strips a LEADING margin when the session's CLI declares one. Four rules keep it honest:
|
||||||
|
|
||||||
1. **A leading strip uses the width the CLI DECLARES, never one derived from the pane.** `capabilities.transcriptGutter` (CLI registry, a bounded integer) is the whole source: claude and codex each declare 2, no other stock entry declares any, and an entry that declares nothing never has a margin taken off a copy. The server publishes the map as `window.__codemanTranscriptGutter`, built by filtering `enabledClis()` on that capability rather than by listing ids, and `_cliGutterColumns()` (terminal-ui.js) looks that session's mode up in it, defaulting to the active session and taking an explicit id for Pane B of a split. ⚠️ **Three ways of deriving the width from the text were built and measured, and two of them shipped looking correct.** The selection's own shared indent fired on **73%** of 401,445 three-row windows across 1,010 tracked files, because a three-row window of nested YAML shares an indent for the same reason a margin does. **Painted trailing padding** — a full-screen TUI writes real spaces across the part of a row it is not using, while a shell leaves those cells never-written for xterm to trim — has no false positives and never over-stripped, and is nonetheless a function of pane WIDTH: the padding exists only while a rendered line stops short of the CLI's own layout width, and Claude Code's prose wraps to fill it. Dragging the same two prose rows of one live transcript at five window sizes, the share of rows carrying padding measured 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 columns, so at every ordinary size the strip did nothing at all while every test and every 282-column measurement said it worked. The **narrowest indent on the surrounding rows** fires at every width and over-strips about 1% of selections, because a file listing inside the transcript can be the narrowest thing on screen. ⚠️ The declared width is a **CEILING, not the answer**: `clean()` strips the lesser of it and the run every selected line shares, so a block can only ever shift as a unit, the relative structure inside a selection survives by construction, and a selection reaching column 0 loses nothing. That is what keeps a `git log` body at its own four spaces inside an agent's two-column gutter. Measured over 1,392,281 selections — every 1, 2, 3, 5, 10 and 20-row window of real Claude screens replayed from live PTY streams at 100, 120, 160, 198, 235 and 282 columns — it over-strips none, breaks no relative indent and alters no text, at 100% of the selections whose own indent covers the gutter. ⚠️ Nothing reads the terminal buffer on this path; the old version scanned up to ~240 rows per `Ctrl+C` to measure a width the CLI can simply state. ⚠️ **The margin strip is not idempotent, so no caller may clean twice.** It takes the lesser of the declared width and the shared run, so a second pass takes up to `margin` columns more off whatever the first left. The trailing trim alone is a fixed point, and `copyTerminalSelection()` relied on that by re-cleaning whatever argument it was handed, which dedented every claude and codex copy twice on the `Ctrl+C` path — the most-used of the four, while right-click, the phone selection button and Auto Copy stayed correct because each hands over the raw selection or reads it live. The gate still tests the CLEANED string, so a padding-only selection still falls through to the PTY, and the RAW string is what travels on. A source-level test pins that branch, because it lives inside `initTerminal`'s `attachCustomKeyEventHandler` closure over a real xterm that the vm harness cannot build.
|
1. **A leading strip uses the width the CLI DECLARES, never one derived from the pane.** `capabilities.transcriptGutter` (CLI registry, a bounded integer) is the whole source: claude and codex each declare 2, no other stock entry declares any, and an entry that declares nothing never has a margin taken off a copy. ⚠️ The lookup keys on the session's LAUNCH mode, not on what is running in the pane: a claude-mode session whose pane has dropped to a plain shell (the CLI exited, or a shell was started inside it) still takes up to two columns off a copy, so a `cat`'d block indented four comes out at two. The ceiling rule below keeps its relative indentation intact and `copyStripMargin` (App Settings, "Trim the pane margin on copy") is the escape hatch, so this is a known limitation, not a guarantee the strip only ever fires on a transcript. The server publishes the map as `window.__codemanTranscriptGutter`, built by filtering `enabledClis()` on that capability rather than by listing ids, and `_cliGutterColumns()` (terminal-ui.js) looks that session's mode up in it, defaulting to the active session and taking an explicit id for Pane B of a split. ⚠️ **Three ways of deriving the width from the text were built and measured, and two of them shipped looking correct.** The selection's own shared indent fired on **73%** of 401,445 three-row windows across 1,010 tracked files, because a three-row window of nested YAML shares an indent for the same reason a margin does. **Painted trailing padding** — a full-screen TUI writes real spaces across the part of a row it is not using, while a shell leaves those cells never-written for xterm to trim — has no false positives and never over-stripped, and is nonetheless a function of pane WIDTH: the padding exists only while a rendered line stops short of the CLI's own layout width, and Claude Code's prose wraps to fill it. Dragging the same two prose rows of one live transcript at five window sizes, the share of rows carrying padding measured 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 columns, so at every ordinary size the strip did nothing at all while every test and every 282-column measurement said it worked. The **narrowest indent on the surrounding rows** fires at every width and over-strips about 1% of selections, because a file listing inside the transcript can be the narrowest thing on screen. ⚠️ The declared width is a **CEILING, not the answer**: `clean()` strips the lesser of it and the run every selected line shares, so a block can only ever shift as a unit, the relative structure inside a selection survives by construction, and a selection reaching column 0 loses nothing. That is what keeps a `git log` body at its own four spaces inside an agent's two-column gutter. Measured over 1,392,281 selections — every 1, 2, 3, 5, 10 and 20-row window of real Claude screens replayed from live PTY streams at 100, 120, 160, 198, 235 and 282 columns — it over-strips none, breaks no relative indent and alters no text, at 100% of the selections whose own indent covers the gutter. ⚠️ Nothing reads the terminal buffer on this path; the old version scanned up to ~240 rows per `Ctrl+C` to measure a width the CLI can simply state. ⚠️ **The margin strip is not idempotent, so no caller may clean twice.** It takes the lesser of the declared width and the shared run, so a second pass takes up to `margin` columns more off whatever the first left. The trailing trim alone is a fixed point, and `copyTerminalSelection()` relied on that by re-cleaning whatever argument it was handed, which dedented every claude and codex copy twice on the `Ctrl+C` path — the most-used of the four, while right-click, the phone selection button and Auto Copy stayed correct because each hands over the raw selection or reads it live. The gate still tests the CLEANED string, so a padding-only selection still falls through to the PTY, and the RAW string is what travels on. A source-level test pins that branch, because it lives inside `initTerminal`'s `attachCustomKeyEventHandler` closure over a real xterm that the vm harness cannot build.
|
||||||
2. **A COLUMN selection is returned untouched.** Alt+drag makes one (xterm's `shouldColumnSelect` keys on `altKey` alone, and neither `Terminal` Codeman builds passes the one option, `macOptionClickForcesSelection`, that would disable it), and a rectangle's rows lining up is the whole point of the gesture. xterm exposes the mode nowhere public, so the check reads `terminal._core._selectionService._activeSelectionMode` (`SelectionMode.COLUMN` is 3) and cleans normally if a future xterm renames it.
|
2. **A COLUMN selection is returned untouched.** Alt+drag makes one (xterm's `shouldColumnSelect` keys on `altKey` alone, and neither `Terminal` Codeman builds passes the one option, `macOptionClickForcesSelection`, that would disable it), and a rectangle's rows lining up is the whole point of the gesture. xterm exposes the mode nowhere public, so the check reads `terminal._core._selectionService._activeSelectionMode` (`SelectionMode.COLUMN` is 3) and cleans normally if a future xterm renames it.
|
||||||
3. **The mid-row flag reads an ORDERED range, and it governs one line.** ⚠️ `getSelectionPosition()` is reversal-aware on the pinned xterm and the review note that called it the raw mousedown anchor is out of date: `CoreBrowserTerminal.ts` reads `_selectionService.selectionStart`, whose getter returns `SelectionModel.finalSelectionStart`, and that swaps the pair when `areSelectionValuesReversed()` says so. Driving a real upward mouse drag through chromium against xterm 6.0 reports the same range as the downward drag of the same rows. `_normalisedSelectionRange()` orders the pair anyway, because the private model one layer down exposes unnormalised fields under the same two names, and a reversed pair would put the mid-row flag on the wrong end of the drag. `range.start.x > 0` then means the first selected line began mid-row and never carried the margin, so that line alone is left out of both the shared-indent ceiling and the strip. ⚠️ This is the ONE thing the mousedown column still decides and it decides it for that line only. In the dropped version it decided whether the first row joined the measurement, which changed what **every** row lost, so the same three rows produced three different clipboard results depending on where the click landed.
|
3. **The mid-row flag reads an ORDERED range, and it governs one line.** ⚠️ `getSelectionPosition()` is reversal-aware on the pinned xterm and the review note that called it the raw mousedown anchor is out of date: `CoreBrowserTerminal.ts` reads `_selectionService.selectionStart`, whose getter returns `SelectionModel.finalSelectionStart`, and that swaps the pair when `areSelectionValuesReversed()` says so. Driving a real upward mouse drag through chromium against xterm 6.0 reports the same range as the downward drag of the same rows. `_normalisedSelectionRange()` orders the pair anyway, because the private model one layer down exposes unnormalised fields under the same two names, and a reversed pair would put the mid-row flag on the wrong end of the drag. `range.start.x > 0` then means the first selected line began mid-row and never carried the margin, so that line alone is left out of both the shared-indent ceiling and the strip. ⚠️ This is the ONE thing the mousedown column still decides and it decides it for that line only. In the dropped version it decided whether the first row joined the measurement, which changed what **every** row lost, so the same three rows produced three different clipboard results depending on where the click landed.
|
||||||
4. **The emptiness gate is `trim()`, not truthiness, and it still clears the selection.** A multi-row drag across padding cleans to line breaks alone, which are truthy, and a bare newline pasted into a chat composer submits it. ⚠️ The clear is FEEDBACK, not protection for the interrupt, and the comments that said otherwise were describing the pre-clean code: the `Ctrl+C` gate above now tests the CLEANED selection, so a padding-only selection left set cleans to `''` on every later press and falls through to the PTY as `0x03` anyway. What the clear buys is that a highlight which copied nothing does not linger unexplained, which is also what the toast is for.
|
4. **The emptiness gate is `trim()`, not truthiness, and it still clears the selection.** A multi-row drag across padding cleans to line breaks alone, which are truthy, and a bare newline pasted into a chat composer submits it. ⚠️ The clear is FEEDBACK, not protection for the interrupt, and the comments that said otherwise were describing the pre-clean code: the `Ctrl+C` gate above now tests the CLEANED selection, so a padding-only selection left set cleans to `''` on every later press and falls through to the PTY as `0x03` anyway. What the clear buys is that a highlight which copied nothing does not linger unexplained, which is also what the toast is for.
|
||||||
|
|||||||
@@ -214,8 +214,8 @@ const CLAUDE: CliEntry = {
|
|||||||
capabilities: {
|
capabilities: {
|
||||||
external: false,
|
external: false,
|
||||||
// Claude indents its transcript body two columns and puts its own ●/✻/❯ markers
|
// Claude indents its transcript body two columns and puts its own ●/✻/❯ markers
|
||||||
// in them, so a copy can drop two and paste flush. The only entry that declares
|
// in them, so a copy can drop two and paste flush. Claude and codex are the only
|
||||||
// this, because it is the only one whose gutter has been measured.
|
// entries that declare this, because theirs are the only gutters that have been measured.
|
||||||
transcriptGutter: 2,
|
transcriptGutter: 2,
|
||||||
// The historical hard-coded pair, now stated as data. `workingLine` matches both the
|
// The historical hard-coded pair, now stated as data. `workingLine` matches both the
|
||||||
// `✻ Actualizing… (39s · ↓ 2.0k tokens)` status line and the bare `esc to interrupt`
|
// `✻ Actualizing… (39s · ↓ 2.0k tokens)` status line and the bare `esc to interrupt`
|
||||||
|
|||||||
@@ -143,6 +143,19 @@ describe('WebServer.renderIndexHtml', () => {
|
|||||||
expect(html).toContain('btn-multimonitor--hidden');
|
expect(html).toContain('btn-multimonitor--hidden');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('injects the transcript-gutter map for a /session/:id window too', async () => {
|
||||||
|
// Every other payload is gated on !soloSessionId, but a solo window copies from a
|
||||||
|
// terminal like the main page does, so it needs the widths the copy strip keys on.
|
||||||
|
const { server } = makeServer();
|
||||||
|
const html = await render(server, 'sess-123');
|
||||||
|
const match = html.match(/window\.__codemanTranscriptGutter=(\{[^<]*\});/);
|
||||||
|
expect(match).not.toBeNull();
|
||||||
|
const map = JSON.parse(match![1]) as Record<string, number>;
|
||||||
|
expect(map.claude).toBe(2);
|
||||||
|
expect(map.codex).toBe(2);
|
||||||
|
expect(map.shell).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
it('escapes the solo id so it cannot break out of the inline <script>', async () => {
|
it('escapes the solo id so it cannot break out of the inline <script>', async () => {
|
||||||
const { server } = makeServer({});
|
const { server } = makeServer({});
|
||||||
const html = await render(server, 'a</script><b>');
|
const html = await render(server, 'a</script><b>');
|
||||||
|
|||||||
Reference in New Issue
Block a user