Repository navigation
fix(openFolder): set NSOpenPanel directoryURL to focused pane's working directory - #2019
BillionClaw wants to merge 1 commit into
Conversation
…ng directory Before: ⌘O always opened the file picker at ~/Documents (NSOpenPanel default). After: ⌘O now opens the picker at the focused pane's current working directory when available, falling back to ~/Documents otherwise. Fixes: manaflow-ai#2010
|
@BillionClaw is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe "Open Folder…" command across both ContentView.swift and cmuxApp.swift now pre-configures the NSOpenPanel's directoryURL to default to the currently selected workspace's focused panel's saved directory when available. Workspace creation calls are qualified with Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes ⌘O ("Open Folder") in both the menu bar and command palette so that
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["User triggers ⌘O\n(menu bar OR command palette)"] --> B["Create NSOpenPanel"]
B --> C{"selectedWorkspace\n.focusedPanelId\n!= nil?"}
C -- No --> E["NSOpenPanel defaults\nto ~/Documents"]
C -- Yes --> D{"panelDirectories\n[focusedPanelId]\nnon-empty?"}
D -- No --> E
D -- Yes --> F["panel.directoryURL =\nURL(fileURLWithPath: preferredDir)"]
F --> G["panel.runModal()"]
E --> G
G -- ".OK" --> H["tabManager.addWorkspace\n(workingDirectory: url.path)"]
G -- "cancelled" --> I["No-op"]
Reviews (1): Last reviewed commit: "fix(openFolder): set NSOpenPanel directo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| panel.title = String(localized: "panel.openFolder.title", defaultValue: "Open Folder") | ||
| panel.prompt = String(localized: "panel.openFolder.prompt", defaultValue: "Open") | ||
| if let preferredDir = self.tabManager.selectedWorkspace?.focusedPanelId.flatMap({ self.tabManager.selectedWorkspace?.panelDirectories[$0] }), !preferredDir.isEmpty { | ||
| panel.directoryURL = URL(fileURLWithPath: preferredDir) |
There was a problem hiding this comment.
isDirectory: true when constructing directory URL
URL(fileURLWithPath:) without isDirectory: true consults the file system to infer whether the path is a file or directory. If the working directory stored in panelDirectories has since been deleted (e.g. the user rm -rf'd a temp dir in the terminal), the stat will fail and the resulting URL will have hasDirectoryPath == false. NSOpenPanel ignores a directoryURL that isn't a directory URL, silently falling back to ~/Documents — the exact behavior this PR is trying to fix.
Explicitly passing isDirectory: true makes the intent unambiguous and ensures the URL is formed correctly regardless of whether the path still exists on disk:
| panel.directoryURL = URL(fileURLWithPath: preferredDir) | |
| panel.directoryURL = URL(fileURLWithPath: preferredDir, isDirectory: true) |
The same issue exists in Sources/cmuxApp.swift:597.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/cmuxApp.swift`:
- Around line 596-598: The current logic sets panel.directoryURL only when a
focused panel's panelDirectories entry exists, so when that lookup is missing it
falls back to the global default; update the code around
activeTabManager.selectedWorkspace?.focusedPanelId and panelDirectories to first
try the focused panel directory, and if that is nil or empty fall back to the
workspace-level directory (e.g.,
activeTabManager.selectedWorkspace?.currentDirectory) before finally defaulting
to ~/Documents; ensure you check for non-empty strings and set
panel.directoryURL = URL(fileURLWithPath: <chosenPath>) using the workspace
currentDirectory when available.
In `@Sources/ContentView.swift`:
- Around line 5978-5980: When setting panel.directoryURL in ContentView, prefer
the focused panel directory but fall back to the workspace-level directory
before letting NSOpenPanel use its default: update the logic around
tabManager.selectedWorkspace?.focusedPanelId and panelDirectories so that if
preferredDir is nil or empty you attempt to read the workspace directory (e.g.
tabManager.selectedWorkspace?.directory or similar workspace-level path
property) and set panel.directoryURL = URL(fileURLWithPath: workspaceDir) when
present; only if both focused panel and workspace directory are absent should
you leave the panel to use NSOpenPanel's default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39a7f169-c6a3-44b3-8722-f43d43ccafb9
📒 Files selected for processing (2)
Sources/ContentView.swiftSources/cmuxApp.swift
| if let preferredDir = self.activeTabManager.selectedWorkspace?.focusedPanelId.flatMap({ self.activeTabManager.selectedWorkspace?.panelDirectories[$0] }), !preferredDir.isEmpty { | ||
| panel.directoryURL = URL(fileURLWithPath: preferredDir) | ||
| } |
There was a problem hiding this comment.
Add workspace-level fallback before defaulting to Documents
Line 596 only uses focusedPanelId -> panelDirectories. If that lookup is missing (e.g., fresh workspace/no OSC7 update yet), the picker still opens at the default location instead of a workspace-contextual path. Please fall back to workspace-level directory (e.g., currentDirectory) before ~/Documents.
💡 Suggested patch
- if let preferredDir = self.activeTabManager.selectedWorkspace?.focusedPanelId.flatMap({ self.activeTabManager.selectedWorkspace?.panelDirectories[$0] }), !preferredDir.isEmpty {
- panel.directoryURL = URL(fileURLWithPath: preferredDir)
- }
+ let workspace = self.activeTabManager.selectedWorkspace
+ let focusedPaneDir = workspace.flatMap { ws in
+ ws.focusedPanelId.flatMap { ws.panelDirectories[$0] }
+ }
+ let startDir = (
+ focusedPaneDir
+ ?? workspace?.currentDirectory
+ ?? NSString(string: "~/Documents").expandingTildeInPath
+ ).trimmingCharacters(in: .whitespacesAndNewlines)
+ if !startDir.isEmpty {
+ panel.directoryURL = URL(fileURLWithPath: startDir)
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/cmuxApp.swift` around lines 596 - 598, The current logic sets
panel.directoryURL only when a focused panel's panelDirectories entry exists, so
when that lookup is missing it falls back to the global default; update the code
around activeTabManager.selectedWorkspace?.focusedPanelId and panelDirectories
to first try the focused panel directory, and if that is nil or empty fall back
to the workspace-level directory (e.g.,
activeTabManager.selectedWorkspace?.currentDirectory) before finally defaulting
to ~/Documents; ensure you check for non-empty strings and set
panel.directoryURL = URL(fileURLWithPath: <chosenPath>) using the workspace
currentDirectory when available.
| if let preferredDir = self.tabManager.selectedWorkspace?.focusedPanelId.flatMap({ self.tabManager.selectedWorkspace?.panelDirectories[$0] }), !preferredDir.isEmpty { | ||
| panel.directoryURL = URL(fileURLWithPath: preferredDir) | ||
| } |
There was a problem hiding this comment.
Use workspace directory fallback before defaulting to NSOpenPanel’s default location.
Line 5978 only uses the focused panel directory. When that value is absent, this still falls back to default picker behavior instead of the workspace directory context, so the issue can reproduce.
💡 Proposed fix
- if let preferredDir = self.tabManager.selectedWorkspace?.focusedPanelId.flatMap({ self.tabManager.selectedWorkspace?.panelDirectories[$0] }), !preferredDir.isEmpty {
- panel.directoryURL = URL(fileURLWithPath: preferredDir)
- }
+ if let workspace = self.tabManager.selectedWorkspace {
+ let focusedDir = workspace.focusedPanelId
+ .flatMap { workspace.panelDirectories[$0] }?
+ .trimmingCharacters(in: .whitespacesAndNewlines)
+ let workspaceDir = workspace.currentDirectory.trimmingCharacters(in: .whitespacesAndNewlines)
+
+ if let focusedDir, !focusedDir.isEmpty, FileManager.default.fileExists(atPath: focusedDir) {
+ panel.directoryURL = URL(fileURLWithPath: focusedDir, isDirectory: true)
+ } else if !workspaceDir.isEmpty, FileManager.default.fileExists(atPath: workspaceDir) {
+ panel.directoryURL = URL(fileURLWithPath: workspaceDir, isDirectory: true)
+ } else {
+ panel.directoryURL = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first
+ }
+ }📝 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.
| if let preferredDir = self.tabManager.selectedWorkspace?.focusedPanelId.flatMap({ self.tabManager.selectedWorkspace?.panelDirectories[$0] }), !preferredDir.isEmpty { | |
| panel.directoryURL = URL(fileURLWithPath: preferredDir) | |
| } | |
| if let workspace = self.tabManager.selectedWorkspace { | |
| let focusedDir = workspace.focusedPanelId | |
| .flatMap { workspace.panelDirectories[$0] }? | |
| .trimmingCharacters(in: .whitespacesAndNewlines) | |
| let workspaceDir = workspace.currentDirectory.trimmingCharacters(in: .whitespacesAndNewlines) | |
| if let focusedDir, !focusedDir.isEmpty, FileManager.default.fileExists(atPath: focusedDir) { | |
| panel.directoryURL = URL(fileURLWithPath: focusedDir, isDirectory: true) | |
| } else if !workspaceDir.isEmpty, FileManager.default.fileExists(atPath: workspaceDir) { | |
| panel.directoryURL = URL(fileURLWithPath: workspaceDir, isDirectory: true) | |
| } else { | |
| panel.directoryURL = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ContentView.swift` around lines 5978 - 5980, When setting
panel.directoryURL in ContentView, prefer the focused panel directory but fall
back to the workspace-level directory before letting NSOpenPanel use its
default: update the logic around tabManager.selectedWorkspace?.focusedPanelId
and panelDirectories so that if preferredDir is nil or empty you attempt to read
the workspace directory (e.g. tabManager.selectedWorkspace?.directory or similar
workspace-level path property) and set panel.directoryURL = URL(fileURLWithPath:
workspaceDir) when present; only if both focused panel and workspace directory
are absent should you leave the panel to use NSOpenPanel's default.
|
This is BillionClaw. Happy to discuss the approach or make adjustments to the fix. |
|
Thanks for the contribution! This has been addressed by #2034 which was just merged. |
2 participants
Before: ⌘O always opened the file picker at ~/Documents (NSOpenPanel default).
After: ⌘O now opens the picker at the focused pane's current working directory when available, falling back to ~/Documents otherwise.\n\nRoot cause: Both the menu bar and command palette Open Folder handlers created NSOpenPanel without setting
directoryURL, so macOS defaulted to ~/Documents.\n\nFixes: #2010Summary by cubic
⌘O now opens the folder picker at the focused pane’s working directory, falling back to ~/Documents when no directory is available. Applies to both the menu bar and command palette.
Written for commit 1403491. Summary will update on new commits.
Summary by CodeRabbit