Merge pull request #469 from irisitymichaelgrundberg/feat/copy-dedent-pane-margin

feat(terminal): take the transcript gutter off a copy, at the width the CLI declares

src/web/public/terminal-ui.js

@@ -368,10 +368,18 @@ Object.assign(CodemanApp.prototype, {
         // 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();
+        //
+        // ⚠️ The gate cleans, and the copy is handed the RAW selection, because
+        // copyTerminalSelection cleans again on its own. The margin strip is not
+        // idempotent: a second pass takes up to `margin` more columns off what
+        // the first pass left, so passing the cleaned string through dedented a
+        // Claude or Codex copy twice. Every other copy path already hands over
+        // the raw selection or reads it live.
+        const raw = this.terminal?.hasSelection?.() ? this.terminal.getSelection() : '';
+        const selection = this.cleanedTerminalSelection(raw);
         if (selection.trim()) {
           ev.preventDefault();
-          void this.copyTerminalSelection(selection);
+          void this.copyTerminalSelection(raw);
           return false;
         }
         // Nothing worth copying. The clear is for feedback, not for the
@@ -4168,11 +4176,16 @@ Object.assign(CodemanApp.prototype, {
    *
    * `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. The transform is idempotent on xterm output, so an
-   * already-cleaned string is an acceptable argument: a CR is consumed by the
-   * parser as a cursor move and never stored in a cell, so the only \r the
-   * selection can carry is the Windows line join, and that is what makes the
-   * trailing scan a fixed point. Fuzzed over 300 000 realistic selections.
+   * not repeated.
+   *
+   * ⚠️ **Pass the RAW selection, never an already-cleaned one.** The trailing
+   * trim alone is a fixed point, because a CR is consumed by the parser as a
+   * cursor move and never stored in a cell, so the only \r the selection can
+   * carry is the Windows line join. The MARGIN strip is not: it takes the
+   * narrower of the declared width and the run every line shares, so a second
+   * pass over an already-stripped block takes up to `margin` columns more. A
+   * caller that cleans to decide whether to copy must still hand the raw text
+   * to copyTerminalSelection, which cleans once on its own.
    *
    * A COLUMN selection comes back untouched. Alt+drag makes one (xterm's
    * shouldColumnSelect keys on altKey alone, and Codeman sets neither of the
@@ -4189,7 +4202,73 @@ Object.assign(CodemanApp.prototype, {
     if (this.terminal?._core?._selectionService?._activeSelectionMode === 3) return raw;
     const clean = window.CodemanCopySelection?.clean;
     if (!clean) return raw;
-    return clean(raw);
+    const range = this._normalisedSelectionRange();
+    return clean(raw, {
+      margin: this._cliGutterColumns(),
+      firstLinePartial: !!range && range.start.x > 0,
+    });
+  },
+
+  /**
+   * xterm's selection range with its two ends in reading order.
+   *
+   * `getSelectionPosition()` reports `start` and `end` as the two ends of the
+   * drag, and on xterm 6.0 it already hands back the earlier one first: it
+   * reads `_selectionService.selectionStart`, whose getter returns the model's
+   * `finalSelectionStart`, and that swaps the pair for a reversed selection.
+   * A real upward mouse drag through chromium confirms it. The ordering here
+   * is a guard rather than a fix. One layer down the same model exposes the
+   * UNNORMALISED fields under the same two names, and a reversed pair would
+   * make the row window below run backwards and collapse, which would report
+   * no margin at all for every upward drag in a deep buffer.
+   *
+   * `terminal` names which xterm to read, defaulting to the primary pane's.
+   * Pane B of a split owns a second terminal and passes it, because this file's
+   * `this` is always the primary pane.
+   */
+  _normalisedSelectionRange(terminal) {
+    const range = (terminal ?? this.terminal)?.getSelectionPosition?.();
+    if (!range?.start || !range?.end) return null;
+    const { start, end } = range;
+    const reversed = end.y < start.y || (end.y === start.y && end.x < start.x);
+    return reversed ? { start: end, end: start } : { start, end };
+  },
+
+  /**
+   * How many columns to take off a copy from one session's pane: the transcript
+   * gutter its CLI declares, or 0 when it declares none. `sessionId` defaults to
+   * the active session, and Pane B of a split passes its own, so both panes of a
+   * split strip the width their own CLI declares rather than Pane A's.
+   *
+   * ⚠️ Read from `window.__codemanTranscriptGutter`, the map the server derives
+   * from the `transcriptGutter` CAPABILITY at render time — never an id literal
+   * here, which is the registry's standing rule and is also what lets a CLI that
+   * declares a gutter later work with no change to this file.
+   *
+   * ⚠️ DECLARED rather than measured off the buffer, and two measured versions
+   * are why. Asking whether the pane painted spaces across the unused part of
+   * each row separates a TUI from a shell perfectly where it fires and never
+   * over-stripped, but it is a function of pane WIDTH, since that 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: the share of padded rows on one live
+   * transcript ran 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298
+   * columns, so the strip did nothing at any ordinary window size. Taking the
+   * narrowest indent on the rows around the selection instead fires at every
+   * width and over-strips on about 1% of them, because a file listing inside the
+   * transcript can be the narrowest thing on screen. A declared width does
+   * neither, and it reads no buffer rows at all on a path that runs on every
+   * Ctrl+C.
+   *
+   * A missing map means no session gets a strip, the same direction an
+   * unmeasured CLI takes by declaring nothing.
+   */
+  _cliGutterColumns(sessionId) {
+    if (!this._copyStripMarginEnabled()) return 0;
+    const byMode = window.__codemanTranscriptGutter;
+    if (!byMode || typeof byMode !== 'object') return 0;
+    const mode = this.sessions?.get(sessionId ?? this.activeSessionId)?.mode;
+    const columns = mode ? byMode[mode] : 0;
+    return Number.isInteger(columns) && columns > 0 ? columns : 0;
   },
 
   // Copy the current terminal selection. Goes through _copyText (Clipboard API,
@@ -4224,6 +4303,26 @@ Object.assign(CodemanApp.prototype, {
     return ok;
   },
 
+  /**
+   * Whether this device wants the pane's left margin off the clipboard
+   * (`copyStripMargin`, per-device, default ON).
+   *
+   * Read here rather than mirrored into a field, for the same reason
+   * `_autoCopySelectionEnabled` is: there is then no apply-path a future
+   * settings save can forget to call, and the toggle takes effect on the next
+   * selection instead of the next reload. ⚠️ The test is `!== false`, not
+   * `=== true`: this one defaults ON, and the desktop branch of
+   * getDefaultSettings returns {} and leans on the read sites for defaults, so
+   * a device that has never opened App Settings has no stored value at all.
+   */
+  _copyStripMarginEnabled() {
+    try {
+      return this.loadAppSettingsFromStorage?.()?.copyStripMargin !== false;
+    } catch {
+      return true;
+    }
+  },
+
   /**
    * Auto Copy's ON/OFF, read at flush time from the CACHED settings object
    * (loadAppSettingsFromStorage memoizes, so this is not a localStorage hit).