diff --git a/Kanban/App/AppModel.swift b/Kanban/App/AppModel.swift index 104708b..4c7d719 100644 --- a/Kanban/App/AppModel.swift +++ b/Kanban/App/AppModel.swift @@ -189,6 +189,25 @@ public final class AppModel { /// reads the pasteboard once and collects every staged tree it no longer names. public let clipboard = ClipboardStore() + // MARK: The provider seam + + /// **The composition root for `HistoryProviding`** (12-editions.md ▸ The provider seam): what a + /// board session's undo stack is built by, called once per board as its session begins. + /// + /// Base binds the native stack — an `NSUndoManager` over inverse `WriteOperation`s + /// (13-native-undo.md) — and that is the default here because it is the *shared* code's + /// implementation: both targets compile it, and Pro runs it too until pro-m1 replaces this + /// closure with the git provider (06-history-undo.md). Nothing in this file is + /// edition-conditional; the edition difference is which closure the root installs. + /// + /// It takes the store because that is what a provider is a history *of*: the git provider needs + /// the board root it is a repository at, and the native one is about to need the same store's + /// snapshots to compute inverses from. A property rather than an initializer argument so a test + /// can bind a fake without a second `AppModel` initializer, `@ObservationIgnored` because + /// nothing renders from it. + @ObservationIgnored + public var makeHistoryProvider: (BoardStore) -> any HistoryProviding = { _ in NativeHistoryProvider() } + // MARK: Sessions /// One open board window and everything hanging off it. @@ -201,6 +220,22 @@ public final class AppModel { /// open-now flag without matching by identity a second time. public let recordID: UUID + /// This board's undo/redo substrate — **one stack per board session, never per window** + /// (13-native-undo.md ▸ Rules). It lives here for the store's reason exactly: the session is + /// what every window over this board shares, and "undo is board-local". + /// + /// Which implementation it is, is the edition's answer and nobody else's + /// (12-editions.md ▸ The provider seam) — see `AppModel.makeHistoryProvider`. + public let history: any HistoryProviding + + /// The same stack, wearing the face AppKit needs (`BoardUndoManager`): what this board's + /// windows hand back from `windowWillReturnUndoManager`, so the Edit menu's Undo/Redo rows + /// and the toolbar's pair resolve to *this* board through the ordinary responder chain. + /// + /// Built once with the session rather than per window, because a second adapter would be a + /// second answer to "what is this board's undo" — and card windows share this one. + let undoManager: BoardUndoManager + /// This board's open card windows. The close flush's step 1 reads it; the card hosts /// maintain it. Empty is the common case. public var cardRefs: Set = [] @@ -507,7 +542,18 @@ public final class AppModel { /// drop 02 forbids — it forbids a failure that was never surfaced disappearing, not one the user /// has since fixed. func beginSession(ref: BoardWindowRef, store: BoardStore, recordID: UUID, access: ScopedAccess?) { - sessions[ref] = BoardSession(store: store, recordID: recordID, cardRefs: [], access: access) + // The board's stack is born here, with the session that owns it, and dies in `tearDown` + // below — the whole of 13-native-undo.md's session-only persistence: "the stack lives with + // the board session and dies at close/quit ... standard macOS behavior". + let history = makeHistoryProvider(store) + sessions[ref] = BoardSession( + store: store, + recordID: recordID, + history: history, + undoManager: BoardUndoManager(history: history), + cardRefs: [], + access: access + ) clearLaunchFailures(naming: [ref.path, store.rootURL.path]) refreshRecents() } @@ -705,6 +751,12 @@ public final class AppModel { }, tearDown: { [weak self] in guard let self, let session = sessions.removeValue(forKey: ref) else { return } + // Session-only persistence, the other half of `beginSession` (13-native-undo.md + // ▸ Rules): "the stack ... dies at close/quit", so reopening the board starts empty. + // Cleared rather than merely dropped because the steps hold closures over the store + // this line is about to release, and a stack that outlived its board would be a + // retain cycle wearing an undo stack's clothes. + session.history.clear() storeRegistry.release(session.store) session.access?.stop() } diff --git a/Kanban/App/BoardWindowHost.swift b/Kanban/App/BoardWindowHost.swift index 1299916..1c5fe58 100644 --- a/Kanban/App/BoardWindowHost.swift +++ b/Kanban/App/BoardWindowHost.swift @@ -264,6 +264,13 @@ struct BoardWindowHost: View { } } + // This window's answer to "what does ⌘Z act on" (13-native-undo.md ▸ Rules; 06 ▸ Undo + // routing) — the *session's* stack, read afresh on every ask so a torn-down board answers + // nothing rather than a stack with no board behind it. The Edit menu's Undo/Redo rows and + // the toolbar's pair are nil-target `undo:`/`redo:`, so this one line is what lights them + // up: `NSWindow` validates and crosses them against exactly this manager. + windowController.boardUndoManager = { appModel.session(for: ref)?.undoManager } + // The window-title widget (03-board-ui.md § Board popover) — **board windows only**, which // is why it is installed here rather than in `WindowAccessor`: welcome, the bootstrap and // card windows share that machinery and have no board to describe. It goes in after the diff --git a/Kanban/App/CardWindowHost.swift b/Kanban/App/CardWindowHost.swift index db98084..8367625 100644 --- a/Kanban/App/CardWindowHost.swift +++ b/Kanban/App/CardWindowHost.swift @@ -523,6 +523,15 @@ struct CardWindowHost: View { CardToolbar.controller(body: bodyPresentation, rawSource: rawSource, attachments: attachments) ) + // **The board's stack, not one of this window's own** (13-native-undo.md ▸ Rules: "not + // per-window: every window over a board (board window, its card windows) shares the store + // and shares the stack"). Same closure shape as the board window's, and deliberately the + // same object: ⌘Z with a card window in front crosses the board step the user last made, + // wherever they made it. The card's *text* surfaces are untouched by this — the body editor + // and the raw-source editor each vend their own manager to the responder chain, which is + // what keeps typing undo out of the board's stack (06-history-undo.md ▸ Undo routing). + windowController.boardUndoManager = { appModel.session(for: ref.board)?.undoManager } + windowController.onAttach = { window in if let recordID, let saved = appModel.boardRegistry.cardWindowFrame(id: recordID, cardID: ref.cardIdentity) { diff --git a/Kanban/App/WindowAccessor.swift b/Kanban/App/WindowAccessor.swift index d9a3409..d759fd7 100644 --- a/Kanban/App/WindowAccessor.swift +++ b/Kanban/App/WindowAccessor.swift @@ -56,6 +56,26 @@ final class HostedWindowController: NSObject, NSWindowDelegate { /// window that has nothing to flush. var onCloseRequested: (() -> Void)? + /// This window's board undo stack, asked for afresh every time AppKit wants it — the board + /// window's and its card windows' shared answer (13-native-undo.md ▸ Rules: "one stack per + /// board, owned by the board session ... `window.undoManager` for board surfaces returns the + /// session's manager"). + /// + /// A closure rather than a stored manager for two reasons: the session does not exist yet when + /// the window attaches, and it stops existing at teardown while the window is still closing — + /// answering `nil` then is what keeps a torn-down board's stack from being reachable through a + /// window that outlived it by a run-loop turn. + /// + /// `nil` on every window that is not showing a board (welcome, the bootstrap, the template + /// chooser), which `BoardUndoRouting` reads as "the platform default". + var boardUndoManager: (() -> UndoManager?)? + + /// The text manager this window hands back while a field editor holds the keyboard, and the one + /// it hands back when there is no board — 06-history-undo.md ▸ Undo routing, via + /// `BoardUndoRouting`. Created on demand, per window, which is what AppKit itself would have + /// done for a window whose delegate answered nothing. + private lazy var textUndoManager = UndoManager() + /// Set by `closeAfterFlush()` so the re-entrant `windowShouldClose` lets the close through /// instead of starting a second flush. private var isFlushed = false @@ -189,6 +209,29 @@ final class HostedWindowController: NSObject, NSWindowDelegate { return false } + /// The window-level half of 06-history-undo.md ▸ Undo routing (see `BoardUndoRouting`, which + /// owns the rule and the reasoning): the board's stack when the keyboard is on the board, a + /// text manager of this window's own while a field editor has it. + /// + /// **Answered here rather than forwarded**, unlike the proxy's other selectors, on the one + /// condition that this window has a board: `responds(to:)` reports this method whatever the + /// previous delegate does, so a `nil` return would leave a window with *no* undo manager at all + /// rather than the one AppKit creates for a delegate that stays silent. A window with no board + /// still defers to SwiftUI's delegate if it has an opinion. + func windowWillReturnUndoManager(_ window: NSWindow) -> UndoManager? { + let board = boardUndoManager?() + if board == nil, let previousDelegate, + previousDelegate.responds(to: #selector(NSWindowDelegate.windowWillReturnUndoManager(_:))), + let inherited = previousDelegate.windowWillReturnUndoManager?(window) { + return inherited + } + return BoardUndoRouting.undoManager( + isTextEditing: BoardUndoRouting.isTextEditing(window.firstResponder), + board: board, + textFallback: textUndoManager + ) + } + func windowDidMove(_ notification: Notification) { reportFrame() previousDelegate?.windowDidMove?(notification) diff --git a/Kanban/History/BoardUndoManager.swift b/Kanban/History/BoardUndoManager.swift new file mode 100644 index 0000000..48ea3cd --- /dev/null +++ b/Kanban/History/BoardUndoManager.swift @@ -0,0 +1,133 @@ +import AppKit + +// MARK: - BoardUndoManager + +/// What AppKit is handed for a board's windows: an `UndoManager` that owns no stack and answers +/// every question from the session's `HistoryProviding`. +/// +/// ### Why an adapter exists at all +/// +/// The responder chain speaks one currency. `NSWindow` implements `undo:`/`redo:` and validates +/// those menu items itself, reading `canUndo`, `canRedo` and `undoMenuItemTitle` off whatever +/// `UndoManager` the window's delegate hands back (`windowWillReturnUndoManager(_:)`) — which is +/// exactly how the system's Edit ▸ Undo row and the toolbar's nil-target pair (`BoardToolbar`) light +/// up, disable and retitle with no code of the app's own. +/// +/// The seam, though, must not be an `NSUndoManager`: base's stack is one, Pro's is git +/// (12-editions.md ▸ The provider seam), and a protocol that vended one could only ever have had a +/// single implementation. So the substrate stays behind `HistoryProviding` and *this* object is the +/// translation — one per board session, over whichever provider that session was composed with. Pro +/// inherits the whole command surface (enablement, dynamic titles, ⌘Z, the toolbar pair) by binding +/// its provider and changing nothing here, which is what "a user moving between editions relearns +/// nothing" (12) has to mean in code. +/// +/// ### It deliberately keeps its inherited stack empty +/// +/// Every question is overridden, so anything that *did* register into this manager by accident (a +/// stray AppKit client finding it through the window) would be invisible rather than half-live: it +/// could never enable Undo, retitle it, or be crossed by ⌘Z. Nothing registers here today; the +/// overrides are what make that safe rather than lucky. +/// +/// `removeAllActions()` is deliberately **not** forwarded to `HistoryProviding.clear()`: AppKit +/// clears a window's undo manager on paths this app does not control, and a card window closing must +/// not empty the board's stack (13-native-undo.md ▸ Rules — the stack belongs to the *session*, and +/// its one clearing point is that session's teardown). +public final class BoardUndoManager: UndoManager { + + /// The substrate this manager is a face for. Strong: the session owns both, and the manager is + /// only ever reachable while the session that made it is alive. + private let history: any HistoryProviding + + public init(history: any HistoryProviding) { + self.history = history + super.init() + } + + // MARK: Enablement + + public override var canUndo: Bool { history.canUndo } + + public override var canRedo: Bool { history.canRedo } + + // MARK: Crossing + + public override func undo() { history.undo() } + + public override func redo() { history.redo() } + + // MARK: Titles + + /// `NSUndoManager`'s own vocabulary for "the phrase, without the verb" — `""` when there is + /// nothing to cross, which is what its menu-title composition expects. + public override var undoActionName: String { history.undoActionName ?? "" } + + public override var redoActionName: String { history.redoActionName ?? "" } + + /// "Undo Move 3 Cards" — composed and localized by the platform (`undoMenuTitle(forUndoActionName:)` + /// reads the `undo.strings` pattern), so the step vocabulary stays the bare phrase and this app + /// never spells the word "Undo" in a title. + /// + /// The trim is for the nameless case: the pattern is `Undo %@`, so an empty action name would + /// otherwise leave a trailing space where `NSUndoManager` itself answers a bare "Undo". + public override var undoMenuItemTitle: String { + undoMenuTitle(forUndoActionName: undoActionName).trimmingCharacters(in: .whitespaces) + } + + public override var redoMenuItemTitle: String { + redoMenuTitle(forUndoActionName: redoActionName).trimmingCharacters(in: .whitespaces) + } +} + +// MARK: - BoardUndoRouting + +/// Which undo a hosted window answers with, given what holds the keyboard — 06-history-undo.md +/// ▸ Undo routing, which 12 records as **edition-independent**: "focus decides text-undo vs +/// board-undo; only the substrate behind board-undo differs". +/// +/// ### Most of the rule is AppKit's, and is not here +/// +/// "While a text-editing surface is focused ... ⌘Z/⇧⌘Z are that editor's own text undo" is the +/// platform's own behaviour for a text view that vends a manager: `NSWindow.undo:` crosses the +/// *first responder's* `undoManager`, and both of this app's real editors vend their own through the +/// text-view delegate (`CardBodySurface`, `CardRawSourceView` — each with an `UndoManager` created +/// per editor, emptied when the session ends). Those never reach this file. +/// +/// ### What is here is the case AppKit gets wrong +/// +/// A **field editor** — the shared `NSTextView` that appears inside every focused `NSTextField` and +/// `NSSearchField` — vends no manager of its own to the window, so `undo:` falls through to the +/// window's, and a reflexive ⌘Z over a typo in the search field or the board-rename field would +/// cross a *board* step. 06 rules that out by name: "Control-class text fields route the same way +/// (settled): the search field ... and the popover's text fields ... own ⌘Z/⇧⌘Z as field-local text +/// undo while focused — a reflexive undo over a typo must never become a tree checkout." So the +/// window-level answer is the board's stack **only when no text-editing surface holds the +/// keyboard**, and a per-window text manager otherwise — which is also exactly what AppKit would +/// have created for such a window on its own, so nothing about typing in a field changes. +/// +/// Pure and free of windows on purpose: the decision is three lines that are invisible until they +/// are wrong, and the responder it reads is the only part a test cannot conjure. +public enum BoardUndoRouting { + + /// Whether `responder` is a text-editing surface — every `NSTextView`, field editors included, + /// which `NSText` is precisely the ancestor of. An `NSTextField` that has focus without editing + /// is *not* one: AppKit installs the field editor the moment editing begins, and until then the + /// board is what the keyboard is acting on. + public static func isTextEditing(_ responder: NSResponder?) -> Bool { + responder is NSText + } + + /// The manager a window's delegate should answer with. + /// + /// `board` is `nil` for every window that is not showing a board — welcome, the restore + /// bootstrap, the template chooser, and a board or card window whose session has already been + /// torn down. Those get the text manager too, which is the platform default they had before this + /// seam existed. + public static func undoManager( + isTextEditing: Bool, + board: UndoManager?, + textFallback: UndoManager + ) -> UndoManager { + guard !isTextEditing, let board else { return textFallback } + return board + } +} diff --git a/Kanban/History/HistoryProviding.swift b/Kanban/History/HistoryProviding.swift new file mode 100644 index 0000000..bdfc616 --- /dev/null +++ b/Kanban/History/HistoryProviding.swift @@ -0,0 +1,143 @@ +import Foundation + +// MARK: - HistoryStepOutcome + +/// What happened when a step was asked to walk its write back (or forward again). +/// +/// The second case is 13-native-undo.md ▸ Rules ▸ **staleness validation**, in the vocabulary the +/// provider needs it in: "target folder gone, or the field no longer holding the step's after-value +/// → the step is **skipped, not applied**: popped from the stack ... and ⌘Z falls through to the +/// next step". The provider reads this answer and nothing else — the *predicate* (field-level, +/// settled) and the info-tone banner that explains a skip both belong to the step, which is the only +/// side that knows what it wrote and which board to say it on. +public enum HistoryStepOutcome: Equatable, Sendable { + + /// The step ran. Its mirror image joins the opposite stack. + case applied + + /// The step declined: the board no longer holds the value this step's write left, so applying + /// its inverse would clobber somebody else's newer edit. Nothing ran, the step is dropped, and + /// the crossing continues with the next one down. + case skipped +} + +// MARK: - HistoryStep + +/// One undoable step: a name, and the pair of actions that walk the board backwards and forwards +/// across it. +/// +/// ### An operation pair, not an `NSUndoManager` registration +/// +/// 13-native-undo.md ▸ Rules puts every undoable change at the Writer boundary — "each Writer call +/// site registers the inverse operation, computed from the pre-write snapshot the store already +/// holds: move → move back ...; rename → restore title" — and both halves of that write are already +/// in the caller's hands: the before-value *is* the inverse, and the after-value is what the write +/// set (which is also what the staleness predicate compares). A step is therefore that pair, in the +/// design's own terms, and deliberately says nothing about how a stack stores it: base's stack is +/// `NSUndoManager`-backed and Pro's is git (12-editions.md ▸ The provider seam), and neither +/// substrate appears here. +/// +/// ### `name` is the 06 vocabulary, unprefixed +/// +/// "The 06 vocabulary supplies menu titles ('Undo Move 3 Cards'), via NSUndoManager's dynamic +/// retitling — the same naming machinery both editions use" (13). What the step carries is the bare +/// phrase — `"Move Card"`, `"Move 3 Cards"`, `"Rename Lane"` — in the vocabulary of +/// 06-history-undo.md ▸ Commit messages, plural-folded by the same rule ("one gesture, one undo +/// step — a multi-card move is one step with a plural title"). The **"Undo "/"Redo " prefix is +/// never part of it**: the platform composes and localizes that (`BoardUndoManager`), and a step +/// that spelled it would read "Undo Undo Move Card" in the Edit menu. +/// +/// ### Both closures are `@MainActor` +/// +/// Everything they touch — the store, the snapshot, the banners — is, and a step exists to be run +/// from a menu command. Marking them says so at the seam instead of leaving each provider to +/// rediscover it. +public struct HistoryStep { + + /// The menu phrase, unprefixed — see the type's note. + public let name: String + + /// Walks the board back across this step. Registered at the Writer boundary as the *inverse* of + /// the write that just landed. + public let undo: @MainActor () -> HistoryStepOutcome + + /// Walks it forward again — the original write, replayed. Reached only after `undo` applied, + /// because that is the only way a step reaches the redo stack. + public let redo: @MainActor () -> HistoryStepOutcome + + public init( + name: String, + undo: @escaping @MainActor () -> HistoryStepOutcome, + redo: @escaping @MainActor () -> HistoryStepOutcome + ) { + self.name = name + self.undo = undo + self.redo = redo + } + + /// The same step read backwards — what a provider puts on the opposite stack once this one has + /// applied. The name does not change, which is the whole of "Undo Move Card" becoming "Redo Move + /// Card": the phrase names the *gesture*, not the direction. + public var reversed: HistoryStep { + HistoryStep(name: name, undo: redo, redo: undo) + } +} + +// MARK: - HistoryProviding + +/// The undo/redo substrate, behind one protocol boundary (12-editions.md ▸ The provider seam). +/// +/// ### One per board session +/// +/// "One stack per board, owned by the board session. Not per-window: every window over a board +/// (board window, its card windows) shares the store and shares the stack" (13-native-undo.md +/// ▸ Rules). `AppModel.BoardSession` is where that ownership lives, and the composition root binds +/// which implementation it gets: base binds `NativeHistoryProvider` (an `NSUndoManager` stack over +/// inverse `WriteOperation`s), Pro binds the git provider in pro-m1 (undo as forward restore commits +/// over HEAD's first-parent ancestry — 06-history-undo.md), Teams inherits Pro's. +/// +/// ### What this protocol deliberately does not say +/// +/// - **No `NSUndoManager`, anywhere in the signature.** It is base's implementation detail, and a +/// seam that vended one would be a seam only one provider could ever satisfy — the opposite of +/// the reason the split exists at all ("base's native undo is the first proof the seam is real", +/// 12). AppKit still needs an `UndoManager` to hand the responder chain; that adapter is +/// `BoardUndoManager`, which sits *over* this protocol rather than inside it. +/// - **No persistence promise.** Base's stack dies with the session (13); Pro's survives relaunch +/// because git does (06). Both are honest implementations of these seven members. +/// - **No routing.** Which surface ⌘Z reaches is focus's answer, not the substrate's +/// (06 ▸ Undo routing, edition-independent) — `BoardUndoRouting`. +@MainActor +public protocol HistoryProviding: AnyObject { + + /// Records one undoable step, on top of the undo stack, clearing the redo stack — the classic + /// rule, and the one every substrate shares. + /// + /// Called once per *gesture*, never once per write: "coalescing follows commit granularity ... + /// a multi-card move is one step with a plural title; an Edit session is one step, registered at + /// the Edit→Preview flip" (13 ▸ Rules). + func register(_ step: HistoryStep) + + /// Whether there is a step to cross. What the Edit menu's Undo row and the toolbar's Undo item + /// enable on, through the same responder-chain answer. + var canUndo: Bool { get } + + var canRedo: Bool { get } + + /// The name of the step ⌘Z would cross, or `nil` when there is none — the phrase the menu title + /// is composed from ("Move 3 Cards" → "Undo Move 3 Cards"). + var undoActionName: String? { get } + + var redoActionName: String? { get } + + /// Crosses one step backwards. A stale step is skipped rather than applied, and the crossing + /// falls through to the next one (13 ▸ Rules ▸ staleness validation); an empty stack does + /// nothing. + func undo() + + func redo() + + /// Drops every step in both directions — session-only persistence (13 ▸ Rules), run at the board + /// session's teardown. Also what a substrate that must re-seed (a branch switch, 06) calls first. + func clear() +} diff --git a/Kanban/History/NativeHistoryProvider.swift b/Kanban/History/NativeHistoryProvider.swift new file mode 100644 index 0000000..6a7f27a --- /dev/null +++ b/Kanban/History/NativeHistoryProvider.swift @@ -0,0 +1,127 @@ +import Foundation + +// MARK: - NativeHistoryProvider + +/// The base edition's undo substrate: one `NSUndoManager`-backed stack per board session +/// (13-native-undo.md). +/// +/// ### Why `NSUndoManager` at all, when the steps are ours +/// +/// Because the *command surface* is the platform's. Edit ▸ Undo and Edit ▸ Redo are the system's own +/// nil-target `undo:`/`redo:` rows (11-command-nexus.md), the toolbar pair carries the same actions +/// (`BoardToolbar`), and both light up, disable and **retitle** from whatever `UndoManager` the +/// focused window hands back — "Undo Move 3 Cards" is `NSUndoManager`'s dynamic retitling, which 13 +/// names as the mechanism and 12 records as the one both editions share. Reimplementing the stack +/// over two arrays would mean reimplementing that, badly. +/// +/// It stays *behind* `HistoryProviding` regardless: nothing outside this file learns that base's +/// stack is an `NSUndoManager`, which is what lets pro-m1 bind git to the same seam +/// (12 ▸ The provider seam). +/// +/// ### One `register` call is exactly one step +/// +/// `groupsByEvent` is turned **off** and every registration is wrapped in its own group. The default +/// (on) closes a group at the end of the run-loop turn, which would silently fold two gestures that +/// happened to land in one event into a single ⌘Z — the opposite of 13's "one gesture, one undo +/// step", and the coalescing decision belongs to the Writer call site (a multi-card move registers +/// *one* step with a plural title), never to the run loop's timing. +/// +/// ### Undo flips to redo by re-registering +/// +/// A step that applies pushes its own mirror image back onto the manager from inside the undo — and +/// `NSUndoManager` routes a registration made while it is undoing onto the **redo** stack, and vice +/// versa. That single rule gives the whole classic dance (undo → redo → undo …) with no second stack +/// of our own, and it is why a *skipped* step leaves nothing behind: it registers nothing, so the +/// empty group is discarded and the step is simply gone (13: "popped from the stack ... and ⌘Z falls +/// through to the next step"). +@MainActor +public final class NativeHistoryProvider: HistoryProviding { + + /// The stack. `private` and never vended: see the type's note. + private let manager = UndoManager() + + /// Set by the step just crossed when it declined to apply, read by the crossing loop below. + /// A flag rather than a return value because the manager, not this object, calls the step. + private var lastCrossingSkipped = false + + public init() { + // See "One `register` call is exactly one step", above. + manager.groupsByEvent = false + } + + // MARK: - HistoryProviding + + public var canUndo: Bool { manager.canUndo } + + public var canRedo: Bool { manager.canRedo } + + /// `NSUndoManager` answers `""` — not `nil` — for a stack with nothing on it *and* for a step + /// registered without a name, so the emptiness check is the honest one. + public var undoActionName: String? { + guard canUndo, !manager.undoActionName.isEmpty else { return nil } + return manager.undoActionName + } + + public var redoActionName: String? { + guard canRedo, !manager.redoActionName.isEmpty else { return nil } + return manager.redoActionName + } + + public func register(_ step: HistoryStep) { + push(step) + } + + public func undo() { + cross(manager.undo, while: { self.manager.canUndo }) + } + + public func redo() { + cross(manager.redo, while: { self.manager.canRedo }) + } + + public func clear() { + manager.removeAllActions() + } + + // MARK: - The stack + + /// Registers `step` as one group of its own, named for the menu. + /// + /// The target is `self` and the payload rides in the closure, which is `NSUndoManager`'s + /// block-based form: the manager references its target **unowned** (its own documented rule), so + /// the provider owning the manager that references the provider is not a cycle. + private func push(_ step: HistoryStep) { + manager.beginUndoGrouping() + manager.registerUndo(withTarget: self) { provider in + provider.apply(step) + } + // Inside the group, deliberately: the name belongs to the group being closed, and on the + // way back it is what retitles Redo. + manager.setActionName(step.name) + manager.endUndoGrouping() + } + + /// Runs one step's action and records what happened. + private func apply(_ step: HistoryStep) { + switch step.undo() { + case .applied: + // Lands on the opposite stack — see "Undo flips to redo by re-registering". + push(step.reversed) + case .skipped: + lastCrossingSkipped = true + } + } + + /// Crosses one step, and keeps going while the steps it crosses decline to apply — 13's + /// fall-through: "the step is skipped, not applied ... and ⌘Z falls through to the next step". + /// + /// The `while` guard is the manager's own emptiness, so a stack of nothing but stale steps + /// empties itself and stops rather than spinning. + private func cross(_ step: () -> Void, while more: () -> Bool) { + repeat { + guard more() else { return } + lastCrossingSkipped = false + step() + } while lastCrossingSkipped + } +} diff --git a/KanbanTests/HistoryProviderTests.swift b/KanbanTests/HistoryProviderTests.swift new file mode 100644 index 0000000..2018770 --- /dev/null +++ b/KanbanTests/HistoryProviderTests.swift @@ -0,0 +1,496 @@ +import AppKit +import Foundation +import Testing +@testable import Kanban + +/// The provider seam and the board stack behind it (12-editions.md ▸ The provider seam; +/// 13-native-undo.md). +/// +/// The steps here are **synthetic** on purpose: what this milestone builds is the stack and the +/// seam, and the real inverses arrive at the Writer boundary in the next one. A step that records +/// its own crossing is therefore the exact fixture — it proves the stack's grammar (one gesture one +/// step, undo flips to redo, a stale step falls through) without needing a board to move cards +/// around on. + +// MARK: - Synthetic steps + +/// Records every crossing, in order, and answers with whatever outcome the test asked for. +@MainActor +private final class StepLog { + + private(set) var crossings: [String] = [] + + /// A step that applies in both directions — the ordinary case. + func step(_ name: String) -> HistoryStep { + step(name, undo: .applied, redo: .applied) + } + + /// A step whose inverse declines — 13's staleness skip, without needing a foreign writer. + func staleStep(_ name: String) -> HistoryStep { + step(name, undo: .skipped, redo: .applied) + } + + func step(_ name: String, undo: HistoryStepOutcome, redo: HistoryStepOutcome) -> HistoryStep { + HistoryStep( + name: name, + undo: { [weak self] in + self?.crossings.append("undo \(name)") + return undo + }, + redo: { [weak self] in + self?.crossings.append("redo \(name)") + return redo + } + ) + } +} + +// MARK: - The native stack + +@MainActor +@Suite("History ▸ the native stack") +struct NativeHistoryProviderTests { + + @Test("A fresh provider has nothing to cross and nothing to say about it") + func emptyStack() { + let provider = NativeHistoryProvider() + #expect(provider.canUndo == false) + #expect(provider.canRedo == false) + #expect(provider.undoActionName == nil) + #expect(provider.redoActionName == nil) + } + + @Test("Registering a step arms Undo and names it — and runs nothing") + func registerArmsUndo() { + let log = StepLog() + let provider = NativeHistoryProvider() + + provider.register(log.step("Move Card")) + + #expect(provider.canUndo) + #expect(provider.canRedo == false) + #expect(provider.undoActionName == "Move Card") + #expect(provider.redoActionName == nil) + #expect(log.crossings.isEmpty, "registration is not application") + } + + @Test("Undo runs the inverse once and flips the step onto Redo, keeping its name") + func undoRunsTheInverseAndFlipsToRedo() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.step("Move 3 Cards")) + + provider.undo() + + #expect(log.crossings == ["undo Move 3 Cards"]) + #expect(provider.canUndo == false) + #expect(provider.canRedo) + // The phrase names the gesture, not the direction — "Undo Move 3 Cards" becomes + // "Redo Move 3 Cards". + #expect(provider.redoActionName == "Move 3 Cards") + #expect(provider.undoActionName == nil) + } + + @Test("Redo replays the write and arms Undo again — the classic dance, both ways") + func redoReplaysAndFlipsBack() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.step("Rename Lane")) + + provider.undo() + provider.redo() + + #expect(log.crossings == ["undo Rename Lane", "redo Rename Lane"]) + #expect(provider.canUndo) + #expect(provider.canRedo == false) + #expect(provider.undoActionName == "Rename Lane") + + provider.undo() + #expect(log.crossings == ["undo Rename Lane", "redo Rename Lane", "undo Rename Lane"]) + } + + @Test("One register call is one step — two registrations are two crossings, newest first") + func oneRegistrationIsOneStep() { + let log = StepLog() + let provider = NativeHistoryProvider() + // Back to back, in one turn of the run loop: `groupsByEvent` must not fold these into one. + provider.register(log.step("Move Card")) + provider.register(log.step("Rename Card")) + + #expect(provider.undoActionName == "Rename Card") + provider.undo() + #expect(log.crossings == ["undo Rename Card"]) + #expect(provider.undoActionName == "Move Card") + + provider.undo() + #expect(log.crossings == ["undo Rename Card", "undo Move Card"]) + #expect(provider.canUndo == false) + } + + @Test("A new step clears the redo stack — classic behaviour") + func registeringClearsRedo() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.step("Move Card")) + provider.undo() + #expect(provider.canRedo) + + provider.register(log.step("Delete Card")) + + #expect(provider.canRedo == false) + #expect(provider.redoActionName == nil) + #expect(provider.undoActionName == "Delete Card") + } + + @Test("Undo on an empty stack does nothing at all") + func undoOnAnEmptyStackIsInert() { + let log = StepLog() + let provider = NativeHistoryProvider() + + provider.undo() + provider.redo() + + #expect(log.crossings.isEmpty) + #expect(provider.canUndo == false) + #expect(provider.canRedo == false) + } + + @Test("A stale step is skipped, not applied — and ⌘Z falls through to the next one") + func aStaleStepIsSkippedAndFallsThrough() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.step("Move Card")) + provider.register(log.staleStep("Rename Card")) + + provider.undo() + + // Both were reached in one ⌘Z: the stale one declined and was dropped, the next one applied. + #expect(log.crossings == ["undo Rename Card", "undo Move Card"]) + #expect(provider.canUndo == false) + // Only the step that actually ran is redoable — a skipped step leaves nothing behind. + #expect(provider.canRedo) + #expect(provider.redoActionName == "Move Card") + } + + @Test("A stack of nothing but stale steps empties itself and stops") + func anEntirelyStaleStackEmptiesItself() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.staleStep("Move Card")) + provider.register(log.staleStep("Rename Card")) + + provider.undo() + + #expect(log.crossings == ["undo Rename Card", "undo Move Card"]) + #expect(provider.canUndo == false) + #expect(provider.canRedo == false) + } + + @Test("Clearing drops both directions — the session-only rule's one call") + func clearEmptiesBothStacks() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.step("Move Card")) + provider.undo() + provider.register(log.step("Delete Card")) + + provider.clear() + + #expect(provider.canUndo == false) + #expect(provider.canRedo == false) + #expect(provider.undoActionName == nil) + #expect(provider.redoActionName == nil) + provider.undo() + #expect(log.crossings == ["undo Move Card"], "nothing crossed after the clear") + } + + @Test("Two providers are two stacks — undo is board-local by construction") + func providersAreIndependent() { + let log = StepLog() + let one = NativeHistoryProvider() + let other = NativeHistoryProvider() + + one.register(log.step("Move Card")) + + #expect(one.canUndo) + #expect(other.canUndo == false) + + other.undo() + #expect(log.crossings.isEmpty, "the other board's ⌘Z crosses nothing of this board's") + #expect(one.canUndo) + } +} + +// MARK: - The AppKit adapter + +/// A provider with no `NSUndoManager` anywhere in it — which is the point: `BoardUndoManager` is +/// tested against *this* rather than against the native stack, because what has to be true is that +/// the adapter works for any implementation of the seam (Pro's git provider binds the same protocol +/// in pro-m1). +@MainActor +private final class FakeHistoryProvider: HistoryProviding { + + var canUndo = false + var canRedo = false + var undoActionName: String? + var redoActionName: String? + private(set) var registered: [String] = [] + private(set) var undoCount = 0 + private(set) var redoCount = 0 + private(set) var clearCount = 0 + + func register(_ step: HistoryStep) { registered.append(step.name) } + func undo() { undoCount += 1 } + func redo() { redoCount += 1 } + func clear() { clearCount += 1 } +} + +@MainActor +@Suite("History ▸ the AppKit adapter") +struct BoardUndoManagerTests { + + @Test("Enablement is the provider's answer, not a stack of the adapter's own") + func enablementMirrorsTheProvider() { + let provider = FakeHistoryProvider() + let manager = BoardUndoManager(history: provider) + + #expect(manager.canUndo == false) + #expect(manager.canRedo == false) + + provider.canUndo = true + provider.canRedo = true + + #expect(manager.canUndo) + #expect(manager.canRedo) + } + + @Test("The menu titles are the platform's composition over the step's own phrase") + func menuTitlesComposeFromTheStepName() { + let provider = FakeHistoryProvider() + let manager = BoardUndoManager(history: provider) + + // Nothing to cross: the bare verb, with no trailing space where the name would go. + #expect(manager.undoMenuItemTitle == "Undo") + #expect(manager.redoMenuItemTitle == "Redo") + + provider.canUndo = true + provider.undoActionName = "Move 3 Cards" + provider.canRedo = true + provider.redoActionName = "Rename Lane" + + #expect(manager.undoActionName == "Move 3 Cards") + #expect(manager.undoMenuItemTitle == "Undo Move 3 Cards") + #expect(manager.redoMenuItemTitle == "Redo Rename Lane") + } + + @Test("Crossing forwards to the provider — what ⌘Z and the toolbar item actually reach") + func crossingForwards() { + let provider = FakeHistoryProvider() + let manager = BoardUndoManager(history: provider) + + manager.undo() + manager.undo() + manager.redo() + + #expect(provider.undoCount == 2) + #expect(provider.redoCount == 1) + } + + @Test("A stray registration into the adapter can never be crossed or shown") + func theAdaptersOwnStackStaysInert() { + let provider = FakeHistoryProvider() + let manager = BoardUndoManager(history: provider) + let sink = FakeHistoryProvider() + + manager.registerUndo(withTarget: sink) { $0.canUndo = true } + + #expect(manager.canUndo == false, "the provider is the only source of truth") + #expect(manager.undoMenuItemTitle == "Undo") + manager.undo() + #expect(sink.canUndo == false, "the stray action was never run") + #expect(provider.undoCount == 1) + } + + @Test("Clearing the adapter's own stack leaves the board's alone") + func removeAllActionsDoesNotClearTheBoard() { + let provider = FakeHistoryProvider() + let manager = BoardUndoManager(history: provider) + provider.canUndo = true + + manager.removeAllActions() + + #expect(provider.clearCount == 0) + #expect(manager.canUndo) + } +} + +// MARK: - Routing + +@MainActor +@Suite("History ▸ undo routing") +struct BoardUndoRoutingTests { + + @Test("A text view — field editors included — is a text-editing surface; a plain view is not") + func textEditingClassification() { + #expect(BoardUndoRouting.isTextEditing(NSTextView())) + #expect(BoardUndoRouting.isTextEditing(NSTextField()) == false, "focused, not yet editing") + #expect(BoardUndoRouting.isTextEditing(NSView()) == false) + #expect(BoardUndoRouting.isTextEditing(NSWindow()) == false) + #expect(BoardUndoRouting.isTextEditing(nil) == false) + } + + @Test("With focus outside every text surface, a board window answers with the board's stack") + func boardFocusAnswersTheBoardStack() { + let board = BoardUndoManager(history: FakeHistoryProvider()) + let fallback = UndoManager() + + let answer = BoardUndoRouting.undoManager(isTextEditing: false, board: board, textFallback: fallback) + + #expect(answer === board) + } + + @Test("A focused field editor answers with the window's text manager, never the board's") + func fieldEditorFocusAnswersTheTextManager() { + let board = BoardUndoManager(history: FakeHistoryProvider()) + let fallback = UndoManager() + + let answer = BoardUndoRouting.undoManager(isTextEditing: true, board: board, textFallback: fallback) + + #expect(answer === fallback) + } + + @Test("A window with no board answers with the text manager either way") + func windowsWithNoBoardFallBack() { + let fallback = UndoManager() + + #expect(BoardUndoRouting.undoManager(isTextEditing: false, board: nil, textFallback: fallback) === fallback) + #expect(BoardUndoRouting.undoManager(isTextEditing: true, board: nil, textFallback: fallback) === fallback) + } +} + +// MARK: - The session that owns the stack + +/// A board on disk, and an `AppModel` whose registry file is in temp rather than in the test host's +/// real Application Support directory — `AppModelTests`' two helpers, in the shape this suite needs. +@MainActor +private func makeBoard() throws -> WriterFixture { + let fixture = try WriterFixture() + try fixture.item("", Item.board) + try fixture.item(Ident.lane1, Item.rich(order: "1024", title: "Todo")) + return fixture +} + +@MainActor +private func makeModel() throws -> (model: AppModel, tearDown: () -> Void) { + let folder = FileManager.default.temporaryDirectory + .appendingPathComponent("HistoryProviderTests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: folder, withIntermediateDirectories: true) + let model = AppModel(registryStorageURL: folder.appendingPathComponent("board-registry.json")) + return (model, { try? FileManager.default.removeItem(at: folder) }) +} + +@MainActor +@discardableResult +private func openBoard(_ model: AppModel, at url: URL) throws -> BoardWindowRef { + let ref = BoardWindowRef(url: url) + let recordID = model.boardRegistry.recordOpen(of: url) + let store = try model.storeRegistry.acquire(url) + model.boardRegistry.setOpenNow(id: recordID) + model.beginSession(ref: ref, store: store, recordID: recordID, access: nil) + return ref +} + +@MainActor +@Suite("History ▸ the board session's stack") +struct BoardSessionHistoryTests { + + @Test("A session is born with a stack, and its adapter is a face for that same stack") + func aSessionOwnsOneStack() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (model, tearDown) = try makeModel() + defer { tearDown() } + + let ref = try openBoard(model, at: fixture.root) + let session = try #require(model.session(for: ref)) + let log = StepLog() + + #expect(session.undoManager.canUndo == false) + session.history.register(log.step("Move Card")) + + // The window hands AppKit the adapter; the adapter is answering from the session's provider. + #expect(session.undoManager.canUndo) + #expect(session.undoManager.undoMenuItemTitle == "Undo Move Card") + + session.undoManager.undo() + #expect(log.crossings == ["undo Move Card"]) + #expect(session.undoManager.canRedo) + } + + @Test("Two open boards are two stacks — never one another's") + func sessionsAreIsolated() throws { + let first = try makeBoard() + defer { first.tearDown() } + let second = try makeBoard() + defer { second.tearDown() } + let (model, tearDown) = try makeModel() + defer { tearDown() } + + let firstRef = try openBoard(model, at: first.root) + let secondRef = try openBoard(model, at: second.root) + let log = StepLog() + + let firstSession = try #require(model.session(for: firstRef)) + let secondSession = try #require(model.session(for: secondRef)) + #expect(firstSession.history !== secondSession.history) + + firstSession.history.register(log.step("Move Card")) + + #expect(firstSession.history.canUndo) + #expect(secondSession.history.canUndo == false) + + secondSession.undoManager.undo() + #expect(log.crossings.isEmpty) + #expect(firstSession.history.canUndo, "the other board's ⌘Z left this one's stack alone") + } + + @Test("Closing a board empties its stack — session-only persistence") + func closingClearsTheStack() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (model, tearDown) = try makeModel() + defer { tearDown() } + + let ref = try openBoard(model, at: fixture.root) + let session = try #require(model.session(for: ref)) + let history = session.history + let log = StepLog() + history.register(log.step("Move Card")) + #expect(history.canUndo) + + await model.closeBoard(ref: ref, cause: .userClose) + + #expect(model.session(for: ref) == nil) + #expect(history.canUndo == false, "reopening the board starts empty") + #expect(history.canRedo == false) + #expect(log.crossings.isEmpty) + } + + @Test("The composition root decides which provider a session gets") + func theProviderIsBoundAtComposition() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (model, tearDown) = try makeModel() + defer { tearDown() } + + let bound = FakeHistoryProvider() + model.makeHistoryProvider = { _ in bound } + + let ref = try openBoard(model, at: fixture.root) + let session = try #require(model.session(for: ref)) + + #expect(session.history === bound) + session.undoManager.undo() + #expect(bound.undoCount == 1, "the window's manager reaches whatever the root bound") + } +}