Repository navigation
Apply a mergeable edit on its own when the previous command can't be undone to merge [patch] - #182
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
…undone to merge [patch] With auto-merge on, Execute undid the previous command before merging, outside any try block. When that Undo threw, Execute threw too, and the new command was never executed or recorded. Every later mergeable edit failed the same way. A failing pre-merge Undo now changes nothing and Execute falls back to the non-merge path: the new command is executed and pushed as its own entry. The failing command is then no longer current, so the next edit merges into the new one rather than retrying. Fixes #177 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3sakBdbLSZUGjwGAi
|
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
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 #177
What was wrong
#168 made the merge path undo the previous command before merging. That
lastCommand.Undo()call sat outside any try block. When it threw,Execute(command)threw too, so the new command was never executed or recorded. The merge chain wasn't broken, so every later mergeable edit took the same path and was lost the same way.Change
ExecuteMergedis nowTryExecuteMerged, and it returnsbool.Undo()throws, it returnsfalse. Nothing has been mutated at that point, and the stack is still accurate.Executethen continues on the ordinary non-merge path. It executes the new command on its own, pushes it as a new entry and raisesCommandExecutedfor it.Execute), the method still restores and rethrows exactly as before.I didn't set
_mergeChainBroken, which the triage comment suggested as an option. After the fallback, the failing command is no longer the current one, so the next edit can't retry that merge. It merges into the new command instead, and that command'sUndoworks. Breaking the chain would only stop ordinary coalescing of the next keystroke.#176 reworks the body of the same method, after the
Undo()call. This change only wraps theUndo()and adds theboolreturn, so any conflict should be small.Tests
New
MergeUndoFailureTests:APreviousCommandWhoseUndoThrows_StillLetsAMergeableEditApply(the acceptance case):Execute(B)applies B,CommandExecutedis raised for A then B, the history holds A then B, and nothing is reported as discarded.AfterTheFallback_TheNextEditMergesIntoTheNewCommand: a third keystroke merges into B, not A, and Undo then returns to A's state.With the service change reverted, both tests fail. With it, the full suite passes: 208/208 on Linux / .NET 10. That includes the #168 in-place
MergeWithtests.Generated by Claude Code