diff --git a/GhosttyTabs.xcodeproj/project.pbxproj b/GhosttyTabs.xcodeproj/project.pbxproj index ab8673d1f..f154b9a1b 100644 --- a/GhosttyTabs.xcodeproj/project.pbxproj +++ b/GhosttyTabs.xcodeproj/project.pbxproj @@ -97,6 +97,7 @@ B9000023A1B2C3D4E5F60719 /* CloseWorkspaceCmdDUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9000022A1B2C3D4E5F60719 /* CloseWorkspaceCmdDUITests.swift */; }; B9000025A1B2C3D4E5F60719 /* CloseWindowConfirmDialogUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9000026A1B2C3D4E5F60719 /* CloseWindowConfirmDialogUITests.swift */; }; D0E0F0B0A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0E0F0B1A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift */; }; + CC000000A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CC000001A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift */; }; D0E0F0B2A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0E0F0B3A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift */; }; FB100000A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FB100001A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift */; }; E1000000A1B2C3D4E5F60718 /* MenuKeyEquivalentRoutingUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1000001A1B2C3D4E5F60718 /* MenuKeyEquivalentRoutingUITests.swift */; }; @@ -294,6 +295,7 @@ B9000022A1B2C3D4E5F60719 /* CloseWorkspaceCmdDUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloseWorkspaceCmdDUITests.swift; sourceTree = ""; }; B9000026A1B2C3D4E5F60719 /* CloseWindowConfirmDialogUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloseWindowConfirmDialogUITests.swift; sourceTree = ""; }; D0E0F0B1A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserPaneNavigationKeybindUITests.swift; sourceTree = ""; }; + CC000001A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GotoSplitCycleUITests.swift; sourceTree = ""; }; D0E0F0B3A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserOmnibarSuggestionsUITests.swift; sourceTree = ""; }; FB100001A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserImportProfilesUITests.swift; sourceTree = ""; }; E1000001A1B2C3D4E5F60718 /* MenuKeyEquivalentRoutingUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MenuKeyEquivalentRoutingUITests.swift; sourceTree = ""; }; @@ -570,6 +572,7 @@ C2577001A1B2C3D4E5F60718 /* TerminalCmdClickUITests.swift */, E6FA9085A1B2C3D4E5F60718 /* WorkspaceDescriptionUITests.swift */, D0E0F0B1A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift */, + CC000001A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift */, D0E0F0B3A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift */, FB100001A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift */, C0B4D9B1A1B2C3D4E5F60718 /* UpdatePillUITests.swift */, @@ -870,6 +873,7 @@ C2577000A1B2C3D4E5F60718 /* TerminalCmdClickUITests.swift in Sources */, E6FA9084A1B2C3D4E5F60718 /* WorkspaceDescriptionUITests.swift in Sources */, D0E0F0B0A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift in Sources */, + CC000000A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift in Sources */, D0E0F0B2A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift in Sources */, FB100000A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift in Sources */, C0B4D9B0A1B2C3D4E5F60718 /* UpdatePillUITests.swift in Sources */, diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 9d04fe352..c4d99c495 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -8513,6 +8513,14 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } guard let tabManager = self.tabManager else { return } + let layout = env["CMUX_UI_TEST_GOTO_SPLIT_LAYOUT"]? + .trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + + if layout == "three_pane_terminal" { + self.setupThreePaneTerminalLayout(tabManager: tabManager) + return + } + let tab = tabManager.addTab() guard let initialPanelId = tab.focusedPanelId else { self.writeGotoSplitTestData(["setupError": "Missing initial panel id"]) @@ -8548,6 +8556,75 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } } + /// Create a 3-pane terminal-only layout: one horizontal split (right) and one vertical split (down). + /// Used by `CMUX_UI_TEST_GOTO_SPLIT_LAYOUT=three_pane_terminal`. + /// Focus changes are recorded by `recordGotoSplitCycleMoveIfNeeded` in the Ghostty action handler. + private func setupThreePaneTerminalLayout(tabManager: TabManager) { + let tab = tabManager.addTab() + guard let initialPanelId = tab.focusedPanelId else { + writeGotoSplitTestData(["setupError": "Missing initial panel id"]) + return + } + + // Create horizontal split (right) + guard tabManager.createSplit( + tabId: tab.id, surfaceId: initialPanelId, direction: .right + ) != nil else { + writeGotoSplitTestData(["setupError": "Failed to create horizontal split"]) + return + } + + // Focus back to initial pane, then create vertical split (down) + tab.focusPanel(initialPanelId) + guard tabManager.createSplit( + tabId: tab.id, surfaceId: initialPanelId, direction: .down + ) != nil else { + writeGotoSplitTestData(["setupError": "Failed to create vertical split"]) + return + } + + // Wait for a terminal surface to become first responder before signaling + // setup complete. Ghostty keybinds only fire when GhosttyNSView has focus. + var observer: NSObjectProtocol? + let deadline = Date().addingTimeInterval(6.0) + + func checkAndSignal() { + guard Date() < deadline else { + if let observer { NotificationCenter.default.removeObserver(observer) } + self.writeGotoSplitTestData(["setupError": "Timed out waiting for terminal focus"]) + return + } + guard let focusedPanelId = tab.focusedPanelId, + tab.terminalPanel(for: focusedPanelId) != nil, + let window = NSApp.mainWindow ?? NSApp.keyWindow, + window.firstResponder is NSView else { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { checkAndSignal() } + return + } + + if let observer { NotificationCenter.default.removeObserver(observer) } + + let allPaneIds = tab.bonsplitController.allPaneIds.map(\.description) + let focusedPaneId = tab.bonsplitController.focusedPaneId?.description ?? "" + + self.writeGotoSplitTestData([ + "paneCount": String(allPaneIds.count), + "allPaneIds": allPaneIds.joined(separator: ","), + "focusedPaneId": focusedPaneId, + "setupComplete": "true", + ]) + } + + observer = NotificationCenter.default.addObserver( + forName: .ghosttyDidFocusSurface, + object: nil, + queue: .main + ) { _ in checkAndSignal() } + + // Also poll in case the notification already fired before we observed. + DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { checkAndSignal() } + } + private func setupBonsplitTabDragUITestIfNeeded() { guard !didSetupBonsplitTabDragUITest else { return } didSetupBonsplitTabDragUITest = true @@ -9483,6 +9560,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent writeGotoSplitTestData(updates) } + func recordGotoSplitCycleMoveIfNeeded(forward: Bool) { + guard isGotoSplitUITestRecordingEnabled() else { return } + guard let tabManager, let workspace = tabManager.selectedWorkspace else { return } + + var updates = gotoSplitFindStateSnapshot(for: workspace) + updates["lastMoveDirection"] = forward ? "next" : "previous" + writeGotoSplitTestData(updates) + } + private func recordGotoSplitSplitIfNeeded(direction: SplitDirection) { guard isGotoSplitUITestRecordingEnabled() else { return } guard let workspace = tabManager?.selectedWorkspace else { return } diff --git a/Sources/GhosttyTerminalView.swift b/Sources/GhosttyTerminalView.swift index 517be0a06..b24d171d7 100644 --- a/Sources/GhosttyTerminalView.swift +++ b/Sources/GhosttyTerminalView.swift @@ -2643,10 +2643,6 @@ class GhosttyApp { private func focusDirection(from direction: ghostty_action_goto_split_e) -> NavigationDirection? { switch direction { - // For previous/next, we use left/right as a reasonable default - // Bonsplit doesn't have cycle-based navigation - case GHOSTTY_GOTO_SPLIT_PREVIOUS: return .left - case GHOSTTY_GOTO_SPLIT_NEXT: return .right case GHOSTTY_GOTO_SPLIT_UP: return .up case GHOSTTY_GOTO_SPLIT_DOWN: return .down case GHOSTTY_GOTO_SPLIT_LEFT: return .left @@ -2834,9 +2830,25 @@ class GhosttyApp { } return true case GHOSTTY_ACTION_GOTO_SPLIT: + let gotoDirection = action.action.goto_split + // Previous/next use cycle-based navigation through all panes in tree order + if gotoDirection == GHOSTTY_GOTO_SPLIT_PREVIOUS || gotoDirection == GHOSTTY_GOTO_SPLIT_NEXT { + guard let tabId = surfaceView.tabId else { return false } + let forward = gotoDirection == GHOSTTY_GOTO_SPLIT_NEXT + return performOnMain { + guard let app = AppDelegate.shared, + let tabManager = app.tabManagerFor(tabId: tabId) ?? app.tabManager else { return false } + let result = tabManager.cycleSplitFocus(tabId: tabId, forward: forward) +#if DEBUG + app.recordGotoSplitCycleMoveIfNeeded(forward: forward) +#endif + return result + } + } + // Directional navigation uses spatial positioning guard let tabId = surfaceView.tabId, let surfaceId = surfaceView.terminalSurface?.id, - let direction = focusDirection(from: action.action.goto_split) else { + let direction = focusDirection(from: gotoDirection) else { return false } return performOnMain { diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 5a793c9c3..d0b3515af 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -4014,6 +4014,13 @@ class TabManager: ObservableObject { return true } + /// Cycle focus to the next or previous pane in tree order, wrapping at the ends. + func cycleSplitFocus(tabId: UUID, forward: Bool) -> Bool { + guard let tab = tabs.first(where: { $0.id == tabId }) else { return false } + tab.cycleFocus(forward: forward) + return true + } + /// Resize split - not directly supported by bonsplit, but we can adjust divider positions func resizeSplit(tabId: UUID, surfaceId: UUID, direction: ResizeDirection, amount: UInt16) -> Bool { guard amount > 0, diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index e94abe72c..b54d9326a 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -10092,6 +10092,35 @@ final class Workspace: Identifiable, ObservableObject { } + /// Cycle focus to the next or previous pane in tree order, wrapping at the ends. + /// Unlike moveFocus(direction:), this visits all panes regardless of spatial layout. + func cycleFocus(forward: Bool) { + let allPaneIds = bonsplitController.allPaneIds + guard allPaneIds.count > 1, + let currentId = bonsplitController.focusedPaneId, + let currentIndex = allPaneIds.firstIndex(of: currentId) else { return } + + // Unfocus the currently-focused panel before navigating. + if let prevPanelId = focusedPanelId, let prev = panels[prevPanelId] { + prev.unfocus() + } + + let targetIndex: Int + if forward { + targetIndex = (currentIndex + 1) % allPaneIds.count + } else { + targetIndex = currentIndex == 0 ? allPaneIds.count - 1 : currentIndex - 1 + } + bonsplitController.focusPane(allPaneIds[targetIndex]) + + // Reconcile selection/focus after navigation so AppKit first-responder and + // bonsplit's focused pane stay aligned. + if let paneId = bonsplitController.focusedPaneId, + let tabId = bonsplitController.selectedTab(inPane: paneId)?.id { + applyTabSelection(tabId: tabId, inPane: paneId) + } + } + // MARK: - Surface Navigation /// Select the next surface in the currently focused pane diff --git a/cmuxUITests/GotoSplitCycleUITests.swift b/cmuxUITests/GotoSplitCycleUITests.swift new file mode 100644 index 000000000..9aa3f81f6 --- /dev/null +++ b/cmuxUITests/GotoSplitCycleUITests.swift @@ -0,0 +1,204 @@ +import XCTest +import Foundation + +/// Tests that goto_split:previous and goto_split:next cycle through ALL panes +/// regardless of split direction (horizontal and vertical), wrapping at the ends. +/// +/// Before the fix, goto_split:previous/next were mapped to directional left/right +/// navigation in Bonsplit, which skipped vertically-split panes and did not wrap. +final class GotoSplitCycleUITests: XCTestCase { + private var dataPath = "" + + override func setUp() { + super.setUp() + continueAfterFailure = false + dataPath = "/tmp/cmux-ui-test-goto-split-cycle-\(UUID().uuidString).json" + try? FileManager.default.removeItem(atPath: dataPath) + } + + // MARK: - Tests + + func testGotoSplitNextCyclesAllPanes() { + // Uses Cmd+] which is Ghostty's default keybind for goto_split:next. + let (app, configCleanup) = launchWithThreePaneLayout() + defer { configCleanup() } + + XCTAssertTrue( + waitForData(keys: ["setupComplete", "allPaneIds", "focusedPaneId"], timeout: 10.0), + "Expected three-pane setup data to be written" + ) + + guard let setup = loadData() else { + XCTFail("Missing setup data") + return + } + XCTAssertEqual(setup["paneCount"], "3", "Expected 3 panes") + + let allPaneIds = Set(setup["allPaneIds"]!.split(separator: ",").map(String.init)) + XCTAssertEqual(allPaneIds.count, 3, "Expected 3 distinct pane IDs") + + let startPane = setup["focusedPaneId"]! + XCTAssertTrue(allPaneIds.contains(startPane), "Start pane should be in allPaneIds") + + // Send goto_split:next (Cmd+]) 3 times — should visit all panes and wrap. + // Ghostty default keybind: super+]=goto_split:next + var visited = [startPane] + for i in 0..<3 { + app.typeKey("]", modifierFlags: [.command]) + + XCTAssertTrue( + waitForDataMatch(timeout: 3.0) { data in + guard let focused = data["focusedPaneId"], !focused.isEmpty else { return false } + return focused != visited.last + }, + "Focus did not change after goto_split:next #\(i + 1)" + ) + + guard let data = loadData(), let focused = data["focusedPaneId"] else { + XCTFail("Missing focusedPaneId after goto_split:next #\(i + 1)") + return + } + visited.append(focused) + } + + let visitedSet = Set(visited.prefix(3)) + XCTAssertEqual(visitedSet, allPaneIds, "goto_split:next should visit all 3 panes") + XCTAssertEqual(visited[3], visited[0], "goto_split:next should wrap back to start") + } + + func testGotoSplitPreviousCyclesAllPanes() { + // Uses Cmd+[ which is Ghostty's default keybind for goto_split:previous. + let (app, configCleanup) = launchWithThreePaneLayout() + defer { configCleanup() } + + XCTAssertTrue( + waitForData(keys: ["setupComplete", "allPaneIds", "focusedPaneId"], timeout: 10.0), + "Expected three-pane setup data to be written" + ) + + guard let setup = loadData() else { + XCTFail("Missing setup data") + return + } + XCTAssertEqual(setup["paneCount"], "3", "Expected 3 panes") + + let allPaneIds = Set(setup["allPaneIds"]!.split(separator: ",").map(String.init)) + XCTAssertEqual(allPaneIds.count, 3, "Expected 3 distinct pane IDs") + + let startPane = setup["focusedPaneId"]! + + var visited = [startPane] + for i in 0..<3 { + app.typeKey("[", modifierFlags: [.command]) + + XCTAssertTrue( + waitForDataMatch(timeout: 3.0) { data in + guard let focused = data["focusedPaneId"], !focused.isEmpty else { return false } + return focused != visited.last + }, + "Focus did not change after goto_split:previous #\(i + 1)" + ) + + guard let data = loadData(), let focused = data["focusedPaneId"] else { + XCTFail("Missing focusedPaneId after goto_split:previous #\(i + 1)") + return + } + visited.append(focused) + } + + let visitedSet = Set(visited.prefix(3)) + XCTAssertEqual(visitedSet, allPaneIds, "goto_split:previous should visit all 3 panes") + XCTAssertEqual(visited[3], visited[0], "goto_split:previous should wrap back to start") + } + + // MARK: - Launch Helpers + + private func launchWithThreePaneLayout() -> (XCUIApplication, () -> Void) { + let fileManager = FileManager.default + guard let appSupport = fileManager.urls(for: .applicationSupportDirectory, in: .userDomainMask).first else { + XCTFail("Missing Application Support directory") + return (XCUIApplication(), {}) + } + + let ghosttyDir = appSupport.appendingPathComponent("com.mitchellh.ghostty", isDirectory: true) + let configURL = ghosttyDir.appendingPathComponent("config.ghostty", isDirectory: false) + + do { + try fileManager.createDirectory(at: ghosttyDir, withIntermediateDirectories: true) + } catch { + XCTFail("Failed to create Ghostty app support dir: \(error)") + return (XCUIApplication(), {}) + } + + let originalConfigData = try? Data(contentsOf: configURL) + let cleanup: () -> Void = { + if let originalConfigData { + try? originalConfigData.write(to: configURL, options: .atomic) + } else { + try? fileManager.removeItem(at: configURL) + } + } + + let home = fileManager.homeDirectoryForCurrentUser + let configContents = "# cmux goto_split cycle UI test\nworking-directory = \(home.path)\n" + + do { + try configContents.write(to: configURL, atomically: true, encoding: .utf8) + } catch { + XCTFail("Failed to write Ghostty config: \(error)") + return (XCUIApplication(), {}) + } + + let app = XCUIApplication() + app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_SETUP"] = "1" + app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_PATH"] = dataPath + app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_LAYOUT"] = "three_pane_terminal" + app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_USE_GHOSTTY_CONFIG"] = "1" + launchAndEnsureForeground(app) + + return (app, cleanup) + } + + // MARK: - Data Polling + + private func waitForData(keys: [String], timeout: TimeInterval) -> Bool { + waitForCondition(timeout: timeout) { + guard let data = self.loadData() else { return false } + return keys.allSatisfy { data[$0] != nil } + } + } + + private func waitForDataMatch(timeout: TimeInterval, predicate: @escaping ([String: String]) -> Bool) -> Bool { + waitForCondition(timeout: timeout) { + guard let data = self.loadData() else { return false } + return predicate(data) + } + } + + private func loadData() -> [String: String]? { + guard let data = try? Data(contentsOf: URL(fileURLWithPath: dataPath)) else { + return nil + } + return (try? JSONSerialization.jsonObject(with: data)) as? [String: String] + } + + private func waitForCondition(timeout: TimeInterval, predicate: @escaping () -> Bool) -> Bool { + let expectation = XCTNSPredicateExpectation( + predicate: NSPredicate { _, _ in predicate() }, + object: nil + ) + return XCTWaiter().wait(for: [expectation], timeout: timeout) == .completed + } + + private func launchAndEnsureForeground(_ app: XCUIApplication, timeout: TimeInterval = 12.0) { + let options = XCTExpectedFailure.Options() + options.isStrict = false + XCTExpectFailure("App activation may fail on headless CI runners", options: options) { + app.launch() + } + + if app.state == .runningForeground { return } + if app.state == .runningBackground { return } + XCTFail("App failed to start. state=\(app.state.rawValue)") + } +}