Skip to content

fix: prevent Japanese IME confirmation Enter from executing command - #2075

Merged
lawrencecchen merged 2 commits into
manaflow-ai:mainfrom
rerun0510:fix/japanese-ime-enter-confirmation
Mar 25, 2026
Merged

lawrencecchen merged 2 commits into
manaflow-ai:mainfrom
rerun0510:fix/japanese-ime-enter-confirmation

Conversation

@rerun0510

@rerun0510 rerun0510 commented Mar 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Fix Japanese IME confirmation Enter from executing the command prematurely.
  • Root cause: commit b64fb30 (fix(terminal): execute Return after Korean IME commit #1671) added logic to forward Return to Ghostty after IME composition commit, intended for Korean IME where Enter both confirms the syllable and executes the command in a single step.
  • Japanese (and Chinese) IME use Enter only to confirm the conversion; a second Enter is needed to execute. The original code did not distinguish between input sources, causing Japanese IME confirmation to also trigger command execution.

What changed

File Change
Sources/GhosttyTerminalView.swift shouldSendCommittedIMEConfirmKey now checks KeyboardLayout.id and only sends the extra Return for Korean input sources
Sources/KeyboardLayout.swift Added debugInputSourceIdOverride (DEBUG only) to allow tests to simulate specific input sources
cmuxTests/CJKIMEInputTests.swift Korean IME test now sets the input source override so it passes regardless of the host's active input method

IME behavior difference

IME Enter behavior
Korean (한글) Enter = confirm syllable + execute command (single step)
Japanese (日本語) Enter = confirm conversion only; second Enter = execute command
Chinese (中文) Enter = confirm conversion only; second Enter = execute command

Testing

  • Verified Japanese IME: type text → convert → press Enter → conversion confirmed, command NOT executed. Press Enter again → command executes.
  • Verified Korean IME unit test (testReturnAfterKoreanCommitAlsoSendsReturnToSurface) passes.
  • Built and launched via reload.sh --tag fix-jp-ime.

Demo Video

v0.62.2 (before the bug occurred)

2026-03-25.11.11.05.mov

Before the fix (latest main branch)

2026-03-25.11.23.07.mov

After the fix

2026-03-25.12.57.50.mov

Summary by cubic

Fixes Enter with Japanese/Chinese IME so it only confirms conversion and does not execute the command; Korean IME behavior (confirm + execute in one Enter) remains unchanged.

  • Bug Fixes
    • Restrict extra Return forwarding in shouldSendCommittedIMEConfirmKey to Korean input sources only, using a case-insensitive ID check.
    • Add debugInputSourceIdOverride (DEBUG-only) to KeyboardLayout and update the Korean IME unit test to use it.

Written for commit d74afef. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Return/Enter key behavior for Korean input method editors so committed text and Enter press are delivered correctly; prevents incorrect extra Return for non-Korean IMEs (e.g., Japanese/Chinese).
  • Tests

    • Updated tests to cover Korean IME Return behavior and prevent regressions.

Korean IME commits a syllable and executes on a single Enter, but
Japanese/Chinese IME use Enter only to confirm conversion — a second
Enter is needed to execute. Restrict the extra Return forwarding in
shouldSendCommittedIMEConfirmKey to Korean input sources only.
@vercel

vercel Bot commented Mar 25, 2026

Copy link
Copy Markdown

@rerun0510 is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Mar 25, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Restricts synthesized "extra Return" behavior to Korean IMEs by gating commit-key logic on the current input source ID; adds a DEBUG-only KeyboardLayout override for tests and updates a regression test to use that override.

Changes

Cohort / File(s) Summary
IME Confirm-Key Input Source Gating
Sources/GhosttyTerminalView.swift
Adjusted shouldSendCommittedIMEConfirmKey to require the input source ID contains "korean" (case-insensitive) in addition to existing keycode/marked-text checks, so the synthesized Return is only applied for Korean IMEs.
Keyboard Layout Debug Override
Sources/KeyboardLayout.swift
Added a DEBUG-only static var debugInputSourceIdOverride: String?. In DEBUG builds, KeyboardLayout.id returns the override when set; non-DEBUG behavior unchanged (runtime lookup via TIS APIs).
Korean IME Regression Test
cmuxTests/CJKIMEInputTests.swift
Updated testReturnAfterKoreanCommitAlsoSendsReturnToSurface to set KeyboardLayout.debugInputSourceIdOverride to a Korean input source ID before the test and clear it in the existing defer cleanup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through keys and found the clue,
Only Hangul gets the extra "Return" too,
A DEBUG flag for tests to see,
I nibble bugs and set it free,
Hooray — Korean input hops with me! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: fixing Japanese IME confirmation Enter from executing commands prematurely, addressing the root cause in the changeset.
Description check ✅ Passed The pull request description is comprehensive and follows the template structure with all required sections properly filled out.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rerun0510
rerun0510 marked this pull request as ready for review March 25, 2026 02:26
@greptile-apps

greptile-apps Bot commented Mar 25, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where pressing Enter during Japanese (and Chinese) IME text confirmation would also trigger command execution, by restricting the extra Return forwarding in shouldSendCommittedIMEConfirmKey to Korean input sources only. The root cause was a prior commit that unconditionally forwarded Return to Ghostty after any IME composition commit, without accounting for the behavioral difference: Korean IME uses a single Enter to both confirm a syllable and execute, whereas Japanese/Chinese IME require a second Enter to execute.

Key changes:

  • GhosttyTerminalView.swift: shouldSendCommittedIMEConfirmKey now guards on KeyboardLayout.id containing "korean" (case-insensitive), returning false for all other input sources including Japanese and Chinese.
  • KeyboardLayout.swift: Adds a #if DEBUG-only debugInputSourceIdOverride static property to allow unit tests to simulate specific input sources without requiring the host machine to have that IME active.
  • CJKIMEInputTests.swift: The Korean IME regression test now injects the override so it passes on any developer's machine, regardless of their active input method.

The fix is minimal, targeted, and well-tested for the Korean positive case. A negative test for Japanese/Chinese (verifying the extra Return is not sent) would further strengthen coverage.

Confidence Score: 5/5

  • Safe to merge; the fix is narrow, well-reasoned, and the prior case-sensitivity concern has been addressed.
  • The logic change is a single-line guard that correctly distinguishes Korean from Japanese/Chinese based on Apple's stable input-source ID format. The #if DEBUG injection mechanism is clean and production-safe. The Korean regression test is correctly updated. The only missing piece is a negative test for Japanese/Chinese, which is a nice-to-have rather than a blocker.
  • No files require special attention.

Important Files Changed

Filename Overview
Sources/GhosttyTerminalView.swift Restricts the extra-Return forwarding in shouldSendCommittedIMEConfirmKey to Korean input sources via a case-insensitive ID check; logic is correct and well-commented.
Sources/KeyboardLayout.swift Adds a debugInputSourceIdOverride static property guarded by #if DEBUG for test injection; clean and well-isolated from production code.
cmuxTests/CJKIMEInputTests.swift Updates the Korean IME regression test to set the input-source override; correctly clears it via defer. No negative test (Japanese/Chinese) for the new guard.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[keyDown: Enter / Return] --> B{markedTextBefore\nAND markedText.length == 0?}
    B -- No --> C[Skip extra Return]
    B -- Yes --> D{keyCode == 36\nor 76?}
    D -- No --> C
    D -- Yes --> E{KeyboardLayout.id\navailable?}
    E -- No --> C
    E -- Yes --> F{sourceId contains\n'korean' case-insensitive?}
    F -- No\nJapanese / Chinese / other --> C[No extra Return sent\nUser must press Enter again to execute]
    F -- Yes\nKorean --> G[Send extra Return to Ghostty\nCommand executes in one step]
Loading

Reviews (2): Last reviewed commit: "refactor: use case-insensitive check for..." | Re-trigger Greptile

Comment thread Sources/GhosttyTerminalView.swift Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

@rerun0510

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

@rerun0510

Copy link
Copy Markdown
Contributor Author

Hi @lawrencecchen ,
The changes in #1671 will have a critical impact on users who use Japanese and Chinese. I would appreciate it if you could review this before the next release.

@lawrencecchen
lawrencecchen merged commit a395e8c into manaflow-ai:main Mar 25, 2026
4 of 5 checks passed
@lawrencecchen

Copy link
Copy Markdown
Contributor

Thank you for the contribution!

bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
…anaflow-ai#2075)

* fix: prevent Japanese IME confirmation Enter from executing command

Korean IME commits a syllable and executes on a single Enter, but
Japanese/Chinese IME use Enter only to confirm conversion — a second
Enter is needed to execute. Restrict the extra Return forwarding in
shouldSendCommittedIMEConfirmKey to Korean input sources only.

* refactor: use case-insensitive check for Korean input source ID
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants