Restore last-surface close preference toggle (#1679)
* test: cover last-surface close preference regression * fix: restore last-surface close preference
This commit is contained in:
Austin Wang
GitHub
parent
b64fb301c1
commit
bdd95902f2
6 changed files with 248 additions and 16 deletions
|
|
@ -43256,13 +43256,13 @@
|
|||
"en": {
|
||||
"stringUnit": {
|
||||
"state": "translated",
|
||||
"value": "Closing Last Surface Closes Workspace"
|
||||
"value": "Keep Workspace Open When Closing Last Surface"
|
||||
}
|
||||
},
|
||||
"ja": {
|
||||
"stringUnit": {
|
||||
"state": "translated",
|
||||
"value": "最後のサーフェスを閉じるとワークスペースも閉じる"
|
||||
"value": "最後のサーフェスを閉じてもワークスペースを残す"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -43273,13 +43273,13 @@
|
|||
"en": {
|
||||
"stringUnit": {
|
||||
"state": "translated",
|
||||
"value": "Closing the last surface keeps the workspace open. Use Cmd+Shift+W to close a workspace explicitly."
|
||||
"value": "When the focused surface is the last one in its workspace, the close-surface shortcut also closes the workspace."
|
||||
}
|
||||
},
|
||||
"ja": {
|
||||
"stringUnit": {
|
||||
"state": "translated",
|
||||
"value": "最後のサーフェスを閉じてもワークスペースは残ります。ワークスペースを明示的に閉じるにはCmd+Shift+Wを使います。"
|
||||
"value": "フォーカス中のサーフェスがそのワークスペースの最後の1つなら、サーフェスを閉じるショートカットはワークスペースも閉じます。"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -43290,13 +43290,13 @@
|
|||
"en": {
|
||||
"stringUnit": {
|
||||
"state": "translated",
|
||||
"value": "Closing the last surface also closes its workspace."
|
||||
"value": "When the focused surface is the last one in its workspace, the close-surface shortcut closes only the surface and keeps the workspace open. Use the close-workspace shortcut to close the workspace explicitly."
|
||||
}
|
||||
},
|
||||
"ja": {
|
||||
"stringUnit": {
|
||||
"state": "translated",
|
||||
"value": "最後のサーフェスを閉じると、そのワークスペースも閉じます。"
|
||||
"value": "フォーカス中のサーフェスがそのワークスペースの最後の1つでも、サーフェスを閉じるショートカットはサーフェスだけを閉じ、ワークスペースは残します。ワークスペースを閉じるショートカットを使うと明示的に閉じられます。"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -60,6 +60,20 @@ enum WorkspaceAutoReorderSettings {
|
|||
}
|
||||
}
|
||||
|
||||
enum LastSurfaceCloseShortcutSettings {
|
||||
static let key = "closeWorkspaceOnLastSurfaceShortcut"
|
||||
// Keep the legacy stored meaning so existing values still map to the same
|
||||
// behavior. The default is flipped to preserve current Cmd+W behavior.
|
||||
static let defaultValue = true
|
||||
|
||||
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
|
||||
|
|
@ -2101,6 +2115,12 @@ class TabManager: ObservableObject {
|
|||
}
|
||||
}
|
||||
|
||||
private func shouldCloseWorkspaceOnLastSurfaceShortcut(_ workspace: Workspace, panelId: UUID) -> Bool {
|
||||
LastSurfaceCloseShortcutSettings.closesWorkspace() &&
|
||||
workspace.panels.count <= 1 &&
|
||||
workspace.panels[panelId] != nil
|
||||
}
|
||||
|
||||
private func closePanelWithConfirmation(tab: Workspace, panelId: UUID) {
|
||||
guard tab.panels[panelId] != nil else {
|
||||
#if DEBUG
|
||||
|
|
@ -2121,18 +2141,20 @@ class TabManager: ObservableObject {
|
|||
if panel is BrowserPanel { return "browser" }
|
||||
return String(describing: type(of: panel))
|
||||
}()
|
||||
let closesWorkspaceOnLastSurfaceShortcut = shouldCloseWorkspaceOnLastSurfaceShortcut(tab, panelId: panelId)
|
||||
#if DEBUG
|
||||
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=\(closesWorkspaceOnLastSurfaceShortcut ? 1 : 0)"
|
||||
)
|
||||
#endif
|
||||
|
||||
// Route Cmd+W through Bonsplit/Workspace close handling so it matches the tab close
|
||||
// button, including shared confirmation, last-surface workspace/window-close behavior,
|
||||
// and the usual replacement-panel flow when the close does not collapse the workspace.
|
||||
if let surfaceId = tab.surfaceIdFromPanelId(panelId) {
|
||||
// The last-surface shortcut preference only affects Cmd+W. The tab close button
|
||||
// continues to use Workspace's explicit-close path when it closes the last surface.
|
||||
if closesWorkspaceOnLastSurfaceShortcut,
|
||||
let surfaceId = tab.surfaceIdFromPanelId(panelId) {
|
||||
tab.markExplicitClose(surfaceId: surfaceId)
|
||||
}
|
||||
let closed = tab.closePanel(panelId)
|
||||
|
|
|
|||
|
|
@ -5164,8 +5164,10 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
/// Prevents repeated close gestures (e.g., middle-click spam) from stacking dialogs.
|
||||
private var pendingCloseConfirmTabIds: Set<TabID> = []
|
||||
|
||||
/// Tab IDs whose next close attempt came from an explicit user close gesture
|
||||
/// (Cmd+W or the tab-strip X button), rather than an internal close/move flow.
|
||||
/// Tab IDs whose next close attempt should be treated as an explicit
|
||||
/// workspace-close gesture from the user (the tab-strip X button, or Cmd+W when
|
||||
/// the shortcut preference is set to close the workspace on the last surface),
|
||||
/// rather than an internal close/move flow.
|
||||
private var explicitUserCloseTabIds: Set<TabID> = []
|
||||
|
||||
/// Deterministic tab selection to apply after a tab closes.
|
||||
|
|
|
|||
|
|
@ -493,9 +493,10 @@ struct cmuxApp: App {
|
|||
Divider()
|
||||
|
||||
// Terminal semantics:
|
||||
// Cmd+W closes the focused tab/surface (with confirmation if needed). When that
|
||||
// was the last surface in the workspace, cmux removes the workspace and closes
|
||||
// the window if it was also the last workspace.
|
||||
// Cmd+W closes the focused tab/surface (with confirmation if needed). By
|
||||
// default, closing the last surface also closes the workspace and the window
|
||||
// if it was also the last workspace. Users can opt into keeping the workspace
|
||||
// open instead.
|
||||
Button(String(localized: "menu.file.closeTab", defaultValue: "Close Tab")) {
|
||||
closePanelOrWindow()
|
||||
}
|
||||
|
|
@ -3593,6 +3594,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
|
||||
|
|
@ -3645,6 +3648,30 @@ struct SettingsView: View {
|
|||
NewWorkspacePlacement(rawValue: newWorkspacePlacement) ?? WorkspacePlacementSettings.defaultPlacement
|
||||
}
|
||||
|
||||
private var keepWorkspaceOpenOnLastSurfaceShortcut: Bool {
|
||||
!closeWorkspaceOnLastSurfaceShortcut
|
||||
}
|
||||
|
||||
private var keepWorkspaceOpenOnLastSurfaceShortcutBinding: Binding<Bool> {
|
||||
Binding(
|
||||
get: { keepWorkspaceOpenOnLastSurfaceShortcut },
|
||||
set: { closeWorkspaceOnLastSurfaceShortcut = !$0 }
|
||||
)
|
||||
}
|
||||
|
||||
private var closeWorkspaceOnLastSurfaceShortcutSubtitle: String {
|
||||
if keepWorkspaceOpenOnLastSurfaceShortcut {
|
||||
return String(
|
||||
localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOn",
|
||||
defaultValue: "When the focused surface is the last one in its workspace, the close-surface shortcut closes only the surface and keeps the workspace open. Use the close-workspace shortcut to close the workspace explicitly."
|
||||
)
|
||||
}
|
||||
return String(
|
||||
localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOff",
|
||||
defaultValue: "When the focused surface is the last one in its workspace, the close-surface shortcut also closes the workspace."
|
||||
)
|
||||
}
|
||||
|
||||
private var selectedSidebarActiveTabIndicatorStyle: SidebarActiveTabIndicatorStyle {
|
||||
SidebarActiveTabIndicatorSettings.resolvedStyle(rawValue: sidebarActiveTabIndicatorStyle)
|
||||
}
|
||||
|
|
@ -4076,6 +4103,17 @@ struct SettingsView: View {
|
|||
|
||||
SettingsCardDivider()
|
||||
|
||||
SettingsCardRow(
|
||||
String(localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut", defaultValue: "Keep Workspace Open When Closing Last Surface"),
|
||||
subtitle: closeWorkspaceOnLastSurfaceShortcutSubtitle
|
||||
) {
|
||||
Toggle("", isOn: keepWorkspaceOpenOnLastSurfaceShortcutBinding)
|
||||
.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.")
|
||||
|
|
@ -5228,6 +5266,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
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@ import XCTest
|
|||
@testable import cmux
|
||||
#endif
|
||||
|
||||
private let lastSurfaceCloseShortcutDefaultsKey = "closeWorkspaceOnLastSurfaceShortcut"
|
||||
|
||||
@MainActor
|
||||
final class AppDelegateShortcutRoutingTests: XCTestCase {
|
||||
private var savedShortcutsByAction: [KeyboardShortcutSettings.Action: StoredShortcut] = [:]
|
||||
|
|
@ -714,6 +716,63 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
|
|||
)
|
||||
}
|
||||
|
||||
func testCmdWKeepsLastSurfaceWorkspaceOpenWhenKeepWorkspaceOpenPreferenceIsEnabled() throws {
|
||||
guard let appDelegate = AppDelegate.shared else {
|
||||
XCTFail("Expected AppDelegate.shared")
|
||||
return
|
||||
}
|
||||
|
||||
let defaults = UserDefaults.standard
|
||||
let originalSetting = defaults.object(forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
defaults.set(false, forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
defer {
|
||||
if let originalSetting {
|
||||
defaults.set(originalSetting, forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
} else {
|
||||
defaults.removeObject(forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
}
|
||||
}
|
||||
|
||||
let windowId = appDelegate.createMainWindow()
|
||||
defer { closeWindow(withId: windowId) }
|
||||
|
||||
guard let targetWindow = window(withId: windowId),
|
||||
let manager = appDelegate.tabManagerFor(windowId: windowId),
|
||||
let workspace = manager.selectedWorkspace,
|
||||
let initialPanelId = workspace.focusedPanelId else {
|
||||
XCTFail("Expected test window, manager, workspace, and focused panel")
|
||||
return
|
||||
}
|
||||
|
||||
guard let event = makeKeyDownEvent(
|
||||
key: "w",
|
||||
modifiers: [.command],
|
||||
keyCode: 13,
|
||||
windowNumber: targetWindow.windowNumber
|
||||
) else {
|
||||
XCTFail("Failed to construct Cmd+W event")
|
||||
return
|
||||
}
|
||||
|
||||
#if DEBUG
|
||||
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event))
|
||||
#else
|
||||
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
|
||||
#endif
|
||||
|
||||
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
|
||||
|
||||
XCTAssertNotNil(
|
||||
self.window(withId: windowId),
|
||||
"Cmd+W should keep the window open when the keep-workspace-open preference is enabled"
|
||||
)
|
||||
XCTAssertEqual(manager.tabs.count, 1)
|
||||
XCTAssertEqual(manager.selectedTabId, workspace.id)
|
||||
XCTAssertNil(workspace.panels[initialPanelId])
|
||||
XCTAssertEqual(workspace.panels.count, 1)
|
||||
XCTAssertNotEqual(workspace.focusedPanelId, initialPanelId)
|
||||
}
|
||||
|
||||
func testCmdWClosesAuxiliaryWindowInsteadOfMainTerminalPanel() throws {
|
||||
guard let appDelegate = AppDelegate.shared else {
|
||||
XCTFail("Expected AppDelegate.shared")
|
||||
|
|
|
|||
|
|
@ -14,6 +14,8 @@ import UserNotifications
|
|||
@testable import cmux
|
||||
#endif
|
||||
|
||||
private let lastSurfaceCloseShortcutDefaultsKey = "closeWorkspaceOnLastSurfaceShortcut"
|
||||
|
||||
private var cmuxUnitTestInspectorAssociationKey: UInt8 = 0
|
||||
private var cmuxUnitTestInspectorOverrideInstalled = false
|
||||
|
||||
|
|
@ -5017,6 +5019,43 @@ final class WorkspaceAutoReorderSettingsTests: XCTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
final class LastSurfaceCloseShortcutSettingsTests: XCTestCase {
|
||||
func testDefaultClosesWorkspace() {
|
||||
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) }
|
||||
|
||||
XCTAssertTrue(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)"
|
||||
|
|
@ -5823,6 +5862,39 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase {
|
|||
XCTAssertTrue(secondWorkspace.panels.isEmpty)
|
||||
}
|
||||
|
||||
func testCloseCurrentPanelKeepsWorkspaceOpenWhenKeepWorkspaceOpenPreferenceIsEnabled() {
|
||||
let defaults = UserDefaults.standard
|
||||
let originalSetting = defaults.object(forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
defaults.set(false, forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
defer {
|
||||
if let originalSetting {
|
||||
defaults.set(originalSetting, forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
} else {
|
||||
defaults.removeObject(forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
}
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
manager.closeCurrentPanelWithConfirmation()
|
||||
drainMainQueue()
|
||||
drainMainQueue()
|
||||
|
||||
XCTAssertEqual(manager.tabs.count, 1)
|
||||
XCTAssertEqual(manager.selectedTabId, initialWorkspaceId)
|
||||
XCTAssertEqual(manager.tabs.first?.id, initialWorkspaceId)
|
||||
XCTAssertNil(workspace.panels[initialPanelId])
|
||||
XCTAssertEqual(workspace.panels.count, 1)
|
||||
XCTAssertNotEqual(workspace.focusedPanelId, initialPanelId)
|
||||
}
|
||||
|
||||
func testClosePanelButtonClosesWorkspaceWhenItOwnsTheLastSurface() {
|
||||
let manager = TabManager()
|
||||
let firstWorkspace = manager.tabs[0]
|
||||
|
|
@ -5853,6 +5925,44 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase {
|
|||
XCTAssertTrue(secondWorkspace.panels.isEmpty)
|
||||
}
|
||||
|
||||
func testClosePanelButtonStillClosesWorkspaceWhenKeepWorkspaceOpenPreferenceIsEnabled() {
|
||||
let defaults = UserDefaults.standard
|
||||
let originalSetting = defaults.object(forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
defaults.set(false, forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
defer {
|
||||
if let originalSetting {
|
||||
defaults.set(originalSetting, forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
} else {
|
||||
defaults.removeObject(forKey: lastSurfaceCloseShortcutDefaultsKey)
|
||||
}
|
||||
}
|
||||
|
||||
let manager = TabManager()
|
||||
let firstWorkspace = manager.tabs[0]
|
||||
let secondWorkspace = manager.addWorkspace()
|
||||
manager.selectWorkspace(secondWorkspace)
|
||||
|
||||
guard let secondPanelId = secondWorkspace.focusedPanelId else {
|
||||
XCTFail("Expected focused panel in selected workspace")
|
||||
return
|
||||
}
|
||||
|
||||
guard let secondSurfaceId = secondWorkspace.surfaceIdFromPanelId(secondPanelId) else {
|
||||
XCTFail("Expected bonsplit surface ID for focused panel")
|
||||
return
|
||||
}
|
||||
|
||||
secondWorkspace.markExplicitClose(surfaceId: secondSurfaceId)
|
||||
XCTAssertFalse(secondWorkspace.closePanel(secondPanelId))
|
||||
drainMainQueue()
|
||||
drainMainQueue()
|
||||
|
||||
XCTAssertEqual(manager.tabs.map(\.id), [firstWorkspace.id])
|
||||
XCTAssertEqual(manager.selectedTabId, firstWorkspace.id)
|
||||
XCTAssertNil(secondWorkspace.panels[secondPanelId])
|
||||
XCTAssertTrue(secondWorkspace.panels.isEmpty)
|
||||
}
|
||||
|
||||
func testGenericClosePanelKeepsWorkspaceOpenWithoutExplicitCloseMarker() {
|
||||
let manager = TabManager()
|
||||
guard let workspace = manager.selectedWorkspace,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue