marketing-shibata50/cmux
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Keep workspaces open when closing the last surface (#1315)

Browse source 
* Add last-surface close regression tests

* Keep workspaces open when closing last surface

* Add Cmd+W last-surface close setting

* Share Cmd+W surface-close path
This commit is contained in:
Lawrence Chen 2026-03-13 03:58:07 -07:00 • committed by GitHub
parent 2596f78380
commit fe7ef33fea
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 335 additions and 80 deletions
Download patch file Download diff file Expand all files Collapse all files

51
Resources/Localizable.xcstrings
View file

@ -41182,6 +41182,57 @@
}
}
},
"settings.app.closeWorkspaceOnLastSurfaceShortcut": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Cmd+W Closes Workspace on Last Surface"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "最後のサーフェスでCmd+Wがワークスペースも閉じる"
}
}
}
},
"settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOff": {
"extractionState": "manual",
"localizations": {
"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."
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "フォーカス中のサーフェスがそのワークスペースの最後の1つでも、Cmd+Wはサーフェスだけを閉じ、ワークスペースは残します。"
}
}
}
},
"settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOn": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "When the focused surface is the last one in its workspace, Cmd+W also closes the workspace."
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "フォーカス中のサーフェスがそのワークスペースの最後の1つなら、Cmd+Wでワークスペースも閉じます。"
}
}
}
},
"settings.app.openSidebarPRLinks": {
"extractionState": "manual",
"localizations": {

100
Sources/TabManager.swift
View file

@ -51,6 +51,18 @@ enum WorkspaceAutoReorderSettings {
}
}
enum LastSurfaceCloseShortcutSettings {
static let key = "closeWorkspaceOnLastSurfaceShortcut"
static let defaultValue = false
static func closesWorkspace(defaults: UserDefaults = .standard) -> Bool {
if defaults.object(forKey: key) == nil {
return defaultValue
}
return defaults.bool(forKey: key)
}
}
enum SidebarBranchLayoutSettings {
static let key = "sidebarBranchVerticalLayout"
static let defaultVerticalLayout = true
@ -1649,7 +1661,21 @@ 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
dlog(
"surface.close.shortcut.skip tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=missingPanel"
)
#endif
return
}
let bonsplitTabCount = tab.bonsplitController.allPaneIds.reduce(0) { partial, paneId in
partial + tab.bonsplitController.tabs(inPane: paneId).count
}
@ -1663,77 +1689,21 @@ class TabManager: ObservableObject {
dlog(
"surface.close.shortcut.begin tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) kind=\(panelKind) " +
"panelCount=\(tab.panels.count) bonsplitTabs=\(bonsplitTabCount)"
"panelCount=\(tab.panels.count) bonsplitTabs=\(bonsplitTabCount) " +
"closeWorkspaceOnLastSurface=\(shouldCloseWorkspaceOnLastSurfaceShortcut(tab) ? 1 : 0)"
)
#endif
// Cmd+W closes the focused Bonsplit tab (a "tab" in the UI). When the workspace only has
// a single tab left, closing it should close the workspace (and possibly the window),
// rather than creating a replacement terminal.
let effectiveSurfaceCount = max(tab.panels.count, bonsplitTabCount)
let isLastTabInWorkspace = effectiveSurfaceCount <= 1
if isLastTabInWorkspace {
let willCloseWindow = tabs.count <= 1
let needsConfirm = workspaceNeedsConfirmClose(tab)
if needsConfirm {
let message = willCloseWindow
? String(localized: "dialog.closeLastTabWindow.message", defaultValue: "This will close the last tab and close the window.")
: String(localized: "dialog.closeLastTabWorkspace.message", defaultValue: "This will close the last tab and close its workspace.")
#if DEBUG
dlog(
"surface.close.shortcut.confirm tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=lastTab"
)
#endif
guard confirmClose(
title: String(localized: "dialog.closeTab.title", defaultValue: "Close tab?"),
message: message,
acceptCmdD: willCloseWindow
) else {
#if DEBUG
dlog(
"surface.close.shortcut.cancel tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=lastTabConfirmDismissed"
)
#endif
return
}
}
AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: tab.id)
if willCloseWindow {
AppDelegate.shared?.closeMainWindowContainingTabId(tab.id)
} else {
closeWorkspace(tab)
}
// 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
}
if let terminalPanel = tab.terminalPanel(for: panelId),
terminalPanel.needsConfirmClose() {
#if DEBUG
dlog(
"surface.close.shortcut.confirm tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=terminalNeedsConfirm"
)
#endif
guard confirmClose(
title: String(localized: "dialog.closeTab.title", defaultValue: "Close tab?"),
message: String(localized: "dialog.closeTab.message", defaultValue: "This will close the current tab."),
acceptCmdD: false
) else {
#if DEBUG
dlog(
"surface.close.shortcut.cancel tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=terminalConfirmDismissed"
)
#endif
return
}
}
// We already confirmed (if needed); bypass Bonsplit's delegate gating.
let closed = tab.closePanel(panelId, force: true)
// 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.
let closed = tab.closePanel(panelId)
#if DEBUG
dlog(
"surface.close.shortcut tab=\(tab.id.uuidString.prefix(5)) " +

1
Sources/Workspace.swift
View file

@ -4647,6 +4647,7 @@ extension Workspace: BonsplitDelegate {
if lastTerminalConfigInheritancePanelId == panelId {
lastTerminalConfigInheritancePanelId = nil
}
AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: id, surfaceId: panelId)
// Keep the workspace invariant for normal close paths.
// Detach/move flows intentionally allow a temporary empty workspace so AppDelegate can

32
Sources/cmuxApp.swift
View file

@ -452,8 +452,9 @@ struct cmuxApp: App {
Divider()
// Terminal semantics:
// Cmd+W closes the focused tab (with confirmation if needed). If this is the last
// tab in the last workspace, it closes the window.
// 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.
Button(String(localized: "menu.file.closeTab", defaultValue: "Close Tab")) {
closePanelOrWindow()
}
@ -3075,6 +3076,8 @@ struct SettingsView: View {
@AppStorage(ShortcutHintDebugSettings.alwaysShowHintsKey)
private var alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints
@AppStorage(WorkspacePlacementSettings.placementKey) private var newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue
@AppStorage(LastSurfaceCloseShortcutSettings.key)
private var closeWorkspaceOnLastSurfaceShortcut = LastSurfaceCloseShortcutSettings.defaultValue
@AppStorage(WorkspaceAutoReorderSettings.key) private var workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue
@AppStorage(SidebarWorkspaceDetailSettings.hideAllDetailsKey)
private var sidebarHideAllDetails = SidebarWorkspaceDetailSettings.defaultHideAllDetails
@ -3120,6 +3123,19 @@ struct SettingsView: View {
NewWorkspacePlacement(rawValue: newWorkspacePlacement) ?? WorkspacePlacementSettings.defaultPlacement
}
private var closeWorkspaceOnLastSurfaceShortcutSubtitle: String {
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."
)
}
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."
)
}
private var selectedSidebarActiveTabIndicatorStyle: SidebarActiveTabIndicatorStyle {
SidebarActiveTabIndicatorSettings.resolvedStyle(rawValue: sidebarActiveTabIndicatorStyle)
}
@ -3487,6 +3503,17 @@ struct SettingsView: View {
SettingsCardDivider()
SettingsCardRow(
String(localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut", defaultValue: "Cmd+W Closes Workspace on Last Surface"),
subtitle: closeWorkspaceOnLastSurfaceShortcutSubtitle
) {
Toggle("", isOn: $closeWorkspaceOnLastSurfaceShortcut)
.labelsHidden()
.controlSize(.small)
}
SettingsCardDivider()
SettingsCardRow(
String(localized: "settings.app.reorderOnNotification", defaultValue: "Reorder on Notification"),
subtitle: String(localized: "settings.app.reorderOnNotification.subtitle", defaultValue: "Move workspaces to the top when they receive a notification. Disable for stable shortcut positions.")
@ -4456,6 +4483,7 @@ struct SettingsView: View {
ShortcutHintDebugSettings.resetVisibilityDefaults()
alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints
newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue
closeWorkspaceOnLastSurfaceShortcut = LastSurfaceCloseShortcutSettings.defaultValue
workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue
sidebarHideAllDetails = SidebarWorkspaceDetailSettings.defaultHideAllDetails
sidebarShowNotificationMessage = SidebarWorkspaceDetailSettings.defaultShowNotificationMessage

160
cmuxTests/CmuxWebViewKeyEquivalentTests.swift
View file

@ -56,6 +56,14 @@ private func installCmuxUnitTestInspectorOverride() {
cmuxUnitTestInspectorOverrideInstalled = true
}
private func drainMainQueue() {
let expectation = XCTestExpectation(description: "drain main queue")
DispatchQueue.main.async {
expectation.fulfill()
}
XCTWaiter().wait(for: [expectation], timeout: 1.0)
}
final class SplitShortcutTransientFocusGuardTests: XCTestCase {
func testSuppressesWhenFirstResponderFallsBackAndHostedViewIsTiny() {
XCTAssertTrue(
@ -4636,6 +4644,43 @@ final class WorkspaceAutoReorderSettingsTests: XCTestCase {
}
}
final class LastSurfaceCloseShortcutSettingsTests: XCTestCase {
func testDefaultKeepsWorkspaceOpen() {
let suiteName = "LastSurfaceCloseShortcutSettingsTests.Default.\(UUID().uuidString)"
guard let defaults = UserDefaults(suiteName: suiteName) else {
XCTFail("Failed to create isolated UserDefaults suite")
return
}
defer { defaults.removePersistentDomain(forName: suiteName) }
XCTAssertFalse(LastSurfaceCloseShortcutSettings.closesWorkspace(defaults: defaults))
}
func testStoredTrueClosesWorkspace() {
let suiteName = "LastSurfaceCloseShortcutSettingsTests.Enabled.\(UUID().uuidString)"
guard let defaults = UserDefaults(suiteName: suiteName) else {
XCTFail("Failed to create isolated UserDefaults suite")
return
}
defer { defaults.removePersistentDomain(forName: suiteName) }
defaults.set(true, forKey: LastSurfaceCloseShortcutSettings.key)
XCTAssertTrue(LastSurfaceCloseShortcutSettings.closesWorkspace(defaults: defaults))
}
func testStoredFalseKeepsWorkspaceOpen() {
let suiteName = "LastSurfaceCloseShortcutSettingsTests.Disabled.\(UUID().uuidString)"
guard let defaults = UserDefaults(suiteName: suiteName) else {
XCTFail("Failed to create isolated UserDefaults suite")
return
}
defer { defaults.removePersistentDomain(forName: suiteName) }
defaults.set(false, forKey: LastSurfaceCloseShortcutSettings.key)
XCTAssertFalse(LastSurfaceCloseShortcutSettings.closesWorkspace(defaults: defaults))
}
}
final class SidebarBranchLayoutSettingsTests: XCTestCase {
func testDefaultUsesVerticalLayout() {
let suiteName = "SidebarBranchLayoutSettingsTests.Default.\(UUID().uuidString)"
@ -5192,6 +5237,121 @@ final class TabManagerCloseWorkspacesWithConfirmationTests: XCTestCase {
}
}
@MainActor
final class TabManagerCloseCurrentPanelTests: XCTestCase {
func testCloseCurrentPanelKeepsWorkspaceOpenWhenItOwnsTheLastSurface() {
let manager = TabManager()
guard let workspace = manager.selectedWorkspace,
let initialPanelId = workspace.focusedPanelId else {
XCTFail("Expected selected workspace and focused panel")
return
}
let initialWorkspaceId = workspace.id
XCTAssertEqual(manager.tabs.count, 1)
XCTAssertEqual(workspace.panels.count, 1)
manager.closeCurrentPanelWithConfirmation()
drainMainQueue()
drainMainQueue()
XCTAssertEqual(manager.tabs.count, 1, "Closing the last surface should not remove the workspace")
XCTAssertEqual(manager.selectedTabId, initialWorkspaceId)
XCTAssertEqual(manager.tabs.first?.id, initialWorkspaceId)
XCTAssertNil(workspace.panels[initialPanelId], "Expected the original surface to be closed")
XCTAssertEqual(workspace.panels.count, 1, "Expected the workspace to stay alive with a replacement surface")
XCTAssertNotEqual(workspace.focusedPanelId, initialPanelId)
}
func testCloseCurrentPanelClosesWorkspaceWhenLastSurfaceShortcutSettingEnabled() {
let defaults = UserDefaults.standard
let originalSetting = defaults.object(forKey: LastSurfaceCloseShortcutSettings.key)
defaults.set(true, forKey: LastSurfaceCloseShortcutSettings.key)
defer {
if let originalSetting {
defaults.set(originalSetting, forKey: LastSurfaceCloseShortcutSettings.key)
} else {
defaults.removeObject(forKey: LastSurfaceCloseShortcutSettings.key)
}
}
let manager = TabManager()
let firstWorkspace = manager.tabs[0]
let secondWorkspace = manager.addWorkspace()
manager.selectWorkspace(secondWorkspace)
XCTAssertEqual(manager.selectedTabId, secondWorkspace.id)
XCTAssertEqual(secondWorkspace.panels.count, 1)
manager.closeCurrentPanelWithConfirmation()
XCTAssertEqual(manager.tabs.map(\.id), [firstWorkspace.id])
XCTAssertEqual(manager.selectedTabId, firstWorkspace.id)
}
func testCloseCurrentPanelWithLegacySettingIgnoresStaleSurfaceId() {
let defaults = UserDefaults.standard
let originalSetting = defaults.object(forKey: LastSurfaceCloseShortcutSettings.key)
defaults.set(true, forKey: LastSurfaceCloseShortcutSettings.key)
defer {
if let originalSetting {
defaults.set(originalSetting, forKey: LastSurfaceCloseShortcutSettings.key)
} else {
defaults.removeObject(forKey: LastSurfaceCloseShortcutSettings.key)
}
}
let manager = TabManager()
let firstWorkspace = manager.tabs[0]
let secondWorkspace = manager.addWorkspace()
manager.closePanelWithConfirmation(tabId: secondWorkspace.id, surfaceId: UUID())
XCTAssertEqual(manager.tabs.map(\.id), [firstWorkspace.id, secondWorkspace.id])
}
func testCloseCurrentPanelClearsNotificationsForClosedSurface() {
let appDelegate = AppDelegate.shared ?? AppDelegate()
let manager = TabManager()
let store = TerminalNotificationStore.shared
let originalTabManager = appDelegate.tabManager
let originalNotificationStore = appDelegate.notificationStore
store.replaceNotificationsForTesting([])
store.configureNotificationDeliveryHandlerForTesting { _, _ in }
appDelegate.tabManager = manager
appDelegate.notificationStore = store
defer {
store.replaceNotificationsForTesting([])
store.resetNotificationDeliveryHandlerForTesting()
appDelegate.tabManager = originalTabManager
appDelegate.notificationStore = originalNotificationStore
}
guard let workspace = manager.selectedWorkspace,
let initialPanelId = workspace.focusedPanelId else {
XCTFail("Expected selected workspace and focused panel")
return
}
store.addNotification(
tabId: workspace.id,
surfaceId: initialPanelId,
title: "Unread",
subtitle: "",
body: ""
)
XCTAssertTrue(store.hasUnreadNotification(forTabId: workspace.id, surfaceId: initialPanelId))
manager.closeCurrentPanelWithConfirmation()
drainMainQueue()
drainMainQueue()
XCTAssertFalse(store.hasUnreadNotification(forTabId: workspace.id, surfaceId: initialPanelId))
}
}
@MainActor
final class TabManagerPendingUnfocusPolicyTests: XCTestCase {
func testDoesNotUnfocusWhenPendingTabIsCurrentlySelected() {

71
cmuxUITests/CloseWorkspaceCmdDUITests.swift
View file

@ -27,23 +27,32 @@ final class CloseWorkspaceCmdDUITests: XCTestCase {
)
}
func testCmdDConfirmsCloseWhenClosingLastTabClosesWindow() {
func testCmdWClosingLastTabKeepsWorkspaceWindowOpen() {
let app = XCUIApplication()
// Closing the last tab should also present a confirmation and accept Cmd+D when it would close the window.
app.launchEnvironment["CMUX_UI_TEST_FORCE_CONFIRM_CLOSE_WORKSPACE"] = "1"
let keyequivPath = "/tmp/cmux-ui-test-keyequiv-\(UUID().uuidString).json"
try? FileManager.default.removeItem(atPath: keyequivPath)
app.launchEnvironment["CMUX_UI_TEST_KEYEQUIV_PATH"] = keyequivPath
app.launch()
app.activate()
// Close current tab (Cmd+W). With a single workspace and a single tab, this will close the window after confirmation.
let baseline = loadJSON(atPath: keyequivPath)?["closePanelInvocations"].flatMap(Int.init) ?? 0
app.typeKey("w", modifierFlags: [.command])
XCTAssertTrue(waitForCloseTabAlert(app: app, timeout: 5.0))
XCTAssertTrue(
waitForKeyequivInt("closePanelInvocations", toBeAtLeast: baseline + 1, atPath: keyequivPath, timeout: 5.0),
"Expected Cmd+W to route through the close-current-tab action"
)
// Cmd+D should accept the destructive close and close the window.
app.typeKey("d", modifierFlags: [.command])
if waitForCloseTabAlert(app: app, timeout: 5.0) {
clickCloseOnCloseTabAlert(app: app)
XCTAssertFalse(
isCloseTabAlertPresent(app: app),
"Expected close tab confirmation to dismiss after confirming the close"
)
}
XCTAssertTrue(
waitForNoWindowsOrAppNotRunningForeground(app: app, timeout: 6.0),
"Expected Cmd+D to confirm close and close the last window"
waitForWindowCount(app: app, atLeast: 1, timeout: 6.0),
"Expected Cmd+W on the last tab to keep the workspace window open"
)
}
@ -608,12 +617,37 @@ final class CloseWorkspaceCmdDUITests: XCTestCase {
private func waitForCloseTabAlert(app: XCUIApplication, timeout: TimeInterval) -> Bool {
let deadline = Date().addingTimeInterval(timeout)
while Date() < deadline {
if app.dialogs.containing(.staticText, identifier: "Close tab?").firstMatch.exists { return true }
if app.alerts.containing(.staticText, identifier: "Close tab?").firstMatch.exists { return true }
if app.staticTexts["Close tab?"].exists { return true }
if isCloseTabAlertPresent(app: app) { return true }
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
}
return false
return isCloseTabAlertPresent(app: app)
}
// Must match the defaultValue for dialog.closeTab.title in TabManager.
private func isCloseTabAlertPresent(app: XCUIApplication) -> Bool {
if app.dialogs.containing(.staticText, identifier: "Close tab?").firstMatch.exists { return true }
if app.alerts.containing(.staticText, identifier: "Close tab?").firstMatch.exists { return true }
return app.staticTexts["Close tab?"].exists
}
// Must match the defaultValue for dialog.closeTab.title in TabManager.
private func clickCloseOnCloseTabAlert(app: XCUIApplication) {
let dialog = app.dialogs.containing(.staticText, identifier: "Close tab?").firstMatch
if dialog.exists {
dialog.buttons["Close"].firstMatch.click()
return
}
let alert = app.alerts.containing(.staticText, identifier: "Close tab?").firstMatch
if alert.exists {
alert.buttons["Close"].firstMatch.click()
return
}
let anyDialog = app.dialogs.firstMatch
if anyDialog.exists, anyDialog.buttons["Close"].exists {
anyDialog.buttons["Close"].firstMatch.click()
}
}
private func waitForWindowCount(app: XCUIApplication, toBe count: Int, timeout: TimeInterval) -> Bool {
@ -644,6 +678,17 @@ final class CloseWorkspaceCmdDUITests: XCTestCase {
return app.state != .runningForeground || app.windows.count == 0
}
private func waitForKeyequivInt(_ key: String, toBeAtLeast expected: Int, atPath path: String, timeout: TimeInterval) -> Bool {
let deadline = Date().addingTimeInterval(timeout)
while Date() < deadline {
let value = loadJSON(atPath: path)?[key].flatMap(Int.init) ?? 0
if value >= expected { return true }
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
}
let value = loadJSON(atPath: path)?[key].flatMap(Int.init) ?? 0
return value >= expected
}
private func waitForAnyJSON(atPath path: String, timeout: TimeInterval) -> Bool {
let deadline = Date().addingTimeInterval(timeout)
while Date() < deadline {