Repository navigation
Optimize batch workspace sidebar actions #4865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1229134
5a75eee
954f6ad
29e24c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,50 @@ 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<UUID>() | ||
| let orderedTargetIds = workspaceIds.filter { seen.insert($0).inserted } | ||
| let targetIds = Set(orderedTargetIds) | ||
| var workspacesById: [UUID: Workspace] = [:] | ||
| var changedIdSet = Set<UUID>() | ||
|
|
||
| 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 { | ||
| // 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] } | ||
| } | ||
|
|
||
| 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 | ||
|
Comment on lines
+5692
to
+5706
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed by making the reversed unpin ordering explicitly reference reorderTabForPinnedState's insert-at-front invariant. — Claude Code |
||
| } | ||
|
|
||
| private func reorderTabForPinnedState(_ tab: Workspace) { | ||
| guard let index = tabs.firstIndex(where: { $0.id == tab.id }) else { return } | ||
| tabs.remove(at: index) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tab.setCustomColor(color)directly, bypassing thesetTabColorwrapper used by the single-item path. Both paths are functionally equivalent today because those wrappers do nothing beyond a lookup and a direct call. But any future side effect added to the wrappers (validation, an additional notification, a persistence hook) would be silently skipped for batch operations. Calling through the wrapper keeps both paths in sync automatically.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving this path direct intentionally: calling setTabColor(tabId:) inside the multi-item loop would reintroduce the per-target lookup this PR removes. The batch method owns the one-pass mutation boundary.
— Claude Code