Keyboard workspace cycling mirrors the sidebar: collapsed groups are one stop

Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Navigation/WorkspaceAdjacentSelection.swift

@@ -0,0 +1,27 @@
+public import Foundation
+
+/// Resolves the target of a next/previous workspace cycling step over the
+/// rows the sidebar shows.
+public enum WorkspaceAdjacentSelection {
+    /// The workspace to select when stepping from `current` by `step`
+    /// (+1 next, -1 previous), wrapping at the edges of `stops`.
+    ///
+    /// - Parameters:
+    ///   - stops: Visible row workspace ids in sidebar order.
+    ///   - current: The selected workspace id.
+    ///   - fallbackStop: Stop standing in for `current` when it is not a
+    ///     stop itself (a hidden member's group header).
+    /// - Returns: The target stop, or nil when there is no position to step
+    ///   from. The target may equal `current` when it is the only stop.
+    public static func target(
+        stops: [UUID],
+        current: UUID,
+        fallbackStop: UUID? = nil,
+        step: Int
+    ) -> UUID? {
+        guard !stops.isEmpty else { return nil }
+        guard let index = stops.firstIndex(of: current)
+            ?? fallbackStop.flatMap({ stops.firstIndex(of: $0) }) else { return nil }
+        return stops[(index + step + stops.count) % stops.count]
+    }
+}

Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Navigation/WorkspaceAdjacentSelectionTests.swift

@@ -0,0 +1,42 @@
+import Foundation
+import Testing
+@testable import CmuxWorkspaces
+
+@Suite("Workspace adjacent selection")
+struct WorkspaceAdjacentSelectionTests {
+    let a = UUID()
+    let b = UUID()
+    let c = UUID()
+    let hidden = UUID()
+
+    @Test func stepsForwardAndWrapsAtTheEnd() {
+        let stops = [a, b, c]
+        #expect(WorkspaceAdjacentSelection.target(stops: stops, current: a, step: 1) == b)
+        #expect(WorkspaceAdjacentSelection.target(stops: stops, current: c, step: 1) == a)
+    }
+
+    @Test func stepsBackwardAndWrapsAtTheStart() {
+        let stops = [a, b, c]
+        #expect(WorkspaceAdjacentSelection.target(stops: stops, current: b, step: -1) == a)
+        #expect(WorkspaceAdjacentSelection.target(stops: stops, current: a, step: -1) == c)
+    }
+
+    @Test func hiddenCurrentStepsFromItsFallbackStop() {
+        let stops = [a, b, c]
+        #expect(WorkspaceAdjacentSelection.target(
+            stops: stops, current: hidden, fallbackStop: b, step: 1
+        ) == c)
+        #expect(WorkspaceAdjacentSelection.target(
+            stops: stops, current: hidden, fallbackStop: b, step: -1
+        ) == a)
+    }
+
+    @Test func singleStopReturnsItself() {
+        #expect(WorkspaceAdjacentSelection.target(stops: [a], current: a, step: 1) == a)
+    }
+
+    @Test func unresolvableCurrentReturnsNil() {
+        #expect(WorkspaceAdjacentSelection.target(stops: [a], current: hidden, step: 1) == nil)
+        #expect(WorkspaceAdjacentSelection.target(stops: [], current: a, step: 1) == nil)
+    }
+}

Sources/TabManager.swift

@@ -3614,39 +3614,51 @@ class TabManager: ObservableObject {
     }
 
     func selectNextTab() {
-        guard let currentId = selectedTabId,
-              let currentIndex = tabs.firstIndex(where: { $0.id == currentId }) else { return }
-        let nextIndex = (currentIndex + 1) % tabs.count
+        selectAdjacentTab(step: 1, debugLabel: "next")
+    }
+
+    func selectPreviousTab() {
+        selectAdjacentTab(step: -1, debugLabel: "prev")
+    }
+
+    /// Cycle selection through the rows the sidebar shows, wrapping at the
+    /// edges. Stops come from `SidebarWorkspaceRenderItem.renderItems`, so a
+    /// collapsed group is one stop (its header) and its hidden members are
+    /// skipped. Previously cycling visited every workspace in `tabs` order,
+    /// which expanded collapsed groups as a side effect.
+    private func selectAdjacentTab(step: Int, debugLabel: String) {
+        guard let currentId = selectedTabId else { return }
+        let groupsById = Dictionary(
+            workspaceGroups.map { ($0.id, $0) },
+            uniquingKeysWith: { first, _ in first }
+        )
+        let stops = SidebarWorkspaceRenderItem
+            .renderItems(tabs: tabs, groupsById: groupsById)
+            .map(\.rowWorkspaceId)
+        // A hidden selection (member of a collapsed group) steps from its header.
+        let fallbackStop = tabs.first(where: { $0.id == currentId })?
+            .groupId.flatMap { groupsById[$0]?.anchorWorkspaceId }
+        guard let target = WorkspaceAdjacentSelection.target(
+            stops: stops,
+            current: currentId,
+            fallbackStop: fallbackStop,
+            step: step
+        ) else { return }
 #if DEBUG
-        let nextId = tabs[nextIndex].id
-        debugPrepareWorkspaceSwitch("next", from: currentId, to: nextId)
+        if target != currentId {
+            debugPrepareWorkspaceSwitch(debugLabel, from: currentId, to: target)
+        }
 #endif
         activateWorkspaceCycleHotWindow()
         selectWorkspaceId(
-            tabs[nextIndex].id,
+            target,
             notificationDismissalContext: .explicitWorkspaceResume
         )
         // Keyboard nav is an explicit "focus one workspace" gesture, so drop
         // any stale sidebar multi-selection (Shift-click range) so subsequent
         // batch actions don't operate on workspaces the user thought they
         // had unselected by moving on.
-        clearSidebarMultiSelection(except: tabs[nextIndex].id)
-    }
-
-    func selectPreviousTab() {
-        guard let currentId = selectedTabId,
-              let currentIndex = tabs.firstIndex(where: { $0.id == currentId }) else { return }
-        let prevIndex = (currentIndex - 1 + tabs.count) % tabs.count
-#if DEBUG
-        let prevId = tabs[prevIndex].id
-        debugPrepareWorkspaceSwitch("prev", from: currentId, to: prevId)
-#endif
-        activateWorkspaceCycleHotWindow()
-        selectWorkspaceId(
-            tabs[prevIndex].id,
-            notificationDismissalContext: .explicitWorkspaceResume
-        )
-        clearSidebarMultiSelection(except: tabs[prevIndex].id)
+        clearSidebarMultiSelection(except: target)
     }
 
     /// Reduce sidebar multi-selection to a single workspace (or clear if

cmux.xcodeproj/project.pbxproj

@@ -2528,6 +2528,7 @@ C0DE71B10000000000000001 /* AppDelegate+AgentChatNotifications.swift in Sources
 		C0DE36230000000000000001 /* WorkspaceFinderDirectoryResolver.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0DE36230000000000000002 /* WorkspaceFinderDirectoryResolver.swift */; };
 		F0ACC0DE000000000000000B /* WorkspaceForkAgentConversationAvailability.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ACC0DE000000000000000C /* WorkspaceForkAgentConversationAvailability.swift */; };
 		F0ACC0DE0000000000000005 /* WorkspaceForkConversationContextMenuTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ACC0DE0000000000000006 /* WorkspaceForkConversationContextMenuTests.swift */; };
+		C9A57031C9A57031C9A57031 /* WorkspaceGroupKeyboardCycleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57032C9A57032C9A57032 /* WorkspaceGroupKeyboardCycleTests.swift */; };
 		C9A57203C9A57203C9A57203 /* WorkspaceGroupMenuSnapshot.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57204C9A57204C9A57204 /* WorkspaceGroupMenuSnapshot.swift */; };
 		C9A57206C9A57206C9A57206 /* WorkspaceGroupMoveToMenuState.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57205C9A57205C9A57205 /* WorkspaceGroupMoveToMenuState.swift */; };
 		C9A57401C9A57401C9A57401 /* WorkspaceGroupMoveToMenuStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57402C9A57402C9A57402 /* WorkspaceGroupMoveToMenuStateTests.swift */; };
@@ -5132,6 +5133,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
 		C0DE36230000000000000002 /* WorkspaceFinderDirectoryResolver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceFinderDirectoryResolver.swift; sourceTree = "<group>"; };
 		F0ACC0DE000000000000000C /* WorkspaceForkAgentConversationAvailability.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceForkAgentConversationAvailability.swift; sourceTree = "<group>"; };
 		F0ACC0DE0000000000000006 /* WorkspaceForkConversationContextMenuTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceForkConversationContextMenuTests.swift; sourceTree = "<group>"; };
+		C9A57032C9A57032C9A57032 /* WorkspaceGroupKeyboardCycleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceGroupKeyboardCycleTests.swift; sourceTree = "<group>"; };
 		C9A57204C9A57204C9A57204 /* WorkspaceGroupMenuSnapshot.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceGroupMenuSnapshot.swift; sourceTree = "<group>"; };
 		C9A57205C9A57205C9A57205 /* WorkspaceGroupMoveToMenuState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceGroupMoveToMenuState.swift; sourceTree = "<group>"; };
 		C9A57402C9A57402C9A57402 /* WorkspaceGroupMoveToMenuStateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceGroupMoveToMenuStateTests.swift; sourceTree = "<group>"; };
@@ -7815,6 +7817,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
 				604500100000000000000004 /* WindowTitleTemplateTests.swift */,
 				D1FFC0DE000000000000C001 /* DiffCommentStoreTests.swift */,
 				C9A57002C9A57002C9A57002 /* WorkspaceGroupTests.swift */,
+				C9A57032C9A57032C9A57032 /* WorkspaceGroupKeyboardCycleTests.swift */,
 				9164A0029164A0029164A002 /* WorkspaceGroupNumberedSelectionTests.swift */,
 				C9A57402C9A57402C9A57402 /* WorkspaceGroupMoveToMenuStateTests.swift */,
 				FEED49850000000000000002 /* FeedEventClassificationTests.swift */,
@@ -10997,6 +11000,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
 				281F3AEA52379AF45C5C1189 /* WorkspaceCustomSidebarPullRequestContextTests.swift in Sources */,
 				72DB1838A0F1B710BFB45DC1 /* WorkspaceEnvironmentTests.swift in Sources */,
 				F0ACC0DE0000000000000005 /* WorkspaceForkConversationContextMenuTests.swift in Sources */,
+				C9A57031C9A57031C9A57031 /* WorkspaceGroupKeyboardCycleTests.swift in Sources */,
 				C9A57401C9A57401C9A57401 /* WorkspaceGroupMoveToMenuStateTests.swift in Sources */,
 				9164A0019164A0019164A001 /* WorkspaceGroupNumberedSelectionTests.swift in Sources */,
 				C9A57001C9A57001C9A57001 /* WorkspaceGroupTests.swift in Sources */,

cmuxTests/WorkspaceGroupKeyboardCycleTests.swift

@@ -0,0 +1,130 @@
+import Foundation
+import Testing
+
+import CmuxFoundation
+import CmuxSettings
+
+#if canImport(cmux_DEV)
+@testable import cmux_DEV
+#elseif canImport(cmux)
+@testable import cmux
+#endif
+
+@MainActor
+@Suite("Workspace group keyboard cycling", .serialized)
+struct WorkspaceGroupKeyboardCycleTests {
+
+    private func makeTabManager() -> TabManager {
+        let suiteName = "cmux.workspace-group-keyboard-cycle-tests.\(UUID().uuidString)"
+        let defaults = UserDefaults(suiteName: suiteName)!
+        defaults.removePersistentDomain(forName: suiteName)
+        let manager = TabManager(
+            autoWelcomeIfNeeded: false,
+            settings: UserDefaultsSettingsClient(defaults: defaults),
+            closeTabWarningDefaults: defaults
+        )
+        manager.addWorkspace(autoWelcomeIfNeeded: false)
+        return manager
+    }
+
+    private func select(_ workspaceId: UUID, in manager: TabManager) throws {
+        manager.selectWorkspace(try #require(manager.tabs.first { $0.id == workspaceId }))
+    }
+
+    /// One ungrouped workspace plus a group with one member.
+    private func makeGroupedFixture() throws -> (TabManager, UUID, UUID, UUID, UUID) {
+        let manager = makeTabManager()
+        let ungroupedId = try #require(manager.selectedTabId)
+        let member = manager.addTab(select: false)
+        let groupId = try #require(manager.createWorkspaceGroup(
+            name: "Grouped",
+            childWorkspaceIds: [member.id]
+        ))
+        let group = try #require(manager.workspaceGroups.first { $0.id == groupId })
+        return (manager, ungroupedId, groupId, group.anchorWorkspaceId, member.id)
+    }
+
+    @Test func collapsedGroupIsASingleStopAndStaysCollapsed() throws {
+        let (manager, ungroupedId, groupId, anchorId, _) = try makeGroupedFixture()
+        manager.setWorkspaceGroupCollapsed(groupId: groupId, isCollapsed: true)
+        try select(ungroupedId, in: manager)
+
+        manager.selectNextTab()
+        #expect(manager.selectedTabId == anchorId)
+
+        manager.selectNextTab()
+        #expect(manager.selectedTabId == ungroupedId)
+
+        let group = try #require(manager.workspaceGroups.first { $0.id == groupId })
+        #expect(group.isCollapsed)
+    }
+
+    @Test func expandedGroupTraversesItsMembers() throws {
+        let (manager, ungroupedId, _, anchorId, memberId) = try makeGroupedFixture()
+        try select(ungroupedId, in: manager)
+
+        manager.selectNextTab()
+        #expect(manager.selectedTabId == anchorId)
+
+        manager.selectNextTab()
+        #expect(manager.selectedTabId == memberId)
+
+        manager.selectNextTab()
+        #expect(manager.selectedTabId == ungroupedId)
+    }
+
+    @Test func previousDirectionTraversesExpandedMembersInReverse() throws {
+        // Three visible stops, so previous is distinguishable from next.
+        let (manager, ungroupedId, _, anchorId, memberId) = try makeGroupedFixture()
+        try select(ungroupedId, in: manager)
+
+        manager.selectPreviousTab()
+        #expect(manager.selectedTabId == memberId)
+
+        manager.selectPreviousTab()
+        #expect(manager.selectedTabId == anchorId)
+
+        manager.selectPreviousTab()
+        #expect(manager.selectedTabId == ungroupedId)
+    }
+
+    @Test func previousDirectionSkipsHiddenMembersOfACollapsedGroup() throws {
+        let (manager, ungroupedId, groupId, anchorId, _) = try makeGroupedFixture()
+        manager.setWorkspaceGroupCollapsed(groupId: groupId, isCollapsed: true)
+        try select(ungroupedId, in: manager)
+
+        manager.selectPreviousTab()
+        #expect(manager.selectedTabId == anchorId)
+
+        manager.selectPreviousTab()
+        #expect(manager.selectedTabId == ungroupedId)
+
+        let group = try #require(manager.workspaceGroups.first { $0.id == groupId })
+        #expect(group.isCollapsed)
+    }
+
+    @Test func hiddenSelectedMemberStepsRelativeToItsGroupHeader() throws {
+        let (manager, ungroupedId, groupId, _, memberId) = try makeGroupedFixture()
+        try select(memberId, in: manager)
+        // The direct setter preserves focus, so the selected row is hidden.
+        manager.setWorkspaceGroupCollapsed(groupId: groupId, isCollapsed: true)
+        #expect(manager.selectedTabId == memberId)
+
+        manager.selectNextTab()
+        #expect(manager.selectedTabId == ungroupedId)
+
+        let group = try #require(manager.workspaceGroups.first { $0.id == groupId })
+        #expect(group.isCollapsed)
+    }
+
+    @Test func singleVisibleStopKeepsSelection() throws {
+        let manager = makeTabManager()
+        let onlyId = try #require(manager.selectedTabId)
+
+        manager.selectNextTab()
+        #expect(manager.selectedTabId == onlyId)
+
+        manager.selectPreviousTab()
+        #expect(manager.selectedTabId == onlyId)
+    }
+}