Repository navigation
Add opt-in terminal-editor routing for Cmd-clicked files (open in nvim tab) - #5765
h4ckm1n-dev wants to merge 2 commits into
Conversation
|
@h4ckm1n-dev is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
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:
📝 WalkthroughWalkthroughAdds opt-in Cmd-click routing to open files in a terminal editor: new settings keys (command + extensions), UI and localization, settings-file template/parser updates, router/workspace integration to open terminal-editor tabs, test coverage, and web schema/docs updates. ChangesTerminal Editor Routing and Configuration
Sequence DiagramsequenceDiagram
participant Defaults as UserDefaults
participant Settings as CmdClickTerminalEditorRouteSettings
participant Router as CommandClickFileOpenRouter
participant Workspace as Workspace
participant Terminal as TerminalPanel
Defaults->>Settings: read command & extensions
Router->>Settings: shouldRoute(path)
alt shouldRoute == true
Router->>Workspace: openTerminalEditorIfRouted(filePath,inPane,sourcePanelId)
Workspace->>Settings: editorInvocation(forFile)
Workspace->>Terminal: openTerminalEditorTab(initialCommand, workingDirectory)
Terminal-->>Workspace: TerminalPanel opened
else
Router->>Router: fallback to markdown/preview routing
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✨ 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 |
Greptile SummaryThis PR adds opt-in terminal-editor routing for files opened from cmux. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (9): Last reviewed commit: "Fix terminal-editor route: resolve edito..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let workingDirectory: String? = { | ||
| if let dir = panelDirectories[panelId]?.trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !dir.isEmpty { | ||
| return dir | ||
| } | ||
| if let dir = terminalPanel(for: panelId)? | ||
| .requestedWorkingDirectory? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !dir.isEmpty { | ||
| return dir | ||
| } | ||
| let dir = currentDirectory.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return dir.isEmpty ? nil : dir | ||
| }() | ||
|
|
||
| return newTerminalSurface( | ||
| inPane: paneId, | ||
| focus: true, | ||
| workingDirectory: workingDirectory, | ||
| initialInput: invocation + "\n" | ||
| ) |
There was a problem hiding this comment.
openTerminalEditorTab inlines a verbatim copy of the three-step directory fallback (panel directory → requestedWorkingDirectory → currentDirectory) that already exists as CommandClickFileOpenRouter.resolveWorkingDirectory(workspace:surfaceId:). Two callsites that express the same invariant will drift independently; if a fourth source (e.g. a process-specific override) is ever added to resolveWorkingDirectory, this path will silently miss it. The Workspace method could instead call CommandClickFileOpenRouter.resolveWorkingDirectory(workspace: self, surfaceId: panelId) — or the shared logic could be promoted to a Workspace extension that both callers use.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| /// Routes Cmd-clicked files of selected extensions into a terminal editor | ||
| /// (e.g. nvim, vim, helix) opened in a new terminal split, instead of the | ||
| /// built-in file preview. The editor runs inside a Ghostty-backed pane using | ||
| /// the user's login shell, so TUI editors get a real TTY and the user's PATH. | ||
| /// | ||
| /// The feature is opt-in: it stays inert until `terminalEditorExtensions` lists | ||
| /// at least one extension, so PDF, Markdown, and every other type keep their | ||
| /// existing cmux behavior unless the user explicitly lists them here. | ||
| enum CmdClickTerminalEditorRouteSettings { | ||
| static let commandKey = "terminalEditorCommand" | ||
| static let extensionsKey = "terminalEditorExtensions" | ||
| static let defaultCommand = "nvim" | ||
|
|
||
| /// The editor command to launch (e.g. "nvim", "vim -R", "hx"). Falls back to | ||
| /// `defaultCommand` when the key is unset; an explicitly blank value disables | ||
| /// the route (mirrors `preferredEditorCommand` semantics). | ||
| static func resolvedCommand(defaults: UserDefaults = .standard) -> String? { | ||
| guard let stored = defaults.string(forKey: commandKey) else { return defaultCommand } | ||
| let trimmed = stored.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return trimmed.isEmpty ? nil : trimmed | ||
| } | ||
|
|
||
| /// Lowercased, dot-stripped set of extensions routed to the terminal editor. | ||
| /// Stored as a newline-joined string (see the settings file store); also | ||
| /// tolerates comma/semicolon/space separators when set via raw defaults. | ||
| static func extensions(defaults: UserDefaults = .standard) -> Set<String> { | ||
| guard let stored = defaults.string(forKey: extensionsKey) else { return [] } | ||
| let separators = CharacterSet(charactersIn: "\n,; ") | ||
| var result: Set<String> = [] | ||
| for token in stored.components(separatedBy: separators) { | ||
| let ext = token | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| .trimmingCharacters(in: CharacterSet(charactersIn: ".")) | ||
| .lowercased() | ||
| if !ext.isEmpty { result.insert(ext) } | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| static func matchesExtension(_ path: String, defaults: UserDefaults = .standard) -> Bool { | ||
| let ext = (path as NSString).pathExtension.lowercased() | ||
| guard !ext.isEmpty else { return false } | ||
| return extensions(defaults: defaults).contains(ext) | ||
| } | ||
|
|
||
| /// Cheap, off-main-thread-safe gate used by the command-click router. Only | ||
| /// routes real, readable files (rejects FIFOs, sockets, broken symlinks), | ||
| /// matching the markdown/file-preview routes. | ||
| static func shouldRoute(path: String, defaults: UserDefaults = .standard) -> Bool { | ||
| guard resolvedCommand(defaults: defaults) != nil, | ||
| matchesExtension(path, defaults: defaults) else { return false } | ||
| return CmdClickSupportedFileRouteSettings.isReadableRegularFile(path: path) | ||
| } | ||
|
|
||
| /// The shell input that launches the editor on `path`, ready to be typed | ||
| /// into a login shell (caller appends the newline that runs it). | ||
| static func editorInvocation(forFile path: String, defaults: UserDefaults = .standard) -> String? { | ||
| guard let command = resolvedCommand(defaults: defaults) else { return nil } | ||
| return "\(command) \(shellQuote(path))" | ||
| } | ||
|
|
||
| private static func shellQuote(_ s: String) -> String { | ||
| "'" + s.replacingOccurrences(of: "'", with: "'\\''") + "'" | ||
| } | ||
| } |
There was a problem hiding this comment.
CmdClickTerminalEditorRouteSettings has no UI or AppKit dependencies — it reads UserDefaults, calls FileManager, and constructs a shell string. The companion test file (TerminalEditorRouteSettingsTests) imports it with @testable import cmux, meaning it has to compile the entire app target. The type fits cleanly in the CmuxSettings SwiftPM package alongside AppCatalogSection (which already owns the corresponding DefaultsKey definitions), or in a thin sibling package. Keeping it in cmuxApp.swift compounds the existing file-size debt and forces every new test for this logic to pull in the full app target.
Rule Used: Flag Swift changes that add too much unrelated res... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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.
Inline comments:
In
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry`+Default.swift:
- Around line 42-43: Curated entries must use English-only bare string literals;
update any entries in CuratedSettingEntry+Default.swift that currently call
String(localized:defaultValue:) (notably the entries with ids "preferred-editor"
and "supported-file-previews") to use plain string literals for title and
synonyms so they match the rest of the curated array (e.g., mirror the style
used by "terminal-editor-command" and "terminal-editor-file-types"); ensure you
only change the title/synonyms to literals and leave section/id/synonyms content
intact.
In `@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swift`:
- Around line 286-315: Localization keys used by the new UI rows
(settings.app.terminalEditorCommand,
settings.app.terminalEditorCommand.subtitle,
settings.app.terminalEditorCommand.placeholder,
settings.app.terminalEditorExtensions,
settings.app.terminalEditorExtensions.subtitle,
settings.app.terminalEditorExtensions.placeholder) are only populated for en and
ja in Resources/Localizable.xcstrings; add non-empty translations for the
remaining locales in the localization catalog so every locale contains values
for those keys (review entries referenced by the UI elements SettingsCardRow and
the TextField bindings terminalEditorCommand and terminalEditorExtensions and
update each locale's stringUnit.value accordingly).
In `@Sources/cmuxApp.swift`:
- Around line 5311-5367: CmdClickTerminalEditorRouteSettings (enum with
resolvedCommand, extensions, matchesExtension, shouldRoute, editorInvocation,
shellQuote) lives in cmuxApp.swift but contains pure
parsing/routing/shell-quoting logic and should be extracted into its own Swift
source file; move the entire enum into a new file under Sources (e.g. Settings/
or Routing/) importing Foundation, keep the API signatures and access level,
update any references in the project to the enum (no behavioral changes), and
ensure there are no AppKit/SwiftUI dependencies left in the new file so the
logic can be compiled and unit-tested independently from cmuxApp.swift.
In `@web/data/cmux.schema.json`:
- Around line 322-326: Add a localized descriptionKey for the
terminalEditorCommand schema entry and update its human-readable description to
say "tab" instead of "split"; specifically, add a descriptionKey like
"schemaDescriptions.app.terminalEditorCommand" on the terminalEditorCommand
schema object and replace "new terminal split" with "new terminal tab" in the
description text, then add matching localized strings under
schemaDescriptions.app.terminalEditorCommand in web/messages/en.json and
web/messages/ja.json with the provided English text and the corresponding
Japanese translation.
- Around line 327-331: Add a descriptionKey for the terminalEditorExtensions
schema entry and update its human-readable description to list all supported
separators (newline, comma, semicolon, and space); specifically, add a
descriptionKey (e.g., "cmux.schema.terminalEditorExtensions") in the schema next
to "description", change the description text to mention "newline, comma,
semicolon, or space" to match CmdClickTerminalEditorRouteSettings.extensions,
and add matching localized strings under that key in web/messages/en.json and
web/messages/ja.json so the UI uses the localized description.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3c3e5ab-9b4c-402f-903d-72de9d1ab27a
📒 Files selected for processing (13)
Packages/CmuxSettings/Sources/CmuxSettings/Keys/AppCatalogSection.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swiftResources/Localizable.xcstringsSources/CommandClickFileOpenRouter.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxprojcmuxTests/TerminalEditorRouteSettingsTests.swiftweb/app/[locale]/docs/configuration/page.tsxweb/data/cmux.schema.json
| enum CmdClickTerminalEditorRouteSettings { | ||
| static let commandKey = "terminalEditorCommand" | ||
| static let extensionsKey = "terminalEditorExtensions" | ||
| static let defaultCommand = "nvim" | ||
|
|
||
| /// The editor command to launch (e.g. "nvim", "vim -R", "hx"). Falls back to | ||
| /// `defaultCommand` when the key is unset; an explicitly blank value disables | ||
| /// the route (mirrors `preferredEditorCommand` semantics). | ||
| static func resolvedCommand(defaults: UserDefaults = .standard) -> String? { | ||
| guard let stored = defaults.string(forKey: commandKey) else { return defaultCommand } | ||
| let trimmed = stored.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return trimmed.isEmpty ? nil : trimmed | ||
| } | ||
|
|
||
| /// Lowercased, dot-stripped set of extensions routed to the terminal editor. | ||
| /// Stored as a newline-joined string (see the settings file store); also | ||
| /// tolerates comma/semicolon/space separators when set via raw defaults. | ||
| static func extensions(defaults: UserDefaults = .standard) -> Set<String> { | ||
| guard let stored = defaults.string(forKey: extensionsKey) else { return [] } | ||
| let separators = CharacterSet(charactersIn: "\n,; ") | ||
| var result: Set<String> = [] | ||
| for token in stored.components(separatedBy: separators) { | ||
| let ext = token | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| .trimmingCharacters(in: CharacterSet(charactersIn: ".")) | ||
| .lowercased() | ||
| if !ext.isEmpty { result.insert(ext) } | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| static func matchesExtension(_ path: String, defaults: UserDefaults = .standard) -> Bool { | ||
| let ext = (path as NSString).pathExtension.lowercased() | ||
| guard !ext.isEmpty else { return false } | ||
| return extensions(defaults: defaults).contains(ext) | ||
| } | ||
|
|
||
| /// Cheap, off-main-thread-safe gate used by the command-click router. Only | ||
| /// routes real, readable files (rejects FIFOs, sockets, broken symlinks), | ||
| /// matching the markdown/file-preview routes. | ||
| static func shouldRoute(path: String, defaults: UserDefaults = .standard) -> Bool { | ||
| guard resolvedCommand(defaults: defaults) != nil, | ||
| matchesExtension(path, defaults: defaults) else { return false } | ||
| return CmdClickSupportedFileRouteSettings.isReadableRegularFile(path: path) | ||
| } | ||
|
|
||
| /// The shell input that launches the editor on `path`, ready to be typed | ||
| /// into a login shell (caller appends the newline that runs it). | ||
| static func editorInvocation(forFile path: String, defaults: UserDefaults = .standard) -> String? { | ||
| guard let command = resolvedCommand(defaults: defaults) else { return nil } | ||
| return "\(command) \(shellQuote(path))" | ||
| } | ||
|
|
||
| private static func shellQuote(_ s: String) -> String { | ||
| "'" + s.replacingOccurrences(of: "'", with: "'\\''") + "'" | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Move the terminal-editor routing policy out of cmuxApp.swift.
CmdClickTerminalEditorRouteSettings is pure parsing/routing logic plus shell-quoting. It does not depend on app bootstrap, SwiftUI state, or AppKit, so keeping it in the root app file makes the feature harder to isolate and test. Extract it into a dedicated Swift file under Sources/ (or the existing settings/routing area) and keep cmuxApp.swift as the composition root.
As per coding guidelines: "Flag features implemented directly in the app target/module's root Sources path when core logic is independent of cmux app lifecycle and can compile/test without AppKit, SwiftUI view state, Ghostty globals, or process-wide singletons."
🤖 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/cmuxApp.swift` around lines 5311 - 5367,
CmdClickTerminalEditorRouteSettings (enum with resolvedCommand, extensions,
matchesExtension, shouldRoute, editorInvocation, shellQuote) lives in
cmuxApp.swift but contains pure parsing/routing/shell-quoting logic and should
be extracted into its own Swift source file; move the entire enum into a new
file under Sources (e.g. Settings/ or Routing/) importing Foundation, keep the
API signatures and access level, update any references in the project to the
enum (no behavioral changes), and ensure there are no AppKit/SwiftUI
dependencies left in the new file so the logic can be compiled and unit-tested
independently from cmuxApp.swift.
Source: Coding guidelines
| "terminalEditorCommand": { | ||
| "type": "string", | ||
| "default": "nvim", | ||
| "description": "Command used to open files whose extension is listed in terminalEditorExtensions. The command runs in a new terminal split using your login shell, so TUI editors like nvim, vim, or helix get a real TTY and your PATH. Leave empty to disable terminal-editor routing." | ||
| }, |
There was a problem hiding this comment.
Add descriptionKey for web documentation localization and fix "split" → "tab".
This schema description is displayed on the web docs configuration page and should be localized per the internationalization guidelines. Add a descriptionKey field and create matching entries in web/messages/en.json and web/messages/ja.json.
Additionally, the description says "new terminal split" but the upstream contract (Workspace.openTerminalEditorTab comment in context snippet 2) specifies "new full-pane tab". Update "split" to "tab" for accuracy.
Suggested schema structure with descriptionKey
"terminalEditorCommand": {
"type": "string",
"default": "nvim",
- "description": "Command used to open files whose extension is listed in terminalEditorExtensions. The command runs in a new terminal split using your login shell, so TUI editors like nvim, vim, or helix get a real TTY and your PATH. Leave empty to disable terminal-editor routing."
+ "descriptionKey": "schemaDescriptions.app.terminalEditorCommand",
+ "description": "Command used to open files whose extension is listed in terminalEditorExtensions. The command runs in a new terminal tab using your login shell, so TUI editors like nvim, vim, or helix get a real TTY and your PATH. Leave empty to disable terminal-editor routing."
},Then add to web/messages/en.json:
"schemaDescriptions": {
"app": {
"terminalEditorCommand": "Command used to open files whose extension is listed in terminalEditorExtensions. The command runs in a new terminal tab using your login shell, so TUI editors like nvim, vim, or helix get a real TTY and your PATH. Leave empty to disable terminal-editor routing."
}
}And the corresponding Japanese translation in web/messages/ja.json.
🤖 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 `@web/data/cmux.schema.json` around lines 322 - 326, Add a localized
descriptionKey for the terminalEditorCommand schema entry and update its
human-readable description to say "tab" instead of "split"; specifically, add a
descriptionKey like "schemaDescriptions.app.terminalEditorCommand" on the
terminalEditorCommand schema object and replace "new terminal split" with "new
terminal tab" in the description text, then add matching localized strings under
schemaDescriptions.app.terminalEditorCommand in web/messages/en.json and
web/messages/ja.json with the provided English text and the corresponding
Japanese translation.
Source: Coding guidelines
| "terminalEditorExtensions": { | ||
| "type": "string", | ||
| "default": "", | ||
| "description": "Comma- or space-separated file extensions (without the leading dot, e.g. \"rs, ts, py\") that Cmd-click opens in the terminal editor from terminalEditorCommand instead of the built-in file preview. PDF, Markdown, images, and other types keep their normal cmux behavior unless you list them here. Empty (the default) disables the feature." | ||
| }, |
There was a problem hiding this comment.
Add descriptionKey for localization and document all supported separators.
This description needs a descriptionKey field with matching entries in web/messages/en.json and web/messages/ja.json per the internationalization guidelines.
Additionally, the description says "Comma- or space-separated" but the upstream parsing code (CmdClickTerminalEditorRouteSettings.extensions in context snippet 1) supports four separators: newline, comma, semicolon, and space (CharacterSet(charactersIn: "\n,; ")). Update the description to mention all supported separators.
Suggested schema structure with descriptionKey and complete separator list
"terminalEditorExtensions": {
"type": "string",
"default": "",
- "description": "Comma- or space-separated file extensions (without the leading dot, e.g. \"rs, ts, py\") that Cmd-click opens in the terminal editor from terminalEditorCommand instead of the built-in file preview. PDF, Markdown, images, and other types keep their normal cmux behavior unless you list them here. Empty (the default) disables the feature."
+ "descriptionKey": "schemaDescriptions.app.terminalEditorExtensions",
+ "description": "Comma-, semicolon-, newline-, or space-separated file extensions (without the leading dot, e.g. \"rs, ts, py\") that Cmd-click opens in the terminal editor from terminalEditorCommand instead of the built-in file preview. PDF, Markdown, images, and other types keep their normal cmux behavior unless you list them here. Empty (the default) disables the feature."
},Then add to web/messages/en.json and web/messages/ja.json with the updated separator list.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "terminalEditorExtensions": { | |
| "type": "string", | |
| "default": "", | |
| "description": "Comma- or space-separated file extensions (without the leading dot, e.g. \"rs, ts, py\") that Cmd-click opens in the terminal editor from terminalEditorCommand instead of the built-in file preview. PDF, Markdown, images, and other types keep their normal cmux behavior unless you list them here. Empty (the default) disables the feature." | |
| }, | |
| "terminalEditorExtensions": { | |
| "type": "string", | |
| "default": "", | |
| "descriptionKey": "schemaDescriptions.app.terminalEditorExtensions", | |
| "description": "Comma-, semicolon-, newline-, or space-separated file extensions (without the leading dot, e.g. \"rs, ts, py\") that Cmd-click opens in the terminal editor from terminalEditorCommand instead of the built-in file preview. PDF, Markdown, images, and other types keep their normal cmux behavior unless you list them here. Empty (the default) disables the feature." | |
| }, |
🤖 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 `@web/data/cmux.schema.json` around lines 327 - 331, Add a descriptionKey for
the terminalEditorExtensions schema entry and update its human-readable
description to list all supported separators (newline, comma, semicolon, and
space); specifically, add a descriptionKey (e.g.,
"cmux.schema.terminalEditorExtensions") in the schema next to "description",
change the description text to mention "newline, comma, semicolon, or space" to
match CmdClickTerminalEditorRouteSettings.extensions, and add matching localized
strings under that key in web/messages/en.json and web/messages/ja.json so the
UI uses the localized description.
Source: Coding guidelines
77aec8a to
bc9f04f
Compare
|
Refined the terminal-editor routing to provide a 'native' experience:
Tested locally: nvim opens immediately on Cmd-click and the tab disappears upon |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@Sources/Workspace.swift`:
- Around line 18079-18094: Extract the duplicated working-directory logic into a
single Workspace helper (e.g., add a method
Workspace.resolveWorkingDirectory(forPanel: String?) -> String?) that implements
the same precedence: check panelDirectories[id] (trim & non-empty), then
terminalPanel(for: id)?.requestedWorkingDirectory (trim & non-empty), then fall
back to currentDirectory (trim & return nil if empty). Replace the inline
closure in Workspace (the let workingDirectory: String? = { ... }()) with a call
to this new helper, and update
CommandClickFileOpenRouter.resolveWorkingDirectory to delegate to
Workspace.resolveWorkingDirectory(forPanel:) so both sites share the exact same
implementation (use the existing symbols panelDirectories, terminalPanel(for:),
and currentDirectory when implementing the helper).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a1549a9a-99c4-4fab-b767-8bb0f582d576
📒 Files selected for processing (2)
Sources/ContentView.swiftSources/Workspace.swift
|
Please accept this pr, i cannot live without the nvim integration and since cmux is a terminal this is a must have feature :) |
|
@codex review |
@h4ckm1n-dev I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,260 of the 240,000 allowed lines of code this month. Reviews resume on 1 July 2026 (in 21 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
(╯°□°）╯ 🐇 ✅ Action performedReview finished.
|
|
To use Codex here, create a Codex account and connect to github. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d29c60ffb1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Editor / AI-agent scratch directories — never commit (not source) | ||
| .claude/ | ||
| .agents/ | ||
| .greptile/ |
There was a problem hiding this comment.
When Greptile is expected to run on PRs, ignoring the whole .greptile/ directory after this same commit deletes .greptile/config.json and .greptile/rules.md prevents the repo-specific status-check and custom review rules from being restored or updated through normal git; the bot will fall back to defaults. If the goal is only to keep local scratch files out, ignore the scratch subpaths instead of the config directory itself.
Useful? React with 👍 / 👎.
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)
cmuxTests/TerminalEditorRouteSettingsTests.swift (1)
214-220:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert temp-file creation success in test helper.
Line 218 ignores
createFile’s return value. If setup fails, some negative-path tests can pass for the wrong reason (missing file instead of routing logic), weakening regression signal.Suggested fix
private func makeTempFile(extension ext: String) -> URL { let url = FileManager.default.temporaryDirectory .appendingPathComponent("cmux-te-\(UUID().uuidString).\(ext)") - FileManager.default.createFile(atPath: url.path, contents: Data("x".utf8)) + let created = FileManager.default.createFile(atPath: url.path, contents: Data("x".utf8)) + `#expect`(created, "Failed to create temp file at \(url.path)") return url }🤖 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 `@cmuxTests/TerminalEditorRouteSettingsTests.swift` around lines 214 - 220, The helper makeTempFile currently ignores FileManager.default.createFile(...) return value; update makeTempFile to check that createFile returned true and fail the test if it didn't (e.g., use XCTAssertTrue or guard with XCTFail/fatalError) so test setup errors surface immediately; reference the makeTempFile function and include the failing file path in the error message for easier debugging.
🤖 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 `@cmuxTests/TerminalEditorRouteSettingsTests.swift`:
- Around line 214-220: The helper makeTempFile currently ignores
FileManager.default.createFile(...) return value; update makeTempFile to check
that createFile returned true and fail the test if it didn't (e.g., use
XCTAssertTrue or guard with XCTFail/fatalError) so test setup errors surface
immediately; reference the makeTempFile function and include the failing file
path in the error message for easier debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dee5b45e-fb7c-4426-bfde-b2b6ac7c7444
📒 Files selected for processing (4)
Sources/CommandClickFileOpenRouter.swiftSources/ContentView.swiftSources/Workspace.swiftcmuxTests/TerminalEditorRouteSettingsTests.swift
|
Upppp :) this is really a needed feature for a terminal app :) |
d29c60f to
a32b823
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
A GUI-launched app inherits the minimal launchd PATH (/usr/bin:/bin), not the user's login PATH, so a bare `nvim` from /opt/homebrew/bin failed to resolve and the terminal-editor tab closed instantly. Wrap the editor invocation in a login-shell launcher script (same idiom as session restore) so the user's PATH resolves the editor; waitAfterCommand:false still closes the tab on exit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7aa8138. Configure here.
| // for it, matching the terminal Cmd-click behavior (shared logic via | ||
| // Workspace.openTerminalEditorIfRouted). Falls back to the built-in file | ||
| // preview otherwise. | ||
| if workspace.openTerminalEditorIfRouted(filePath: filePath, inPane: paneId) != nil { |
There was a problem hiding this comment.
Sidebar editor ignores panel cwd
Medium Severity
Files sidebar calls openTerminalEditorIfRouted with only inPane, so openTerminalEditorTab resolves cwd via resolvedTerminalWorkingDirectory(forPanelId: nil) and falls back to the workspace directory. Cmd-click passes sourcePanelId and uses the originating terminal’s cwd.
Reviewed by Cursor Bugbot for commit 7aa8138. Configure here.
| snapshot.managedUserDefaults[CmdClickTerminalEditorRouteSettings.commandKey] = .string(value) | ||
| } | ||
| if let value = jsonString(section["terminalEditorExtensions"]) { | ||
| snapshot.managedUserDefaults[CmdClickTerminalEditorRouteSettings.extensionsKey] = .string(value) |
There was a problem hiding this comment.
JSON settings import export gap
Medium Severity
terminalEditorCommand and terminalEditorExtensions are parsed from cmux.json into managed defaults here, but they are not registered in AppSettingsFileMapping.stringSettings or supportedSettingsJSONPaths like preferredEditor. UI and schema advertise these keys while the file-store mapping layer omits them.
Reviewed by Cursor Bugbot for commit 7aa8138. Configure here.
teamleaderleo
added
area: terminal
This branch has not been deployed
2 participants
Summary
What changed? Adds two opt-in
appsettings that open Cmd-clicked files ofchosen types in a terminal editor (nvim, vim, helix, …) in a new full-pane
terminal tab, instead of the built-in plain-text file preview:
Also configurable from Settings → App ("Terminal Editor" + "Terminal Editor
File Types").
terminalEditorExtensionsaccepts specific extensions or thespecial
*token (open every file except cmux's native preview types:Markdown, PDF, images, audio, video). PDF/Markdown/etc. keep their cmux viewers
unless listed; the feature is inert until at least one extension is set.
Why? Cmd-clicking a code file opens cmux's plain-text preview (no syntax
highlighting), and the existing
preferredEditorcan't drive TUI editors — itruns
/bin/sh -c "<cmd> <path>"with no controlling TTY, sonvimexits andcmux falls back to
NSWorkspace.open. Users who live in nvim/vim want a realeditor tab, scoped to chosen file types, while keeping the cmux PDF/Markdown
readers.
How?
CmdClickTerminalEditorRouteSettings(mirrors the existingCmdClick*RouteSettings).Workspace.openTerminalEditorIfRouted(filePath:inPane:sourcePanelId:)used by both the terminal Cmd-click router (
CommandClickFileOpenRouter)and the Files sidebar (
ContentView.openFilePreviewFromSidebar) — one routing path.command(executed through macOS
login(1), a real login shell), so it opens directly(no echoed command) with the user's PATH, and the tab closes on
:q(
waitAfterCommand=false).Workspace.resolvedTerminalWorkingDirectory(forPanelId:)for cwd.locales in
Localizable.xcstrings.TerminalEditorRouteSettingsTests, wired into the cmuxTests target.Behavior / compatibility: zero behavior change by default
(
terminalEditorExtensionsdefaults to""); opt-in per file type.Testing
./scripts/reload.sh --tag terminal-editor(Debug app)and exercised the running app:
.json/.ts/.gofile in a terminal → opens innvimin afull-pane tab;
.md/.png/.pdf→ still open in cmux's viewers.terminalEditorExtensions: "*", every non-preview type opens in nvim;Markdown/PDF/images/audio/video correctly excluded.
:q) closes the tab.TerminalEditorRouteSettingsTestscovers command resolution,extension parsing, the
*wildcard + native-type exclusions, shell quoting,and the routing gate (incl. real temp-file readability checks). Wired into
cmux.xcodeproj;./scripts/lint-pbxproj-test-wiring.shpasses.xcodebuild; theCmuxSettings/CmuxSettingsUISwiftPM packages build.Demo Video
Not applicable — behavior is described above and covered by unit tests. Happy to
attach a short screen recording if a reviewer wants one.
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by CodeRabbit
New Features
Tests
Documentation
Note
Medium Risk
Changes Cmd-click and sidebar file-open routing and spawns arbitrary user-configured shell commands in new terminals; defaults preserve prior behavior but misconfiguration could affect file-open expectations.
Overview
Adds opt-in terminal editor routing so Cmd-click (and Files sidebar opens) can launch configured TUI editors (default
nvim) in a new full-pane terminal tab instead of cmux’s built-in preview.New
appsettingsterminalEditorCommandandterminalEditorExtensions(Settings → App,cmux.json, schema/docs, localized strings). Routing stays inert until extensions are listed;*opens most files while Markdown/PDF/images/audio/video keep native viewers unless explicitly listed.CmdClickTerminalEditorRouteSettingshandles command resolution, extension/wildcardmatching, and shell-safe invocations.Workspace.openTerminalEditorIfRoutedis shared byCommandClickFileOpenRouter(checked first, overrides markdown/preview) andContentView.openFilePreviewFromSidebar. Editors run via a login-shell launcher script for PATH/TTY, withwaitAfterCommandOverride: falseso tabs close when the editor exits.resolvedTerminalWorkingDirectory(forPanelId:)centralizes cwd for router and opener.Includes
TerminalEditorRouteSettingsTestsand cmux.json template/file-store wiring.Reviewed by Cursor Bugbot for commit 7aa8138. Bugbot is set up for automated code reviews on this repo. Configure here.