mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 22:49:41 +02:00
fix(terminal): trim the padding and shared indent out of a copied selection
xterm hands back whole screen rows and trims only the 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, and every line carried the two-space transcript indent as well. Windows Terminal, iTerm2 and GNOME Terminal all trim that for you, decideAutoCopy already calls a wall of spaces "never what the gesture meant", and _selectTouchSelectionLine already treats those cells as padding — the mouse and keyboard paths never had the same rule. CodemanCopySelection.clean lives in constants.js beside decideAutoCopy, its pure sibling. It drops the trailing run from each line, and removes the leading run only where every selected row shares one. A selection of a single row keeps its run, because one row shares nothing with anything and stripping it would silently reindent one line of `git log` body text or one line out of `less`. A drag that began inside a row keeps its partial first line untouched and out of the measurement, which otherwise pins the shared run to zero and leaves every following row indented. Every pass over a line is a scan rather than a regex. `/[ \t]+(\r?)$/` is quadratic on a line whose spaces are followed by a non-space character, which is what right-aligned or centred TUI content looks like: measured over 50 000 rows with a 280-column run it took 2.9s, against 1.3ms for the scan, and a 2 000-column run took 16s. The scan is also the faster of the two on an ordinary padded row. cleanedTerminalSelection in terminal-ui.js is the half that needs the live terminal. It returns a COLUMN selection untouched: Alt+drag makes one, and a rectangle's rows lining up is the point of the gesture, so both halves of the clean would destroy it. xterm exposes the mode nowhere public, so the check reads terminal._core._selectionService, the way this file already reads terminal._core for cell dimensions, and cleans normally if a future xterm renames the field. A test pins that assumption against the library rather than against a stub repeating the literal. The Ctrl+C chord decides on the cleaned selection, not the raw one. A drag across the blank part of a row selects real padding spaces, so the raw text is truthy, and testing it would spend that press on a copy of nothing and make the user press again to interrupt. A padding-only selection is now dropped and the press falls through to the PTY, while Ctrl+Shift+C still never falls through. copyTerminalSelection gates on trim() for the same reason, since a multi-row drag across padding cleans to line breaks alone and a bare newline pasted into a chat composer submits it. All four of the main terminal's copy paths go through it: the Ctrl+C chord, right-click, the phone selection button and Auto Copy. The browser's own Edit menu copy, a disabled copy shortcut and the subagent windows still copy raw rows, as they did before, and the invariants doc now says so rather than claiming every copy is cleaned. Auto Copy resolves its own toggle before it reads the selection, since it is off by default and a selection can run to the 50 000-row scrollback ceiling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
20fc7b3c3d
commit
f9edb33d15
@@ -806,6 +806,94 @@ function decideAutoCopy({ enabled, text, lastCopied, pending } = {}) {
|
||||
return 'copy';
|
||||
}
|
||||
|
||||
// The text a copy should put on the clipboard, given xterm's raw selection.
|
||||
// Pure: the caller reads the selection and decides the mode, this transforms.
|
||||
//
|
||||
// xterm hands back whole screen ROWS, and its own trim only drops cells that
|
||||
// were never written to. A full-screen TUI writes real spaces across the part
|
||||
// of a row it is not using, so that padding counts as content and rides along
|
||||
// to the clipboard: measured against Claude Code in a 282-column pane, single
|
||||
// lines arrived carrying 138 trailing spaces. Native terminals trim it on copy
|
||||
// (Windows Terminal, iTerm2 and GNOME Terminal all do), decideAutoCopy above
|
||||
// already calls a wall of spaces "never what the gesture meant", and
|
||||
// _selectTouchSelectionLine already treats those cells as padding. This is that
|
||||
// same rule for the mouse and keyboard paths, which never had it.
|
||||
//
|
||||
// The leading run is the other half, and it applies ACROSS ROWS ONLY. A TUI
|
||||
// that indents its whole transcript repeats the indent on every row, so a
|
||||
// multi-row selection arrives with the chrome baked into each line; only the
|
||||
// run every selected row shares is removed, which is a no-op for shell output
|
||||
// and keeps the relative indentation of anything nested inside. A selection of
|
||||
// ONE row shares nothing with anything, so its leading spaces are content and
|
||||
// stay put — otherwise a single line of `git log` body text, or one line out of
|
||||
// `less`, would silently lose its indentation.
|
||||
//
|
||||
// ⚠ The trailing trim takes spaces AND tabs while the leading run counts spaces
|
||||
// only, so one tab-led row disables the strip for its whole block. Terminals
|
||||
// expand tabs into cells, so a tab should never reach either rule; the
|
||||
// asymmetry is deliberate caution rather than an oversight.
|
||||
//
|
||||
// ⚠ A WRAPPED logical line keeps its continuation indent. xterm appends a
|
||||
// wrapped row to the previous entry instead of starting a new line, so rows
|
||||
// 2..n of one wrapped line sit mid-string where no line rule can see them. That
|
||||
// is inherent to cleaning xterm's output rather than a gap to fix here.
|
||||
function cleanCopiedSelection(text, { startedMidRow = false } = {}) {
|
||||
if (typeof text !== 'string' || !text) return '';
|
||||
// Split on \n and leave any \r in place: xterm joins rows with \r\n on
|
||||
// Windows, and the clipboard should keep the endings xterm chose.
|
||||
// Scanned rather than matched, throughout. A selection can run to the 50 000-row
|
||||
// scrollback ceiling, and `/[ \t]+(\r?)$/` is QUADRATIC on a line whose spaces
|
||||
// are followed by any non-space character, which is what right-aligned or
|
||||
// centred TUI content looks like: the engine retries the run from every
|
||||
// whitespace position and backtracks over it. Measured over 50 000 rows with a
|
||||
// 280-column run, that regex took 2.9s against 1.3ms for the scan below, and a
|
||||
// 2 000-column run took 16s. It is also the faster of the two on an ordinary
|
||||
// padded row. A length is returned rather than a trimmed string so a
|
||||
// \r-terminated line costs no substring either.
|
||||
const trimEnd = (line) => {
|
||||
let end = line.length;
|
||||
if (end > 0 && line[end - 1] === '\r') end--;
|
||||
let cut = end;
|
||||
while (cut > 0 && (line[cut - 1] === ' ' || line[cut - 1] === '\t')) cut--;
|
||||
return cut === end ? line : line.slice(0, cut) + line.slice(end);
|
||||
};
|
||||
const lines = text.split('\n').map(trimEnd);
|
||||
const bareLen = (line) => (line.endsWith('\r') ? line.length - 1 : line.length);
|
||||
const leadingRun = (line) => {
|
||||
let n = 0;
|
||||
while (n < line.length && line[n] === ' ') n++;
|
||||
return n;
|
||||
};
|
||||
|
||||
// ⚠ Counted over EVERY line, including a partial first line the loop below
|
||||
// skips. A mid-row drag across two rows therefore measures one row and strips
|
||||
// it. That is deliberate: both rows of a wrapped paragraph wear the TUI's
|
||||
// margin, and the drag only hid the first one's. The cost is that a two-row
|
||||
// mid-row drag over genuinely indented content loses that indent.
|
||||
let contentRows = 0;
|
||||
for (const line of lines) if (bareLen(line)) contentRows++;
|
||||
if (contentRows < 2) return lines.join('\n');
|
||||
|
||||
// startedMidRow keeps the first line out of the measurement. A drag that
|
||||
// begins inside a row gives a first line with no leading run at all, which
|
||||
// would otherwise pin the shared run to zero and leave every row after it
|
||||
// still wearing the indent. That partial line is never stripped either.
|
||||
let shared = Infinity;
|
||||
for (let i = startedMidRow ? 1 : 0; i < lines.length; i++) {
|
||||
// A row left blank by the trailing trim says nothing about the indent, and
|
||||
// counting it as zero would disable the strip for the whole block.
|
||||
if (!bareLen(lines[i])) continue;
|
||||
shared = Math.min(shared, leadingRun(lines[i]));
|
||||
if (shared === 0) break;
|
||||
}
|
||||
if (!shared || shared === Infinity) return lines.join('\n');
|
||||
// The clamp is what keeps a blank row's lone \r intact when the shared run is
|
||||
// wider than that row is long.
|
||||
return lines
|
||||
.map((line, i) => (startedMidRow && i === 0 ? line : line.slice(Math.min(shared, leadingRun(line)))))
|
||||
.join('\n');
|
||||
}
|
||||
|
||||
if (typeof window !== 'undefined') {
|
||||
window.WEBGL_FALLBACK = WEBGL_FALLBACK;
|
||||
window.evaluateWebGLLongTaskTrip = evaluateWebGLLongTaskTrip;
|
||||
@@ -854,6 +942,9 @@ if (typeof window !== 'undefined') {
|
||||
decide: decideAutoCopy,
|
||||
MAX_CHARS: AUTO_COPY_MAX_CHARS,
|
||||
};
|
||||
window.CodemanCopySelection = {
|
||||
clean: cleanCopiedSelection,
|
||||
};
|
||||
window.CodemanTerminalFont = {
|
||||
DEFAULT_STACK: TERMINAL_FONT_DEFAULT_STACK,
|
||||
resolve: resolveTerminalFontFamily,
|
||||
|
||||
@@ -364,12 +364,23 @@ Object.assign(CodemanApp.prototype, {
|
||||
// this handler before its own cancel()), so preventDefault is explicit:
|
||||
// without it the browser runs its native copy on top of ours.
|
||||
if (this.shouldCopyTerminalSelectionFromShortcut?.(ev)) {
|
||||
const selection = this.terminal.hasSelection?.() ? this.terminal.getSelection() : '';
|
||||
if (selection) {
|
||||
// The CLEANED selection decides, not the raw one. A drag across the blank
|
||||
// part of a row selects real padding spaces, which are truthy, so testing
|
||||
// the raw text would spend this press on a copy of nothing and make the
|
||||
// user press again to interrupt.
|
||||
const selection = this.cleanedTerminalSelection();
|
||||
if (selection.trim()) {
|
||||
ev.preventDefault();
|
||||
void this.copyTerminalSelection(selection);
|
||||
return false;
|
||||
}
|
||||
// Nothing worth copying. Drop a padding-only selection first, or it would
|
||||
// intercept every following press too, then fall through exactly as an
|
||||
// empty selection does so this press still reaches the PTY as 0x03.
|
||||
if (this.terminal?.hasSelection?.()) {
|
||||
this.terminal.clearSelection?.();
|
||||
this.showToast('Nothing to copy', 'warning');
|
||||
}
|
||||
if (ev.shiftKey) {
|
||||
ev.preventDefault();
|
||||
return false;
|
||||
@@ -4069,12 +4080,51 @@ Object.assign(CodemanApp.prototype, {
|
||||
return !ev.altKey && (ev.key || '').toLowerCase() === 'c';
|
||||
},
|
||||
|
||||
/**
|
||||
* xterm's current selection, cleaned for the clipboard. The transform itself
|
||||
* is CodemanCopySelection.clean in constants.js, beside decideAutoCopy; this
|
||||
* is the half that needs the live terminal.
|
||||
*
|
||||
* `text` is for the callers that already read the selection to decide whether
|
||||
* to copy at all (the Ctrl+C gate and the right-click handler), so the read is
|
||||
* not repeated. It must be the selection xterm holds RIGHT NOW, because the
|
||||
* mid-row flag below comes from the live selection rather than from `text`.
|
||||
*
|
||||
* A COLUMN selection comes back untouched. Alt+drag makes one — xterm's
|
||||
* shouldColumnSelect keys on altKey alone, and Codeman sets neither of the
|
||||
* terminals it creates with the one option that would disable it — and a rectangle's whole point is that its rows line
|
||||
* up, which both halves of the clean would destroy. xterm exposes the mode
|
||||
* nowhere public, so this reads the private field the way this file already
|
||||
* reads terminal._core for cell dimensions, and falls back to cleaning
|
||||
* normally if a future xterm renames it. SelectionMode.COLUMN is 3.
|
||||
*/
|
||||
cleanedTerminalSelection(text) {
|
||||
const raw = text ?? (this.terminal?.hasSelection?.() ? this.terminal.getSelection() : '');
|
||||
if (!raw) return '';
|
||||
if (this.terminal?._core?._selectionService?._activeSelectionMode === 3) return raw;
|
||||
const clean = window.CodemanCopySelection?.clean;
|
||||
if (!clean) return raw;
|
||||
const start = this.terminal?.getSelectionPosition?.()?.start;
|
||||
return clean(raw, { startedMidRow: !!start && start.x > 0 });
|
||||
},
|
||||
|
||||
// Copy the current terminal selection. Goes through _copyText (Clipboard API,
|
||||
// then a hidden-textarea + execCommand fallback) because install.sh's LAN
|
||||
// option serves plain HTTP, where navigator.clipboard is undefined.
|
||||
async copyTerminalSelection(text) {
|
||||
const selection = text ?? (this.terminal.hasSelection?.() ? this.terminal.getSelection() : '');
|
||||
if (!selection) return false;
|
||||
const selection = this.cleanedTerminalSelection(text);
|
||||
// trim(), not emptiness: a multi-row drag across padding cleans to newlines
|
||||
// alone, which are truthy, and a bare newline pasted into a chat composer
|
||||
// or a shell submits the line. decideAutoCopy applies the same rule.
|
||||
if (!selection.trim()) {
|
||||
// Clearing matters as much as the toast. The Ctrl+C gate tests the RAW
|
||||
// selection, so a padding-only selection left set would make every later
|
||||
// Ctrl+C copy nothing instead of interrupting — the exact failure
|
||||
// docs/architecture-invariants.md warns about under Terminal smart copy.
|
||||
this.terminal?.clearSelection?.();
|
||||
this.showToast('Nothing to copy', 'warning');
|
||||
return false;
|
||||
}
|
||||
const ok = await this._copyText(selection);
|
||||
if (ok) {
|
||||
// Clearing is what makes a second Ctrl+C an interrupt (and xterm already
|
||||
@@ -4123,9 +4173,15 @@ Object.assign(CodemanApp.prototype, {
|
||||
async _flushAutoCopySelection() {
|
||||
const decide = window.CodemanAutoCopy?.decide;
|
||||
if (!decide || !this.terminal) return;
|
||||
const text = this.terminal.hasSelection?.() ? this.terminal.getSelection() : '';
|
||||
// The toggle is read FIRST because Auto Copy is off by default: reading and
|
||||
// cleaning a selection that can run to the 50 000-row scrollback ceiling
|
||||
// costs real time on a phone, and every mouseup would pay it for nothing.
|
||||
// Cleaning before decide() then means its dedupe and size cap both measure
|
||||
// the text that actually reaches the clipboard, not the padded rows behind.
|
||||
const enabled = this._autoCopySelectionEnabled();
|
||||
const text = enabled ? this.cleanedTerminalSelection() : '';
|
||||
const verdict = decide({
|
||||
enabled: this._autoCopySelectionEnabled(),
|
||||
enabled,
|
||||
text,
|
||||
lastCopied: this._autoCopyLastText,
|
||||
pending: !!this._autoCopyPending,
|
||||
|
||||
Reference in New Issue
Block a user