Honor macos-option-as-alt left/right: send sided modifier bits to libghostty#6007
Open
austinywang wants to merge 11 commits into
Open
Honor macos-option-as-alt left/right: send sided modifier bits to libghostty#6007austinywang wants to merge 11 commits into
austinywang wants to merge 11 commits into
Conversation
Move GhosttyNSView's NSEvent-flags -> ghostty mods mapping and the duplicated libghostty translation-mods -> NSEvent.ModifierFlags loop into free functions (cmuxGhosttyModsFromFlags, cmuxTranslationModifierFlags), and add a KeyboardLayout.textInputCharacter overload that translates against a specific input source ID. No behavior change; this provides the runtime seams for Option/Alt regression coverage (#5993). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds centralized NSEvent-to-libghostty modifier helpers (including right-side bits), applies them across key and mouse event paths, introduces a DEBUG keyboard-layout lookup, adds a focused Option/Alt regression test suite, wires sources into the Xcode project and CI, and updates the ghostty submodule and checksum. ChangesOption key side-bit and composition handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge. The change is a narrow NSEvent→libghostty modifier-bit translation fix with no new concurrency, no blocking primitives, no user-facing strings, and a #if DEBUG guard on the test seam. The previous two concerns flagged by review were both addressed in subsequent commits on this branch. The fix is self-contained: three nonisolated free functions replace duplicated modifier-translation loops, the key/mouse path split is complete and verified, and the #if DEBUG guard on installedInputSource(forID:) matches the file's existing pattern. The regression suite covers side-bit mapping, mouse-path exclusion, translation-flag round-tripping, and four real keyboard layout compositions via UCKeyTranslate. The dedicated CI gate closes the crash-tolerance gap in the full test run. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[NSEvent keypress / flagsChanged] --> B[modsFromEvent]
B --> C[cmuxGhosttyModsFromFlags
device-independent + NX_DEVICER*KEYMASK
sets GHOSTTY_MODS_*_RIGHT]
C --> D[ghostty_surface_key_translation_mods
libghostty checks macos-option-as-alt
left/right via side bits]
D --> E[cmuxTranslationModifierFlags
map ghostty translation mods
back to NSEvent.ModifierFlags]
E --> F{Option in translation mods?}
F -->|Yes - composing side| G[Keep .option → AppKit composes character]
F -->|No - Alt/Meta side| H[Strip .option → libghostty encodes as Alt]
A2[NSEvent mouse / hover / flagsChanged mouse_pos] --> B2[mouseModsFromEvent]
B2 --> C2[cmuxGhosttyMouseModsFromFlags
device-independent only, no side bits]
C2 --> D2[ghostty_surface_mouse_pos / mouse_button
no false re-dirty on right-side held]
Reviews (7): Last reviewed commit: "Refresh Swift file length budget for mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Covers the consolidated Option/Alt issue: NSEvent device side bits must map to GHOSTTY_MODS_*_RIGHT so libghostty can apply macos-option-as-alt = left|right per physical key, translation flags must keep Option on the composing side, and Option composition must resolve on US (Opt+; -> ...), German (Opt+L -> @), Polish Pro (Opt+A -> a-ogonek), and Canadian-CSA (Opt -> /) layouts. The four right-side-bit tests fail at this commit by design (two-commit regression policy): cmux currently maps both physical Option keys to the generic GHOSTTY_MODS_ALT bit. The follow-up commit adds the side bits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ghostty (#5993) Set GHOSTTY_MODS_{SHIFT,CTRL,ALT,SUPER}_RIGHT from the NSEvent device-dependent flags when translating modifiers for libghostty, mirroring Ghostty.app's ghosttyMods. With the side bits present: - ghostty_surface_key_translation_mods strips Alt only for the configured side, so with macos-option-as-alt = left the right Option key keeps composing characters (Opt+; -> ..., right-Opt+L -> @ on German, right-Opt+A -> a-ogonek on Polish Pro, AltGr -> / on Canadian-CSA) while the left Option sends Alt/Meta - and mirrored for = right. - The key encoder's legacyAltPrefix and kitty associated-text rules apply per physical side instead of treating every Option as the left key. Closes the gap where cmux captured both Option keys regardless of the macos-option-as-alt setting while standalone Ghostty honored it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
austinywang
force-pushed
the
issue-5993-option-as-alt
branch
from
f50f5a5 to
9b2bb43
Compare
The inputSourceID-parameterized textInputCharacter overload and its TISCreateInputSourceList helper exist for the Option-composition regression suite; keep them out of the release binary (Greptile P2, matches the existing debugInputSourceIdOverride precedent). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With sided modifier mapping, subtracting only the generic .command flag left NX_DEVICE*CMDKEYMASK bits behind, producing a SUPER_RIGHT-only mod during command-hover suppression. libghostty stores binding-only mouse mods, so the side-only value compared unequal on every hover event and re-dirtied the screen while suppression was active. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
libghostty stores only binding modifiers for mouse state but compares incoming mods against that stored value, so sided mods on the mouse path made every event with a held right-side modifier register as a modifier change and re-dirty the screen (full-row rebuild per mouse move). Split the boundary: key events keep the side bits macos-option-as-alt needs (cmuxGhosttyModsFromFlags); mouse, hover, and link updates send normalized binding bits (cmuxGhosttyMouseModsFromFlags), restoring cmux's pre-fix mouse behavior. Supersedes the narrower hover-suppression device-bit fix and locks the boundary with a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pulls manaflow-ai/ghostty 05c3e2908 (feat-renderer-realized-offscreen): modsChanged and the key callback's link-highlight gate now compare binding mods against binding mods, so the sided modifier bits cmux sends for macos-option-as-alt = left|right no longer register as a modifier change on every key/cursor event (full-screen re-dirty + link refresh per keystroke while a right-side modifier is held). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
main commit 11f1f6c (#5651) grew TerminalNotificationStore.swift to 2623 lines and Feed/FeedCoordinator.swift to 1255 without refreshing the budget, leaving workflow-guard-tests red on main and on any branch that merges it. Accept that growth here so the merge is buildable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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
cmux's NSEvent → libghostty modifier translation only ever set the generic
GHOSTTY_MODS_ALTbit, so libghostty could never tell the left Option key from the right one.macos-option-as-alt = left|right— which works in standalone Ghostty with the same libghostty engine — therefore had no effect in cmux: with= leftboth Option keys were stripped before character translation (right Option could not compose…,@,ą,/, …; the key encoder ESC-prefixed instead), and with= rightneither Option was treated as Alt (Zellij/Helix Alt bindings dead).The fix mirrors Ghostty.app's
ghosttyMods: setGHOSTTY_MODS_{SHIFT,CTRL,ALT,SUPER}_RIGHTfrom the NSEvent device-dependent flags (NX_DEVICER*KEYMASK) so that:ghostty_surface_key_translation_modsstrips Alt only for the configured side — the other Option key stays available to AppKit for character composition, andlegacyAltPrefix/ kitty associated-text rules (which readmods.sides.altfrom the key event itself) apply per physical side.Commits (two-commit regression policy, plus a seam commit)
cmuxGhosttyModsFromFlags,cmuxTranslationModifierFlags); add aKeyboardLayout.textInputCharacter(forKeyCode:modifierFlags:inputSourceID:)overload that translates against a specific installed input source. No behavior change.-only-testing:cmuxTests/GhosttyOptionAsAltModsTestsCI gate step (same pattern as the Browser pane routes localhost/LAN requests through the macOS system proxy — local dev servers unreachable under a global proxy (no implicit loopback bypass) #5888 browser-proxy gate). The gate is required because the full "Run unit tests" step tolerates app-host crashes by parsing only the last test summary: on the first push of the tests-only commit, the app-host run crashed and timed out before this suite executed and the job still reported green ("All failures are expected, treating as pass") — the suite compiled but never ran. The four right-side-bit tests fail at this commit by design.*_RIGHTbits from the device-dependent flags.Acceptance criteria from #5993
= left→ left Option sends Alt/Meta, right Option composes (…,@,ą,/) — side bits now reach libghostty'sMods.translation.= right→ mirror image.= true/= false→ unchanged (side-independent in libghostty), now consistent with sided encoding rules.…), German (Opt+L →@), Polish Pro (Opt+A →ą), Canadian-CSA (Opt →/, baseé), all via the realUCKeyTranslatetext-input path against the named Apple layouts, headless-safe (no live Metal surface needed), enforced by the focused CI gate.Related
Localization audit
No user-facing strings, settings rows, menus, shortcuts, schema/config text, or docs changed — Swift key-event plumbing, tests, and a CI step only, so there is nothing to localize for en/ja.
Closes #5993, closes #691, closes #1349, closes #1397, closes #2369, closes #647, closes #725, closes #1469, closes #5025
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores