Window Manager: move to the next display, and undo the last arrange - #27
Open
havokentity wants to merge 2 commits into
Open
Window Manager: move to the next display, and undo the last arrange#27havokentity wants to merge 2 commits into
havokentity wants to merge 2 commits into
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave 2 Phase A.
Implementation: Window Manager: move to the next display, and undo the last arrange
Review fixes: Window Manager: make the undo undo the gesture, not the last write
Built by an isolated agent, then adversarially reviewed by an independent agent that checked the branch out and ran
swift build+ the full suite itself. Every review finding was fixed, with a regression test added for each high and medium.🤖 Generated with Claude Code