Stop treating position -1 as clean after trimming or saving - #79
Merged
Merged
Conversation
HasUnsavedChanges assumed position -1 was the untouched initial state whenever no boundaries existed, so it reported false after MaxStackSize trimming shifted a never-saved edit there, or after a branch removed the only save point. AdjustPositions also discarded a boundary shifted to exactly -1, although -1 is a reachable position. Track whether the initial state is still clean explicitly: it is on construction and Clear(), and stops being so once anything is saved or commands are trimmed. Keep boundaries that land on -1. Fixes #76 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2BLMsT5ih3DnMNxTyGUHh
Keeps them clear of the exception-safety tests added alongside, so the two PRs merge independently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2BLMsT5ih3DnMNxTyGUHh
|
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
2 participants
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.
Fixes #76
Summary
SaveBoundaryManagerassumed position -1 always holds the clean initial document. That stops being true in two cases, and it caused two bugs:HasUnsavedChangesreturnedfalseat -1 whenever no boundaries existed. AfterMaxStackSizetrimming, or after a branch removed the only save point, -1 holds a state that was never saved. An app that prompts "save before closing?" would skip the prompt and lose the edit.AdjustPositionsdropped any boundary shifted to -1, even though -1 is a reachable position that can be saved.The fix tracks whether the initial state is clean with a private flag instead of assuming it:
Clear().CreateSaveBoundary. Once something is saved, the file on disk no longer holds the initial state. The existing behavior already reports -1 as unsaved when boundaries exist.AdjustPositions, because -1 then holds the result of the trimmed commands.I did not seed a visible implicit boundary at -1 as the issue suggested.
SaveBoundariesis public, and the existing tests assert its count, so a hidden flag gives the same result without changing what callers see.Tests
Three new tests in
UndoRedoStackTests, one for each scenario in the issue:HasUnsavedChanges_AtStartAfterTrimming_IsTrue(A)HasUnsavedChanges_AtStartAfterBranchRemovesTheOnlyBoundary_IsTrue(B)AdjustPositions_BoundaryShiftedToStart_IsKept(C): exactly one boundary is left, at -1, and undoing to it reports saved.With the fix reverted, all three fail. With the fix, the whole suite passes (53/53) on net10.0, including the existing
HasUnsavedChangesand serialization round-trip tests.🤖 Generated with Claude Code
https://claude.ai/code/session_01L2BLMsT5ih3DnMNxTyGUHh
Generated by Claude Code