Skip to content

fix(terminal): execute Return after Korean IME commit - #1671

Merged
lawrencecchen merged 2 commits into
mainfrom
feat-korean-ime-enter
Mar 18, 2026
Merged

lawrencecchen merged 2 commits into
mainfrom
feat-korean-ime-enter

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Mar 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add a regression test for Return after a Korean IME commit
  • send the committed text and the confirming Return on the same keypress

Test

  • xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-korean-ime-enter-red -only-testing:cmuxTests/KoreanIMEReturnCommitRegressionTests/testReturnAfterKoreanCommitAlsoSendsReturnToSurface test\n- xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-korean-ime-enter-test -only-testing:cmuxTests/KoreanIMEReturnCommitRegressionTests/testReturnAfterKoreanCommitAlsoSendsReturnToSurface test\n- ./scripts/reload.sh --tag feat-korean-ime-enter

Summary by cubic

Fixes a bug where pressing Return to commit Korean (Hangul) via IME didn’t execute the command. We now send the commit and a Return press to the terminal surface in the same keydown.

  • Bug Fixes
    • When a Korean IME commit clears marked text and the key is Return/Keypad Enter (keyCode 36/76), send a follow-up key press with no text and no mods to the surface so the command runs once.
    • Adds KoreanIMEReturnCommitRegressionTests to ensure Return is forwarded after IME commit.

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

Summary by CodeRabbit

  • Bug Fixes

    • Fixed handling of committed CJK IME key events to properly forward keypresses after IME composition completion.
  • Tests

    • Added regression test for Korean IME Return key behavior.

@vercel

vercel Bot commented Mar 18, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 18, 2026 4:53am

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Mar 18, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5a993bd3-6129-43ab-be74-2bbefc2df42a

📥 Commits

Reviewing files that changed from the base of the PR and between 3bca43d and d748223.

📒 Files selected for processing (2)
  • Sources/GhosttyTerminalView.swift
  • cmuxTests/CJKIMEInputTests.swift

📝 Walkthrough

Walkthrough

This pull request adds IME (Input Method Editor) handling for detecting and forwarding committed CJK composition key confirmation. A new helper function detects when an IME composition is committed with specific key codes (Return or the IME confirm key), and routes these keys through a separate send path. Supporting test infrastructure and regression tests verify Korean IME Return key behavior.

Changes

Cohort / File(s) Summary
IME Commit Detection
Sources/GhosttyTerminalView.swift
Adds shouldSendCommittedIMEConfirmKey(event:markedTextBefore:) helper function to detect IME-committed state with specific key codes (36, 76), integrated into keyDown path after text accumulation to send key events without consuming modifiers.
IME Test Infrastructure
cmuxTests/CJKIMEInputTests.swift
Introduces Objective-C runtime swizzling utilities to intercept interpretKeyEvents(_:) calls, adds findGhosttyNSView(in:) hierarchy traversal helper, and creates KoreanIMEReturnCommitRegressionTests class to verify Return key forwarding after Korean IME text commit.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Through IME's winding composition dance,
Return key ends the preedit's trance,
Committed text, no marks remain,
Korean confirms what Ghostty gained! 🎌

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.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 'fix(terminal): execute Return after Korean IME commit' clearly and concisely describes the main change: fixing terminal behavior to execute Return key after Korean IME commit.
Description check ✅ Passed The description covers the summary (what changed and why) and testing (how tested with specific xcodebuild commands), but is missing demo video and incomplete checklist sections from the template.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-korean-ime-enter
📝 Coding Plan
  • Generate coding plan for human review comments

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.

@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 2 files

@lawrencecchen
lawrencecchen merged commit b64fb30 into main Mar 18, 2026
15 checks passed
@lawrencecchen
lawrencecchen deleted the feat-korean-ime-enter branch March 18, 2026 05:07
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
* test(terminal): cover Return after Korean IME commit

* fix(terminal): execute Return after Korean IME commit

---------

Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
Preview — d748223a Deployed Mar 18, 2026 by vercel[bot]
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.

1 participant