Skip to content

Commit 76c1e63

Browse files
austinywangaustinywang
authored
Merge pull request #1895 from manaflow-ai/issue-1863-pinned-workspace-close-protect
Confirm before closing pinned workspaces
2 parents 374de63 + 0580de4 commit 76c1e63

7 files changed

Lines changed: 248 additions & 17 deletions

‎Resources/Localizable.xcstrings‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29831,6 +29831,40 @@
2983129831
}
2983229832
}
2983329833
},
29834+
"dialog.closePinnedWorkspace.message": {
29835+
"extractionState": "manual",
29836+
"localizations": {
29837+
"en": {
29838+
"stringUnit": {
29839+
"state": "translated",
29840+
"value": "This workspace is pinned. Closing it will close the workspace and all of its panels."
29841+
}
29842+
},
29843+
"ja": {
29844+
"stringUnit": {
29845+
"state": "translated",
29846+
"value": "このワークスペースはピン留めされています。閉じると、このワークスペースとその中のすべてのパネルが閉じます。"
29847+
}
29848+
}
29849+
}
29850+
},
29851+
"dialog.closePinnedWorkspace.title": {
29852+
"extractionState": "manual",
29853+
"localizations": {
29854+
"en": {
29855+
"stringUnit": {
29856+
"state": "translated",
29857+
"value": "Close pinned workspace?"
29858+
}
29859+
},
29860+
"ja": {
29861+
"stringUnit": {
29862+
"state": "translated",
29863+
"value": "ピン留めされたワークスペースを閉じますか？"
29864+
}
29865+
}
29866+
}
29867+
},
2983429868
"dialog.closeWorkspace.message": {
2983529869
"extractionState": "manual",
2983629870
"localizations": {
@@ -62666,6 +62700,23 @@
6266662700
}
6266762701
}
6266862702
},
62703+
"sidebar.pinnedWorkspaceProtected.tooltip": {
62704+
"extractionState": "manual",
62705+
"localizations": {
62706+
"en": {
62707+
"stringUnit": {
62708+
"state": "translated",
62709+
"value": "Pinned workspace. Closing requires confirmation."
62710+
}
62711+
},
62712+
"ja": {
62713+
"stringUnit": {
62714+
"state": "translated",
62715+
"value": "ピン留めされたワークスペースです。閉じるには確認が必要です。"
62716+
}
62717+
}
62718+
}
62719+
},
6266962720
"sidebar.folderIcon.dragHint": {
6267062721
"extractionState": "manual",
6267162722
"localizations": {
@@ -74899,6 +74950,23 @@
7489974950
}
7490074951
}
7490174952
},
74953+
"workspace.closeProtected.message": {
74954+
"extractionState": "manual",
74955+
"localizations": {
74956+
"en": {
74957+
"stringUnit": {
74958+
"state": "translated",
74959+
"value": "Pinned workspaces can't be closed while pinned. Unpin the workspace first."
74960+
}
74961+
},
74962+
"ja": {
74963+
"stringUnit": {
74964+
"state": "translated",
74965+
"value": "ピン留めされたワークスペースは閉じられません。先にピンを外してください。"
74966+
}
74967+
}
74968+
}
74969+
},
7490274970
"workspace.placement.afterCurrent": {
7490374971
"extractionState": "manual",
7490474972
"localizations": {

‎Sources/ContentView.swift‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6933,21 +6933,21 @@ struct ContentView: View {
69336933
private func closeOtherSelectedWorkspaces() {
69346934
guard let workspace = tabManager.selectedWorkspace else { return }
69356935
let workspaceIds = tabManager.tabs.compactMap { $0.id == workspace.id ? nil : $0.id }
6936-
closeWorkspaceIds(workspaceIds, allowPinned: false)
6936+
closeWorkspaceIds(workspaceIds, allowPinned: true)
69376937
}
69386938

69396939
private func closeSelectedWorkspacesBelow() {
69406940
guard tabManager.selectedWorkspace != nil,
69416941
let anchorIndex = selectedWorkspaceIndex() else { return }
69426942
let workspaceIds = tabManager.tabs.suffix(from: anchorIndex + 1).map(\.id)
6943-
closeWorkspaceIds(workspaceIds, allowPinned: false)
6943+
closeWorkspaceIds(workspaceIds, allowPinned: true)
69446944
}
69456945

69466946
private func closeSelectedWorkspacesAbove() {
69476947
guard tabManager.selectedWorkspace != nil,
69486948
let anchorIndex = selectedWorkspaceIndex() else { return }
69496949
let workspaceIds = tabManager.tabs.prefix(upTo: anchorIndex).map(\.id)
6950-
closeWorkspaceIds(workspaceIds, allowPinned: false)
6950+
closeWorkspaceIds(workspaceIds, allowPinned: true)
69516951
}
69526952

69536953
private func syncSidebarSelectedWorkspaceIds() {
@@ -10881,6 +10881,13 @@ private struct TabItemView: View, Equatable {
1088110881

1088210882
var body: some View {
1088310883
let closeWorkspaceTooltip = String(localized: "sidebar.closeWorkspace.tooltip", defaultValue: "Close Workspace")
10884+
let protectedWorkspaceTooltip = String(
10885+
localized: "sidebar.pinnedWorkspaceProtected.tooltip",
10886+
defaultValue: "Pinned workspace. Closing requires confirmation."
10887+
)
10888+
let closeButtonTooltip = tab.isPinned
10889+
? protectedWorkspaceTooltip
10890+
: KeyboardShortcutSettings.Action.closeWorkspace.tooltip(closeWorkspaceTooltip)
1088410891
let accessibilityHintText = String(localized: "sidebar.workspace.accessibilityHint", defaultValue: "Activate to focus this workspace. Drag to reorder, or use Move Up and Move Down actions.")
1088510892
let moveUpActionText = String(localized: "sidebar.workspace.moveUpAction", defaultValue: "Move Up")
1088610893
let moveDownActionText = String(localized: "sidebar.workspace.moveDownAction", defaultValue: "Move Down")
@@ -10942,6 +10949,7 @@ private struct TabItemView: View, Equatable {
1094210949
Image(systemName: "pin.fill")
1094310950
.font(.system(size: 9, weight: .semibold))
1094410951
.foregroundColor(activeSecondaryColor(0.8))
10952+
.safeHelp(protectedWorkspaceTooltip)
1094510953
}
1094610954

1094710955
Text(tab.title)
@@ -10965,7 +10973,7 @@ private struct TabItemView: View, Equatable {
1096510973
.foregroundColor(activeSecondaryColor(0.7))
1096610974
}
1096710975
.buttonStyle(.plain)
10968-
.safeHelp(KeyboardShortcutSettings.Action.closeWorkspace.tooltip(closeWorkspaceTooltip))
10976+
.safeHelp(closeButtonTooltip)
1096910977
.frame(width: SidebarTrailingAccessoryWidthPolicy.closeButtonWidth, height: 16, alignment: .center)
1097010978
.opacity(showCloseButton && !showsWorkspaceShortcutHint ? 1 : 0)
1097110979
.allowsHitTesting(showCloseButton && !showsWorkspaceShortcutHint)
@@ -11599,19 +11607,19 @@ private struct TabItemView: View, Equatable {
1159911607
private func closeOtherTabs(_ targetIds: [UUID]) {
1160011608
let keepIds = Set(targetIds)
1160111609
let idsToClose = tabManager.tabs.compactMap { keepIds.contains($0.id) ? nil : $0.id }
11602-
closeTabs(idsToClose, allowPinned: false)
11610+
closeTabs(idsToClose, allowPinned: true)
1160311611
}
1160411612

1160511613
private func closeTabsBelow(tabId: UUID) {
1160611614
guard let anchorIndex = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return }
1160711615
let idsToClose = tabManager.tabs.suffix(from: anchorIndex + 1).map { $0.id }
11608-
closeTabs(idsToClose, allowPinned: false)
11616+
closeTabs(idsToClose, allowPinned: true)
1160911617
}
1161011618

1161111619
private func closeTabsAbove(tabId: UUID) {
1161211620
guard let anchorIndex = tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return }
1161311621
let idsToClose = tabManager.tabs.prefix(upTo: anchorIndex).map { $0.id }
11614-
closeTabs(idsToClose, allowPinned: false)
11622+
closeTabs(idsToClose, allowPinned: true)
1161511623
}
1161611624

1161711625
private func markTabsRead(_ targetIds: [UUID]) {

‎Sources/TabManager.swift‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2328,13 +2328,34 @@ class TabManager: ObservableObject {
23282328
closeWorkspaceWithConfirmation(workspace)
23292329
}
23302330

2331-
func closeWorkspaceWithConfirmation(_ workspace: Workspace) {
2331+
func canCloseWorkspace(_ workspace: Workspace, allowPinned: Bool = false) -> Bool {
2332+
allowPinned || !workspace.isPinned
2333+
}
2334+
2335+
@discardableResult
2336+
func closeWorkspaceWithConfirmation(_ workspace: Workspace) -> Bool {
2337+
if workspace.isPinned {
2338+
guard confirmClose(
2339+
title: String(localized: "dialog.closePinnedWorkspace.title", defaultValue: "Close pinned workspace?"),
2340+
message: String(
2341+
localized: "dialog.closePinnedWorkspace.message",
2342+
defaultValue: "This workspace is pinned. Closing it will close the workspace and all of its panels."
2343+
),
2344+
acceptCmdD: tabs.count <= 1
2345+
) else {
2346+
return false
2347+
}
2348+
closeWorkspaceIfRunningProcess(workspace, requiresConfirmation: false)
2349+
return true
2350+
}
23322351
closeWorkspaceIfRunningProcess(workspace)
2352+
return true
23332353
}
23342354

2335-
func closeWorkspaceWithConfirmation(tabId: UUID) {
2336-
guard let workspace = tabs.first(where: { $0.id == tabId }) else { return }
2337-
closeWorkspaceWithConfirmation(workspace)
2355+
@discardableResult
2356+
func closeWorkspaceWithConfirmation(tabId: UUID) -> Bool {
2357+
guard let workspace = tabs.first(where: { $0.id == tabId }) else { return false }
2358+
return closeWorkspaceWithConfirmation(workspace)
23382359
}
23392360

23402361
func setSidebarSelectedWorkspaceIds(_ workspaceIds: Set<UUID>) {

‎Sources/TerminalController.swift‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3436,14 +3436,29 @@ class TerminalController {
34363436
}
34373437

34383438
var found = false
3439+
var protected = false
34393440
v2MainSync {
34403441
if let ws = tabManager.tabs.first(where: { $0.id == wsId }) {
3442+
guard tabManager.canCloseWorkspace(ws) else {
3443+
protected = true
3444+
found = true
3445+
return
3446+
}
34413447
tabManager.closeWorkspace(ws)
34423448
found = true
34433449
}
34443450
}
34453451

34463452
let windowId = v2ResolveWindowId(tabManager: tabManager)
3453+
if protected {
3454+
return .err(code: "protected", message: workspaceCloseProtectedMessage(), data: [
3455+
"window_id": v2OrNull(windowId?.uuidString),
3456+
"window_ref": v2Ref(kind: .window, uuid: windowId),
3457+
"workspace_id": wsId.uuidString,
3458+
"workspace_ref": v2Ref(kind: .workspace, uuid: wsId),
3459+
"pinned": true
3460+
])
3461+
}
34473462
return found
34483463
? .ok([
34493464
"window_id": v2OrNull(windowId?.uuidString),
@@ -3456,6 +3471,14 @@ class TerminalController {
34563471
"workspace_ref": v2Ref(kind: .workspace, uuid: wsId)
34573472
])
34583473
}
3474+
3475+
private func workspaceCloseProtectedMessage() -> String {
3476+
String(
3477+
localized: "workspace.closeProtected.message",
3478+
defaultValue: "Pinned workspaces can't be closed while pinned. Unpin the workspace first."
3479+
)
3480+
}
3481+
34593482
private func v2WorkspaceMoveToWindow(params: [String: Any]) -> V2CallResult {
34603483
guard let wsId = v2UUID(params, "workspace_id") else {
34613484
return .err(code: "invalid_params", message: "Missing or invalid workspace_id", data: nil)
@@ -12958,14 +12981,18 @@ class TerminalController {
1295812981
guard let tabManager = tabManager else { return "ERROR: TabManager not available" }
1295912982
guard let uuid = UUID(uuidString: tabId) else { return "ERROR: Invalid tab ID" }
1296012983

12961-
var success = false
12984+
var result = "ERROR: Tab not found"
1296212985
DispatchQueue.main.sync {
1296312986
if let tab = tabManager.tabs.first(where: { $0.id == uuid }) {
12987+
guard tabManager.canCloseWorkspace(tab) else {
12988+
result = "ERROR: \(workspaceCloseProtectedMessage())"
12989+
return
12990+
}
1296412991
tabManager.closeTab(tab)
12965-
success = true
12992+
result = "OK"
1296612993
}
1296712994
}
12968-
return success ? "OK" : "ERROR: Tab not found"
12995+
return result
1296912996
}
1297012997

1297112998
private func selectWorkspace(_ arg: String) -> String {

‎Sources/cmuxApp.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,21 +1067,21 @@ struct cmuxApp: App {
10671067
private func closeOtherSelectedWorkspacePeers(in manager: TabManager) {
10681068
guard let workspace = manager.selectedWorkspace else { return }
10691069
let workspaceIds = manager.tabs.compactMap { $0.id == workspace.id ? nil : $0.id }
1070-
closeWorkspaceIds(workspaceIds, in: manager, allowPinned: false)
1070+
closeWorkspaceIds(workspaceIds, in: manager, allowPinned: true)
10711071
}
10721072

10731073
private func closeSelectedWorkspacesBelow(in manager: TabManager) {
10741074
guard let workspace = manager.selectedWorkspace,
10751075
let anchorIndex = selectedWorkspaceIndex(in: manager, workspaceId: workspace.id) else { return }
10761076
let workspaceIds = manager.tabs.suffix(from: anchorIndex + 1).map(\.id)
1077-
closeWorkspaceIds(workspaceIds, in: manager, allowPinned: false)
1077+
closeWorkspaceIds(workspaceIds, in: manager, allowPinned: true)
10781078
}
10791079

10801080
private func closeSelectedWorkspacesAbove(in manager: TabManager) {
10811081
guard let workspace = manager.selectedWorkspace,
10821082
let anchorIndex = selectedWorkspaceIndex(in: manager, workspaceId: workspace.id) else { return }
10831083
let workspaceIds = manager.tabs.prefix(upTo: anchorIndex).map(\.id)
1084-
closeWorkspaceIds(workspaceIds, in: manager, allowPinned: false)
1084+
closeWorkspaceIds(workspaceIds, in: manager, allowPinned: true)
10851085
}
10861086

10871087
private func selectedWorkspaceHasUnreadNotifications(in manager: TabManager) -> Bool {

‎cmuxTests/TabManagerUnitTests.swift‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,75 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase {
399399
XCTAssertTrue(secondWorkspace.panels.isEmpty)
400400
}
401401

402+
func testCloseCurrentPanelPromptsBeforeClosingPinnedWorkspaceLastSurface() {
403+
let manager = TabManager()
404+
_ = manager.tabs[0]
405+
let pinnedWorkspace = manager.addWorkspace()
406+
manager.setPinned(pinnedWorkspace, pinned: true)
407+
manager.selectWorkspace(pinnedWorkspace)
408+
409+
guard let pinnedPanelId = pinnedWorkspace.focusedPanelId else {
410+
XCTFail("Expected focused panel in pinned workspace")
411+
return
412+
}
413+
414+
XCTAssertEqual(manager.selectedTabId, pinnedWorkspace.id)
415+
XCTAssertEqual(pinnedWorkspace.panels.count, 1)
416+
417+
var prompts: [(title: String, message: String, acceptCmdD: Bool)] = []
418+
manager.confirmCloseHandler = { title, message, acceptCmdD in
419+
prompts.append((title, message, acceptCmdD))
420+
return false
421+
}
422+
423+
manager.closeCurrentPanelWithConfirmation()
424+
drainMainQueue()
425+
drainMainQueue()
426+
427+
XCTAssertEqual(prompts.count, 1)
428+
XCTAssertEqual(
429+
prompts.first?.title,
430+
String(localized: "dialog.closePinnedWorkspace.title", defaultValue: "Close pinned workspace?")
431+
)
432+
XCTAssertEqual(
433+
prompts.first?.message,
434+
String(
435+
localized: "dialog.closePinnedWorkspace.message",
436+
defaultValue: "This workspace is pinned. Closing it will close the workspace and all of its panels."
437+
)
438+
)
439+
XCTAssertEqual(prompts.first?.acceptCmdD, false)
440+
XCTAssertEqual(manager.tabs.count, 2)
441+
XCTAssertTrue(manager.tabs.contains(where: { $0.id == pinnedWorkspace.id }))
442+
XCTAssertEqual(manager.selectedTabId, pinnedWorkspace.id)
443+
XCTAssertNotNil(pinnedWorkspace.panels[pinnedPanelId])
444+
XCTAssertEqual(pinnedWorkspace.panels.count, 1)
445+
}
446+
447+
func testCloseCurrentPanelClosesPinnedWorkspaceAfterConfirmation() {
448+
let manager = TabManager()
449+
let firstWorkspace = manager.tabs[0]
450+
let pinnedWorkspace = manager.addWorkspace()
451+
manager.setPinned(pinnedWorkspace, pinned: true)
452+
manager.selectWorkspace(pinnedWorkspace)
453+
454+
guard let pinnedPanelId = pinnedWorkspace.focusedPanelId else {
455+
XCTFail("Expected focused panel in pinned workspace")
456+
return
457+
}
458+
459+
manager.confirmCloseHandler = { _, _, _ in true }
460+
461+
manager.closeCurrentPanelWithConfirmation()
462+
drainMainQueue()
463+
drainMainQueue()
464+
465+
XCTAssertEqual(manager.tabs.map(\.id), [firstWorkspace.id])
466+
XCTAssertEqual(manager.selectedTabId, firstWorkspace.id)
467+
XCTAssertNil(pinnedWorkspace.panels[pinnedPanelId])
468+
XCTAssertTrue(pinnedWorkspace.panels.isEmpty)
469+
}
470+
402471
func testCloseCurrentPanelKeepsWorkspaceOpenWhenKeepWorkspaceOpenPreferenceIsEnabled() {
403472
let defaults = UserDefaults.standard
404473
let originalSetting = defaults.object(forKey: lastSurfaceCloseShortcutDefaultsKey)

0 commit comments

Comments
 (0)