Skip to content

Handle Cmd+O in handleCustomShortcut to prevent Documents folder open #2034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lawrencecchen merged 4 commits into from
Mar 25, 2026
Merged

Handle Cmd+O in handleCustomShortcut to prevent Documents folder open #2034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d4ffcb
Handle Cmd+O in handleCustomShortcut to prevent Documents folder open
Mar 24, 2026
553d35c
Fix fallback directory loss and deduplicate Open Folder logic
Mar 24, 2026
d3538d4
Set NSOpenPanel directoryURL to current terminal working directory
Mar 24, 2026
43d8469
Use shared main-window resolver and openWorkspaceForExternalDirectory…
lawrencecchen Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions Sources/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5363,6 +5363,30 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
_ = createMainWindow()
}

/// Shows the "Open Folder" panel and creates a workspace for the selected directory.
/// Called from both the SwiftUI menu and `handleCustomShortcut`.
func showOpenFolderPanel() {
let panel = NSOpenPanel()
panel.canChooseFiles = false
panel.canChooseDirectories = true
panel.allowsMultipleSelection = false
panel.title = String(localized: "menu.file.openFolder.panelTitle", defaultValue: "Open Folder")
panel.prompt = String(localized: "menu.file.openFolder.panelPrompt", defaultValue: "Open")
// Seed the panel with the active workspace's directory. Use the shared
// main-window resolver so this works even when an auxiliary window is key.
if let context = preferredMainWindowContextForWorkspaceCreation(debugSource: "openFolderPanel.seed"),
let cwd = context.tabManager.selectedWorkspace?.currentDirectory,
!cwd.isEmpty {
panel.directoryURL = URL(fileURLWithPath: cwd)
}
if panel.runModal() == .OK, let url = panel.url {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
openWorkspaceForExternalDirectory(
workingDirectory: url.path,
debugSource: "shortcut.openFolder"
)
Comment on lines +5382 to +5386

@coderabbitai coderabbitai Bot Mar 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Mark confirmed folder picks as explicit startup opens.

If this runs before the initial session-restore attempt and there is no main window yet, openWorkspaceForExternalDirectory(...) can fall back to createMainWindow(initialWorkingDirectory:), and registerMainWindow(...) will still restore the saved session over the selected folder. Call prepareForExplicitOpenIntentAtStartup() only after panel.url is confirmed.

💡 Suggested fix 
         if panel.runModal() == .OK, let url = panel.url {
+            prepareForExplicitOpenIntentAtStartup()
             openWorkspaceForExternalDirectory(
                 workingDirectory: url.path,
                 debugSource: "shortcut.openFolder"
             )
         }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/AppDelegate.swift` around lines 5382 - 5386, The code currently marks
folder picks as explicit startup opens too early; only mark them after the user
actually confirmed a URL. Move the call to
prepareForExplicitOpenIntentAtStartup() so it runs after you verify panel.url
(i.e., inside the if where panel.runModal() == .OK and let url = panel.url) and
before calling openWorkspaceForExternalDirectory(workingDirectory:debugSource:);
this ensures openWorkspaceForExternalDirectory can still fall back to
createMainWindow(initialWorkingDirectory:) and that registerMainWindow will not
overwrite the confirmed explicit open with the saved session.

}
}
Comment on lines +5366 to +5388

@greptile-apps greptile-apps Bot Mar 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Docstring overstates extraction scope; SwiftUI menu still has its own inline copy

The doc comment says "Extracted so it can be called from both the SwiftUI menu and handleCustomShortcut", but cmuxApp.swift (lines 589–608) was not updated to call showOpenFolderPanel(). The inline panel setup there is byte-for-byte identical to this new function, so the extraction is only half-done and the docstring is currently inaccurate.

This means there are now two divergent code paths for the same action:

  • AppDelegate.showOpenFolderPanel() → called from handleCustomShortcut (this PR)
  • Inline NSOpenPanel block in cmuxApp.swift → still called from the SwiftUI menu

A future change to panel titles, localization keys, or the addWorkspaceInPreferredMainWindow fallback would need to be applied in both places. Consider either:

  1. Updating cmuxApp.swift to call AppDelegate.shared?.showOpenFolderPanel() so there is one canonical implementation, or
  2. Updating the docstring to reflect that this function is only for the handleCustomShortcut path, e.g. "Shows the 'Open Folder' panel when the shortcut is triggered via handleCustomShortcut."

@lawrencecchen lawrencecchen Mar 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 43d8469. The docstring is now accurate (cmuxApp.swift already delegates to showOpenFolderPanel() since 553d35c), and the method now uses openWorkspaceForExternalDirectory for consistent behavior.


@objc func openWindow(
_ pasteboard: NSPasteboard,
userData: String?,
Expand Down Expand Up @@ -9309,6 +9333,14 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return true
}

// Open Folder: Cmd+O
// Handled here to prevent AppKit's default NSDocumentController from opening
// the Documents folder when SwiftUI menu dispatch fails due to focus bugs.
if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .openFolder)) {
showOpenFolderPanel()
return true
}

// Check Show Notifications shortcut
if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .showNotifications)) {
toggleNotificationsPopover(animated: false, anchorView: fullscreenControlsViewModel?.notificationsAnchorView)
Expand Down
19 changes: 1 addition & 18 deletions Sources/cmuxApp.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,24 +587,7 @@ struct cmuxApp: App {
}

splitCommandButton(title: String(localized: "menu.file.openFolder", defaultValue: "Open Folder…"), shortcut: openFolderMenuShortcut) {
let panel = NSOpenPanel()
panel.canChooseFiles = false
panel.canChooseDirectories = true
panel.allowsMultipleSelection = false
panel.title = String(localized: "menu.file.openFolder.panelTitle", defaultValue: "Open Folder")
panel.prompt = String(localized: "menu.file.openFolder.panelPrompt", defaultValue: "Open")
if panel.runModal() == .OK, let url = panel.url {
if let appDelegate = AppDelegate.shared {
if appDelegate.addWorkspaceInPreferredMainWindow(
workingDirectory: url.path,
debugSource: "menu.openFolder"
) == nil {
appDelegate.openNewMainWindow(nil)
}
} else {
activeTabManager.addWorkspace(workingDirectory: url.path)
}
}
AppDelegate.shared?.showOpenFolderPanel()
}
}

Expand Down
Loading