From 68b7d122e8c28f597f85aad2c7780008dc23536e Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:26:34 +0000 Subject: [PATCH 01/16] test(next panes): resize keys move a strip column's divider in the arrow's direction; one press is one undo step (red) FRAME OWNER REFACTOR KEYS (lead decision, STATUS 8923). Resize Pane Left/Right on a strip column with no horizontal split grew or shrank the focused column alone: the last column's left edge (the divider a person sees) moved against the arrow, and the columns after a middle one moved with it. A right dock grew with Right, so its inner edge moved left. One press must move one divider one step toward the arrow, and change only the two sides of it. A trade is one press: its two widths go out as one daemon transaction, so Undo Layout Change takes it back in one step (each command took its own number). Red job: pending. --- .../KeyboardResizeArrowDirectionTests.swift | 239 ++++++++++++++++++ 1 file changed, 239 insertions(+) create mode 100644 Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift new file mode 100644 index 000000000000..7fb2c8e3d671 --- /dev/null +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift @@ -0,0 +1,239 @@ +import AppKit +import CmuxNextBridge +@testable import CmuxNextApp +@testable import CmuxNextDaemon +@testable import CmuxNextLayout +import CmuxNextTestSupport +import Testing + +/// FRAME OWNER REFACTOR KEYS (lead decision, STATUS 8923; Lawrence asked the lead to decide): Resize Pane Left/Right +/// (Ctrl-Shift-H/L, Ctrl-Cmd-Left/Right) moves the divider in the arrow's direction, as Ghostty's `resize_split` does, +/// in a scrolling strip too. One press moves exactly one divider one step; only the two sides of that divider change; +/// nothing else on screen moves. Before: a strip column with no horizontal split grew or shrank on its own (Right grew +/// the focused column): the last column's left edge, the divider a person sees, moved LEFT (the strip kept the column +/// at the viewport's edge), and the columns after a middle one moved with it. A right dock grew with Right: its inner +/// edge moved left. +@MainActor +@Suite(.closesItsWindows) +struct KeyboardResizeArrowDirectionTests { + final class Panes: LayoutPaneContentProvider { + func makeContentView(for pane: LayoutPaneID) -> NSView { NSView() } + } + + final class Rig { + let provider = Panes() + let model: LayoutModel + let root: LayoutRootView + let window: NSWindow + var ended: [LayoutIntent] = [] + + init(_ columns: [LayoutColumn], focus: LayoutPaneID, animates: Bool = false) { + _ = NSApplication.shared + model = LayoutModel(screens: [LayoutScreen(id: "s", name: "s", layout: .columns(columns))], activeScreenID: "s", + focusedPane: focus) + root = LayoutRootView(model: model, contentProvider: provider) + // Off: each press lands in its own turn (the springs have their own test below). + root.context.reduceMotionOverride = !animates + window = NSWindow(contentRect: NSRect(x: 0, y: 0, width: 1200, height: 800), styleMask: [.borderless], backing: .buffered, + defer: false) + window.isReleasedWhenClosed = false + window.contentView = root + root.layoutSubtreeIfNeeded() + root.driver.isManual = true + model.intentHandler = { [unowned self] intent in + if case .setColumnWidth(_, _, _, _, .ended) = intent { ended.append(intent) } + } + } + + var screen: ScreenContentView { get throws { try #require(root.screenViews["s"]) } } + + /// Every pane's frame as the screen shows it now. + func shown() throws -> [LayoutPaneID: CGRect] { + let screen = try screen + var frames: [LayoutPaneID: CGRect] = [:] + for pane in model.screens[0].layout.panes { frames[pane] = try #require(screen.displayedFrame(of: pane), "\(pane) shows no frame") } + return frames + } + + /// One press, landed. + @discardableResult + func press(_ pane: LayoutPaneID, _ direction: LayoutDirection) -> Bool { + let moved = PaneResize.press(pane, direction, model: model, view: root) + var frames = 0 + while frames < 240, root.advanceFrame(1.0 / 120) { frames += 1 } + root.layoutSubtreeIfNeeded() + return moved + } + + /// One step of a column press in points: 5% of the strip. + var step: CGFloat { (root.bounds.width - root.context.style.stripGap) * CGFloat(PaneResize.step) } + } + + static func columns(_ widths: [Double]) -> [LayoutColumn] { + widths.enumerated().map { LayoutColumn(id: LayoutColumnID("c\($0.offset)"), width: $0.element, root: .leaf(LayoutPaneID("p\($0.offset)"))) } + } + + /// A press moves the divider between `left` and `right` by one step toward `direction` and changes nothing else on + /// screen: every other pane edge, the other columns and the strip's offset stay. + static func expectOnlyTheDividerMoved(_ before: [LayoutPaneID: CGRect], _ after: [LayoutPaneID: CGRect], left: LayoutPaneID, right: LayoutPaneID, + direction: LayoutDirection, step: CGFloat, sourceLocation: SourceLocation = #_sourceLocation) { + let sign: CGFloat = direction == .right ? 1 : -1 + let leftMove = (after[left]?.maxX ?? .nan) - (before[left]?.maxX ?? .nan) + let rightMove = (after[right]?.minX ?? .nan) - (before[right]?.minX ?? .nan) + #expect(abs(leftMove - sign * step) <= 1, "the divider's left side moved \(leftMove), want \(sign * step)", sourceLocation: sourceLocation) + #expect(abs(rightMove - sign * step) <= 1, "the divider's right side moved \(rightMove), want \(sign * step)", sourceLocation: sourceLocation) + #expect(abs((after[left]?.minX ?? .nan) - (before[left]?.minX ?? .nan)) < 0.01, + "\(left)'s left edge moved: \(before[left]?.minX ?? .nan) -> \(after[left]?.minX ?? .nan)", sourceLocation: sourceLocation) + #expect(abs((after[right]?.maxX ?? .nan) - (before[right]?.maxX ?? .nan)) < 0.01, + "\(right)'s right edge moved: \(before[right]?.maxX ?? .nan) -> \(after[right]?.maxX ?? .nan)", sourceLocation: sourceLocation) + for (pane, frame) in before where pane != left && pane != right { + #expect(after[pane] == frame, "\(pane) moved: \(frame) -> \(String(describing: after[pane]))", sourceLocation: sourceLocation) + } + } + + // MARK: Strip columns + + /// The last column has no right neighbor: its left edge is the divider, and it moves with the arrow. + @Test(arguments: [LayoutDirection.right, .left]) + func theLastColumnMovesItsLeftDivider(direction: LayoutDirection) throws { + let rig = Rig(Self.columns([0.5, 0.5]), focus: "p1") + defer { rig.window.closeAfterTest() } + let before = try rig.shown() + #expect(rig.press("p1", direction)) + Self.expectOnlyTheDividerMoved(before, try rig.shown(), left: "p0", right: "p1", direction: direction, step: rig.step) + } + + /// A column with a right neighbor moves its right edge, and the columns after the pair stay where they are. + @Test(arguments: [LayoutDirection.right, .left]) + func aColumnWithARightNeighborMovesItsRightEdgeAndNothingAfterIt(direction: LayoutDirection) throws { + let rig = Rig(Self.columns([0.3, 0.3, 0.3]), focus: "p1") + defer { rig.window.closeAfterTest() } + let before = try rig.shown() + #expect(rig.press("p1", direction)) + Self.expectOnlyTheDividerMoved(before, try rig.shown(), left: "p1", right: "p2", direction: direction, step: rig.step) + } + + /// A strip wider than the window, the last column focused (the strip shows its end): the strip offset stays. + @Test(arguments: [LayoutDirection.right, .left]) + func anOverflowingStripKeepsItsOffset(direction: LayoutDirection) throws { + let rig = Rig(Self.columns([0.5, 0.5, 0.5]), focus: "p2") + defer { rig.window.closeAfterTest() } + let before = try rig.shown() + let offset = try rig.screen.scroll.target + #expect(rig.press("p2", direction)) + #expect(try rig.screen.scroll.target == offset, "the press scrolled the strip") + Self.expectOnlyTheDividerMoved(before, try rig.shown(), left: "p1", right: "p2", direction: direction, step: rig.step) + } + + /// The divider at the edge of the window (the right neighbor is off screen) cannot move out of it: the press is + /// refused as at a minimum width, and nothing moves. + @Test func aDividerAtTheWindowsEdgeDoesNotMoveOutOfIt() throws { + let rig = Rig(Self.columns([0.5, 0.5, 0.5]), focus: "p1") + defer { rig.window.closeAfterTest() } + let before = try rig.shown() + #expect(!rig.press("p1", .right), "the press moved the divider out of the window, or scrolled the strip") + #expect(try rig.shown() == before) + #expect(rig.ended.isEmpty) + } + + /// Every frame of the press's spring: the divider only moves toward the arrow, and the outer edges never move. + @Test func everyFrameOfThePressMovesTheDividerTowardTheArrow() throws { + let rig = Rig(Self.columns([0.5, 0.5]), focus: "p1", animates: true) + defer { rig.window.closeAfterTest() } + let start = try rig.shown() + #expect(PaneResize.press("p1", .right, model: rig.model, view: rig.root)) + var last = try #require(start["p0"]).maxX, frames = 0 + while frames < 240, rig.root.advanceFrame(1.0 / 120) { + rig.root.layoutSubtreeIfNeeded() + let now = try rig.shown() + let divider = try #require(now["p0"]).maxX + #expect(divider >= last - 0.01, "frame \(frames): the divider moved left (\(last) -> \(divider))") + #expect(now["p0"]?.minX == start["p0"]?.minX && now["p1"]?.maxX == start["p1"]?.maxX, "frame \(frames): an outer edge moved") + last = divider + frames += 1 + } + #expect(frames > 1, "the press did not spring") + #expect(abs(last - (try #require(start["p0"]).maxX + rig.step)) <= 1, "the divider rests at \(last)") + } + + /// A trade is one press: both widths go out under one layout transaction. + @Test func aTradeSendsBothWidthsInOneTransaction() throws { + let rig = Rig(Self.columns([0.5, 0.5]), focus: "p1") + defer { rig.window.closeAfterTest() } + #expect(rig.press("p1", .right)) + let sent = rig.ended.compactMap { intent -> (LayoutColumnID, Double, LayoutTransactionID)? in + guard case let .setColumnWidth(column, _, width, transaction, .ended) = intent else { return nil } + return (column, width, transaction) + } + #expect(sent.count == 2, "want the two columns' widths, got \(sent)") + #expect(Set(sent.map(\.2)).count == 1, "the trade's widths went out in \(Set(sent.map(\.2)).count) transactions") + #expect(Set(sent.map(\.0)) == ["c0", "c1"]) + } + + /// One scrolling column has no divider beside it: its right edge moves with the arrow (as before), up to the full + /// strip. + @Test func aSingleColumnMovesItsRightEdge() throws { + let rig = Rig(Self.columns([0.5]), focus: "p0") + defer { rig.window.closeAfterTest() } + let before = try #require(try rig.shown()["p0"]) + #expect(rig.press("p0", .right)) + let wider = try #require(try rig.shown()["p0"]) + #expect(wider.minX == before.minX && abs(wider.maxX - before.maxX - rig.step) <= 1, "\(before) -> \(wider)") + #expect(rig.press("p0", .left)) + let back = try #require(try rig.shown()["p0"]) + #expect(back.minX == before.minX && abs(back.maxX - before.maxX) <= 1, "\(wider) -> \(back)") + } + + // MARK: Side docks + + /// A right dock's inner edge is its left edge: Right moves it right (the dock shrinks), Left moves it left. + @Test(arguments: [LayoutDirection.right, .left]) + func aRightDocksInnerEdgeMovesWithTheArrow(direction: LayoutDirection) throws { + let dock = LayoutColumn(id: "d", width: 0.3, root: .leaf("pd"), dock: DockColumn(edge: .right, mode: .docked)) + let rig = Rig([LayoutColumn(id: "c0", width: 1, root: .leaf("p0")), dock], focus: "pd") + defer { rig.window.closeAfterTest() } + let before = try #require(try rig.shown()["pd"]) + #expect(rig.press("pd", direction)) + let after = try #require(try rig.shown()["pd"]) + let moved = after.minX - before.minX + #expect(direction == .right ? moved > 0 : moved < 0, "Right/Left moved the right dock's inner edge by \(moved)") + #expect(abs(after.maxX - before.maxX) < 0.01, "the dock left its edge") + } + + /// A left dock's inner edge is its right edge: Right grows the dock. + @Test(arguments: [LayoutDirection.right, .left]) + func aLeftDocksInnerEdgeMovesWithTheArrow(direction: LayoutDirection) throws { + let dock = LayoutColumn(id: "d", width: 0.3, root: .leaf("pd"), dock: DockColumn(edge: .left, mode: .docked)) + let rig = Rig([dock, LayoutColumn(id: "c0", width: 1, root: .leaf("p0"))], focus: "pd") + defer { rig.window.closeAfterTest() } + let before = try #require(try rig.shown()["pd"]) + #expect(rig.press("pd", direction)) + let after = try #require(try rig.shown()["pd"]) + let moved = after.maxX - before.maxX + #expect(direction == .right ? moved > 0 : moved < 0, "Right/Left moved the left dock's inner edge by \(moved)") + } +} + +/// Undo Layout Change takes one press back in one step: the daemon keeps one undo entry per transaction +/// (`layout_undo_coalesces_every_target_in_one_resize_transaction`), so every value of one layout transaction (a +/// trade's two widths, an Option or Command drag's release) must go out under one daemon transaction. Each command +/// took a new number: one press was two undo steps. +@MainActor +@Suite(.closesItsWindows) +struct KeyboardResizeOneUndoStepTests { + @Test func theValuesOfOneLayoutTransactionGoOutAsOneDaemonTransaction() throws { + let shown = try PaneCloseControllerTests.Shown([ColumnSnapshot(id: 10, width: 0.5, layout: .leaf(1)), + ColumnSnapshot(id: 11, width: 0.5, layout: .leaf(2))]) + defer { shown.end() } + let content = try #require(shown.content) + let (left, right) = (LayoutPaneID(try shown.key(1)), LayoutPaneID(try shown.key(2))) + let press = LayoutTransactionID.make() + let first = content.nextGestureTransaction + content.handle(.setColumnWidth(LayoutColumnID("column:10"), anyPane: left, width: 0.55, transaction: press, phase: .ended)) + content.handle(.setColumnWidth(LayoutColumnID("column:11"), anyPane: right, width: 0.45, transaction: press, phase: .ended)) + #expect(content.nextGestureTransaction == first + 1, "one press took \(content.nextGestureTransaction - first) daemon transactions") + // The next press is its own undo step. + content.handle(.setColumnWidth(LayoutColumnID("column:10"), anyPane: left, width: 0.6, transaction: .make(), phase: .ended)) + #expect(content.nextGestureTransaction == first + 2) + } +} From ee942a676bac5d341b6776e8b8f4958e14f6c168 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 17:15:10 +0000 Subject: [PATCH 02/16] fix(next panes): resize keys move a strip column's divider in the arrow's direction; one press is one undo step FRAME OWNER REFACTOR KEYS (lead decision, STATUS 8923; Lawrence asked the lead to decide). One press moves exactly one visible divider one step toward the arrow, as Ghostty's resize_split does, and changes only the two sides of that divider. - Strip columns (no split along the axis): the divider is the column's right edge when a strip column follows it, else its left edge (the last column). The two columns trade width (`ShownResizeRange.trade`, the Option-drag trade): the columns after them and the camera stay. The press keeps the left column of the pair in place and reveals nothing (`ColumnScrollState.keptColumn`; camera rule L1 kept the focused column, whose left edge is the divider when it is the last one). The trade is whole device pixels and stops at a column's minimum, the widest share, and the edge of the strip the window shows (a press past it would move the divider out of view). One column alone moves its right edge (as before). - Side docks: the inner edge moves with the arrow (a right dock shrinks with Right). - Splits, rows, bands: unchanged. - Undo: every value of one layout transaction goes out under one daemon transaction (`GestureTransactionNumbers`), so the daemon keeps one undo entry for a trade, and for an Option or Command drag's release too (each command took its own number: one press, or one such release, was several undo steps). - `shownResizeRange` and `keyboardResizeWillChange` move to LayoutRootView+KeyResize.swift. - Specs: PaneResize's doc comment, terminal-shortcuts.md (Resize pane), column-scroll.md L6. Red: bec924e7d90dc1e88a29d9f4 at 68b7d122e8c2 (KeyboardResizeArrowDirectionTests, KeyboardResizeOneUndoStepTests): FAIL, 8 of 10 tests (a single column's right edge and a left dock already moved with the arrow). Green: the same command at this commit, job id in the PR body. --- .../WorkspaceContentController+Intents.swift | 12 ++--- .../WorkspaceContentController.swift | 2 +- .../GestureTransactionNumbers.swift | 24 +++++++++ .../Sources/CmuxNextBridge/PaneResize.swift | 52 +++++++++++++++---- .../Geometry/ShownResizeRange.swift | 42 ++++++++++++++- .../Scroll/ColumnScrollReducer.swift | 11 ++-- .../Scroll/ColumnScrollState.swift | 4 ++ .../Views/LayoutRootView+KeyResize.swift | 24 +++++++++ .../CmuxNextLayout/Views/LayoutRootView.swift | 14 ----- .../KeyboardResizeShownLimitTests.swift | 3 ++ plans/cmux-next/column-scroll.md | 4 +- plans/cmux-next/terminal-shortcuts.md | 2 +- .../highlights/resize-keys-arrow-direction.md | 5 ++ 13 files changed, 160 insertions(+), 39 deletions(-) create mode 100644 Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift create mode 100644 Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift create mode 100644 release-notes/next/highlights/resize-keys-arrow-direction.md diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift index 232cfab72c0b..817a3d1e52be 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift @@ -18,13 +18,13 @@ extension WorkspaceContentController { focus.send(.focusPane(pane.rawValue, source: .intent)) case .setSplitRatio(let split, let ratio, let transaction, _): guard let handle = handles.splits[split] else { return layoutModel.rejectTransaction(transaction) } - let daemonTransaction = nextDaemonTransaction() + let daemonTransaction = gestureTransactions.number(for: transaction) sendGesture(transaction, label: "set-split-ratio") { connection in try await connection.setSplitRatio(handle, ratio: ratio, transaction: daemonTransaction) } case .setColumnWidth(_, let anyPane, let width, let transaction, _): guard let handle = handles.panes[anyPane] else { return layoutModel.rejectTransaction(transaction) } - let daemonTransaction = nextDaemonTransaction() + let daemonTransaction = gestureTransactions.number(for: transaction) sendGesture(transaction, label: "set-viewport-pane-width") { connection in try await connection.setColumnWidth(of: handle, width: width, transaction: daemonTransaction) } @@ -38,7 +38,7 @@ extension WorkspaceContentController { if dock?.edge.isBand == true, !daemon.supports(DaemonCapability.edgeDocks.rawValue) { return layoutModel.rejectTransaction(transaction) } - let daemonTransaction = nextDaemonTransaction() + let daemonTransaction = gestureTransactions.number(for: transaction) let wire = dock.map(LayoutMapping.snapshot) sendGesture(transaction, label: "set-column-dock") { connection in try await connection.setColumnDock(of: handle, dock: wire, transaction: daemonTransaction) @@ -200,10 +200,8 @@ extension WorkspaceContentController { } } - private func nextDaemonTransaction() -> UInt64 { - nextGestureTransaction += 1 - return nextGestureTransaction - } + /// The daemon transaction number the newest layout transaction took (``GestureTransactionNumbers/latest``). + var nextGestureTransaction: UInt64 { gestureTransactions.latest } /// Sends a gesture's one command; its result settles or rolls back the layout's override. private func sendGesture(_ transaction: LayoutTransactionID, label: String, _ body: @escaping DaemonCommandBody) { diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController.swift index 91799f35cfbf..7345a8b96311 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController.swift @@ -49,7 +49,7 @@ final class WorkspaceContentController: LayoutPaneContentProvider { /// plans/cmux-next/focus.md). Every focus change in this content goes /// through it. let focus: FocusCoordinator - var nextGestureTransaction: UInt64 = UInt64(Date().timeIntervalSince1970 * 1000) << 8 + var gestureTransactions = GestureTransactionNumbers() /// The last key-turn pane create (Cmd-D's split, New Column, New Row) still sending: the next one sends after /// it, so one pressed on its stand-in names the daemon's pane (`createInKeysTurn`, `PaneHandlers.split`). var pendingCreate: ActionWork? diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift new file mode 100644 index 000000000000..9665a37e9456 --- /dev/null +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift @@ -0,0 +1,24 @@ +public import CmuxNextLayout +import Foundation + +/// The daemon `transaction` numbers of one workspace's layout gestures. A layout transaction is one gesture (a drag's +/// release, a key press, a split's sizing): every value of it goes out under one number, so the daemon keeps one undo +/// entry for all of them (`layout_undo_coalesces_every_target_in_one_resize_transaction` in cmux-tui-core) and Undo +/// Layout Change takes the gesture back in one step. A gesture sends its values in one turn, one after the other, so +/// the newest transaction is all this needs to remember. +public struct GestureTransactionNumbers { + /// The number the newest layout transaction took; the next one takes the number after it. + public private(set) var latest = UInt64(Date().timeIntervalSince1970 * 1000) << 8 + private var latestTransaction: LayoutTransactionID? + + public init() {} + + /// The daemon number of `transaction`: the newest one's again, or the next one. + public mutating func number(for transaction: LayoutTransactionID) -> UInt64 { + if transaction != latestTransaction { + latest += 1 + latestTransaction = transaction + } + return latest + } +} diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift index 64b51df87970..6d6bc2b2f6f9 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift @@ -1,15 +1,24 @@ public import CmuxNextLayout /// Keyboard pane resizing and column navigation over one screen's layout. -/// Resizing moves the divider nearest the pane in the arrow's direction; -/// in columns mode a left or right -/// resize with no horizontal split in the column changes the column width. -/// A press steps from the size the screen shows and stops at the size it can show (`shown`): one equal step, the -/// part step that reaches a pane's minimum, or no change at all; never a model change that moves nothing. +/// +/// One press moves one divider one step in the arrow's direction, as Ghostty's `resize_split` does, and changes only +/// the two sides of that divider (FRAME OWNER REFACTOR KEYS, lead decision 2026-11-03): +/// - A split: the deepest split along the arrow's axis that holds the pane; its ratio moves toward the arrow. +/// - A strip column (columns mode, no split along the axis): the divider is the column's right edge when a strip +/// column follows it, else its left edge (the last column). The two columns beside it trade width (an Option-drag's +/// trade): the columns after them and the strip's camera stay. One column alone moves its right edge. +/// - A side dock: its inner edge (a left dock grows with Right, a right dock shrinks with Right). A top or bottom +/// dock: its inner edge with Up and Down. A row (rows-v1): the row edge with Up and Down. +/// A press steps from the size the screen shows and stops at the size it can show (`shown`): one equal step, the part +/// step that reaches a pane's minimum or the edge of the strip the window shows, or no change at all; never a model +/// change that moves nothing. public nonisolated enum PaneResize { public enum Change: Equatable, Sendable { case splitRatio(LayoutSplitID, Double) case columnWidth(LayoutColumnID, Double) + /// Two neighbor strip columns, left then right, trade width across the divider between them. + case columnTrade(LayoutColumnID, Double, LayoutColumnID, Double) /// Every row height of a rows-v1 column (permille), one row edge moved. case rowHeights(LayoutColumnID, [RowHeight]) } @@ -45,7 +54,19 @@ public nonisolated enum PaneResize { return extent == column.width ? nil : .columnWidth(column.id, extent) } guard axis == .horizontal else { return nil } - return stepped(column.width, by: delta, within: range, shown: shown?.width(of: column)).map { .columnWidth(column.id, $0) } + let parts = DockStripGeometry.docks(layout.columns) + // A side dock's inner edge: a left dock's right edge, a right dock's left edge. + if parts.left?.id == column.id || parts.right?.id == column.id { + let grow = parts.right?.id == column.id ? -delta : delta + return stepped(column.width, by: grow, within: range, shown: nil).map { .columnWidth(column.id, $0) } + } + let strip = parts.scrolling + guard let index = strip.firstIndex(where: { $0.id == column.id }) else { return nil } + guard strip.count > 1 else { + return stepped(column.width, by: delta, within: range, shown: shown?.width(of: column)).map { .columnWidth(column.id, $0) } + } + let (left, right) = index + 1 < strip.count ? (strip[index], strip[index + 1]) : (strip[index - 1], strip[index]) + return shown?.trade(left, right, by: delta).map { .columnTrade(left.id, $0.left, right.id, $0.right) } } /// `value` moved by `delta` inside `range` cut to `shown`, from where the screen shows it: a value past what the @@ -67,14 +88,25 @@ public nonisolated enum PaneResize { @MainActor public static func press(_ pane: LayoutPaneID, _ direction: LayoutDirection, model: LayoutModel, view: LayoutRootView) -> Bool { guard let screen = model.screen(containing: pane) else { return true } - let change = change(for: pane, direction: direction, in: screen.layout, shown: view.shownResizeRange(ofScreenContaining: pane)) - // Every pane of the screen keeps its content's size through the spring and lays it out once at the landing. - if change != nil { view.keyboardResizeWillChange(screenContaining: pane) } + guard let change = change(for: pane, direction: direction, in: screen.layout, shown: view.shownResizeRange(ofScreenContaining: pane)) else { + return false + } + // Every pane of the screen keeps its content's size through the spring and lays it out once at the landing; a + // trade keeps its left column (and the camera) in place, so only the divider between the two moves. + if case .columnTrade(let left, _, _, _) = change { + view.keyboardResizeWillChange(screenContaining: pane, keeping: left) + } else { + view.keyboardResizeWillChange(screenContaining: pane) + } switch change { case .splitRatio(let split, let ratio): model.setSplitRatio(split, ratio: ratio, transaction: .make(), phase: .ended) case .columnWidth(let column, let width): model.setColumnWidth(column, width: width, transaction: .make(), phase: .ended) + case .columnTrade(let left, let leftWidth, let right, let rightWidth): + // One press, one transaction: the daemon keeps one undo entry for both widths. + let transaction = LayoutTransactionID.make() + model.setColumnWidth(left, width: leftWidth, transaction: transaction, phase: .ended) + model.setColumnWidth(right, width: rightWidth, transaction: transaction, phase: .ended) case .rowHeights(let column, let heights): model.setRowHeights(column, heights: heights, fit: heights.reduce(0) { $0 + $1.height } == 1000) - case nil: return false } // The spring starts in the key's own turn (as a split's frame does, `LayoutRootView.syncWithModel`): the very // next display frame moves, never one that ran before the layout's observation turn. diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift index 3ae9397e9262..4edeb7320b1c 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift @@ -9,15 +9,19 @@ public nonisolated struct ShownResizeRange { let style: LayoutStyle /// The device pixels per point the geometry was placed on (`ScreenGeometry.compute`'s `scale`). let scale: CGFloat + /// Where the strip's camera rests (content-space x of its viewport's leading edge); nil when unknown. + let stripOffset: CGFloat? /// - Parameters: /// - geometry: The screen's geometry at its rest (the one its springs head for). /// - style: The style that geometry was computed with. /// - scale: The backing scale that geometry was computed at. - public init(geometry: ScreenGeometry, style: LayoutStyle, scale: CGFloat = 2) { + /// - stripOffset: Where the strip's camera rests: a trade's divider stays in the part of the strip it shows. + public init(geometry: ScreenGeometry, style: LayoutStyle, scale: CGFloat = 2, stripOffset: CGFloat? = nil) { self.geometry = geometry self.style = style self.scale = scale + self.stripOffset = stripOffset } /// The ratios of `split` that place its divider somewhere new: each side keeps its minimum extent. @@ -41,6 +45,42 @@ public nonisolated struct ShownResizeRange { return lower...max(lower, ColumnWidthPreset.widthRange.upperBound) } + /// The widths of two neighbor strip columns after a keyboard press moves the divider between them by `delta` (a + /// share of the strip; right is positive): `left` takes what `right` gives, or gives what it takes, so the columns + /// after them and the strip stay where they are (the trade of an Option-drag, ``StripStart/trading(_:to:)``). The + /// move is whole device pixels, so both widths round the same way and neither outer edge moves by a pixel. It stops + /// where a column reaches its minimum or the widest share, and where the divider would leave the part of the strip + /// the window shows (``stripOffset``): past it the press would move nothing a person sees. Each column counts from + /// the width it shows (a stored width below its minimum shows at the minimum). Nil when nothing would move. + public func trade(_ left: LayoutColumn, _ right: LayoutColumn, by delta: Double) -> (left: Double, right: Double)? { + guard geometry.isColumns, let leftFrame = geometry.columns[left.id], let rightFrame = geometry.columns[right.id], + case let unit = geometry.stripWidth - style.stripGap, unit > 0, delta != 0 else { return nil } + let range = ColumnWidthPreset.widthRange + func floor(_ column: LayoutColumn) -> Double { + max(range.lowerBound, ColumnStripGeometry.fraction(forPixelWidth: column.minimumSize(style: style).width, + viewportWidth: geometry.stripWidth, gap: style.stripGap)) + } + let shownLeft = min(max(left.width, floor(left)), range.upperBound) + let shownRight = min(max(right.width, floor(right)), range.upperBound) + // How far the divider may go, in points: right until a column reaches a bound, left the same. + var most = CGFloat(min(range.upperBound - shownLeft, shownRight - floor(right))) * unit + var least = -CGFloat(min(shownLeft - floor(left), range.upperBound - shownRight)) * unit + if let stripOffset { + let leading = max(0, geometry.uncoveredMinX - geometry.stripMinX) + let trailing = max(0, geometry.stripMinX + geometry.stripWidth - geometry.uncoveredMaxX) + most = min(most, stripOffset + geometry.stripWidth - trailing - rightFrame.minX) + least = max(least, stripOffset + leading - leftFrame.maxX) + } + // Toward the arrow only, in whole pixels: the step to the nearest one, the room to the last one inside it + // (`edgeTolerance` of a pixel of float slack), so a press never passes a bound. + let pixel = scale > 0 ? scale : 1 + let room = max(0, delta > 0 ? most : -least) + let pixels = min((abs(CGFloat(delta)) * unit * pixel).rounded(), (room * pixel + ColumnStripGeometry.edgeTolerance).rounded(.down)) + guard pixels > 0 else { return nil } + let share = Double((delta > 0 ? pixels : -pixels) / pixel / unit) + return (min(max(shownLeft + share, range.lowerBound), range.upperBound), min(max(shownRight - share, range.lowerBound), range.upperBound)) + } + /// Every row height of `column` after a keyboard press moves the row edge next to `pane`'s row by `delta` /// (a share of the column's height; down is positive): the edge below that row, or above it for the last row. /// The press trades whole permille between the two rows (or, in a scrolling stack, sizes the upper row alone), diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Scroll/ColumnScrollReducer.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Scroll/ColumnScrollReducer.swift index 0929e7fe116c..e299dc8be501 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Scroll/ColumnScrollReducer.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Scroll/ColumnScrollReducer.swift @@ -14,7 +14,9 @@ nonisolated extension ColumnScrollState { switch event { case let .sync(strip, focused, source, animated, reveals, anchor): let before = spring.target - let shift = sync(strip, focused: focused, source: source, reveals: reveals, anchor: anchor) + let kept = keptColumn + keptColumn = nil + let shift = sync(strip, focused: focused, source: source, reveals: reveals, anchor: anchor ?? kept, keepsView: kept != nil) // A sync that leaves the target where the camera shift put it never snaps: a fling or // wheel spring in flight keeps moving while pane layout motion is off (motion.md rule 9). let moved = abs(strip.clamp(spring.target) - (before + shift)) > 0.25 @@ -90,7 +92,8 @@ nonisolated extension ColumnScrollState { /// Returns the camera shift it applied to the value and the target (step 1). @discardableResult - private mutating func sync(_ new: ColumnStrip, focused: PaneID?, source: ColumnFocusSource, reveals: Bool, anchor anchorOverride: ColumnID?) -> CGFloat { + private mutating func sync(_ new: ColumnStrip, focused: PaneID?, source: ColumnFocusSource, reveals: Bool, anchor anchorOverride: ColumnID?, + keepsView: Bool = false) -> CGFloat { let old = strip let oldPane = focusedPane let oldColumn = focusedColumn @@ -171,8 +174,8 @@ nonisolated extension ColumnScrollState { } // 3. Reveal. A live gesture owns the target; its end keeps the - // focused column visible. - guard gesture == nil else { return delta } + // focused column visible. A key press's trade (`keptColumn`) moves only its divider, inside the window. + guard gesture == nil, !keepsView else { return delta } guard reveals else { revealDeferred = true return delta diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Scroll/ColumnScrollState.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Scroll/ColumnScrollState.swift index e47fe8bb01de..560bb0f36a7c 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Scroll/ColumnScrollState.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Scroll/ColumnScrollState.swift @@ -22,6 +22,10 @@ public nonisolated struct ColumnScrollState: Hashable, Sendable { public var remembered: [ColumnID: PaneID] = [:] /// A sync skipped its reveal (live resize drag); the next one reveals. public var revealDeferred = false + /// A keyboard resize press that trades width between two columns (`PaneResize.press`): the next sync keeps this + /// column where it shows and reveals nothing, so only the divider the press moves moves (the camera rule L1 would + /// keep the focused column, whose left edge is that divider when it is the last one). That sync clears it. + public var keptColumn: ColumnID? public struct Gesture: Hashable, Sendable { /// Unbanded offset accumulated from the deltas. diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift new file mode 100644 index 000000000000..450089c87155 --- /dev/null +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift @@ -0,0 +1,24 @@ +import AppKit + +/// The keyboard resize seam (`PaneResize.press`, `resizePane*`): what a press may change on a screen, and what the +/// screen does for it. +extension LayoutRootView { + /// The split ratios and column widths `pane`'s screen can show (``ShownResizeRange``), from the geometry its springs + /// head for and where its strip camera rests; nil for a screen not sized yet. A keyboard resize steps inside them + /// (RESIZE KEYS K1). + public func shownResizeRange(ofScreenContaining pane: PaneID) -> ShownResizeRange? { + guard let screen = model.screen(containing: pane), let view = screenViews[screen.id], view.bounds.width > 0 else { return nil } + return ShownResizeRange(geometry: view.baseGeometry, style: context.style, scale: view.reconciledScale ?? view.scale, + stripOffset: view.scroll.target) + } + + /// A keyboard resize press (`resizePane*`) is about to move `pane`'s screen: until that spring lands, every pane of + /// the screen keeps its content at its size and lays it out once at the landing (RESIZE KEYS K3). `keeping`: the + /// strip column whose place the press's layout keeps (the left one of two columns that trade width): the camera + /// does not move for that layout, so only the divider between them moves (FRAME OWNER REFACTOR KEYS). + public func keyboardResizeWillChange(screenContaining pane: PaneID, keeping column: ColumnID? = nil) { + guard let screen = model.screen(containing: pane), let view = screenViews[screen.id], !view.isHidden else { return } + view.holdsContentForKeyResize = true + view.scrollState.keptColumn = column + } +} diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView.swift index 8abc0b7a707f..d6540dda8f04 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView.swift @@ -257,20 +257,6 @@ public final class LayoutRootView: NSView { return view.navigationFrames } - /// The split ratios and column widths `pane`'s screen can show (``ShownResizeRange``), from the geometry its springs - /// head for; nil for a screen not sized yet. A keyboard resize steps inside them (RESIZE KEYS K1). - public func shownResizeRange(ofScreenContaining pane: PaneID) -> ShownResizeRange? { - guard let screen = model.screen(containing: pane), let view = screenViews[screen.id], view.bounds.width > 0 else { return nil } - return ShownResizeRange(geometry: view.baseGeometry, style: context.style, scale: view.reconciledScale ?? view.scale) - } - - /// A keyboard resize press (`resizePane*`) is about to move `pane`'s screen: until that spring lands, every pane of - /// the screen keeps its content at its size and lays it out once at the landing (RESIZE KEYS K3). - public func keyboardResizeWillChange(screenContaining pane: PaneID) { - guard let screen = model.screen(containing: pane), let view = screenViews[screen.id], !view.isHidden else { return } - view.holdsContentForKeyResize = true - } - /// The width `pane`'s content rests at in the layout this view shows: its frame in the geometry its /// screen's springs head for (the same geometry ``navigationFrames(ofScreenContaining:)`` reads), /// less the pane padding. A split's or a close's panes read their final widths from the turn the diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeShownLimitTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeShownLimitTests.swift index 2576b6866ac9..7ab92188d56e 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeShownLimitTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeShownLimitTests.swift @@ -27,6 +27,8 @@ struct KeyboardResizeShownLimitTests { switch PaneResize.change(for: pane, direction: direction, in: layout, shown: shown) { case .splitRatio(let split, let ratio): return layout.settingRatio(ratio, for: split) case .columnWidth(let column, let width): return layout.settingWidth(width, for: column) + case .columnTrade(let left, let leftWidth, let right, let rightWidth): + return layout.settingWidth(leftWidth, for: left).settingWidth(rightWidth, for: right) case .rowHeights(let column, let heights): return layout.settingRowHeights(heights, for: column) case nil: return nil } @@ -89,6 +91,7 @@ struct KeyboardResizeShownLimitTests { Self.check(towardLimit: Self.moves(from: divider(base), right.edges), back: Self.moves(from: lastRight, backLeft.edges), step: step) } + /// p1 is the last column: Left moves its left edge, so c0 narrows to its minimum and c1 takes the width it gives. @Test func columnPressesStopAtTheColumnMinimumAndComeBackAWholeStep() { let start = ScreenLayout.columns([LayoutColumn(id: "c0", width: 0.5, root: .leaf("p0")), LayoutColumn(id: "c1", width: 0.5, root: .leaf("p1"))]) diff --git a/plans/cmux-next/column-scroll.md b/plans/cmux-next/column-scroll.md index b7091cb93737..1524e08597f4 100644 --- a/plans/cmux-next/column-scroll.md +++ b/plans/cmux-next/column-scroll.md @@ -67,7 +67,9 @@ re-fits. Focusing any other column forgets it. L6. Resizing: the focused column's left edge stays (L1); widening it at the right edge reveals the new edge. During a column-edge or divider drag the view holds and the reveal -runs once the drag ends. +runs once the drag ends. A Resize Pane Left/Right press that trades width between two +columns keeps the left one of them in place and reveals nothing (`ColumnScrollState.keptColumn`): +only the divider between them moves, and it stops at the edge of the strip the window shows. L7. Window resize: frames snap; the focused column keeps its screen x, then F1 fits it. No animation during live resize. diff --git a/plans/cmux-next/terminal-shortcuts.md b/plans/cmux-next/terminal-shortcuts.md index d37373f6095d..8e5624c60c3b 100644 --- a/plans/cmux-next/terminal-shortcuts.md +++ b/plans/cmux-next/terminal-shortcuts.md @@ -49,7 +49,7 @@ terminal program gets the encoded key. FIXED = changed by this audit. | Focus pane in a direction | Cmd-Opt-arrows | Cmd-Opt-arrows `goto_split` | Cmd-Opt-arrows `focusLeft` etc. (also Ctrl-Cmd-H/J/K/L). 2f3e1f95983 had moved Cmd-Opt-Left / Right to the sidebar; back to pane focus 2026-10-20 and reserved (schemas/keybindings/ghostty-reserved-keys.json) | match | | Focus previous / next pane, focus history | Cmd-[ / Cmd-] focus history Back / Forward (no pane previous / next key) | Cmd-[ / Cmd-] `goto_split:previous/next` | Cmd-[ / Cmd-] `focusHistoryBack` / `focusHistoryForward` everywhere but a web page (Back / Forward there), also Ctrl-- / Ctrl-Shift-- outside a terminal. Before: the key router consumed Cmd-[ / Cmd-] outside a browser, so they did nothing in a terminal | FIXED (chief decision): matches the shipping cmux; a `super+[` / `super+]` keybind in the user's Ghostty config still wins | | Focus pane N | Ctrl-Cmd-1...9 | none | Ctrl-Cmd-1...9 `focusPaneByNumber` | match | -| Resize pane | Ctrl-Shift-H/J/K/L; Ctrl-Cmd-arrows (Ghostty) | Ctrl-Cmd-arrows `resize_split` | both, `resizePane*` | match | +| Resize pane | Ctrl-Shift-H/J/K/L; Ctrl-Cmd-arrows (Ghostty) | Ctrl-Cmd-arrows `resize_split` | both, `resizePane*`. One press moves one divider one step in the arrow's direction, as `resize_split` does: the deepest split along that axis; in a strip without one, the column's right edge (its left edge for the last column), the two columns trading width; a side dock's inner edge (`PaneResize`, 2026-11-03) | match | | Zoom pane | Cmd-Shift-Return | Cmd-Shift-Return `toggle_split_zoom` | Cmd-Shift-Return `toggleSplitZoom` | match | | Equalize | Ctrl-Shift-Cmd-= | Ctrl-Cmd-= `equalize_splits` | Ctrl-Cmd-= `equalizeSplits` (Lawrence 2026-10-19; UI scale moved to Ctrl-Shift-Cmd-=/-/0) | matches Ghostty, differs from the shipping cmux | | New pane, auto layout | Ctrl-Cmd-N | none | Ctrl-Cmd-N `newPaneAutoLayout` | match | diff --git a/release-notes/next/highlights/resize-keys-arrow-direction.md b/release-notes/next/highlights/resize-keys-arrow-direction.md new file mode 100644 index 000000000000..9d093bd490e6 --- /dev/null +++ b/release-notes/next/highlights/resize-keys-arrow-direction.md @@ -0,0 +1,5 @@ +title: Resize keys move the divider the way the arrow points +category: improved +docs: https://cmux.com/docs/next/keyboard-shortcuts + +Ctrl-Shift-H and L (or Ctrl-Cmd-Left and Right) move the divider beside the pane toward the arrow, as in Ghostty, in a row of columns and at a side dock too. Only the two panes beside it change size. From 3f079dc47050688a86fa43fce30d55ec126b41ab Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 17:34:35 +0000 Subject: [PATCH 03/16] test(next panes): build the window-edge resize case explicitly The first version assumed the strip camera rests at 0 with the middle column focused. In the test window the screen gets its size after the first sync, so the camera reveals the focused column and the next one shows (the divider was mid-window, and the press may move it: green bec924e7's successor b1940035 showed that). Now the strip shows its start (focus on p0), a click focuses p1 (no scroll: it is fully visible), and the test requires the divider between p1 and p2 at the window's edge before it presses: Right is refused, Left moves the divider into the window. Red (the base plus this test, no fix): scratch commit ed71b453723e, job id in the PR body. Green: the same command at this commit, job id in the PR body. --- .../KeyboardResizeArrowDirectionTests.swift | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift index 7fb2c8e3d671..b65625330813 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift @@ -126,14 +126,22 @@ struct KeyboardResizeArrowDirectionTests { } /// The divider at the edge of the window (the right neighbor is off screen) cannot move out of it: the press is - /// refused as at a minimum width, and nothing moves. + /// refused as at a minimum width, and nothing moves. Left still moves it, into the window. @Test func aDividerAtTheWindowsEdgeDoesNotMoveOutOfIt() throws { - let rig = Rig(Self.columns([0.5, 0.5, 0.5]), focus: "p1") + // The strip shows its start (focus on p0), then a click focuses p1: p0 and p1 fill the window, p2 is past it. + let rig = Rig(Self.columns([0.5, 0.5, 0.5]), focus: "p0") defer { rig.window.closeAfterTest() } + rig.model.focus("p1", source: .pointer) + rig.root.syncWithModel() let before = try rig.shown() + let edge = try rig.screen.bounds.maxX + let p1 = try #require(before["p1"]), p2 = try #require(before["p2"]) + try #require(abs(p2.minX - edge) <= 1 && abs(p1.maxX - edge) <= 1, "the divider between p1 and p2 is not at the window's edge: \(before)") #expect(!rig.press("p1", .right), "the press moved the divider out of the window, or scrolled the strip") #expect(try rig.shown() == before) #expect(rig.ended.isEmpty) + #expect(rig.press("p1", .left)) + Self.expectOnlyTheDividerMoved(before, try rig.shown(), left: "p1", right: "p2", direction: .left, step: rig.step) } /// Every frame of the press's spring: the divider only moves toward the arrow, and the outer edges never move. From c7fa1168c31c935ed4e73b074f6369e50a446d41 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:48:51 +0000 Subject: [PATCH 04/16] test(next panes): a trade from a column shown at its minimum moves no outer edge; a dock at its cap or a lone dock refuses the press (red) Review findings on #34123 (Codex FIX at 3f079dc47050): - ShownResizeRange.trade started a column shown at its minimum from the unrounded minimum, while ColumnStripGeometry.frames shows it at the minimum rounded up to a pixel: with a minimum of 200.1 pt, a press grew it by a pixel less than its neighbor shrank, so the pair's outer edge moved a pixel (scale 1 and 2). - A side dock's press had no shown limit: a dock shown at its share cap (or at the room the strip leaves) took a wider stored width and moved nothing, and the press back took several presses. - A screen whose only column is a dock shows it full width; a press changed its stored width and moved nothing. - aDividerAtTheWindowsEdgeDoesNotMoveOutOfIt checked both sides of the divider against the window's edge, which holds only with a strip gap of at most 1 pt; it checks p1 shown whole and p2's left edge at the window's edge now. Red: job id in the PR body. --- .../KeyboardResizeArrowDirectionTests.swift | 81 ++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift index b65625330813..f07b5b3529e3 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift @@ -134,9 +134,10 @@ struct KeyboardResizeArrowDirectionTests { rig.model.focus("p1", source: .pointer) rig.root.syncWithModel() let before = try rig.shown() + // p1 shows whole, and the divider (the gap from p1's right edge to p2's left edge) ends at the window's edge. let edge = try rig.screen.bounds.maxX let p1 = try #require(before["p1"]), p2 = try #require(before["p2"]) - try #require(abs(p2.minX - edge) <= 1 && abs(p1.maxX - edge) <= 1, "the divider between p1 and p2 is not at the window's edge: \(before)") + try #require(p1.maxX <= edge + 0.01 && abs(p2.minX - edge) <= 0.5, "the divider between p1 and p2 is not at the window's edge: \(before)") #expect(!rig.press("p1", .right), "the press moved the divider out of the window, or scrolled the strip") #expect(try rig.shown() == before) #expect(rig.ended.isEmpty) @@ -222,6 +223,84 @@ struct KeyboardResizeArrowDirectionTests { } } +/// Review of #34123 (Codex): a press steps from what the screen shows, so neither outer edge of a trade moves by a +/// pixel, and a press that would change nothing on screen is refused (no model write). +struct KeyboardResizeShownStepTests { + static let viewport = CGSize(width: 1153, height: 857) + + /// A column minimum with a fraction of a point: `ColumnStripGeometry.frames` shows a column below it at the + /// minimum rounded up to a pixel. + static var style: LayoutStyle { + var style = LayoutStyle() + style.minimumPaneContentSize = CGSize(width: 200.1, height: 64) + return style + } + + static func change(_ pane: LayoutPaneID, _ direction: LayoutDirection, _ layout: ScreenLayout, scale: CGFloat) -> PaneResize.Change? { + let geometry = ScreenGeometry.compute(layout, viewport: viewport, style: style, scale: scale) + return PaneResize.change(for: pane, direction: direction, in: layout, shown: ShownResizeRange(geometry: geometry, style: style, scale: scale)) + } + + /// c0 is stored below its minimum, so it shows widened to the minimum rounded up to a pixel. Right on p0 trades + /// with c1: c0's left edge and c1's right edge stay to the pixel, and the divider moves one whole step. + @Test(arguments: [CGFloat(1), 2]) + func aTradeFromAColumnShownAtItsMinimumMovesNoOuterEdge(scale: CGFloat) throws { + let style = Self.style + let minimum = LayoutColumn(id: "m", root: .leaf("m")).minimumSize(style: style).width + let unit = Self.viewport.width - style.stripGap + let below = ColumnStripGeometry.fraction(forPixelWidth: minimum - 10, viewportWidth: Self.viewport.width, gap: style.stripGap) + let layout = ScreenLayout.columns([LayoutColumn(id: "c0", width: below, root: .leaf("p0")), LayoutColumn(id: "c1", width: 0.5, root: .leaf("p1"))]) + let before = ScreenGeometry.compute(layout, viewport: Self.viewport, style: style, scale: scale) + let c0 = try #require(before.columns["c0"]), c1 = try #require(before.columns["c1"]) + try #require(c0.width > minimum - 10 + 1, "c0 is not shown widened to its minimum (\(c0.width))") + guard case let .columnTrade(left, leftWidth, right, rightWidth)? = Self.change("p0", .right, layout, scale: scale) else { + Issue.record("Right on p0 made no trade") + return + } + let after = ScreenGeometry.compute(layout.settingWidth(leftWidth, for: left).settingWidth(rightWidth, for: right), + viewport: Self.viewport, style: style, scale: scale) + let c0After = try #require(after.columns["c0"]), c1After = try #require(after.columns["c1"]) + #expect(c0After.minX == c0.minX, "c0's left edge moved: \(c0.minX) -> \(c0After.minX)") + #expect(c1After.maxX == c1.maxX, "c1's right edge moved: \(c1.maxX) -> \(c1After.maxX)") + let step = (unit * CGFloat(PaneResize.step) * scale).rounded() / scale + #expect(c0After.maxX - c0.maxX == step, "the divider's left side moved \(c0After.maxX - c0.maxX), want \(step)") + #expect(c1After.minX - c1.minX == step, "the divider's right side moved \(c1After.minX - c1.minX), want \(step)") + // Left on p0 would take c0 below the minimum it shows at: refused. + #expect(Self.change("p0", .left, layout, scale: scale) == nil) + } + + /// A side dock's width shows between its minimum and its share of the window (`DockStripGeometry.maxShare`). A + /// right dock stored past that share shows at the share: Left (grow) is refused, and Right (shrink) is a whole step + /// from the width it shows. + @Test func aDockAtItsShareCapRefusesToGrowAndShrinksAWholeStep() throws { + let style = LayoutStyle() + let dock = LayoutColumn(id: "d", width: 0.9, root: .leaf("pd"), dock: DockColumn(edge: .right, mode: .docked)) + let layout = ScreenLayout.columns([LayoutColumn(id: "c0", width: 1, root: .leaf("p0")), dock]) + func change(_ direction: LayoutDirection) -> PaneResize.Change? { + let geometry = ScreenGeometry.compute(layout, viewport: Self.viewport, style: style) + return PaneResize.change(for: "pd", direction: direction, in: layout, shown: ShownResizeRange(geometry: geometry, style: style)) + } + #expect(change(.left) == nil, "Left grew a dock shown at its share cap: the press changes the model and moves nothing") + guard case let .columnWidth(id, width)? = change(.right) else { + Issue.record("Right on a right dock made no change") + return + } + let before = try #require(ScreenGeometry.compute(layout, viewport: Self.viewport, style: style).columns["d"]) + let after = try #require(ScreenGeometry.compute(layout.settingWidth(width, for: id), viewport: Self.viewport, style: style).columns["d"]) + let step = (Self.viewport.width - style.stripGap) * CGFloat(PaneResize.step) + #expect(abs(after.minX - before.minX - step) <= 1, "the dock's inner edge moved \(after.minX - before.minX), want \(step)") + } + + /// A screen whose only column is a dock shows it full width (`DockStripGeometry.docks`): no press moves it, so none + /// changes its stored width. + @Test(arguments: [LayoutDirection.left, .right]) + func aLoneDockRefusesThePress(direction: LayoutDirection) { + let layout = ScreenLayout.columns([LayoutColumn(id: "d", width: 0.3, root: .leaf("pd"), dock: DockColumn(edge: .right, mode: .docked))]) + let change = Self.change("pd", direction, layout, scale: 2) + #expect(change == nil, "\(direction) on a lone dock made \(String(describing: change)): it moves nothing") + } +} + /// Undo Layout Change takes one press back in one step: the daemon keeps one undo entry per transaction /// (`layout_undo_coalesces_every_target_in_one_resize_transaction`), so every value of one layout transaction (a /// trade's two widths, an Option or Command drag's release) must go out under one daemon transaction. Each command From ce81356da01d4e350c2e04f336cb1da0f946ca96 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:50:04 +0000 Subject: [PATCH 05/16] fix(next panes): a key trade starts from the width a column shows; a side dock steps inside the widths it shows; a lone dock refuses the press Review findings on #34123 (Codex FIX at 3f079dc47050): - ShownResizeRange.trade: a column's floor is its minimum rounded up to a pixel (as ColumnStripGeometry.frames widens it), so a press from a column shown at its minimum moves both sides of the divider by the same whole pixels and neither outer edge. - ShownResizeRange.dockWidth: a side dock shows between its panes' minimum and its share of the window, and a docked dock up to the room the strip leaves at its minimum width beside the other docked dock; PaneResize steps the dock inside it, so a press at the cap is refused and the press back is a whole step. Past that room two docked docks share it in proportion (DockStripGeometry.place), so a press there moves both inner edges: the one case where only one divider cannot move. - A screen whose only column is a dock shows it full width: PaneResize refuses the press. Red: the commit before this one (job id in the PR body). Green: job id in the PR body. --- .../Sources/CmuxNextBridge/PaneResize.swift | 7 +++-- .../Geometry/ShownResizeRange.swift | 29 +++++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift index 6d6bc2b2f6f9..867bebf5579e 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift @@ -55,14 +55,17 @@ public nonisolated enum PaneResize { } guard axis == .horizontal else { return nil } let parts = DockStripGeometry.docks(layout.columns) - // A side dock's inner edge: a left dock's right edge, a right dock's left edge. + // A side dock's inner edge: a left dock's right edge, a right dock's left edge, inside the widths it shows. if parts.left?.id == column.id || parts.right?.id == column.id { let grow = parts.right?.id == column.id ? -delta : delta - return stepped(column.width, by: grow, within: range, shown: nil).map { .columnWidth(column.id, $0) } + return stepped(column.width, by: grow, within: range, shown: shown?.dockWidth(of: column)).map { .columnWidth(column.id, $0) } } let strip = parts.scrolling guard let index = strip.firstIndex(where: { $0.id == column.id }) else { return nil } guard strip.count > 1 else { + // A screen whose only column is a dock shows it full width whatever its width (`DockStripGeometry.docks`): no + // press moves it. + guard strip[index].dock == nil else { return nil } return stepped(column.width, by: delta, within: range, shown: shown?.width(of: column)).map { .columnWidth(column.id, $0) } } let (left, right) = index + 1 < strip.count ? (strip[index], strip[index + 1]) : (strip[index - 1], strip[index]) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift index 4edeb7320b1c..b8b23e277443 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift @@ -51,14 +51,18 @@ public nonisolated struct ShownResizeRange { /// move is whole device pixels, so both widths round the same way and neither outer edge moves by a pixel. It stops /// where a column reaches its minimum or the widest share, and where the divider would leave the part of the strip /// the window shows (``stripOffset``): past it the press would move nothing a person sees. Each column counts from - /// the width it shows (a stored width below its minimum shows at the minimum). Nil when nothing would move. + /// the width it shows: a stored width below its minimum shows at the minimum rounded up to a pixel + /// (`ColumnStripGeometry.frames`), so the trade starts there. Nil when + /// nothing would move. public func trade(_ left: LayoutColumn, _ right: LayoutColumn, by delta: Double) -> (left: Double, right: Double)? { guard geometry.isColumns, let leftFrame = geometry.columns[left.id], let rightFrame = geometry.columns[right.id], case let unit = geometry.stripWidth - style.stripGap, unit > 0, delta != 0 else { return nil } let range = ColumnWidthPreset.widthRange + let widest = max(1, geometry.stripWidth - style.stripGap * 2) func floor(_ column: LayoutColumn) -> Double { - max(range.lowerBound, ColumnStripGeometry.fraction(forPixelWidth: column.minimumSize(style: style).width, - viewportWidth: geometry.stripWidth, gap: style.stripGap)) + let minimum = SplitGeometry.ceilToPixel(min(column.minimumSize(style: style).width, widest), scale: scale) + return max(range.lowerBound, ColumnStripGeometry.fraction(forPixelWidth: minimum, viewportWidth: geometry.stripWidth, + gap: style.stripGap)) } let shownLeft = min(max(left.width, floor(left)), range.upperBound) let shownRight = min(max(right.width, floor(right)), range.upperBound) @@ -81,6 +85,25 @@ public nonisolated struct ShownResizeRange { return (min(max(shownLeft + share, range.lowerBound), range.upperBound), min(max(shownRight - share, range.lowerBound), range.upperBound)) } + /// The width fractions of a side dock the screen shows as different widths (`DockStripGeometry.place`): + /// from its panes' minimum to its share of the window, and for a docked dock to the room the strip leaves at its + /// minimum width beside the other docked dock as it shows. Past that room the two docked docks share it in + /// proportion, so a change of one moves both inner edges. Nil for a column that is not a placed side dock. + public func dockWidth(of column: LayoutColumn) -> ClosedRange? { + let sides = geometry.dock.filter { !$0.dock.edge.isBand } + guard let entry = sides.first(where: { $0.column == column.id }) else { return nil } + let width = geometry.viewport.width, gap = style.stripGap + var widest = max(1, width * (sides.count == 2 ? DockStripGeometry.maxShareBoth : DockStripGeometry.maxShare)) + if entry.dock.mode == .docked { + let others = sides.filter { $0.column != column.id && $0.dock.mode == .docked } + let room = width - min(DockStripGeometry.minimumStripWidth, width / 2) - gap * CGFloat(others.count + 1) + widest = min(widest, room - others.reduce(0) { $0 + $1.frame.width }) + } + let lower = ColumnStripGeometry.fraction(forPixelWidth: column.minimumSize(style: style).width, viewportWidth: width, gap: gap) + let upper = ColumnStripGeometry.fraction(forPixelWidth: widest, viewportWidth: width, gap: gap) + return lower...max(lower, upper) + } + /// Every row height of `column` after a keyboard press moves the row edge next to `pane`'s row by `delta` /// (a share of the column's height; down is positive): the edge below that row, or above it for the last row. /// The press trades whole permille between the two rows (or, in a scrolling stack, sizes the upper row alone), From 2b6dc4d4b7c4fcd11502e596ae6ba2ca2ce7ed5e Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:10:17 +0000 Subject: [PATCH 06/16] test(next panes): key trades on odd-pixel strips move only the divider before and after the f32 echo; a trade settles at its last reply; a refused press keeps no camera anchor (red) Claude review of #34123 at 3f079dc47050 (FIX): - On a strip of an odd number of device pixels 0.5 is an exact half pixel, and the column whose ideal end reaches the window's edge is cut to it: a trade of whole pixels on each stored width rounded the pair two ways or lost the cut, so the last column's right edge moved a pixel at the press (1201 pt at 1x: 1802 -> 1803) and again at the daemon's f32 echo (-> 1801). New: everyPressOnAnOddPixelStripMovesOnlyTheDivider (1x and 2x, camera at the strip's start and end, each press checked at the press and after the echo). - The first of a trade's two replies settled the whole transaction, so a daemon snapshot before the second width's event dropped that width's override: for a frame the right column showed its old width. New: aTradeSettlesWhenItsLastWidthIsAnswered. - A press whose widths the app refused at once never synced, so its kept camera column stayed for the next sync, which skipped its reveal. New: aRefusedPressLeavesTheNextRevealAlone. - The undo test reads GestureTransactionNumbers.latest (the App accessor goes). Red: job id in the PR body. --- .../KeyboardResizeArrowDirectionTests.swift | 111 +++++++++++++++++- 1 file changed, 108 insertions(+), 3 deletions(-) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift index f07b5b3529e3..5984c73c4c08 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift @@ -291,6 +291,58 @@ struct KeyboardResizeShownStepTests { #expect(abs(after.minX - before.minX - step) <= 1, "the dock's inner edge moved \(after.minX - before.minX), want \(step)") } + /// Claude review of #34123: on a strip of an odd number of device pixels, 0.5 of it is an exact half pixel, and the + /// last column is cut back to the strip's end when the columns fill it (`ColumnStripGeometry.frames`): a trade made of + /// whole pixels on each stored width rounded the two columns different ways or left the cut, so a press moved the + /// last column's right edge by a pixel; the daemon keeps a width as `f32`, and its echo moved it again. Every press of + /// a walk across the strip, at 1x and 2x, moves only the divider, by whole pixels toward the arrow, before and after + /// the echo (or is refused). + @Test(arguments: [(CGFloat(1201), CGFloat(1), true), (1963, 1, true), (1453.5, 2, true), (1201, 1, false), (1453.5, 2, false)]) + func everyPressOnAnOddPixelStripMovesOnlyTheDivider(width: CGFloat, scale: CGFloat, atEnd: Bool) throws { + // The app's default strip: edge to edge, no gap (`Metrics.stripColumnGap`). + var style = LayoutStyle() + style.columnGap = 0 + try #require(style.stripGap == 0) + let viewport = CGSize(width: width, height: 800) + var layout = ScreenLayout.columns([LayoutColumn(id: "c0", width: 0.5, root: .leaf("p0")), + LayoutColumn(id: "c1", width: 0.5, root: .leaf("p1")), + LayoutColumn(id: "c2", width: 0.5, root: .leaf("p2"))]) + /// The daemon's echo of a width: the `f32` it keeps. + func echoed(_ layout: ScreenLayout) -> ScreenLayout { + layout.columns.reduce(layout) { $0.settingWidth(Double(Float($1.width)), for: $1.id) } + } + // The camera at the strip's start (c0 and c1 show) or at its end (c1 and c2 show), where the press keeps it. + let presses: [(LayoutPaneID, LayoutDirection)] = atEnd + ? [("p2", .right), ("p2", .left), ("p2", .left), ("p1", .right), ("p0", .right), ("p2", .right)] + : [("p0", .right), ("p1", .left), ("p0", .left), ("p0", .right), ("p1", .right), ("p0", .left)] + let offset = atEnd ? ScreenGeometry.compute(layout, viewport: viewport, style: style, scale: scale).maxOffset : 0 + var moved = 0 + for (pane, direction) in presses { + let before = ScreenGeometry.compute(layout, viewport: viewport, style: style, scale: scale) + let shown = ShownResizeRange(geometry: before, style: style, scale: scale, stripOffset: offset) + guard case let .columnTrade(left, leftWidth, right, rightWidth)? = PaneResize.change(for: pane, direction: direction, in: layout, + shown: shown) else { continue } + let next = layout.settingWidth(leftWidth, for: left).settingWidth(rightWidth, for: right) + for (when, after) in [("at the press", next), ("after the echo", echoed(next))] { + let placed = ScreenGeometry.compute(after, viewport: viewport, style: style, scale: scale) + let l0 = try #require(before.columns[left]), r0 = try #require(before.columns[right]) + let l1 = try #require(placed.columns[left]), r1 = try #require(placed.columns[right]) + let step = l1.maxX - l0.maxX + #expect(direction == .right ? step > 0 : step < 0, "\(pane) \(direction) \(when): the divider moved \(step)") + #expect(r1.minX - r0.minX == step, "\(pane) \(direction) \(when): the divider's sides moved \(step) and \(r1.minX - r0.minX)") + #expect(l1.minX == l0.minX && r1.maxX == r0.maxX, + "\(pane) \(direction) \(when): an outer edge moved (\(l0.minX) -> \(l1.minX), \(r0.maxX) -> \(r1.maxX))") + for (id, frame) in before.columns where id != left && id != right { + #expect(placed.columns[id] == frame, "\(pane) \(direction) \(when): column \(id) moved") + } + #expect((step * scale).rounded() == step * scale, "\(pane) \(direction) \(when): a step of \(step) is no whole pixel") + } + layout = echoed(next) + moved += 1 + } + #expect(moved >= 4, "only \(moved) of \(presses.count) presses moved a divider") + } + /// A screen whose only column is a dock shows it full width (`DockStripGeometry.docks`): no press moves it, so none /// changes its stored width. @Test(arguments: [LayoutDirection.left, .right]) @@ -301,6 +353,59 @@ struct KeyboardResizeShownStepTests { } } +/// Claude review of #34123: a trade sends two widths, one daemon command each, under one transaction. The first +/// command's reply settled the whole transaction, so a daemon snapshot that came before the second command's event +/// (the store applies events once per display frame) dropped the second width's override, and for a frame that column +/// showed its old width and every column after it jumped. The transaction settles when its last value is answered. +@MainActor +struct KeyboardResizeTradeSettleTests { + @Test func aTradeSettlesWhenItsLastWidthIsAnswered() throws { + let columns = [LayoutColumn(id: "c0", width: 0.5, root: .leaf("p0")), LayoutColumn(id: "c1", width: 0.5, root: .leaf("p1"))] + let model = LayoutModel(screens: [LayoutScreen(id: "s", name: "s", layout: .columns(columns))], activeScreenID: "s", focusedPane: "p0") + model.intentHandler = { _ in } // the app sends each width as its own command, and settles it at its reply + func width(_ column: LayoutColumnID) -> Double? { model.screens.first?.layout.columns.first { $0.id == column }?.width } + func daemon(_ c0: Double, _ c1: Double) -> [LayoutScreen] { + [LayoutScreen(id: "s", name: "s", layout: .columns([LayoutColumn(id: "c0", width: c0, root: .leaf("p0")), + LayoutColumn(id: "c1", width: c1, root: .leaf("p1"))]))] + } + let press = LayoutTransactionID.make() + model.setColumnWidth("c0", width: 0.55, transaction: press, phase: .ended) + model.setColumnWidth("c1", width: 0.45, transaction: press, phase: .ended) + // c0's reply, then a daemon snapshot with c0's change only (c1's event comes in the next frame). + model.settleTransaction(press) + model.apply(screens: daemon(0.55, 0.5)) + #expect(width("c0") == 0.55) + #expect(width("c1") == 0.45, "the first reply let a snapshot without the second width show c1 at \(String(describing: width("c1")))") + // c1's reply and its event: the daemon's widths, nothing held over them. + model.settleTransaction(press) + model.apply(screens: daemon(0.55, 0.45)) + model.apply(screens: daemon(0.6, 0.4)) + #expect(width("c0") == 0.6 && width("c1") == 0.4, "a settled trade still held its widths over the daemon's") + } +} + +/// Claude review of #34123: a press whose widths the app refused at once (a pane with no daemon handle yet, such as a +/// column still being made) changed nothing, so its own sync never ran, and the column it kept in place stayed for the +/// next sync, which then skipped its reveal: the column focused next did not scroll into view. +@MainActor +@Suite(.closesItsWindows) +struct KeyboardResizeRefusedPressTests { + @Test func aRefusedPressLeavesTheNextRevealAlone() throws { + let rig = KeyboardResizeArrowDirectionTests.Rig(KeyboardResizeArrowDirectionTests.columns([0.5, 0.5, 0.5]), focus: "p0") + defer { rig.window.closeAfterTest() } + rig.model.intentHandler = { [model = rig.model] intent in + if case let .setColumnWidth(_, _, _, transaction, .ended) = intent { model.rejectTransaction(transaction) } + } + let screen = try rig.screen + #expect(screen.scroll.target == 0) + _ = rig.press("p0", .right) + // A key focus on the column past the window's edge scrolls it into view. + rig.model.focus("p2", source: .keyboard) + rig.root.syncWithModel() + #expect(screen.scroll.target > 0, "the focus after a refused press did not reveal p2") + } +} + /// Undo Layout Change takes one press back in one step: the daemon keeps one undo entry per transaction /// (`layout_undo_coalesces_every_target_in_one_resize_transaction`), so every value of one layout transaction (a /// trade's two widths, an Option or Command drag's release) must go out under one daemon transaction. Each command @@ -315,12 +420,12 @@ struct KeyboardResizeOneUndoStepTests { let content = try #require(shown.content) let (left, right) = (LayoutPaneID(try shown.key(1)), LayoutPaneID(try shown.key(2))) let press = LayoutTransactionID.make() - let first = content.nextGestureTransaction + let first = content.gestureTransactions.latest content.handle(.setColumnWidth(LayoutColumnID("column:10"), anyPane: left, width: 0.55, transaction: press, phase: .ended)) content.handle(.setColumnWidth(LayoutColumnID("column:11"), anyPane: right, width: 0.45, transaction: press, phase: .ended)) - #expect(content.nextGestureTransaction == first + 1, "one press took \(content.nextGestureTransaction - first) daemon transactions") + #expect(content.gestureTransactions.latest == first + 1, "one press took \(content.gestureTransactions.latest - first) daemon transactions") // The next press is its own undo step. content.handle(.setColumnWidth(LayoutColumnID("column:10"), anyPane: left, width: 0.6, transaction: .make(), phase: .ended)) - #expect(content.nextGestureTransaction == first + 2) + #expect(content.gestureTransactions.latest == first + 2) } } From 7dc25a2a2dabd712f7e0800ad32b2796fc8f1229 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:15:29 +0000 Subject: [PATCH 07/16] fix(next panes): a key trade keeps the pair's ideal sum and checks its placement; a trade settles at its last reply; a press's camera anchor ends with its sync Claude review of #34123 at 3f079dc47050 (FIX): - ShownResizeRange.trade moves whole pixels from the widths the columns show, and picks the new stored widths so the pair's ideal sum stays (the columns after it and the cut of a column that ends at the window's edge do not change) and both ideal widths sit the same way from their pixels, away from a rounding tie where the sum allows; else the shown widths themselves. The widths are what the daemon keeps (f32), so its echo moves nothing. It places the candidate as the screen does and takes it only when exactly the divider moves by the step; else the press is refused. - LayoutModel counts the ended values of each transaction (TransactionReplies) and settles it at the reply to its last one, so a snapshot between a trade's two events (the store applies events once per display frame) no longer drops the second width's override. The same held for Option and Command drag releases and a split's sizing. GestureOverride (the override value type) moves out of the LayoutModel type to keep it under its count. - PaneResize.press ends the press's kept camera column after its own sync (LayoutRootView.keyboardResizeDidChange), so a press whose widths were refused at once leaves the next reveal alone. - The App's nextGestureTransaction accessor goes (tests read GestureTransactionNumbers); its doc says a dock pin with a width is still two undo steps. Highlight wording. Red: the commit before this one (job id in the PR body). Green: job id in the PR body. --- .../WorkspaceContentController+Intents.swift | 3 - .../GestureTransactionNumbers.swift | 8 +- .../Sources/CmuxNextBridge/PaneResize.swift | 3 +- .../Geometry/ShownResizeRange.swift | 79 +++++++++++++------ .../Model/LayoutModel+Dock.swift | 1 + .../Model/LayoutModel+SplitSizing.swift | 2 +- .../CmuxNextLayout/Model/LayoutModel.swift | 28 +++---- .../Model/TransactionReplies.swift | 36 +++++++++ .../Views/LayoutRootView+KeyResize.swift | 7 ++ .../highlights/resize-keys-arrow-direction.md | 2 +- 10 files changed, 121 insertions(+), 48 deletions(-) create mode 100644 Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift index 817a3d1e52be..294664813cbb 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift @@ -200,9 +200,6 @@ extension WorkspaceContentController { } } - /// The daemon transaction number the newest layout transaction took (``GestureTransactionNumbers/latest``). - var nextGestureTransaction: UInt64 { gestureTransactions.latest } - /// Sends a gesture's one command; its result settles or rolls back the layout's override. private func sendGesture(_ transaction: LayoutTransactionID, label: String, _ body: @escaping DaemonCommandBody) { Task { [daemon, layoutModel] in diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift index 9665a37e9456..cffb13e501a6 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift @@ -3,9 +3,11 @@ import Foundation /// The daemon `transaction` numbers of one workspace's layout gestures. A layout transaction is one gesture (a drag's /// release, a key press, a split's sizing): every value of it goes out under one number, so the daemon keeps one undo -/// entry for all of them (`layout_undo_coalesces_every_target_in_one_resize_transaction` in cmux-tui-core) and Undo -/// Layout Change takes the gesture back in one step. A gesture sends its values in one turn, one after the other, so -/// the newest transaction is all this needs to remember. +/// entry for all its widths and ratios (`layout_undo_coalesces_every_target_in_one_resize_transaction` in +/// cmux-tui-core) and Undo Layout Change takes the gesture back in one step. (The daemon keeps a dock change apart +/// from resizes, so a dock pin with a width is still two steps.) A gesture sends its values in one turn, one after +/// the other, so the newest transaction is all this needs to remember; a value of an older one (none sends late) +/// would take a new number, a step of its own, as before. public struct GestureTransactionNumbers { /// The number the newest layout transaction took; the next one takes the number after it. public private(set) var latest = UInt64(Date().timeIntervalSince1970 * 1000) << 8 diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift index 867bebf5579e..749571ef2912 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift @@ -69,7 +69,7 @@ public nonisolated enum PaneResize { return stepped(column.width, by: delta, within: range, shown: shown?.width(of: column)).map { .columnWidth(column.id, $0) } } let (left, right) = index + 1 < strip.count ? (strip[index], strip[index + 1]) : (strip[index - 1], strip[index]) - return shown?.trade(left, right, by: delta).map { .columnTrade(left.id, $0.left, right.id, $0.right) } + return shown?.trade(left, right, by: delta, in: layout).map { .columnTrade(left.id, $0.left, right.id, $0.right) } } /// `value` moved by `delta` inside `range` cut to `shown`, from where the screen shows it: a value past what the @@ -114,6 +114,7 @@ public nonisolated enum PaneResize { // The spring starts in the key's own turn (as a split's frame does, `LayoutRootView.syncWithModel`): the very // next display frame moves, never one that ran before the layout's observation turn. view.syncWithModel() + view.keyboardResizeDidChange(screenContaining: pane) return true } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift index b8b23e277443..8f88f6c5affd 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/ShownResizeRange.swift @@ -45,44 +45,77 @@ public nonisolated struct ShownResizeRange { return lower...max(lower, ColumnWidthPreset.widthRange.upperBound) } - /// The widths of two neighbor strip columns after a keyboard press moves the divider between them by `delta` (a - /// share of the strip; right is positive): `left` takes what `right` gives, or gives what it takes, so the columns - /// after them and the strip stay where they are (the trade of an Option-drag, ``StripStart/trading(_:to:)``). The - /// move is whole device pixels, so both widths round the same way and neither outer edge moves by a pixel. It stops - /// where a column reaches its minimum or the widest share, and where the divider would leave the part of the strip - /// the window shows (``stripOffset``): past it the press would move nothing a person sees. Each column counts from - /// the width it shows: a stored width below its minimum shows at the minimum rounded up to a pixel - /// (`ColumnStripGeometry.frames`), so the trade starts there. Nil when + /// The widths of two neighbor strip columns of `layout` after a keyboard press moves the divider between them by + /// `delta` (a share of the strip; right is positive): `left` takes what `right` gives, or gives what it takes, so the + /// columns after them and the strip stay where they are (the trade of an Option-drag, ``StripStart/trading(_:to:)``). + /// + /// The move is whole device pixels from the widths the columns show, and stops where a column reaches its minimum + /// (shown rounded up to a pixel) or the widest share, and where the divider would leave the part of the strip the + /// window shows (``stripOffset``): past it the press would move nothing a person sees. The new widths keep the + /// pair's ideal (unrounded) sum, so the columns after it and the cut of a column that ends at the window's edge + /// (`ColumnStripGeometry.frames`) do not change, and put both ideal widths the same way from their shown pixels, as + /// far from a rounding tie as that allows. They are what the daemon keeps (`f32`), so its echo moves nothing. The + /// result is checked by placing it: the press moves exactly the divider by the step, or it is refused (nil), as when /// nothing would move. - public func trade(_ left: LayoutColumn, _ right: LayoutColumn, by delta: Double) -> (left: Double, right: Double)? { + public func trade(_ left: LayoutColumn, _ right: LayoutColumn, by delta: Double, in layout: ScreenLayout) -> (left: Double, right: Double)? { guard geometry.isColumns, let leftFrame = geometry.columns[left.id], let rightFrame = geometry.columns[right.id], case let unit = geometry.stripWidth - style.stripGap, unit > 0, delta != 0 else { return nil } - let range = ColumnWidthPreset.widthRange - let widest = max(1, geometry.stripWidth - style.stripGap * 2) - func floor(_ column: LayoutColumn) -> Double { - let minimum = SplitGeometry.ceilToPixel(min(column.minimumSize(style: style).width, widest), scale: scale) - return max(range.lowerBound, ColumnStripGeometry.fraction(forPixelWidth: minimum, viewportWidth: geometry.stripWidth, - gap: style.stripGap)) + let strip = geometry.stripWidth, gap = style.stripGap, range = ColumnWidthPreset.widthRange + let pixel = scale > 0 ? scale : 1 + let widest = max(1, strip - gap * 2) + func ideal(_ fraction: Double) -> CGFloat { ColumnStripGeometry.pixelWidth(fraction: fraction, viewportWidth: strip, gap: gap) } + /// The narrowest width a column shows: its minimum rounded up to a pixel, and never below the narrowest share. + func floor(_ column: LayoutColumn) -> CGFloat { + max(SplitGeometry.ceilToPixel(min(column.minimumSize(style: style).width, widest), scale: scale), + SplitGeometry.ceilToPixel(ideal(range.lowerBound), scale: scale)) } - let shownLeft = min(max(left.width, floor(left)), range.upperBound) - let shownRight = min(max(right.width, floor(right)), range.upperBound) // How far the divider may go, in points: right until a column reaches a bound, left the same. - var most = CGFloat(min(range.upperBound - shownLeft, shownRight - floor(right))) * unit - var least = -CGFloat(min(shownLeft - floor(left), range.upperBound - shownRight)) * unit + var most = min(widest - leftFrame.width, rightFrame.width - floor(right)) + var least = -min(leftFrame.width - floor(left), widest - rightFrame.width) if let stripOffset { let leading = max(0, geometry.uncoveredMinX - geometry.stripMinX) let trailing = max(0, geometry.stripMinX + geometry.stripWidth - geometry.uncoveredMaxX) - most = min(most, stripOffset + geometry.stripWidth - trailing - rightFrame.minX) + most = min(most, stripOffset + strip - trailing - rightFrame.minX) least = max(least, stripOffset + leading - leftFrame.maxX) } // Toward the arrow only, in whole pixels: the step to the nearest one, the room to the last one inside it // (`edgeTolerance` of a pixel of float slack), so a press never passes a bound. - let pixel = scale > 0 ? scale : 1 let room = max(0, delta > 0 ? most : -least) let pixels = min((abs(CGFloat(delta)) * unit * pixel).rounded(), (room * pixel + ColumnStripGeometry.edgeTolerance).rounded(.down)) guard pixels > 0 else { return nil } - let share = Double((delta > 0 ? pixels : -pixels) / pixel / unit) - return (min(max(shownLeft + share, range.lowerBound), range.upperBound), min(max(shownRight - share, range.lowerBound), range.upperBound)) + let move = (delta > 0 ? pixels : -pixels) / pixel + let (leftShown, rightShown) = (leftFrame.width + move, rightFrame.width - move) + // The ideal widths that show those: the pair's ideal sum kept, both the same way from their pixels; else (a pair + // whose ideal widths are both on a tie, where that way is the tie itself) the shown widths themselves. + let (leftIdeal, rightIdeal) = (ideal(left.width), ideal(right.width)) + let shift = (leftShown - leftIdeal + rightIdeal - rightShown) / 2 + for (a, b) in [(leftIdeal + shift, rightIdeal - shift), (leftShown, rightShown)] { + let widths = (kept(fraction(a)), kept(fraction(b))) + if movesOnlyTheDivider(layout.settingWidth(widths.0, for: left.id).settingWidth(widths.1, for: right.id), + left: left.id, right: right.id, by: move) { + return widths + } + } + return nil + } + + /// The share of the strip whose ideal width is `width` points. + private func fraction(_ width: CGFloat) -> Double { + ColumnStripGeometry.fraction(forPixelWidth: width, viewportWidth: geometry.stripWidth, gap: style.stripGap) + } + + /// `width` as the daemon keeps a column width (`f32`, cmux-tui `LayoutColumn.width`): the value its echo brings back. + private func kept(_ width: Double) -> Double { Double(Float(width)) } + + /// Whether `candidate`, placed as the screen places it, shows every column where it shows now but the divider between + /// `left` and `right`, moved by `move` points. + private func movesOnlyTheDivider(_ candidate: ScreenLayout, left: ColumnID, right: ColumnID, by move: CGFloat) -> Bool { + let placed = ScreenGeometry.compute(candidate, viewport: geometry.viewport, style: style, scale: scale) + guard let l0 = geometry.columns[left], let r0 = geometry.columns[right], let l1 = placed.columns[left], + let r1 = placed.columns[right] else { return false } + func same(_ a: CGFloat, _ b: CGFloat) -> Bool { abs(a - b) < 0.001 } + guard same(l1.minX, l0.minX), same(r1.maxX, r0.maxX), same(l1.maxX, l0.maxX + move), same(r1.minX, r0.minX + move) else { return false } + return geometry.columns.allSatisfy { id, frame in id == left || id == right || placed.columns[id] == frame } } /// The width fractions of a side dock the screen shows as different widths (`DockStripGeometry.place`): diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel+Dock.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel+Dock.swift index 30db2a3a609c..8f1d7205f609 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel+Dock.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel+Dock.swift @@ -42,6 +42,7 @@ extension LayoutModel { guard let anyPane = screens.lazy.compactMap({ $0.layout.columns.first { $0.id == column }?.root.panes.first }).first else { return .unknownColumn } + expectReply(transaction) emit(.setColumnDock(column, anyPane: anyPane, dock: dock, transaction: transaction)) return nil } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel+SplitSizing.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel+SplitSizing.swift index ebc7fe881395..4a910bc9b0ff 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel+SplitSizing.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel+SplitSizing.swift @@ -83,7 +83,7 @@ extension LayoutModel { func holdSplitRatio(_ split: SplitID, ratio: Double, transaction: LayoutTransactionID) { let ratio = min(max(ratio, SplitRatio.range.lowerBound), SplitRatio.range.upperBound) updateScreen(where: { $0.ratio(of: split) != nil }) { $0.settingRatio(ratio, for: split) } - splitOverrides[split] = Override(value: ratio, transaction: transaction, ended: false) + splitOverrides[split] = GestureOverride(value: ratio, transaction: transaction, ended: false) } /// Whether `split` still shows the value `transaction` set (no later gesture took it over). diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel.swift index ac96ae0aa652..6eb213eea8ea 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel.swift @@ -179,7 +179,7 @@ public final class LayoutModel { /// Receives every intent. Set by the App (or `MockLayoutSource`). @ObservationIgnored public var intentHandler: (@MainActor (LayoutIntent) -> Void)? - @ObservationIgnored var splitOverrides: [SplitID: Override] = [:] + @ObservationIgnored var splitOverrides: [SplitID: GestureOverride] = [:] /// Panes a pending close hides from the shown screens (`showPaneClose`). @ObservationIgnored var paneCloses: [PaneID: PaneCloseHold] = [:] /// The last pending close's order (`PaneCloseHold.sequence`). @@ -195,23 +195,14 @@ public final class LayoutModel { static let explicitReflowWindow: TimeInterval = 0.6 /// An explicit equalize is in its window now. public var animatesReflow: Bool { ProcessInfo.processInfo.systemUptime < explicitReflowUntil } - @ObservationIgnored var widthOverrides: [ColumnID: Override] = [:] + @ObservationIgnored var widthOverrides: [ColumnID: GestureOverride] = [:] + @ObservationIgnored var replies = TransactionReplies() /// A pane whose shown content the next structural change moves into the pane it adds, until an /// uptime (`handOffContent(of:)`). @ObservationIgnored var contentHandoff: (pane: PaneID, until: TimeInterval)? static let contentHandoffWindow: TimeInterval = 3 - struct Override { - var value: Double - var transaction: LayoutTransactionID - var ended = false - /// The daemon accepted the gesture's final command: the next snapshot - /// is authoritative (it was requested after the command committed on - /// the same serial connection), so the override yields to it. - var settled = false - } - /// The last daemon snapshot, without local overrides. Restored when a /// transaction is rejected. @ObservationIgnored private(set) var daemonScreens: [LayoutScreen] = [] @@ -274,6 +265,7 @@ public final class LayoutModel { /// gesture's override is dropped at the next snapshot; a live gesture /// keeps tracking the pointer. public func settleTransaction(_ transaction: LayoutTransactionID) { + guard replies.answered(transaction) else { return } // a value of it is still on its way for (key, value) in splitOverrides where value.transaction == transaction && value.ended { splitOverrides[key]?.settled = true } @@ -285,6 +277,7 @@ public final class LayoutModel { /// The daemon rejected the command carrying `transaction`: drop its /// overrides and show the last daemon value at once. public func rejectTransaction(_ transaction: LayoutTransactionID) { + replies.forget(transaction) let splits = splitOverrides.count, widths = widthOverrides.count splitOverrides = splitOverrides.filter { $0.value.transaction != transaction } widthOverrides = widthOverrides.filter { $0.value.transaction != transaction } @@ -414,8 +407,8 @@ public final class LayoutModel { public func setSplitRatio(_ split: SplitID, ratio: Double, transaction: LayoutTransactionID, phase: LayoutGesturePhase) { let ratio = min(max(ratio, SplitRatio.range.lowerBound), SplitRatio.range.upperBound) updateScreen(where: { $0.ratio(of: split) != nil }) { $0.settingRatio(ratio, for: split) } - splitOverrides[split] = Override(value: ratio, transaction: transaction, ended: phase == .ended) - if phase == .ended { emit(.setSplitRatio(split, ratio: ratio, transaction: transaction, phase: phase)) } + splitOverrides[split] = GestureOverride(value: ratio, transaction: transaction, ended: phase == .ended) + if phase == .ended { expectReply(transaction); emit(.setSplitRatio(split, ratio: ratio, transaction: transaction, phase: phase)) } } /// Equalizes `split` so every cell in its same-axis chain gets equal size. An explicit @@ -447,8 +440,8 @@ public final class LayoutModel { let width = min(max(width, ColumnWidthPreset.widthRange.lowerBound), ColumnWidthPreset.widthRange.upperBound) guard let anyPane = screens.lazy.compactMap({ $0.layout.columns.first { $0.id == column }?.root.panes.first }).first else { return } updateScreen(where: { layout in layout.columns.contains { $0.id == column } }) { $0.settingWidth(width, for: column) } - widthOverrides[column] = Override(value: width, transaction: transaction, ended: phase == .ended) - if phase == .ended { emit(.setColumnWidth(column, anyPane: anyPane, width: width, transaction: transaction, phase: phase)) } + widthOverrides[column] = GestureOverride(value: width, transaction: transaction, ended: phase == .ended) + if phase == .ended { expectReply(transaction); emit(.setColumnWidth(column, anyPane: anyPane, width: width, transaction: transaction, phase: phase)) } } /// Marks the start and end of a pointer gesture so views skip animation @@ -498,4 +491,7 @@ public final class LayoutModel { func emit(_ intent: LayoutIntent) { intentHandler?(intent) } + + /// An ended value of `transaction` goes out: a command the app settles at its reply (``settleTransaction(_:)``). + func expectReply(_ transaction: LayoutTransactionID) { if intentHandler != nil { replies.expect(transaction) } } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift new file mode 100644 index 000000000000..3241ea1f8653 --- /dev/null +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift @@ -0,0 +1,36 @@ +/// The ended values of each layout transaction the daemon has not answered yet. The app sends every ended value as its +/// own command and settles the transaction at each reply (`WorkspaceContentController.sendGesture`), so a transaction +/// of several values (a keyboard trade's two widths, an Option or Command drag's release, a split's sizing) settles only +/// at the reply to its last one: settled earlier, a daemon snapshot taken between two of its commands' events dropped the +/// values still on their way, and for a frame the screen showed them at their old sizes. +struct TransactionReplies { + private var unanswered: [LayoutTransactionID: Int] = [:] + + /// One more value of `transaction` goes to the daemon. + mutating func expect(_ transaction: LayoutTransactionID) { unanswered[transaction, default: 0] += 1 } + + /// A reply for `transaction` came: true when it was the last one its values wait for (or one nobody counted). + mutating func answered(_ transaction: LayoutTransactionID) -> Bool { + guard let left = unanswered[transaction], left > 1 else { + unanswered[transaction] = nil + return true + } + unanswered[transaction] = left - 1 + return false + } + + /// `transaction` was refused or cancelled: no reply settles it. + mutating func forget(_ transaction: LayoutTransactionID) { unanswered[transaction] = nil } +} + +/// A divider or column value a gesture set locally (`LayoutModel.splitOverrides`, `widthOverrides`), shown over the +/// daemon's snapshots until the daemon answers its transaction. +struct GestureOverride { + var value: Double + var transaction: LayoutTransactionID + var ended = false + /// The daemon accepted the gesture's final command: the next snapshot + /// is authoritative (it was requested after the command committed on + /// the same serial connection), so the override yields to it. + var settled = false +} diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift index 450089c87155..5b6c734c6bf5 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift @@ -21,4 +21,11 @@ extension LayoutRootView { view.holdsContentForKeyResize = true view.scrollState.keptColumn = column } + + /// The press's own sync ran, or had nothing to show (its widths were refused at once): the column it kept in place + /// applies to no later sync. + public func keyboardResizeDidChange(screenContaining pane: PaneID) { + guard let screen = model.screen(containing: pane), let view = screenViews[screen.id] else { return } + view.scrollState.keptColumn = nil + } } diff --git a/release-notes/next/highlights/resize-keys-arrow-direction.md b/release-notes/next/highlights/resize-keys-arrow-direction.md index 9d093bd490e6..5e15d202cd14 100644 --- a/release-notes/next/highlights/resize-keys-arrow-direction.md +++ b/release-notes/next/highlights/resize-keys-arrow-direction.md @@ -2,4 +2,4 @@ title: Resize keys move the divider the way the arrow points category: improved docs: https://cmux.com/docs/next/keyboard-shortcuts -Ctrl-Shift-H and L (or Ctrl-Cmd-Left and Right) move the divider beside the pane toward the arrow, as in Ghostty, in a row of columns and at a side dock too. Only the two panes beside it change size. +Ctrl-Shift-H and L (or Ctrl-Cmd-Left and Right) move the divider beside the pane toward the arrow, as in Ghostty, between scrolling columns and at a side dock too. Only the panes on its two sides change size. From d0a50f6d9ba0866d79bab8f8adc022afc5e7d889 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:47:56 +0000 Subject: [PATCH 08/16] test(next panes): a key trade is all or nothing: a refused width sends neither, an early reply settles nothing, a stand-in column refuses the press (red) Codex re-review of #34123 at 7dc25a2a2dab (FIX): - The app refused a trade's first width at once (no handle for its pane), which rolled the press back, and the press then set and sent the second width anyway. - A reply in the turn of the first width's command settled the trade before its second width was counted, so a snapshot before the second event showed the first column at its old width. - A trade beside a stand-in column (a pane the daemon has not made) sent both widths; the daemon applies one and refuses the other: half a trade. - The odd-pixel walks now require every press to move the whole step. Red: job id in the PR body. --- .../KeyboardResizeArrowDirectionTests.swift | 70 ++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift index 5984c73c4c08..8b17f30fa247 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift @@ -323,6 +323,7 @@ struct KeyboardResizeShownStepTests { guard case let .columnTrade(left, leftWidth, right, rightWidth)? = PaneResize.change(for: pane, direction: direction, in: layout, shown: shown) else { continue } let next = layout.settingWidth(leftWidth, for: left).settingWidth(rightWidth, for: right) + let full = ((viewport.width - style.stripGap) * CGFloat(PaneResize.step) * scale).rounded() / scale for (when, after) in [("at the press", next), ("after the echo", echoed(next))] { let placed = ScreenGeometry.compute(after, viewport: viewport, style: style, scale: scale) let l0 = try #require(before.columns[left]), r0 = try #require(before.columns[right]) @@ -335,12 +336,13 @@ struct KeyboardResizeShownStepTests { for (id, frame) in before.columns where id != left && id != right { #expect(placed.columns[id] == frame, "\(pane) \(direction) \(when): column \(id) moved") } - #expect((step * scale).rounded() == step * scale, "\(pane) \(direction) \(when): a step of \(step) is no whole pixel") + #expect(abs(step) == full, "\(pane) \(direction) \(when): a step of \(step), want the whole step \(full)") } layout = echoed(next) moved += 1 } - #expect(moved >= 4, "only \(moved) of \(presses.count) presses moved a divider") + // None of these presses is at a limit: each one moves the divider. + #expect(moved == presses.count, "only \(moved) of \(presses.count) presses moved a divider") } /// A screen whose only column is a dock shows it full width (`DockStripGeometry.docks`): no press moves it, so none @@ -406,6 +408,70 @@ struct KeyboardResizeRefusedPressTests { } } +/// Codex re-review of #34123 (FIX at 7dc25a2a2dab): a trade is one change, all or nothing, on the screen and on the wire. +/// - The app refused the trade's first width at once (no handle for its pane), which rolled the press back, and then +/// the press set and sent the second width anyway: only one column changed, so every column after the pair moved. +/// - A reply that came in the turn of the first width's command (a handler that settles at once) settled the trade +/// before its second width was counted, so a daemon snapshot before the second width's event showed the first column +/// at its old width. +/// - A trade beside a column the daemon has not made yet (a stand-in, `ProvisionalPane`) sent the other column's width, +/// which the daemon applied, and the stand-in's, which it refused: half a trade at the daemon. The press is refused +/// at once instead, before anything moves, as other pane verbs on a stand-in are. +@MainActor +@Suite(.closesItsWindows) +struct KeyboardResizeTradeAtomicTests { + @Test func aTradeWhoseFirstWidthIsRefusedShowsAndSendsNeither() throws { + let rig = KeyboardResizeArrowDirectionTests.Rig(KeyboardResizeArrowDirectionTests.columns([0.5, 0.5]), focus: "p1") + defer { rig.window.closeAfterTest() } + var sent: [LayoutColumnID] = [] + rig.model.intentHandler = { [model = rig.model] intent in + guard case let .setColumnWidth(column, _, _, transaction, .ended) = intent else { return } + // c0's pane has no handle: the app refuses its width at once. + if column == "c0" { model.rejectTransaction(transaction) } else { sent.append(column) } + } + _ = rig.press("p1", .right) + #expect(sent.isEmpty, "a trade refused for c0 still sent \(sent)") + let widths = rig.model.screens[0].layout.columns.map(\.width) + #expect(widths == [0.5, 0.5], "a trade refused for c0 shows widths \(widths)") + } + + @Test func aReplyInTheFirstWidthsTurnDoesNotSettleTheTrade() throws { + let rig = KeyboardResizeArrowDirectionTests.Rig(KeyboardResizeArrowDirectionTests.columns([0.5, 0.5]), focus: "p1") + defer { rig.window.closeAfterTest() } + rig.model.intentHandler = { [model = rig.model] intent in + // c0's command is answered in its own turn; c1's reply comes later. + if case .setColumnWidth("c0", _, _, let transaction, .ended) = intent { model.settleTransaction(transaction) } + } + #expect(rig.press("p1", .right)) + let trade = rig.model.screens[0].layout.columns.map(\.width) + try #require(trade[0] > 0.5 && trade[1] < 0.5, "the press made no trade: \(trade)") + // A daemon snapshot from before either event. + rig.model.apply(screens: [LayoutScreen(id: "s", name: "s", layout: .columns(KeyboardResizeArrowDirectionTests.columns([0.5, 0.5])))]) + #expect(rig.model.screens[0].layout.columns.map(\.width) == trade, "the snapshot undid half the trade before its second reply") + } + + @Test func aTradeBesideAColumnTheDaemonHasNotMadeIsRefusedAndSendsNothing() throws { + // c11's pane is a stand-in: its handle is in the range no daemon pane has (`ProvisionalPane`, from 1 << 62). + let standIn: UInt64 = (1 << 62) + 7 + let screen = ScreenSnapshot(id: 1, layout: .leaf(DaemonPaneID(rawValue: 1)), + columns: [ColumnSnapshot(id: 10, width: 0.5, layout: .leaf(DaemonPaneID(rawValue: 1))), + ColumnSnapshot(id: 11, width: 0.5, layout: .leaf(DaemonPaneID(rawValue: standIn)))], + panes: [PaneSnapshot(id: DaemonPaneID(rawValue: 1), tabs: [TabSnapshot(surface: SurfaceID(rawValue: 1), title: "~")]), + PaneSnapshot(id: DaemonPaneID(rawValue: standIn), tabs: [TabSnapshot(surface: SurfaceID(rawValue: 2), title: "~")])]) + let shown = try PaneCloseControllerTests.Shown(tree: DaemonTree(workspaces: [WorkspaceSnapshot(id: 1, key: nil, name: "w1", screens: [screen])])) + defer { shown.end() } + let content = try #require(shown.content) + let pane = LayoutPaneID(try shown.key(1)) + try #require(content.layoutModel.screens.first?.layout.columns.count == 2, "the stand-in column does not show") + let first = content.gestureTransactions.latest + let before = content.layoutModel.screens.first?.layout.columns.map(\.width) + #expect(!PaneResize.press(pane, .right, model: content.layoutModel, view: content.layoutView), + "a trade with a column the daemon has not made went ahead") + #expect(content.gestureTransactions.latest == first, "the refused trade sent \(content.gestureTransactions.latest - first) transactions") + #expect(content.layoutModel.screens.first?.layout.columns.map(\.width) == before) + } +} + /// Undo Layout Change takes one press back in one step: the daemon keeps one undo entry per transaction /// (`layout_undo_coalesces_every_target_in_one_resize_transaction`), so every value of one layout transaction (a /// trade's two widths, an Option or Command drag's release) must go out under one daemon transaction. Each command From 035fa46049280a3a7aa81e9a053a9c772423991c Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:51:01 +0000 Subject: [PATCH 09/16] fix(next panes): a key press's or a drag release's values are one change, all or nothing; a trade beside a stand-in column is refused at once Codex re-review of #34123 at 7dc25a2a2dab (FIX): - EndedValues (CmuxNextLayout): the values a key press or a drag's release ends with are checked before anything shows (every target exists and LayoutModel.sendsNow can send it now), shown in one turn, counted before the first goes out (the transaction settles at the reply to its last value), and sent under one transaction; a value the app refuses while they go out stops the rest, and the transaction shows the daemon's values again. - LayoutHandleMap.sendsNow (CmuxNextBridge): a split needs a handle, a column's pane a handle of a pane the daemon made (ProvisionalPane.isStandIn: a stand-in's commands are refused, PaneNotReady). The app wires it next to its intent handler. A key trade beside a column the daemon has not made is refused at once, before anything moves (as other pane verbs on a stand-in), instead of half a trade at the daemon. - PaneResize.press sends split ratios, widths and trades through EndedValues and returns what went; keyboardResizeDidChange also ends the content hold when nothing moves. DividerDrag sends a release's values through EndedValues too (Option and Command releases set several). - Still two commands for two values: a value the daemon refuses after the other was applied leaves that one applied; atomic commits wait for layout op batches (resize-modifiers.md). Red: the commit before this one (job id in the PR body). Green: job id in the PR body. --- .../WorkspaceContentController.swift | 1 + .../CmuxNextBridge/LayoutHandleMap.swift | 10 +++ .../Sources/CmuxNextBridge/PaneResize.swift | 27 +++--- .../Store/ProvisionalPane.swift | 3 + .../CmuxNextLayout/Model/EndedValues.swift | 85 +++++++++++++++++++ .../CmuxNextLayout/Model/LayoutModel.swift | 2 + .../CmuxNextLayout/Views/DividerDrag.swift | 5 ++ .../Views/LayoutRootView+KeyResize.swift | 6 +- 8 files changed, 126 insertions(+), 13 deletions(-) create mode 100644 Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController.swift index 7345a8b96311..fc2a72b5e10f 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController.swift @@ -140,6 +140,7 @@ final class WorkspaceContentController: LayoutPaneContentProvider { self.state = state focus = state.focus layoutModel.intentHandler = { [weak self] intent in self?.handle(intent) } + layoutModel.sendsNow = { [weak self] intent in self?.handles.sendsNow(intent) ?? false } _ = layoutView // built here, then the bar and the content view, as before // A scroll the layout took is timed as the layout's (`key.scroll_frame target=layout`). layoutView.onScrollTaken = { [inputDispatch = services.inputDispatch] in inputDispatch.noteLayoutTookScroll() } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/LayoutHandleMap.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/LayoutHandleMap.swift index 518d73943306..ac56519d62a8 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/LayoutHandleMap.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/LayoutHandleMap.swift @@ -14,6 +14,16 @@ public struct LayoutHandleMap: Sendable, Equatable { public init() {} + /// Whether `intent`'s command can go to the daemon now (`LayoutModel.sendsNow`): its split has a handle, its column's + /// pane a handle of a pane the daemon made (a stand-in's names nothing there). + public func sendsNow(_ intent: LayoutIntent) -> Bool { + switch intent { + case let .setSplitRatio(split, _, _, _): splits[split] != nil + case let .setColumnWidth(_, anyPane, _, _, _): panes[anyPane].map { !ProvisionalPane.isStandIn($0) } ?? false + default: true + } + } + mutating func addPane(_ id: LayoutPaneID, handle: DaemonPaneID) { panes[id] = handle paneIDs[handle] = id diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift index 749571ef2912..3a223525e116 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/PaneResize.swift @@ -94,28 +94,33 @@ public nonisolated enum PaneResize { guard let change = change(for: pane, direction: direction, in: screen.layout, shown: view.shownResizeRange(ofScreenContaining: pane)) else { return false } + // A split ratio, a width or a trade is one change, all or nothing (`EndedValues`): refused before anything moves + // when the app cannot send all of it now (a column the daemon has not made yet). + let values: EndedValues? = switch change { + case .splitRatio(let split, let ratio): EndedValues([.ratio(split, ratio)]) + case .columnWidth(let column, let width): EndedValues([.width(column, width)]) + case .columnTrade(let left, let leftWidth, let right, let rightWidth): EndedValues([.width(left, leftWidth), .width(right, rightWidth)]) + case .rowHeights: nil + } + if let values, !values.canSend(in: model) { return false } // Every pane of the screen keeps its content's size through the spring and lays it out once at the landing; a - // trade keeps its left column (and the camera) in place, so only the divider between the two moves. + // trade keeps its left column (and the camera) in place, so only the divider between them moves. if case .columnTrade(let left, _, _, _) = change { view.keyboardResizeWillChange(screenContaining: pane, keeping: left) } else { view.keyboardResizeWillChange(screenContaining: pane) } - switch change { - case .splitRatio(let split, let ratio): model.setSplitRatio(split, ratio: ratio, transaction: .make(), phase: .ended) - case .columnWidth(let column, let width): model.setColumnWidth(column, width: width, transaction: .make(), phase: .ended) - case .columnTrade(let left, let leftWidth, let right, let rightWidth): - // One press, one transaction: the daemon keeps one undo entry for both widths. - let transaction = LayoutTransactionID.make() - model.setColumnWidth(left, width: leftWidth, transaction: transaction, phase: .ended) - model.setColumnWidth(right, width: rightWidth, transaction: transaction, phase: .ended) - case .rowHeights(let column, let heights): model.setRowHeights(column, heights: heights, fit: heights.reduce(0) { $0 + $1.height } == 1000) + var sent = true + if let values { + sent = values.send(in: model) + } else if case .rowHeights(let column, let heights) = change { + model.setRowHeights(column, heights: heights, fit: heights.reduce(0) { $0 + $1.height } == 1000) } // The spring starts in the key's own turn (as a split's frame does, `LayoutRootView.syncWithModel`): the very // next display frame moves, never one that ran before the layout's observation turn. view.syncWithModel() view.keyboardResizeDidChange(screenContaining: pane) - return true + return sent } /// The column before (`forward == false`) or after the pane's column. diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Store/ProvisionalPane.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Store/ProvisionalPane.swift index ce0e16cadfe1..d49470647da1 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Store/ProvisionalPane.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Store/ProvisionalPane.swift @@ -9,6 +9,9 @@ public struct ProvisionalPane: Sendable, Hashable { private static let handleBase: UInt64 = 1 << 62 /// The lowest stand-in handle: a pane handle at or above it is a stand-in. static let firstHandle: UInt64 = handleBase + + /// Whether `handle` is a stand-in's: a pane the daemon has not made, whose commands it refuses (`PaneNotReady`). + public static func isStandIn(_ handle: PaneID) -> Bool { handle.rawValue >= firstHandle } @MainActor private static var next: UInt64 = 0 /// The provisional pane's handle in the store. diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift new file mode 100644 index 000000000000..1f0f37357ec0 --- /dev/null +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift @@ -0,0 +1,85 @@ +/// The values a key press or a drag's release ends with (a split ratio, a column width, a key trade's two widths, an +/// Option or Command drag's widths and ratios), sent as one change, all or nothing (FRAME OWNER REFACTOR KEYS, reviews of +/// #34123): +/// - Checked before anything shows: every value has its target, and the app can send it now (`LayoutModel.sendsNow`: a +/// pane the daemon has not made yet refuses every command). Else nothing shows and nothing goes. +/// - Shown in one turn, and counted before the first goes out, so the transaction settles at the reply to its last value +/// (`TransactionReplies`), whenever the replies come. +/// - Sent one command per value under one transaction (one undo step). A value the app refuses while they go out ends +/// the change: nothing after it goes, and every value of the transaction shows the daemon's again. +/// Two commands are still two daemon commits: a value the daemon refuses after the other was applied leaves that one +/// applied (atomic multi-value commits wait for layout op batches, resize-modifiers.md). +@MainActor +public struct EndedValues { + public let values: [DividerDragValue] + public let transaction: LayoutTransactionID + + public init(_ values: [DividerDragValue], transaction: LayoutTransactionID = .make()) { + self.values = values + self.transaction = transaction + } + + /// Whether every value has its target in `model` and can go to the daemon now. + public func canSend(in model: LayoutModel) -> Bool { + values.allSatisfy { value in intent(for: value, in: model).map { model.sendsNow?($0) ?? true } ?? false } + } + + /// Shows the values in `model` and sends them. False when they could not all go: nothing of them shows. + @discardableResult + public func send(in model: LayoutModel) -> Bool { + guard !values.isEmpty else { return true } + guard canSend(in: model) else { + // A drag's live values of this transaction go back too. + model.rejectTransaction(transaction) + return false + } + for value in values { + switch value { + case let .ratio(split, ratio): model.setSplitRatio(split, ratio: ratio, transaction: transaction, phase: .changed) + case let .width(column, width): model.setColumnWidth(column, width: width, transaction: transaction, phase: .changed) + } + } + let intents = values.compactMap { end($0, in: model) } + for _ in intents { model.expectReply(transaction) } + for intent in intents { + model.emit(intent) + guard holds(model) else { return false } + } + return true + } + + /// The command `value` would send, with the clamped value the model shows once it is set; nil when its target is gone. + private func intent(for value: DividerDragValue, in model: LayoutModel) -> LayoutIntent? { + switch value { + case let .ratio(split, ratio): + guard model.screens.contains(where: { $0.layout.ratio(of: split) != nil }) else { return nil } + return .setSplitRatio(split, ratio: ratio, transaction: transaction, phase: .ended) + case let .width(column, width): + guard let anyPane = model.screens.lazy.compactMap({ $0.layout.columns.first { $0.id == column }?.root.panes.first }).first else { + return nil + } + return .setColumnWidth(column, anyPane: anyPane, width: width, transaction: transaction, phase: .ended) + } + } + + /// Marks `value`'s shown override ended (the daemon's next snapshot does not replace it until its reply) and returns + /// the command that sends it. + private func end(_ value: DividerDragValue, in model: LayoutModel) -> LayoutIntent? { + switch value { + case let .ratio(split, _): + guard let shown = model.splitOverrides[split], shown.transaction == transaction else { return nil } + model.splitOverrides[split]?.ended = true + return intent(for: .ratio(split, shown.value), in: model) + case let .width(column, _): + guard let shown = model.widthOverrides[column], shown.transaction == transaction else { return nil } + model.widthOverrides[column]?.ended = true + return intent(for: .width(column, shown.value), in: model) + } + } + + /// Whether `model` still shows values of the transaction: a refusal took them all back. + private func holds(_ model: LayoutModel) -> Bool { + model.splitOverrides.values.contains { $0.transaction == transaction } + || model.widthOverrides.values.contains { $0.transaction == transaction } + } +} diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel.swift index 6eb213eea8ea..0eaa8d50ab44 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/LayoutModel.swift @@ -197,6 +197,8 @@ public final class LayoutModel { public var animatesReflow: Bool { ProcessInfo.processInfo.systemUptime < explicitReflowUntil } @ObservationIgnored var widthOverrides: [ColumnID: GestureOverride] = [:] @ObservationIgnored var replies = TransactionReplies() + /// Whether the app can send an ended value's command now (`EndedValues`); nil sends every one. + @ObservationIgnored public var sendsNow: (@MainActor (LayoutIntent) -> Bool)? /// A pane whose shown content the next structural change moves into the pane it adds, until an /// uptime (`handOffContent(of:)`). @ObservationIgnored var contentHandoff: (pane: PaneID, until: TimeInterval)? diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/DividerDrag.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/DividerDrag.swift index 34ea71f4d54f..185d0aa0ab55 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/DividerDrag.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/DividerDrag.swift @@ -144,6 +144,11 @@ struct DividerDrag { drag = phase == .ended ? drag : showingHint(drag) screen.activeDrag = drag model.restoreGestureValues(restored, transaction: drag.transaction) + // The release's values are one change, all or nothing (`EndedValues`): what the app cannot send takes the drag back. + if phase == .ended { + EndedValues(targets, transaction: drag.transaction).send(in: model) + return + } for target in targets { switch target { case let .ratio(split, ratio): model.setSplitRatio(split, ratio: ratio, transaction: drag.transaction, phase: phase) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift index 5b6c734c6bf5..e98bbc10f1d0 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Views/LayoutRootView+KeyResize.swift @@ -22,10 +22,12 @@ extension LayoutRootView { view.scrollState.keptColumn = column } - /// The press's own sync ran, or had nothing to show (its widths were refused at once): the column it kept in place - /// applies to no later sync. + /// The press's own sync ran, or had nothing to show (its values were refused while they went out): the column it + /// kept in place applies to no later sync, and with nothing moving its content hold ends too (a later structural + /// change must not start held). public func keyboardResizeDidChange(screenContaining pane: PaneID) { guard let screen = model.screen(containing: pane), let view = screenViews[screen.id] else { return } view.scrollState.keptColumn = nil + if !view.hasMotion { view.holdsContentForKeyResize = false } } } From 46efe549b4b0225e314f537bbc3ea66899eab6df Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:34:31 +0000 Subject: [PATCH 10/16] test(next panes): a trade answered and applied in its last width's turn is a press (red) Codex review 3 of #34123 at 035fa4604928 (FIX): EndedValues.send read a missing override as a refusal, but a handler that settles the last width and applies the daemon's snapshot in that turn (a synchronous source, such as MockLayoutSource) removes the overrides because the trade succeeded; PaneResize.press then said nothing moved. Red: job id in the PR body. --- .../KeyboardResizeArrowDirectionTests.swift | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift index 8b17f30fa247..49e9fbfc0635 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/KeyboardResizeArrowDirectionTests.swift @@ -450,6 +450,26 @@ struct KeyboardResizeTradeAtomicTests { #expect(rig.model.screens[0].layout.columns.map(\.width) == trade, "the snapshot undid half the trade before its second reply") } + /// Codex review 3 of #34123: a handler that answers the last width at once and applies the daemon's snapshot in that + /// turn (a synchronous source, such as the layout demo's) removes the trade's overrides because it succeeded; the press + /// read that as a refusal and said nothing moved. + @Test func aTradeAnsweredAndAppliedInItsLastWidthsTurnIsAPress() throws { + let rig = KeyboardResizeArrowDirectionTests.Rig(KeyboardResizeArrowDirectionTests.columns([0.5, 0.5]), focus: "p1") + defer { rig.window.closeAfterTest() } + var widths: [LayoutColumnID: Double] = [:] + rig.model.intentHandler = { [model = rig.model] intent in + guard case let .setColumnWidth(column, _, width, transaction, .ended) = intent else { return } + widths[column] = width + model.settleTransaction(transaction) + guard widths.count == 2 else { return } + let columns = KeyboardResizeArrowDirectionTests.columns([widths["c0"] ?? 0.5, widths["c1"] ?? 0.5]) + model.apply(screens: [LayoutScreen(id: "s", name: "s", layout: .columns(columns))]) + } + #expect(rig.press("p1", .right), "a trade the daemon applied in its turn read as refused") + let shown = rig.model.screens[0].layout.columns.map(\.width) + #expect(shown[0] > 0.5 && shown[1] < 0.5, "the applied trade does not show: \(shown)") + } + @Test func aTradeBesideAColumnTheDaemonHasNotMadeIsRefusedAndSendsNothing() throws { // c11's pane is a stand-in: its handle is in the range no daemon pane has (`ProvisionalPane`, from 1 << 62). let standIn: UInt64 = (1 << 62) + 7 From bf4dfe9891a763ea4a3f206c404bb7d68bbb40b4 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:35:11 +0000 Subject: [PATCH 11/16] fix(next panes): EndedValues notes a refusal where it happens, not from missing overrides Codex review 3 of #34123 at 035fa4604928 (FIX): a handler that settles the last width and applies the daemon's snapshot in that turn removed the trade's overrides because it succeeded, and EndedValues.send read that as a refusal. LayoutModel.rejectTransaction now notes a refusal of the transaction going out (TransactionReplies.beginSending/forget/ endSending); send stops at it and returns whether it happened, and an applied change is a sent one. A synchronous refusal of any value, the last included, still stops the rest. Red: the commit before this one (job id in the PR body). Green: job id in the PR body. --- .../CmuxNextLayout/Model/EndedValues.swift | 16 +++++++--------- .../Model/TransactionReplies.swift | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift index 1f0f37357ec0..5742be064c5e 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift @@ -6,7 +6,8 @@ /// - Shown in one turn, and counted before the first goes out, so the transaction settles at the reply to its last value /// (`TransactionReplies`), whenever the replies come. /// - Sent one command per value under one transaction (one undo step). A value the app refuses while they go out ends -/// the change: nothing after it goes, and every value of the transaction shows the daemon's again. +/// the change (`LayoutModel.rejectTransaction` notes it): nothing after it goes, and every value of the transaction +/// shows the daemon's again. A change the app answers and applies in this turn is a sent one. /// Two commands are still two daemon commits: a value the daemon refuses after the other was applied leaves that one /// applied (atomic multi-value commits wait for layout op batches, resize-modifiers.md). @MainActor @@ -41,11 +42,14 @@ public struct EndedValues { } let intents = values.compactMap { end($0, in: model) } for _ in intents { model.expectReply(transaction) } + // A refusal is noted where it happens (`LayoutModel.rejectTransaction`): a handler that answers and applies the + // daemon's snapshot in this turn removes the overrides too, because the change succeeded. + model.replies.beginSending(transaction) for intent in intents { model.emit(intent) - guard holds(model) else { return false } + if model.replies.sending?.refused == true { break } } - return true + return !model.replies.endSending(transaction) } /// The command `value` would send, with the clamped value the model shows once it is set; nil when its target is gone. @@ -76,10 +80,4 @@ public struct EndedValues { return intent(for: .width(column, shown.value), in: model) } } - - /// Whether `model` still shows values of the transaction: a refusal took them all back. - private func holds(_ model: LayoutModel) -> Bool { - model.splitOverrides.values.contains { $0.transaction == transaction } - || model.widthOverrides.values.contains { $0.transaction == transaction } - } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift index 3241ea1f8653..2a8da57b45fe 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift @@ -5,6 +5,8 @@ /// values still on their way, and for a frame the screen showed them at their old sizes. struct TransactionReplies { private var unanswered: [LayoutTransactionID: Int] = [:] + /// The transaction whose values are going out now (`EndedValues.send`), and whether the app refused it meanwhile. + private(set) var sending: (transaction: LayoutTransactionID, refused: Bool)? /// One more value of `transaction` goes to the daemon. mutating func expect(_ transaction: LayoutTransactionID) { unanswered[transaction, default: 0] += 1 } @@ -20,7 +22,19 @@ struct TransactionReplies { } /// `transaction` was refused or cancelled: no reply settles it. - mutating func forget(_ transaction: LayoutTransactionID) { unanswered[transaction] = nil } + mutating func forget(_ transaction: LayoutTransactionID) { + unanswered[transaction] = nil + if sending?.transaction == transaction { sending?.refused = true } + } + + /// `transaction`'s values start going out: a refusal while they do is noted (``forget(_:)``), whatever it removed. + mutating func beginSending(_ transaction: LayoutTransactionID) { sending = (transaction, false) } + + /// `transaction`'s values went out (or stopped): whether the app refused it meanwhile. + mutating func endSending(_ transaction: LayoutTransactionID) -> Bool { + defer { sending = nil } + return sending?.transaction == transaction && sending?.refused == true + } } /// A divider or column value a gesture set locally (`LayoutModel.splitOverrides`, `widthOverrides`), shown over the From 6f32972184d0363a5077e8dc1673ecc03df5ee7f Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:50:30 +0000 Subject: [PATCH 12/16] test(next panes): a change sent from inside another keeps the outer one's refusal; last-value, unrelated and no-app cases (red) Codex review 4 of #34123 at bf4dfe9891a7 (FIX): TransactionReplies kept the change being sent in one slot, so a second change sent from inside the first one's intent handler took it over and a refusal of the first after that was lost: the first sent its other values and said they went. New file EndedValuesTests: the nested case (red), and guards for a refusal of the last value, a refusal of another transaction, and no intent handler. Red: job id in the PR body. --- .../CmuxNextAppTests/EndedValuesTests.swift | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift new file mode 100644 index 000000000000..e656fc4ed9bc --- /dev/null +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift @@ -0,0 +1,58 @@ +@testable import CmuxNextLayout +import Testing + +/// `EndedValues` (FRAME OWNER REFACTOR KEYS, reviews of #34123): a key press's or a drag release's values go out as one +/// change, all or nothing, and `send` says whether they went. Codex review 4 (FIX at bf4dfe9891a7): the refusal of the +/// change being sent lived in one slot, so a second change sent from inside the first one's intent handler took it over, +/// and a refusal of the first after that was lost: the first sent its other values and said they went. +@MainActor +struct EndedValuesTests { + static func model() -> LayoutModel { + let columns = [LayoutColumn(id: "c0", width: 0.5, root: .leaf("p0")), LayoutColumn(id: "c1", width: 0.5, root: .leaf("p1")), + LayoutColumn(id: "c2", width: 0.5, root: .leaf("p2"))] + return LayoutModel(screens: [LayoutScreen(id: "s", name: "s", layout: .columns(columns))], activeScreenID: "s", focusedPane: "p0") + } + + static func widths(_ model: LayoutModel) -> [Double] { model.screens[0].layout.columns.map(\.width) } + + @Test func aChangeSentFromInsideAnotherKeepsTheOuterRefusal() { + let model = Self.model() + let outer = EndedValues([.width("c0", 0.55), .width("c1", 0.45)]), inner = EndedValues([.width("c2", 0.6)]) + var sent: [ColumnID] = [] + model.intentHandler = { intent in + guard case let .setColumnWidth(column, _, _, transaction, .ended) = intent else { return } + sent.append(column) + // The outer change's first value: another change goes out in its turn, then the app refuses the outer one. + if column == "c0" { + #expect(inner.send(in: model)) + model.rejectTransaction(transaction) + } + } + #expect(!outer.send(in: model), "the outer change said it went after the app refused it") + #expect(sent == ["c0", "c2"], "the refused outer change still sent \(sent)") + #expect(Self.widths(model) == [0.5, 0.5, 0.6]) + } + + @Test func aRefusalOfTheLastValueSaysTheChangeDidNotGo() { + let model = Self.model() + model.intentHandler = { intent in + if case .setColumnWidth("c1", _, _, let transaction, .ended) = intent { model.rejectTransaction(transaction) } + } + #expect(!EndedValues([.width("c0", 0.55), .width("c1", 0.45)]).send(in: model)) + #expect(Self.widths(model) == [0.5, 0.5, 0.5], "a refused change still shows \(Self.widths(model))") + } + + @Test func aRefusalOfAnotherChangeLeavesThisOneSent() { + let model = Self.model() + let other = LayoutTransactionID.make() + model.intentHandler = { _ in model.rejectTransaction(other) } + #expect(EndedValues([.width("c0", 0.55), .width("c1", 0.45)]).send(in: model)) + #expect(Self.widths(model) == [0.55, 0.45, 0.5]) + } + + @Test func withNoAppTheChangeShowsAndCountsAsSent() { + let model = Self.model() + #expect(EndedValues([.width("c0", 0.55), .width("c1", 0.45)]).send(in: model)) + #expect(Self.widths(model) == [0.55, 0.45, 0.5]) + } +} From 3eefb96da39d92c4427352eb31d276070f9dd6a5 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:51:59 +0000 Subject: [PATCH 13/16] test(next panes): a transaction keeps its daemon number when another goes out between its values (red) Codex review 4 of #34123 at bf4dfe9891a7: GestureTransactionNumbers remembered the newest transaction only, so a change sent between two values of another gave that one's second value a new number (two undo steps for one press). Red: job id in the PR body. --- .../Tests/CmuxNextAppTests/EndedValuesTests.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift index e656fc4ed9bc..31c089a6e01b 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift @@ -1,3 +1,4 @@ +import CmuxNextBridge @testable import CmuxNextLayout import Testing @@ -55,4 +56,14 @@ struct EndedValuesTests { #expect(EndedValues([.width("c0", 0.55), .width("c1", 0.45)]).send(in: model)) #expect(Self.widths(model) == [0.55, 0.45, 0.5]) } + + /// Codex review 4: the daemon numbers remembered the newest transaction only, so a change sent between two values of + /// another gave the other's second value a new number: two undo steps for one press. + @Test func aTransactionKeepsItsNumberWhenAnotherGoesOutBetweenItsValues() throws { + var numbers = GestureTransactionNumbers() + let a = LayoutTransactionID.make(), b = LayoutTransactionID.make() + let first = numbers.number(for: a) + _ = numbers.number(for: b) + #expect(numbers.number(for: a) == first, "the second value of a took a new number after b went out") + } } From d307c16972b546b8c1be6be1bdb233448d3be940 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:52:21 +0000 Subject: [PATCH 14/16] fix(next panes): a change keeps its refusal and its daemon number when another goes out inside it Codex review 4 of #34123 at bf4dfe9891a7 (FIX): - TransactionReplies keeps the refusal state per transaction being sent, so a change sent from inside another's intent handler no longer takes the outer one's state over; EndedValues checks its own transaction after each emit. - GestureTransactionNumbers keeps each recent transaction's number (the last 32), so a change sent between two values of another does not split that one into two undo steps. Red: the two commits before this one (job ids in the PR body). Green: job id in the PR body. --- .../GestureTransactionNumbers.swift | 22 +++++++++++-------- .../CmuxNextLayout/Model/EndedValues.swift | 2 +- .../Model/TransactionReplies.swift | 17 +++++++------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift index cffb13e501a6..4a604aa414df 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift @@ -5,22 +5,26 @@ import Foundation /// release, a key press, a split's sizing): every value of it goes out under one number, so the daemon keeps one undo /// entry for all its widths and ratios (`layout_undo_coalesces_every_target_in_one_resize_transaction` in /// cmux-tui-core) and Undo Layout Change takes the gesture back in one step. (The daemon keeps a dock change apart -/// from resizes, so a dock pin with a width is still two steps.) A gesture sends its values in one turn, one after -/// the other, so the newest transaction is all this needs to remember; a value of an older one (none sends late) -/// would take a new number, a step of its own, as before. +/// from resizes, so a dock pin with a width is still two steps.) A gesture's values go out in one turn, but another +/// gesture's can go out between them (a change sent from inside another's intent handler), so each recent transaction +/// keeps its number; the oldest are let go past ``remembered`` (a gesture never sends late). public struct GestureTransactionNumbers { + /// How many recent transactions keep their numbers. + public static let remembered = 32 /// The number the newest layout transaction took; the next one takes the number after it. public private(set) var latest = UInt64(Date().timeIntervalSince1970 * 1000) << 8 - private var latestTransaction: LayoutTransactionID? + private var numbers: [LayoutTransactionID: UInt64] = [:] + private var order: [LayoutTransactionID] = [] public init() {} - /// The daemon number of `transaction`: the newest one's again, or the next one. + /// The daemon number of `transaction`: the one it took, or the next one. public mutating func number(for transaction: LayoutTransactionID) -> UInt64 { - if transaction != latestTransaction { - latest += 1 - latestTransaction = transaction - } + if let number = numbers[transaction] { return number } + latest += 1 + numbers[transaction] = latest + order.append(transaction) + if order.count > Self.remembered { numbers[order.removeFirst()] = nil } return latest } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift index 5742be064c5e..9e024299c61f 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift @@ -47,7 +47,7 @@ public struct EndedValues { model.replies.beginSending(transaction) for intent in intents { model.emit(intent) - if model.replies.sending?.refused == true { break } + if model.replies.refused(transaction) { break } } return !model.replies.endSending(transaction) } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift index 2a8da57b45fe..b0bd13e50fb2 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift @@ -5,8 +5,9 @@ /// values still on their way, and for a frame the screen showed them at their old sizes. struct TransactionReplies { private var unanswered: [LayoutTransactionID: Int] = [:] - /// The transaction whose values are going out now (`EndedValues.send`), and whether the app refused it meanwhile. - private(set) var sending: (transaction: LayoutTransactionID, refused: Bool)? + /// The transactions whose values are going out now (`EndedValues.send`; one change can be sent from inside another's + /// intent handler), each with whether the app refused it meanwhile. + private var sending: [LayoutTransactionID: Bool] = [:] /// One more value of `transaction` goes to the daemon. mutating func expect(_ transaction: LayoutTransactionID) { unanswered[transaction, default: 0] += 1 } @@ -24,17 +25,17 @@ struct TransactionReplies { /// `transaction` was refused or cancelled: no reply settles it. mutating func forget(_ transaction: LayoutTransactionID) { unanswered[transaction] = nil - if sending?.transaction == transaction { sending?.refused = true } + if sending[transaction] != nil { sending[transaction] = true } } /// `transaction`'s values start going out: a refusal while they do is noted (``forget(_:)``), whatever it removed. - mutating func beginSending(_ transaction: LayoutTransactionID) { sending = (transaction, false) } + mutating func beginSending(_ transaction: LayoutTransactionID) { sending[transaction] = false } + + /// Whether the app refused `transaction` while its values go out. + func refused(_ transaction: LayoutTransactionID) -> Bool { sending[transaction] == true } /// `transaction`'s values went out (or stopped): whether the app refused it meanwhile. - mutating func endSending(_ transaction: LayoutTransactionID) -> Bool { - defer { sending = nil } - return sending?.transaction == transaction && sending?.refused == true - } + mutating func endSending(_ transaction: LayoutTransactionID) -> Bool { sending.removeValue(forKey: transaction) == true } } /// A divider or column value a gesture set locally (`LayoutModel.splitOverrides`, `widthOverrides`), shown over the From a448d7f4d411bb3aca4c3f2546fb1ba017a268ea Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:03:38 +0000 Subject: [PATCH 15/16] test(next panes): a nested send of the same transaction keeps its refusal; a transaction keeps its number while many others go out (red) Codex review 5 of #34123 at d307c16972b5 (FIX): - TransactionReplies kept one refusal entry per transaction, so a change sent from inside another of the same transaction ended it, and a refusal after that was lost. - GestureTransactionNumbers kept the last 32 transactions only, so 32 others numbered between two values of one transaction gave its second value a new number. Red: job id in the PR body. --- .../CmuxNextAppTests/EndedValuesTests.swift | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift index 31c089a6e01b..703e011163ce 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift @@ -66,4 +66,35 @@ struct EndedValuesTests { _ = numbers.number(for: b) #expect(numbers.number(for: a) == first, "the second value of a took a new number after b went out") } + + /// Codex review 5 (FIX at d307c16972b5): the refusal state was one entry per transaction, so a change sent from inside + /// another of the same transaction ended that entry, and a refusal after it was lost. + @Test func aChangeSentInsideOneOfItsOwnTransactionKeepsTheRefusal() { + let model = Self.model() + let transaction = LayoutTransactionID.make() + let outer = EndedValues([.width("c0", 0.55), .width("c1", 0.45)], transaction: transaction) + let inner = EndedValues([.width("c2", 0.6)], transaction: transaction) + var sent: [ColumnID] = [], nested = false + model.intentHandler = { intent in + guard case let .setColumnWidth(column, _, _, transaction, .ended) = intent else { return } + sent.append(column) + if column == "c0", !nested { + nested = true + _ = inner.send(in: model) + model.rejectTransaction(transaction) + } + } + #expect(!outer.send(in: model), "the outer change said it went after the app refused its transaction") + #expect(!sent.contains("c1"), "the refused outer change still sent c1: \(sent)") + } + + /// Codex review 5: the daemon numbers kept only the last 32 transactions, so 32 others numbered between two values of + /// one transaction gave its second value a new number (two undo steps for one change). + @Test func aTransactionKeepsItsNumberWhileManyOthersGoOut() { + var numbers = GestureTransactionNumbers() + let a = LayoutTransactionID.make() + let first = numbers.number(for: a) + for _ in 0..<40 { _ = numbers.number(for: .make()) } + #expect(numbers.number(for: a) == first, "a's second value took a new number after 40 other transactions") + } } From c4b8d663930b2f643ebc48708a9bc3a29de68677 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:05:39 +0000 Subject: [PATCH 16/16] fix(next panes): nested sends keep a refusal scope each; a transaction keeps its daemon number while a command of it is in flight Codex review 5 of #34123 at d307c16972b5 (FIX): - TransactionReplies keeps a stack of changes going out, one scope per send (a change can be sent from inside another, even of the same transaction); a refusal marks every scope of its transaction, and EndedValues reads its own (innermost) scope after each emit. - GestureTransactionNumbers keeps a transaction's number while any command of it is in flight (count up per command, down at its answer in sendGesture), with no fixed window: every value of a gesture takes one number whatever goes out between them, and nothing stays once the last command is answered. - EndedValuesTests: the transaction answered in full takes a new number. Not changed (named in the PR): two values are two daemon commits; the daemon refusing the second after the first applied leaves the first applied (atomic commits wait for layout op batches). Red: the commit before this one (job id in the PR body). Green: job id in the PR body. --- .../WorkspaceContentController+Intents.swift | 10 +++---- .../GestureTransactionNumbers.swift | 28 +++++++++++-------- .../CmuxNextLayout/Model/EndedValues.swift | 4 +-- .../Model/TransactionReplies.swift | 21 +++++++------- .../CmuxNextAppTests/EndedValuesTests.swift | 13 +++++++++ 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift index 294664813cbb..dcef97be48ed 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/WorkspaceContentController+Intents.swift @@ -202,12 +202,10 @@ extension WorkspaceContentController { /// Sends a gesture's one command; its result settles or rolls back the layout's override. private func sendGesture(_ transaction: LayoutTransactionID, label: String, _ body: @escaping DaemonCommandBody) { - Task { [daemon, layoutModel] in - if await daemon.run(label, body) { - layoutModel.settleTransaction(transaction) - } else { - layoutModel.rejectTransaction(transaction) - } + Task { [weak self, daemon, layoutModel] in + let ok = await daemon.run(label, body) + self?.gestureTransactions.answered(transaction) + ok ? layoutModel.settleTransaction(transaction) : layoutModel.rejectTransaction(transaction) } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift index 4a604aa414df..b5c7944697e6 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextBridge/GestureTransactionNumbers.swift @@ -5,26 +5,30 @@ import Foundation /// release, a key press, a split's sizing): every value of it goes out under one number, so the daemon keeps one undo /// entry for all its widths and ratios (`layout_undo_coalesces_every_target_in_one_resize_transaction` in /// cmux-tui-core) and Undo Layout Change takes the gesture back in one step. (The daemon keeps a dock change apart -/// from resizes, so a dock pin with a width is still two steps.) A gesture's values go out in one turn, but another -/// gesture's can go out between them (a change sent from inside another's intent handler), so each recent transaction -/// keeps its number; the oldest are let go past ``remembered`` (a gesture never sends late). +/// from resizes, so a dock pin with a width is still two steps.) A transaction keeps its number while any of its +/// commands is in flight: its values go out in one turn and are answered in later ones, so every value of it, whatever +/// other gestures go out between them, takes the same number, and nothing of it is kept once its last command is answered. public struct GestureTransactionNumbers { - /// How many recent transactions keep their numbers. - public static let remembered = 32 /// The number the newest layout transaction took; the next one takes the number after it. public private(set) var latest = UInt64(Date().timeIntervalSince1970 * 1000) << 8 - private var numbers: [LayoutTransactionID: UInt64] = [:] - private var order: [LayoutTransactionID] = [] + private var open: [LayoutTransactionID: (number: UInt64, inFlight: Int)] = [:] public init() {} - /// The daemon number of `transaction`: the one it took, or the next one. + /// The daemon number for one more command of `transaction`: the one its commands in flight have, or the next one. public mutating func number(for transaction: LayoutTransactionID) -> UInt64 { - if let number = numbers[transaction] { return number } + if let entry = open[transaction] { + open[transaction] = (entry.number, entry.inFlight + 1) + return entry.number + } latest += 1 - numbers[transaction] = latest - order.append(transaction) - if order.count > Self.remembered { numbers[order.removeFirst()] = nil } + open[transaction] = (latest, 1) return latest } + + /// A command of `transaction` was answered, or failed. + public mutating func answered(_ transaction: LayoutTransactionID) { + guard let entry = open[transaction] else { return } + open[transaction] = entry.inFlight > 1 ? (entry.number, entry.inFlight - 1) : nil + } } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift index 9e024299c61f..56bf14d37fe9 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/EndedValues.swift @@ -47,9 +47,9 @@ public struct EndedValues { model.replies.beginSending(transaction) for intent in intents { model.emit(intent) - if model.replies.refused(transaction) { break } + if model.replies.refusedInnermost { break } } - return !model.replies.endSending(transaction) + return !model.replies.endSending() } /// The command `value` would send, with the clamped value the model shows once it is set; nil when its target is gone. diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift index b0bd13e50fb2..0e0022d59510 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Model/TransactionReplies.swift @@ -5,9 +5,9 @@ /// values still on their way, and for a frame the screen showed them at their old sizes. struct TransactionReplies { private var unanswered: [LayoutTransactionID: Int] = [:] - /// The transactions whose values are going out now (`EndedValues.send`; one change can be sent from inside another's - /// intent handler), each with whether the app refused it meanwhile. - private var sending: [LayoutTransactionID: Bool] = [:] + /// The changes going out now (`EndedValues.send`), innermost last: one can be sent from inside another's intent + /// handler, even one of the same transaction. Each notes whether the app refused its transaction meanwhile. + private var sending: [(transaction: LayoutTransactionID, refused: Bool)] = [] /// One more value of `transaction` goes to the daemon. mutating func expect(_ transaction: LayoutTransactionID) { unanswered[transaction, default: 0] += 1 } @@ -25,17 +25,18 @@ struct TransactionReplies { /// `transaction` was refused or cancelled: no reply settles it. mutating func forget(_ transaction: LayoutTransactionID) { unanswered[transaction] = nil - if sending[transaction] != nil { sending[transaction] = true } + for index in sending.indices where sending[index].transaction == transaction { sending[index].refused = true } } - /// `transaction`'s values start going out: a refusal while they do is noted (``forget(_:)``), whatever it removed. - mutating func beginSending(_ transaction: LayoutTransactionID) { sending[transaction] = false } + /// A change of `transaction` starts going out: a refusal of it while it does is noted (``forget(_:)``), whatever it + /// removed. + mutating func beginSending(_ transaction: LayoutTransactionID) { sending.append((transaction, false)) } - /// Whether the app refused `transaction` while its values go out. - func refused(_ transaction: LayoutTransactionID) -> Bool { sending[transaction] == true } + /// Whether the app refused the innermost change going out: the caller's own once its emit returned (sends nest). + var refusedInnermost: Bool { sending.last?.refused == true } - /// `transaction`'s values went out (or stopped): whether the app refused it meanwhile. - mutating func endSending(_ transaction: LayoutTransactionID) -> Bool { sending.removeValue(forKey: transaction) == true } + /// The innermost change went out (or stopped): whether the app refused its transaction meanwhile. + mutating func endSending() -> Bool { sending.popLast()?.refused == true } } /// A divider or column value a gesture set locally (`LayoutModel.splitOverrides`, `widthOverrides`), shown over the diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift index 703e011163ce..78f567b39cc0 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/EndedValuesTests.swift @@ -97,4 +97,17 @@ struct EndedValuesTests { for _ in 0..<40 { _ = numbers.number(for: .make()) } #expect(numbers.number(for: a) == first, "a's second value took a new number after 40 other transactions") } + + /// A transaction whose commands were all answered is done: a value after it is a step of its own. + @Test func aTransactionAnsweredInFullTakesANewNumber() { + var numbers = GestureTransactionNumbers() + let a = LayoutTransactionID.make() + let first = numbers.number(for: a) + #expect(numbers.number(for: a) == first) + numbers.answered(a) + #expect(numbers.number(for: a) == first, "a still had a command in flight") + numbers.answered(a) + numbers.answered(a) + #expect(numbers.number(for: a) != first) + } }