fix(terminal): forward Ctrl+<letter> to PTY under non-Latin IME#4917
fix(terminal): forward Ctrl+<letter> to PTY under non-Latin IME#4917wnduqrla wants to merge 1 commit into
Conversation
Korean (and other non-Latin) IME maps physical letter keys to non-ASCII glyphs: pressing O while Korean 2-set is active yields event.key="ㅐ" instead of "o". xterm derives the Ctrl+<letter> control byte from event.key, so it silently drops every Ctrl+<letter> chord typed while such an IME is active — breaking TUI app shortcuts like Claude Code's Ctrl+O (toggle transcript) and Ctrl+K (clear input). The fix intercepts these chords in createTerminalKeyEventHandler before xterm runs: when ctrlKey is set, no other modifier is held, event.code is a letter key, and event.key is non-ASCII (charCode > 127), compute the control byte from the physical key position (event.code) and write it directly to the PTY via terminal.input(). event.code is always layout- and IME-agnostic, so the correct byte is sent regardless of which input source is active. Relates to superset-sh#3365, extends the event.code approach from superset-sh#3391 to the xterm key-encoder path.
|
Ready to review this PR? Stage has broken it down into 2 individual chapters for you:
Chapters generated by Stage for commit 40238a1 on May 25, 2026 8:59am UTC. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a workaround for Ctrl+ key combinations when using non-Latin IMEs like Korean. The handler detects non-ASCII characters in ChangesNon-Latin IME Ctrl+ Fix
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Greptile SummaryThis PR fixes Ctrl+letter shortcuts in xterm being silently dropped when a non-Latin IME (Korean, Japanese, Chinese) is active, because xterm derives the control byte from
Confidence Score: 4/5Safe to merge; the intercept is tightly scoped and does not affect any existing Latin-IME or multi-modifier path. The fix is logically correct and well-placed in the handler chain. Two minor rough edges exist: wasUserInput is passed as false while the adjacent translateLineEditChord path uses true (risks missing viewport scroll-to-bottom on Ctrl+letter with Korean IME), and there is no isComposing guard, which could cause the intercept to fire during an active composition sequence that xterm itself would have suppressed. apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts — the new IME intercept block has both issues.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts | Adds IME intercept block for bare Ctrl+letter when event.key is non-ASCII; correct placement in handler chain but passes wasUserInput=false inconsistently with adjacent translateLineEditChord path and lacks an isComposing guard |
| apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.test.ts | Four new unit tests cover Korean IME Ctrl+O and Ctrl+K, Latin pass-through, and Shift modifier guard; missing coverage for isComposing edge case |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[KeyboardEvent] --> B{resolveHotkeyFromEvent?}
B -- yes --> C[return false: host handles hotkey]
B -- no --> D{translateLineEditChord?}
D -- yes --> E[terminal.input translation true, return false]
D -- no --> F{shouldSelectAllShortcut?}
F -- yes --> G[terminal.selectAll, return false]
F -- no --> H{shouldBubbleClipboardShortcut?}
H -- yes --> I[return false: browser paste pipeline]
H -- no --> J{keydown AND ctrlKey AND NOT meta/alt/shift?}
J -- no --> M[return true: xterm handles]
J -- yes --> K{code matches Key A-Z AND key.charCodeAt 0 > 127?}
K -- no --> M
K -- yes --> L[Compute byte from code, terminal.input charCode false, return false]
style L fill:#d4edda,stroke:#28a745
style E fill:#d4edda,stroke:#28a745
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts:94
**Possible double-interception when `isComposing` is true**
If an IME composition is in progress (e.g. the user is mid-way through composing a Korean syllable) and presses Ctrl+letter, `isComposing` may be `true`. xterm's own handler skips events where `isComposing` is true, so it would naturally ignore that event. The new block doesn't guard for this state, meaning it would still call `terminal.input()` and `preventDefault()` — potentially swallowing an event that should have been ignored and interrupting IME composition unexpectedly. Adding `&& !event.isComposing` to the outer condition would match xterm's own behavior and avoid this edge case.
### Issue 2 of 2
apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts:97
**`wasUserInput: false` is inconsistent with the rest of the handler**
The `translateLineEditChord` path directly above (line 54) calls `terminal.input(translation, true)`. When `wasUserInput` is `true`, xterm scrolls the viewport to the bottom before delivering the bytes — the expected behavior for a real keypress. Passing `false` here means a user who has scrolled up and then presses Ctrl+K (or any Ctrl+letter) while Korean IME is active won't have the viewport snapped to the current prompt, so the shell's response appears off-screen. Since this block only fires on a genuine `keydown`, `true` is the correct value.
```suggestion
terminal.input(String.fromCharCode(charCode), true);
```
Reviews (1): Last reviewed commit: "fix(terminal): forward Ctrl+<letter> to ..." | Re-trigger Greptile
| !event.shiftKey | ||
| ) { | ||
| const codeMatch = event.code.match(/^Key([A-Z])$/); | ||
| if (codeMatch && event.key.charCodeAt(0) > 127) { |
There was a problem hiding this comment.
Possible double-interception when
isComposing is true
If an IME composition is in progress (e.g. the user is mid-way through composing a Korean syllable) and presses Ctrl+letter, isComposing may be true. xterm's own handler skips events where isComposing is true, so it would naturally ignore that event. The new block doesn't guard for this state, meaning it would still call terminal.input() and preventDefault() — potentially swallowing an event that should have been ignored and interrupting IME composition unexpectedly. Adding && !event.isComposing to the outer condition would match xterm's own behavior and avoid this edge case.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts
Line: 94
Comment:
**Possible double-interception when `isComposing` is true**
If an IME composition is in progress (e.g. the user is mid-way through composing a Korean syllable) and presses Ctrl+letter, `isComposing` may be `true`. xterm's own handler skips events where `isComposing` is true, so it would naturally ignore that event. The new block doesn't guard for this state, meaning it would still call `terminal.input()` and `preventDefault()` — potentially swallowing an event that should have been ignored and interrupting IME composition unexpectedly. Adding `&& !event.isComposing` to the outer condition would match xterm's own behavior and avoid this edge case.
How can I resolve this? If you propose a fix, please make it concise.| if (codeMatch && event.key.charCodeAt(0) > 127) { | ||
| event.preventDefault(); | ||
| const charCode = codeMatch[1].charCodeAt(0) - 64; // A→1 … Z→26 | ||
| terminal.input(String.fromCharCode(charCode), false); |
There was a problem hiding this comment.
wasUserInput: false is inconsistent with the rest of the handler
The translateLineEditChord path directly above (line 54) calls terminal.input(translation, true). When wasUserInput is true, xterm scrolls the viewport to the bottom before delivering the bytes — the expected behavior for a real keypress. Passing false here means a user who has scrolled up and then presses Ctrl+K (or any Ctrl+letter) while Korean IME is active won't have the viewport snapped to the current prompt, so the shell's response appears off-screen. Since this block only fires on a genuine keydown, true is the correct value.
| terminal.input(String.fromCharCode(charCode), false); | |
| terminal.input(String.fromCharCode(charCode), true); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts
Line: 97
Comment:
**`wasUserInput: false` is inconsistent with the rest of the handler**
The `translateLineEditChord` path directly above (line 54) calls `terminal.input(translation, true)`. When `wasUserInput` is `true`, xterm scrolls the viewport to the bottom before delivering the bytes — the expected behavior for a real keypress. Passing `false` here means a user who has scrolled up and then presses Ctrl+K (or any Ctrl+letter) while Korean IME is active won't have the viewport snapped to the current prompt, so the shell's response appears off-screen. Since this block only fires on a genuine `keydown`, `true` is the correct value.
```suggestion
terminal.input(String.fromCharCode(charCode), true);
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts:97">
P2: Mark this forwarded control byte as user input so xterm applies normal keypress behavior (including scroll-to-bottom and user-input side effects) consistently with the other keyboard translation path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (codeMatch && event.key.charCodeAt(0) > 127) { | ||
| event.preventDefault(); | ||
| const charCode = codeMatch[1].charCodeAt(0) - 64; // A→1 … Z→26 | ||
| terminal.input(String.fromCharCode(charCode), false); |
There was a problem hiding this comment.
P2: Mark this forwarded control byte as user input so xterm applies normal keypress behavior (including scroll-to-bottom and user-input side effects) consistently with the other keyboard translation path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/lib/terminal/terminal-key-event-handler.ts, line 97:
<comment>Mark this forwarded control byte as user input so xterm applies normal keypress behavior (including scroll-to-bottom and user-input side effects) consistently with the other keyboard translation path.</comment>
<file context>
@@ -76,6 +76,29 @@ export function createTerminalKeyEventHandler(
+ if (codeMatch && event.key.charCodeAt(0) > 127) {
+ event.preventDefault();
+ const charCode = codeMatch[1].charCodeAt(0) - 64; // A→1 … Z→26
+ terminal.input(String.fromCharCode(charCode), false);
+ return false;
+ }
</file context>
| terminal.input(String.fromCharCode(charCode), false); | |
| terminal.input(String.fromCharCode(charCode), true); |
Problem
When a non-Latin IME (e.g. Korean 2-set, Japanese, Chinese) is active, physical letter keys are mapped to non-ASCII glyphs by the OS. Pressing Ctrl+O while Korean input is active yields:
event.code = "KeyO"✅ (always layout-agnostic)event.key = "ㅐ"❌ (Korean glyph, charCode 4432)event.ctrlKey = truexterm derives the Ctrl+ control byte from
event.key. Since"ㅐ"is not a Latin letter, xterm silently drops the chord — the byte is never written to the PTY.This breaks any TUI app shortcut that uses a bare Ctrl+letter while a non-Latin IME is active. Confirmed with:
Ctrl+O(toggle transcript) andCtrl+K(clear input line)Root cause
createTerminalKeyEventHandlerreturnstrueto let xterm handle Ctrl+letter chords, but xterm's own key encoder usesevent.keyto compute the control byte and fails silently when the value is non-ASCII.Fix
Intercept the chord before xterm runs: when
ctrlKeyis set, no other modifier is held,event.codeis a letter key (Key[A-Z]), andevent.keyis non-ASCII (charCodeAt(0) > 127), compute the correct control byte fromevent.codeand write it directly to the PTY viaterminal.input().event.codeis always physical-key-position based and unaffected by IME state, so the correct byte is sent regardless of active input source.Relation to prior work
event.code-based matching to the hotkey recorder and resolver but did not cover the xterm key-encoder path.Ctrl+C/D/Z) was already working because xterm handles those as signals internally; this fix covers the remaining Ctrl+letter chords forwarded as raw bytes.Testing
Added 4 unit tests in
terminal-key-event-handler.test.ts:\x0fsent\x0bsentReproduction
claude(Claude Code CLI) in the terminalCtrl+O→ transcript toggle does not fire (bug)Ctrl+Oworks correctly ✅Summary by cubic
Fixes Ctrl+letter chords being dropped under non‑Latin IMEs by deriving the control byte from
event.codeand sending it to the PTY. Restores TUI shortcuts like Ctrl+O and Ctrl+K when Korean/Japanese/Chinese input is active.event.codeisKey[A-Z]andevent.keyis non‑ASCII; computesA→\x01 … Z→\x1aand writes viaterminal.input().event.keyis ASCII (Latin IME) or when Shift/Alt/Meta is pressed; letsxtermhandle normally.Written for commit 40238a1. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Bug Fixes
Tests