-
Notifications
You must be signed in to change notification settings - Fork 953
fix(desktop): adopt Ghostty keyboard model in v2 terminal #3700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
07755a8
36cf276
4741cce
cd821a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,10 @@ import type { SearchAddon } from "@xterm/addon-search"; | |
| import { SerializeAddon } from "@xterm/addon-serialize"; | ||
| import { Terminal as XTerm } from "@xterm/xterm"; | ||
| import { resolveHotkeyFromEvent } from "renderer/hotkeys"; | ||
| import { | ||
| shouldBubbleClipboardShortcut, | ||
| shouldSelectAllShortcut, | ||
| } from "renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts"; | ||
| import { DEFAULT_TERMINAL_SCROLLBACK } from "shared/constants"; | ||
| import type { TerminalAppearance } from "./appearance"; | ||
| import { loadAddons } from "./terminal-addons"; | ||
|
|
@@ -14,12 +18,101 @@ const DIMS_KEY_PREFIX = "terminal-dims:"; | |
| const DEFAULT_COLS = 120; | ||
| const DEFAULT_ROWS = 32; | ||
|
|
||
| // xterm's _keyDown calls stopPropagation after processing, which kills the | ||
| // bubble to react-hotkeys-hook. Returning false from the custom handler makes | ||
| // xterm bail before that, so app hotkeys reach document. (VSCode pattern: | ||
| // terminalInstance.ts:1116-1175) | ||
| function isAppHotkey(event: KeyboardEvent): boolean { | ||
| return resolveHotkeyFromEvent(event) !== null; | ||
| // xterm's _keyDown calls stopPropagation after processing, so any chord we | ||
| // want the host (react-hotkeys-hook, Electron menu accelerators) or the shell | ||
| // (Ctrl+A/E/U escape sequences for line edit) to see must short-circuit xterm | ||
| // before it runs. (VSCode pattern: terminalInstance.ts:1116-1175.) | ||
| // | ||
| // Kitty keyboard protocol is enabled, which means every Mac Cmd chord xterm | ||
| // sees gets CSI-u encoded and leaks into TUIs as a literal char. Ghostty | ||
| // sidesteps this by suppressing all super/Cmd chords on macOS before the | ||
| // encoder runs (ghostty/src/input/key_encode.zig:534-545). We do the same via | ||
| // shouldBubbleClipboardShortcut's Mac branch. | ||
| function createKeyEventHandler(terminal: XTerm) { | ||
| const platform = | ||
| typeof navigator !== "undefined" ? navigator.platform.toLowerCase() : ""; | ||
| const isMac = platform.includes("mac"); | ||
| const isWindows = platform.includes("win"); | ||
|
|
||
| return (event: KeyboardEvent): boolean => { | ||
| if (resolveHotkeyFromEvent(event) !== null) return false; | ||
|
|
||
| const translation = translateLineEditChord(event, { isMac, isWindows }); | ||
| if (translation !== null) { | ||
| if (event.type === "keydown") { | ||
| event.preventDefault(); | ||
| terminal.input(translation, true); | ||
| } | ||
| return false; | ||
| } | ||
|
Comment on lines
+40
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a chord (e.g. Cmd+Left) is matched, the handler returns Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts
Line: 40-47
Comment:
**`translateLineEditChord` suppresses `keyup` for matched chords**
When a chord (e.g. Cmd+Left) is matched, the handler returns `false` for every event type, including `keyup`. Only `keydown` sends `terminal.input`, but xterm never sees the `keyup` for that chord. In practice xterm's custom key handler is primarily meaningful for `keydown`, so this is unlikely to cause problems, but a comment clarifying the intent would improve readability.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| if (shouldSelectAllShortcut(event, isMac)) { | ||
| if (event.type === "keydown") { | ||
| event.preventDefault(); | ||
| terminal.selectAll(); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| if ( | ||
| shouldBubbleClipboardShortcut(event, { | ||
| isMac, | ||
| isWindows, | ||
| hasSelection: terminal.hasSelection(), | ||
| }) | ||
| ) { | ||
| // Do NOT preventDefault — the browser's keydown → paste-command pipeline | ||
| // is what fires the `paste` event on xterm's textarea. VS Code and Tabby | ||
| // preventDefault here only because they implement paste themselves via | ||
| // the command system / ClipboardAddon; we rely on xterm's built-in paste | ||
| // listener, so the default must run. | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| }; | ||
| } | ||
|
|
||
| /** True when `mod` is the only non-shift modifier held. */ | ||
| function onlyMod(event: KeyboardEvent, mod: "meta" | "alt" | "ctrl"): boolean { | ||
| return ( | ||
| event.metaKey === (mod === "meta") && | ||
| event.altKey === (mod === "alt") && | ||
| event.ctrlKey === (mod === "ctrl") && | ||
| !event.shiftKey | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Translate Mac Cmd+/Option+ and Windows Ctrl+ arrow / backspace chords into | ||
| * the escape sequences shells expect. Returns the bytes to send, or null if | ||
| * this chord isn't a line-edit translation. | ||
| * | ||
| * Mirrors v1 helpers.ts:319-427. These translations only exist because xterm's | ||
| * default encoding (with kitty on) would send a CSI-u sequence that most | ||
| * shells don't map to line-edit commands. | ||
| */ | ||
| function translateLineEditChord( | ||
| event: KeyboardEvent, | ||
| options: { isMac: boolean; isWindows: boolean }, | ||
| ): string | null { | ||
| const { isMac, isWindows } = options; | ||
| const { key } = event; | ||
|
|
||
| if (isMac && onlyMod(event, "meta")) { | ||
| if (key === "Backspace") return "\x15\x1b[D"; | ||
| if (key === "ArrowLeft") return "\x01"; | ||
| if (key === "ArrowRight") return "\x05"; | ||
| } | ||
| if (isMac && onlyMod(event, "alt")) { | ||
| if (key === "ArrowLeft") return "\x1bb"; | ||
| if (key === "ArrowRight") return "\x1bf"; | ||
| } | ||
| if (isWindows && onlyMod(event, "ctrl")) { | ||
| if (key === "ArrowLeft") return "\x1bb"; | ||
| if (key === "ArrowRight") return "\x1bf"; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| export interface TerminalRuntime { | ||
|
|
@@ -178,7 +271,7 @@ export function createRuntime( | |
| wrapper.style.height = "100%"; | ||
| terminal.open(wrapper); | ||
|
|
||
| terminal.attachCustomKeyEventHandler((event) => !isAppHotkey(event)); | ||
| terminal.attachCustomKeyEventHandler(createKeyEventHandler(terminal)); | ||
|
|
||
| // Activate Unicode 11 widths (inside loadAddons) before restoring the buffer, | ||
| // else CJK/emoji/ZWJ widths get baked wrong into the replay. (#3572) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
navigator.platformis deprecatednavigator.platformwas deprecated in the Living Standard and may be removed in future browser/Electron versions. The recommended replacement isnavigator.userAgentData?.platform(available in Chromium-based Electron builds), with a graceful fallback.Prompt To Fix With AI