From 2a94402277c62d6e04d164b8220dc772b3854433 Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Tue, 21 Jul 2026 13:01:15 +0530 Subject: [PATCH 1/2] Window Manager: move to the next display, and undo the last arrange MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two actions the snap grid was missing on a multi-display desk: throw the focused window onto the next screen, and put it back the way it was when the snap turns out to be the wrong one. Both are ordinary `WindowAction` cases, so they inherit the whole existing apparatus for free — Accessibility gating, the popover-target snapshot that keeps cross-display clicks honest, the shortcut recorder, persistence, and the duplicate/registration-failure reporting. They ship on ⌃⌥⌘→ and ⌃⌥⌫; the move escalates to ⌘ because ⌃⌥→ is already Right Half. The two do not fit `frame(for:in:)`, whose whole contract is "target frame from one usable area": moving needs the source *and* destination areas plus the window's own frame, and restore needs a frame remembered from earlier. Rather than pretend, that function now returns nil for them and `isAreaRelative` says so on the action itself, which is what lets the geometry tests keep asserting they cover the area-relative set exhaustively instead of quietly shrinking. The mapping is proportional, through visible frames on both sides, not a coordinate copy. Displays differ in point size and in how much the menu bar and Dock take off the top: copying coordinates puts a window that sat comfortably on a 2560x1440 screen half off a 1440x900 one, and copying the size leaves a maximized window short of the edge or spilling past it. The mapped rect is then clamped into the destination, which is what handles a window bigger than the screen it is moving to and one that was already hanging off its source screen — the two cases where "keep the proportions" alone would strand the title bar somewhere the user cannot grab it. Identical source and destination short-circuit to the untouched frame, so on a single-display Mac the action is a true no-op rather than a round trip that would haul a deliberately offscreen window back. Cycling follows the physical layout (left to right, then top to bottom), not `NSScreen.screens` order, which is primary-first plus registration order and feels random on a three-display arrangement. Restore points live in a bounded LRU keyed on AXWindowNumber (pid alone for apps that withhold it). Bounded because the key space is not: windows close without telling us, so an unbounded map would accumulate for the helper's lifetime. The memory is written only after the frame verifies, so a snap an app refuses cannot consume or overwrite the undo the user still has, and it is process-local — window numbers do not survive a relaunch, so a persisted frame would restore some other window. Tests cover the mapping across four aspect ratios, a window larger than the destination, one hanging off each corner of its source, the single-display no-op, degenerate areas during a display reconfigure, the cycle order and its wrap, and the memory's eviction, recency refresh and consume-on-restore. Co-Authored-By: Claude Opus 4.8 --- .../DMonteCore/WindowManagerController.swift | 61 ++- Sources/DMonteCore/WindowManagerKit.swift | 182 ++++++- Sources/DMonteCore/WindowManagerView.swift | 24 + .../WindowManagerDisplayActionTests.swift | 456 ++++++++++++++++++ .../WindowManagerKitTests.swift | 26 +- .../WindowManagerScreenMatchingTests.swift | 14 +- 6 files changed, 745 insertions(+), 18 deletions(-) create mode 100644 Tests/DMonteCoreTests/WindowManagerDisplayActionTests.swift diff --git a/Sources/DMonteCore/WindowManagerController.swift b/Sources/DMonteCore/WindowManagerController.swift index 370df21..969912f 100644 --- a/Sources/DMonteCore/WindowManagerController.swift +++ b/Sources/DMonteCore/WindowManagerController.swift @@ -11,6 +11,13 @@ public enum WindowApplyResult: Equatable, Sendable { case success(appName: String?) case needsPermission case noFocusedWindow + /// "Move to next display" on a Mac with only one display. A distinct outcome rather than + /// `.failed`: nothing went wrong, there is simply nowhere to move to, and the window is + /// deliberately left untouched. + case noOtherDisplay + /// "Restore" with nothing remembered for this window — it hasn't been arranged yet in this + /// session, or its restore point was already consumed. + case nothingToRestore case failed } @@ -59,6 +66,11 @@ public final class WindowManagerController: NSObject, ObservableObject { /// top-right). Popover tile clicks never touch this — a click is always a single action. private var chordState: WindowManagerKit.ChordState? + /// Pre-action frames for the `restorePrevious` undo. Bounded LRU (see `WindowFrameMemory`), and + /// process-local: window numbers don't survive a relaunch, so persisting it would restore the + /// wrong window. + private var frameMemory = WindowFrameMemory() + private var permissionTimer: Timer? private var windowSelectionMonitor: Any? private let shortcutStore: WindowShortcutStore @@ -462,22 +474,55 @@ public final class WindowManagerController: NSObject, ObservableObject { guard let currentFrame = Self.frame(of: window) else { return .failed } + let identity = WindowIdentity(pid: targetApp.processIdentifier, windowNumber: Self.windowNumber(of: window)) // Pick the screen the window mostly lives on, then compute the target in AX (top-left) // space. The pure matcher gets the real screens as plain rects; if the window overlaps no // screen at all, fall back to the primary screen (never NSScreen.main — when a snap tile // is clicked, the key window is our own popover, so NSScreen.main is the popover's screen, // not anything to do with the target window). - let screens = NSScreen.screens - let areas = screens.map { Self.axRect(fromCocoa: $0.visibleFrame) } - let matched = WindowManagerKit.areaIndex(forWindow: currentFrame, in: areas).map { screens[$0] } - guard let screen = matched ?? screens.first else { - return .failed + let areas = NSScreen.screens.map { Self.axRect(fromCocoa: $0.visibleFrame) } + guard !areas.isEmpty else { return .failed } + let areaIndex = WindowManagerKit.areaIndex(forWindow: currentFrame, in: areas) ?? 0 + let axArea = areas[areaIndex] + + let targetFrame: CGRect + switch action { + case .nextDisplay: + guard let destination = WindowManagerKit.nextAreaIndex(after: areaIndex, in: areas) else { + return .noOtherDisplay + } + targetFrame = WindowManagerKit.frame(movingWindow: currentFrame, from: axArea, to: areas[destination]) + case .restorePrevious: + guard let remembered = frameMemory.frame(for: identity) else { + return .nothingToRestore + } + // The display the frame was captured on may since have been unplugged or rearranged, + // which would strand the window in coordinates no screen covers. Clamp into whichever + // screen the remembered frame still belongs to, falling back to the window's current + // screen when it belongs to none. + let host = WindowManagerKit.areaIndex(forWindow: remembered, in: areas).map { areas[$0] } ?? axArea + targetFrame = WindowManagerKit.clamped(remembered, into: host) + default: + guard let frame = WindowManagerKit.frame(for: action, in: axArea) else { + return .failed + } + targetFrame = frame } - let axArea = Self.axRect(fromCocoa: screen.visibleFrame) - let targetFrame = WindowManagerKit.frame(for: action, in: axArea) - return setFrameVerified(targetFrame, on: window, appElement: appElement, appName: targetApp.localizedName) + let result = setFrameVerified(targetFrame, on: window, appElement: appElement, appName: targetApp.localizedName) + + // Only a landed move changes what "before the last action" means, so the restore point is + // updated after verification, never before: a snap the app refused must not consume or + // overwrite the undo the user still has. + if case .success = result { + if action == .restorePrevious { + frameMemory.forget(identity) + } else { + frameMemory.remember(currentFrame, for: identity) + } + } + return result } /// Writes `target` to the window and verifies it landed, retrying with alternating set diff --git a/Sources/DMonteCore/WindowManagerKit.swift b/Sources/DMonteCore/WindowManagerKit.swift index b8639d1..afe6819 100644 --- a/Sources/DMonteCore/WindowManagerKit.swift +++ b/Sources/DMonteCore/WindowManagerKit.swift @@ -9,9 +9,26 @@ public enum WindowAction: String, CaseIterable, Sendable, Identifiable { case leftThird, centerThird, rightThird case firstTwoThirds, lastTwoThirds case maximize, center, almostMaximize + case nextDisplay, restorePrevious public var id: String { rawValue } + /// Whether the target frame is fully determined by one screen's usable area, i.e. whether + /// `WindowManagerKit.frame(for:in:)` can answer for this action. The two display actions are + /// not: `nextDisplay` needs the source *and* destination areas plus the window's own frame, + /// and `restorePrevious` needs a frame remembered from before the last action. Keeping the + /// distinction on the action (rather than in a comment) is what lets the tests assert the + /// area-relative set is exhaustively covered instead of quietly shrinking. + public var isAreaRelative: Bool { + switch self { + case .nextDisplay, .restorePrevious: false + default: true + } + } + + /// The actions `WindowManagerKit.frame(for:in:)` answers for. + public static var areaRelativeCases: [WindowAction] { allCases.filter(\.isAreaRelative) } + public var title: String { switch self { case .leftHalf: "Left Half" @@ -30,6 +47,8 @@ public enum WindowAction: String, CaseIterable, Sendable, Identifiable { case .maximize: "Maximize" case .center: "Center" case .almostMaximize: "Almost Maximize" + case .nextDisplay: "Next Display" + case .restorePrevious: "Restore" } } @@ -51,6 +70,8 @@ public enum WindowAction: String, CaseIterable, Sendable, Identifiable { case .maximize: "rectangle.fill" case .center: "rectangle.center.inset.filled" case .almostMaximize: "rectangle.inset.filled" + case .nextDisplay: "display.2" + case .restorePrevious: "arrow.uturn.backward" } } @@ -65,6 +86,11 @@ public enum WindowAction: String, CaseIterable, Sendable, Identifiable { case .bottomHalf: WindowShortcut(keyCode: HotKeyCode.down, modifiers: HotKeyModifier.controlOption) case .maximize: WindowShortcut(keyCode: HotKeyCode.returnKey, modifiers: HotKeyModifier.controlOption) case .center: WindowShortcut(keyCode: HotKeyCode.c, modifiers: HotKeyModifier.controlOption) + // The display actions take ⌃⌥⌘ rather than plain ⌃⌥: ⌃⌥→ is already Right Half, and + // adding ⌘ is the same escalation Rectangle uses for its move-to-display bindings, so the + // muscle memory carries over. + case .nextDisplay: WindowShortcut(keyCode: HotKeyCode.right, modifiers: HotKeyModifier.controlOptionCommand) + case .restorePrevious: WindowShortcut(keyCode: HotKeyCode.delete, modifiers: HotKeyModifier.controlOption) default: nil } } @@ -79,6 +105,7 @@ public enum HotKeyCode { public static let returnKey: UInt32 = 0x24 public static let keypadEnter: UInt32 = 0x4C public static let c: UInt32 = 0x08 + public static let delete: UInt32 = 0x33 } /// Carbon modifier masks (mirrors Carbon's `cmdKey`/`shiftKey`/`optionKey`/`controlKey` without @@ -89,6 +116,7 @@ public enum HotKeyModifier { public static let option: UInt32 = 0x0800 public static let control: UInt32 = 0x1000 public static let controlOption: UInt32 = control | option + public static let controlOptionCommand: UInt32 = control | option | command } /// A global keyboard shortcut as Carbon understands it: virtual key code + modifier mask. @@ -241,7 +269,11 @@ public enum WindowManagerKit { /// Computes the target frame for `action` within `area` (a top-left-origin rect: y grows down). /// Pure arithmetic — no global state — so it is exhaustively unit-tested. - public static func frame(for action: WindowAction, in area: CGRect) -> CGRect { + /// + /// Returns nil for the actions that are not area-relative (`WindowAction.isAreaRelative`): a + /// single usable area cannot express "move to the next display" or "put it back where it was". + /// Those are computed by `frame(movingWindow:from:to:)` and the remembered-frame store instead. + public static func frame(for action: WindowAction, in area: CGRect) -> CGRect? { let x = area.minX let y = area.minY let w = area.width @@ -273,9 +305,89 @@ public enum WindowManagerKit { case .almostMaximize: let inset = min(w, h) * 0.05 return CGRect(x: x + inset, y: y + inset, width: w - 2 * inset, height: h - 2 * inset) + case .nextDisplay, .restorePrevious: + return nil } } + // MARK: - Cross-display mapping (pure) + + /// Shrinks `frame` to fit inside `area` and slides it until it lies entirely within it. + /// Used by every cross-display move (a window can legitimately be wider than the display it is + /// moving to) and by restore (the display a remembered frame came from may since have been + /// unplugged). Pure. + public static func clamped(_ frame: CGRect, into area: CGRect) -> CGRect { + // A degenerate area carries no information to clamp against; returning the frame untouched + // is the only answer that cannot make things worse. + guard area.width > 0, area.height > 0 else { return frame } + let width = min(frame.width, area.width) + let height = min(frame.height, area.height) + // width ≤ area.width, so `area.maxX - width` is never left of `area.minX` and the two + // bounds cannot cross. + let x = min(max(frame.minX, area.minX), area.maxX - width) + let y = min(max(frame.minY, area.minY), area.maxY - height) + return CGRect(x: x, y: y, width: width, height: height) + } + + /// Maps `windowFrame` from the `source` usable area onto the `destination` usable area, keeping + /// its relative position and its size *as a proportion of the screen*. All three rects are in AX + /// top-left space, and both areas are visible frames — not full display frames — so the menu bar + /// and the Dock are already accounted for on each side. + /// + /// Proportional rather than absolute because displays differ in point size and in how much the + /// menu bar and Dock take off the top and bottom: copying the coordinates across puts a window + /// that sat comfortably on a 2560×1440 screen half off a 1440×900 one, and copying the size + /// leaves a maximized window either short of the edge or spilling past it. The result is finally + /// clamped into `destination`, which is what handles a window larger than the destination and a + /// window that was hanging off the edge of its source screen. + /// + /// Identical areas return `windowFrame` untouched: on a single-display Mac "move to the next + /// display" must be a true no-op, not a round-trip through the mapping that would quietly haul a + /// deliberately half-offscreen window back on-screen. + public static func frame(movingWindow windowFrame: CGRect, from source: CGRect, to destination: CGRect) -> CGRect { + guard source != destination else { return windowFrame } + // Zero-sized areas appear while a display is being reconfigured. Dividing by them yields + // NaN origins that AX happily accepts and the user then cannot find the window. + guard source.width > 0, source.height > 0, destination.width > 0, destination.height > 0 else { + return windowFrame + } + + let scaleX = destination.width / source.width + let scaleY = destination.height / source.height + let mapped = CGRect( + x: destination.minX + (windowFrame.minX - source.minX) * scaleX, + y: destination.minY + (windowFrame.minY - source.minY) * scaleY, + width: windowFrame.width * scaleX, + height: windowFrame.height * scaleY + ) + return clamped(mapped, into: destination) + } + + /// The display indices in the order "next display" cycles through them: left to right, then + /// top to bottom, ties broken by the caller's own order. `NSScreen.screens` is ordered by the + /// system's arrangement bookkeeping (primary first, then however the displays were registered), + /// which does not match the physical layout — cycling in that order feels random on a + /// three-display desk, whereas cycling rightwards matches what the user sees. Pure. + public static func displayCycleOrder(of areas: [CGRect]) -> [Int] { + areas.indices.sorted { lhs, rhs in + let a = areas[lhs] + let b = areas[rhs] + if a.minX != b.minX { return a.minX < b.minX } + if a.minY != b.minY { return a.minY < b.minY } + return lhs < rhs + } + } + + /// The display to move to after `current`, wrapping around. Nil when there is nowhere to go — + /// a single display, an empty list, or an index that isn't in the list — which the caller + /// reports as a no-op rather than moving the window anywhere. Pure. + public static func nextAreaIndex(after current: Int, in areas: [CGRect]) -> Int? { + guard areas.count > 1, areas.indices.contains(current) else { return nil } + let order = displayCycleOrder(of: areas) + guard let position = order.firstIndex(of: current) else { return nil } + return order[(position + 1) % order.count] + } + // MARK: - Frame verification (pure) /// Per-component tolerance, in points, within which an achieved window frame counts as @@ -400,3 +512,71 @@ public enum WindowManagerKit { return .half(current) } } + +/// Identifies the window a remembered frame belongs to. The Quartz window number (read from the +/// private-but-universally-implemented `AXWindowNumber`) is stable for the life of a window even as +/// it moves and resizes, which is exactly the identity "restore" needs. Apps that withhold it fall +/// back to the owning pid alone: coarser — the app's windows then share one restore slot — but +/// still better than having no undo for those apps at all. +public struct WindowIdentity: Hashable, Sendable { + public let pid: pid_t + public let windowNumber: Int? + + public init(pid: pid_t, windowNumber: Int?) { + self.pid = pid + self.windowNumber = windowNumber + } +} + +/// Remembers each window's frame from just before the last Window Manager action, so `restore` has +/// something to put back. Pure value type, so the eviction and undo semantics are unit-tested +/// without any windows. +/// +/// Bounded and least-recently-used, because the natural key space is unbounded: every window of +/// every app the user ever snaps would otherwise accumulate an entry for the lifetime of the helper, +/// and windows that close never tell us so. `capacity` entries is far more than the handful of +/// windows anyone actually arranges in a session, and the eviction victim is by construction the +/// window whose undo is least likely to still be wanted. +/// +/// Deliberately in-memory only: window numbers are not stable across a relaunch, so a persisted +/// frame would restore some *other* window, or nothing at all. +public struct WindowFrameMemory: Sendable { + /// Sized for a working set, not a history: arranging a dozen windows in one session is a lot, + /// and 32 leaves generous headroom while keeping the store trivially small. + public static let defaultCapacity = 32 + + private let capacity: Int + private var frames: [WindowIdentity: CGRect] = [:] + /// Identities in least-recently-remembered order, so eviction is a `removeFirst`. + private var recency: [WindowIdentity] = [] + + public init(capacity: Int = defaultCapacity) { + self.capacity = max(1, capacity) + } + + public var count: Int { frames.count } + + /// Records the frame a window had before an action was applied to it, replacing any earlier + /// one: "restore" means undoing the *last* action, not walking a history back. + public mutating func remember(_ frame: CGRect, for identity: WindowIdentity) { + recency.removeAll { $0 == identity } + recency.append(identity) + frames[identity] = frame + while recency.count > capacity { + frames.removeValue(forKey: recency.removeFirst()) + } + } + + /// The remembered frame, without consuming it — the caller only knows whether the restore + /// actually landed after it has tried to write it. + public func frame(for identity: WindowIdentity) -> CGRect? { + frames[identity] + } + + /// Drops the entry once a restore has succeeded, so a second restore reports "nothing to + /// restore" instead of silently re-applying a frame that is now the window's own history. + public mutating func forget(_ identity: WindowIdentity) { + frames.removeValue(forKey: identity) + recency.removeAll { $0 == identity } + } +} diff --git a/Sources/DMonteCore/WindowManagerView.swift b/Sources/DMonteCore/WindowManagerView.swift index f6f9ea9..9c933ea 100644 --- a/Sources/DMonteCore/WindowManagerView.swift +++ b/Sources/DMonteCore/WindowManagerView.swift @@ -127,6 +127,7 @@ public struct WindowManagerPopoverView: View { private let corners: [WindowAction] = [.topLeft, .topRight, .bottomLeft, .bottomRight] private let thirds: [WindowAction] = [.leftThird, .centerThird, .rightThird, .firstTwoThirds, .lastTwoThirds] private let sizing: [WindowAction] = [.maximize, .almostMaximize, .center] + private let displays: [WindowAction] = [.nextDisplay, .restorePrevious] public var body: some View { VStack(spacing: 0) { @@ -143,6 +144,8 @@ public struct WindowManagerPopoverView: View { cornerChordHint section("Thirds", thirds, columns: 5) section("Size", sizing, columns: 3) + section("Display & Undo", displays, columns: 2) + displayActionHint resultFeedback shortcutSection } @@ -227,6 +230,19 @@ public struct WindowManagerPopoverView: View { .foregroundStyle(.tertiary) } + /// "Restore" on its own reads as "restore what?", and "Next Display" gives no clue that the + /// window keeps its proportions rather than its coordinates. One line spells out both. + private var displayActionHint: some View { + HStack(alignment: .top, spacing: s(5)) { + Image(systemName: "info.circle") + .font(.system(size: s(8), weight: .semibold)) + Text("Next Display keeps the window's relative place and proportions on the new screen. Restore undoes the last arrange.") + .font(.system(size: s(9))) + .fixedSize(horizontal: false, vertical: true) + } + .foregroundStyle(.tertiary) + } + private func section(_ title: String, _ actions: [WindowAction], columns: Int) -> some View { VStack(alignment: .leading, spacing: s(6)) { Text(title) @@ -281,6 +297,14 @@ public struct WindowManagerPopoverView: View { Text("No focused window to arrange.") .font(.system(size: s(9.5))) .foregroundStyle(.orange) + case .noOtherDisplay: + Text("Only one display — the window stayed put.") + .font(.system(size: s(9.5))) + .foregroundStyle(.secondary) + case .nothingToRestore: + Text("Nothing to restore — arrange this window first.") + .font(.system(size: s(9.5))) + .foregroundStyle(.secondary) case .failed: Text("The focused window couldn't be moved — its app may not allow it.") .font(.system(size: s(9.5))) diff --git a/Tests/DMonteCoreTests/WindowManagerDisplayActionTests.swift b/Tests/DMonteCoreTests/WindowManagerDisplayActionTests.swift new file mode 100644 index 0000000..5d3d72f --- /dev/null +++ b/Tests/DMonteCoreTests/WindowManagerDisplayActionTests.swift @@ -0,0 +1,456 @@ +import AppKit +import XCTest +@testable import DMonteCore + +/// Covers the two display actions: moving the focused window to the next display, and restoring +/// the frame it had before the last Window Manager action. +/// +/// Everything here is the pure half — the cross-display mapping, the cycle order, and the bounded +/// restore store — because that is where the failure modes live: a window that lands off-screen on +/// a differently-shaped display, a "next display" that mangles the window on a single-display Mac, +/// or a cache that grows for the lifetime of the helper. All rects are AX top-left visible frames, +/// exactly what the controller feeds in. +final class WindowManagerDisplayActionTests: XCTestCase { + + // The reporter's real three-display Mac Studio arrangement (AX-converted visible frames): + // AW3225QF primary in the middle with menu bar + dock insets, BenQ PD3226G to the LEFT at + // negative x, BenQ EX3210U to the RIGHT. + private let primary = CGRect(x: 0, y: 30, width: 2560, height: 1332) + private let leftScreen = CGRect(x: -2560, y: 30, width: 2560, height: 1410) + private let rightScreen = CGRect(x: 2560, y: 30, width: 2560, height: 1410) + + /// Where a frame sits inside an area, as fractions of that area — the quantity the mapping is + /// supposed to preserve. + private func fractions(of frame: CGRect, in area: CGRect) -> [CGFloat] { + [ + (frame.minX - area.minX) / area.width, + (frame.minY - area.minY) / area.height, + frame.width / area.width, + frame.height / area.height + ] + } + + private func assertInside(_ frame: CGRect, _ area: CGRect, _ message: String = "", file: StaticString = #filePath, line: UInt = #line) { + XCTAssertGreaterThanOrEqual(frame.minX, area.minX - 0.001, "\(message) escaped left", file: file, line: line) + XCTAssertGreaterThanOrEqual(frame.minY, area.minY - 0.001, "\(message) escaped top", file: file, line: line) + XCTAssertLessThanOrEqual(frame.maxX, area.maxX + 0.001, "\(message) escaped right", file: file, line: line) + XCTAssertLessThanOrEqual(frame.maxY, area.maxY + 0.001, "\(message) escaped bottom", file: file, line: line) + } + + // MARK: - Cross-display mapping + + func testMappingPreservesRelativePositionAndProportionalSize() { + // A window occupying a known fraction of the primary must occupy the same fraction of the + // destination, whatever that display's shape. + let window = CGRect( + x: primary.minX + primary.width * 0.25, + y: primary.minY + primary.height * 0.2, + width: primary.width * 0.5, + height: primary.height * 0.4 + ) + let moved = WindowManagerKit.frame(movingWindow: window, from: primary, to: rightScreen) + for (expected, actual) in zip(fractions(of: window, in: primary), fractions(of: moved, in: rightScreen)) { + XCTAssertEqual(actual, expected, accuracy: 0.0001) + } + assertInside(moved, rightScreen, "proportional move") + } + + func testMappingAcrossDifferentAspectRatios() { + // 16:9 → 16:10 → 4:3 → ultrawide 21:9. The fractions must survive every hop, and the + // result must stay on the destination each time. + let shapes: [CGRect] = [ + CGRect(x: 0, y: 30, width: 2560, height: 1410), // 16:9 with a menu bar inset + CGRect(x: 0, y: 25, width: 1440, height: 875), // 16:10 laptop + CGRect(x: -1024, y: 0, width: 1024, height: 768), // 4:3, negative origin + CGRect(x: 3440, y: 40, width: 3440, height: 1400) // ultrawide + ] + let source = shapes[0] + let window = CGRect(x: source.minX + 640, y: source.minY + 282, width: 1280, height: 705) + let expected = fractions(of: window, in: source) + + for destination in shapes.dropFirst() { + let moved = WindowManagerKit.frame(movingWindow: window, from: source, to: destination) + for (want, got) in zip(expected, fractions(of: moved, in: destination)) { + XCTAssertEqual(got, want, accuracy: 0.0001, "aspect \(destination.width)x\(destination.height)") + } + assertInside(moved, destination, "aspect \(destination.width)x\(destination.height)") + } + } + + func testMaximizedWindowStaysMaximizedOnTheDestination() { + // The whole point of mapping through *visible* frames: a window filling a screen with a + // 30pt menu bar and a 78pt dock must fill the destination's own usable area, not inherit + // the source's insets. + let moved = WindowManagerKit.frame(movingWindow: primary, from: primary, to: leftScreen) + XCTAssertEqual(moved, leftScreen) + } + + func testWindowLargerThanDestinationIsClampedToFitIt() { + // A 3000×2000 window on a big screen cannot keep its size on a 1440×900 laptop display. + let laptop = CGRect(x: 2560, y: 25, width: 1440, height: 875) + let oversized = CGRect(x: primary.minX + 100, y: primary.minY + 100, width: 3000, height: 2000) + let moved = WindowManagerKit.frame(movingWindow: oversized, from: primary, to: laptop) + + XCTAssertLessThanOrEqual(moved.width, laptop.width + 0.001) + XCTAssertLessThanOrEqual(moved.height, laptop.height + 0.001) + assertInside(moved, laptop, "oversized window") + XCTAssertGreaterThan(moved.width, 0) + XCTAssertGreaterThan(moved.height, 0) + } + + func testPartiallyOffscreenWindowIsPulledFullyOntoTheDestination() { + // Hanging off the left and top of the primary. Copying coordinates (or the fractions + // alone) would keep it hanging off the destination, where the user may not be able to + // reach its title bar at all. + let hangingOff = CGRect(x: primary.minX - 400, y: primary.minY - 200, width: 900, height: 700) + let moved = WindowManagerKit.frame(movingWindow: hangingOff, from: primary, to: rightScreen) + assertInside(moved, rightScreen, "window hanging off source top-left") + XCTAssertEqual(moved.minX, rightScreen.minX, accuracy: 0.001) + XCTAssertEqual(moved.minY, rightScreen.minY, accuracy: 0.001) + } + + func testWindowHangingOffTheBottomRightIsPulledIn() { + let hangingOff = CGRect(x: primary.maxX - 200, y: primary.maxY - 150, width: 900, height: 700) + let moved = WindowManagerKit.frame(movingWindow: hangingOff, from: primary, to: leftScreen) + assertInside(moved, leftScreen, "window hanging off source bottom-right") + XCTAssertEqual(moved.maxX, leftScreen.maxX, accuracy: 0.001) + XCTAssertEqual(moved.maxY, leftScreen.maxY, accuracy: 0.001) + } + + func testSingleDisplayMoveIsAnExactNoOp() { + // On a one-display Mac the source and destination are the same area. The window must come + // back byte-identical — including a deliberately offscreen one, which a round-trip through + // the clamp would have hauled back on-screen. + let window = CGRect(x: 137, y: 411, width: 913, height: 622) + XCTAssertEqual(WindowManagerKit.frame(movingWindow: window, from: primary, to: primary), window) + + let offscreen = CGRect(x: -500, y: -400, width: 900, height: 700) + XCTAssertEqual(WindowManagerKit.frame(movingWindow: offscreen, from: primary, to: primary), offscreen) + } + + func testDegenerateAreasLeaveTheWindowUntouched() { + // Zero-sized areas show up while a display is being reconfigured; dividing by them would + // produce NaN coordinates that AX accepts and the user then cannot find. + let window = CGRect(x: 100, y: 100, width: 800, height: 600) + XCTAssertEqual(WindowManagerKit.frame(movingWindow: window, from: .zero, to: rightScreen), window) + XCTAssertEqual(WindowManagerKit.frame(movingWindow: window, from: primary, to: CGRect(x: 5000, y: 0, width: 0, height: 0)), window) + } + + func testMappingNeverProducesNonFiniteGeometry() { + let window = CGRect(x: primary.minX + 10, y: primary.minY + 10, width: 640, height: 480) + for destination in [leftScreen, rightScreen, CGRect(x: 0, y: -1080, width: 1920, height: 1080)] { + let moved = WindowManagerKit.frame(movingWindow: window, from: primary, to: destination) + XCTAssertTrue(moved.origin.x.isFinite && moved.origin.y.isFinite) + XCTAssertTrue(moved.width.isFinite && moved.height.isFinite) + XCTAssertGreaterThan(moved.width, 0) + XCTAssertGreaterThan(moved.height, 0) + } + } + + // MARK: - Clamping + + func testClampLeavesAnAlreadyContainedFrameAlone() { + let inside = CGRect(x: primary.minX + 50, y: primary.minY + 50, width: 400, height: 300) + XCTAssertEqual(WindowManagerKit.clamped(inside, into: primary), inside) + } + + func testClampShrinksAndRepositionsAsNeeded() { + let huge = CGRect(x: -9000, y: -9000, width: 99_999, height: 99_999) + XCTAssertEqual(WindowManagerKit.clamped(huge, into: primary), primary) + } + + func testClampOnADegenerateAreaReturnsTheFrameUnchanged() { + let frame = CGRect(x: 10, y: 20, width: 30, height: 40) + XCTAssertEqual(WindowManagerKit.clamped(frame, into: .zero), frame) + } + + // MARK: - Display cycling + + func testNextDisplayIsNilWhenThereIsNowhereToGo() { + XCTAssertNil(WindowManagerKit.nextAreaIndex(after: 0, in: []), "no displays") + XCTAssertNil(WindowManagerKit.nextAreaIndex(after: 0, in: [primary]), "single display") + XCTAssertNil(WindowManagerKit.nextAreaIndex(after: 7, in: [primary, rightScreen]), "index out of range") + XCTAssertNil(WindowManagerKit.nextAreaIndex(after: -1, in: [primary, rightScreen]), "negative index") + } + + func testCycleFollowsPhysicalLeftToRightOrderNotScreensOrder() { + // NSScreen.screens puts the primary first regardless of where it physically sits, so the + // list order here is primary, left, right — cycling must still go left → primary → right. + let areas = [primary, leftScreen, rightScreen] + XCTAssertEqual(WindowManagerKit.displayCycleOrder(of: areas), [1, 0, 2]) + XCTAssertEqual(WindowManagerKit.nextAreaIndex(after: 1, in: areas), 0) + XCTAssertEqual(WindowManagerKit.nextAreaIndex(after: 0, in: areas), 2) + XCTAssertEqual(WindowManagerKit.nextAreaIndex(after: 2, in: areas), 1, "wraps back to the leftmost") + } + + func testCycleVisitsEveryDisplayExactlyOnceBeforeReturning() { + let areas = [primary, leftScreen, rightScreen, CGRect(x: 0, y: -1080, width: 1920, height: 1080)] + for start in areas.indices { + var visited = [start] + var current = start + for _ in 1.. WindowIdentity { + WindowIdentity(pid: pid, windowNumber: number) + } + + func testRememberPeekAndForget() { + var memory = WindowFrameMemory() + let key = identity(501, 42) + let frame = CGRect(x: 10, y: 20, width: 800, height: 600) + + XCTAssertNil(memory.frame(for: key), "nothing remembered yet") + memory.remember(frame, for: key) + XCTAssertEqual(memory.frame(for: key), frame) + XCTAssertEqual(memory.frame(for: key), frame, "peeking does not consume — the write may still fail") + + memory.forget(key) + XCTAssertNil(memory.frame(for: key), "a landed restore consumes its entry") + XCTAssertEqual(memory.count, 0) + } + + func testRememberingAgainReplacesTheEarlierFrame() { + // Restore undoes the *last* action, so a second snap must overwrite rather than stack. + var memory = WindowFrameMemory() + let key = identity(501, 42) + memory.remember(CGRect(x: 0, y: 0, width: 100, height: 100), for: key) + memory.remember(CGRect(x: 5, y: 5, width: 200, height: 200), for: key) + XCTAssertEqual(memory.frame(for: key), CGRect(x: 5, y: 5, width: 200, height: 200)) + XCTAssertEqual(memory.count, 1) + } + + func testDistinctWindowsAndAppsGetDistinctSlots() { + var memory = WindowFrameMemory() + let sameAppWindowA = identity(501, 1) + let sameAppWindowB = identity(501, 2) + let otherApp = identity(777, 1) + // An app that withholds AXWindowNumber shares one slot per pid — coarser, but distinct + // from the numbered windows of the same app. + let numberless = identity(501, nil) + + memory.remember(CGRect(x: 1, y: 0, width: 10, height: 10), for: sameAppWindowA) + memory.remember(CGRect(x: 2, y: 0, width: 10, height: 10), for: sameAppWindowB) + memory.remember(CGRect(x: 3, y: 0, width: 10, height: 10), for: otherApp) + memory.remember(CGRect(x: 4, y: 0, width: 10, height: 10), for: numberless) + + XCTAssertEqual(memory.count, 4) + XCTAssertEqual(memory.frame(for: sameAppWindowA)?.minX, 1) + XCTAssertEqual(memory.frame(for: sameAppWindowB)?.minX, 2) + XCTAssertEqual(memory.frame(for: otherApp)?.minX, 3) + XCTAssertEqual(memory.frame(for: numberless)?.minX, 4) + } + + func testMemoryIsBoundedAndEvictsTheLeastRecentlyRemembered() { + // Windows close without telling us, so the store must never grow with the number of + // windows the user has ever snapped. + var memory = WindowFrameMemory(capacity: 3) + for index in 0..<10 { + memory.remember(CGRect(x: CGFloat(index), y: 0, width: 10, height: 10), for: identity(501, index)) + } + XCTAssertEqual(memory.count, 3) + XCTAssertNil(memory.frame(for: identity(501, 6)), "old entries are gone") + XCTAssertEqual(memory.frame(for: identity(501, 7))?.minX, 7) + XCTAssertEqual(memory.frame(for: identity(501, 9))?.minX, 9) + } + + func testReRememberingRefreshesRecencySoAnActiveWindowIsNotEvicted() { + var memory = WindowFrameMemory(capacity: 3) + let pinned = identity(501, 1) + memory.remember(CGRect(x: 1, y: 0, width: 10, height: 10), for: pinned) + memory.remember(CGRect(x: 2, y: 0, width: 10, height: 10), for: identity(501, 2)) + memory.remember(CGRect(x: 3, y: 0, width: 10, height: 10), for: identity(501, 3)) + // Touch the oldest entry, then push two more in: the touched one must survive. + memory.remember(CGRect(x: 11, y: 0, width: 10, height: 10), for: pinned) + memory.remember(CGRect(x: 4, y: 0, width: 10, height: 10), for: identity(501, 4)) + memory.remember(CGRect(x: 5, y: 0, width: 10, height: 10), for: identity(501, 5)) + + XCTAssertEqual(memory.count, 3) + XCTAssertEqual(memory.frame(for: pinned)?.minX, 11) + XCTAssertNil(memory.frame(for: identity(501, 2)), "the genuinely oldest entry was evicted") + XCTAssertNil(memory.frame(for: identity(501, 3))) + } + + func testForgettingAnUnknownIdentityIsHarmless() { + var memory = WindowFrameMemory(capacity: 2) + memory.forget(identity(501, 1)) + XCTAssertEqual(memory.count, 0) + memory.remember(CGRect(x: 1, y: 0, width: 10, height: 10), for: identity(501, 1)) + memory.forget(identity(501, 2)) + XCTAssertEqual(memory.count, 1) + } + + func testCapacityIsNeverZeroSoAtLeastOneUndoAlwaysExists() { + var memory = WindowFrameMemory(capacity: 0) + memory.remember(CGRect(x: 1, y: 0, width: 10, height: 10), for: identity(501, 1)) + XCTAssertEqual(memory.count, 1) + } + + /// A remembered frame from a display that has since been unplugged must be clamped onto a + /// screen that still exists — the controller does exactly this before writing a restore. + func testRestoringAFrameFromAnUnpluggedDisplayLandsOnScreen() { + let stale = CGRect(x: 5000, y: 5000, width: 900, height: 700) + XCTAssertNil(WindowManagerKit.areaIndex(forWindow: stale, in: [primary]), "the old display is gone") + let rescued = WindowManagerKit.clamped(stale, into: primary) + assertInside(rescued, primary, "restored frame from an unplugged display") + XCTAssertEqual(rescued.size, stale.size, "the remembered size still fits, so it is kept") + } + + // MARK: - Action wiring + + func testDisplayActionsAreNotAreaRelative() { + XCTAssertFalse(WindowAction.nextDisplay.isAreaRelative) + XCTAssertFalse(WindowAction.restorePrevious.isAreaRelative) + XCTAssertTrue(WindowAction.leftHalf.isAreaRelative) + XCTAssertTrue(WindowAction.maximize.isAreaRelative) + } + + func testDisplayActionsCarryUiMetadataAndDefaultShortcuts() { + for action in [WindowAction.nextDisplay, .restorePrevious] { + XCTAssertFalse(action.title.isEmpty) + XCTAssertFalse(action.symbol.isEmpty) + let shortcut = action.defaultShortcut + XCTAssertNotNil(shortcut, "\(action.rawValue) should ship with a binding like the other actions") + XCTAssertTrue(shortcut?.isUsableGlobally == true, "\(action.rawValue) default must be safe to claim globally") + XCTAssertFalse(shortcut?.displayString.contains("Key ") == true, "\(action.rawValue) shortcut lacks a glyph") + } + XCTAssertEqual(WindowAction.nextDisplay.defaultShortcut?.displayString, "⌃⌥⌘→") + XCTAssertEqual(WindowAction.restorePrevious.defaultShortcut?.displayString, "⌃⌥⌫") + } + + func testDisplayActionShortcutsDoNotCollideWithTheSnapDefaults() { + // ⌃⌥→ is already Right Half; the display move escalates to ⌃⌥⌘→ rather than stealing it. + XCTAssertNotEqual(WindowAction.nextDisplay.defaultShortcut, WindowAction.rightHalf.defaultShortcut) + var seen = Set() + for action in WindowAction.allCases { + guard let shortcut = action.defaultShortcut else { continue } + XCTAssertTrue(seen.insert(shortcut).inserted, "\(action.rawValue) duplicates another default") + } + } + + func testDisplayActionsRoundTripThroughTheShortcutStore() { + // They must be bindable exactly like the snaps: same persistence, same rawValue keying. + let suite = "test.windowmanager.displayactions.\(UUID().uuidString)" + guard let defaults = UserDefaults(suiteName: suite) else { + return XCTFail("could not create a scratch defaults suite") + } + defaults.removePersistentDomain(forName: suite) + addTeardownBlock { UserDefaults(suiteName: suite)?.removePersistentDomain(forName: suite) } + + let store = WindowShortcutStore(defaults: defaults) + XCTAssertEqual(store.effectiveShortcuts()[.nextDisplay], WindowAction.nextDisplay.defaultShortcut) + XCTAssertEqual(store.effectiveShortcuts()[.restorePrevious], WindowAction.restorePrevious.defaultShortcut) + + let custom = WindowShortcut(keyCode: 105, modifiers: 0) // bare F13 + store.save(custom, for: .nextDisplay) + XCTAssertEqual(store.effectiveShortcuts()[.nextDisplay], custom) + XCTAssertEqual(store.storedShortcuts()[.nextDisplay], custom) + store.reset() + XCTAssertEqual(store.effectiveShortcuts()[.nextDisplay], WindowAction.nextDisplay.defaultShortcut) + } + + @MainActor + func testControllerBindsTheDisplayActionsLikeAnyOther() { + let suite = "test.windowmanager.displaycontroller.\(UUID().uuidString)" + guard let defaults = UserDefaults(suiteName: suite) else { + return XCTFail("could not create a scratch defaults suite") + } + defaults.removePersistentDomain(forName: suite) + addTeardownBlock { UserDefaults(suiteName: suite)?.removePersistentDomain(forName: suite) } + + let controller = WindowManagerController(defaults: defaults) + defer { controller.unregisterHotKeys() } + + XCTAssertEqual(controller.shortcut(for: .nextDisplay), WindowAction.nextDisplay.defaultShortcut) + XCTAssertEqual(controller.shortcut(for: .restorePrevious), WindowAction.restorePrevious.defaultShortcut) + + // Duplicate prevention covers them too. + guard let leftHalf = WindowAction.leftHalf.defaultShortcut else { + return XCTFail("left half should have a default") + } + XCTAssertEqual(controller.assignShortcut(leftHalf, to: .nextDisplay), .conflict(.leftHalf)) + + let custom = WindowShortcut(keyCode: 0x2D, modifiers: HotKeyModifier.controlOptionCommand) // ⌃⌥⌘N + XCTAssertEqual(controller.assignShortcut(custom, to: .nextDisplay), .assigned) + XCTAssertEqual(controller.shortcut(for: .nextDisplay), custom) + } + + /// Without Accessibility the display actions must refuse in the same way the snaps do, rather + /// than reporting a bogus success. The grant is a system state we cannot fake, so this asserts + /// whichever branch the test machine is actually in. + @MainActor + func testDisplayActionsHonourTheAccessibilityGate() { + let suite = "test.windowmanager.displaypermission.\(UUID().uuidString)" + guard let defaults = UserDefaults(suiteName: suite) else { + return XCTFail("could not create a scratch defaults suite") + } + defaults.removePersistentDomain(forName: suite) + addTeardownBlock { UserDefaults(suiteName: suite)?.removePersistentDomain(forName: suite) } + + let controller = WindowManagerController(defaults: defaults) + defer { controller.unregisterHotKeys() } + + let result = controller.apply(.restorePrevious) + if AXIsProcessTrusted() { + XCTAssertNotEqual(result, .needsPermission, "trusted process should get past the gate") + } else { + XCTAssertEqual(result, .needsPermission) + XCTAssertFalse(controller.hasAccessibility) + } + } +} diff --git a/Tests/DMonteCoreTests/WindowManagerKitTests.swift b/Tests/DMonteCoreTests/WindowManagerKitTests.swift index 1fe2cf7..bfb1a96 100644 --- a/Tests/DMonteCoreTests/WindowManagerKitTests.swift +++ b/Tests/DMonteCoreTests/WindowManagerKitTests.swift @@ -9,8 +9,10 @@ final class WindowManagerKitTests: XCTestCase { /// bugs surface. private let area = CGRect(x: 100, y: 50, width: 1200, height: 800) + /// Every action asserted on below is area-relative, so a nil here is a kit bug. `.infinite` + /// makes that bug fail the containment assertions loudly instead of being swallowed. private func frame(_ action: WindowAction) -> CGRect { - WindowManagerKit.frame(for: action, in: area) + WindowManagerKit.frame(for: action, in: area) ?? .infinite } // MARK: - Corner chords @@ -143,16 +145,16 @@ final class WindowManagerKitTests: XCTestCase { } func testAllActionsProduceFinitePositiveSizes() { - for action in WindowAction.allCases { - let f = WindowManagerKit.frame(for: action, in: area) + for action in WindowAction.areaRelativeCases { + let f = frame(action) XCTAssertTrue(f.width > 0 && f.height > 0, "\(action.rawValue) produced non-positive size \(f)") XCTAssertTrue(f.width.isFinite && f.height.isFinite, "\(action.rawValue) produced non-finite size") } } func testAllActionsStayWithinArea() { - for action in WindowAction.allCases { - let f = WindowManagerKit.frame(for: action, in: area) + for action in WindowAction.areaRelativeCases { + let f = frame(action) XCTAssertGreaterThanOrEqual(f.minX, area.minX - 0.001, "\(action.rawValue) escaped left") XCTAssertGreaterThanOrEqual(f.minY, area.minY - 0.001, "\(action.rawValue) escaped top") XCTAssertLessThanOrEqual(f.maxX, area.maxX + 0.001, "\(action.rawValue) escaped right") @@ -160,6 +162,20 @@ final class WindowManagerKitTests: XCTestCase { } } + /// The area-relative set must stay exactly "everything except the two display actions", so + /// switching the loops above off `allCases` cannot become a way for coverage to quietly shrink. + func testAreaRelativeCasesCoverEverythingButTheDisplayActions() { + XCTAssertEqual( + Set(WindowAction.areaRelativeCases), + Set(WindowAction.allCases).subtracting([.nextDisplay, .restorePrevious]) + ) + for action in WindowAction.areaRelativeCases { + XCTAssertNotNil(WindowManagerKit.frame(for: action, in: area), "\(action.rawValue) has no area frame") + } + XCTAssertNil(WindowManagerKit.frame(for: .nextDisplay, in: area)) + XCTAssertNil(WindowManagerKit.frame(for: .restorePrevious, in: area)) + } + func testActionMetadataIsComplete() { for action in WindowAction.allCases { XCTAssertFalse(action.title.isEmpty, "\(action.rawValue) missing title") diff --git a/Tests/DMonteCoreTests/WindowManagerScreenMatchingTests.swift b/Tests/DMonteCoreTests/WindowManagerScreenMatchingTests.swift index d136fa2..8725332 100644 --- a/Tests/DMonteCoreTests/WindowManagerScreenMatchingTests.swift +++ b/Tests/DMonteCoreTests/WindowManagerScreenMatchingTests.swift @@ -50,8 +50,11 @@ final class WindowManagerScreenMatchingTests: XCTestCase { for (index, area) in realArrangement.enumerated() { let matched = WindowManagerKit.areaIndex(forWindow: window(centeredIn: area), in: realArrangement) XCTAssertEqual(matched, index) - for action in WindowAction.allCases { - let target = WindowManagerKit.frame(for: action, in: area) + for action in WindowAction.areaRelativeCases { + guard let target = WindowManagerKit.frame(for: action, in: area) else { + XCTFail("\(action.rawValue) is area-relative but produced no frame") + continue + } XCTAssertGreaterThanOrEqual(target.minX, area.minX - 0.001, "\(action.rawValue) escaped screen \(index) left") XCTAssertGreaterThanOrEqual(target.minY, area.minY - 0.001, "\(action.rawValue) escaped screen \(index) top") XCTAssertLessThanOrEqual(target.maxX, area.maxX + 0.001, "\(action.rawValue) escaped screen \(index) right") @@ -165,8 +168,11 @@ final class WindowManagerRealScreensDiagnosticTests: XCTestCase { let matched = WindowManagerKit.areaIndex(forWindow: window, in: areas) XCTAssertEqual(matched, index, "synthetic window centered on screen \(index) matched \(String(describing: matched))") - for action in WindowAction.allCases { - let target = WindowManagerKit.frame(for: action, in: area) + for action in WindowAction.areaRelativeCases { + guard let target = WindowManagerKit.frame(for: action, in: area) else { + XCTFail("\(action.rawValue) is area-relative but produced no frame") + continue + } XCTAssertGreaterThanOrEqual(target.minX, area.minX - 0.001, "\(action.rawValue) escaped screen \(index)") XCTAssertGreaterThanOrEqual(target.minY, area.minY - 0.001, "\(action.rawValue) escaped screen \(index)") XCTAssertLessThanOrEqual(target.maxX, area.maxX + 0.001, "\(action.rawValue) escaped screen \(index)") From 3ac7d919d0bd6dbdaf6757d1a4a0184ecee25449 Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Tue, 21 Jul 2026 23:13:57 +0530 Subject: [PATCH 2/2] Window Manager: make the undo undo the gesture, not the last write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the display actions turned up four ways the restore point ended up pointing somewhere the user never was, plus one way a new default shortcut could double-book a combination the user had already bound. The restore point was written on every verified apply, which is wrong three different times. A corner chord (the popover's own tip: tap Right then Up) is one gesture that costs two applies, and the second overwrote the frame the first had stored with the intermediate half-snap — Restore put the window back to the right half instead of where the user had it. Pressing the same snap twice overwrote it with the window's already-snapped frame, so the undo silently became a no-op. And an app that accepts the position but drops the size — the live-diagnosed AXEnhancedUserInterface case this file already documents — moves the window hundreds of points, fails verification, and stored nothing at all, so Restore answered "nothing to restore" for a move the user had just watched happen. All three come from asking the wrong question. What matters is not whether the frame landed exactly where we asked but whether the window *moved*, so the decision now runs off the frame read back after the writes: it is remembered when the window ended up somewhere other than where it started, kept when nothing changed, and kept when this apply is the second half of a chord whose opening half already recorded the pre-gesture frame. The chord carries that fact through ChordState, so a chord whose opening half stored nothing still records its own. The policy is a pure function over (previous, target, achieved), which is what makes a gesture testable without a real window. Restore also clamped the remembered frame into a single screen's visible area unconditionally. A window straddling two displays is a placement the user chose and the one this feature exists for: areaIndex picks whichever screen holds more of it and the clamp then slides the window onto that screen, 760 pt from where it was on the reporter's arrangement. The clamp now only runs when the remembered frame meets no screen at all, which is the case it was written for — the display it came from was unplugged, and writing it back verbatim would strand the window where the user cannot grab it. Finally, restorePrevious's default is a combination a 0.14.0 user may already have bound to something else. Layering defaults under overrides let both actions hold it, and since the registrar claims one Carbon hotkey per action, both would register and one keypress would fire both in no fixed order. An explicit binding now outranks a built-in default: the colliding default is dropped and its action shows as unset until the user picks something. The registrar refuses a repeated combination within a pass as well, so the invariant holds at the point of consequence and not only at its source. Co-Authored-By: Claude Opus 4.8 --- .../DMonteCore/WindowManagerController.swift | 127 ++++++++++---- Sources/DMonteCore/WindowManagerKit.swift | 94 +++++++++- .../WindowManagerDisplayActionTests.swift | 160 +++++++++++++++++- .../WindowManagerKitTests.swift | 24 ++- .../DMonteCoreTests/WindowShortcutTests.swift | 55 ++++++ 5 files changed, 413 insertions(+), 47 deletions(-) diff --git a/Sources/DMonteCore/WindowManagerController.swift b/Sources/DMonteCore/WindowManagerController.swift index 969912f..892945a 100644 --- a/Sources/DMonteCore/WindowManagerController.swift +++ b/Sources/DMonteCore/WindowManagerController.swift @@ -320,12 +320,19 @@ public final class WindowManagerController: NSObject, ObservableObject { registrationFailures = [:] var nextID: UInt32 = 1 + // Every combination may be claimed once per pass. Carbon happily registers the same combo + // twice under different ids, and both handlers then fire on one keypress — two actions + // fighting over the same window in no fixed order. `WindowShortcutStore` already keeps a + // default from colliding with a user override; this is the backstop at the point of + // consequence, for any path that ever hands us a duplicate. + var claimedCombos: Set = [] for action in WindowAction.allCases { guard let shortcut = shortcuts[action] else { continue } let keyCodes = Self.registrationKeyCodes(for: shortcut).filter { keyCode in keyCode == shortcut.keyCode || actionUsing(WindowShortcut(keyCode: keyCode, modifiers: shortcut.modifiers), excluding: action) == nil } for keyCode in keyCodes { + guard claimedCombos.insert(WindowShortcut(keyCode: keyCode, modifiers: shortcut.modifiers)).inserted else { continue } let id = nextID nextID += 1 do { @@ -436,12 +443,15 @@ public final class WindowManagerController: NSObject, ObservableObject { } let now = Date() switch WindowManagerKit.resolveChord(previous: chordState, current: action, now: now) { - case .corner(let corner): + case .corner(let corner, let preservesRestorePoint): chordState = nil - apply(corner) + // The chord is two applies but one gesture. The half already stored the frame the user + // had before any of it; overwriting that here with the intermediate half-snapped frame + // would leave Restore undoing the chord back to the half, not to where they were. + applyOutcome(corner, preservesRestorePoint: preservesRestorePoint) case .half(let half): - chordState = WindowManagerKit.ChordState(action: half, at: now) - apply(half) + let outcome = applyOutcome(half, preservesRestorePoint: false) + chordState = WindowManagerKit.ChordState(action: half, at: now, storedRestorePoint: outcome.storedRestorePoint) } } @@ -449,30 +459,48 @@ public final class WindowManagerController: NSObject, ObservableObject { /// publishes it to `lastResult`. @discardableResult public func apply(_ action: WindowAction) -> WindowApplyResult { - lastResult = applyResult(for: action) - return lastResult ?? .failed + applyOutcome(action, preservesRestorePoint: false).result } - private func applyResult(for action: WindowAction) -> WindowApplyResult { + /// One apply's full outcome. `storedRestorePoint` is what the corner completing a chord needs + /// from the half that opened it: whether there is already an undo for this gesture to preserve. + private struct ApplyOutcome { + let result: WindowApplyResult + let storedRestorePoint: Bool + + init(_ result: WindowApplyResult, storedRestorePoint: Bool = false) { + self.result = result + self.storedRestorePoint = storedRestorePoint + } + } + + @discardableResult + private func applyOutcome(_ action: WindowAction, preservesRestorePoint: Bool) -> ApplyOutcome { + let outcome = applyResult(for: action, preservesRestorePoint: preservesRestorePoint) + lastResult = outcome.result + return outcome + } + + private func applyResult(for action: WindowAction, preservesRestorePoint: Bool) -> ApplyOutcome { guard AXIsProcessTrusted() else { hasAccessibility = false - return .needsPermission + return ApplyOutcome(.needsPermission) } hasAccessibility = true guard let target = targetApplication(), let targetApp = target.app else { - return .noFocusedWindow + return ApplyOutcome(.noFocusedWindow) } let appElement = AXUIElementCreateApplication(targetApp.processIdentifier) // Bound how long a hung app may block us; without this a beachballing app freezes the // helper (and the popover) for the system default of several seconds per call. AXUIElementSetMessagingTimeout(appElement, Self.axMessagingTimeoutSeconds) guard let window = target.windowSnapshot.flatMap({ Self.window(in: appElement, matching: $0) }) ?? Self.focusedWindow(of: appElement) else { - return .noFocusedWindow + return ApplyOutcome(.noFocusedWindow) } guard let currentFrame = Self.frame(of: window) else { - return .failed + return ApplyOutcome(.failed) } let identity = WindowIdentity(pid: targetApp.processIdentifier, windowNumber: Self.windowNumber(of: window)) @@ -482,7 +510,7 @@ public final class WindowManagerController: NSObject, ObservableObject { // is clicked, the key window is our own popover, so NSScreen.main is the popover's screen, // not anything to do with the target window). let areas = NSScreen.screens.map { Self.axRect(fromCocoa: $0.visibleFrame) } - guard !areas.isEmpty else { return .failed } + guard !areas.isEmpty else { return ApplyOutcome(.failed) } let areaIndex = WindowManagerKit.areaIndex(forWindow: currentFrame, in: areas) ?? 0 let axArea = areas[areaIndex] @@ -490,39 +518,62 @@ public final class WindowManagerController: NSObject, ObservableObject { switch action { case .nextDisplay: guard let destination = WindowManagerKit.nextAreaIndex(after: areaIndex, in: areas) else { - return .noOtherDisplay + return ApplyOutcome(.noOtherDisplay) } targetFrame = WindowManagerKit.frame(movingWindow: currentFrame, from: axArea, to: areas[destination]) case .restorePrevious: guard let remembered = frameMemory.frame(for: identity) else { - return .nothingToRestore + return ApplyOutcome(.nothingToRestore) } - // The display the frame was captured on may since have been unplugged or rearranged, - // which would strand the window in coordinates no screen covers. Clamp into whichever - // screen the remembered frame still belongs to, falling back to the window's current - // screen when it belongs to none. - let host = WindowManagerKit.areaIndex(forWindow: remembered, in: areas).map { areas[$0] } ?? axArea - targetFrame = WindowManagerKit.clamped(remembered, into: host) + // Written back as remembered whenever it still meets a screen — a frame spanning two + // displays is a placement restore promises to return to, not one to "fix". Only a frame + // whose display has since been unplugged or rearranged is rescued onto this window's + // current screen (see `restoreFrame`). + targetFrame = WindowManagerKit.restoreFrame(remembered, onScreens: areas, fallback: axArea) default: guard let frame = WindowManagerKit.frame(for: action, in: axArea) else { - return .failed + return ApplyOutcome(.failed) } targetFrame = frame } - let result = setFrameVerified(targetFrame, on: window, appElement: appElement, appName: targetApp.localizedName) + let attempt = setFrameVerified(targetFrame, on: window, appElement: appElement, appName: targetApp.localizedName) - // Only a landed move changes what "before the last action" means, so the restore point is - // updated after verification, never before: a snap the app refused must not consume or - // overwrite the undo the user still has. - if case .success = result { - if action == .restorePrevious { - frameMemory.forget(identity) - } else { - frameMemory.remember(currentFrame, for: identity) - } + // Bookkeeping runs on what the window actually ended up with, never on the reported result: + // an app that moves the window but drops the size fails verification and still owes the user + // an undo, while a snap the window was already sitting in changes nothing and must leave the + // undo it has alone. The policy itself is pure (`restorePointUpdate`). + var storedRestorePoint = false + switch WindowManagerKit.restorePointUpdate( + for: action, + previous: currentFrame, + target: targetFrame, + achieved: attempt.achieved, + preservesRestorePoint: preservesRestorePoint + ) { + case .remember: + frameMemory.remember(currentFrame, for: identity) + storedRestorePoint = true + case .keep: + break + case .forget: + frameMemory.forget(identity) + } + return ApplyOutcome(attempt.result, storedRestorePoint: storedRestorePoint) + } + + /// What one `setFrameVerified` pass produced: the outcome to report, plus the frame the window + /// actually ended up with (nil when nothing was written or it could not be read back). The + /// achieved frame is what separates "the app ignored us" from "the app moved the window, just + /// not exactly where we asked", which is what decides whether there is an undo to record. + private struct FrameSetAttempt { + let result: WindowApplyResult + let achieved: CGRect? + + init(_ result: WindowApplyResult, achieved: CGRect? = nil) { + self.result = result + self.achieved = achieved } - return result } /// Writes `target` to the window and verifies it landed, retrying with alternating set @@ -531,7 +582,7 @@ public final class WindowManagerController: NSObject, ObservableObject { /// with `AXEnhancedUserInterface` set — the window moved but kept its size). Reads back after /// each attempt and only reports success when the achieved frame matches within /// `WindowManagerKit.frameMatchTolerance`. - private func setFrameVerified(_ target: CGRect, on window: AXUIElement, appElement: AXUIElement, appName: String?) -> WindowApplyResult { + private func setFrameVerified(_ target: CGRect, on window: AXUIElement, appElement: AXUIElement, appName: String?) -> FrameSetAttempt { // Clear AXEnhancedUserInterface for the duration of the writes (restore after): while it // is set, the app animates position changes and silently drops size changes that arrive // mid-animation. With it cleared, the same writes apply exactly, first try. @@ -549,21 +600,23 @@ public final class WindowManagerController: NSObject, ObservableObject { for order in WindowManagerKit.frameSetAttempts { let results = performFrameSets(target, on: window, order: order) if results.contains(.apiDisabled) { - return .needsPermission + // The grant was pulled mid-write. Whatever landed before that is unknown, so the + // achieved frame from an earlier attempt is deliberately not carried forward. + return FrameSetAttempt(.needsPermission) } // Tiny settle so apps that apply geometry asynchronously finish before the read-back. usleep(Self.frameVerifyDelayMicroseconds) guard let now = Self.frame(of: window) else { - return .failed + return FrameSetAttempt(.failed) } achieved = now if WindowManagerKit.frameMatches(now, target: target) { - return .success(appName: appName) + return FrameSetAttempt(.success(appName: appName), achieved: now) } } Self.log.debug("Snap failed verification: target \(target.debugDescription, privacy: .public), achieved \(achieved?.debugDescription ?? "nil", privacy: .public), app \(appName ?? "?", privacy: .public)") - return .failed + return FrameSetAttempt(.failed, achieved: achieved) } /// One write pass in the given order. Both orders write the redundant first attribute again diff --git a/Sources/DMonteCore/WindowManagerKit.swift b/Sources/DMonteCore/WindowManagerKit.swift index afe6819..8aaea82 100644 --- a/Sources/DMonteCore/WindowManagerKit.swift +++ b/Sources/DMonteCore/WindowManagerKit.swift @@ -228,13 +228,24 @@ public struct WindowShortcutStore { return result } - /// The shortcuts that should actually be registered: defaults overlaid with stored overrides. + /// The shortcuts that should actually be registered: defaults overlaid with stored overrides, + /// with any default that collides with an override dropped. + /// + /// The collision check is what keeps "one combination, one action" true across upgrades. A + /// user who bound ⌃⌥⌫ to Center before Restore shipped with that combination as its default + /// would otherwise get both actions holding it: the registrar claims a Carbon hotkey per + /// action, so one keypress would center *and* restore the window, in no fixed order. An + /// explicit user binding outranks a built-in default, so the default yields and its action is + /// left unbound (the UI shows "Set", and the user can pick something else). public func effectiveShortcuts() -> [WindowAction: WindowShortcut] { + let overrides = storedShortcuts() + let claimed = Set(overrides.values) var result: [WindowAction: WindowShortcut] = [:] for action in WindowAction.allCases { - if let shortcut = action.defaultShortcut { result[action] = shortcut } + guard let shortcut = action.defaultShortcut, !claimed.contains(shortcut) else { continue } + result[action] = shortcut } - for (action, shortcut) in storedShortcuts() { result[action] = shortcut } + for (action, shortcut) in overrides { result[action] = shortcut } return result } @@ -329,6 +340,24 @@ public enum WindowManagerKit { return CGRect(x: x, y: y, width: width, height: height) } + /// The frame a restore should actually write for a `remembered` one, given the displays that + /// exist *now*. `fallback` is the area to rescue it into (the controller passes the window's + /// current screen). + /// + /// A remembered frame that still meets a screen is written back untouched — including one that + /// straddles two displays, which is a placement the user chose and restore exists to return to. + /// Clamping unconditionally into whichever single screen holds most of the window would move a + /// straddling window by hundreds of points, i.e. defeat the feature on exactly the multi-display + /// desk it is for. + /// + /// The clamp is kept for the case it was written for: the display the frame came from has since + /// been unplugged or rearranged, so the frame lies in coordinates no screen covers any more and + /// writing it back verbatim would strand the window where the user cannot grab it. Pure. + public static func restoreFrame(_ remembered: CGRect, onScreens areas: [CGRect], fallback: CGRect) -> CGRect { + guard areaIndex(forWindow: remembered, in: areas) == nil else { return remembered } + return clamped(remembered, into: fallback) + } + /// Maps `windowFrame` from the `source` usable area onto the `destination` usable area, keeping /// its relative position and its size *as a proportion of the screen*. All three rects are in AX /// top-left space, and both areas are visible frames — not full display frames — so the menu bar @@ -482,16 +511,25 @@ public enum WindowManagerKit { public struct ChordState: Equatable, Sendable { public let action: WindowAction public let at: Date - public init(action: WindowAction, at: Date) { + /// Whether that half-snap stored a restore point. A chord is two applies but one gesture, + /// so the corner that completes it must leave the pre-gesture frame the half stored alone; + /// when the half stored nothing (it changed nothing, or the app refused it) there is + /// nothing to preserve and the corner records its own. + public let storedRestorePoint: Bool + + public init(action: WindowAction, at: Date, storedRestorePoint: Bool = false) { self.action = action self.at = at + self.storedRestorePoint = storedRestorePoint } } /// What a hotkey-fired half-snap should do given the previous one. public enum ChordOutcome: Equatable, Sendable { - /// Apply this corner and clear the chord state. - case corner(WindowAction) + /// Apply this corner and clear the chord state. `preservesRestorePoint` carries the opening + /// half's `storedRestorePoint`: true means the undo for this gesture is already recorded and + /// this apply must not overwrite it with the intermediate half-snapped frame. + case corner(WindowAction, preservesRestorePoint: Bool) /// Apply this half and remember it (with `now`) as the new chord state. case half(WindowAction) } @@ -507,10 +545,52 @@ public enum WindowManagerKit { if let previous, now.timeIntervalSince(previous.at) <= window, let corner = cornerCombining(previous.action, current) { - return .corner(corner) + return .corner(corner, preservesRestorePoint: previous.storedRestorePoint) } return .half(current) } + + // MARK: - Restore points (pure) + + /// What one apply does to the window's restore point. + public enum RestorePointUpdate: Equatable, Sendable { + /// Store the window's pre-action frame: this apply moved it, so there is now an undo. + case remember + /// Leave the existing entry (if any) exactly as it is. + case keep + /// Drop the entry: a restore landed, so its undo has been spent. + case forget + } + + /// Decides the restore-point bookkeeping for one apply, from what actually happened to the + /// window. `previous` is the frame it had before, `target` what we asked for, `achieved` what + /// it has now (nil when it could not be read back at all). + /// + /// Three rules, each of which the naive "remember on success" version got wrong: + /// - a restore is undo, not a new arrangement: it consumes its entry when it lands, and keeps + /// it when it does not, so a refused restore can be retried; + /// - `preservesRestorePoint` marks the second apply of a corner chord, which is one user + /// gesture: overwriting there would leave Restore undoing only back to the intermediate + /// half-snap; + /// - otherwise what counts is whether the window *moved*, not whether it landed exactly where + /// we asked. Re-pressing a snap the window already sits in must not overwrite the undo with + /// the window's own current frame, and an app that moves the window but drops the size (the + /// live-diagnosed `AXEnhancedUserInterface` case) has genuinely rearranged it, so the user + /// must be able to undo that too. Pure. + public static func restorePointUpdate( + for action: WindowAction, + previous: CGRect, + target: CGRect, + achieved: CGRect?, + preservesRestorePoint: Bool = false + ) -> RestorePointUpdate { + guard let achieved else { return .keep } // nothing was written, or the window is unreadable + if action == .restorePrevious { + return frameMatches(achieved, target: target) ? .forget : .keep + } + if preservesRestorePoint { return .keep } + return frameMatches(achieved, target: previous) ? .keep : .remember + } } /// Identifies the window a remembered frame belongs to. The Quartz window number (read from the diff --git a/Tests/DMonteCoreTests/WindowManagerDisplayActionTests.swift b/Tests/DMonteCoreTests/WindowManagerDisplayActionTests.swift index 5d3d72f..8f19f00 100644 --- a/Tests/DMonteCoreTests/WindowManagerDisplayActionTests.swift +++ b/Tests/DMonteCoreTests/WindowManagerDisplayActionTests.swift @@ -346,11 +346,169 @@ final class WindowManagerDisplayActionTests: XCTestCase { func testRestoringAFrameFromAnUnpluggedDisplayLandsOnScreen() { let stale = CGRect(x: 5000, y: 5000, width: 900, height: 700) XCTAssertNil(WindowManagerKit.areaIndex(forWindow: stale, in: [primary]), "the old display is gone") - let rescued = WindowManagerKit.clamped(stale, into: primary) + let rescued = WindowManagerKit.restoreFrame(stale, onScreens: [primary], fallback: primary) assertInside(rescued, primary, "restored frame from an unplugged display") XCTAssertEqual(rescued.size, stale.size, "the remembered size still fits, so it is kept") } + /// The clamp exists for frames whose display is gone, and must not touch anything else: a + /// window deliberately spanning two displays is a placement restore promises to return to. + func testRestoringAFrameThatStraddlesTwoDisplaysPutsItBackUntouched() { + let areas = [primary, rightScreen] + // 760 pt of this window is on the primary, 1240 pt on the right screen. + let straddling = CGRect(x: 1800, y: 200, width: 2000, height: 900) + XCTAssertEqual( + WindowManagerKit.restoreFrame(straddling, onScreens: areas, fallback: primary), + straddling, + "restore relocated a window that legitimately spanned two displays" + ) + // The bug this pins: clamping into the single best-overlapping screen slides it 760 pt. + XCTAssertNotEqual(WindowManagerKit.clamped(straddling, into: rightScreen), straddling) + } + + func testRestoringAFrameThatStillFitsIsExact() { + let inside = CGRect(x: 137, y: 411, width: 913, height: 622) + XCTAssertEqual(WindowManagerKit.restoreFrame(inside, onScreens: [primary, rightScreen], fallback: primary), inside) + // Even one hanging off the edge of its own screen: the user put it there. + let hangingOff = CGRect(x: -300, y: 100, width: 900, height: 700) + XCTAssertEqual(WindowManagerKit.restoreFrame(hangingOff, onScreens: [primary], fallback: primary), hangingOff) + } + + // MARK: - Restore-point policy + + /// One apply, run through the same two steps the controller performs: decide the restore-point + /// update from what actually happened to the window, then apply it to the memory. Returns + /// whether this apply stored a restore point (what the controller feeds into the chord state). + @discardableResult + private func simulateApply( + _ action: WindowAction, + previous: CGRect, + target: CGRect, + achieved: CGRect?, + preservesRestorePoint: Bool = false, + into memory: inout WindowFrameMemory, + for key: WindowIdentity + ) -> Bool { + switch WindowManagerKit.restorePointUpdate( + for: action, + previous: previous, + target: target, + achieved: achieved, + preservesRestorePoint: preservesRestorePoint + ) { + case .remember: + memory.remember(previous, for: key) + return true + case .keep: + return false + case .forget: + memory.forget(key) + return false + } + } + + private func areaFrame(_ action: WindowAction) -> CGRect { + WindowManagerKit.frame(for: action, in: primary) ?? .infinite + } + + /// ⌃⌥→ then ⌃⌥↑ is one gesture that costs two applies. The second must not overwrite the undo + /// the first stored, or Restore puts the window back to the intermediate half-snap instead of + /// where the user had it. + func testCornerChordLeavesTheUndoPointingAtThePreGestureFrame() { + var memory = WindowFrameMemory() + let key = identity(501, 7) + let original = CGRect(x: 300, y: 200, width: 1200, height: 800) + let rightHalf = areaFrame(.rightHalf) + let topRight = areaFrame(.topRight) + + let halfStored = simulateApply(.rightHalf, previous: original, target: rightHalf, achieved: rightHalf, into: &memory, for: key) + XCTAssertTrue(halfStored, "the opening half is the apply that has an undo to record") + XCTAssertEqual(memory.frame(for: key), original) + + simulateApply( + .topRight, + previous: rightHalf, + target: topRight, + achieved: topRight, + preservesRestorePoint: true, + into: &memory, + for: key + ) + XCTAssertEqual(memory.frame(for: key), original, "the chord's corner overwrote the undo with the half-snap frame") + } + + /// The flip side, so "preserve" cannot quietly become "never overwrite": two half-snaps far + /// enough apart to be separate gestures each move the undo forward. + func testTwoIndependentHalfSnapsEachMoveTheUndoForward() { + var memory = WindowFrameMemory() + let key = identity(501, 7) + let original = CGRect(x: 300, y: 200, width: 1200, height: 800) + let rightHalf = areaFrame(.rightHalf) + let topHalf = areaFrame(.topHalf) + + simulateApply(.rightHalf, previous: original, target: rightHalf, achieved: rightHalf, into: &memory, for: key) + simulateApply(.topHalf, previous: rightHalf, target: topHalf, achieved: topHalf, into: &memory, for: key) + XCTAssertEqual(memory.frame(for: key), rightHalf, "a separate snap undoes back to the previous snap") + } + + /// Pressing a snap the window is already sitting in changes nothing, so it must not replace the + /// undo with the window's own current frame — that silently destroys the only way back. + func testRepeatingTheSameSnapKeepsTheOriginalUndo() { + var memory = WindowFrameMemory() + let key = identity(501, 7) + let original = CGRect(x: 300, y: 200, width: 1200, height: 800) + let leftHalf = areaFrame(.leftHalf) + + simulateApply(.leftHalf, previous: original, target: leftHalf, achieved: leftHalf, into: &memory, for: key) + let storedAgain = simulateApply(.leftHalf, previous: leftHalf, target: leftHalf, achieved: leftHalf, into: &memory, for: key) + XCTAssertFalse(storedAgain, "a no-op apply has nothing to remember") + XCTAssertEqual(memory.frame(for: key), original) + } + + /// The live-diagnosed Electron case: the app takes the position and drops the size, so the + /// frame fails verification while the window has plainly moved. There must still be an undo. + func testAMoveThatFailsVerificationStillLeavesAnUndo() { + var memory = WindowFrameMemory() + let key = identity(501, 7) + let original = CGRect(x: 300, y: 200, width: 900, height: 700) + let rightHalf = areaFrame(.rightHalf) + let achieved = CGRect(x: rightHalf.minX, y: rightHalf.minY, width: 900, height: 700) // size dropped + + XCTAssertFalse(WindowManagerKit.frameMatches(achieved, target: rightHalf), "this is a verification failure") + let stored = simulateApply(.rightHalf, previous: original, target: rightHalf, achieved: achieved, into: &memory, for: key) + XCTAssertTrue(stored, "the window moved hundreds of points; the user must be able to undo it") + XCTAssertEqual(memory.frame(for: key), original) + } + + func testARestoreConsumesItsUndoOnlyWhenItLands() { + var memory = WindowFrameMemory() + let key = identity(501, 7) + let original = CGRect(x: 300, y: 200, width: 1200, height: 800) + let leftHalf = areaFrame(.leftHalf) + memory.remember(original, for: key) + + // A restore the app refused keeps the entry, so pressing it again can still work. + simulateApply(.restorePrevious, previous: leftHalf, target: original, achieved: leftHalf, into: &memory, for: key) + XCTAssertEqual(memory.frame(for: key), original) + + simulateApply(.restorePrevious, previous: leftHalf, target: original, achieved: original, into: &memory, for: key) + XCTAssertNil(memory.frame(for: key), "a landed restore spends its undo") + } + + /// Nothing readable came back (the window vanished, or the grant was pulled mid-write), so the + /// apply knows nothing about what happened and must not touch the undo either way. + func testAnUnreadableWindowLeavesTheUndoAlone() { + var memory = WindowFrameMemory() + let key = identity(501, 7) + let original = CGRect(x: 300, y: 200, width: 1200, height: 800) + memory.remember(original, for: key) + + simulateApply(.leftHalf, previous: original, target: areaFrame(.leftHalf), achieved: nil, into: &memory, for: key) + XCTAssertEqual(memory.frame(for: key), original) + simulateApply(.restorePrevious, previous: original, target: original, achieved: nil, into: &memory, for: key) + XCTAssertEqual(memory.frame(for: key), original, "a restore that wrote nothing keeps its undo") + } + // MARK: - Action wiring func testDisplayActionsAreNotAreaRelative() { diff --git a/Tests/DMonteCoreTests/WindowManagerKitTests.swift b/Tests/DMonteCoreTests/WindowManagerKitTests.swift index bfb1a96..d1b163c 100644 --- a/Tests/DMonteCoreTests/WindowManagerKitTests.swift +++ b/Tests/DMonteCoreTests/WindowManagerKitTests.swift @@ -38,7 +38,7 @@ final class WindowManagerKitTests: XCTestCase { let previous = WindowManagerKit.ChordState(action: .rightHalf, at: t0) XCTAssertEqual( WindowManagerKit.resolveChord(previous: previous, current: .topHalf, now: t0.addingTimeInterval(0.2)), - .corner(.topRight) + .corner(.topRight, preservesRestorePoint: false) ) } @@ -47,7 +47,27 @@ final class WindowManagerKitTests: XCTestCase { let previous = WindowManagerKit.ChordState(action: .rightHalf, at: t0) XCTAssertEqual( WindowManagerKit.resolveChord(previous: previous, current: .bottomHalf, now: t0.addingTimeInterval(WindowManagerKit.cornerChordWindow - 0.02)), - .corner(.bottomRight) + .corner(.bottomRight, preservesRestorePoint: false) + ) + } + + /// The corner that completes a chord must be told the opening half already stored the undo, or + /// it overwrites it with the intermediate half-snapped frame and Restore stops undoing the + /// gesture the user actually made. + func testResolveChordCarriesTheOpeningHalfsRestorePoint() { + let t0 = Date(timeIntervalSinceReferenceDate: 1000) + let stored = WindowManagerKit.ChordState(action: .rightHalf, at: t0, storedRestorePoint: true) + XCTAssertEqual( + WindowManagerKit.resolveChord(previous: stored, current: .topHalf, now: t0.addingTimeInterval(0.1)), + .corner(.topRight, preservesRestorePoint: true) + ) + + // A half that stored nothing (it moved nothing, or its app refused it) leaves the corner to + // record the undo itself — there is no pre-gesture frame to protect. + let notStored = WindowManagerKit.ChordState(action: .rightHalf, at: t0, storedRestorePoint: false) + XCTAssertEqual( + WindowManagerKit.resolveChord(previous: notStored, current: .topHalf, now: t0.addingTimeInterval(0.1)), + .corner(.topRight, preservesRestorePoint: false) ) } diff --git a/Tests/DMonteCoreTests/WindowShortcutTests.swift b/Tests/DMonteCoreTests/WindowShortcutTests.swift index c5df792..b98e2c0 100644 --- a/Tests/DMonteCoreTests/WindowShortcutTests.swift +++ b/Tests/DMonteCoreTests/WindowShortcutTests.swift @@ -124,6 +124,61 @@ final class WindowShortcutTests: XCTestCase { XCTAssertTrue(store.storedShortcuts().isEmpty) } + /// The upgrade hazard: a user bound ⌃⌥⌫ to Center in 0.14.0, then Restore shipped with ⌃⌥⌫ as + /// its default. Both actions holding it means two Carbon registrations for one combination and + /// one keypress firing both, so the shipped default has to yield to the explicit binding. + func testANewDefaultYieldsToAPreExistingUserOverride() { + let store = WindowShortcutStore(defaults: scratchDefaults()) + guard let restoreDefault = WindowAction.restorePrevious.defaultShortcut else { + return XCTFail("restore should ship with a default") + } + store.save(restoreDefault, for: .center) + + let effective = store.effectiveShortcuts() + XCTAssertEqual(effective[.center], restoreDefault, "the user's own binding wins") + XCTAssertNil(effective[.restorePrevious], "the colliding default must not also register") + XCTAssertEqual(effective[.leftHalf], WindowAction.leftHalf.defaultShortcut, "unrelated defaults are untouched") + } + + /// Whichever default a future release adds, no combination may ever come back bound twice. + func testEffectiveShortcutsNeverBindOneCombinationTwice() { + for action in WindowAction.allCases { + guard let shortcut = action.defaultShortcut else { continue } + let store = WindowShortcutStore(defaults: scratchDefaults()) + store.save(shortcut, for: .topLeft) // .topLeft ships with no default of its own + + let effective = store.effectiveShortcuts() + XCTAssertEqual(effective[.topLeft], shortcut) + XCTAssertNil(effective[action], "\(action.rawValue) kept a default the user had claimed") + XCTAssertEqual( + Set(effective.values).count, + effective.count, + "two actions share a combination after overriding \(action.rawValue)'s default" + ) + } + } + + @MainActor + func testControllerDropsADefaultThatCollidesWithAStoredOverride() { + let defaults = scratchDefaults() + guard let restoreDefault = WindowAction.restorePrevious.defaultShortcut else { + return XCTFail("restore should ship with a default") + } + WindowShortcutStore(defaults: defaults).save(restoreDefault, for: .center) + + let controller = WindowManagerController(defaults: defaults) + defer { controller.unregisterHotKeys() } + + XCTAssertEqual(controller.shortcut(for: .center), restoreDefault) + XCTAssertNil(controller.shortcut(for: .restorePrevious)) + XCTAssertEqual(controller.actionUsing(restoreDefault), .center, "exactly one action answers for the combination") + XCTAssertEqual( + Set(controller.shortcuts.values).count, + controller.shortcuts.count, + "the registrar would claim the same combination twice" + ) + } + func testStoreIgnoresCorruptEntries() { let defaults = scratchDefaults() defaults.set(