Skip to content

Commit 2357946

Browse files
committed
fix(terminal): copy selected web terminal text
1 parent 7aaebec commit 2357946

4 files changed

Lines changed: 115 additions & 0 deletions

File tree

61.9 KB
Loading
41.3 KB
Loading

packages/terminal/src/web/terminal-copy-interaction.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,18 @@ type TerminalSelectionTarget = {
1717
readonly hasSelection: () => boolean
1818
}
1919

20+
export type TerminalCopyKeyboardEvent = {
21+
readonly altKey: boolean
22+
readonly ctrlKey: boolean
23+
readonly key: string
24+
readonly metaKey: boolean
25+
readonly type: string
26+
}
27+
2028
export type TerminalCopyInteractionTerminal = TerminalSelectionTarget & {
29+
readonly attachCustomKeyEventHandler?: (
30+
handler: (event: TerminalCopyKeyboardEvent) => boolean
31+
) => void
2132
readonly modes: {
2233
readonly mouseTrackingMode: TerminalMouseTrackingMode
2334
}
@@ -60,6 +71,41 @@ const isSecondaryMouseButton = (event: TerminalMouseButtonEvent): boolean => eve
6071
const hasActiveMouseTracking = (terminal: TerminalCopyInteractionTerminal): boolean =>
6172
terminal.modes.mouseTrackingMode !== "none"
6273

74+
const isKeyboardCopyShortcut = (event: TerminalCopyKeyboardEvent): boolean =>
75+
event.type === "keydown" &&
76+
!event.altKey &&
77+
(event.ctrlKey || event.metaKey) &&
78+
event.key.toLowerCase() === "c"
79+
80+
/**
81+
* Decides whether xterm key processing must step aside for native browser copy.
82+
*
83+
* @param event - Keyboard event seen by xterm before it translates keys into pty input.
84+
* @param terminal - Terminal selection facade.
85+
* @returns True iff the event is a system copy shortcut and selected terminal text is non-empty.
86+
* @pure true
87+
* @effect terminal.hasSelection(), terminal.getSelection().
88+
* @invariant result => no ETX input is sent for selected terminal text copy.
89+
* @precondition `event` and `terminal` are non-null.
90+
* @postcondition True means xterm should return false from its custom key handler.
91+
* @complexity O(n) where n = selected text length.
92+
* @throws Never
93+
*/
94+
// CHANGE: keep keyboard copy shortcuts out of terminal input when text is selected
95+
// WHY: Ctrl/Cmd+C must copy the selected terminal text instead of sending SIGINT to the pty
96+
// QUOTE(ТЗ): "Text easy coping"
97+
// REF: issue-353
98+
// SOURCE: n/a
99+
// FORMAT THEOREM: selected(t) and copyShortcut(e) => browserCopy(e,t)
100+
// PURITY: CORE
101+
// EFFECT: reads terminal selection through the injected terminal facade
102+
// INVARIANT: empty selection never blocks terminal Ctrl+C semantics
103+
// COMPLEXITY: O(n)/O(1)
104+
export const shouldLetBrowserHandleTerminalCopyShortcut = (
105+
event: TerminalCopyKeyboardEvent,
106+
terminal: TerminalSelectionTarget
107+
): boolean => isKeyboardCopyShortcut(event) && terminal.hasSelection() && terminal.getSelection().length > 0
108+
63109
export const shouldForceBrowserTerminalSelection = (
64110
event: TerminalMouseButtonEvent,
65111
terminal: TerminalCopyInteractionTerminal
@@ -158,13 +204,17 @@ class TerminalCopyInteractionController {
158204
}
159205

160206
readonly attach = (): { readonly dispose: () => void } => {
207+
this.args.terminal.attachCustomKeyEventHandler?.(this.onTerminalKeyEvent)
161208
this.args.host.addEventListener("mousedown", this.onMouseDown, true)
162209
this.args.host.addEventListener("mouseup", this.onMouseUp, true)
163210
this.args.host.addEventListener("contextmenu", this.onContextMenu, true)
164211
this.args.host.addEventListener("copy", this.onCopy, true)
165212
return { dispose: this.dispose }
166213
}
167214

215+
private readonly onTerminalKeyEvent = (event: TerminalCopyKeyboardEvent): boolean =>
216+
!shouldLetBrowserHandleTerminalCopyShortcut(event, this.args.terminal)
217+
168218
private readonly shouldProtectSelectionContext = (event: TerminalCopyMouseEvent): boolean =>
169219
isSecondaryMouseButton(event) &&
170220
hasActiveMouseTracking(this.args.terminal) &&

packages/terminal/tests/web/terminal-copy-interaction.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import {
55
forceTerminalSelectionModifier,
66
shouldForceBrowserTerminalSelection,
77
shouldForceTerminalSelectionContext,
8+
shouldLetBrowserHandleTerminalCopyShortcut,
89
type TerminalCopyInteractionTerminal,
10+
type TerminalCopyKeyboardEvent,
911
type TerminalMouseTrackingMode,
1012
writeTerminalSelectionToClipboardData
1113
} from "../../src/web/terminal-copy-interaction.js"
@@ -27,6 +29,26 @@ const terminalWithSelection = (
2729
modes: { mouseTrackingMode }
2830
})
2931

32+
const keyboardEvent = (
33+
key: string,
34+
options: Partial<Pick<TerminalCopyKeyboardEvent, "altKey" | "ctrlKey" | "metaKey" | "type">> = {}
35+
): TerminalCopyKeyboardEvent => ({
36+
altKey: options.altKey ?? false,
37+
ctrlKey: options.ctrlKey ?? false,
38+
key,
39+
metaKey: options.metaKey ?? false,
40+
type: options.type ?? "keydown"
41+
})
42+
43+
const requireKeyHandler = (
44+
keyHandler: ((event: TerminalCopyKeyboardEvent) => boolean) | null
45+
): (event: TerminalCopyKeyboardEvent) => boolean => {
46+
if (keyHandler === null) {
47+
return expect.fail("Expected terminal copy key handler to be registered.")
48+
}
49+
return keyHandler
50+
}
51+
3052
describe("terminal copy interaction", () => {
3153
it("forces browser selection for primary mouse input while terminal mouse tracking is active", () => {
3254
expect(shouldForceBrowserTerminalSelection({ button: 0 }, terminalWithSelection("any", ""))).toBe(true)
@@ -42,6 +64,49 @@ describe("terminal copy interaction", () => {
4264
expect(shouldForceTerminalSelectionContext({ button: 0 }, terminalWithSelection("any", "selected"))).toBe(false)
4365
})
4466

67+
it("lets browser copy handle Ctrl/Cmd+C only for selected terminal text", () => {
68+
const selectedTerminal = terminalWithSelection("any", "selected")
69+
const emptyTerminal = terminalWithSelection("any", "")
70+
71+
expect(shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("c", { ctrlKey: true }), selectedTerminal))
72+
.toBe(true)
73+
expect(shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("C", { metaKey: true }), selectedTerminal))
74+
.toBe(true)
75+
expect(shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("c", { ctrlKey: true }), emptyTerminal))
76+
.toBe(false)
77+
expect(
78+
shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("c", { altKey: true, ctrlKey: true }), selectedTerminal)
79+
)
80+
.toBe(false)
81+
expect(shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("v", { ctrlKey: true }), selectedTerminal))
82+
.toBe(false)
83+
expect(
84+
shouldLetBrowserHandleTerminalCopyShortcut(keyboardEvent("c", { ctrlKey: true, type: "keyup" }), selectedTerminal)
85+
).toBe(false)
86+
})
87+
88+
it("registers a custom key handler that bypasses xterm input for selected text copy", () => {
89+
let keyHandler: ((event: TerminalCopyKeyboardEvent) => boolean) | null = null
90+
const terminal: TerminalCopyInteractionTerminal = {
91+
attachCustomKeyEventHandler: (handler) => {
92+
keyHandler = handler
93+
},
94+
getSelection: () => "selected",
95+
hasSelection: () => true,
96+
modes: { mouseTrackingMode: "none" }
97+
}
98+
const host = new FakeTerminalCopyHost(null)
99+
100+
const disposable = attachTerminalCopyInteraction({ host, terminal })
101+
const handleKey = requireKeyHandler(keyHandler)
102+
103+
expect(handleKey(keyboardEvent("c", { ctrlKey: true }))).toBe(false)
104+
expect(handleKey(keyboardEvent("c", { ctrlKey: true, type: "keyup" }))).toBe(true)
105+
expect(handleKey(keyboardEvent("c"))).toBe(true)
106+
107+
disposable.dispose()
108+
})
109+
45110
it("uses Shift as the forced selection modifier on non-Mac platforms", () => {
46111
const event = { altKey: false, shiftKey: false }
47112

0 commit comments

Comments
 (0)