diff --git a/UndoRedo.Test/UndoRedoStackTests.cs b/UndoRedo.Test/UndoRedoStackTests.cs index e70b1bc..56c80b0 100644 --- a/UndoRedo.Test/UndoRedoStackTests.cs +++ b/UndoRedo.Test/UndoRedoStackTests.cs @@ -812,6 +812,59 @@ await Assert.ThrowsExactlyAsync(() => Assert.IsTrue(stack.CanRedo, "C was undone, so it must be redoable"); } + [TestMethod] + public async Task UndoToSaveBoundary_BoundaryHeldAcrossTrim_UndoesToSavedState() + { + // Arrange + UndoRedoService stack = new(new StackManager(), new SaveBoundaryManager(), new CommandMerger(), UndoRedoOptions.Create(maxStackSize: 3)); + int value = 0; + SaveBoundary? heldBoundary = null; + stack.SaveBoundaryCreated += (_, e) => heldBoundary = e.SaveBoundary; + + stack.Execute(new DelegateCommand("A", () => value++, () => value--)); + stack.MarkAsSaved(); + stack.Execute(new DelegateCommand("B", () => value++, () => value--)); + stack.Execute(new DelegateCommand("C", () => value++, () => value--)); + stack.Execute(new DelegateCommand("D", () => value++, () => value--)); // Trims A, moving the save point to -1 + Assert.IsNotNull(heldBoundary); + Assert.AreEqual(3, stack.GetCommandsToUndo(heldBoundary).Count(), "The held boundary should resolve to the save point's current position"); + + // Act + bool result = await stack.UndoToSaveBoundaryAsync(heldBoundary, navigateToLastChange: false).ConfigureAwait(false); + + // Assert + Assert.IsTrue(result, "UndoToSaveBoundary should resolve a boundary held across a trim"); + Assert.AreEqual(1, value, "The value should be back at the saved state"); + Assert.AreEqual(-1, stack.CurrentPosition); + Assert.IsFalse(stack.HasUnsavedChanges, "The stack should be at the save point"); + } + + [TestMethod] + public async Task UndoToSaveBoundary_BoundaryRemovedByBranching_ReturnsFalse() + { + // Arrange + UndoRedoService stack = CreateService(); + int value = 0; + stack.Execute(new DelegateCommand("A", () => value++, () => value--)); + stack.Execute(new DelegateCommand("B", () => value++, () => value--)); + stack.MarkAsSaved(); + SaveBoundary removedBoundary = stack.SaveBoundaries[0]; + await stack.UndoAsync(navigateToChange: false).ConfigureAwait(false); + await stack.UndoAsync(navigateToChange: false).ConfigureAwait(false); + stack.Execute(new DelegateCommand("C", () => value++, () => value--)); // Branches, discarding the save point + stack.Execute(new DelegateCommand("D", () => value++, () => value--)); + stack.Execute(new DelegateCommand("E", () => value++, () => value--)); + + // Act + bool result = await stack.UndoToSaveBoundaryAsync(removedBoundary, navigateToLastChange: false).ConfigureAwait(false); + + // Assert + Assert.IsFalse(result, "UndoToSaveBoundary should reject a boundary that no longer exists"); + Assert.AreEqual(3, value, "Nothing should have been undone"); + Assert.AreEqual(2, stack.CurrentPosition); + Assert.IsEmpty(stack.GetCommandsToUndo(removedBoundary)); + } + [TestMethod] public async Task UndoToSaveBoundary_WhenAlreadyAtPosition_ReturnsFalse() { diff --git a/UndoRedo/Models/SaveBoundary.cs b/UndoRedo/Models/SaveBoundary.cs index 992e086..4fd566a 100644 --- a/UndoRedo/Models/SaveBoundary.cs +++ b/UndoRedo/Models/SaveBoundary.cs @@ -9,6 +9,17 @@ namespace ktsu.UndoRedo; /// Optional description public sealed class SaveBoundary(int position, string? description = null) { + /// + /// Creates a copy of at a new position that is still the same save point, + /// so a caller holding the original can have it resolved to where the save point is now + /// + internal SaveBoundary(SaveBoundary original, int position) + : this(position, original.Description) + { + Identity = original.Identity; + Timestamp = original.Timestamp; + } + /// /// The position in the stack where this save boundary was created /// @@ -23,4 +34,14 @@ public sealed class SaveBoundary(int position, string? description = null) /// Optional description of what was saved /// public string? Description { get; } = description; + + /// + /// Shared by every copy of one save point as its position is adjusted + /// + internal object Identity { get; } = new(); + + /// + /// Whether this boundary and describe the same save point + /// + internal bool IsSameSavePointAs(SaveBoundary other) => ReferenceEquals(Identity, other.Identity); } diff --git a/UndoRedo/Services/SaveBoundaryManager.cs b/UndoRedo/Services/SaveBoundaryManager.cs index e6e4bd1..3d92029 100644 --- a/UndoRedo/Services/SaveBoundaryManager.cs +++ b/UndoRedo/Services/SaveBoundaryManager.cs @@ -81,8 +81,9 @@ public void AdjustPositions(int adjustment) } else { - // Create a new boundary with adjusted position - _saveBoundaries[i] = new SaveBoundary(newPosition, boundary.Description); + // Create a new boundary with adjusted position that is still the same save point, so a + // boundary a caller already holds can be resolved to it + _saveBoundaries[i] = new SaveBoundary(boundary, newPosition); } } } diff --git a/UndoRedo/Services/UndoRedoService.cs b/UndoRedo/Services/UndoRedoService.cs index 152b1c2..660ff30 100644 --- a/UndoRedo/Services/UndoRedoService.cs +++ b/UndoRedo/Services/UndoRedoService.cs @@ -217,14 +217,31 @@ public void Clear() } /// - public IEnumerable GetCommandsToUndo(SaveBoundary saveBoundary) => - _saveBoundaryManager.GetCommandsToUndo(saveBoundary, _stackManager.CurrentPosition, _stackManager.Commands); + public IEnumerable GetCommandsToUndo(SaveBoundary saveBoundary) + { + Ensure.NotNull(saveBoundary); + + SaveBoundary? liveBoundary = FindLiveSaveBoundary(saveBoundary); + return liveBoundary == null + ? [] + : _saveBoundaryManager.GetCommandsToUndo(liveBoundary, _stackManager.CurrentPosition, _stackManager.Commands); + } /// public async Task UndoToSaveBoundaryAsync(SaveBoundary saveBoundary, bool navigateToLastChange = true, CancellationToken cancellationToken = default) { Ensure.NotNull(saveBoundary); + // A boundary the caller has held since before the stack was trimmed carries a stale position, + // and one removed by branching or clearing no longer marks a reachable saved state + SaveBoundary? liveBoundary = FindLiveSaveBoundary(saveBoundary); + if (liveBoundary == null) + { + return false; + } + + saveBoundary = liveBoundary; + if (_stackManager.CurrentPosition <= saveBoundary.Position) { return false; @@ -254,6 +271,13 @@ public async Task UndoToSaveBoundaryAsync(SaveBoundary saveBoundary, bool return true; } + /// + /// Resolves a save boundary, which may have been obtained before the stack was trimmed, to the live + /// boundary for the same save point, or null if that save point no longer exists + /// + private SaveBoundary? FindLiveSaveBoundary(SaveBoundary saveBoundary) => + _saveBoundaryManager.SaveBoundaries.FirstOrDefault(boundary => boundary.IsSameSavePointAs(saveBoundary)); + /// /// Navigates to where a change was made, after the undo or redo has already been applied. /// Navigation is best effort: any failure is swallowed, because an exception here would tell the diff --git a/docs/api-reference.md b/docs/api-reference.md index 48fdfef..f88c1c8 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -278,7 +278,7 @@ LoadNewDocument(); ```csharp IEnumerable GetCommandsToUndo(SaveBoundary saveBoundary); ``` -Gets commands that would be undone to reach the specified save boundary. +Gets commands that would be undone to reach the specified save boundary. Returns nothing for a boundary whose save point no longer exists. **Parameters:** - `saveBoundary`: The target save boundary @@ -292,7 +292,7 @@ Gets commands that would be undone to reach the specified save boundary. ```csharp Task UndoToSaveBoundaryAsync(SaveBoundary saveBoundary, bool navigateToLastChange = true, CancellationToken cancellationToken = default); ``` -Undoes commands until reaching the specified save boundary. +Undoes commands until reaching the specified save boundary. A boundary obtained earlier, for example from `SaveBoundaryCreated`, still resolves to its save point after `MaxStackSize` trims the stack. A boundary whose save point no longer exists, because a new branch or `Clear()` removed it, is rejected and nothing is undone. **Parameters:** - `saveBoundary`: The target save boundary