Repository navigation
File explorer keyboard open selection - #6001
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds two user-configurable file-explorer open-selection shortcuts (Return and Cmd+Down), registers them in settings and schema, enforces chord/bare-first policies, wires runtime handling across outline/search views, updates localization and web docs, adds tests, and updates Xcode project wiring. ChangesFile Explorer Keyboard Open Shortcuts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 passed)
✨ 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 |
Contributor
Greptile SummaryThis PR adds two configurable file-explorer keyboard shortcuts —
Confidence Score: 5/5Safe to merge — all previously identified issues were addressed in prior commits, localization is complete across all 20 app and web locales, and the event-routing additions follow the established stale-menu-suppression and bare-start-routing patterns. The change is a well-scoped feature addition: new shortcut actions are fully gated by allowsChordShortcut and allowsBareFirstStroke, excluded from app-wide bare-start routing and stale-menu suppression loops, and dispatched through a focused view-hierarchy walk rather than a broad event intercept. The three previously flagged issues (compile error in the NSEvent extension, unbound-shortcut beep in results table, and missing xcstrings locales) are all confirmed fixed. Web and app locale coverage is complete. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant NSApp as NSApplication.sendEvent
participant Win as NSWindow.sendEvent
participant AD as AppDelegate
participant FE as FileExplorer view
participant Coord as Coordinator
User->>NSApp: keyDown / keyEquivalent
NSApp->>AD: handleFocusedFileExplorerOpenSelectionShortcut(event)
AD->>AD: shortcutFileExplorerFocusView(for: firstResponder)
alt outline view focused
AD->>FE: outlineView.handleOpenSelectionShortcut(event)
FE->>Coord: openSelectedNode(in: outlineView)
Coord->>Coord: openNode(expand/collapse or openFile)
else results table focused
AD->>FE: resultsView.handleOpenSelectionShortcut(event)
FE-->>AD: calls onCommit
else search field focused
AD->>FE: searchField.handleOpenSelectionShortcut(event)
FE-->>AD: calls onCommit (if not IME, not plain text)
else not file explorer
AD-->>NSApp: false
NSApp->>Win: shouldSuppressStaleCmuxMenuShortcut?
Win->>AD: handleFocusedFileExplorerOpenSelectionShortcut(event)
AD-->>Win: false, continue normal routing
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant NSApp as NSApplication.sendEvent
participant Win as NSWindow.sendEvent
participant AD as AppDelegate
participant FE as FileExplorer view
participant Coord as Coordinator
User->>NSApp: keyDown / keyEquivalent
NSApp->>AD: handleFocusedFileExplorerOpenSelectionShortcut(event)
AD->>AD: shortcutFileExplorerFocusView(for: firstResponder)
alt outline view focused
AD->>FE: outlineView.handleOpenSelectionShortcut(event)
FE->>Coord: openSelectedNode(in: outlineView)
Coord->>Coord: openNode(expand/collapse or openFile)
else results table focused
AD->>FE: resultsView.handleOpenSelectionShortcut(event)
FE-->>AD: calls onCommit
else search field focused
AD->>FE: searchField.handleOpenSelectionShortcut(event)
FE-->>AD: calls onCommit (if not IME, not plain text)
else not file explorer
AD-->>NSApp: false
NSApp->>Win: shouldSuppressStaleCmuxMenuShortcut?
Win->>AD: handleFocusedFileExplorerOpenSelectionShortcut(event)
AD-->>Win: false, continue normal routing
end
Reviews (50): Last reviewed commit: "Merge branch 'main' of https://github.co..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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 (2)
Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift (1)
302-308: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude the two file-explorer open actions in the colocated sidebar ordering list.
settingsVisibleActionscurrently colocates.focusRightSidebar,.toggleRightSidebar, and.findInDirectory, but omits.fileExplorerOpenSelectionand.fileExplorerOpenSelectionFinderAlias. This drifts from the app-side ordering and leaves the new actions outside the intended sidebar shortcut cluster.Suggested fix
let colocated: [ShortcutAction] = [ .focusRightSidebar, .toggleRightSidebar, + .fileExplorerOpenSelection, + .fileExplorerOpenSelectionFinderAlias, .findInDirectory, ].filter(base.contains)Also applies to: 317-319
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift` around lines 302 - 308, The colocated sidebar ordering in the static var settingsVisibleActions currently omits the two file-explorer actions, causing them to be placed outside the intended shortcut cluster; update settingsVisibleActions (and the other similar list around the 317–319 region) to include .fileExplorerOpenSelection and .fileExplorerOpenSelectionFinderAlias alongside .focusRightSidebar, .toggleRightSidebar, and .findInDirectory so the file-explorer actions are grouped with the sidebar shortcuts.Sources/KeyboardShortcutSettings.swift (1)
903-914: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDisallowed chords can still be persisted for file-explorer open actions.
Line 903 currently accepts
resolvedRecordedShortcutIgnoringConflictsresults without enforcingallowsChordShortcut. Since that resolver returns.acceptedfor most actions, a two-stroke binding can still be written for actions that explicitly disallow chords.Suggested fix
private static func storedShortcutForPersistence( _ shortcut: StoredShortcut, action: Action ) -> StoredShortcut? { if shortcut.isUnbound { return shortcut } + if shortcut.hasChord && !action.allowsChordShortcut { + return nil + } switch action.resolvedRecordedShortcutIgnoringConflicts(shortcut) { case let .accepted(normalizedShortcut): return normalizedShortcut case .rejected: if action.usesNumberedDigitMatching || action == .showHideAllWindows || action == .globalSearch || !action.allowsChordShortcut { return nil } return shortcut } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/KeyboardShortcutSettings.swift` around lines 903 - 914, The accepted branch of resolvedRecordedShortcutIgnoringConflicts currently returns normalizedShortcut unconditionally, allowing chord shortcuts where action.allowsChordShortcut is false; modify the .accepted(normalizedShortcut) case to detect chord shortcuts (e.g., normalizedShortcut.isChord or equivalent) and if the action disallows chords (action.allowsChordShortcut == false) treat it like the .rejected branch (apply the same logic used there: if action.usesNumberedDigitMatching || action == .showHideAllWindows || action == .globalSearch || !action.allowsChordShortcut then return nil else return the original shortcut) so disallowed chords are not persisted for actions like file-explorer open.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift`:
- Around line 302-308: The colocated sidebar ordering in the static var
settingsVisibleActions currently omits the two file-explorer actions, causing
them to be placed outside the intended shortcut cluster; update
settingsVisibleActions (and the other similar list around the 317–319 region) to
include .fileExplorerOpenSelection and .fileExplorerOpenSelectionFinderAlias
alongside .focusRightSidebar, .toggleRightSidebar, and .findInDirectory so the
file-explorer actions are grouped with the sidebar shortcuts.
In `@Sources/KeyboardShortcutSettings.swift`:
- Around line 903-914: The accepted branch of
resolvedRecordedShortcutIgnoringConflicts currently returns normalizedShortcut
unconditionally, allowing chord shortcuts where action.allowsChordShortcut is
false; modify the .accepted(normalizedShortcut) case to detect chord shortcuts
(e.g., normalizedShortcut.isChord or equivalent) and if the action disallows
chords (action.allowsChordShortcut == false) treat it like the .rejected branch
(apply the same logic used there: if action.usesNumberedDigitMatching || action
== .showHideAllWindows || action == .globalSearch || !action.allowsChordShortcut
then return nil else return the original shortcut) so disallowed chords are not
persisted for actions like file-explorer open.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ffe312b1-f814-40ff-9a28-1e688d82bbd1
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (12)
Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftPackages/CmuxSettings/Tests/CmuxSettingsTests/ShortcutActionNumberedDigitTests.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/FileExplorerKeyboardShortcuts.swiftSources/FileExplorerView.swiftSources/KeyboardShortcutSettings.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AppDelegateFileExplorerShortcutRoutingTests.swiftcmuxTests/FileExplorerShortcutSettingsTests.swiftweb/data/cmux.schema.json
…rer-keyboard-open
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…rer-keyboard-open
…rer-keyboard-open # Conflicts: # .github/swift-file-length-budget.tsv
…-5996-file-explorer-keyboard-open # Conflicts: # .github/swift-file-length-budget.tsv
…rer-keyboard-open
…-5996-file-explorer-keyboard-open
This branch was successfully deployed
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
Notes
Verification
git diff --checkjq empty Resources/Localizable.xcstrings web/data/cmux.schema.jsonnode -e "const fs=require('fs'); JSON.parse(fs.readFileSync('web/data/cmux.schema.json','utf8')); JSON.parse(fs.readFileSync('Resources/Localizable.xcstrings','utf8')); console.log('json ok')"Localization
shortcut.fileExplorerOpenSelection.labelandshortcut.fileExplorerOpenSelectionFinderAlias.labeltoResources/Localizable.xcstringswith English and Japanese translations.Closes #5996
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds configurable File Explorer open-selection shortcuts (Return and Finder‑style Cmd+Down) across the outline, search field, and results, routed through the shared open handler. Visible in Keyboard Shortcut Settings with shared
CmuxSettingsordering and localized labels; implements #5996.New Features
fileExplorerOpenSelection(Return) andfileExplorerOpenSelectionFinderAlias(Cmd+Down); toggles folders and opens files per preview/default/preferred editor rules; works from outline, search field, and results.ShortcutAction.settingsVisibleActionsordering (after right‑sidebar toggles and “Find in Directory”); bare first strokes allowed, chords disallowed. Schema/docs/localization updated; web renders ↩ and ⌘↓ vialocalizedShortcutText. Split File Explorer into focused AppKit views with no UI changes.Bug Fixes
Written for commit f5b0d27. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Documentation