Fix keyboard shortcuts with CJK input sources - #1649
Merged
lawrencecchen merged 1 commit intoMar 20, 2026
Merged
Conversation
…se, Japanese) When a non-Latin input source is active, event.charactersIgnoringModifiers returns CJK characters that cannot match Latin shortcut keys. This adds ASCII-capable input source fallback in KeyboardLayout and updates the matchShortcut guard to skip early-return when event chars are non-ASCII. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@sumniy is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThe PR adds ASCII validation to command shortcuts to prevent non-Latin input sources from falsely triggering Latin-key shortcuts, and introduces a fallback mechanism in keyboard layout resolution that attempts ASCII-capable input sources when the current source lacks Unicode key layout data (e.g., CJK sources). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppDelegate as AppDelegate
participant KeyboardLayout as KeyboardLayout
participant InputSource as Input Source (Current)
participant ASCIISource as Input Source (ASCII)
User->>AppDelegate: Press key (non-ASCII source active)
AppDelegate->>AppDelegate: Check eventCharsAreASCII
alt Chars are ASCII
AppDelegate->>KeyboardLayout: character(forKeyCode:modifierFlags:)
KeyboardLayout->>InputSource: Try UCKeyTranslate with current source
alt Unicode layout data exists
InputSource-->>KeyboardLayout: Character resolved
else No layout data (CJK source)
KeyboardLayout->>ASCIISource: Fall back to ASCII source
ASCIISource-->>KeyboardLayout: Character resolved
end
KeyboardLayout-->>AppDelegate: Character result
AppDelegate->>AppDelegate: Route command shortcut
else Chars are non-ASCII
AppDelegate->>AppDelegate: Skip command shortcuts
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/KeyboardLayout.swift (1)
24-34: Narrow fallback to the intended missing-layout case.At Line 24 through Line 34, fallback runs whenever
characterFromInputSource(...)returnsnil, including non-layout translation failures. That’s a bit broader than the comment intent and can mask true translation errors.♻️ Proposed refinement
- if let source = TISCopyCurrentKeyboardInputSource()?.takeRetainedValue(), - let result = characterFromInputSource(source, forKeyCode: keyCode, modifierFlags: modifierFlags) { - return result - } + if let source = TISCopyCurrentKeyboardInputSource()?.takeRetainedValue() { + let hasUnicodeLayoutData = + TISGetInputSourceProperty(source, kTISPropertyUnicodeKeyLayoutData) != nil + if let result = characterFromInputSource(source, forKeyCode: keyCode, modifierFlags: modifierFlags) { + return result + } + if hasUnicodeLayoutData { + return nil + } + } // Current input source has no Unicode layout data (e.g. Korean, Chinese, Japanese IME). // Fall back to the ASCII-capable source so shortcut matching still works. if let asciiSource = TISCopyCurrentASCIICapableKeyboardInputSource()?.takeRetainedValue(), let result = characterFromInputSource(asciiSource, forKeyCode: keyCode, modifierFlags: modifierFlags) { return result }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/KeyboardLayout.swift` around lines 24 - 34, The current fallback to the ASCII-capable source runs anytime characterFromInputSource(...) returns nil, which masks real translation failures; change the logic in the block that calls TISCopyCurrentKeyboardInputSource() and characterFromInputSource(...) so you only attempt the ASCII fallback when the original input source truly lacks Unicode layout data (check the input source's kTISPropertyUnicodeKeyLayoutData / kTISPropertyInputSourceType via TISGetInputSourceProperty on the source returned by TISCopyCurrentKeyboardInputSource()), otherwise propagate the nil result from characterFromInputSource (i.e., do not call TISCopyCurrentASCIICapableKeyboardInputSource() if the current source has layout data but translation failed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/KeyboardLayout.swift`:
- Around line 24-34: The current fallback to the ASCII-capable source runs
anytime characterFromInputSource(...) returns nil, which masks real translation
failures; change the logic in the block that calls
TISCopyCurrentKeyboardInputSource() and characterFromInputSource(...) so you
only attempt the ASCII fallback when the original input source truly lacks
Unicode layout data (check the input source's kTISPropertyUnicodeKeyLayoutData /
kTISPropertyInputSourceType via TISGetInputSourceProperty on the source returned
by TISCopyCurrentKeyboardInputSource()), otherwise propagate the nil result from
characterFromInputSource (i.e., do not call
TISCopyCurrentASCIICapableKeyboardInputSource() if the current source has layout
data but translation failed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f2dac0d6-0d11-441c-910b-543415ba59d4
📒 Files selected for processing (2)
Sources/AppDelegate.swiftSources/KeyboardLayout.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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="Sources/KeyboardLayout.swift">
<violation number="1" location="Sources/KeyboardLayout.swift:30">
P2: Fallback uses ASCII-capable *input source* API, but translation requires Unicode keyboard layout data; use ASCII-capable keyboard *layout* API to avoid nil on IME setups.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| // Current input source has no Unicode layout data (e.g. Korean, Chinese, Japanese IME). | ||
| // Fall back to the ASCII-capable source so shortcut matching still works. | ||
| if let asciiSource = TISCopyCurrentASCIICapableKeyboardInputSource()?.takeRetainedValue(), |
There was a problem hiding this comment.
P2: Fallback uses ASCII-capable input source API, but translation requires Unicode keyboard layout data; use ASCII-capable keyboard layout API to avoid nil on IME setups.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/KeyboardLayout.swift, line 30:
<comment>Fallback uses ASCII-capable *input source* API, but translation requires Unicode keyboard layout data; use ASCII-capable keyboard *layout* API to avoid nil on IME setups.</comment>
<file context>
@@ -15,12 +15,31 @@ class KeyboardLayout {
+ }
+ // Current input source has no Unicode layout data (e.g. Korean, Chinese, Japanese IME).
+ // Fall back to the ASCII-capable source so shortcut matching still works.
+ if let asciiSource = TISCopyCurrentASCIICapableKeyboardInputSource()?.takeRetainedValue(),
+ let result = characterFromInputSource(asciiSource, forKeyCode: keyCode, modifierFlags: modifierFlags) {
+ return result
</file context>
Suggested change
| if let asciiSource = TISCopyCurrentASCIICapableKeyboardInputSource()?.takeRetainedValue(), | |
| if let asciiSource = TISCopyCurrentASCIICapableKeyboardLayoutInputSource()?.takeRetainedValue(), |
4 of 6 tasks
Contributor
|
Thank you for the contribution! |
This was referenced Mar 21, 2026
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
…se, Japanese) (manaflow-ai#1649) When a non-Latin input source is active, event.charactersIgnoringModifiers returns CJK characters that cannot match Latin shortcut keys. This adds ASCII-capable input source fallback in KeyboardLayout and updates the matchShortcut guard to skip early-return when event chars are non-ASCII. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
KeyboardLayout.character()for CJK IMEs that lackkTISPropertyUnicodeKeyLayoutDatamatchShortcut()guard to skip early-return whencharactersIgnoringModifierscontains non-ASCII charactersProblem
When a CJK input source is active,
event.charactersIgnoringModifiersreturns non-Latin characters (e.g. Korean jamo "ㅔ" for the P key). The existing shortcut matching logic had two guards that treated these characters as valid Latin input, causing all fallback paths (layout-based and keyCode-based) to be blocked.Solution
KeyboardLayout.swift:character(forKeyCode:modifierFlags:)now tries the current input source first, then falls back toTISCopyCurrentASCIICapableKeyboardInputSource()when no Unicode layout data is available. Extracted shared logic intocharacterFromInputSource()helper.AppDelegate.swift: AddedeventCharsAreASCIIcheck so non-ASCII event characters don't trigger the early-return guard for Cmd+letter shortcuts. AddedhasUsableEventCharsso the ANSI keyCode fallback correctly activates for CJK input sources.Scope
This covers shortcuts handled by
handleCustomShortcut(local event monitor). Menu/SwiftUI.keyboardShortcut()shortcuts are not yet covered.Test plan
🤖 Generated with Claude Code
Summary by cubic
Fixes Command shortcuts (e.g., Cmd+P, Cmd+B, Cmd+D) not working when Korean, Chinese, or Japanese input sources are active. Adds an ASCII-capable layout fallback and smarter event-character checks so shortcuts work across layouts without regressions.
KeyboardLayout.character(...)now tries the current input source first and falls back toTISCopyCurrentASCIICapableKeyboardInputSource()whenkTISPropertyUnicodeKeyLayoutDatais missing; shared logic extracted tocharacterFromInputSource(...).AppDelegateto ignore non-ASCIIcharactersIgnoringModifiersfor Command-letter shortcuts and treat them as absent, enabling layout-based matching and ANSI keyCode fallback when needed.Written for commit 19204c7. Summary will update on new commits.
Summary by CodeRabbit