diff --git a/docs/pr-screenshots/issue-353/terminal-copy-pass.png b/docs/pr-screenshots/issue-353/terminal-copy-pass.png new file mode 100644 index 00000000..27180c45 Binary files /dev/null and b/docs/pr-screenshots/issue-353/terminal-copy-pass.png differ diff --git a/docs/pr-screenshots/issue-353/terminal-copy-selected.png b/docs/pr-screenshots/issue-353/terminal-copy-selected.png new file mode 100644 index 00000000..dffe6c5c Binary files /dev/null and b/docs/pr-screenshots/issue-353/terminal-copy-selected.png differ diff --git a/packages/terminal/src/web/terminal-copy-interaction.ts b/packages/terminal/src/web/terminal-copy-interaction.ts index ce69d0af..71e5629f 100644 --- a/packages/terminal/src/web/terminal-copy-interaction.ts +++ b/packages/terminal/src/web/terminal-copy-interaction.ts @@ -17,7 +17,18 @@ type TerminalSelectionTarget = { readonly hasSelection: () => boolean } +export type TerminalCopyKeyboardEvent = { + readonly altKey: boolean + readonly ctrlKey: boolean + readonly key: string + readonly metaKey: boolean + readonly type: string +} + export type TerminalCopyInteractionTerminal = TerminalSelectionTarget & { + readonly attachCustomKeyEventHandler?: ( + handler: (event: TerminalCopyKeyboardEvent) => boolean + ) => void readonly modes: { readonly mouseTrackingMode: TerminalMouseTrackingMode } @@ -60,6 +71,41 @@ const isSecondaryMouseButton = (event: TerminalMouseButtonEvent): boolean => eve const hasActiveMouseTracking = (terminal: TerminalCopyInteractionTerminal): boolean => terminal.modes.mouseTrackingMode !== "none" +const isKeyboardCopyShortcut = (event: TerminalCopyKeyboardEvent): boolean => + event.type === "keydown" && + !event.altKey && + (event.ctrlKey || event.metaKey) && + event.key.toLowerCase() === "c" + +/** + * Decides whether xterm key processing must step aside for native browser copy. + * + * @param event - Keyboard event seen by xterm before it translates keys into pty input. + * @param terminal - Terminal selection facade. + * @returns True iff the event is a system copy shortcut and selected terminal text is non-empty. + * @pure true + * @effect terminal.hasSelection(), terminal.getSelection(). + * @invariant result => no ETX input is sent for selected terminal text copy. + * @precondition `event` and `terminal` are non-null. + * @postcondition True means xterm should return false from its custom key handler. + * @complexity O(n) where n = selected text length. + * @throws Never + */ +// CHANGE: keep keyboard copy shortcuts out of terminal input when text is selected +// WHY: Ctrl/Cmd+C must copy the selected terminal text instead of sending SIGINT to the pty +// QUOTE(ТЗ): "Text easy coping" +// REF: issue-353 +// SOURCE: n/a +// FORMAT THEOREM: selected(t) and copyShortcut(e) => browserCopy(e,t) +// PURITY: CORE +// EFFECT: reads terminal selection through the injected terminal facade +// INVARIANT: empty selection never blocks terminal Ctrl+C semantics +// COMPLEXITY: O(n)/O(1) +export const shouldLetBrowserHandleTerminalCopyShortcut = ( + event: TerminalCopyKeyboardEvent, + terminal: TerminalSelectionTarget +): boolean => isKeyboardCopyShortcut(event) && terminal.hasSelection() && terminal.getSelection().length > 0 + export const shouldForceBrowserTerminalSelection = ( event: TerminalMouseButtonEvent, terminal: TerminalCopyInteractionTerminal @@ -158,6 +204,7 @@ class TerminalCopyInteractionController { } readonly attach = (): { readonly dispose: () => void } => { + this.args.terminal.attachCustomKeyEventHandler?.(this.onTerminalKeyEvent) this.args.host.addEventListener("mousedown", this.onMouseDown, true) this.args.host.addEventListener("mouseup", this.onMouseUp, true) this.args.host.addEventListener("contextmenu", this.onContextMenu, true) @@ -165,6 +212,9 @@ class TerminalCopyInteractionController { return { dispose: this.dispose } } + private readonly onTerminalKeyEvent = (event: TerminalCopyKeyboardEvent): boolean => + !shouldLetBrowserHandleTerminalCopyShortcut(event, this.args.terminal) + private readonly shouldProtectSelectionContext = (event: TerminalCopyMouseEvent): boolean => isSecondaryMouseButton(event) && hasActiveMouseTracking(this.args.terminal) && diff --git a/packages/terminal/tests/web/terminal-copy-interaction.test.ts b/packages/terminal/tests/web/terminal-copy-interaction.test.ts index 669d9f82..ed5ca787 100644 --- a/packages/terminal/tests/web/terminal-copy-interaction.test.ts +++ b/packages/terminal/tests/web/terminal-copy-interaction.test.ts @@ -5,7 +5,9 @@ import { forceTerminalSelectionModifier, shouldForceBrowserTerminalSelection, shouldForceTerminalSelectionContext, + shouldLetBrowserHandleTerminalCopyShortcut, type TerminalCopyInteractionTerminal, + type TerminalCopyKeyboardEvent, type TerminalMouseTrackingMode, writeTerminalSelectionToClipboardData } from "../../src/web/terminal-copy-interaction.js" @@ -27,6 +29,26 @@ const terminalWithSelection = ( modes: { mouseTrackingMode } }) +const keyboardEvent = ( + key: string, + options: Partial> = {} +): TerminalCopyKeyboardEvent => ({ + altKey: options.altKey ?? false, + ctrlKey: options.ctrlKey ?? false, + key, + metaKey: options.metaKey ?? false, + type: options.type ?? "keydown" +}) + +const requireKeyHandler = ( + keyHandler: ((event: TerminalCopyKeyboardEvent) => boolean) | null +): (event: TerminalCopyKeyboardEvent) => boolean => { + if (keyHandler === null) { + return expect.fail("Expected terminal copy key handler to be registered.") + } + return keyHandler +} + describe("terminal copy interaction", () => { it("forces browser selection for primary mouse input while terminal mouse tracking is active", () => { expect(shouldForceBrowserTerminalSelection({ button: 0 }, terminalWithSelection("any", ""))).toBe(true) @@ -42,6 +64,49 @@ describe("terminal copy interaction", () => { expect(shouldForceTerminalSelectionContext({ button: 0 }, terminalWithSelection("any", "selected"))).toBe(false) }) + it("lets browser copy handle Ctrl/Cmd+C only for selected terminal text", () => { + const selectedTerminal = terminalWithSelection("any", "selected") + const emptyTerminal = terminalWithSelection("any", "") + + expect(shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("c", { ctrlKey: true }), selectedTerminal)) + .toBe(true) + expect(shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("C", { metaKey: true }), selectedTerminal)) + .toBe(true) + expect(shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("c", { ctrlKey: true }), emptyTerminal)) + .toBe(false) + expect( + shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("c", { altKey: true, ctrlKey: true }), selectedTerminal) + ) + .toBe(false) + expect(shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("v", { ctrlKey: true }), selectedTerminal)) + .toBe(false) + expect( + shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("c", { ctrlKey: true, type: "keyup" }), selectedTerminal) + ).toBe(false) + }) + + it("registers a custom key handler that bypasses xterm input for selected text copy", () => { + let keyHandler: ((event: TerminalCopyKeyboardEvent) => boolean) | null = null + const terminal: TerminalCopyInteractionTerminal = { + attachCustomKeyEventHandler: (handler) => { + keyHandler = handler + }, + getSelection: () => "selected", + hasSelection: () => true, + modes: { mouseTrackingMode: "none" } + } + const host = new FakeTerminalCopyHost(null) + + const disposable = attachTerminalCopyInteraction({ host, terminal }) + const handleKey = requireKeyHandler(keyHandler) + + expect(handleKey(keyboardEvent("c", { ctrlKey: true }))).toBe(false) + expect(handleKey(keyboardEvent("c", { ctrlKey: true, type: "keyup" }))).toBe(true) + expect(handleKey(keyboardEvent("c"))).toBe(true) + + disposable.dispose() + }) + it("uses Shift as the forced selection modifier on non-Mac platforms", () => { const event = { altKey: false, shiftKey: false }