From a3fe210fb8f25fa2b5cf5ddc05eed433126b9f5a Mon Sep 17 00:00:00 2001 From: Neil Date: Sun, 13 Sep 2026 22:12:43 -0700 Subject: [PATCH] fix(terminal): honour the gutter-trim setting on mobile copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mobile stripped the gutter unconditionally, so turning "Trim Gutter on Copy" off left one surface still rewriting the clipboard. Mobile now mirrors the desktop preference through the existing settings.get RPC — a host predating the setting sends no key, which reads as on, matching the desktop default. Also folds the single-use gutter helpers into their callers so the shared module exposes one function. --- .../mobile-session-route-parity.test.ts | 8 +-- .../use-mobile-session-accessory-selection.ts | 7 ++- .../terminal-copy-gutter-preference.ts | 43 +++++++++++++ .../settings-read-operations.test.ts | 21 ++++++- .../src/transport/settings-read-operations.ts | 24 ++++++++ .../terminal-clipboard-selection-text.test.ts | 2 +- .../terminal-clipboard-selection-text.ts | 7 +-- src/shared/terminal-selection-gutter.test.ts | 13 ++-- src/shared/terminal-selection-gutter.ts | 61 +++++++++---------- 9 files changed, 136 insertions(+), 50 deletions(-) create mode 100644 mobile/src/terminal/terminal-copy-gutter-preference.ts diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index 76435561664..7d0c5c6de46 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -62,11 +62,11 @@ const HOST_COMPONENT_NAMES = new Set([ 'View' ]) -const HEAD_MAIN_HOOK_SHA256 = 'c3e33699e3e3fa7e24408f3d4946fcc451e9b9419442d985c4ccde01782e5114' -const HEAD_HOOK_BINDING_SHA256 = '7f907e028893721d662eeee0aa9002ad1e00359948f39fb148d274596cd9b3c0' +const HEAD_MAIN_HOOK_SHA256 = '11cd92aec686a6e47b23114ec31da86152a850b064821578b165fabfbce53b27' +const HEAD_HOOK_BINDING_SHA256 = 'f8bce7101a26b4d794bb58dee54702424a4965cc81dec5c758ca56cd5a6f4ce8' const HEAD_CALLBACK_IDENTITY_SHA256 = '2a9e4825df007f6ef53b81aa5004991d6318eee7507b44d625c07e630be432eb' -const HEAD_CALLBACK_BODY_SHA256 = 'af7f3c62954250d4be7ee432ecd10dc2689792aad8230fed2d1d68bbc892d776' +const HEAD_CALLBACK_BODY_SHA256 = '85c4f4605e66c45e2b6bc7de739cb3493d9e2d0db9c9242c379db8ed34a8cefe' const HEAD_EFFECT_SHA256 = 'd9ebfaabc1e79773cdada7ab370b20459ed972f1f8edce1652199f4d0391cd13' const HEAD_CONTENT_HOOK_SHA256 = '9c3b612fef3f370d66873aefdbe1d701f20cb64ded31fef5cc45fde6f8189581' const HEAD_NESTED_FUNCTION_SHA256 = @@ -472,7 +472,7 @@ describe('mobile session route extraction parity', () => { const contentBindings = CONTENT_COMPONENT_NAMES.flatMap( (name) => readHookFacts(name, definitions).bindings ) - expect(main.hooks).toHaveLength(267) + expect(main.hooks).toHaveLength(268) expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256) expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256) expect(main.callbacks).toHaveLength(77) diff --git a/mobile/src/session/use-mobile-session-accessory-selection.ts b/mobile/src/session/use-mobile-session-accessory-selection.ts index baa46de8716..ad0116cb9c6 100644 --- a/mobile/src/session/use-mobile-session-accessory-selection.ts +++ b/mobile/src/session/use-mobile-session-accessory-selection.ts @@ -15,6 +15,7 @@ import type { import type { createTerminalLiveAccessoryInput } from '../terminal/terminal-live-accessory-input' import { clearTerminalLiveInputFocusTimer } from '../terminal/terminal-live-input' import { stripTerminalSelectionGutter } from '../../../src/shared/terminal-selection-gutter' +import { useTerminalCopyTrimsGutter } from '../terminal/terminal-copy-gutter-preference' import { getRepoIdFromMobileWorktreeId } from './mobile-session-route-helpers' import type { RuntimeRepoSummary } from './mobile-session-route-types' import type { MobileSessionTerminalInputModel } from './use-mobile-session-terminal-input' @@ -24,6 +25,7 @@ export function useMobileSessionAccessorySelection(scope: MobileSessionTerminalI worktreeId, isFloatingWorkspaceRoute, client, + connState, setTerminalKeyboardMetrics, setSelectModeActive, setCanPaste, @@ -42,6 +44,7 @@ export function useMobileSessionAccessorySelection(scope: MobileSessionTerminalI handleAccessoryKey, clearSessionTabActionSheetKeyboardListener } = scope + const trimsGutterRef = useTerminalCopyTrimsGutter(client, connState) // Why: hold-to-repeat matches iOS cadence (400ms then 45ms); non-repeatable keys fire once (holding is destructive). const repeatTimeoutRef = useRef | null>(null) const repeatIntervalRef = useRef | null>(null) @@ -116,7 +119,9 @@ export function useMobileSessionAccessorySelection(scope: MobileSessionTerminalI return } try { - await Clipboard.setStringAsync(stripTerminalSelectionGutter(text)) + await Clipboard.setStringAsync( + trimsGutterRef.current ? stripTerminalSelectionGutter(text) : text + ) triggerSuccess() // Why: Android 13+ shows its own system copy toast; iOS shows none, so only iOS needs our in-app toast. if (Platform.OS === 'ios') { diff --git a/mobile/src/terminal/terminal-copy-gutter-preference.ts b/mobile/src/terminal/terminal-copy-gutter-preference.ts new file mode 100644 index 00000000000..b5e80d542d3 --- /dev/null +++ b/mobile/src/terminal/terminal-copy-gutter-preference.ts @@ -0,0 +1,43 @@ +import { useEffect, useRef, type RefObject } from 'react' +import { terminalCopyTrimsGutterRead } from '../transport/settings-read-operations' +import type { RpcClient } from '../transport/rpc-client' +import type { ConnectionState } from '../transport/types' + +/** + * Desktop owns "Trim Gutter on Copy" (GlobalSettings.terminalCopyTrimsGutter); + * mobile mirrors it so turning the setting off yields verbatim screen cells on + * every surface. Read once per connection — the mobile RPC has no + * settings-change stream — and default to on while the read is in flight. + */ +export function useTerminalCopyTrimsGutter( + client: RpcClient | null, + connState: ConnectionState +): RefObject { + const trimsGutterRef = useRef(true) + + useEffect(() => { + if (!client || connState !== 'connected') { + return + } + let stale = false + void terminalCopyTrimsGutterRead + .request(client) + .then((response) => { + if (stale) { + return + } + const preference = terminalCopyTrimsGutterRead.interpret(response) + if (preference.accepted) { + trimsGutterRef.current = preference.value + } + }) + .catch(() => { + // Best-effort: an unreachable host leaves the on-by-default trim in place. + }) + return () => { + stale = true + } + }, [client, connState]) + + return trimsGutterRef +} diff --git a/mobile/src/transport/settings-read-operations.test.ts b/mobile/src/transport/settings-read-operations.test.ts index 3a81da9942f..07f4ff771d4 100644 --- a/mobile/src/transport/settings-read-operations.test.ts +++ b/mobile/src/transport/settings-read-operations.test.ts @@ -8,7 +8,8 @@ import { settingsRead, optionalSettingsRead, botOverridesRead, - newTabSettingsRead + newTabSettingsRead, + terminalCopyTrimsGutterRead } from './settings-read-operations' import type { RpcResponse } from './types' @@ -91,6 +92,24 @@ describe('settings historical acceptance', () => { expect(botOverridesRead.interpret(empty)).toEqual({ accepted: true, value: [] }) }) + it('reads the gutter-trim preference, treating an older host as opted in', async () => { + const off = await terminalCopyTrimsGutterRead.request( + replyWith(success({ settings: { terminalCopyTrimsGutter: false } })) + ) + expect(terminalCopyTrimsGutterRead.interpret(off)).toEqual({ accepted: true, value: false }) + const on = await terminalCopyTrimsGutterRead.request( + replyWith(success({ settings: { terminalCopyTrimsGutter: true } })) + ) + expect(terminalCopyTrimsGutterRead.interpret(on)).toEqual({ accepted: true, value: true }) + // A host predating the setting sends no key; the desktop default is on. + const absent = await terminalCopyTrimsGutterRead.request(replyWith(success({ settings: {} }))) + expect(terminalCopyTrimsGutterRead.interpret(absent)).toEqual({ accepted: true, value: true }) + const empty = await terminalCopyTrimsGutterRead.request(replyWith(success(null))) + expect(terminalCopyTrimsGutterRead.interpret(empty)).toEqual({ accepted: true, value: true }) + const refused = await terminalCopyTrimsGutterRead.request(replyWith(refusal())) + expect(terminalCopyTrimsGutterRead.interpret(refused)).toEqual({ accepted: false }) + }) + it('does not read a stale payload until its caller permits interpretation', async () => { const read = vi.fn(() => ({})) const reply = await settingsRead.request( diff --git a/mobile/src/transport/settings-read-operations.ts b/mobile/src/transport/settings-read-operations.ts index f33471dcaf2..c98c87798b3 100644 --- a/mobile/src/transport/settings-read-operations.ts +++ b/mobile/src/transport/settings-read-operations.ts @@ -82,6 +82,30 @@ export const newTabSettingsRead = bindDeferredRpcOperation( }) ) +const copyTrimsGutterReader: RpcCompatibleReader = (raw) => { + const settings = raw == null ? undefined : settingsMember(raw) + const trims: unknown = + settings == null ? undefined : Reflect.get(Object(settings), 'terminalCopyTrimsGutter') + return { + compatible: true, + variant: 'copy-trims-gutter', + // Why `!== false`: a host predating the setting sends no key, and the + // desktop default is on, so absence must read as on. + value: trims !== false, + salvage: { droppedPaths: [], droppedCount: 0 } + } +} + +export const terminalCopyTrimsGutterRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'settings.terminal-copy-trims-gutter-or-skip', + method: 'settings.get', + acceptance: 'success-result-or-skip', + barrier: 'after-caller-barrier', + read: copyTrimsGutterReader + }) +) + export const botOverridesRead = bindDeferredRpcOperation( defineRpcOperation({ name: 'settings.bot-logins-or-skip', diff --git a/src/renderer/src/components/terminal-pane/terminal-clipboard-selection-text.test.ts b/src/renderer/src/components/terminal-pane/terminal-clipboard-selection-text.test.ts index 4dc712ce3f2..2307831bf59 100644 --- a/src/renderer/src/components/terminal-pane/terminal-clipboard-selection-text.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-clipboard-selection-text.test.ts @@ -47,7 +47,7 @@ describe('copyTerminalSelection gutter handling', () => { expect(writeClipboardText).toHaveBeenCalledWith(UNGUTTERED) }) - it('still reports no selection when the gutter is all there was', async () => { + it('still reports no selection for an empty xterm selection', async () => { const writeClipboardText = vi.fn<(text: string) => Promise>().mockResolvedValue() await expect( copyTerminalSelection({ diff --git a/src/renderer/src/components/terminal-pane/terminal-clipboard-selection-text.ts b/src/renderer/src/components/terminal-pane/terminal-clipboard-selection-text.ts index 953f74f350b..5bc1b2b3c72 100644 --- a/src/renderer/src/components/terminal-pane/terminal-clipboard-selection-text.ts +++ b/src/renderer/src/components/terminal-pane/terminal-clipboard-selection-text.ts @@ -7,10 +7,9 @@ import { stripTerminalSelectionGutter } from '../../../../shared/terminal-select * minus the left gutter the agent CLI painted them behind (#19770). */ export function readTerminalClipboardSelection(terminal: Pick): string { - return applyTerminalClipboardGutterSetting(terminal.getSelection()) -} - -export function applyTerminalClipboardGutterSetting(selection: string): string { + const selection = terminal.getSelection() + // Why `=== false`: profiles saved before the setting existed have no key, and + // they should trim like every new profile does. if (useAppStore.getState().settings?.terminalCopyTrimsGutter === false) { return selection } diff --git a/src/shared/terminal-selection-gutter.test.ts b/src/shared/terminal-selection-gutter.test.ts index 9b64d865e80..8e745f34ff2 100644 --- a/src/shared/terminal-selection-gutter.test.ts +++ b/src/shared/terminal-selection-gutter.test.ts @@ -1,8 +1,5 @@ import { describe, expect, it } from 'vitest' -import { - measureTerminalSelectionGutter, - stripTerminalSelectionGutter -} from './terminal-selection-gutter' +import { stripTerminalSelectionGutter } from './terminal-selection-gutter' // Shape agent CLIs paint: a marker column, then continuation lines behind a // two-space gutter. Selecting the body is what users copy to paste elsewhere. @@ -49,11 +46,15 @@ describe('stripTerminalSelectionGutter', () => { }) it('ignores blank lines when measuring the gutter', () => { - expect(measureTerminalSelectionGutter([' a', '', ' b'].join('\n'))).toBe(2) + expect(stripTerminalSelectionGutter([' a', '', ' b'].join('\n'))).toBe( + ['a', '', 'b'].join('\n') + ) }) it('ignores whitespace-only lines when measuring the gutter', () => { - expect(measureTerminalSelectionGutter([' a', ' ', ' b'].join('\n'))).toBe(4) + expect(stripTerminalSelectionGutter([' a', ' ', ' b'].join('\n'))).toBe( + ['a', '', 'b'].join('\n') + ) }) it('leaves an all-whitespace selection untouched', () => { diff --git a/src/shared/terminal-selection-gutter.ts b/src/shared/terminal-selection-gutter.ts index 36871714006..b6b358b7327 100644 --- a/src/shared/terminal-selection-gutter.ts +++ b/src/shared/terminal-selection-gutter.ts @@ -1,54 +1,49 @@ -// Why: xterm selections are screen cells, not logical text. Agent CLIs paint -// their messages behind a fixed left gutter (Claude Code indents continuation -// lines by two spaces), so every copied line carries that gutter into the -// clipboard and pasted replies come out indented (#19770). +// Why: an xterm selection is a rectangle of screen cells, not logical text. +// Agent CLIs paint their messages behind a fixed left gutter, so every copied +// line carried that gutter into the clipboard and pasted replies came out +// indented (#19770). // -// Only the run of spaces that *every* selected line shares is removed, so -// relative indentation — nested bullets, fenced code, YAML — survives intact. -// A selection that starts mid-line has a non-space first line, which makes the -// shared run zero and turns this into a no-op. +// Only the run of spaces that *every* non-blank line shares is removed, so +// relative indentation — nested bullets, fenced code, YAML — survives. A +// selection that starts mid-line, or that covers any column-0 line, shares a +// run of zero and comes back untouched. +// Terminal cells never hold tabs (the emulator expands them) and xterm folds +// non-breaking spaces into plain ones, so spaces are the whole alphabet here. const LEADING_SPACES = /^ */ -// xterm writes CRLF joins on Windows; keep the terminator it chose. -function splitCarriageReturn(line: string): [string, string] { - return line.endsWith('\r') ? [line.slice(0, -1), '\r'] : [line, ''] +function measureIndent(line: string): number { + return LEADING_SPACES.exec(line)?.[0].length ?? 0 } -/** Width of the space run shared by every non-blank line, or 0 when there is none. */ -export function measureTerminalSelectionGutter(selection: string): number { +// xterm joins rows with CRLF on Windows, so split('\n') leaves the CR behind. +function splitTerminator(rawLine: string): readonly [text: string, terminator: string] { + return rawLine.endsWith('\r') ? [rawLine.slice(0, -1), '\r'] : [rawLine, ''] +} + +function measureGutter(lines: readonly string[]): number { let gutter = Number.POSITIVE_INFINITY - for (const rawLine of selection.split('\n')) { - const [line] = splitCarriageReturn(rawLine) - const indent = LEADING_SPACES.exec(line)?.[0].length ?? 0 - // Blank and whitespace-only lines carry no gutter evidence either way. + for (const line of lines) { + const indent = measureIndent(line) + // Blank and whitespace-only lines are evidence of nothing either way. if (indent === line.length) { continue } - if (indent < gutter) { - gutter = indent - if (gutter === 0) { - return 0 - } + gutter = Math.min(gutter, indent) + if (gutter === 0) { + return 0 } } return Number.isFinite(gutter) ? gutter : 0 } export function stripTerminalSelectionGutter(selection: string): string { - if (!selection) { - return selection - } - const gutter = measureTerminalSelectionGutter(selection) + const lines = selection.split('\n').map(splitTerminator) + const gutter = measureGutter(lines.map(([text]) => text)) if (gutter === 0) { return selection } - return selection - .split('\n') - .map((rawLine) => { - const [line, terminator] = splitCarriageReturn(rawLine) - const indent = LEADING_SPACES.exec(line)?.[0].length ?? 0 - return line.slice(Math.min(indent, gutter)) + terminator - }) + return lines + .map(([text, terminator]) => text.slice(Math.min(measureIndent(text), gutter)) + terminator) .join('\n') }