Repository navigation
Revert "Fix Chinese IME Enter swallowed in terminal (#3762)" - #3846
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR reverts #3767, which fixed Chinese IME Enter being swallowed in the terminal. The no-marked-text branch of
Confidence Score: 4/5The revert is internally consistent — the logic change, helper removal, and test deletion all align. The re-introduced Enter-swallowing behavior is intentional and documented by the removed test. The change is a clean, self-consistent revert. The only concern is minor duplication of the modifier-flag strip logic, now repeated verbatim in two functions after the shared helper was removed. Sources/GhosttyNSView+IMEComposition.swift — contains the duplicated modifier-flag computation worth consolidating. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["shouldSuppressGhosttyKeyForwardingAfterIMEHandling()"] --> B{accumulatedText empty?}
B -- No --> C[return false]
B -- Yes --> D{hadMarkedText OR hasMarkedText?}
D -- Yes --> E{text changed?}
E -- Yes --> F[return true]
E -- No --> G{selection changed?}
G -- Yes --> F
G -- No --> H{event + isInputMethodSource?}
H -- No --> C
H -- Yes --> I["shouldKeepIMECompositionCommandInsideTextInput(event)"]
D -- No --> J{textInputHandledEvent AND isInputMethodSource?}
J -- No --> C
J -- Yes --> K["!shouldAllowDeferredNumpadIMEFallback(event)"]
subgraph BEFORE ["Before revert (PR #3767)"]
L["shouldKeepNoMarkedIMECommandInsideTextInput(event) AND !shouldAllowDeferredNumpadIMEFallback(event)"]
L -- "kVK_Return → false" --> M["return false — Enter reaches terminal"]
L -- "arrow/space → true" --> N["return true — suppress"]
end
subgraph AFTER ["After revert (this PR)"]
K -- "kVK_Return → true" --> O["return true — Enter SWALLOWED"]
K -- "numpad digit → false" --> P["return false"]
end
Reviews (1): Last reviewed commit: "Revert "Fix Chinese IME Enter swallowed ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let flags = event.modifierFlags | ||
| .intersection(.deviceIndependentFlagsMask) | ||
| .subtracting([.numericPad, .function, .capsLock]) | ||
| guard flags.isEmpty || flags == [.shift] else { return false } |
There was a problem hiding this comment.
The hasOnlyTextInputCommandModifiers helper that was removed alongside shouldKeepNoMarkedIMECommandInsideTextInput contained the modifier-flag computation now inlined verbatim into both shouldRememberZhuyinCandidateInteraction (line 128–131) and shouldKeepIMECompositionCommandInsideTextInput (line 158–161). If the set of stripped modifiers ever needs to change, two sites must be updated in sync. Extracting this as a private helper would keep the rule in one place.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIME suppression logic refactored: consumed-input no-marked-text suppression now depends solely on deferred numpad fallback eligibility. Shared modifier-check helpers removed and inlined at call sites. Regression test removed. ChangesIME Suppression Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
This branch was successfully deployed
1 participant
Reverts #3767
Note
Cursor Bugbot is generating a summary for commit 6871cc5. Configure here.
Summary by cubic
Reverts the Enter-key handling change for Chinese IMEs and restores the previous IME command-key routing in the terminal. Removes the “no-marked-text” IME command path and the Zhuyin Return forwarding test; simplifies modifier checks by inlining the filter in call sites.
Written for commit 6871cc5. Summary will update on new commits.
Summary by CodeRabbit
Refactor
Chores