Repository navigation
Open group config in the user's configured editor - #5250
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughConfig file opening refactored to route through ChangesPreferred Editor Settings Integration
🎯 2 (Simple) | ⏱️ ~10 minutes
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (16 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 SummaryRoutes the "Edit Group Configuration…" and "Open Config in External Editor" actions through
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to routing two config-file open calls through an existing, well-tested helper, with no data model or auth impact. Both affected callsites are straightforward one-liner substitutions that delegate to PreferredEditorSettings.open, which is already used by the other config openers. The previously flagged issue (injected closure never called in SidebarWorkspaceGroupConfigOpener) is correctly resolved in this revision: open(configURL) is called at line 35. New test suites cover the resolver logic and the injection routing end-to-end. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Edit Group Configuration… (menu)"] --> B["SidebarWorkspaceGroupConfigOpener\n.openCmuxConfigInEditor()"]
C["Open Config in External Editor (Settings)"] --> D["HostSettingsActions\n.openConfigInExternalEditor()"]
B --> E["openCmuxConfigInEditor(home:open:)\nMaterialize ~/.config/cmux/cmux.json if absent"]
E --> F["open(configURL)\n↳ PreferredEditorSettings.open(_:)"]
D --> F
F --> G{preferredEditorCommand\nset & non-blank?}
G -- Yes --> H["Launch configured editor\n(e.g. 'code', 'nvim')"]
G -- No --> I["NSWorkspace.shared.open\n(OS default handler)"]
style F fill:#d4edda,stroke:#28a745
style G fill:#fff3cd,stroke:#ffc107
Reviews (3): Last reviewed commit: "Open cmux config files in the user's con..." | Re-trigger Greptile |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
The workspace-group "Edit Group Configuration…" opener calls NSWorkspace.shared.open on ~/.config/cmux/cmux.json, which routes through Launch Services to the default .json handler and ignores the user's preferredEditorCommand setting. Introduce a testable seam — openCmuxConfigInEditor(home:open:) — and a regression test asserting the config file is routed through the injected opener. The seam still calls NSWorkspace directly here, so the injected opener is never invoked and the test fails: this commit is intentionally red to prove the test catches the bug (fix follows). Also add resolver coverage for PreferredEditorSettings.resolvedCommand. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Route both config-file openers through the editor-resolving path so they honor preferredEditorCommand and fall back to the OS default — the same semantics as "Open Cmux Settings File" and the Settings config window: - SidebarWorkspaceGroupConfigOpener.openCmuxConfigInEditor (the workspace-group "Edit Group Configuration…" action) now hands the config file to the injected opener, which defaults to PreferredEditorSettings.open. - HostSettingsActions.openConfigInExternalEditor likewise routes through PreferredEditorSettings.open instead of NSWorkspace.shared.open. This turns the regression test added in the previous commit green and eliminates the "this opener forgot to honor the editor setting" class of bug. openWorkspaceGroupsDocs (web docs) stays on NSWorkspace, correctly opening in a browser. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
austinywang
force-pushed
the
feat-group-config-preferred-editor
branch
from
June 2, 2026 23:16
c8c57b5 to
b1ce514
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b1ce514. Configure here.
![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/SidebarWorkspaceGroupConfigOpener.swift (1)
22-36:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInjected
openclosure is never called — config still routes throughNSWorkspace.Line 35 calls
NSWorkspace.shared.open(configURL)instead of the injectedopenclosure. As written, theopenparameter is dead, so:
- The config file is still handed to Launch Services (OS default
.jsonhandler), defeating the entire purpose of this PR —preferredEditorCommandis not honored.- The regression test
routesConfigFileThroughInjectedOpenerwill fail:openedstays empty, so#expect(opened.count == 1)cannot pass.🐛 Proposed fix
- NSWorkspace.shared.open(configURL) + open(configURL)🤖 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/SidebarWorkspaceGroupConfigOpener.swift` around lines 22 - 36, The injected opener closure passed to openCmuxConfigInEditor is never used — the function calls NSWorkspace.shared.open(configURL) instead of invoking the provided open closure; replace the direct call to NSWorkspace.shared.open with a call to the injected open(configURL) so the preferredEditorCommand path is exercised (ensure you still create the file and directories as before and pass the same configURL variable to open). This change targets the openCmuxConfigInEditor function and should make the routesConfigFileThroughInjectedOpener test observe the injected opener being invoked.
🤖 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 `@Sources/SidebarWorkspaceGroupConfigOpener.swift`:
- Around line 22-36: The injected opener closure passed to
openCmuxConfigInEditor is never used — the function calls
NSWorkspace.shared.open(configURL) instead of invoking the provided open
closure; replace the direct call to NSWorkspace.shared.open with a call to the
injected open(configURL) so the preferredEditorCommand path is exercised (ensure
you still create the file and directories as before and pass the same configURL
variable to open). This change targets the openCmuxConfigInEditor function and
should make the routesConfigFileThroughInjectedOpener test observe the injected
opener being invoked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b49cf70-9f7f-42bd-ae56-e5365cc0edf2
📒 Files selected for processing (4)
Sources/SidebarWorkspaceGroupConfigOpener.swiftcmux.xcodeproj/project.pbxprojcmuxTests/PreferredEditorSettingsTests.swiftcmuxTests/SidebarWorkspaceGroupConfigOpenerTests.swift
This was referenced Jun 4, 2026
2 of 5 tasks
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
The workspace-group Edit Group Configuration… action handed
~/.config/cmux/cmux.jsontoNSWorkspace.shared.open, which routes through Launch Services to the default.jsonhandler (e.g. Antigravity) and ignores the user'spreferredEditorCommandsetting. The Open Config in External Editor Settings action had the same gap.Both now route through the existing
PreferredEditorSettings.open(_:)helper — already the single source of truth used by Open Cmux Settings File (openCmuxSettingsFileInEditor) and the Settings config window (ConfigSettingsView.openCurrentSourceInEditor) — so every cmux-config opener honors the configured editor and falls back to the OS default identically. This eliminates the "this opener forgot to honor the editor setting" class of bug.openWorkspaceGroupsDocs()(web docs → browser) andrevealCurrentSourceInFinder()(folder → Finder) are intentionally left onNSWorkspace.shared.open.Tests
Adds
PreferredEditorSettingsTests(Swift Testing) coveringPreferredEditorSettings.resolvedCommand(defaults:)through its injectedUserDefaultsseam: returns the configured command (trimmed) when set, andnilwhen unset or blank (the OS-default fallback path). The actualopendispatch is Launch-Services/UI level and not unit-testable.Localization
No user-facing strings changed — menu/Settings labels are untouched; the change only reroutes the open path and adds internal comments. No
Localizable.xcstringsupdate needed.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Small behavior fix aligning two open actions with existing editor routing; no auth, data, or security surface changes.
Overview
Edit Group Configuration… and Open Config in External Editor no longer call
NSWorkspace.shared.openoncmux.json. They now usePreferredEditorSettings.open, sopreferredEditorCommandis honored instead of the system default.jsonapp.The sidebar opener gains a
openCmuxConfigInEditor(home:open:)seam (production passesPreferredEditorSettings.open) so tests can assert the config URL is handed to the editor path. Docs and other non-config opens stay onNSWorkspace.New
PreferredEditorSettingsTestscoverresolvedCommand(unset/blank → OS default, trimmed command when set).SidebarWorkspaceGroupConfigOpenerTestsverify routing through the injected opener and empty-config materialization under a temp home.Reviewed by Cursor Bugbot for commit 6e6f8c3. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Group and settings config files now open in the user's preferred editor instead of the OS
.jsondefault. This aligns all config openers and fixes the "opens in wrong app" bug.PreferredEditorSettings.open(_:)with OS-default fallback.SidebarWorkspaceGroupConfigOpener.openCmuxConfigInEditor(home:open:)(defaults toPreferredEditorSettings.open) to ensure editor routing.openWorkspaceGroupsDocs) and Finder reveal (revealCurrentSourceInFinder) onNSWorkspace.shared.open.PreferredEditorSettingsTestsforresolvedCommandandSidebarWorkspaceGroupConfigOpenerTeststo verify routing.Written for commit 6e6f8c3. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests