From 1229134a353ab1f96e1d1b16ca6edfd1861cff5d Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Wed, 27 May 2026 00:14:11 -0700 Subject: [PATCH 1/3] Optimize batch workspace sidebar actions --- Sources/ContentView.swift | 19 +++-- Sources/TabManager.swift | 65 +++++++++++++- Sources/WorkspaceActionDispatcher.swift | 19 ++--- .../WorkspaceActionDispatcherTests.swift | 25 ++++++ cmuxTests/WorkspaceStressProfileTests.swift | 84 +++++++++++++++++++ cmuxTests/WorkspaceUnitTests.swift | 28 +++++++ 6 files changed, 221 insertions(+), 19 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 093d205cd706..7234d953a997 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -15385,13 +15385,20 @@ private struct TabItemView: View, Equatable { } private func toggleWorkspaceTerminalScrollBarHidden(targetIds: [UUID]) { - let currentlyHidden = !targetIds.isEmpty && targetIds.allSatisfy { targetId in - tabManager.tabs.first(where: { $0.id == targetId })?.terminalScrollBarHidden == true - } + let targetIdSet = Set(targetIds) + var liveTargetCount = 0 + var hiddenTargetCount = 0 + for workspace in tabManager.tabs where targetIdSet.contains(workspace.id) { + liveTargetCount += 1 + if workspace.terminalScrollBarHidden { + hiddenTargetCount += 1 + } + } + let currentlyHidden = !targetIdSet.isEmpty && + liveTargetCount == targetIdSet.count && + hiddenTargetCount == liveTargetCount let hideScrollBar = !currentlyHidden - for targetId in targetIds { - tabManager.setWorkspaceTerminalScrollBarHidden(tabId: targetId, hidden: hideScrollBar) - } + tabManager.setWorkspaceTerminalScrollBarHidden(hidden: hideScrollBar, forWorkspaceIds: targetIds) } private func promptCustomColor(targetIds: [UUID]) { diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index e485a1b7ef7e..a8e191d9f08b 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -5614,8 +5614,15 @@ class TabManager: ObservableObject { } func applyWorkspaceColor(_ color: String?, toWorkspaceIds workspaceIds: [UUID]) { - for workspaceId in workspaceIds { + guard !workspaceIds.isEmpty else { return } + if workspaceIds.count == 1, let workspaceId = workspaceIds.first { setTabColor(tabId: workspaceId, color: color) + return + } + + let targetIds = Set(workspaceIds) + for tab in tabs where targetIds.contains(tab.id) { + tab.setCustomColor(color) } } @@ -5629,6 +5636,19 @@ class TabManager: ObservableObject { tab.setTerminalScrollBarHidden(hidden) } + func setWorkspaceTerminalScrollBarHidden(hidden: Bool, forWorkspaceIds workspaceIds: [UUID]) { + guard !workspaceIds.isEmpty else { return } + if workspaceIds.count == 1, let workspaceId = workspaceIds.first { + setWorkspaceTerminalScrollBarHidden(tabId: workspaceId, hidden: hidden) + return + } + + let targetIds = Set(workspaceIds) + for tab in tabs where targetIds.contains(tab.id) { + tab.setTerminalScrollBarHidden(hidden) + } + } + func togglePin(tabId: UUID) { guard let index = tabs.firstIndex(where: { $0.id == tabId }) else { return } let tab = tabs[index] @@ -5642,6 +5662,49 @@ class TabManager: ObservableObject { postWorkspaceOrderDidChange(movedWorkspaceIds: [tab.id]) } + @discardableResult + func setPinned(workspaceIds: [UUID], pinned: Bool) -> [UUID] { + guard !workspaceIds.isEmpty else { return [] } + if workspaceIds.count == 1, + let workspaceId = workspaceIds.first, + let tab = tabs.first(where: { $0.id == workspaceId }) { + let changed = tab.isPinned != pinned + setPinned(tab, pinned: pinned) + return changed ? [workspaceId] : [] + } + + var seen = Set() + let orderedTargetIds = workspaceIds.filter { seen.insert($0).inserted } + let targetIds = Set(orderedTargetIds) + var workspacesById: [UUID: Workspace] = [:] + var changedIdSet = Set() + + for workspace in tabs { + workspacesById[workspace.id] = workspace + guard targetIds.contains(workspace.id), workspace.isPinned != pinned else { continue } + workspace.isPinned = pinned + changedIdSet.insert(workspace.id) + } + + guard !changedIdSet.isEmpty else { return [] } + let changedIds = orderedTargetIds.filter { changedIdSet.contains($0) } + + let changedWorkspaces: [Workspace] + if pinned { + changedWorkspaces = changedIds.compactMap { workspacesById[$0] } + } else { + // Match the existing repeated setPinned behavior: each unpinned item + // moves to the front of the unpinned segment as it is processed. + changedWorkspaces = changedIds.reversed().compactMap { workspacesById[$0] } + } + + let remainingPinned = tabs.filter { $0.isPinned && !changedIdSet.contains($0.id) } + let remainingUnpinned = tabs.filter { !$0.isPinned && !changedIdSet.contains($0.id) } + tabs = remainingPinned + changedWorkspaces + remainingUnpinned + postWorkspaceOrderDidChange(movedWorkspaceIds: changedIds) + return changedIds + } + private func reorderTabForPinnedState(_ tab: Workspace) { guard let index = tabs.firstIndex(where: { $0.id == tab.id }) else { return } tabs.remove(at: index) diff --git a/Sources/WorkspaceActionDispatcher.swift b/Sources/WorkspaceActionDispatcher.swift index f7cb2c3bc65d..f4813905b9c8 100644 --- a/Sources/WorkspaceActionDispatcher.swift +++ b/Sources/WorkspaceActionDispatcher.swift @@ -32,14 +32,15 @@ enum WorkspaceActionDispatcher { in tabManager: TabManager, target: Target ) -> PinState? { + let workspacesById = Dictionary(uniqueKeysWithValues: tabManager.tabs.map { ($0.id, $0) }) let targetWorkspaceIds = liveWorkspaceIds(in: tabManager, from: target.workspaceIds) guard !targetWorkspaceIds.isEmpty else { return nil } let anchorWorkspaceId = target.anchorWorkspaceId.flatMap { anchorId in - tabManager.tabs.contains { $0.id == anchorId } ? anchorId : nil + workspacesById[anchorId] == nil ? nil : anchorId } ?? targetWorkspaceIds[0] - guard let anchorWorkspace = tabManager.tabs.first(where: { $0.id == anchorWorkspaceId }) else { + guard let anchorWorkspace = workspacesById[anchorWorkspaceId] else { return nil } @@ -67,16 +68,10 @@ enum WorkspaceActionDispatcher { in tabManager: TabManager ) -> PinResult { let targetWorkspaceIds = liveWorkspaceIds(in: tabManager, from: state.targetWorkspaceIds) - var changedWorkspaceIds: [UUID] = [] - - for workspaceId in targetWorkspaceIds { - guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { continue } - let wasPinned = workspace.isPinned - tabManager.setPinned(workspace, pinned: state.pinned) - if wasPinned != state.pinned { - changedWorkspaceIds.append(workspaceId) - } - } + let changedWorkspaceIds = tabManager.setPinned( + workspaceIds: targetWorkspaceIds, + pinned: state.pinned + ) return PinResult( targetWorkspaceIds: targetWorkspaceIds, diff --git a/cmuxTests/WorkspaceActionDispatcherTests.swift b/cmuxTests/WorkspaceActionDispatcherTests.swift index 80091715d6d7..8287f6ad07a1 100644 --- a/cmuxTests/WorkspaceActionDispatcherTests.swift +++ b/cmuxTests/WorkspaceActionDispatcherTests.swift @@ -54,6 +54,31 @@ final class WorkspaceActionDispatcherTests: XCTestCase { XCTAssertEqual(manager.tabs.map(\.id), [second.id, third.id, first.id]) } + func testPinActionUnpinsMultipleTargetsWithExistingOrdering() throws { + let manager = TabManager() + let first = try XCTUnwrap(manager.tabs.first) + let second = manager.addWorkspace() + let third = manager.addWorkspace() + manager.setPinned(first, pinned: true) + manager.setPinned(second, pinned: true) + manager.setPinned(third, pinned: true) + let target = WorkspaceActionDispatcher.Target( + workspaceIds: [second.id, third.id], + anchorWorkspaceId: second.id + ) + + let state = try XCTUnwrap(WorkspaceActionDispatcher.pinState(in: manager, target: target)) + let result = WorkspaceActionDispatcher.performPinAction(state, in: manager) + + XCTAssertFalse(state.pinned) + XCTAssertEqual(result.targetWorkspaceIds, [second.id, third.id]) + XCTAssertEqual(result.changedWorkspaceIds, [second.id, third.id]) + XCTAssertTrue(first.isPinned) + XCTAssertFalse(second.isPinned) + XCTAssertFalse(third.isPinned) + XCTAssertEqual(manager.tabs.map(\.id), [first.id, third.id, second.id]) + } + func testCapturedPinStateKeepsLabelAndActionConsistent() throws { let manager = TabManager() let workspace = try XCTUnwrap(manager.tabs.first) diff --git a/cmuxTests/WorkspaceStressProfileTests.swift b/cmuxTests/WorkspaceStressProfileTests.swift index bebb48d9ae56..15c63bf99458 100644 --- a/cmuxTests/WorkspaceStressProfileTests.swift +++ b/cmuxTests/WorkspaceStressProfileTests.swift @@ -200,6 +200,90 @@ final class WorkspaceStressProfileTests: XCTestCase { } } + func testWorkspaceBatchActionsStressProfile() { + let config = StressConfig.current() + let welcomeWasShown = UserDefaults.standard.object(forKey: WelcomeSettings.shownKey) + UserDefaults.standard.set(true, forKey: WelcomeSettings.shownKey) + defer { + if let welcomeWasShown { + UserDefaults.standard.set(welcomeWasShown, forKey: WelcomeSettings.shownKey) + } else { + UserDefaults.standard.removeObject(forKey: WelcomeSettings.shownKey) + } + } + + let manager = TabManager(autoWelcomeIfNeeded: false) + for workspaceIndex in 1.. 0 else { return } while workspace.panels.count < tabsPerWorkspace { diff --git a/cmuxTests/WorkspaceUnitTests.swift b/cmuxTests/WorkspaceUnitTests.swift index 26286ed0f434..c26adbb9cbe5 100644 --- a/cmuxTests/WorkspaceUnitTests.swift +++ b/cmuxTests/WorkspaceUnitTests.swift @@ -228,6 +228,34 @@ final class SidebarSelectedWorkspaceColorTests: XCTestCase { XCTAssertEqual(background.color?.hexString(), "#C0392B") XCTAssertEqual(background.opacity, 0.7, accuracy: 0.001) } + + @MainActor + func testBatchWorkspaceColorAppliesOnlyRequestedWorkspaces() { + let manager = TabManager() + let first = manager.tabs[0] + let second = manager.addWorkspace() + let third = manager.addWorkspace() + + manager.applyWorkspaceColor("#1565C0", toWorkspaceIds: [first.id, third.id]) + + XCTAssertEqual(first.customColor, "#1565C0") + XCTAssertNil(second.customColor) + XCTAssertEqual(third.customColor, "#1565C0") + } + + @MainActor + func testBatchWorkspaceTerminalScrollBarVisibilityAppliesOnlyRequestedWorkspaces() { + let manager = TabManager() + let first = manager.tabs[0] + let second = manager.addWorkspace() + let third = manager.addWorkspace() + + manager.setWorkspaceTerminalScrollBarHidden(hidden: true, forWorkspaceIds: [first.id, third.id]) + + XCTAssertTrue(first.terminalScrollBarHidden) + XCTAssertFalse(second.terminalScrollBarHidden) + XCTAssertTrue(third.terminalScrollBarHidden) + } } From 5a75eee7faf5d29e54b0422f7a5a7e74ea9d2438 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Wed, 27 May 2026 03:04:29 -0700 Subject: [PATCH 2/3] Preserve workspace selection after drag reorder --- Sources/ContentView.swift | 56 ++++++++++++++++++++++++++---- cmuxTests/WorkspaceUnitTests.swift | 43 +++++++++++++++++++++++ 2 files changed, 92 insertions(+), 7 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 7234d953a997..227358a4a7c0 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -16094,6 +16094,38 @@ private struct SidebarBonsplitTabDropDelegate: DropDelegate { } } +@MainActor +enum SidebarWorkspaceSelectionSyncPolicy { + static func reconciledSelection( + previousSelectionIds: Set, + liveWorkspaceIds: [UUID], + fallbackSelectedWorkspaceId: UUID? + ) -> Set { + let liveIdSet = Set(liveWorkspaceIds) + let liveSelectionIds = previousSelectionIds.filter { liveIdSet.contains($0) } + if !liveSelectionIds.isEmpty { + return liveSelectionIds + } + if let fallbackSelectedWorkspaceId, liveIdSet.contains(fallbackSelectedWorkspaceId) { + return [fallbackSelectedWorkspaceId] + } + return [] + } + + static func anchorIndex( + preferredWorkspaceId: UUID?, + selectedWorkspaceIds: Set, + liveWorkspaceIds: [UUID] + ) -> Int? { + if let preferredWorkspaceId, + selectedWorkspaceIds.contains(preferredWorkspaceId), + let preferredIndex = liveWorkspaceIds.firstIndex(of: preferredWorkspaceId) { + return preferredIndex + } + return liveWorkspaceIds.firstIndex { selectedWorkspaceIds.contains($0) } + } +} + @MainActor private struct SidebarTabDropDelegate: DropDelegate { let targetTabId: UUID? @@ -16191,14 +16223,9 @@ private struct SidebarTabDropDelegate: DropDelegate { #if DEBUG cmuxDebugLog("sidebar.drop.commit tab=\(draggedTabId.uuidString.prefix(5)) from=\(fromIndex) to=\(targetIndex)") #endif + let selectionBeforeReorder = selectedTabIds _ = tabManager.reorderWorkspace(tabId: draggedTabId, toIndex: targetIndex) - if let selectedId = tabManager.selectedTabId { - selectedTabIds = [selectedId] - syncSidebarSelection(preferredSelectedTabId: selectedId) - } else { - selectedTabIds = [] - syncSidebarSelection() - } + syncSidebarSelection(preserving: selectionBeforeReorder) return true } @@ -16226,6 +16253,21 @@ private struct SidebarTabDropDelegate: DropDelegate { } } + private func syncSidebarSelection(preserving previousSelectionIds: Set) { + let liveWorkspaceIds = tabManager.tabs.map(\.id) + let nextSelectionIds = SidebarWorkspaceSelectionSyncPolicy.reconciledSelection( + previousSelectionIds: previousSelectionIds, + liveWorkspaceIds: liveWorkspaceIds, + fallbackSelectedWorkspaceId: tabManager.selectedTabId + ) + selectedTabIds = nextSelectionIds + lastSidebarSelectionIndex = SidebarWorkspaceSelectionSyncPolicy.anchorIndex( + preferredWorkspaceId: tabManager.selectedTabId, + selectedWorkspaceIds: nextSelectionIds, + liveWorkspaceIds: liveWorkspaceIds + ) + } + private func debugIndicator(_ indicator: SidebarDropIndicator?) -> String { guard let indicator else { return "nil" } let tabText = indicator.tabId.map { String($0.uuidString.prefix(5)) } ?? "end" diff --git a/cmuxTests/WorkspaceUnitTests.swift b/cmuxTests/WorkspaceUnitTests.swift index c26adbb9cbe5..e808edd7c33a 100644 --- a/cmuxTests/WorkspaceUnitTests.swift +++ b/cmuxTests/WorkspaceUnitTests.swift @@ -3582,6 +3582,49 @@ final class SidebarWorkspaceAuxiliaryDetailVisibilityTests: XCTestCase { } +final class SidebarWorkspaceSelectionSyncPolicyTests: XCTestCase { + @MainActor + func testReconciledSelectionPreservesMultiSelectionAfterReorder() { + let first = UUID() + let second = UUID() + let third = UUID() + let fourth = UUID() + let previousSelection: Set = [second, third] + + let result = SidebarWorkspaceSelectionSyncPolicy.reconciledSelection( + previousSelectionIds: previousSelection, + liveWorkspaceIds: [first, third, fourth, second], + fallbackSelectedWorkspaceId: second + ) + + XCTAssertEqual(result, previousSelection) + XCTAssertEqual( + SidebarWorkspaceSelectionSyncPolicy.anchorIndex( + preferredWorkspaceId: second, + selectedWorkspaceIds: result, + liveWorkspaceIds: [first, third, fourth, second] + ), + 3 + ) + } + + @MainActor + func testReconciledSelectionFallsBackToActiveWorkspaceWhenPreviousSelectionIsGone() { + let first = UUID() + let second = UUID() + let removed = UUID() + + let result = SidebarWorkspaceSelectionSyncPolicy.reconciledSelection( + previousSelectionIds: [removed], + liveWorkspaceIds: [first, second], + fallbackSelectedWorkspaceId: second + ) + + XCTAssertEqual(result, [second]) + } +} + + final class WorkspaceReorderTests: XCTestCase { @MainActor func testReorderWorkspacePostsMovedWorkspaceId() { From 29e24c6fc0ac827612e85cd348a4d66b1e160d17 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Wed, 27 May 2026 03:32:25 -0700 Subject: [PATCH 3/3] Harden workspace batch action coverage --- Sources/TabManager.swift | 5 +++-- cmuxTests/WorkspaceStressProfileTests.swift | 16 ++++++++++++---- cmuxTests/WorkspaceUnitTests.swift | 12 +++++++----- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index a8e191d9f08b..86a55cb210d9 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -5693,8 +5693,9 @@ class TabManager: ObservableObject { if pinned { changedWorkspaces = changedIds.compactMap { workspacesById[$0] } } else { - // Match the existing repeated setPinned behavior: each unpinned item - // moves to the front of the unpinned segment as it is processed. + // Keep parity with reorderTabForPinnedState: each unpinned item + // is inserted at the front of the unpinned segment, so rebuilding a + // batch in one pass must reverse the changed input order. changedWorkspaces = changedIds.reversed().compactMap { workspacesById[$0] } } diff --git a/cmuxTests/WorkspaceStressProfileTests.swift b/cmuxTests/WorkspaceStressProfileTests.swift index 15c63bf99458..a2d49caf06df 100644 --- a/cmuxTests/WorkspaceStressProfileTests.swift +++ b/cmuxTests/WorkspaceStressProfileTests.swift @@ -233,17 +233,21 @@ final class WorkspaceStressProfileTests: XCTestCase { timed("pass-\(label(for: pass))-color-apply", collectInto: &colorSamples) { manager.applyWorkspaceColor("#1565C0", toWorkspaceIds: workspaceIds) } + XCTAssertTrue(manager.tabs.allSatisfy { $0.customColor == "#1565C0" }) timed("pass-\(label(for: pass))-color-clear", collectInto: &colorSamples) { manager.applyWorkspaceColor(nil, toWorkspaceIds: workspaceIds) } + XCTAssertTrue(manager.tabs.allSatisfy { $0.customColor == nil }) timed("pass-\(label(for: pass))-scrollbar-hide", collectInto: &scrollBarSamples) { manager.setWorkspaceTerminalScrollBarHidden(hidden: true, forWorkspaceIds: workspaceIds) } + XCTAssertTrue(manager.tabs.allSatisfy { $0.terminalScrollBarHidden }) timed("pass-\(label(for: pass))-scrollbar-show", collectInto: &scrollBarSamples) { manager.setWorkspaceTerminalScrollBarHidden(hidden: false, forWorkspaceIds: workspaceIds) } - timed("pass-\(label(for: pass))-pin", collectInto: &pinSamples) { - _ = WorkspaceActionDispatcher.performPinAction( + XCTAssertTrue(manager.tabs.allSatisfy { !$0.terminalScrollBarHidden }) + let pinResult = timed("pass-\(label(for: pass))-pin", collectInto: &pinSamples) { + WorkspaceActionDispatcher.performPinAction( WorkspaceActionDispatcher.PinState( targetWorkspaceIds: workspaceIds, anchorWorkspaceId: anchorWorkspaceId, @@ -252,8 +256,10 @@ final class WorkspaceStressProfileTests: XCTestCase { in: manager ) } - timed("pass-\(label(for: pass))-unpin", collectInto: &unpinSamples) { - _ = WorkspaceActionDispatcher.performPinAction( + XCTAssertFalse(pinResult.changedWorkspaceIds.isEmpty) + XCTAssertTrue(manager.tabs.allSatisfy { $0.isPinned }) + let unpinResult = timed("pass-\(label(for: pass))-unpin", collectInto: &unpinSamples) { + WorkspaceActionDispatcher.performPinAction( WorkspaceActionDispatcher.PinState( targetWorkspaceIds: workspaceIds, anchorWorkspaceId: anchorWorkspaceId, @@ -262,6 +268,8 @@ final class WorkspaceStressProfileTests: XCTestCase { in: manager ) } + XCTAssertFalse(unpinResult.changedWorkspaceIds.isEmpty) + XCTAssertTrue(manager.tabs.allSatisfy { !$0.isPinned }) } XCTAssertEqual(manager.tabs.count, config.workspaceCount) diff --git a/cmuxTests/WorkspaceUnitTests.swift b/cmuxTests/WorkspaceUnitTests.swift index e808edd7c33a..7513082ca48a 100644 --- a/cmuxTests/WorkspaceUnitTests.swift +++ b/cmuxTests/WorkspaceUnitTests.swift @@ -235,11 +235,12 @@ final class SidebarSelectedWorkspaceColorTests: XCTestCase { let first = manager.tabs[0] let second = manager.addWorkspace() let third = manager.addWorkspace() + manager.applyWorkspaceColor("#C0392B", toWorkspaceIds: [second.id]) manager.applyWorkspaceColor("#1565C0", toWorkspaceIds: [first.id, third.id]) XCTAssertEqual(first.customColor, "#1565C0") - XCTAssertNil(second.customColor) + XCTAssertEqual(second.customColor, "#C0392B") XCTAssertEqual(third.customColor, "#1565C0") } @@ -249,12 +250,13 @@ final class SidebarSelectedWorkspaceColorTests: XCTestCase { let first = manager.tabs[0] let second = manager.addWorkspace() let third = manager.addWorkspace() + manager.setWorkspaceTerminalScrollBarHidden(hidden: true, forWorkspaceIds: [first.id, second.id, third.id]) - manager.setWorkspaceTerminalScrollBarHidden(hidden: true, forWorkspaceIds: [first.id, third.id]) + manager.setWorkspaceTerminalScrollBarHidden(hidden: false, forWorkspaceIds: [first.id, third.id]) - XCTAssertTrue(first.terminalScrollBarHidden) - XCTAssertFalse(second.terminalScrollBarHidden) - XCTAssertTrue(third.terminalScrollBarHidden) + XCTAssertFalse(first.terminalScrollBarHidden) + XCTAssertTrue(second.terminalScrollBarHidden) + XCTAssertFalse(third.terminalScrollBarHidden) } }