Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/swift-file-length-budget.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
17717 Sources/AppDelegate.swift
16112 Sources/ContentView.swift
13950 Sources/TerminalController.swift
12758 Sources/Workspace.swift
12772 Sources/Workspace.swift
12144 cmuxTests/AppDelegateShortcutRoutingTests.swift
11924 Sources/Panels/BrowserPanel.swift
11867 Sources/GhosttyTerminalView.swift
Expand All @@ -20,7 +20,7 @@
6084 Sources/TextBoxInput.swift
5915 cmuxTests/TerminalAndGhosttyTests.swift
5573 cmuxTests/BrowserConfigTests.swift
5566 Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
5507 Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
4487 Sources/Panels/FilePreviewPanel.swift
4478 Sources/cmuxApp.swift
4401 cmuxTests/BrowserPanelTests.swift
Expand Down Expand Up @@ -123,9 +123,9 @@
754 Sources/TerminalController+ControlWorkspaceContext.swift
752 cmuxUITests/CloseWorkspaceCmdDUITests.swift
738 Packages/macOS/CMUXProjectModel/Sources/CMUXProjectModel/XcodeProjectAdapter.swift
736 Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift
726 cmuxTests/CLICodexHookTimeoutRegressionTests.swift
722 Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift
718 Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift
716 Sources/TaskManagerSnapshot.swift
715 Sources/AppleScriptSupport.swift
710 Sources/TerminalSSHSessionDetector.swift
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,22 @@ public final class PaneTreeModel<Panel> {

/// Mapping from bonsplit `TabID` (surface id) to the owning panel id
/// (legacy `Workspace.surfaceIdToPanelId`).
public var surfaceIdToPanelId: [TabID: UUID] = [:]
///
/// A panel can be mounted under only one bonsplit surface at a time.
/// Rebinding the same panel id to a new surface id removes any stale
/// surface entries for that panel so focus and input never resolve two
/// tabs to one live PTY.
public private(set) var surfaceIdToPanelId: [TabID: UUID] = [:]

/// Snapshot of the spatially ordered panel ids from the last geometry
/// notification, used to gate `paneLayoutVersion` bumps to genuine
/// reorder events (legacy `Workspace.lastOrderedPanelIds`).
public var lastOrderedPanelIds: [UUID] = []

/// Reverse index for targeted panel lookups and stale-surface removal.
@ObservationIgnored
private var panelIdToSurfaceId: [UUID: TabID] = [:]

@ObservationIgnored
private weak var host: (any PaneTreeHosting<Panel>)?

Expand All @@ -55,17 +64,50 @@ public final class PaneTreeModel<Panel> {
self.host = host
}

/// Binds a bonsplit surface id to a panel id.
///
/// The binding is exclusive by panel id: a live panel can be represented by
/// only one surface at a time, so rebinding the panel removes stale surface
/// entries before installing the new owner.
public func bindSurface(_ surfaceId: TabID, toPanelId panelId: UUID) {
if let previousSurfaceId = panelIdToSurfaceId[panelId],
previousSurfaceId != surfaceId {
surfaceIdToPanelId.removeValue(forKey: previousSurfaceId)
}
if let previousPanelId = surfaceIdToPanelId[surfaceId],
previousPanelId != panelId,
panelIdToSurfaceId[previousPanelId] == surfaceId {
panelIdToSurfaceId.removeValue(forKey: previousPanelId)
}

surfaceIdToPanelId[surfaceId] = panelId
panelIdToSurfaceId[panelId] = surfaceId
}

/// Removes the mapping for one bonsplit surface id.
public func removeSurfaceMapping(forSurfaceId surfaceId: TabID) {
if let panelId = surfaceIdToPanelId.removeValue(forKey: surfaceId),
panelIdToSurfaceId[panelId] == surfaceId {
panelIdToSurfaceId.removeValue(forKey: panelId)
}
}

/// Removes every mapping that can still resolve to a closed panel.
public func removeSurfaceMappings(forPanelId panelId: UUID) {
if let surfaceId = panelIdToSurfaceId.removeValue(forKey: panelId) {
surfaceIdToPanelId.removeValue(forKey: surfaceId)
}
}

/// Resolves the owning panel id for a bonsplit surface id (legacy
/// `Workspace.panelIdFromSurfaceId`).
public func panelId(forSurfaceId surfaceId: TabID) -> UUID? {
surfaceIdToPanelId[surfaceId]
}

/// Resolves the bonsplit surface id currently mapped to a panel id
/// (legacy `Workspace.surfaceIdFromPanelId`). When multiple surfaces map
/// to the same panel the match is dictionary-order arbitrary, exactly as
/// the legacy `first(where:)` lookup was.
/// (legacy `Workspace.surfaceIdFromPanelId`).
public func surfaceId(forPanelId panelId: UUID) -> TabID? {
surfaceIdToPanelId.first { $0.value == panelId }?.key
panelIdToSurfaceId[panelId]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ struct PaneTreeModelTests {
let model = PaneTreeModel<String>()
let tabId = TabID()
let panelId = UUID()
model.surfaceIdToPanelId[tabId] = panelId
model.bindSurface(tabId, toPanelId: panelId)
#expect(model.surfaceIdToPanelId[tabId] == panelId)
model.lastOrderedPanelIds = [panelId]
#expect(model.lastOrderedPanelIds == [panelId])
Expand All @@ -94,11 +94,74 @@ struct PaneTreeModelTests {
let model = PaneTreeModel<String>()
let tabId = TabID()
let panelId = UUID()
model.surfaceIdToPanelId[tabId] = panelId
model.bindSurface(tabId, toPanelId: panelId)

#expect(model.panelId(forSurfaceId: tabId) == panelId)
#expect(model.surfaceId(forPanelId: panelId) == tabId)
#expect(model.panelId(forSurfaceId: TabID()) == nil)
#expect(model.surfaceId(forPanelId: UUID()) == nil)
}

/// Rebinding one live panel to a new bonsplit surface must not leave the
/// old surface id resolving to the same panel.
@Test func rebindingPanelToNewSurfaceInvalidatesOldSurfaceMapping() {
let model = PaneTreeModel<String>()
let oldTabId = TabID()
let newTabId = TabID()
let panelId = UUID()

model.bindSurface(oldTabId, toPanelId: panelId)
model.bindSurface(newTabId, toPanelId: panelId)

#expect(model.panelId(forSurfaceId: oldTabId) == nil)
#expect(model.panelId(forSurfaceId: newTabId) == panelId)
#expect(model.surfaceId(forPanelId: panelId) == newTabId)
}

/// Reusing one bonsplit surface for a different panel must also clear the
/// old panel's reverse lookup.
@Test func rebindingSurfaceToNewPanelInvalidatesOldPanelMapping() {
let model = PaneTreeModel<String>()
let tabId = TabID()
let oldPanelId = UUID()
let newPanelId = UUID()

model.bindSurface(tabId, toPanelId: oldPanelId)
model.bindSurface(tabId, toPanelId: newPanelId)

#expect(model.panelId(forSurfaceId: tabId) == newPanelId)
#expect(model.surfaceId(forPanelId: oldPanelId) == nil)
#expect(model.surfaceId(forPanelId: newPanelId) == tabId)
}

/// Close cleanup removes the surface owned by the closed panel.
@Test func closedPanelCleanupRemovesClosedSurfaceMapping() {
let model = PaneTreeModel<String>()
let closedPanelTabId = TabID()
let closedPanelId = UUID()

model.bindSurface(closedPanelTabId, toPanelId: closedPanelId)
model.removeSurfaceMappings(forPanelId: closedPanelId)

#expect(model.panelId(forSurfaceId: closedPanelTabId) == nil)
#expect(model.surfaceId(forPanelId: closedPanelId) == nil)
}

/// Close cleanup removes stale aliases for the closed panel without using
/// a stale tab id to drop a surface that has already moved to another panel.
@Test func closedPanelCleanupKeepsReboundSurfaceMapping() {
let model = PaneTreeModel<String>()
let reboundTabId = TabID()
let closedPanelId = UUID()
let livePanelId = UUID()

model.bindSurface(reboundTabId, toPanelId: closedPanelId)
model.bindSurface(reboundTabId, toPanelId: livePanelId)

model.removeSurfaceMappings(forPanelId: closedPanelId)

#expect(model.panelId(forSurfaceId: reboundTabId) == livePanelId)
#expect(model.surfaceId(forPanelId: closedPanelId) == nil)
#expect(model.surfaceId(forPanelId: livePanelId) == reboundTabId)
}
Comment thread
austinywang marked this conversation as resolved.
}
6 changes: 3 additions & 3 deletions Sources/Workspace+CustomSidebarPane.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ extension Workspace {
return nil
}

surfaceIdToPanelId[newTabId] = customPanel.id
bindSurface(newTabId, toPanelId: customPanel.id)
if let targetIndex {
_ = bonsplitController.reorderTab(newTabId, toIndex: targetIndex)
}
Expand Down Expand Up @@ -149,7 +149,7 @@ extension Workspace {
isLoading: false,
isPinned: false
)
surfaceIdToPanelId[newTab.id] = customPanel.id
bindSurface(newTab.id, toPanelId: customPanel.id)
let previousHostedView = focusedTerminalPanel?.hostedView

isProgrammaticSplit = true
Expand All @@ -162,7 +162,7 @@ extension Workspace {
) else {
panels.removeValue(forKey: customPanel.id)
panelTitles.removeValue(forKey: customPanel.id)
surfaceIdToPanelId.removeValue(forKey: newTab.id)
removeSurfaceMapping(forSurfaceId: newTab.id)
return nil
}

Expand Down
6 changes: 1 addition & 5 deletions Sources/Workspace+PanelLifecycle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,7 @@ extension Workspace {
panels.removeValue(forKey: panelId)
untrackRemoteTerminalSurface(panelId)
pendingRemoteTerminalChildExitSurfaceIds.remove(panelId)
if let tabId {
surfaceIdToPanelId.removeValue(forKey: tabId)
} else {
surfaceIdToPanelId = surfaceIdToPanelId.filter { $0.value != panelId }
}
removeSurfaceMappings(forPanelId: panelId)

panelDirectories.removeValue(forKey: panelId)
panelGitBranches.removeValue(forKey: panelId)
Expand Down
Loading
Loading