Skip to content

Revert "Fix McBopomofo Bopomofo candidate opening" - #3845

Merged
austinywang merged 1 commit into
mainfrom
revert-3836-issue-3825-mcbopomofo-candidate-window
May 11, 2026
Merged

austinywang merged 1 commit into
mainfrom
revert-3836-issue-3825-mcbopomofo-candidate-window

Conversation

@austinywang

@austinywang austinywang commented May 11, 2026 •

Copy link
Copy Markdown
Contributor

Reverts #3836


Note

Medium Risk
Touches macOS IME key handling and candidate-window heuristics; could regress candidate opening/navigation for non-Apple Zhuyin/Bopomofo input sources while improving correctness for the built-in Traditional Chinese Zhuyin IME.

Overview
Reverts the prior expansion of the Zhuyin/Bopomofo candidate-opening workaround by scoping it only to Apple’s Traditional Chinese Zhuyin input source (com.apple.inputmethod.TCIM.Zhuyin).

Renames the internal state/helpers from Bopomofo to Zhuyin and updates detection to match TCIM.Zhuyin specifically, which stops triggering the synthetic-space candidate-open path for third-party Bopomofo IMEs. The test suite is updated accordingly by removing the McBopomofo coverage and renaming related debug/test accessors.

Reviewed by Cursor Bugbot for commit 808e1a9. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Reverts the McBopomofo candidate-opening logic and restores handling to only support Apple's Traditional Zhuyin (TCIM.Zhuyin). Removes the McBopomofo test and renames internal bopomofo identifiers to zhuyin to match the scope.

Written for commit 808e1a9. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced Traditional Chinese input support by refining the Zhuyin input method's candidate window behavior, improving keyboard event handling for character selection, and better tracking of user interactions with candidate windows.
  • Tests

    • Updated test suite to validate changes to input method composition behavior and candidate window interactions for Traditional Chinese input.

Review Change Stack

@vercel

vercel Bot commented May 11, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Building Building Preview, Comment May 11, 2026 7:55am
cmux-staging Building Building Preview, Comment May 11, 2026 7:55am

@austinywang
austinywang merged commit ae5715e into main May 11, 2026
15 of 22 checks passed
@coderabbitai

coderabbitai Bot commented May 11, 2026 •

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2e921c18-06ed-4a79-8273-e2c13a55e99a

📥 Commits

Reviewing files that changed from the base of the PR and between b7ce782 and 808e1a9.

📒 Files selected for processing (3)
  • Sources/GhosttyNSView+IMEComposition.swift
  • Sources/GhosttyTerminalView.swift
  • cmuxTests/CJKIMEMarkedSelectionTests.swift

📝 Walkthrough

Walkthrough

The PR refactors IME candidate-opening logic from Bopomofo to Zhuyin input source detection. Helper functions, predicates, and state flags are renamed and repurposed to support Traditional Zhuyin composition instead of Bopomofo, with corresponding test updates.

Changes

Zhuyin IME Candidate Handling

Layer / File(s) Summary
Input Source Detection
Sources/GhosttyNSView+IMEComposition.swift
New isTraditionalZhuyinInputSource() helper identifies Zhuyin input sources by matching TCIM.Zhuyin.
Candidate Opening Predicates
Sources/GhosttyNSView+IMEComposition.swift
shouldOpenZhuyinCandidatesWithSyntheticSpace() and shouldRememberZhuyinCandidateInteraction() switch to Zhuyin input source detection; isBopomofoInputSource() is removed.
IME State Management
Sources/GhosttyTerminalView.swift
zhuyinCandidateOpenRequested flag replaces Bopomofo variant; cleared in resignFirstResponder and unmarkText; exposed via zhuyinCandidateOpenRequestedForTesting property.
Key Event and Synthetic Space Processing
Sources/GhosttyTerminalView.swift
keyDown dispatches synthesized space events via zhuyinCandidateOpenSpaceEvent() after evaluating Zhuyin candidate conditions; both main and "remember interaction" branches updated to use Zhuyin predicates and state.
Test Updates
cmuxTests/CJKIMEMarkedSelectionTests.swift
McBopomofo Down-arrow test removed; testUnmarkTextPreservesSuppressedKeyUpStateWithoutMarkedText updated to use Zhuyin-specific flag names.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • manaflow-ai/cmux#3694: Both PRs modify IME composition code in GhosttyNSView+IMEComposition.swift and GhosttyTerminalView.swift to switch Bopomofo handling to Zhuyin.
  • manaflow-ai/cmux#3574: Both PRs update IME handling in Ghostty for Zhuyin/Bopomofo candidate and composition behavior.
  • manaflow-ai/cmux#3836: Both PRs modify IME candidate-opening logic and input-source predicates for Zhuyin/Bopomofo handling.

Poem

🐰 A bunny hops through input streams,
Trading Bopomofo for Zhuyin dreams,
Candidates open with a synthetic space,
Each keyDown finds its proper place,
Tests updated, predicates true,
The IME now knows what to do!

✨ 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 revert-3836-issue-3825-mcbopomofo-candidate-window

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.

@greptile-apps

greptile-apps Bot commented May 11, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR reverts #3836 ("Fix McBopomofo Bopomofo candidate opening"), restoring the input-source detection for the synthetic-space candidate-open path to Apple's TCIM Zhuyin only. As part of the revert, the helpers and state variable are renamed from the "Bopomofo" namespace to "Zhuyin" to better reflect their narrowed scope.

  • isTraditionalZhuyinInputSource now matches only "TCIM.Zhuyin" (Apple's built-in TCIM), dropping the "Bopomofo" branch that Fix McBopomofo Bopomofo candidate opening #3836 added for McBopomofo/OpenVanilla — those users will no longer receive the synthetic-space candidate trigger.
  • All production symbols (zhuyinCandidateOpenRequested, shouldOpenZhuyinCandidatesWithSyntheticSpace, etc.) and the corresponding test accessors are renamed consistently.
  • The McBopomofo-specific integration test (testMcBopomofoDownArrowRequestsCandidatesViaSpaceCommandFallback) is removed along with the behaviour it covered.

Confidence Score: 4/5

Safe to merge — this is a focused revert with consistent renaming; the only stray item is a single stale "Bopomofo" string in a test assertion message.

The production logic change is intentional and internally consistent. All call sites that previously used the Bopomofo-keyed names have been updated to the Zhuyin-keyed equivalents, and the state lifecycle (set on candidate open, cleared on unmarkText and focus loss) is preserved. One test assertion message at line 419 still reads "Bopomofo candidate arrows" after everything else was renamed to Zhuyin, which is a minor leftover from the rename sweep.

cmuxTests/CJKIMEMarkedSelectionTests.swift — contains the stale assertion message; the two production Swift files look clean.

Important Files Changed

Filename Overview
Sources/GhosttyNSView+IMEComposition.swift Renames helper functions from "Bopomofo" to "Zhuyin" and narrows isTraditionalZhuyinInputSource to only match "TCIM.Zhuyin" (Apple's TCIM), dropping the previous McBopomofo/Bopomofo match added in #3836.
Sources/GhosttyTerminalView.swift Renames bopomofoCandidateOpenRequested and related call sites to zhuyinCandidateOpenRequested; all usages are internally consistent and the state lifecycle is unchanged.
cmuxTests/CJKIMEMarkedSelectionTests.swift Removes the McBopomofo-specific candidate test and renames bopomofoCandidateOpenRequested parameter; one stale "Bopomofo" string remains in an assertion message at line 419.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[keyDown event] --> B{markedTextBefore?}
    B -- No --> Z[Forward to Ghostty]
    B -- Yes --> C{isTraditionalZhuyinInputSource?}
    C -- No --> Z
    C -- Yes --> D{shouldOpenZhuyinCandidatesWithSyntheticSpace?}
    D -- Yes --> E[Inject synthetic Space event, zhuyinCandidateOpenRequested = true]
    D -- No --> F{shouldRememberZhuyinCandidateInteraction?}
    F -- Yes --> G[zhuyinCandidateOpenRequested = true]
    F -- No --> H[zhuyinCandidateOpenRequested = false]
    E --> I[syncPreedit]
Loading

Comments Outside Diff (1)

  1. cmuxTests/CJKIMEMarkedSelectionTests.swift, line 419 (link)

    P2 The assertion message still says "Bopomofo" even though the surrounding test was updated to use "com.apple.inputmethod.TCIM.Zhuyin" and all other renamed symbols now use "Zhuyin". This stale string slipped through the rename sweep.

Reviews (1): Last reviewed commit: "Revert "Fix McBopomofo Bopomofo candidat..." | Re-trigger Greptile

This branch was successfully deployed

1 active deployment
Preview – cmux — 808e1a93 Deployed May 11, 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