From 9eefc80e32b94e6fd84ea78fc321114cc8e52068 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 13 Mar 2026 04:16:59 -0700 Subject: [PATCH] Share last-surface close handling across close actions --- Resources/Localizable.xcstrings | 12 ++++++------ Sources/TabManager.swift | 24 ++++++++++-------------- Sources/Workspace.swift | 20 ++++++++++++++++++++ Sources/cmuxApp.swift | 9 +++++---- 4 files changed, 41 insertions(+), 24 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 4a57d594..d25a5104 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -41188,13 +41188,13 @@ "en": { "stringUnit": { "state": "translated", - "value": "Cmd+W Closes Workspace on Last Surface" + "value": "Closing Last Surface Closes Workspace" } }, "ja": { "stringUnit": { "state": "translated", - "value": "最後のサーフェスでCmd+Wがワークスペースも閉じる" + "value": "最後のサーフェスを閉じるとワークスペースも閉じる" } } } @@ -41205,13 +41205,13 @@ "en": { "stringUnit": { "state": "translated", - "value": "When the focused surface is the last one in its workspace, Cmd+W closes only the surface and keeps the workspace open." + "value": "Closing the last surface keeps the workspace open. Use Cmd+Shift+W to close a workspace explicitly." } }, "ja": { "stringUnit": { "state": "translated", - "value": "フォーカス中のサーフェスがそのワークスペースの最後の1つでも、Cmd+Wはサーフェスだけを閉じ、ワークスペースは残します。" + "value": "最後のサーフェスを閉じてもワークスペースは残ります。ワークスペースを明示的に閉じるにはCmd+Shift+Wを使います。" } } } @@ -41222,13 +41222,13 @@ "en": { "stringUnit": { "state": "translated", - "value": "When the focused surface is the last one in its workspace, Cmd+W also closes the workspace." + "value": "Closing the last surface also closes its workspace." } }, "ja": { "stringUnit": { "state": "translated", - "value": "フォーカス中のサーフェスがそのワークスペースの最後の1つなら、Cmd+Wでワークスペースも閉じます。" + "value": "最後のサーフェスを閉じると、そのワークスペースも閉じます。" } } } diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index c0829e4a..bd4dad61 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -909,6 +909,7 @@ class TabManager: ObservableObject { portOrdinal: ordinal, configTemplate: inheritedConfig ) + newWorkspace.owningTabManager = self wireClosedBrowserTracking(for: newWorkspace) let insertIndex = newTabInsertIndex(placementOverride: placementOverride) if insertIndex >= 0 && insertIndex <= tabs.count { @@ -1357,6 +1358,7 @@ class TabManager: ObservableObject { AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: workspace.id) unwireClosedBrowserTracking(for: workspace) workspace.teardownAllPanels() + workspace.owningTabManager = nil tabs.remove(at: index) @@ -1379,6 +1381,7 @@ class TabManager: ObservableObject { let removed = tabs.remove(at: index) unwireClosedBrowserTracking(for: removed) + removed.owningTabManager = nil lastFocusedPanelByTab.removeValue(forKey: removed.id) if tabs.isEmpty { @@ -1397,6 +1400,7 @@ class TabManager: ObservableObject { /// Attach an existing workspace to this window. func attachWorkspace(_ workspace: Workspace, at index: Int? = nil, select: Bool = true) { + workspace.owningTabManager = self wireClosedBrowserTracking(for: workspace) let insertIndex: Int = { guard let index else { return tabs.count } @@ -1661,10 +1665,6 @@ class TabManager: ObservableObject { } } - private func shouldCloseWorkspaceOnLastSurfaceShortcut(_ workspace: Workspace) -> Bool { - LastSurfaceCloseShortcutSettings.closesWorkspace() && workspace.panels.count <= 1 - } - private func closePanelWithConfirmation(tab: Workspace, panelId: UUID) { guard tab.panels[panelId] != nil else { #if DEBUG @@ -1690,19 +1690,13 @@ class TabManager: ObservableObject { "surface.close.shortcut.begin tab=\(tab.id.uuidString.prefix(5)) " + "panel=\(panelId.uuidString.prefix(5)) kind=\(panelKind) " + "panelCount=\(tab.panels.count) bonsplitTabs=\(bonsplitTabCount) " + - "closeWorkspaceOnLastSurface=\(shouldCloseWorkspaceOnLastSurfaceShortcut(tab) ? 1 : 0)" + "closeWorkspaceOnLastSurfaceSetting=\(LastSurfaceCloseShortcutSettings.closesWorkspace() ? 1 : 0)" ) #endif - // Cmd+W should match Bonsplit's tab close button semantics by default. Users can opt - // back into the legacy behavior where closing the last surface also closes its workspace. - if shouldCloseWorkspaceOnLastSurfaceShortcut(tab) { - closeWorkspaceIfRunningProcess(tab) - return - } - - // Route the default Cmd+W path through Bonsplit/Workspace close handling so it matches - // the tab close button behavior, including shared confirmation and replacement-panel flow. + // Route Cmd+W through Bonsplit/Workspace close handling so it matches the tab close + // button, including shared confirmation, setting-controlled last-surface behavior, and + // replacement-panel flow. let closed = tab.closePanel(panelId) #if DEBUG dlog( @@ -4006,6 +4000,7 @@ extension TabManager { workingDirectory: workspaceSnapshot.currentDirectory, portOrdinal: ordinal ) + workspace.owningTabManager = self workspace.restoreSessionSnapshot(workspaceSnapshot) wireClosedBrowserTracking(for: workspace) newTabs.append(workspace) @@ -4015,6 +4010,7 @@ extension TabManager { let ordinal = Self.nextPortOrdinal Self.nextPortOrdinal += 1 let fallback = Workspace(title: "Terminal 1", portOrdinal: ordinal) + fallback.owningTabManager = self wireClosedBrowserTracking(for: fallback) newTabs.append(fallback) } diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 6564e457..8a26b3fd 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -946,6 +946,7 @@ final class Workspace: Identifiable, ObservableObject { /// Callback used by TabManager to capture recently closed browser panels for Cmd+Shift+T restore. var onClosedBrowserPanel: ((ClosedBrowserPanelRestoreSnapshot) -> Void)? + weak var owningTabManager: TabManager? // Closing tabs mutates split layout immediately; terminal views handle their own AppKit @@ -4114,6 +4115,19 @@ final class Workspace: Identifiable, ObservableObject { // MARK: - BonsplitDelegate extension Workspace: BonsplitDelegate { + @MainActor + private func shouldCloseWorkspaceOnLastSurface(for tabId: TabID) -> Bool { + let manager = owningTabManager ?? AppDelegate.shared?.tabManagerFor(tabId: id) ?? AppDelegate.shared?.tabManager + guard LastSurfaceCloseShortcutSettings.closesWorkspace(), + panels.count <= 1, + panelIdFromSurfaceId(tabId) != nil, + let manager, + manager.tabs.contains(where: { $0.id == id }) else { + return false + } + return true + } + @MainActor private func confirmClosePanel(for tabId: TabID) async -> Bool { let alert = NSAlert() @@ -4529,6 +4543,12 @@ extension Workspace: BonsplitDelegate { return false } + if shouldCloseWorkspaceOnLastSurface(for: tab.id) { + clearStagedClosedBrowserRestoreSnapshot(for: tab.id) + owningTabManager?.closeWorkspaceWithConfirmation(self) + return false + } + // Check if the panel needs close confirmation guard let panelId = panelIdFromSurfaceId(tab.id), let terminalPanel = terminalPanel(for: panelId) else { diff --git a/Sources/cmuxApp.swift b/Sources/cmuxApp.swift index 46e76f6b..24152007 100644 --- a/Sources/cmuxApp.swift +++ b/Sources/cmuxApp.swift @@ -454,7 +454,8 @@ struct cmuxApp: App { // Terminal semantics: // Cmd+W closes the focused tab/surface (with confirmation if needed) and keeps // the workspace open by default. Cmd+Shift+W is the explicit workspace-close - // action, unless the user opts back into the legacy last-surface Cmd+W behavior. + // action, unless the user opts into closing the workspace when its last surface + // is closed. Button(String(localized: "menu.file.closeTab", defaultValue: "Close Tab")) { closePanelOrWindow() } @@ -3127,12 +3128,12 @@ struct SettingsView: View { if closeWorkspaceOnLastSurfaceShortcut { return String( localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOn", - defaultValue: "When the focused surface is the last one in its workspace, Cmd+W also closes the workspace." + defaultValue: "Closing the last surface also closes its workspace." ) } return String( localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOff", - defaultValue: "When the focused surface is the last one in its workspace, Cmd+W closes only the surface and keeps the workspace open." + defaultValue: "Closing the last surface keeps the workspace open. Use Cmd+Shift+W to close a workspace explicitly." ) } @@ -3504,7 +3505,7 @@ struct SettingsView: View { SettingsCardDivider() SettingsCardRow( - String(localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut", defaultValue: "Cmd+W Closes Workspace on Last Surface"), + String(localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut", defaultValue: "Closing Last Surface Closes Workspace"), subtitle: closeWorkspaceOnLastSurfaceShortcutSubtitle ) { Toggle("", isOn: $closeWorkspaceOnLastSurfaceShortcut)