fix: isolate external changes into separate undo steps (#2007)#9926
Open
DukeDeSouth wants to merge 2 commits intoVSCodeVim:masterfrom
Open
fix: isolate external changes into separate undo steps (#2007)#9926DukeDeSouth wants to merge 2 commits intoVSCodeVim:masterfrom
DukeDeSouth wants to merge 2 commits intoVSCodeVim:masterfrom
Conversation
When external tools (format-on-save, Prettier, IntelliSense, AI suggestions) modify the document, those changes now get their own undo boundary instead of being merged with the next user action. Root cause: `onDidChangeTextDocument` had no way to distinguish user- triggered changes from external ones. External changes were either merged with the next keypress (Normal mode) or suppressed entirely (Insert mode), causing `u` to undo multiple unrelated changes at once. Fix: Add an `isHandlingKey` flag to `ModeHandler` that is `true` only while processing a user keystroke (`handleKeyEventLangmapped`). When `onDidChangeTextDocument` fires with `isHandlingKey === false`, the change is force-tracked and a separate undo step is created. Closes VSCodeVim#2007 Co-authored-by: Cursor <cursoragent@cursor.com>
Add isApplyingChange flag to HistoryTracker to prevent onDidChangeTextDocument from creating spurious undo boundaries when the HistoryTracker itself is applying undo/redo changes. The external change detection introduced for VSCodeVim#2007 was incorrectly intercepting document changes triggered by goBackHistoryStep(), goForwardHistoryStep(), and goBackHistoryStepsOnLine(), because these methods modify the document outside of handleKeyEventLangmapped (where isHandlingKey is true). The fix wraps all three change-application loops in try/finally blocks that set isApplyingChange=true, and adds this check to the external change detection condition in extensionBase.ts. Fixes regression in redo.test.ts where :undo/:redo via ExCommandLine caused undo stack corruption. Co-authored-by: Cursor <cursoragent@cursor.com>
Member
|
Wasn't this fixed by c544e2c? How does this change differ? |
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.
Human View
Summary
Fixes #2007 — a 9-year-old bug where pressing
u(undo) would sometimes undo the entire undo stack instead of just the last change.Root cause: External document changes (format-on-save, Prettier, IntelliSense, AI suggestions) were not isolated into their own undo steps. They were either merged with the next user action (Normal mode) or suppressed entirely (Insert mode).
Fix: Add an
isHandlingKeyflag toModeHandlerthat istrueonly while processing a user keystroke. WhenonDidChangeTextDocumentfires withisHandlingKey === false, the change is force-tracked withaddChange(true)and a separate undo boundary is created viafinishCurrentStep().Changes
src/mode/modeHandler.tsisHandlingKeyproperty; wraphandleKeyEventLangmappedintry/finallyto manage the flagextensionBase.tsonDidChangeTextDocumentwhen!mh.isHandlingKey; create separate undo boundaryTotal: ~25 lines added, 0 lines removed (before auto-formatting).
How it works
handleKeyEventLangmapped()setsisHandlingKey = trueisHandlingKey === false, it must be externalHistoryStepunow correctly undoes only the last user actionEdge cases handled
isHandlingKey = trueduring replay — no extra undo pointshandleKeyEventLangmapped— correctly flaggedfinallyblock guarantees flag resetaddChange()deduplicates via document version checkReproduction scenario (from issue)
EscEscu— Before: undoes both the edit AND the format. After: undoes only the last edit.Testing
node_modules/@types/globandtest/index.ts)AI View (DCCE Protocol v1.0)
Metadata
AI Contribution Summary
Verification Steps Performed
Human Review Guidance
src/mode/modeHandler.ts,extensionBase.ts,test/index.tsMade with M7 Cursor