diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-kitty-commit-encoding.ts b/src/renderer/src/components/terminal-pane/terminal-ime-kitty-commit-encoding.ts new file mode 100644 index 000000000..c47b77f53 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-ime-kitty-commit-encoding.ts @@ -0,0 +1,76 @@ +// Reuses xterm's own kitty encoder rather than hand-rolling CSI-u. It lives in +// the package's `src/` tree and is absent from the public typings, so this is a +// deep import into a pinned dependency — acceptable here because the version is +// already pinned by a patch that would fail to apply across a bump. +import { KittyKeyboard } from '@xterm/xterm/src/common/input/KittyKeyboard' + +/** + * `report_all_keys_as_escape_codes`. Bit 3 is the only flag that changes what a + * plain printable key should put on the wire; 1/2/4/16 leave it as text. + */ +const KITTY_REPORT_ALL_KEYS_AS_ESCAPE_CODES = 0b1000 + +/** + * `KittyKeyboardEventType.PRESS` / `.REPEAT`. Inlined because the upstream enum is a + * `const enum`, which does not survive an import across module boundaries. + */ +const KITTY_EVENT_TYPE_PRESS = 1 +const KITTY_EVENT_TYPE_REPEAT = 2 + +const kittyKeyboardEncoder = new KittyKeyboard() + +/** The physical keydown that produced a commit, captured before the input event. */ +export type ImeCommitKeyPress = { + key: string + code?: string + shiftKey: boolean + /** An auto-repeat keydown; the protocol distinguishes it from a fresh press. */ + repeat?: boolean +} + +/** + * A pane that negotiated bit 3 asked for every printable key as a CSI-u report, + * so writing IME-committed text raw hands it the legacy byte stream it declined. + * Re-encode the press that produced the commit instead. + * + * The gate is bit 3 ALONE. "Kitty is active" and `flags !== 0` are both wrong: + * a pane negotiating only disambiguation or event types still expects printable + * keys as text, and encoding there would drop every substituted character. + * + * Returns null when the commit should be written raw, which is every case except + * bit 3. + * + * Known limit: the report carries the *physical* key's codepoint, not the + * committed glyph — bit 3 is the app declaring it does not want text, and bit 4 + * (`report_associated_text`) is how it asks for text back. xterm's encoder + * derives that text field from the same `key` it derives the keycode from, so + * carrying the committed glyph under bit 4 needs an encoder change, not a flag. + */ +export function encodeImeCommitAsKittyReport( + press: ImeCommitKeyPress | null, + kittyKeyboardFlags: number +): string | null { + if ((kittyKeyboardFlags & KITTY_REPORT_ALL_KEYS_AS_ESCAPE_CODES) === 0 || !press) { + return null + } + // Why: the forwarder only claims presses with no control chord, so the + // modifier fields are known-false rather than read from a live event. + const encoded = kittyKeyboardEncoder.evaluate( + { + type: 'keydown', + key: press.key, + code: press.code ?? '', + keyCode: 0, + shiftKey: press.shiftKey, + altKey: false, + ctrlKey: false, + metaKey: false + }, + kittyKeyboardFlags, + // Why: a held key emits repeated keydowns, and the protocol reports those as REPEAT. + // Defaulting them all to PRESS would make one held key look like N separate strikes to + // an app that counts presses or filters repeats. + press.repeat === true ? KITTY_EVENT_TYPE_REPEAT : KITTY_EVENT_TYPE_PRESS + ) + return encoded.key ?? null +} diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-native-text-forwarder.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-native-text-forwarder.test.ts index 11f9261f4..2894df2e2 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-native-text-forwarder.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-native-text-forwarder.test.ts @@ -326,6 +326,90 @@ describe('installTerminalImeNativeTextForwarder', () => { }) }) + describe('the kitty read is scoped to the commit', () => { + function installWithFlags( + getKittyKeyboardFlags: () => number, + isComposing: () => boolean = () => false + ): { + forwarder: ReturnType + sendInput: ReturnType + } { + const sendInput = vi.fn() + const forwarder = installTerminalImeNativeTextForwarder({ + terminalElement: element, + isComposing, + sendInput, + getKittyKeyboardFlags + }) + return { forwarder, sendInput } + } + + it('never reads the flags on a keydown, only on the commit', () => { + const getKittyKeyboardFlags = vi.fn(() => 8) + const { forwarder } = installWithFlags(getKittyKeyboardFlags) + + forwarder.claimKeyEvent(keyEvent({ key: ',' })) + forwarder.claimKeyEvent(keyEvent({ key: ',', type: 'keypress' })) + expect(getKittyKeyboardFlags).not.toHaveBeenCalled() + + dispatchInsertText(textarea, '，') + expect(getKittyKeyboardFlags).toHaveBeenCalledOnce() + }) + + // A held key emits repeated keydowns. The protocol reports those as REPEAT (event type 2); + // encoding them all as PRESS would make one held key read as N separate strikes to an app + // that counts presses or filters repeats. + // Flags 8|2: the event type only appears on the wire when report_event_types is also + // negotiated, which is exactly the pane that can tell a repeat from a press. + it('encodes an auto-repeat commit as REPEAT, not as another PRESS', () => { + const { forwarder, sendInput } = installWithFlags(() => 0b1010) + + expect(forwarder.claimKeyEvent(keyEvent({ key: 'a', code: 'KeyA' }))).toBe(true) + dispatchInsertText(textarea, 'a') + const firstPress = sendInput.mock.calls[0][0] + + expect(forwarder.claimKeyEvent(keyEvent({ key: 'a', code: 'KeyA', repeat: true }))).toBe(true) + dispatchInsertText(textarea, 'a') + const repeated = sendInput.mock.calls[1][0] + + expect(firstPress).toBe('') + expect(repeated).toBe('[97;1:2u') + }) + + it('claims the keydown under bit 3 exactly as it does without it', () => { + // The predicate stays structural: the protocol changes what the commit + // writes, never whether the keystroke is owned. + const { forwarder } = installWithFlags(() => 8) + expect(forwarder.claimKeyEvent(keyEvent({ key: ',' }))).toBe(true) + }) + + it('leaves a composing keystroke to the composition path even under bit 3', () => { + // Scope boundary: a composing IME (Hangul, kana) is never claimed here, so + // its commit is not this path's to re-encode. Bit 3 fidelity for + // composition commits would be a change to the composition path. + const { forwarder, sendInput } = installWithFlags( + () => 8, + () => true + ) + expect(forwarder.claimKeyEvent(keyEvent({ key: 'r' }))).toBe(false) + dispatchInsertText(textarea, '한') + expect(sendInput).not.toHaveBeenCalled() + }) + + it('writes the commit raw when the caller tracks no flags at all', () => { + // The preview bridge installs the forwarder with no pane to negotiate with. + const sendInput = vi.fn() + const forwarder = installTerminalImeNativeTextForwarder({ + terminalElement: element, + isComposing: () => false, + sendInput + }) + forwarder.claimKeyEvent(keyEvent({ key: ',' })) + dispatchInsertText(textarea, '，') + expect(sendInput).toHaveBeenCalledExactlyOnceWith('，') + }) + }) + it('stops forwarding after dispose', () => { const { forwarder, sendInput } = install() forwarder.claimKeyEvent(keyEvent({ key: ',' })) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-native-text-forwarder.ts b/src/renderer/src/components/terminal-pane/terminal-ime-native-text-forwarder.ts index 4d268f60a..c200e3c04 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-native-text-forwarder.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-native-text-forwarder.ts @@ -1,4 +1,5 @@ import type { IDisposable } from '@xterm/xterm' +import { encodeImeCommitAsKittyReport } from './terminal-ime-kitty-commit-encoding' // Why: a plain printable keydown never produces terminal bytes. Bytes for // printable characters come only from the `input` event, which on macOS *is* @@ -13,6 +14,8 @@ import type { IDisposable } from '@xterm/xterm' type ClaimedKeyPress = { key: string code?: string + shiftKey: boolean + repeat?: boolean } export type ImeNativeTextKeyEvent = { @@ -22,6 +25,8 @@ export type ImeNativeTextKeyEvent = { metaKey: boolean ctrlKey: boolean altKey: boolean + shiftKey?: boolean + repeat?: boolean isComposing?: boolean } @@ -82,6 +87,12 @@ export function installTerminalImeNativeTextForwarder(args: { terminalElement: HTMLElement | null | undefined isComposing: () => boolean sendInput: (data: string) => void + /** + * The pane's negotiated kitty flags. Read once per commit, never on the + * keydown — `claimKeyEvent` stays structural and protocol-blind so the hot + * path keeps no kitty state. Absent means no pane to negotiate with. + */ + getKittyKeyboardFlags?: () => number }): TerminalImeNativeTextForwarder { if (!args.terminalElement) { return { @@ -114,7 +125,12 @@ export function installTerminalImeNativeTextForwarder(args: { // never arrived (the input source swallowed the key) — no timer needed. pendingForward = true forwardedPressBytes = false - claimedPress = { key: event.key, code: event.code } + claimedPress = { + key: event.key, + code: event.code, + shiftKey: event.shiftKey === true, + repeat: event.repeat === true + } return true } if (!claimedPress) { @@ -161,7 +177,11 @@ export function installTerminalImeNativeTextForwarder(args: { return } if (event.data) { - args.sendInput(event.data) + const kittyReport = encodeImeCommitAsKittyReport( + claimedPress, + args.getKittyKeyboardFlags?.() ?? 0 + ) + args.sendInput(kittyReport ?? event.data) forwardedPressBytes = true } event.stopImmediatePropagation() diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-substituted-text-commit.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-substituted-text-commit.test.ts index 9dfa43075..267c7a610 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-substituted-text-commit.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-substituted-text-commit.test.ts @@ -25,7 +25,8 @@ function open(kittyKeyboardFlags = 0) { const forwarder = installTerminalImeNativeTextForwarder({ terminalElement: terminal.element, isComposing: () => false, - sendInput: (data) => terminal.input(data) + sendInput: (data) => terminal.input(data), + getKittyKeyboardFlags: () => kittyKeyboardFlags }) terminal.attachCustomKeyEventHandler((event) => { if (forwarder.claimKeyEvent(event)) { @@ -188,12 +189,59 @@ describe('input-source text substitution reaches the terminal', () => { expect(type([COMMA], 1)).toBe('，') }) - // Pins a deliberate hole rather than a desired behaviour. Flag 8 asks for every printable key as - // an escape code, and this path sends the committed text raw instead — a mature native terminal - // makes the same trade, preferring correct characters to protocol fidelity. Recorded here so the - // choice is visible: if this ever needs closing, gate on flag 8 alone, never on "kitty active", - // which would disable the substitution for every pane that negotiates anything. - it('sends the substitution raw even when kitty asks for all keys as escape codes', () => { - expect(type([COMMA], 8)).toBe('，') + // The gate is bit 3 alone. Every other flag leaves printable keys as text, so the substituted + // character must still reach the pane; gating on "kitty active" instead would strip the + // substitution from every pane that negotiates anything at all. + describe.each([ + ['none', 0], + ['disambiguate', 1], + ['event types', 2], + ['alternate keys', 4], + ['disambiguate + alternate keys', 5], + ['disambiguate + event types + alternate keys', 7], + // Bit 4 asks for associated text, which only decorates a report bit 3 would already have + // produced — on its own it does not turn a printable key into one. + ['associated text', 16] + ])('with kitty flags %s (%d) negotiated', (_name, flags) => { + it('sends the substituted character raw', () => { + expect(type([COMMA], flags)).toBe('，') + }) + }) + + // Bit 3 is `report_all_keys_as_escape_codes`: the app asked for every printable key as a CSI-u + // report, so committing raw UTF-8 hands it a byte stream it declined. Re-encode the press that + // produced the commit. This is not CJK-specific — it is every printable key in such a pane. + describe.each([ + ['all keys as escape codes', 8], + ['all keys + disambiguate', 9], + ['all keys + disambiguate + event types + alternate keys', 15] + ])('with kitty flags %s (%d) negotiated', (_name, flags) => { + it('sends a CSI-u report for the physical key instead of the substituted character', () => { + // `,` is the physical Comma key; U+002C is 44. The committed `，` is deliberately absent — + // see terminal-ime-kitty-commit-encoding.ts on why bit 3 without an encoder change cannot + // carry it. + expect(type([COMMA], flags)).toBe('\x1b[44u') + }) + }) + + it('reports the physical key as the associated text under bit 3 + bit 4, not the substitution', () => { + // Pins the limit named in terminal-ime-kitty-commit-encoding.ts: bit 4 is where the committed + // glyph U+FF0C (65292) would ride, and 44 shows up in that slot instead. Closing that needs the + // encoder to take the text separately from the key, so it is not reachable by widening a gate. + expect(type([COMMA], 24)).toBe('\x1b[44;;44u') + }) + + it('encodes a shifted substitution as a CSI-u report with the shift modifier', () => { + // Shift is the one modifier the claim keeps eligible, so it has to survive the encoding. + // 63 is `?`, not 47 for the unshifted `/`: xterm's encoder only unwinds a shifted key to its + // base through `Digit*`/`Key*` codes, and this press carries `Slash`. Pinned as-is — that is + // the shared encoder's behaviour for shifted punctuation on every path, not something the + // commit path introduces. + expect(type([QUESTION], 8)).toBe('\x1b[63;2u') + }) + + it('encodes a multi-character substitution as one report, not one per character', () => { + // One press produced `——`; bit 3 reports keys, and this was a single key. + expect(type([EM_DASH], 8)).toBe('\x1b[95;2u') }) }) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 759e7ad86..631287b72 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -920,7 +920,9 @@ export function useTerminalPaneLifecycle({ ? installTerminalImeNativeTextForwarder({ terminalElement: pane.terminal.element, isComposing: () => imeCompositionTracker.isActive(), - sendInput: (data) => pane.terminal.input(data) + sendInput: (data) => pane.terminal.input(data), + getKittyKeyboardFlags: () => + paneKittyKeyboardModesRef.current.get(pane.id)?.flags ?? 0 }) : { claimKeyEvent: () => false,