Repository navigation
Summarize multi-workspace close confirmation #1329
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.
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
Changes from all commits
3e90755
f0d15b9
13353b4
c8770d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7741,6 +7741,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent | |
| // equivalents working and avoid surprising actions while the confirmation is up. | ||
| let closeConfirmationTitles = [ | ||
| String(localized: "dialog.closeWorkspace.title", defaultValue: "Close workspace?"), | ||
| String(localized: "dialog.closeWorkspaces.title", defaultValue: "Close workspaces?"), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Gate Cmd+D handling on the dialog’s Prompt for AI agents |
||
| String(localized: "dialog.closeTab.title", defaultValue: "Close tab?"), | ||
| String(localized: "dialog.closeOtherTabs.title", defaultValue: "Close other tabs?"), | ||
| String(localized: "dialog.closeWindow.title", defaultValue: "Close window?"), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1330,6 +1330,7 @@ struct ContentView: View { | |
| @State private var retiringWorkspaceId: UUID? | ||
| @State private var workspaceHandoffGeneration: UInt64 = 0 | ||
| @State private var workspaceHandoffFallbackTask: Task<Void, Never>? | ||
| @State private var didApplyUITestSidebarSelection = false | ||
| @State private var titlebarThemeGeneration: UInt64 = 0 | ||
| @State private var sidebarDraggedTabId: UUID? | ||
| @State private var titlebarTextUpdateCoalescer = NotificationBurstCoalescer(delay: 1.0 / 30.0) | ||
|
|
@@ -2233,6 +2234,8 @@ struct ContentView: View { | |
| selectedTabIds = [selectedId] | ||
| lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == selectedId } | ||
| } | ||
| syncSidebarSelectedWorkspaceIds() | ||
| applyUITestSidebarSelectionIfNeeded(tabs: tabManager.tabs) | ||
| updateTitlebarText() | ||
|
|
||
| // Startup recovery (#399): if session restore or a race condition leaves the | ||
|
|
@@ -2267,6 +2270,9 @@ struct ContentView: View { | |
| didRecover = true | ||
| } | ||
|
|
||
| syncSidebarSelectedWorkspaceIds() | ||
| applyUITestSidebarSelectionIfNeeded(tabs: tabManager.tabs) | ||
|
|
||
| if didRecover { | ||
| #if DEBUG | ||
| dlog("startup.recovery tabCount=\(tabManager.tabs.count) selected=\(tabManager.selectedTabId?.uuidString.prefix(8) ?? "nil") mounted=\(mountedWorkspaceIds.count)") | ||
|
|
@@ -2302,6 +2308,10 @@ struct ContentView: View { | |
| updateTitlebarText() | ||
| }) | ||
|
|
||
| view = AnyView(view.onChange(of: selectedTabIds) { _ in | ||
| syncSidebarSelectedWorkspaceIds() | ||
| }) | ||
|
|
||
| view = AnyView(view.onChange(of: tabManager.isWorkspaceCycleHot) { _ in | ||
| #if DEBUG | ||
| if let snapshot = tabManager.debugCurrentWorkspaceSwitchSnapshot() { | ||
|
|
@@ -2401,6 +2411,8 @@ struct ContentView: View { | |
| lastSidebarSelectionIndex = nil | ||
| } | ||
| } | ||
| syncSidebarSelectedWorkspaceIds() | ||
| applyUITestSidebarSelectionIfNeeded(tabs: tabs) | ||
| }) | ||
|
|
||
| view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: SidebarDragLifecycleNotification.stateDidChange)) { notification in | ||
|
|
@@ -5962,11 +5974,7 @@ struct ContentView: View { | |
| } | ||
|
|
||
| private func closeWorkspaceIds(_ workspaceIds: [UUID], allowPinned: Bool) { | ||
| for workspaceId in workspaceIds { | ||
| guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { continue } | ||
| guard allowPinned || !workspace.isPinned else { continue } | ||
| tabManager.closeWorkspaceWithConfirmation(workspace) | ||
| } | ||
| tabManager.closeWorkspacesWithConfirmation(workspaceIds, allowPinned: allowPinned) | ||
| } | ||
|
|
||
| private func closeOtherSelectedWorkspaces() { | ||
|
|
@@ -5976,19 +5984,53 @@ struct ContentView: View { | |
| } | ||
|
|
||
| private func closeSelectedWorkspacesBelow() { | ||
| guard let workspace = tabManager.selectedWorkspace, | ||
| guard tabManager.selectedWorkspace != nil, | ||
| let anchorIndex = selectedWorkspaceIndex() else { return } | ||
| let workspaceIds = tabManager.tabs.suffix(from: anchorIndex + 1).map(\.id) | ||
| closeWorkspaceIds(workspaceIds, allowPinned: false) | ||
| } | ||
|
|
||
| private func closeSelectedWorkspacesAbove() { | ||
| guard let workspace = tabManager.selectedWorkspace, | ||
| guard tabManager.selectedWorkspace != nil, | ||
| let anchorIndex = selectedWorkspaceIndex() else { return } | ||
| let workspaceIds = tabManager.tabs.prefix(upTo: anchorIndex).map(\.id) | ||
| closeWorkspaceIds(workspaceIds, allowPinned: false) | ||
| } | ||
|
|
||
| private func syncSidebarSelectedWorkspaceIds() { | ||
| tabManager.setSidebarSelectedWorkspaceIds(selectedTabIds) | ||
| } | ||
|
|
||
| private func applyUITestSidebarSelectionIfNeeded(tabs: [Workspace]) { | ||
| #if DEBUG | ||
| guard !didApplyUITestSidebarSelection else { return } | ||
| let env = ProcessInfo.processInfo.environment | ||
| guard let rawValue = env["CMUX_UI_TEST_SIDEBAR_SELECTED_WORKSPACE_INDICES"]? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !rawValue.isEmpty else { | ||
| return | ||
| } | ||
|
|
||
| var indices: [Int] = [] | ||
| for token in rawValue.split(separator: ",") { | ||
| let trimmed = token.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard let index = Int(trimmed), index >= 0 else { return } | ||
| if !indices.contains(index) { | ||
| indices.append(index) | ||
| } | ||
| } | ||
|
|
||
| guard let lastIndex = indices.last, !indices.isEmpty, lastIndex < tabs.count else { return } | ||
|
|
||
| let selectedIds = Set(indices.map { tabs[$0].id }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Validate every parsed workspace index before mapping into Prompt for AI agents
Comment on lines
+6023
to
+6025
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
| selectedTabIds = selectedIds | ||
|
Comment on lines
+6004
to
+6026
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate all parsed UI-test indices before subscripting Line 6023 only validates 💡 Proposed fix- guard let lastIndex = indices.last, !indices.isEmpty, lastIndex < tabs.count else { return }
+ guard !indices.isEmpty else { return }
+ guard indices.allSatisfy({ $0 < tabs.count }) else { return }
+ guard let lastIndex = indices.last else { return }
let selectedIds = Set(indices.map { tabs[$0].id })🤖 Prompt for AI Agents |
||
| lastSidebarSelectionIndex = lastIndex | ||
| tabManager.selectWorkspace(tabs[lastIndex]) | ||
| sidebarSelectionState.selection = .tabs | ||
| didApplyUITestSidebarSelection = true | ||
| #endif | ||
| } | ||
|
|
||
| private func beginRenameWorkspaceFlow() { | ||
| guard let workspace = tabManager.selectedWorkspace else { | ||
| NSSound.beep() | ||
|
|
@@ -10289,16 +10331,7 @@ private struct TabItemView: View, Equatable { | |
| } | ||
|
|
||
| private func closeTabs(_ targetIds: [UUID], allowPinned: Bool) { | ||
| let idsToClose = targetIds.filter { id in | ||
| guard let tab = tabManager.tabs.first(where: { $0.id == id }) else { return false } | ||
| return allowPinned || !tab.isPinned | ||
| } | ||
| for id in idsToClose { | ||
| if let tab = tabManager.tabs.first(where: { $0.id == id }) { | ||
| tabManager.closeWorkspaceWithConfirmation(tab) | ||
| } | ||
| } | ||
| selectedTabIds.subtract(idsToClose) | ||
| tabManager.closeWorkspacesWithConfirmation(targetIds, allowPinned: allowPinned) | ||
| syncSelectionAfterMutation() | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including
dialog.closeWorkspaces.titleincloseConfirmationTitlescauseshandleCustomShortcutto routeCmd+Dto the alert's Close button unconditionally (thematchShortcut(... "d")branch), even when the alert did not opt intoCmd+D. For non-window multi-workspace closes,closeWorkspacesPlansetsacceptCmdDtofalse, so pressingCmd+Don that new summary dialog can unexpectedly confirm a destructive close instead of doing nothing.Useful? React with 👍 / 👎.