Move the stack position only after Undo/Redo succeeds - #78
Merged
Merged
Conversation
UndoAsync, RedoAsync and UndoToSaveBoundaryAsync moved the position before running the command, so a command that threw left the stack out of step with the application: the next Undo skipped the failed command, and a failed Redo could later be undone without ever having been applied. Run the command first and move only on success, matching the exception-safe Execute(). Fixes #75 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 #75
Summary
UndoAsync,RedoAsyncand the loop inUndoToSaveBoundaryAsyncmoved the stack position before running the command. If the command threw, the position had moved but the application state had not:CanRedo == truefor a command that was never undone. The next Undo skipped it.Undo()on a change that was never made.Each path now runs the command first and moves the position only on success. This is the same ordering the exception-safe
Execute()already uses (#70).UndoToSaveBoundaryAsyncstops at the first failure, with the position on the command that failed.Tests
Three new tests in
UndoRedoStackTests:Undo_CommandThrowsException_LeavesPositionUnchanged: position,CanUndoandCanRedoare unchanged, and the next Undo retries B rather than skipping to A.Redo_CommandThrowsException_LeavesPositionUnchanged: the failed command is still redoable and is not undoable.UndoToSaveBoundary_CommandThrowsException_StopsWithAccuratePositionWith the fix reverted, all three fail on the position assertion. With the fix, the whole suite passes (53/53) on net10.0.
🤖 Generated with Claude Code
https://claude.ai/code/session_01L2BLMsT5ih3DnMNxTyGUHh
Generated by Claude Code