From 65272a1205a9057946736df3f1e2db0e0871462d Mon Sep 17 00:00:00 2001 From: hesam-oxe <193629756+hesam-oxe@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:10:22 +0000 Subject: [PATCH 1/2] fix(history): re-derive merged undo steps; mirror native undo/redo; guard documents Three complementary fixes for the cursed undo corruption reported in #2007: 1. HistoryStep.merge() combined multi-diff steps with pairwise range intersection math that is only correct for tail-aligned overlaps. Any edit inside an earlier edit's span (macros, multi-action remaps, :normal, U) silently corrupted the step and broke later undo/redo. Merge now un-applies the step from the current text to recover its base and re-diffs base -> current with the same engine addChange() uses, so a merged step is byte-identical to recording it in a single diff. A validation guard refuses to merge when re-derivation cannot be proven exact. The diff-walk is extracted into a shared pure module (src/history/textChange.ts) used by both addChange() and merge(). 2. Native undo/redo (Ctrl+Z / Ctrl+Shift+Z) used to be recorded as new forward changes, so the next `u` would re-do instead of undo and the two stacks diverged. The onDidChangeTextDocument listener now latches the event's reason, and addChange() simulates the latched sequence against the last synced text: on an exact match the stack pointer is silently mirrored (marks restore themselves since they are versioned per step), otherwise it falls back to recording the net diff exactly like before. Mirroring can therefore never corrupt the stack. 3. previousDocumentState now records the document identity (object + URI) and refuses to diff across documents: identical text re-stamps the identity, anything else drops history and re-syncs instead of recording a bogus delete-everything/insert-everything step. The focusChanged guard in ModeHandler was dead (no code path ever set it) and is removed in favor of this check. Also syncs the version when the text is unchanged to avoid re-diffing on every keypress. --- extensionBase.ts | 19 +-- src/history/historyTracker.ts | 285 ++++++++++++++++++++++++++-------- src/history/textChange.ts | 195 +++++++++++++++++++++++ src/mode/modeHandler.ts | 13 +- 4 files changed, 425 insertions(+), 87 deletions(-) create mode 100644 src/history/textChange.ts diff --git a/extensionBase.ts b/extensionBase.ts index 6b4d24231f8..b81ac52030d 100644 --- a/extensionBase.ts +++ b/extensionBase.ts @@ -56,15 +56,6 @@ export async function getAndUpdateModeHandler( previousActiveEditorUri = activeTextEditor.document.uri; - if (curHandler.focusChanged) { - curHandler.focusChanged = false; - - if (previousActiveEditorUri) { - const prevHandler = ModeHandlerMap.get(previousActiveEditorUri); - prevHandler!.focusChanged = true; - } - } - return curHandler; } @@ -161,6 +152,11 @@ export async function activate(context: vscode.ExtensionContext, handleLocal: bo if (mh.vimState.currentMode === Mode.Insert) { mh.vimState.historyTracker.currentContentChanges.push(...event.contentChanges); } + + // Let the history tracker know about native undo/redo so it can mirror them onto + // its own undo stack instead of recording them as new forward changes (see #2007). + // Anything else (including our own programmatic edits) carries no reason and is ignored. + mh.vimState.historyTracker.noteNativeUndoRedo(event.reason); } }); @@ -280,11 +276,6 @@ export async function activate(context: vscode.ExtensionContext, handleLocal: bo return; } - if (mh.focusChanged) { - mh.focusChanged = false; - return; - } - if (mh.vimState.currentMode === Mode.EasyMotionMode) { return; } diff --git a/src/history/historyTracker.ts b/src/history/historyTracker.ts index 6fa1e2417ef..c923058bd0d 100644 --- a/src/history/historyTracker.ts +++ b/src/history/historyTracker.ts @@ -9,7 +9,6 @@ * * Undo/Redo will advance forward or backwards through Steps. */ -import * as DiffMatchPatch from 'diff-match-patch'; import * as vscode from 'vscode'; import { Position } from 'vscode'; @@ -23,11 +22,15 @@ import { StatusBar } from '../statusBar'; import { Logger } from '../util/logger'; import { VimState } from './../state/vimState'; import { TextEditor } from './../textEditor'; - -const diffEngine = new DiffMatchPatch.diff_match_patch(); -diffEngine.Diff_Timeout = 1; // 1 second - -class DocumentChange { +import { + applyTextChanges, + diffTextsToChanges, + mergeDocumentChanges, + unapplyTextChanges, + type ITextChange, +} from './textChange'; + +class DocumentChange implements ITextChange { /** * The Position at which this change starts */ @@ -165,49 +168,34 @@ class HistoryStep { } /** - * Collapse individual character changes into larger blocks of changes + * Collapse the changes of this step into an equivalent, canonical change list. + * + * A step can accumulate changes from several diffs (macros, multi-action remaps, + * `:normal`, `U`, ...). Merging them pairwise by intersecting ranges is only correct + * when every overlap lines up at the tail of the previous change; anything else — + * an edit inside an earlier edit's span, overlapping replaces — silently corrupts + * the step and breaks later undo/redo (see VSCodeVim/Vim#2007). Instead, delegate to + * `mergeDocumentChanges()`, which re-derives the step from base to current text. */ public merge(document: vscode.TextDocument): void { if (this.changes.length < 2) { return; } - // merged will replace this.changes - const merged: DocumentChange[] = []; - // manually reduce() this.changes with variables `current` and `next` - // we can't use reduce() directly because the loop can emit multiple elements - let current = this.changes[0]; - for (const next of this.changes.slice(1)) { - if (current.before.length + current.after.length === 0) { - // current is eliminated, replace it with top of merged, or adopt next as current - // see also add+del case - if (merged.length > 0) { - current = merged.pop()!; - } else { - current = next; - continue; - } - } + const merged = mergeDocumentChanges(this.changes, document.getText()); + this.changes = merged.map((change) => + DocumentChange.replace(change.start, change.before, change.after), + ); + } - const intersect = current.afterRange.intersection(next.beforeRange); - if (intersect) { - const [first, second] = current.start.isBeforeOrEqual(next.start) - ? [current, next] - : [next, current]; - const intersectLength = - document.offsetAt(intersect.end) - document.offsetAt(intersect.start); - current = DocumentChange.replace( - first.start, - first.before + second.before.slice(intersectLength), - first.after.slice(0, first.after.length - intersectLength) + second.after, - ); - } else { - merged.push(current); - current = next; - } - } - merged.push(current); - this.changes = merged; + /** + * The net change in document length produced by applying this step's changes. + */ + public netLengthDelta(): number { + return this.changes.reduce( + (sum, change) => sum + change.after.length - change.before.length, + 0, + ); } /** @@ -317,6 +305,17 @@ class UndoStack { this.historySteps.push(step); } + /** + * Discards all history. Only used when the tracker is rebound to a different document, + * in which case none of the recorded steps (nor the local marks versioned with them) + * can meaningfully apply anymore. + */ + public clear(): void { + this.historySteps = []; + this.currentStepIndex = -1; + this.initialMarks = []; + } + public getCurrentMarkList(): IMark[] { const step = this.getCurrentHistoryStep(); return step?.marks ?? this.initialMarks; @@ -395,19 +394,40 @@ export class HistoryTracker { /** * The state of the document the last time HistoryTracker.addChange() or HistoryTracker.ignoreChange() was called. * This is used to avoid retrieiving the document text and doing a full diff when it isn't necessary. + * + * Besides text and version this also records the document's identity: without it, a + * handler reused across documents would diff one file's text against another's and + * record a bogus delete-everything/insert-everything step (see VSCodeVim/Vim#2007). */ private previousDocumentState: { text: string; versionNumber: number; + document: vscode.TextDocument | undefined; + documentUri: string | undefined; }; + /** + * Native (non-Vim) undo/redo operations observed since the last sync, oldest first. + * + * Latched by `noteNativeUndoRedo()` from the global `onDidChangeTextDocument` listener + * (which is the only place that sees the event's `reason`) and consumed by `addChange()`. + * Capped: beyond `maxPendingNativeUndoRedo` we stop trusting the sequence and fall back + * to recording the net diff, exactly like before. + */ + private pendingNativeUndoRedo: Array<'undo' | 'redo'> = []; + private pendingNativeUndoRedoOverflowed = false; + private static readonly maxPendingNativeUndoRedo = 100; + private readonly vimState: VimState; constructor(vimState: VimState) { this.vimState = vimState; + const document = this.vimState.editor?.document; this.previousDocumentState = { text: this.getDocumentText(), versionNumber: this.getDocumentVersion(), + document, + documentUri: document?.uri.toString(), }; } @@ -682,7 +702,24 @@ export class HistoryTracker { * Determines what changed by diffing the document against what it used to look like. */ public addChange(force: boolean = false): boolean { - if (this.getDocumentVersion() === this.previousDocumentState.versionNumber) { + const document = this.vimState.editor?.document; + const versionNumber = document?.version ?? -1; + + if ( + document !== this.previousDocumentState.document || + document?.uri.toString() !== this.previousDocumentState.documentUri + ) { + // The handler is looking at a different document than we last synced (this should + // barely happen since handlers are keyed by document, but it must never produce a + // cross-document diff). Rebind instead of recording a bogus whole-file change. + this.rebindToDocument(document); + return false; + } + + if (versionNumber === this.previousDocumentState.versionNumber) { + // Nothing changed since the last sync. (A document change event always bumps the + // version, so any latched native undo/redo would be stale — drop it.) + this.clearPendingNativeUndoRedo(); return false; } @@ -698,11 +735,27 @@ export class HistoryTracker { // We can ignore changes while we're in insert/replace mode, since we can't interact with them (via undo, etc.) until we're back to normal mode // This allows us to avoid a little bit of work per keystroke, but more importantly, it means we'll get bigger contiguous edit chunks to merge. // This is particularly impactful when there are multiple cursors, which are otherwise difficult to optimize. + // NOTE: latched native undo/redo is deliberately *not* consumed here; it persists + // until the insert/replace session is flushed and reconciled below. return false; } const newText = this.getDocumentText(); + + if (this.pendingNativeUndoRedo.length > 0) { + // The document was (at least partially) changed by native undo/redo. If those + // operations exactly undid/redid our own steps, mirror them onto the stack instead + // of recording them as new forward changes; otherwise fall through and record the + // net diff exactly like before. + if (this.tryMirrorNativeUndoRedo(newText)) { + return false; + } + } + if (newText === this.previousDocumentState.text) { + // The version bumped but the text is identical (e.g. something was natively undone + // and redone). Sync the version so we don't re-diff on every subsequent keypress. + this.previousDocumentState.versionNumber = versionNumber; return false; } @@ -716,34 +769,137 @@ export class HistoryTracker { // Couldn't we also ditch this diffing approach entirely and just use `TextDocumentContentChangeEvent`s? - const diffs = diffEngine.diff_main(this.previousDocumentState.text, newText); - diffEngine.diff_cleanupEfficiency(diffs); + for (const change of diffTextsToChanges(this.previousDocumentState.text, newText)) { + this.undoStack.pushChange(DocumentChange.replace(change.start, change.before, change.after)); + } - let currentPosition = new Position(0, 0); + this.previousDocumentState.text = newText; + this.previousDocumentState.versionNumber = versionNumber; - for (const diff of diffs) { - const [whatHappened, text] = diff; - const added = whatHappened === DiffMatchPatch.DIFF_INSERT; - const removed = whatHappened === DiffMatchPatch.DIFF_DELETE; + return true; + } - if (added || removed) { - this.undoStack.pushChange( - added - ? DocumentChange.insert(currentPosition, text) - : DocumentChange.delete(currentPosition, text), - ); - } + /** + * Rebinds the tracker to `document`, which differs from the last synced one. + * + * If the text is identical (rename, save-as, or swapping between identical files) the + * geometry is unchanged, so recorded steps still apply and only the identity is + * re-stamped. Otherwise all history is dropped — a fresh handler would start empty — + * instead of recording a bogus delete-everything/insert-everything step. + */ + private rebindToDocument(document: vscode.TextDocument | undefined): void { + const newText = document?.getText() ?? ''; + if (newText === this.previousDocumentState.text) { + this.previousDocumentState.document = document; + this.previousDocumentState.documentUri = document?.uri.toString(); + this.previousDocumentState.versionNumber = document?.version ?? -1; + } else { + this.undoStack.clear(); + this.previousDocumentState = { + text: newText, + versionNumber: document?.version ?? -1, + document, + documentUri: document?.uri.toString(), + }; + } + this.clearPendingNativeUndoRedo(); + } + + /** + * Records a native (non-Vim) undo/redo of the tracked document. + * + * Called from the global `onDidChangeTextDocument` listener in `extensionBase.ts`, which + * is the only place that observes the change event's `reason`. Anything else (including + * our own programmatic edits, whose `reason` is `undefined`) is ignored. + */ + public noteNativeUndoRedo(reason: vscode.TextDocumentChangeReason | undefined): void { + if (reason === vscode.TextDocumentChangeReason.Undo) { + this.pushPendingNativeUndoRedo('undo'); + } else if (reason === vscode.TextDocumentChangeReason.Redo) { + this.pushPendingNativeUndoRedo('redo'); + } + } + + private pushPendingNativeUndoRedo(op: 'undo' | 'redo'): void { + if (this.pendingNativeUndoRedo.length >= HistoryTracker.maxPendingNativeUndoRedo) { + this.pendingNativeUndoRedoOverflowed = true; + return; + } + this.pendingNativeUndoRedo.push(op); + } + + private clearPendingNativeUndoRedo(): void { + this.pendingNativeUndoRedo = []; + this.pendingNativeUndoRedoOverflowed = false; + } + + /** + * Attempts to mirror latched native undo/redo operations onto the undo stack. + * + * Simulates the latched sequence against the last synced text: an `undo` un-applies the + * current tip step, a `redo` re-applies the next one. If the simulation lands exactly on + * `newText`, the stack pointer is silently moved there (as if the user had pressed `u` / + * `Ctrl-r`), the state is synced, and no phantom forward change is recorded. Marks need + * no updating: they are versioned per step, so moving the pointer restores them. + * + * If anything doesn't line up (mixed with other edits, only partially overlapping our + * steps, stack boundaries, overflow, ...), returns false and the caller falls back to + * recording the net diff as a forward change — the previous behavior. Mirroring can + * therefore never corrupt the stack; it only avoids recording when it is provably exact. + * + * @returns true if the operations were mirrored (and the state synced). + */ + private tryMirrorNativeUndoRedo(newText: string): boolean { + const ops = this.pendingNativeUndoRedo; + const overflowed = this.pendingNativeUndoRedoOverflowed; + this.clearPendingNativeUndoRedo(); + + if (overflowed || ops.length === 0) { + return false; + } - if (!removed) { - currentPosition = currentPosition.advancePositionByText(text); + // Cheap precheck: simulate only the net length deltas before touching any strings. + let index = this.undoStack.getCurrentHistoryStepIndex(); + let expectedLength = this.previousDocumentState.text.length; + for (const op of ops) { + const step = + op === 'undo' + ? this.undoStack.getHistoryStepAtIndex(index) + : this.undoStack.getHistoryStepAtIndex(index + 1); + if (step === undefined) { + return false; } + expectedLength += op === 'undo' ? -step.netLengthDelta() : step.netLengthDelta(); + index += op === 'undo' ? -1 : 1; + } + if (expectedLength !== newText.length) { + return false; } - this.previousDocumentState = { - text: newText, - versionNumber: this.getDocumentVersion(), - }; + // Full simulation against the last synced text. + let simulated = this.previousDocumentState.text; + index = this.undoStack.getCurrentHistoryStepIndex(); + for (const op of ops) { + // Existence was verified by the precheck above. + const step = this.undoStack.getHistoryStepAtIndex(op === 'undo' ? index : index + 1)!; + simulated = + op === 'undo' + ? unapplyTextChanges(simulated, step.changes) + : applyTextChanges(simulated, step.changes); + index += op === 'undo' ? -1 : 1; + } + if (simulated !== newText) { + return false; + } + for (const op of ops) { + if (op === 'undo') { + this.undoStack.stepBackward(); + } else { + this.undoStack.stepForward(); + } + } + this.ignoreChange(); return true; } @@ -753,9 +909,12 @@ export class HistoryTracker { * the HistoryTracker. */ public ignoreChange(): void { + const document = this.vimState.editor?.document; this.previousDocumentState = { text: this.getDocumentText(), versionNumber: this.getDocumentVersion(), + document, + documentUri: document?.uri.toString(), }; } diff --git a/src/history/textChange.ts b/src/history/textChange.ts new file mode 100644 index 00000000000..58e10751981 --- /dev/null +++ b/src/history/textChange.ts @@ -0,0 +1,195 @@ +/** + * Pure text-change helpers shared by the undo history tracker. + * + * `HistoryTracker` records document edits as `ITextChange`s (`start`, `before`, `after`) + * and needs to (re)apply them to plain strings in a few places — most notably when merging + * the changes of a history step, and when reconciling Vim's undo stack with VS Code's + * native undo/redo. Those operations must not depend on a live editor, so they live here + * as pure functions operating on strings and `vscode.Position`s. + * + * Positions follow the same conventions as the rest of the extension: they are produced + * with `Position.advancePositionByText` (see `src/common/motion/position.ts`) and offsets + * are UTF-16 code units, exactly like `TextDocument.offsetAt`. + */ +import * as DiffMatchPatch from 'diff-match-patch'; +import { Position } from 'vscode'; + +import '../common/motion/position'; + +/** + * A single contiguous document edit: the text `before` occupied the range starting at + * `start` (in the pre-change document), and was replaced with the text `after`. + * + * `DocumentChange` (in `historyTracker.ts`) implements this structurally; this module + * only ever needs the data, never the editor-bound behavior. + */ +export interface ITextChange { + readonly start: Position; + readonly before: string; + readonly after: string; +} + +const diffEngine = new DiffMatchPatch.diff_match_patch(); +diffEngine.Diff_Timeout = 1; // 1 second + +/** + * Equivalent of `TextDocument.offsetAt` for a plain string. + * + * Line breaks are `\r\n` (length 2), `\n` and `\r` (length 1 each), matching VS Code. + */ +export function offsetAtText(text: string, position: Position): number { + let line = 0; + let offset = 0; + while (line < position.line && offset < text.length) { + const ch = text[offset]; + if (ch === '\r') { + offset += text[offset + 1] === '\n' ? 2 : 1; + line++; + } else if (ch === '\n') { + offset += 1; + line++; + } else { + offset++; + } + } + return offset + position.character; +} + +/** + * Applies a single change to `text` and returns the result. + */ +export function applyTextChange(text: string, change: ITextChange): string { + const startOffset = offsetAtText(text, change.start); + // `before.length` counts UTF-16 code units, which is exactly what document offsets count. + return text.slice(0, startOffset) + change.after + text.slice(startOffset + change.before.length); +} + +/** + * Applies `changes` (oldest first) to `text` and returns the result. + */ +export function applyTextChanges(text: string, changes: readonly ITextChange[]): string { + let result = text; + for (const change of changes) { + result = applyTextChange(result, change); + } + return result; +} + +/** + * Undoes `changes` (which must be ordered oldest-first) from `text` and returns the result. + */ +export function unapplyTextChanges(text: string, changes: readonly ITextChange[]): string { + let result = text; + for (let i = changes.length - 1; i >= 0; i--) { + const change = changes[i]; + result = applyTextChange(result, { + start: change.start, + before: change.after, + after: change.before, + }); + } + return result; +} + +/** + * Diffs `oldText` into `newText` and returns the canonical change list for that transition. + * + * This is the single place where raw diff hunks are translated into `ITextChange`s; both + * `HistoryTracker.addChange()` and `mergeDocumentChanges()` build on it so that a step + * accumulated from several diffs ends up shaped exactly like a step recorded in one go. + */ +export function diffTextsToChanges(oldText: string, newText: string): ITextChange[] { + const diffs = diffEngine.diff_main(oldText, newText); + diffEngine.diff_cleanupEfficiency(diffs); + + const changes: ITextChange[] = []; + let currentPosition = new Position(0, 0); + + for (const diff of diffs) { + const [whatHappened, text] = diff; + const added = whatHappened === DiffMatchPatch.DIFF_INSERT; + const removed = whatHappened === DiffMatchPatch.DIFF_DELETE; + + if (added || removed) { + changes.push({ + start: currentPosition, + before: removed ? text : '', + after: added ? text : '', + }); + } + + if (!removed) { + currentPosition = currentPosition.advancePositionByText(text); + } + } + + return changes; +} + +/** + * Coalesces a pure deletion immediately followed by a pure insertion at the same position + * into a single replace change. + * + * Unlike general change merging this is exactly equivalent by construction — removing `X` + * at `P` and then inserting `Y` at `P` is precisely the replace `X` → `Y` at `P` — and it + * preserves the historical single-change shape of steps like `cw`, `s` or `R`-typing, whose + * consumers (`U`, the status bar change count, `'`/`[` marks) were built around it. + */ +export function coalesceAdjacentReplacePairs(changes: readonly ITextChange[]): ITextChange[] { + const coalesced: ITextChange[] = []; + for (const change of changes) { + const prev = coalesced[coalesced.length - 1]; + if ( + prev !== undefined && + prev.after === '' && + prev.before !== '' && + change.before === '' && + change.after !== '' && + change.start.isEqual(prev.start) + ) { + coalesced[coalesced.length - 1] = { + start: prev.start, + before: prev.before, + after: change.after, + }; + } else { + coalesced.push(change); + } + } + return coalesced; +} + +/** + * Merges the changes of one history step into an equivalent, canonical change list. + * + * `changes` must be ordered oldest-first and must describe the contiguous transition from + * some base text to `currentText` (which is exactly what a step's accumulated diffs are). + * + * Rather than combining ranges pairwise — which is only correct when every overlap lines + * up at the tail of the previous change (see VSCodeVim/Vim#2007) — this recovers the base + * text by un-applying the changes from `currentText` and re-diffs base → current with the + * same engine `addChange()` uses. The result is therefore, by construction, identical to + * what recording the whole step in a single diff would have produced. + * + * Safety net: if `changes` do not actually span base → `currentText` (which can only happen + * if a caller passes inconsistent data), the re-derived list is discarded and the input is + * returned untouched, so merging can never corrupt a step that used to apply cleanly. + */ +export function mergeDocumentChanges( + changes: readonly ITextChange[], + currentText: string, +): ITextChange[] { + if (changes.length < 2) { + return [...changes]; + } + + const baseText = unapplyTextChanges(currentText, changes); + const merged = coalesceAdjacentReplacePairs(diffTextsToChanges(baseText, currentText)); + + // Validate before trusting the re-derived list: it must map base -> current exactly. + if (applyTextChanges(baseText, merged) !== currentText) { + return [...changes]; + } + + return merged; +} diff --git a/src/mode/modeHandler.ts b/src/mode/modeHandler.ts index 2b12abfd02b..bb8fdb69d2f 100644 --- a/src/mode/modeHandler.ts +++ b/src/mode/modeHandler.ts @@ -74,8 +74,6 @@ export class ModeHandler implements vscode.Disposable, IModeHandler { public lastMovementFailed: boolean = false; - public focusChanged = false; - private searchDecorationCacheKey: { searchString: string; documentVersion: number } | undefined; private readonly disposables: vscode.Disposable[] = []; @@ -904,14 +902,9 @@ export class ModeHandler implements vscode.Disposable, IModeHandler { this.vimState.dotCommandStatus = DotCommandStatus.Waiting; } - // track undo history - if (!this.focusChanged) { - // important to ensure that focus didn't change, otherwise - // we'll grab the text of the incorrect active window and assume the - // whole document changed! - - this.vimState.historyTracker.addChange(); - } + // Track undo history. Cross-document protection lives in HistoryTracker itself, + // which refuses to diff across documents by checking the document identity. + this.vimState.historyTracker.addChange(); // Don't record an undo point for every action of a macro, only at the very end if ( From 0dc92e8c42892f3bbf949ecc0ae9ff535ecaa2ca Mon Sep 17 00:00:00 2001 From: hesam-oxe <193629756+hesam-oxe@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:10:33 +0000 Subject: [PATCH 2/2] test(history): first coverage for the undo core The undo core (addChange/merge/goBack/goForward) previously had zero tests, which is how the #2007 corruption survived for years. Add 50 tests: - test/textChange.test.ts: pure unit tests for the new text-change helpers (offset math incl. CRLF, apply/unapply, diff replay, replace coalescing) plus 12 hand-written merge scenarios with exact-shape assertions, a deterministic 300-iteration fuzz round-trip, a validation-guard case, and a real-editor apply check. - test/historyTracker.test.ts: tracker-level integration tests on real documents: multi-change step round-trips for every known merge corruption shape, U-then-u round-trips, native undo/redo mirroring (incl. end-to-end wiring through the real document listener), fallback recording, latch overflow/deferral, and document identity/rebinding. - test/macro.test.ts: end-to-end key-driven tests proving a macro and a remap that edit inside their own earlier edits survive u + Ctrl+R. Verified by mutation: 17 historyTracker + 2 e2e tests fail on the old code and all pass with the fix. --- test/historyTracker.test.ts | 519 ++++++++++++++++++++++++++++++++++++ test/macro.test.ts | 27 ++ test/textChange.test.ts | 358 +++++++++++++++++++++++++ 3 files changed, 904 insertions(+) create mode 100644 test/textChange.test.ts diff --git a/test/historyTracker.test.ts b/test/historyTracker.test.ts index 29c40ee3b83..c4c0855c797 100644 --- a/test/historyTracker.test.ts +++ b/test/historyTracker.test.ts @@ -4,8 +4,11 @@ import vscode, { Position } from 'vscode'; import { HistoryTracker, ILocalMark, IMark } from '../src/history/historyTracker'; import { Jump } from '../src/jumps/jump'; +import { Mode } from '../src/mode/mode'; +import { ModeHandlerMap } from '../src/mode/modeHandlerMap'; import { globalState } from '../src/state/globalState'; import { VimState } from '../src/state/vimState'; +import { StatusBar } from '../src/statusBar'; suite('historyTracker unit tests', () => { let sandbox: sinon.SinonSandbox; @@ -167,6 +170,522 @@ suite('historyTracker unit tests', () => { }); }); +suite('historyTracker undo/redo integrity (#2007)', () => { + let sandbox: sinon.SinonSandbox; + + setup(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(StatusBar, 'setText'); + }); + + teardown(async () => { + sandbox.restore(); + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); + }); + + const openUntitled = async ( + content: string, + ): Promise<{ document: vscode.TextDocument; editor: vscode.TextEditor }> => { + const document = await vscode.workspace.openTextDocument({ content }); + const editor = await vscode.window.showTextDocument(document); + return { document, editor }; + }; + + const setupTracker = ( + editor: vscode.TextEditor, + ): { vimState: VimState; historyTracker: HistoryTracker } => { + const vimState = sandbox.createStubInstance(VimState) as unknown as VimState; + vimState.editor = editor; + vimState.modeData = { mode: Mode.Normal }; + return { vimState, historyTracker: new HistoryTracker(vimState) }; + }; + + const applyEdit = async ( + editor: vscode.TextEditor, + range: vscode.Range, + text: string, + ): Promise => { + assert.ok(await editor.edit((builder) => builder.replace(range, text)), 'Edit failed'); + }; + + const R = (startLine: number, startChar: number, endLine: number, endChar: number) => + new vscode.Range(new Position(startLine, startChar), new Position(endLine, endChar)); + + /** Records one finished step from a list of sequential edits (macro-like accumulation). */ + const recordStep = async ( + historyTracker: HistoryTracker, + editor: vscode.TextEditor, + edits: Array<[vscode.Range, string]>, + ): Promise => { + for (const [range, text] of edits) { + await applyEdit(editor, range, text); + historyTracker.addChange(); + } + historyTracker.finishCurrentStep(); + }; + + const assertRoundTrip = async ( + historyTracker: HistoryTracker, + editor: vscode.TextEditor, + initial: string, + current: string, + ): Promise => { + assert.strictEqual(editor.document.getText(), current); + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), initial, 'undo did not restore the initial text'); + await historyTracker.goForwardHistoryStep(); + assert.strictEqual(editor.document.getText(), current, 'redo did not restore the edited text'); + }; + + test('records a single edit and round-trips it', async () => { + const { editor } = await openUntitled('hello'); + const { historyTracker } = setupTracker(editor); + + await applyEdit(editor, R(0, 1, 0, 4), 'i'); + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + + await assertRoundTrip(historyTracker, editor, 'hello', 'hio'); + }); + + test('merges a cw-like delete plus adjacent insert into one undoable step', async () => { + const { editor } = await openUntitled('foo bar'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, [ + [R(0, 0, 0, 3), ''], + [R(0, 0, 0, 0), 'XYZ'], + ]); + + await assertRoundTrip(historyTracker, editor, 'foo bar', 'XYZ bar'); + }); + + test('round-trips a deletion strictly inside a previous insertion', async () => { + const { editor } = await openUntitled('12'); + const { historyTracker } = setupTracker(editor); + + // The old merge recorded 'abc' here, corrupting the redo into '1abc2'. + await recordStep(historyTracker, editor, [ + [R(0, 1, 0, 1), 'abcdef'], + [R(0, 3, 0, 6), ''], + ]); + + await assertRoundTrip(historyTracker, editor, '12', '1abf2'); + }); + + test('round-trips an insertion inside a previous insertion', async () => { + const { editor } = await openUntitled('12'); + const { historyTracker } = setupTracker(editor); + + // The old merge recorded 'ABCXY' here, corrupting the redo into '1ABCXY2'. + await recordStep(historyTracker, editor, [ + [R(0, 1, 0, 1), 'ABC'], + [R(0, 2, 0, 2), 'XY'], + ]); + + await assertRoundTrip(historyTracker, editor, '12', '1AXYBC2'); + }); + + test('round-trips an insertion followed by a superset deletion', async () => { + const { editor } = await openUntitled('12'); + const { historyTracker } = setupTracker(editor); + + // The old merge recorded an invalid range here, corrupting both undo and redo. + await recordStep(historyTracker, editor, [ + [R(0, 1, 0, 1), 'ABC'], + [R(0, 0, 0, 5), ''], + ]); + + await assertRoundTrip(historyTracker, editor, '12', ''); + }); + + test('round-trips sequential inserts at the same position in order', async () => { + const { editor } = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + // The old merge recorded 'XY' here, corrupting the redo into 'aXYb'. + await recordStep(historyTracker, editor, [ + [R(0, 1, 0, 1), 'X'], + [R(0, 1, 0, 1), 'Y'], + ]); + + await assertRoundTrip(historyTracker, editor, 'ab', 'aYXb'); + }); + + test('round-trips a longer chain of overlapping changes', async () => { + const { editor } = await openUntitled('abcdef'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, [ + [R(0, 2, 0, 2), 'XX'], + [R(0, 3, 0, 5), ''], + [R(0, 1, 0, 2), 'ZZ'], + ]); + + await assertRoundTrip(historyTracker, editor, 'abcdef', 'aZZXdef'); + }); + + test('U undoes only the most recently changed line', async () => { + const { editor } = await openUntitled('aaa\nbbb'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, [[R(0, 0, 0, 3), 'XXX']]); + await recordStep(historyTracker, editor, [[R(1, 0, 1, 3), 'YYY']]); + assert.strictEqual(editor.document.getText(), 'XXX\nYYY'); + + await historyTracker.goBackHistoryStepsOnLine(); + assert.strictEqual(editor.document.getText(), 'XXX\nbbb'); + + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'XXX\nYYY'); + }); + + test('U followed by u round-trips a merged multi-change step', async () => { + const { editor } = await openUntitled('12'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, [ + [R(0, 1, 0, 1), 'ABC'], + [R(0, 2, 0, 2), 'XY'], + ]); + assert.strictEqual(editor.document.getText(), '1AXYBC2'); + + await historyTracker.goBackHistoryStepsOnLine(); + assert.strictEqual(editor.document.getText(), '12'); + + // The old merge corrupted this redo into '1ABCXY2'. + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), '1AXYBC2'); + }); +}); + +suite('historyTracker native undo/redo mirroring (#2007)', () => { + let sandbox: sinon.SinonSandbox; + + setup(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(StatusBar, 'setText'); + }); + + teardown(async () => { + sandbox.restore(); + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); + }); + + const openUntitled = async (content: string): Promise => { + const document = await vscode.workspace.openTextDocument({ content }); + return vscode.window.showTextDocument(document); + }; + + const setupTracker = ( + editor: vscode.TextEditor, + ): { vimState: VimState; historyTracker: HistoryTracker } => { + const vimState = sandbox.createStubInstance(VimState) as unknown as VimState; + vimState.editor = editor; + vimState.modeData = { mode: Mode.Normal }; + return { vimState, historyTracker: new HistoryTracker(vimState) }; + }; + + const applyEdit = async ( + editor: vscode.TextEditor, + range: vscode.Range, + text: string, + ): Promise => { + assert.ok(await editor.edit((builder) => builder.replace(range, text)), 'Edit failed'); + }; + + const R = (startLine: number, startChar: number, endLine: number, endChar: number) => + new vscode.Range(new Position(startLine, startChar), new Position(endLine, endChar)); + + const recordStep = async ( + historyTracker: HistoryTracker, + editor: vscode.TextEditor, + range: vscode.Range, + text: string, + ): Promise => { + await applyEdit(editor, range, text); + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + }; + + test('mirrors a native undo of the tip step instead of recording it', async () => { + const editor = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, R(0, 1, 0, 1), 'X'); + await recordStep(historyTracker, editor, R(0, 2, 0, 2), 'Y'); + assert.strictEqual(editor.document.getText(), 'aXYb'); + + await vscode.commands.executeCommand('undo'); + assert.strictEqual(editor.document.getText(), 'aXb'); + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Undo); + + // Mirrored: no new step is recorded ... + assert.strictEqual(historyTracker.addChange(), false); + + // ... so the next `u` undoes the older step instead of re-doing the undone one. + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'ab'); + }); + + test('mirrors consecutive native undos', async () => { + const editor = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, R(0, 1, 0, 1), 'X'); + await recordStep(historyTracker, editor, R(0, 2, 0, 2), 'Y'); + await recordStep(historyTracker, editor, R(0, 3, 0, 3), 'Z'); + assert.strictEqual(editor.document.getText(), 'aXYZb'); + + await vscode.commands.executeCommand('undo'); + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Undo); + await vscode.commands.executeCommand('undo'); + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Undo); + assert.strictEqual(editor.document.getText(), 'aXb'); + + assert.strictEqual(historyTracker.addChange(), false); + + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'ab'); + }); + + test('mirrors a native redo after a mirrored native undo', async () => { + const editor = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, R(0, 1, 0, 1), 'X'); + await recordStep(historyTracker, editor, R(0, 2, 0, 2), 'Y'); + + await vscode.commands.executeCommand('undo'); + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Undo); + assert.strictEqual(historyTracker.addChange(), false); + + await vscode.commands.executeCommand('redo'); + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Redo); + assert.strictEqual(editor.document.getText(), 'aXYb'); + assert.strictEqual(historyTracker.addChange(), false); + + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'aXb'); + }); + + test('falls back to recording when native undo is mixed with new edits', async () => { + const editor = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, R(0, 1, 0, 1), 'X'); + assert.strictEqual(editor.document.getText(), 'aXb'); + + await vscode.commands.executeCommand('undo'); + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Undo); + await applyEdit(editor, R(0, 0, 0, 0), 'Q'); + assert.strictEqual(editor.document.getText(), 'Qab'); + + // Not exactly our tip step being undone: record the net diff as before. + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'aXb'); + }); + + test('falls back to recording when the stack cannot explain the native undo', async () => { + const editor = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + // Edits the tracker never recorded (empty stack). + await applyEdit(editor, R(0, 1, 0, 1), 'X'); + await applyEdit(editor, R(0, 2, 0, 2), 'Y'); + await vscode.commands.executeCommand('undo'); + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Undo); + assert.strictEqual(editor.document.getText(), 'aXb'); + + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'ab'); + }); + + test('ignores changes without a native undo/redo reason', async () => { + const editor = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, R(0, 1, 0, 1), 'X'); + + historyTracker.noteNativeUndoRedo(undefined); + await applyEdit(editor, R(0, 2, 0, 2), 'Y'); + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'aXb'); + }); + + test('falls back to recording when the latch overflows', async () => { + const editor = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, R(0, 1, 0, 1), 'X'); + + for (let i = 0; i < 105; i++) { + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Undo); + } + await vscode.commands.executeCommand('undo'); + assert.strictEqual(editor.document.getText(), 'ab'); + + // The sequence is no longer trusted: record instead of mirroring. + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'aXb'); + }); + + test('defers the latch through insert mode and mirrors on flush', async () => { + const editor = await openUntitled('ab'); + const { vimState, historyTracker } = setupTracker(editor); + + await recordStep(historyTracker, editor, R(0, 1, 0, 1), 'X'); + + vimState.modeData = { mode: Mode.Insert, highSurrogate: undefined }; + await vscode.commands.executeCommand('undo'); + historyTracker.noteNativeUndoRedo(vscode.TextDocumentChangeReason.Undo); + assert.strictEqual(editor.document.getText(), 'ab'); + assert.strictEqual(historyTracker.addChange(), false); + + vimState.modeData = { mode: Mode.Normal }; + assert.strictEqual(historyTracker.addChange(), false); + + // Mirrored onto the stack: nothing left to undo. + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'ab'); + }); + + test('syncs the version when the text is unchanged', async () => { + const editor = await openUntitled('ab'); + const { historyTracker } = setupTracker(editor); + + // Bump the version twice with no net text change. + await applyEdit(editor, R(0, 1, 0, 1), 'X'); + await vscode.commands.executeCommand('undo'); + assert.strictEqual(editor.document.getText(), 'ab'); + + assert.strictEqual(historyTracker.addChange(), false); + const synced = historyTracker as unknown as { + previousDocumentState: { versionNumber: number }; + }; + assert.strictEqual(synced.previousDocumentState.versionNumber, editor.document.version); + }); + + test('wires native undo through the real onDidChangeTextDocument listener', async () => { + const document = await vscode.workspace.openTextDocument({ content: 'ab' }); + const editor = await vscode.window.showTextDocument(document); + const [modeHandler] = await ModeHandlerMap.getOrCreate(editor); + try { + const historyTracker = modeHandler.vimState.historyTracker; + + await applyEdit(editor, R(0, 1, 0, 1), 'X'); + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + await applyEdit(editor, R(0, 2, 0, 2), 'Y'); + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + assert.strictEqual(editor.document.getText(), 'aXYb'); + + // No direct latch call: the extension's listener must observe `reason === Undo`. + await vscode.commands.executeCommand('undo'); + assert.strictEqual(editor.document.getText(), 'aXb'); + assert.strictEqual(historyTracker.addChange(), false); + + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editor.document.getText(), 'ab'); + } finally { + ModeHandlerMap.delete(document.uri); + } + }); +}); + +suite('historyTracker document identity (#2007)', () => { + let sandbox: sinon.SinonSandbox; + + setup(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(StatusBar, 'setText'); + }); + + teardown(async () => { + sandbox.restore(); + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); + }); + + const openUntitled = async (content: string): Promise => { + const document = await vscode.workspace.openTextDocument({ content }); + return vscode.window.showTextDocument(document); + }; + + const applyEdit = async ( + editor: vscode.TextEditor, + range: vscode.Range, + text: string, + ): Promise => { + assert.ok(await editor.edit((builder) => builder.replace(range, text)), 'Edit failed'); + }; + + const R = (startLine: number, startChar: number, endLine: number, endChar: number) => + new vscode.Range(new Position(startLine, startChar), new Position(endLine, endChar)); + + test('keeps history when rebound to a document with identical text', async () => { + const editorA = await openUntitled('ab'); + const vimState = sandbox.createStubInstance(VimState) as unknown as VimState; + vimState.editor = editorA; + vimState.modeData = { mode: Mode.Normal }; + const historyTracker = new HistoryTracker(vimState); + + await applyEdit(editorA, R(0, 1, 0, 1), 'X'); + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + assert.strictEqual(editorA.document.getText(), 'aXb'); + + const editorB = await openUntitled('aXb'); + vimState.editor = editorB; + assert.strictEqual(historyTracker.addChange(), false); + + // Identical geometry: the recorded step still applies. + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editorB.document.getText(), 'ab'); + }); + + test('resets history instead of diffing across documents', async () => { + const editorA = await openUntitled('aaa'); + const vimState = sandbox.createStubInstance(VimState) as unknown as VimState; + vimState.editor = editorA; + vimState.modeData = { mode: Mode.Normal }; + const historyTracker = new HistoryTracker(vimState); + + await applyEdit(editorA, R(0, 0, 0, 3), 'XXX'); + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + + const editorB = await openUntitled('hello world'); + vimState.editor = editorB; + + // Must not record a delete-everything/insert-everything step ... + assert.strictEqual(historyTracker.addChange(), false); + + // ... so there is nothing to undo and the other file is untouched. + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editorB.document.getText(), 'hello world'); + + // And fresh history works from the new baseline. + await applyEdit(editorB, R(0, 5, 0, 11), '!'); + assert.strictEqual(historyTracker.addChange(), true); + historyTracker.finishCurrentStep(); + await historyTracker.goBackHistoryStep(); + assert.strictEqual(editorB.document.getText(), 'hello world'); + }); +}); + class TextEditorStub implements vscode.TextEditor { readonly document!: vscode.TextDocument; selection!: vscode.Selection; diff --git a/test/macro.test.ts b/test/macro.test.ts index 5e267791917..d7044a44a37 100644 --- a/test/macro.test.ts +++ b/test/macro.test.ts @@ -305,3 +305,30 @@ suite('Record and execute a macro', () => { }); }); }); + +suite('Undo/redo integrity after macros and remaps (#2007)', () => { + setup(async () => { + await setupWorkspace(); + }); + + newTest({ + title: 'Can undo and redo a macro that edits inside its own earlier edits', + start: ['|12'], + keysPressed: 'qaiABChhiXYq@au', + end: ['X|XYABCYABC12'], + }); + + newTestWithRemaps({ + title: 'Can undo and redo a remap that edits inside its own earlier edits', + start: ['|word'], + remaps: ['nmap X ciw[]P'], + steps: [ + { + keysPressed: 'Xu', + stepResult: { + end: ['|[word]'], + }, + }, + ], + }); +}); diff --git a/test/textChange.test.ts b/test/textChange.test.ts new file mode 100644 index 00000000000..7509df8bc5a --- /dev/null +++ b/test/textChange.test.ts @@ -0,0 +1,358 @@ +import { strict as assert } from 'assert'; +import * as vscode from 'vscode'; +import { Position, Range } from 'vscode'; + +import { + applyTextChanges, + coalesceAdjacentReplacePairs, + diffTextsToChanges, + mergeDocumentChanges, + offsetAtText, + unapplyTextChanges, + type ITextChange, +} from '../src/history/textChange'; + +suite('textChange', () => { + const P = (line: number, character: number): Position => new Position(line, character); + const C = (line: number, character: number, before: string, after: string): ITextChange => ({ + start: P(line, character), + before, + after, + }); + const shapeOf = (changes: readonly ITextChange[]): string => + changes + .map( + (c) => + `${c.start.line},${c.start.character}:${JSON.stringify(c.before)}=>${JSON.stringify(c.after)}`, + ) + .join(' | '); + + suite('offsetAtText', () => { + test('computes offsets in a single-line string', () => { + assert.strictEqual(offsetAtText('hello', P(0, 0)), 0); + assert.strictEqual(offsetAtText('hello', P(0, 3)), 3); + assert.strictEqual(offsetAtText('hello', P(0, 5)), 5); + }); + + test('computes offsets across LF line breaks', () => { + const text = 'ab\ncde\nf'; + assert.strictEqual(offsetAtText(text, P(0, 2)), 2); + assert.strictEqual(offsetAtText(text, P(1, 0)), 3); + assert.strictEqual(offsetAtText(text, P(1, 3)), 6); + assert.strictEqual(offsetAtText(text, P(2, 1)), 8); + }); + + test('counts CRLF as a two-unit line break', () => { + const text = 'ab\r\ncde\r\nf'; + assert.strictEqual(offsetAtText(text, P(0, 2)), 2); + assert.strictEqual(offsetAtText(text, P(1, 0)), 4); + assert.strictEqual(offsetAtText(text, P(1, 3)), 7); + assert.strictEqual(offsetAtText(text, P(2, 1)), 10); + }); + + test('matches TextDocument.offsetAt on real documents', async () => { + for (const content of ['hello', 'ab\ncde\nf', 'ab\r\ncde\r\nf', 'a\n\nb', '']) { + const document = await vscode.workspace.openTextDocument({ content }); + const text = document.getText(); + for (let line = 0; line < document.lineCount; line++) { + const lineLength = document.lineAt(line).text.length; + for (const character of [0, Math.floor(lineLength / 2), lineLength]) { + const position = P(line, character); + assert.strictEqual( + offsetAtText(text, position), + document.offsetAt(position), + `content=${JSON.stringify(content)} pos=${line},${character}`, + ); + } + } + } + }); + }); + + suite('applyTextChanges / unapplyTextChanges', () => { + test('applies a single insertion', () => { + assert.strictEqual(applyTextChanges('ab', [C(0, 1, '', 'XY')]), 'aXYb'); + }); + + test('applies a single deletion', () => { + assert.strictEqual(applyTextChanges('aXYb', [C(0, 1, 'XY', '')]), 'ab'); + }); + + test('applies a single replace', () => { + assert.strictEqual(applyTextChanges('foo', [C(0, 0, 'foo', 'bar')]), 'bar'); + }); + + test('applies multiline changes', () => { + assert.strictEqual(applyTextChanges('aaa\nbbb', [C(0, 1, 'aa\nb', 'X\nY\nZ')]), 'aX\nY\nZbb'); + }); + + test('unapply inverts apply', () => { + const initial = 'aaa\nbbb\nccc'; + const changes = [C(1, 1, 'b', 'XY'), C(0, 0, 'aaa\nbXYb', 'Q')]; + const current = applyTextChanges(initial, changes); + assert.strictEqual(current, 'Q\nccc'); + assert.strictEqual(unapplyTextChanges(current, changes), initial); + }); + }); + + suite('diffTextsToChanges', () => { + test('produces no changes for identical text', () => { + assert.deepStrictEqual(diffTextsToChanges('abc', 'abc'), []); + }); + + test('translates hunks into changes that replay the transition', () => { + const cases: Array<[string, string]> = [ + ['ab', 'aXYb'], + ['foo bar', 'XYZ bar'], + ['aaa\nbbb\nccc', 'Q\nccc'], + ['line1\nline2\nline3\n', 'line1\nline3\nline4\n'], + ['', 'whole new file\n'], + ['whole old file\n', ''], + ]; + for (const [oldText, newText] of cases) { + const changes = diffTextsToChanges(oldText, newText); + assert.strictEqual( + applyTextChanges(oldText, changes), + newText, + `old=${JSON.stringify(oldText)} new=${JSON.stringify(newText)}`, + ); + assert.strictEqual( + unapplyTextChanges(newText, changes), + oldText, + `old=${JSON.stringify(oldText)} new=${JSON.stringify(newText)}`, + ); + } + }); + }); + + suite('coalesceAdjacentReplacePairs', () => { + test('merges a delete plus an insert at the same position', () => { + assert.strictEqual( + shapeOf(coalesceAdjacentReplacePairs([C(0, 0, 'foo', ''), C(0, 0, '', 'XYZ')])), + '0,0:"foo"=>"XYZ"', + ); + }); + + test('leaves anything else untouched', () => { + const changes = [ + C(0, 0, '', 'AA'), + C(0, 5, '', 'BB'), // disjoint insert + C(0, 1, 'x', 'y'), // already a replace + ]; + assert.strictEqual(shapeOf(coalesceAdjacentReplacePairs(changes)), shapeOf(changes)); + }); + }); + + suite('mergeDocumentChanges', () => { + /** + * Asserts that merging `changes` (which must describe initial -> current) is + * correct in both directions, and — when `expectedShape` is given — that the + * merged list has exactly the expected canonical shape. + */ + const assertMerge = (initial: string, changes: ITextChange[], expectedShape?: string): void => { + const current = applyTextChanges(initial, changes); + const merged = mergeDocumentChanges(changes, current); + assert.strictEqual( + applyTextChanges(initial, merged), + current, + `redo broken: merged=[${shapeOf(merged)}]`, + ); + const reversed = merged + .map((c) => C(c.start.line, c.start.character, c.after, c.before)) + .reverse(); + assert.strictEqual( + applyTextChanges(current, reversed), + initial, + `undo broken: merged=[${shapeOf(merged)}]`, + ); + if (expectedShape !== undefined) { + assert.strictEqual(shapeOf(merged), expectedShape); + } + }; + + test('keeps a single change as-is', () => { + assertMerge('ab', [C(0, 1, '', 'X')], '0,1:""=>"X"'); + }); + + test('keeps disjoint changes separate', () => { + assertMerge( + '0123456789', + [C(0, 1, '', 'AA'), C(0, 5, '', 'BB')], + '0,1:""=>"AA" | 0,5:""=>"BB"', + ); + }); + + test('merges cw-like delete plus adjacent insert into one replace', () => { + assertMerge('foo bar', [C(0, 0, 'foo', ''), C(0, 0, '', 'XYZ')], '0,0:"foo"=>"XYZ"'); + }); + + // The cases below corrupted undo and/or redo with the old pairwise + // range-intersection merge (VSCodeVim/Vim#2007). + + test('merges a deletion strictly inside a previous insertion', () => { + assertMerge('12', [C(0, 1, '', 'abcdef'), C(0, 3, 'cde', '')], '0,1:""=>"abf"'); + }); + + test('merges an insertion inside a previous insertion', () => { + assertMerge('12', [C(0, 1, '', 'ABC'), C(0, 2, '', 'XY')], '0,1:""=>"AXYBC"'); + }); + + test('merges a replace of the head of a previous insertion', () => { + assertMerge('12', [C(0, 1, '', 'abcdef'), C(0, 1, 'ab', 'XY')], '0,1:""=>"XYcdef"'); + }); + + test('merges sequential inserts at the same position in order', () => { + assertMerge('ab', [C(0, 1, '', 'X'), C(0, 1, '', 'Y')], '0,1:""=>"YX"'); + }); + + test('merges an insertion followed by a superset deletion', () => { + assertMerge('12', [C(0, 1, '', 'ABC'), C(0, 0, '1ABC2', '')], '0,0:"12"=>""'); + }); + + test('merges overlapping multiline changes', () => { + assertMerge( + 'aaa\nbbb\nccc', + [C(1, 1, 'b', 'XY'), C(0, 0, 'aaa\nbXYb', 'Q')], + '0,0:"aaa\\nbbb"=>"Q"', + ); + }); + + test('merges longer chains of overlapping changes', () => { + assertMerge( + 'abcdef', + [C(0, 2, '', 'XX'), C(0, 3, 'Xc', ''), C(0, 1, 'b', 'ZZ')], + '0,1:"bc"=>"ZZX"', + ); + }); + + test('merges changes in CRLF documents', () => { + assertMerge('ab\r\ncd', [C(0, 1, '', 'X'), C(1, 1, 'd', 'Y')], '0,1:""=>"X" | 1,1:"d"=>"Y"'); + }); + + test('returns the input untouched when re-derivation cannot be validated', () => { + // Adversarial line-break soup (found by brute-forcing over CR/LF-heavy inputs): + // diff-walk positions split lines on LF only, so with lone CRs mixed in, the + // re-derived list cannot be mapped back to offsets and fails validation. Merging + // must then return the input untouched rather than a list spanning nowhere. + const initial = 'Zb\na\r\ncb'; + const current = 'Zb\rb\r\n\r\n'; + const changes = diffTextsToChanges(initial, current); + assert.ok(changes.length >= 2); + assert.strictEqual(shapeOf(mergeDocumentChanges(changes, current)), shapeOf(changes)); + }); + + test('round-trips random multi-change steps without corruption', () => { + // Deterministic PRNG so failures reproduce. + /* eslint-disable no-bitwise */ + const mulberry32 = (seed: number): (() => number) => { + let a = seed >>> 0; + return () => { + a |= 0; + a = (a + 0x6d2b79f5) | 0; + let t = Math.imul(a ^ (a >>> 15), 1 | a); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; + }; + /* eslint-enable no-bitwise */ + const alphabet = ['a', 'b', 'c', 'X', ' ', '\n']; + const positionOf = (text: string, offset: number): Position => { + let line = 0; + let character = 0; + for (let i = 0; i < offset; i++) { + if (text[i] === '\n') { + line++; + character = 0; + } else { + character++; + } + } + return P(line, character); + }; + + for (let iter = 0; iter < 300; iter++) { + const rng = mulberry32(iter * 2654435761 + 97); + const randInt = (n: number): number => Math.floor(rng() * n); + const randText = (maxLen: number): string => { + let s = ''; + for (let i = 0, n = randInt(maxLen); i < n; i++) { + s += alphabet[randInt(alphabet.length)]; + } + return s; + }; + + const initial = randText(14); + let current = initial; + const changes: ITextChange[] = []; + for (let e = 0, nEdits = 2 + randInt(3); e < nEdits; e++) { + const startOffset = randInt(current.length + 1); + const deleteLength = randInt(Math.min(5, current.length - startOffset) + 1); + const before = current.slice(startOffset, startOffset + deleteLength); + const after = randText(5); + if (before === '' && after === '') { + continue; + } + changes.push({ start: positionOf(current, startOffset), before, after }); + current = + current.slice(0, startOffset) + after + current.slice(startOffset + deleteLength); + } + if (changes.length < 2) { + continue; + } + + const merged = mergeDocumentChanges(changes, current); + assert.strictEqual( + applyTextChanges(initial, merged), + current, + `iter=${iter} redo broken: merged=[${shapeOf(merged)}]`, + ); + const reversed = merged + .map((c) => C(c.start.line, c.start.character, c.after, c.before)) + .reverse(); + assert.strictEqual( + applyTextChanges(current, reversed), + initial, + `iter=${iter} undo broken: merged=[${shapeOf(merged)}]`, + ); + } + }); + + test('merged lists apply cleanly through real editor edits', async () => { + // End-to-end shape check: drive the merged changes through actual `editor.edit` + // calls (which reject invalid ranges) in both directions. + const runThroughEditor = async (initial: string, changes: ITextChange[]): Promise => { + const current = applyTextChanges(initial, changes); + const merged = mergeDocumentChanges(changes, current); + const document = await vscode.workspace.openTextDocument({ content: initial }); + const editor = await vscode.window.showTextDocument(document); + try { + for (const change of merged) { + const range = new Range( + change.start, + change.start.advancePositionByText(change.before), + ); + assert.ok( + await editor.edit((builder) => builder.replace(range, change.after)), + `redo edit failed for [${shapeOf(merged)}]`, + ); + } + assert.strictEqual(editor.document.getText(), current); + for (const change of [...merged].reverse()) { + const range = new Range(change.start, change.start.advancePositionByText(change.after)); + assert.ok( + await editor.edit((builder) => builder.replace(range, change.before)), + `undo edit failed for [${shapeOf(merged)}]`, + ); + } + assert.strictEqual(editor.document.getText(), initial); + } finally { + await vscode.commands.executeCommand('workbench.action.closeActiveEditor'); + } + }; + + await runThroughEditor('12', [C(0, 1, '', 'ABC'), C(0, 2, '', 'XY')]); + await runThroughEditor('12', [C(0, 1, '', 'ABC'), C(0, 0, '1ABC2', '')]); + await runThroughEditor('foo bar', [C(0, 0, 'foo', ''), C(0, 0, '', 'XYZ')]); + }); + }); +});