From b965c3d34602b3d62ef44e7f8ad67007af0990be Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 7 Oct 2026 03:33:05 +0200 Subject: [PATCH] refactor(split): a tile's Ctrl+C copies through the primary pane's copy helpers TerminalTile carried its own copy of the smart-copy branch (clean the selection with this session's gutter, copy, clear, toast). PR 1 gave cleanedTerminalSelection and copyTerminalSelection a `{ terminal, sessionId }` target for exactly this, and nothing passed it. The tile now calls both with its own terminal and session, and its copy code is gone. Two things change for a tile, both to the primary pane's rule: a clipboard write that fails keeps the selection (nothing was copied, so it stays for a retry) instead of clearing it, and focus returns to the tile's xterm after the copy, which matters when the execCommand fallback focused a temporary textarea. Pinned in terminal-tile-input with the write failing and succeeding, and the no-selection Ctrl+C / Ctrl+Shift+C split. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/public/terminal-tile.js | 61 ++++++++------------------ test/terminal-tile-input.test.ts | 73 ++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 43 deletions(-) diff --git a/src/web/public/terminal-tile.js b/src/web/public/terminal-tile.js index 11b449be..10ab66a8 100644 --- a/src/web/public/terminal-tile.js +++ b/src/web/public/terminal-tile.js @@ -280,57 +280,32 @@ } return false; } - // Smart copy (mirrors terminal-ui.js's Ctrl+C gate, #211): with a - // selection, Ctrl+C copies THIS pane's own selection instead of - // sending ^C; with none, plain Ctrl+C must fall through unchanged or - // the interrupt key is lost. Ctrl+Shift+C is different: it is the - // explicit, never-falls-through copy chord, and the predicate above - // does not distinguish it from plain Ctrl+C — ev.shiftKey does, below. - // xterm's own evaluateKeyboardEvent routes a shifted ctrl-letter into - // a branch that assigns c.key only for a couple of special cases - // ("_"->US, "@"->NUL), neither of which is "c", so it emits NOTHING - // for Ctrl+Shift+C either way — this is not about an accidental - // interrupt byte reaching the PTY (verified live: it does not). - // Gating this whole block on hasSelection() (an earlier draft) meant - // that with no selection Ctrl+Shift+C skipped straight to `return - // true`, silently ceding the keystroke to the BROWSER's own handling - // (e.g. Chrome's Inspect-Element binding) with no feedback and no - // attempt to copy, unlike Pane A, which always intercepts it. - // Re-implemented against this.terminal rather than reusing - // app.copyTerminalSelection(), which reads app.terminal — Pane A's — - // and would copy the wrong pane's selection. + // Smart copy, the primary pane's rule (terminal-ui.js's Ctrl+C gate, + // #211) through the SAME helpers, aimed at THIS pane: the gutter width + // comes from this session's run mode, the partial first line from this + // terminal's selection, and the clear and refocus after the copy land + // here. With a selection worth copying, Ctrl+C copies instead of + // sending ^C; with none, plain Ctrl+C falls through unchanged or the + // interrupt key is lost. Ctrl+Shift+C is the explicit copy chord and + // never falls through (ev.shiftKey, below): with nothing to copy it + // would otherwise reach the browser's own binding for that chord. + // As in the primary gate, the CLEANED selection decides and the copy is + // handed the RAW one, because the margin strip is not idempotent. if (ev.type === 'keydown' && global.app?.shouldCopyTerminalSelectionFromShortcut?.(ev)) { + const app = global.app; + const target = { terminal: this.terminal, sessionId: this.sessionId }; const raw = this.terminal?.getSelection?.() || ''; - const isColumnSelection = this.terminal?._core?._selectionService?._activeSelectionMode === 3; - // Both clean options are read for THIS pane, never the primary one: - // the gutter width comes from this.sessionId's own run mode, and the - // partial-first-line flag from this terminal's own selection range. - // Passing neither left Pane B keeping a margin Pane A dropped, on the - // same split and the same keystroke. - const range = global.app?._normalisedSelectionRange?.(this.terminal); - const selection = isColumnSelection - ? raw - : (global.CodemanCopySelection?.clean?.(raw, { - margin: global.app?._cliGutterColumns?.(this.sessionId) ?? 0, - firstLinePartial: !!range && range.start.x > 0, - }) ?? raw); - if (selection.trim()) { + if (app.cleanedTerminalSelection?.(raw, target)?.trim()) { ev.preventDefault(); - void global.app._copyText?.(selection).then((ok) => { - this.terminal?.clearSelection?.(); - global.app.showToast?.(ok ? 'Copied to clipboard' : 'Failed to copy', ok ? 'success' : 'error'); - }); + void app.copyTerminalSelection(raw, target); return false; } - // Nothing worth copying — clear for feedback (a padding-only - // selection cleans to '' and this press still falls through to the - // PTY as 0x03, matching the primary pane's own rule). + // Nothing worth copying: cleared for feedback, and the press still + // reaches the PTY as 0x03, as in the primary pane. if (this.terminal?.hasSelection?.()) { this.terminal.clearSelection?.(); - global.app.showToast?.('Nothing to copy', 'warning'); + app.showToast?.('Nothing to copy', 'warning'); } - // Ctrl+Shift+C never falls through, even with nothing to copy — - // matches terminal-ui.js's own ev.shiftKey branch. if (ev.shiftKey) { ev.preventDefault(); return false; diff --git a/test/terminal-tile-input.test.ts b/test/terminal-tile-input.test.ts index dd87d6fc..f703166d 100644 --- a/test/terminal-tile-input.test.ts +++ b/test/terminal-tile-input.test.ts @@ -125,6 +125,18 @@ class FakeTerminal { type(data: string) { this.dataCb?.(data); } + /** What a drag selected; '' is no selection. */ + selection = ''; + hasSelection() { + return this.selection !== ''; + } + getSelection() { + return this.selection; + } + clearSelection = vi.fn(() => { + this.selection = ''; + }); + focus = vi.fn(); } const fetchMock = vi.fn(); @@ -695,6 +707,67 @@ describe('TerminalTile links and paste follow THIS pane', () => { }); }); +describe("TerminalTile Ctrl+C copies through the primary pane's copy helpers", () => { + const ctrlC = (extra: Record = {}) => ({ + type: 'keydown', + key: 'c', + code: 'KeyC', + ctrlKey: true, + preventDefault: vi.fn(), + ...extra, + }); + + it("copies THIS pane's selection; a failed write keeps it and focus returns to this pane", async () => { + const app = makeApp(); + const copyText = vi.fn(async () => false); + app._copyText = copyText; + const { term } = await connectTile(app); + term.selection = 'npm run build'; + + const ev = ctrlC(); + expect(term.keyHandler!(ev)).toBe(false); + await new Promise((r) => setTimeout(r, 0)); + + expect(ev.preventDefault).toHaveBeenCalled(); + expect(copyText).toHaveBeenCalledWith('npm run build'); + expect(app.showToast).toHaveBeenCalledWith('Failed to copy', 'error'); + // As in the primary pane: nothing was copied, so the selection stays for a retry. + expect(term.clearSelection).not.toHaveBeenCalled(); + expect(term.selection).toBe('npm run build'); + // The execCommand fallback focuses a temporary textarea; the keyboard comes back here. + expect(term.focus).toHaveBeenCalled(); + }); + + it('a successful write clears the selection (a second Ctrl+C interrupts) and refocuses this pane', async () => { + const app = makeApp(); + app._copyText = vi.fn(async () => true); + const { term } = await connectTile(app); + term.selection = 'npm run build'; + + expect(term.keyHandler!(ctrlC())).toBe(false); + await new Promise((r) => setTimeout(r, 0)); + + expect(app.showToast).toHaveBeenCalledWith('Copied to clipboard', 'success'); + expect(term.clearSelection).toHaveBeenCalled(); + expect(term.focus).toHaveBeenCalled(); + }); + + it('with nothing selected, Ctrl+C reaches the PTY and Ctrl+Shift+C does not', async () => { + const app = makeApp(); + const copyText = vi.fn(async () => true); + app._copyText = copyText; + const { term } = await connectTile(app); + + const plain = ctrlC(); + expect(term.keyHandler!(plain)).toBe(true); + expect(plain.preventDefault).not.toHaveBeenCalled(); + const shifted = ctrlC({ key: 'C', shiftKey: true }); + expect(term.keyHandler!(shifted)).toBe(false); + expect(shifted.preventDefault).toHaveBeenCalled(); + expect(copyText).not.toHaveBeenCalled(); + }); +}); + describe('TerminalTile claims the keyboard for the app-level shortcuts', () => { it('focusing its terminal makes it the focused pane; destroy() hands the keyboard back', async () => { const app = makeApp();