diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index d1f2735f..d7c5854d 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -29814,6 +29814,40 @@ } } }, + "dialog.closePinnedWorkspace.message": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "This workspace is pinned. Closing it will close the workspace and all of its panels." + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "このワークスペースはピン留めされています。閉じると、このワークスペースとその中のすべてのパネルが閉じます。" + } + } + } + }, + "dialog.closePinnedWorkspace.title": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Close pinned workspace?" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "ピン留めされたワークスペースを閉じますか？" + } + } + } + }, "dialog.closeWorkspace.message": { "extractionState": "manual", "localizations": { @@ -62655,13 +62689,13 @@ "en": { "stringUnit": { "state": "translated", - "value": "Pinned workspace protected from closing. Unpin to close." + "value": "Pinned workspace. Closing requires confirmation." } }, "ja": { "stringUnit": { "state": "translated", - "value": "ピン留めされたワークスペースは閉じられません。閉じるにはピンを外してください。" + "value": "ピン留めされたワークスペースです。閉じるには確認が必要です。" } } } diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index c196282d..6a4963dc 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -1664,7 +1664,6 @@ struct ContentView: View { static let workspaceHasCustomName = "workspace.hasCustomName" static let workspaceMinimalModeEnabled = "workspace.minimalModeEnabled" static let workspaceShouldPin = "workspace.shouldPin" - static let workspaceCanClose = "workspace.canClose" static let workspaceHasPullRequests = "workspace.hasPullRequests" static let workspaceHasSplits = "workspace.hasSplits" static let workspaceHasPeers = "workspace.hasPeers" @@ -4950,7 +4949,6 @@ struct ContentView: View { snapshot.setString(CommandPaletteContextKeys.workspaceName, workspaceDisplayName(workspace)) snapshot.setBool(CommandPaletteContextKeys.workspaceHasCustomName, workspace.customTitle != nil) snapshot.setBool(CommandPaletteContextKeys.workspaceShouldPin, !workspace.isPinned) - snapshot.setBool(CommandPaletteContextKeys.workspaceCanClose, tabManager.canCloseWorkspace(workspace)) snapshot.setBool( CommandPaletteContextKeys.workspaceHasPullRequests, !workspace.sidebarPullRequestsInDisplayOrder().isEmpty @@ -5113,8 +5111,7 @@ struct ContentView: View { title: constant(String(localized: "command.closeWorkspace.title", defaultValue: "Close Workspace")), subtitle: constant(String(localized: "command.closeWorkspace.subtitle", defaultValue: "Workspace")), shortcutHint: "⌘⇧W", - keywords: ["close", "workspace"], - enablement: { $0.bool(CommandPaletteContextKeys.workspaceCanClose) } + keywords: ["close", "workspace"] ) ) contributions.append( @@ -6936,21 +6933,21 @@ struct ContentView: View { private func closeOtherSelectedWorkspaces() { guard let workspace = tabManager.selectedWorkspace else { return } let workspaceIds = tabManager.tabs.compactMap { $0.id == workspace.id ? nil : $0.id } - closeWorkspaceIds(workspaceIds, allowPinned: false) + closeWorkspaceIds(workspaceIds, allowPinned: true) } private func closeSelectedWorkspacesBelow() { guard tabManager.selectedWorkspace != nil, let anchorIndex = selectedWorkspaceIndex() else { return } let workspaceIds = tabManager.tabs.suffix(from: anchorIndex + 1).map(\.id) - closeWorkspaceIds(workspaceIds, allowPinned: false) + closeWorkspaceIds(workspaceIds, allowPinned: true) } private func closeSelectedWorkspacesAbove() { guard tabManager.selectedWorkspace != nil, let anchorIndex = selectedWorkspaceIndex() else { return } let workspaceIds = tabManager.tabs.prefix(upTo: anchorIndex).map(\.id) - closeWorkspaceIds(workspaceIds, allowPinned: false) + closeWorkspaceIds(workspaceIds, allowPinned: true) } private func syncSidebarSelectedWorkspaceIds() { @@ -10886,8 +10883,11 @@ private struct TabItemView: View, Equatable { let closeWorkspaceTooltip = String(localized: "sidebar.closeWorkspace.tooltip", defaultValue: "Close Workspace") let protectedWorkspaceTooltip = String( localized: "sidebar.pinnedWorkspaceProtected.tooltip", - defaultValue: "Pinned workspace protected from closing. Unpin to close." + defaultValue: "Pinned workspace. Closing requires confirmation." ) + let closeButtonTooltip = tab.isPinned + ? protectedWorkspaceTooltip + : KeyboardShortcutSettings.Action.closeWorkspace.tooltip(closeWorkspaceTooltip) let accessibilityHintText = String(localized: "sidebar.workspace.accessibilityHint", defaultValue: "Activate to focus this workspace. Drag to reorder, or use Move Up and Move Down actions.") let moveUpActionText = String(localized: "sidebar.workspace.moveUpAction", defaultValue: "Move Up") let moveDownActionText = String(localized: "sidebar.workspace.moveDownAction", defaultValue: "Move Down") @@ -10962,30 +10962,21 @@ private struct TabItemView: View, Equatable { Spacer(minLength: 0) ZStack(alignment: .trailing) { - if tab.isPinned { - Image(systemName: "lock.fill") + Button(action: { + #if DEBUG + dlog("sidebar.close workspace=\(tab.id.uuidString.prefix(5)) method=button") + #endif + tabManager.closeWorkspaceWithConfirmation(tab) + }) { + Image(systemName: "xmark") .font(.system(size: 9, weight: .medium)) - .foregroundColor(activeSecondaryColor(0.65)) - .safeHelp(protectedWorkspaceTooltip) - .frame(width: SidebarTrailingAccessoryWidthPolicy.closeButtonWidth, height: 16, alignment: .center) - .opacity(showCloseButton && !showsWorkspaceShortcutHint ? 1 : 0) - } else { - Button(action: { - #if DEBUG - dlog("sidebar.close workspace=\(tab.id.uuidString.prefix(5)) method=button") - #endif - tabManager.closeWorkspaceWithConfirmation(tab) - }) { - Image(systemName: "xmark") - .font(.system(size: 9, weight: .medium)) - .foregroundColor(activeSecondaryColor(0.7)) - } - .buttonStyle(.plain) - .safeHelp(KeyboardShortcutSettings.Action.closeWorkspace.tooltip(closeWorkspaceTooltip)) - .frame(width: SidebarTrailingAccessoryWidthPolicy.closeButtonWidth, height: 16, alignment: .center) - .opacity(showCloseButton && !showsWorkspaceShortcutHint ? 1 : 0) - .allowsHitTesting(showCloseButton && !showsWorkspaceShortcutHint) + .foregroundColor(activeSecondaryColor(0.7)) } + .buttonStyle(.plain) + .safeHelp(closeButtonTooltip) + .frame(width: SidebarTrailingAccessoryWidthPolicy.closeButtonWidth, height: 16, alignment: .center) + .opacity(showCloseButton && !showsWorkspaceShortcutHint ? 1 : 0) + .allowsHitTesting(showCloseButton && !showsWorkspaceShortcutHint) if showsWorkspaceShortcutHint, let workspaceShortcutLabel { Text(workspaceShortcutLabel) @@ -11306,10 +11297,6 @@ private struct TabItemView: View, Equatable { multi: String(localized: "contextMenu.closeWorkspaces", defaultValue: "Close Workspaces"), single: String(localized: "contextMenu.closeWorkspace", defaultValue: "Close Workspace"), isMulti: isMulti) - let hasClosableTargets = targetIds.contains { workspaceId in - guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { return false } - return tabManager.canCloseWorkspace(workspace) - } let markReadLabel = contextMenuLabel( multi: String(localized: "contextMenu.markWorkspacesRead", defaultValue: "Mark Workspaces as Read"), single: String(localized: "contextMenu.markWorkspaceRead", defaultValue: "Mark Workspace as Read"), @@ -11448,15 +11435,15 @@ private struct TabItemView: View, Equatable { if let key = closeWorkspaceShortcut.keyEquivalent { Button(closeLabel) { - closeTabs(targetIds, allowPinned: false) + closeTabs(targetIds, allowPinned: true) } .keyboardShortcut(key, modifiers: closeWorkspaceShortcut.eventModifiers) - .disabled(!hasClosableTargets) + .disabled(targetIds.isEmpty) } else { Button(closeLabel) { - closeTabs(targetIds, allowPinned: false) + closeTabs(targetIds, allowPinned: true) } - .disabled(!hasClosableTargets) + .disabled(targetIds.isEmpty) } Button(String(localized: "contextMenu.closeOtherWorkspaces", defaultValue: "Close Other Workspaces")) { @@ -11620,19 +11607,19 @@ private struct TabItemView: View, Equatable { private func closeOtherTabs(_ targetIds: [UUID]) { let keepIds = Set(targetIds) let idsToClose = tabManager.tabs.compactMap { keepIds.contains($0.id) ? nil : $0.id } - closeTabs(idsToClose, allowPinned: false) + closeTabs(idsToClose, allowPinned: true) } private func closeTabsBelow(tabId: UUID) { guard let anchorIndex = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return } let idsToClose = tabManager.tabs.suffix(from: anchorIndex + 1).map { $0.id } - closeTabs(idsToClose, allowPinned: false) + closeTabs(idsToClose, allowPinned: true) } private func closeTabsAbove(tabId: UUID) { guard let anchorIndex = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return } let idsToClose = tabManager.tabs.prefix(upTo: anchorIndex).map { $0.id } - closeTabs(idsToClose, allowPinned: false) + closeTabs(idsToClose, allowPinned: true) } private func markTabsRead(_ targetIds: [UUID]) { diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 26bb2ae5..a7dcfd99 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -2201,7 +2201,7 @@ class TabManager: ObservableObject { #endif let sidebarSelectionIds = orderedSidebarSelectedWorkspaceIds() if sidebarSelectionIds.count > 1 { - closeWorkspacesWithConfirmation(sidebarSelectionIds, allowPinned: false) + closeWorkspacesWithConfirmation(sidebarSelectionIds, allowPinned: true) return } guard let selectedId = selectedTabId, @@ -2215,9 +2215,19 @@ class TabManager: ObservableObject { @discardableResult func closeWorkspaceWithConfirmation(_ workspace: Workspace) -> Bool { - guard canCloseWorkspace(workspace) else { - signalProtectedWorkspaceCloseAttempt(workspace) - return false + if workspace.isPinned { + guard confirmClose( + title: String(localized: "dialog.closePinnedWorkspace.title", defaultValue: "Close pinned workspace?"), + message: String( + localized: "dialog.closePinnedWorkspace.message", + defaultValue: "This workspace is pinned. Closing it will close the workspace and all of its panels." + ), + acceptCmdD: tabs.count <= 1 + ) else { + return false + } + closeWorkspaceIfRunningProcess(workspace, requiresConfirmation: false) + return true } closeWorkspaceIfRunningProcess(workspace) return true @@ -2236,15 +2246,7 @@ class TabManager: ObservableObject { func closeWorkspacesWithConfirmation(_ workspaceIds: [UUID], allowPinned: Bool) { let workspaces = orderedClosableWorkspaces(workspaceIds, allowPinned: allowPinned) - guard !workspaces.isEmpty else { - if !allowPinned, - workspaceIds.contains(where: { id in - tabs.first(where: { $0.id == id })?.isPinned == true - }) { - signalProtectedWorkspaceCloseAttempt() - } - return - } + guard !workspaces.isEmpty else { return } guard workspaces.count > 1 else { closeWorkspaceWithConfirmation(workspaces[0]) return @@ -2373,17 +2375,6 @@ class TabManager: ObservableObject { } } - private func signalProtectedWorkspaceCloseAttempt(_ workspace: Workspace? = nil) { -#if DEBUG - if let workspace { - dlog("workspace.close.skip workspace=\(workspace.id.uuidString.prefix(5)) reason=pinned") - } else { - dlog("workspace.close.skip reason=pinned") - } -#endif - NSSound.beep() - } - private func closeWorkspacesPlan(for workspaces: [Workspace]) -> CloseWorkspacesPlan { let willCloseWindow = workspaces.count == tabs.count let title = willCloseWindow diff --git a/Sources/cmuxApp.swift b/Sources/cmuxApp.swift index 608fa08c..9ff61772 100644 --- a/Sources/cmuxApp.swift +++ b/Sources/cmuxApp.swift @@ -644,9 +644,6 @@ struct cmuxApp: App { splitCommandButton(title: String(localized: "menu.file.closeWorkspace", defaultValue: "Close Workspace"), shortcut: closeWorkspaceMenuShortcut) { closeTabOrWindow() } - .disabled( - activeTabManager.selectedWorkspace.map { !activeTabManager.canCloseWorkspace($0) } ?? true - ) Menu(String(localized: "commandPalette.switcher.workspaceLabel", defaultValue: "Workspace")) { workspaceCommandMenuContent(manager: activeTabManager) @@ -1070,21 +1067,21 @@ struct cmuxApp: App { private func closeOtherSelectedWorkspacePeers(in manager: TabManager) { guard let workspace = manager.selectedWorkspace else { return } let workspaceIds = manager.tabs.compactMap { $0.id == workspace.id ? nil : $0.id } - closeWorkspaceIds(workspaceIds, in: manager, allowPinned: false) + closeWorkspaceIds(workspaceIds, in: manager, allowPinned: true) } private func closeSelectedWorkspacesBelow(in manager: TabManager) { guard let workspace = manager.selectedWorkspace, let anchorIndex = selectedWorkspaceIndex(in: manager, workspaceId: workspace.id) else { return } let workspaceIds = manager.tabs.suffix(from: anchorIndex + 1).map(\.id) - closeWorkspaceIds(workspaceIds, in: manager, allowPinned: false) + closeWorkspaceIds(workspaceIds, in: manager, allowPinned: true) } private func closeSelectedWorkspacesAbove(in manager: TabManager) { guard let workspace = manager.selectedWorkspace, let anchorIndex = selectedWorkspaceIndex(in: manager, workspaceId: workspace.id) else { return } let workspaceIds = manager.tabs.prefix(upTo: anchorIndex).map(\.id) - closeWorkspaceIds(workspaceIds, in: manager, allowPinned: false) + closeWorkspaceIds(workspaceIds, in: manager, allowPinned: true) } private func selectedWorkspaceHasUnreadNotifications(in manager: TabManager) -> Bool { @@ -1174,7 +1171,7 @@ struct cmuxApp: App { Button(String(localized: "menu.file.closeWorkspace", defaultValue: "Close Workspace")) { manager.closeCurrentWorkspaceWithConfirmation() } - .disabled(workspace.map { !manager.canCloseWorkspace($0) } ?? true) + .disabled(workspace == nil) Button(String(localized: "contextMenu.closeOtherWorkspaces", defaultValue: "Close Other Workspaces")) { closeOtherSelectedWorkspacePeers(in: manager) diff --git a/cmuxTests/TabManagerUnitTests.swift b/cmuxTests/TabManagerUnitTests.swift index fc5fbc1c..6f31d82d 100644 --- a/cmuxTests/TabManagerUnitTests.swift +++ b/cmuxTests/TabManagerUnitTests.swift @@ -304,7 +304,7 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase { XCTAssertTrue(secondWorkspace.panels.isEmpty) } - func testCloseCurrentPanelKeepsPinnedWorkspaceOpenWhenItOwnsTheLastSurface() { + func testCloseCurrentPanelPromptsBeforeClosingPinnedWorkspaceLastSurface() { let manager = TabManager() _ = manager.tabs[0] let pinnedWorkspace = manager.addWorkspace() @@ -319,10 +319,29 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase { XCTAssertEqual(manager.selectedTabId, pinnedWorkspace.id) XCTAssertEqual(pinnedWorkspace.panels.count, 1) + var prompts: [(title: String, message: String, acceptCmdD: Bool)] = [] + manager.confirmCloseHandler = { title, message, acceptCmdD in + prompts.append((title, message, acceptCmdD)) + return false + } + manager.closeCurrentPanelWithConfirmation() drainMainQueue() drainMainQueue() + XCTAssertEqual(prompts.count, 1) + XCTAssertEqual( + prompts.first?.title, + String(localized: "dialog.closePinnedWorkspace.title", defaultValue: "Close pinned workspace?") + ) + XCTAssertEqual( + prompts.first?.message, + String( + localized: "dialog.closePinnedWorkspace.message", + defaultValue: "This workspace is pinned. Closing it will close the workspace and all of its panels." + ) + ) + XCTAssertEqual(prompts.first?.acceptCmdD, false) XCTAssertEqual(manager.tabs.count, 2) XCTAssertTrue(manager.tabs.contains(where: { $0.id == pinnedWorkspace.id })) XCTAssertEqual(manager.selectedTabId, pinnedWorkspace.id) @@ -330,6 +349,30 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase { XCTAssertEqual(pinnedWorkspace.panels.count, 1) } + func testCloseCurrentPanelClosesPinnedWorkspaceAfterConfirmation() { + let manager = TabManager() + let firstWorkspace = manager.tabs[0] + let pinnedWorkspace = manager.addWorkspace() + manager.setPinned(pinnedWorkspace, pinned: true) + manager.selectWorkspace(pinnedWorkspace) + + guard let pinnedPanelId = pinnedWorkspace.focusedPanelId else { + XCTFail("Expected focused panel in pinned workspace") + return + } + + manager.confirmCloseHandler = { _, _, _ in true } + + manager.closeCurrentPanelWithConfirmation() + drainMainQueue() + drainMainQueue() + + XCTAssertEqual(manager.tabs.map(\.id), [firstWorkspace.id]) + XCTAssertEqual(manager.selectedTabId, firstWorkspace.id) + XCTAssertNil(pinnedWorkspace.panels[pinnedPanelId]) + XCTAssertTrue(pinnedWorkspace.panels.isEmpty) + } + func testCloseCurrentPanelKeepsWorkspaceOpenWhenKeepWorkspaceOpenPreferenceIsEnabled() { let defaults = UserDefaults.standard let originalSetting = defaults.object(forKey: lastSurfaceCloseShortcutDefaultsKey)