diff --git a/UndoRedo.Test/UndoRedoStackTests.cs b/UndoRedo.Test/UndoRedoStackTests.cs index 0cdee0d..202862f 100644 --- a/UndoRedo.Test/UndoRedoStackTests.cs +++ b/UndoRedo.Test/UndoRedoStackTests.cs @@ -273,6 +273,71 @@ public void Execute_MaxStackSizeReached_RemovesOldestCommands() Assert.AreEqual("Command 5", stack.Commands[2].Description); // Newest command } + [TestMethod] + public void HasUnsavedChanges_AtStartAfterTrimming_IsTrue() + { + // Arrange: B trims A, so position -1 now holds A's never-saved result + UndoRedoService stack = new(new StackManager(), new SaveBoundaryManager(), new CommandMerger(), UndoRedoOptions.Create(maxStackSize: 1)); + int value = 0; + + stack.Execute(new DelegateCommand("A", () => value = 1, () => value = 0)); + stack.Execute(new DelegateCommand("B", () => value = 2, () => value = 1)); + + // Act + stack.Undo(); + + // Assert + Assert.AreEqual(-1, stack.CurrentPosition); + Assert.AreEqual(1, value); + Assert.IsTrue(stack.HasUnsavedChanges, "Position -1 is not the initial state once trimming has shifted A's result there"); + } + + [TestMethod] + public void HasUnsavedChanges_AtStartAfterBranchRemovesTheOnlyBoundary_IsTrue() + { + // Arrange: A is saved, then undone and branched away from + UndoRedoService stack = CreateService(); + int value = 0; + + stack.Execute(new DelegateCommand("A", () => value = 1, () => value = 0)); + stack.MarkAsSaved(); + stack.Undo(); + Assert.IsTrue(stack.HasUnsavedChanges, "The saved file holds A, not the initial state"); + + stack.Execute(new DelegateCommand("B", () => value = 2, () => value = 0)); + Assert.IsEmpty(stack.SaveBoundaries, "The branch invalidates the boundary at A"); + + // Act + stack.Undo(); + + // Assert: the file on disk still holds A + Assert.AreEqual(-1, stack.CurrentPosition); + Assert.AreEqual(0, value); + Assert.IsTrue(stack.HasUnsavedChanges, "The initial state was never what was saved"); + } + + [TestMethod] + public void AdjustPositions_BoundaryShiftedToStart_IsKept() + { + // Arrange + UndoRedoService stack = new(new StackManager(), new SaveBoundaryManager(), new CommandMerger(), UndoRedoOptions.Create(maxStackSize: 2)); + + stack.Execute(new DelegateCommand("A", () => { }, () => { })); + stack.MarkAsSaved("after A"); + stack.Execute(new DelegateCommand("B", () => { }, () => { })); + + // Act: C trims A, which shifts the save point after A to -1 + stack.Execute(new DelegateCommand("C", () => { }, () => { })); + + // Assert + Assert.HasCount(1, stack.SaveBoundaries, "A save point shifted to -1 is still reachable and must be kept"); + Assert.AreEqual(-1, stack.SaveBoundaries[0].Position); + + stack.Undo(); + stack.Undo(); + Assert.IsFalse(stack.HasUnsavedChanges, "Undoing back to the save point after A must report it as saved"); + } + [TestMethod] public void CommandMerging_ConsecutiveCommands_MergesCorrectly() { diff --git a/UndoRedo/Services/SaveBoundaryManager.cs b/UndoRedo/Services/SaveBoundaryManager.cs index b6a1ebb..e6e4bd1 100644 --- a/UndoRedo/Services/SaveBoundaryManager.cs +++ b/UndoRedo/Services/SaveBoundaryManager.cs @@ -11,16 +11,20 @@ public sealed class SaveBoundaryManager : ISaveBoundaryManager { private readonly List _saveBoundaries = []; + // Whether position -1 still holds the untouched initial state, which is clean without a boundary. + // It stops being true once anything is saved, since the saved state replaces it, and once trimming + // shifts later commands' results down to -1. + private bool _initialStateIsClean = true; + /// public IReadOnlyList SaveBoundaries => _saveBoundaries.AsReadOnly(); /// public bool HasUnsavedChanges(int currentPosition) { - // If no save boundaries exist, we have unsaved changes unless at initial position - if (_saveBoundaries.Count == 0) + if (currentPosition == -1 && _initialStateIsClean) { - return currentPosition >= 0; + return false; } // No unsaved changes if we're exactly at a save boundary position @@ -32,6 +36,7 @@ public SaveBoundary CreateSaveBoundary(int position, string? description = null) { SaveBoundary saveBoundary = new(position, description); _saveBoundaries.Add(saveBoundary); + _initialStateIsClean = false; return saveBoundary; } @@ -58,12 +63,19 @@ public void AdjustPositions(int adjustment) return; } + if (adjustment < 0) + { + // Commands were trimmed from the bottom, so -1 is now the state after them, not the initial one + _initialStateIsClean = false; + } + for (int i = _saveBoundaries.Count - 1; i >= 0; i--) { SaveBoundary boundary = _saveBoundaries[i]; int newPosition = boundary.Position + adjustment; - if (newPosition < 0) + // -1 is a reachable position, so a boundary shifted exactly there is still a valid save point + if (newPosition < -1) { _saveBoundaries.RemoveAt(i); } @@ -90,5 +102,9 @@ public IEnumerable GetCommandsToUndo(SaveBoundary saveBoundary, int cu } /// - public void Clear() => _saveBoundaries.Clear(); + public void Clear() + { + _saveBoundaries.Clear(); + _initialStateIsClean = true; + } }