fix bonesplit to show custom modifier keys#32
fix bonesplit to show custom modifier keys#32gmmeyer wants to merge 1 commit intomanaflow-ai:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Bonsplit/Internal/Views/TabBarView.swift">
<violation number="1" location="Sources/Bonsplit/Internal/Views/TabBarView.swift:735">
P2: Resolved modifier flags are not validated after masking; invalid nonzero stored values can become empty flags and incorrectly trigger hint mode with no modifier pressed.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| private static func resolvedSurfaceFlags(defaults: UserDefaults) -> NSEvent.ModifierFlags { | ||
| let raw = defaults.integer(forKey: surfaceModifierKey) | ||
| guard raw != 0 else { return defaultSurfaceFlags } | ||
| return NSEvent.ModifierFlags(rawValue: UInt(raw)).intersection(.deviceIndependentFlagsMask) |
There was a problem hiding this comment.
P2: Resolved modifier flags are not validated after masking; invalid nonzero stored values can become empty flags and incorrectly trigger hint mode with no modifier pressed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Bonsplit/Internal/Views/TabBarView.swift, line 735:
<comment>Resolved modifier flags are not validated after masking; invalid nonzero stored values can become empty flags and incorrectly trigger hint mode with no modifier pressed.</comment>
<file context>
@@ -702,6 +712,34 @@ enum TabControlShortcutHintPolicy {
+ private static func resolvedSurfaceFlags(defaults: UserDefaults) -> NSEvent.ModifierFlags {
+ let raw = defaults.integer(forKey: surfaceModifierKey)
+ guard raw != 0 else { return defaultSurfaceFlags }
+ return NSEvent.ModifierFlags(rawValue: UInt(raw)).intersection(.deviceIndependentFlagsMask)
+ }
+
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Bonsplit/Internal/Views/TabBarView.swift (1)
671-682:⚠️ Potential issue | 🟠 MajorRefresh the hint-policy tests for the new flags-based return value.
Line 681 and Line 682 now return
.flags(...), butTests/BonsplitTests/BonsplitTests.swift:623-624still expect.commandand.control. That leaves the test target broken until those assertions are updated to the new enum shape.Suggested follow-up in
Tests/BonsplitTests/BonsplitTests.swift-XCTAssertEqual(TabControlShortcutHintPolicy.hintModifier(for: [.command], defaults: defaults), .command) -XCTAssertEqual(TabControlShortcutHintPolicy.hintModifier(for: [.control], defaults: defaults), .control) +XCTAssertEqual( + TabControlShortcutHintPolicy.hintModifier(for: [.command], defaults: defaults), + .flags([.command]) +) +XCTAssertEqual( + TabControlShortcutHintPolicy.hintModifier(for: [.control], defaults: defaults), + .flags([.control]) +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Bonsplit/Internal/Views/TabBarView.swift` around lines 671 - 682, The tests still assert the old enum cases (.command and .control) but the function TabBarView.hintModifier(for:defaults:) now returns TabControlShortcutModifier.flags(...) with surface/workspace masks; update the test assertions in the BonsplitTests to expect .flags(...) instead of .command/.control and construct the expected value by calling resolvedSurfaceFlags(defaults:) and resolvedWorkspaceFlags(defaults:) (or by using the same flag masks used in production) so the assertions compare TabControlShortcutModifier.flags(resolvedSurfaceFlags(...)) and TabControlShortcutModifier.flags(resolvedWorkspaceFlags(...)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Sources/Bonsplit/Internal/Views/TabBarView.swift`:
- Around line 671-682: The tests still assert the old enum cases (.command and
.control) but the function TabBarView.hintModifier(for:defaults:) now returns
TabControlShortcutModifier.flags(...) with surface/workspace masks; update the
test assertions in the BonsplitTests to expect .flags(...) instead of
.command/.control and construct the expected value by calling
resolvedSurfaceFlags(defaults:) and resolvedWorkspaceFlags(defaults:) (or by
using the same flag masks used in production) so the assertions compare
TabControlShortcutModifier.flags(resolvedSurfaceFlags(...)) and
TabControlShortcutModifier.flags(resolvedWorkspaceFlags(...)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52396ded-4d5d-4333-979d-c89f3c6914ea
📒 Files selected for processing (1)
Sources/Bonsplit/Internal/Views/TabBarView.swift
1 participant
i wanted to allow custom modifier keys to work on cmux, I'll edit the cmux PR with this PR number and do the reverse here. I tested this on cmux and it works.
manaflow-ai/cmux#1819
Summary by cubic
Show pane/tab shortcut hints using the user’s configured modifier keys and display the correct symbols. Hints now trigger on both surface and workspace modifiers, matching
cmuxsettings.UserDefaultskeysdigitShortcutSurfaceModifierFlagsanddigitShortcutWorkspaceModifierFlags(defaults:.controland.command).Written for commit b5a0c93. Summary will update on new commits.
Summary by CodeRabbit