diff --git a/apps/docs/app/(diffs)/docs/Editor/constants.ts b/apps/docs/app/(diffs)/docs/Editor/constants.ts index 222fa9565..cc01bf071 100644 --- a/apps/docs/app/(diffs)/docs/Editor/constants.ts +++ b/apps/docs/app/(diffs)/docs/Editor/constants.ts @@ -775,22 +775,15 @@ editor.cleanUp(); editor.cleanUp(true); // Apply text edits to the attached document. Positions are zero-based. -// Pass true as the second argument to push the edits onto the undo stack. +// Edits always join the undo stack, exactly like typed input. The optional +// updateHistory argument defaults to true; false remaps live selections instead +// of restoring snapshots but keeps the text edit undoable. editor.applyEdits([ { range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }, newText: 'Hello, world!', }, ]); -editor.applyEdits( - [ - { - range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }, - newText: 'Hello, world!', - }, - ], - true -); // Live FileContents for the attached document. Undefined when nothing is // attached. diff --git a/apps/docs/app/(diffs)/docs/Editor/content.mdx b/apps/docs/app/(diffs)/docs/Editor/content.mdx index 2f8225dd6..a527af307 100644 --- a/apps/docs/app/(diffs)/docs/Editor/content.mdx +++ b/apps/docs/app/(diffs)/docs/Editor/content.mdx @@ -210,11 +210,16 @@ while you edit. Pass an empty array to clear them. #### History -The editor keeps an undo stack for user edits and for programmatic edits applied -with `applyEdits(..., true)`. Use `editor.canUndo` and `editor.canRedo` to -enable or disable toolbar buttons, and call `editor.undo()` or `editor.redo()` -to step through history from code. These methods share the same stack as the -keyboard shortcuts (Cmd/Ctrl-Z and Cmd/Ctrl-Shift-Z). +The editor keeps a single undo stack covering user edits and programmatic edits +alike — `applyEdits` joins the same timeline as typed input, so a mixed sequence +undoes exactly like an all-local one. Its optional `updateHistory` argument +defaults to `true`; passing `false` remaps live selections instead of restoring +snapshots but keeps the text edit undoable. Use `editor.canUndo` and +`editor.canRedo` to enable or disable toolbar buttons, and call `editor.undo()` +or `editor.redo()` to step through history from code. These methods share the +same stack as the keyboard shortcuts (Cmd/Ctrl-Z and + +Cmd/Ctrl-Shift-Z). `undo()` and `redo()` are no-ops when there is nothing to undo or redo. Both run through the same change path as a normal edit, so your `onChange` handler fires diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index e18803b89..54c1de3fb 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -447,9 +447,16 @@ export class Editor implements DiffsEditor { } /** - * Apply edits to current attached file. + * Apply edits to current attached file. Every edit joins the undo timeline: + * a programmatic edit must leave the document and its history exactly as + * the same edit typed by the user would (history equivalence — see + * TextDocument.applyResolvedEdits), so it is undoable like any other edit. + * + * @param updateHistory Whether to record caller selection snapshots for + * exact undo/redo restoration. Defaults to true. When false, live selections + * are remapped during replay and the text edit still joins the undo timeline. */ - applyEdits(edits: TextEdit[], updateHistory = false): void { + applyEdits(edits: TextEdit[], updateHistory = true): void { const textDocument = this.#textDocument; if (textDocument == null) { throw new Error('Editor is not attached'); @@ -2446,25 +2453,55 @@ export class Editor implements DiffsEditor { break; case 'undo': - if (this.#textDocument?.canUndo === true) { - const undoResult = this.#textDocument.undo(); - if (undoResult !== undefined) { - this.#applyChange(...undoResult); - } - } - break; - case 'redo': - if (this.#textDocument?.canRedo === true) { - const redoResult = this.#textDocument.redo(); - if (redoResult !== undefined) { - this.#applyChange(...redoResult); - } - } + this.#applyHistoryChange(command); break; } } + // Replays history and remaps live selections when the entry intentionally + // has no stored selection metadata. + #applyHistoryChange(command: 'undo' | 'redo'): void { + const textDocument = this.#textDocument; + if (textDocument === undefined) { + return; + } + if ( + (command === 'undo' && !textDocument.canUndo) || + (command === 'redo' && !textDocument.canRedo) + ) { + return; + } + const selections = this.#selections; + const selectionOffsets = selections?.map( + (selection) => + [ + textDocument.offsetAt(selection.start), + textDocument.offsetAt(selection.end), + ] as const + ); + const result = + command === 'undo' ? textDocument.undo() : textDocument.redo(); + if (result === undefined) { + return; + } + const [change, recordedSelections, lineAnnotations, selectionEdits] = + result; + const nextSelections = + recordedSelections ?? + (selections !== undefined && + selectionOffsets !== undefined && + selectionEdits !== undefined + ? remapSelectionsAfterEdits( + textDocument, + selections, + selectionOffsets, + selectionEdits + ) + : undefined); + this.#applyChange(change, nextSelections, lineAnnotations); + } + /** Applies one undoable command batch and records its resulting selections. */ #applyCommandEdits( edits: TextEdit[], @@ -4105,7 +4142,7 @@ export class Editor implements DiffsEditor { get selection(): EditorSelection { return getActiveSelection(); }, - applyEdits: (edits: TextEdit[]) => this.applyEdits(edits, true), + applyEdits: (edits: TextEdit[]) => this.applyEdits(edits), getSelectionText: () => this.#textDocument?.getText(getActiveSelection()) ?? '', replaceSelectionText: (text: string) => { diff --git a/packages/diffs/src/editor/textDocument.ts b/packages/diffs/src/editor/textDocument.ts index fc5ae3824..52fa17369 100644 --- a/packages/diffs/src/editor/textDocument.ts +++ b/packages/diffs/src/editor/textDocument.ts @@ -60,6 +60,15 @@ export interface TextDocumentChange { ][]; } +// Metadata-less replay results include the resolved edits so Editor can remap +// its live selections without storing a snapshot on the history entry. +type TextDocumentHistoryResult = [ + change: TextDocumentChange, + selections?: EditorSelection[], + lineAnnotations?: DiffLineAnnotation[], + selectionEdits?: ResolvedTextEdit[], +]; + /** * A vscode-languageserver-textdocument compatible text document. */ @@ -199,7 +208,7 @@ export class TextDocument { applyEdits( edits: TextEdit[], - updateHistory = false, + updateHistory = true, selectionsBefore?: EditorSelection[], selectionsAfter?: EditorSelection[], undoBoundary = false @@ -225,7 +234,7 @@ export class TextDocument { applyResolvedEdits( edits: ResolvedTextEdit[], - updateHistory = false, + updateHistory = true, selectionsBefore?: EditorSelection[], selectionsAfter?: EditorSelection[], undoBoundary = false @@ -244,8 +253,16 @@ export class TextDocument { ); } - // Shared mutation path after public edit coordinates are normalized and - // validated. Undo and redo apply their stored inverse offsets separately. + // Every edit joins the undo timeline: an edit applied with + // `updateHistory=false` affects the edit stack exactly as the identical + // call with `updateHistory=true` and no selections or undo boundary would. + // The flag only controls whether the caller's interaction metadata is + // recorded on the entry; the entry itself is always pushed (clearing any + // pending redo) and coalesces by the normal geometry rules. This keeps a + // mixed tracked/untracked sequence equivalent to the all-tracked sequence — + // undo-to-exhaustion restores the original text, redo-to-exhaustion the + // final text — instead of leaving frozen entries whose offsets go stale. + // Only history replay (undo/redo) writes to the buffer without recording. #applyResolvedEdits( resolvedEdits: ResolvedTextEdit[], updateHistory: boolean, @@ -253,35 +270,30 @@ export class TextDocument { selectionsAfter: EditorSelection[] | undefined, undoBoundary: boolean ): TextDocumentChange { - if (updateHistory) { - const entry = createEditStackEntry( - this, - resolvedEdits, - this.#version, - this.#version + 1, - selectionsBefore, - selectionsAfter - ); - if (undoBoundary) { - entry.undoBoundary = true; - } - const previousEntry = this.#editStack.peekUndo(); - const change = this.#applyResolvedEditsToBuffer(resolvedEdits); - this.#version++; - if ( - change.lineDelta === 0 && - shouldCoalesceEditStackEntry(previousEntry, entry) - ) { - this.#editStack.replaceLastUndo( - coalesceEditStackEntries(previousEntry!, entry) - ); - } else { - this.#editStack.push(entry); - } - return change; + const entry = createEditStackEntry( + this, + resolvedEdits, + this.#version, + this.#version + 1, + updateHistory ? selectionsBefore : undefined, + updateHistory ? selectionsAfter : undefined + ); + if (updateHistory && undoBoundary) { + entry.undoBoundary = true; } + const previousEntry = this.#editStack.peekUndo(); const change = this.#applyResolvedEditsToBuffer(resolvedEdits); this.#version++; + if ( + change.lineDelta === 0 && + shouldCoalesceEditStackEntry(previousEntry, entry) + ) { + this.#editStack.replaceLastUndo( + coalesceEditStackEntries(previousEntry!, entry) + ); + } else { + this.#editStack.push(entry); + } return change; } @@ -299,13 +311,7 @@ export class TextDocument { ); } - undo(): - | [ - change: TextDocumentChange, - selections?: EditorSelection[], - lineAnnotations?: DiffLineAnnotation[], - ] - | undefined { + undo(): TextDocumentHistoryResult | undefined { const entry = this.#editStack.popUndoToRedo(); if (entry === undefined) { return undefined; @@ -315,20 +321,18 @@ export class TextDocument { return undefined; } this.#version = entry.versionBefore; + const selections = entry.selectionsBefore?.slice(); return [ change, - entry.selectionsBefore?.slice(), + selections, entry.lineAnnotationsBefore?.slice(), + selections === undefined + ? entry.inverseEdits.map((edit) => ({ ...edit })) + : undefined, ]; } - redo(): - | [ - change: TextDocumentChange, - selections?: EditorSelection[], - lineAnnotations?: DiffLineAnnotation[], - ] - | undefined { + redo(): TextDocumentHistoryResult | undefined { const entry = this.#editStack.popRedoToUndo(); if (entry === undefined) { return undefined; @@ -338,10 +342,14 @@ export class TextDocument { return undefined; } this.#version = entry.versionAfter; + const selections = entry.selectionsAfter?.slice(); return [ change, - entry.selectionsAfter?.slice(), + selections, entry.lineAnnotationsAfter?.slice(), + selections === undefined + ? entry.forwardEdits.map((edit) => ({ ...edit })) + : undefined, ]; } diff --git a/packages/diffs/test/README.md b/packages/diffs/test/README.md index 815492cd8..25b52f73f 100644 --- a/packages/diffs/test/README.md +++ b/packages/diffs/test/README.md @@ -112,18 +112,3 @@ order. If you touch one of these areas, consider adding the missing coverage: and backspace followed by forward-delete at the same pivot coalesces into a single undo step instead of getting an undo stop when the delete direction flips. -- **History equivalence across non-history edits** (7) — edits applied with - `updateHistory=false` are an implementation detail of how an edit reaches - `applyEdits`, not a separate semantic class: a mixed programmatic/local - sequence must leave history equivalent to the same sequence applied all-local, - so undo-to-exhaustion restores the original byte-exact text and - redo-to-exhaustion the final text. Today untracked edits bypass the edit stack - and existing entries apply at stale offsets, so exhaustion corrupts instead: a - stale-offset undo deletes the wrong characters, a replacement batch breaks - across an interleaved untracked insert, an untracked interior insert is erased - while tracked text is stranded, an untracked whole-document replace produces - spliced states that never existed on any timeline, typing coalesced around an - untracked insert unwinds to the untracked remainder instead of the original - text, the unwind that should restore recorded selections verbatim never - reaches the original text, and a pending redo survives an untracked edit - (instead of being cleared like any new edit) and replays at stale offsets. diff --git a/packages/diffs/test/editorApplyEdits.test.ts b/packages/diffs/test/editorApplyEdits.test.ts index da2b9bc54..db8250edc 100644 --- a/packages/diffs/test/editorApplyEdits.test.ts +++ b/packages/diffs/test/editorApplyEdits.test.ts @@ -598,7 +598,7 @@ describe('Editor.applyEdits selection sync', () => { } }); - test('restores the remapped caret on redo when history is updated', async () => { + test('restores the remapped caret on redo with default history tracking', async () => { const { cleanup, content, editor, window } = await createEditorFixture( 'alpha\nbravo\ncharlie' ); @@ -612,18 +612,15 @@ describe('Editor.applyEdits selection sync', () => { }, ]); - editor.applyEdits( - [ - { - range: { - start: { line: 0, character: 0 }, - end: { line: 0, character: 0 }, - }, - newText: 'NEW\n', + editor.applyEdits([ + { + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, }, - ], - true - ); + newText: 'NEW\n', + }, + ]); pressUndoRedo(window, content, false); expect(editor.getText()).toBe('alpha\nbravo\ncharlie'); @@ -651,6 +648,51 @@ describe('Editor.applyEdits selection sync', () => { } }); + test('remaps live selections when history metadata is disabled', async () => { + const { cleanup, editor } = await createEditorFixture( + 'alpha\nbravo\ncharlie' + ); + + try { + editor.setSelections([ + { + start: { line: 2, character: 3 }, + end: { line: 2, character: 3 }, + direction: 'none', + }, + ]); + editor.applyEdits( + [ + { + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + }, + newText: 'NEW\n', + }, + ], + false + ); + + editor.undo(); + expect(editor.getText()).toBe('alpha\nbravo\ncharlie'); + expect(editor.getState().selections).toEqual([caret(2, 3)]); + + editor.setSelections([ + { + start: { line: 0, character: 2 }, + end: { line: 0, character: 2 }, + direction: 'none', + }, + ]); + editor.redo(); + expect(editor.getText()).toBe('NEW\nalpha\nbravo\ncharlie'); + expect(editor.getState().selections).toEqual([caret(1, 2)]); + } finally { + cleanup(); + } + }); + test('does not steal focus when the editor is not focused', async () => { const { cleanup, content, editor } = await createEditorFixture( 'alpha\nbravo\ncharlie' diff --git a/packages/diffs/test/editorEditStack.test.ts b/packages/diffs/test/editorEditStack.test.ts index 08b583f40..dbe1d6ccc 100644 --- a/packages/diffs/test/editorEditStack.test.ts +++ b/packages/diffs/test/editorEditStack.test.ts @@ -818,318 +818,288 @@ function localEdit( ); } -// A non-history edit: updateHistory=false, exactly what Editor.applyEdits -// defaults to for programmatic/remote changes. +// A non-history edit: updateHistory=false, the TextDocument-level shape of a +// programmatic/remote change. It still joins the undo timeline (history +// equivalence), just without selection/undo-boundary metadata. function remoteEdit( d: ReturnType, startCharacter: number, endCharacter: number, newText: string ) { - d.applyEdits([lineEdit(startCharacter, endCharacter, newText)]); + d.applyEdits([lineEdit(startCharacter, endCharacter, newText)], false); } // Undo/redo interacting with non-history edits (updateHistory=false). // -// DESIGN MODEL (equivalence): applying an edit with updateHistory=false is an -// implementation detail of how the edit reaches applyEdits (the default for -// Editor.applyEdits — collaborative patches, programmatic fixes, etc.), not a -// separate semantic class of edit. A mixed tracked/untracked sequence must -// leave the document and its history in a state equivalent to the same -// sequence applied all-tracked. The observable contract — chosen because it -// sidesteps per-step grouping ambiguity — is exhaustion: after any edit -// sequence, undo-to-exhaustion restores the ORIGINAL byte-exact text and -// redo-to-exhaustion restores the FINAL text, no matter which edits skipped -// history tracking. Where a per-step value is asserted, it is the value the -// all-tracked reference run of the same script produces (probed with -// undoBoundary=true per edit to defeat coalescing noise); the literals below -// are embedded from those reference runs. The previously pinned rebasing -// model — untracked edits survive undo while frozen history entries remap -// around them — is rejected. -// -// KNOWN BUG shared by every test.failing below: untracked edits bypass the -// edit stack entirely and existing entries are never reconciled with them, so -// traversal strands or erases untracked text and applies inverse/forward -// edits at stale offsets, corrupting the buffer instead of reproducing the -// all-tracked timeline. Two passing tests in this describe pin today's -// stopgap behavior in the geometries where it happens not to corrupt (the -// after-the-tracked-range baseline and the stack-liveness test); they predate -// the equivalence model and will need flipping when it lands. +// DESIGN MODEL (equivalence, implemented in TextDocument.applyResolvedEdits): +// applying an edit with updateHistory=false is an implementation detail of +// how the edit reaches applyEdits (the shape of programmatic/remote changes — +// collaborative patches, codemod fixes, etc.), not a separate semantic class +// of edit. Every edit joins the undo timeline: an untracked edit +// affects the edit stack exactly as the identical tracked call with no +// selections and no undo boundary would — it gets an entry (clearing any +// pending redo), coalesces by the normal geometry rules, and is unwound by +// undo and replayed by redo like any other entry, never rebased around. A +// mixed tracked/untracked sequence therefore leaves the document and its +// history in a state equivalent to the same sequence applied all-tracked. +// The headline contract — chosen because it sidesteps per-step grouping +// ambiguity — is exhaustion: after any edit sequence, undo-to-exhaustion +// restores the ORIGINAL byte-exact text and redo-to-exhaustion restores the +// FINAL text, no matter which edits skipped history tracking. Where a +// per-step value is asserted, it is the value the all-tracked reference run +// of the same script produces; the literals below are embedded from those +// reference runs. The rejected alternative — untracked edits survive undo +// while frozen history entries remap around them (rebasing) — is pinned +// against explicitly (the interior-insert and whole-document-replace tests). describe('undo/redo across non-history edits', () => { - // TODAY'S BEHAVIOR, not an equivalence pin: an untracked edit strictly - // AFTER the tracked range leaves the entry's stored offsets valid, so the - // single undo happens to restore exactly the tracked change without - // corrupting anything. Under the equivalence model even this geometry - // changes — the all-tracked reference unwinds the suffix first - // ("sour lemon", then "lemon") — so this pin documents the one arrangement - // where today's frozen entries stay coherent, and will need flipping when - // equivalence lands. - test('undo works when the non-history edit sits after the tracked range', () => { + // An untracked edit after the tracked range is its own timeline entry + // sitting above the tracked one (insert at 10 is not adjacent to the + // tracked insert's end at 5, so no coalescing), and LIFO order unwinds it + // first — matching the all-tracked reference "sour lemon tart" -> + // "sour lemon" -> "lemon". Previously this geometry pinned the old bypass + // model (a single undo that skipped the untracked suffix). + test('an untracked edit after the tracked range is its own undo step above it', () => { const d = doc('lemon'); localEdit(d, 0, 0, 'sour '); // tracked: "sour lemon" remoteEdit(d, 10, 10, ' tart'); // remote suffix: "sour lemon tart" + expect(d.getText()).toBe('sour lemon tart'); + d.undo(); + expect(d.getText()).toBe('sour lemon'); + expect(d.canUndo).toBe(true); + expect(d.canRedo).toBe(true); d.undo(); - expect(d.getText()).toBe('lemon tart'); + expect(d.getText()).toBe('lemon'); expect(d.canUndo).toBe(false); expect(d.canRedo).toBe(true); d.redo(); + expect(d.getText()).toBe('sour lemon'); + d.redo(); expect(d.getText()).toBe('sour lemon tart'); + expect(d.canRedo).toBe(false); }); - // KNOWN BUG: the equivalence expectation is that the two untracked inserts - // are part of the timeline, so exhaustion reproduces the all-tracked - // reference ("syncpilot?" -> "syncpilot" -> "pilot" -> "" and back). - // Actual today: the untracked inserts never enter history and the lone - // tracked entry's inverse applies at its stale offsets, deleting the - // untracked prefix plus part of the shifted typed text — undo-to-exhaustion - // yields "ilot?" and redo-to-exhaustion compounds it to "pilotilot?". - test.failing( - 'a mixed tracked/untracked insert sequence unwinds and replays like the all-tracked timeline', - () => { - const d = doc(''); - localEdit(d, 0, 0, 'pilot'); // tracked typing - remoteEdit(d, 0, 0, 'sync'); // untracked prefix: "syncpilot" - remoteEdit(d, 9, 9, '?'); // untracked suffix: "syncpilot?" - expect(d.getText()).toBe('syncpilot?'); - - // Undo-to-exhaustion restores the original byte-exact text... - undoAll(d); - expect(d.getText()).toBe(''); + // The two untracked inserts are part of the timeline, so exhaustion + // reproduces the all-tracked reference ("syncpilot?" -> "syncpilot" -> + // "pilot" -> "" and back); geometry blocks coalescing at both steps. + test('a mixed tracked/untracked insert sequence unwinds and replays like the all-tracked timeline', () => { + const d = doc(''); + localEdit(d, 0, 0, 'pilot'); // tracked typing + remoteEdit(d, 0, 0, 'sync'); // untracked prefix: "syncpilot" + remoteEdit(d, 9, 9, '?'); // untracked suffix: "syncpilot?" + expect(d.getText()).toBe('syncpilot?'); - // ...and redo-to-exhaustion restores the final text. - redoAll(d); - expect(d.getText()).toBe('syncpilot?'); - } - ); + // Undo-to-exhaustion restores the original byte-exact text... + undoAll(d); + expect(d.getText()).toBe(''); - // KNOWN BUG: the equivalence expectation is that the untracked insert is - // one more logical step, so exhaustion reproduces the all-tracked reference - // ("UV####WXYZ" -> "UVWXYZ" -> "pqr" and back). Actual today: the batch - // entry's chained inverse offsets ignore the untracked insert that landed - // between its sub-edits, so undo treats the untracked text as if it were - // the tracked replacements — undo-to-exhaustion yields "pqrWXYZ" and - // redo-to-exhaustion "UVWXYZWXYZ". - test.failing( - 'a replacement batch with an interleaved untracked insert unwinds and replays like the all-tracked timeline', - () => { - const d = doc('pqr'); - // One tracked batch of three adjacent replacements. - d.applyEdits( - [lineEdit(0, 1, 'UV'), lineEdit(1, 2, 'WX'), lineEdit(2, 3, 'YZ')], - true, - [caretAt(0, 3)] - ); - expect(d.getText()).toBe('UVWXYZ'); + // ...and redo-to-exhaustion restores the final text. + redoAll(d); + expect(d.getText()).toBe('syncpilot?'); + }); - // Untracked insert exactly between the first and second replacements. - remoteEdit(d, 2, 2, '####'); - expect(d.getText()).toBe('UV####WXYZ'); + // The untracked insert is one more logical step (it cannot coalesce with a + // three-edit batch), so exhaustion reproduces the all-tracked reference + // ("UV####WXYZ" -> "UVWXYZ" -> "pqr" and back) — the untracked text is + // unwound before the batch inverse applies, keeping its offsets valid. + test('a replacement batch with an interleaved untracked insert unwinds and replays like the all-tracked timeline', () => { + const d = doc('pqr'); + // One tracked batch of three adjacent replacements. + d.applyEdits( + [lineEdit(0, 1, 'UV'), lineEdit(1, 2, 'WX'), lineEdit(2, 3, 'YZ')], + true, + [caretAt(0, 3)] + ); + expect(d.getText()).toBe('UVWXYZ'); - // Undo-to-exhaustion restores the original byte-exact text — the - // untracked insert is unwound too, exactly as if it had been tracked. - undoAll(d); - expect(d.getText()).toBe('pqr'); + // Untracked insert exactly between the first and second replacements. + remoteEdit(d, 2, 2, '####'); + expect(d.getText()).toBe('UV####WXYZ'); - // Redo-to-exhaustion restores the final text. - redoAll(d); - expect(d.getText()).toBe('UV####WXYZ'); - } - ); + // Undo-to-exhaustion restores the original byte-exact text — the + // untracked insert is unwound too, exactly as if it had been tracked. + undoAll(d); + expect(d.getText()).toBe('pqr'); - // KNOWN BUG: under the equivalence model the untracked interior insert - // does NOT survive undo — it is one of the undo steps like any other edit, - // and exhaustion reproduces the all-tracked reference ("WXjYZ" -> "WXYZ" -> - // "" and back). Actual today: the tracked insertion's stale inverse [0,4) - // is deleted from "WXjYZ" wholesale, erasing the untracked "j" and - // stranding a tracked "Z" — undo-to-exhaustion yields "Z" and - // redo-to-exhaustion "WXYZZ". - test.failing( - 'an untracked insert inside a tracked insertion unwinds and replays like the all-tracked timeline', - () => { - const d = doc(''); - localEdit(d, 0, 0, 'WXYZ'); // tracked insertion - remoteEdit(d, 2, 2, 'j'); // untracked insert in the middle of it - expect(d.getText()).toBe('WXjYZ'); + // Redo-to-exhaustion restores the final text. + redoAll(d); + expect(d.getText()).toBe('UV####WXYZ'); + }); - undoAll(d); - expect(d.getText()).toBe(''); + // The untracked interior insert does NOT survive undo — it is one of the + // undo steps like any other edit (rebasing it around the tracked entry is + // the rejected model), and exhaustion reproduces the all-tracked reference + // ("WXjYZ" -> "WXYZ" -> "" and back). + test('an untracked insert inside a tracked insertion unwinds and replays like the all-tracked timeline', () => { + const d = doc(''); + localEdit(d, 0, 0, 'WXYZ'); // tracked insertion + remoteEdit(d, 2, 2, 'j'); // untracked insert in the middle of it + expect(d.getText()).toBe('WXjYZ'); - redoAll(d); - expect(d.getText()).toBe('WXjYZ'); - } - ); + undoAll(d); + expect(d.getText()).toBe(''); - // KNOWN BUG: an untracked replace that wipes the region a tracked insertion - // lives in must read as one more logical step: the all-tracked reference - // unwinds "core" -> "oGHk" -> "ok" and replays "oGHk" -> "core", and no - // state along the way mixes the two texts. Actual today: the frozen entry - // points at text that no longer exists, so undo deletes unrelated - // characters ("core" -> "ce") and redo splices the tracked insert into the - // replacement text ("cGHe") — a corruption artifact that never existed on - // any timeline. - test.failing( - 'an untracked whole-document replace over a tracked insertion unwinds and replays like the all-tracked timeline', - () => { - const d = doc('ok'); - localEdit(d, 1, 1, 'GH'); // tracked insert: "oGHk" - remoteEdit(d, 0, 4, 'core'); // untracked replace of the whole doc - expect(d.getText()).toBe('core'); - - const visited: string[] = []; - while (d.canUndo) { - d.undo(); - visited.push(d.getText()); - } - expect(d.getText()).toBe('ok'); + redoAll(d); + expect(d.getText()).toBe('WXjYZ'); + }); - while (d.canRedo) { - d.redo(); - visited.push(d.getText()); - } - expect(d.getText()).toBe('core'); + // An untracked replace that wipes the region a tracked insertion lives in + // reads as one more logical step: the all-tracked reference unwinds + // "core" -> "oGHk" -> "ok" and replays "oGHk" -> "core", and no state along + // the way mixes the two texts — the strongest anti-corruption pin, since a + // mixture like "cGHe" never existed on any timeline. + test('an untracked whole-document replace over a tracked insertion unwinds and replays like the all-tracked timeline', () => { + const d = doc('ok'); + localEdit(d, 1, 1, 'GH'); // tracked insert: "oGHk" + remoteEdit(d, 0, 4, 'core'); // untracked replace of the whole doc + expect(d.getText()).toBe('core'); + + const visited: string[] = []; + while (d.canUndo) { + d.undo(); + visited.push(d.getText()); + } + expect(d.getText()).toBe('ok'); - // Every state the traversal visits must be one the all-tracked - // timeline visits — no mixtures like "cGHe". - for (const state of visited) { - expect(['ok', 'oGHk', 'core']).toContain(state); - } + while (d.canRedo) { + d.redo(); + visited.push(d.getText()); } - ); + expect(d.getText()).toBe('core'); - // KNOWN BUG: under the equivalence model stored selections need no - // remapping at all — by the time the tracked delete's entry is undone, the - // untracked insert has itself been unwound, so the document is back in the - // exact coordinate space the selections were recorded in and they restore - // verbatim. The all-tracked reference unwinds " worXYld" -> " world" -> - // "hello world" with the final undo returning carets [6, 11] unchanged. - // Actual today: the selections half already matches (they come back - // verbatim), but the text half fails — the untracked insert is never - // unwound, so the single undo reinserts "hello" at a stale offset and - // exhaustion ends at "hello worXYld" instead of "hello world". - test.failing( - 'undo exhaustion across an untracked insert restores the original text and the verbatim recorded selections', - () => { - const d = doc('hello world'); - // Tracked delete of the leading word, with two carets recorded. - d.applyEdits([lineEdit(0, 5, '')], true, [caretAt(0, 6), caretAt(0, 11)]); - expect(d.getText()).toBe(' world'); - - // Untracked insert; in the original coordinates this lands at offset 9, - // between the two stored carets. - remoteEdit(d, 4, 4, 'XY'); - expect(d.getText()).toBe(' worXYld'); - - let lastUndo: ReturnType; - while (d.canUndo) { - lastUndo = d.undo(); - } - expect(d.getText()).toBe('hello world'); - // Mirrors the all-tracked reference: the final undo hands back the - // entry's stored selectionsBefore untouched. - expect(lastUndo?.[1]?.map((s) => s.start.character)).toEqual([6, 11]); + // Every state the traversal visits must be one the all-tracked + // timeline visits — no mixtures like "cGHe". + for (const state of visited) { + expect(['ok', 'oGHk', 'core']).toContain(state); + } + }); - redoAll(d); - expect(d.getText()).toBe(' worXYld'); + // Stored selections need no remapping at all — by the time the tracked + // delete's entry is undone, the untracked insert has itself been unwound, + // so the document is back in the exact coordinate space the selections + // were recorded in and they restore verbatim. The all-tracked reference + // unwinds " worXYld" -> " world" -> "hello world" with the final undo + // returning carets [6, 11] unchanged. + test('undo exhaustion across an untracked insert restores the original text and the verbatim recorded selections', () => { + const d = doc('hello world'); + // Tracked delete of the leading word, with two carets recorded. + d.applyEdits([lineEdit(0, 5, '')], true, [caretAt(0, 6), caretAt(0, 11)]); + expect(d.getText()).toBe(' world'); + + // Untracked insert; in the original coordinates this lands at offset 9, + // between the two stored carets. + remoteEdit(d, 4, 4, 'XY'); + expect(d.getText()).toBe(' worXYld'); + + let lastUndo: ReturnType; + while (d.canUndo) { + lastUndo = d.undo(); } - ); + expect(d.getText()).toBe('hello world'); + // Mirrors the all-tracked reference: the final undo hands back the + // entry's stored selectionsBefore untouched. + expect(lastUndo?.[1]?.map((s) => s.start.character)).toEqual([6, 11]); - // KNOWN BUG (was a passing pin under the old rebasing model, where undo - // "keeping the untracked text" looked correct by geometric accident): - // under the equivalence model the untracked "b" is part of the timeline - // like any other edit, so undo-to-exhaustion restores the original (empty) - // text — not an untracked remainder. The all-tracked run of this script - // unwinds "acb" -> "ab" -> "" and replays "" -> "ab" -> "acb" (the "b" - // coalesces into the "a" keystroke; "c" lands inside the merged insert and - // starts a new step); however the mixed run groups its steps, exhaustion - // must hit the same endpoints. Actual today: the two tracked keystrokes - // coalesce into one entry whose single undo leaves exactly "b", and - // canUndo goes false with the untracked text stranded as if it were - // original. - test.failing( - 'typing around an untracked insert unwinds to the original text, not to an untracked remainder', - () => { - const d = doc(''); - localEdit(d, 0, 0, 'a'); // tracked keystroke: "a" - remoteEdit(d, 1, 1, 'b'); // untracked: "ab" - localEdit(d, 1, 1, 'c'); // tracked keystroke right after the "a": "acb" - expect(d.getText()).toBe('acb'); - - // Undo-to-exhaustion restores the original byte-exact text... - undoAll(d); - expect(d.getText()).toBe(''); + redoAll(d); + expect(d.getText()).toBe(' worXYld'); + }); - // ...and redo-to-exhaustion restores the final text. - redoAll(d); - expect(d.getText()).toBe('acb'); - } - ); + // The untracked "b" is part of the timeline like any other edit — and + // subject to NORMAL typing coalescing, not a forced boundary — so + // undo-to-exhaustion restores the original (empty) text, not an untracked + // remainder. The all-tracked run of this script unwinds "acb" -> "ab" -> "" + // and replays "" -> "ab" -> "acb" (the "b" coalesces into the "a" + // keystroke; "c" lands inside the merged insert and starts a new step); + // however the mixed run groups its steps, exhaustion must hit the same + // endpoints. + test('typing around an untracked insert unwinds to the original text, not to an untracked remainder', () => { + const d = doc(''); + localEdit(d, 0, 0, 'a'); // tracked keystroke: "a" + remoteEdit(d, 1, 1, 'b'); // untracked: "ab" + localEdit(d, 1, 1, 'c'); // tracked keystroke right after the "a": "acb" + expect(d.getText()).toBe('acb'); + + // Undo-to-exhaustion restores the original byte-exact text... + undoAll(d); + expect(d.getText()).toBe(''); + + // ...and redo-to-exhaustion restores the final text. + redoAll(d); + expect(d.getText()).toBe('acb'); + }); - // TODAY'S BEHAVIOR, not an equivalence pin: untracked edits currently touch - // neither stack, so undo entries survive and a pending redo is not cleared. - // Geometry keeps every untracked edit after the tracked offsets so the - // surviving entries also APPLY cleanly today. Under the equivalence model - // an untracked edit is a new edit like any other — it joins the undo - // timeline and clears a pending redo (the failing pin below) — so this pin - // documents the stopgap and will need flipping when equivalence lands. - test('a non-history edit neither consumes undo entries nor clears the redo stack', () => { + // An untracked edit is a new edit like any other: it joins the undo + // timeline (here coalescing into the tracked keystroke by the normal + // insert-adjacency rule, exactly as the all-tracked reference does) and, + // while a redo entry is pending, pushing it clears the redo stack — the + // parked entry never replays anywhere. Previously pinned the old bypass + // model, where untracked edits touched neither stack. + test('a non-history edit joins the undo timeline and clears the redo stack', () => { const d = doc('alpha'); localEdit(d, 5, 5, '!'); // tracked: "alpha!" expect(d.canUndo).toBe(true); expect(d.canRedo).toBe(false); + // Starts exactly where the tracked insert ends, so it coalesces into the + // "!" entry like continued typing would. remoteEdit(d, 6, 6, ' beta'); // "alpha! beta" + expect(d.getText()).toBe('alpha! beta'); expect(d.canUndo).toBe(true); expect(d.canRedo).toBe(false); + // One coalesced step unwinds both inserts. d.undo(); - expect(d.getText()).toBe('alpha beta'); + expect(d.getText()).toBe('alpha'); expect(d.canUndo).toBe(false); expect(d.canRedo).toBe(true); - // A non-history edit while a redo entry is pending keeps it alive. - remoteEdit(d, 10, 10, '?'); // "alpha beta?" - expect(d.canRedo).toBe(true); + // A non-history edit while a redo entry is pending pushes a new entry + // and clears the redo — the parked "! beta" entry is gone for good. + remoteEdit(d, 5, 5, '?'); // "alpha?" + expect(d.getText()).toBe('alpha?'); + expect(d.canUndo).toBe(true); + expect(d.canRedo).toBe(false); - d.redo(); - expect(d.getText()).toBe('alpha! beta?'); + d.redo(); // no-op: nothing left to redo + expect(d.getText()).toBe('alpha?'); expect(d.canUndo).toBe(true); expect(d.canRedo).toBe(false); + + expect(undoAll(d)).toBe(1); + expect(d.getText()).toBe('alpha'); + redoAll(d); + expect(d.getText()).toBe('alpha?'); }); - // KNOWN BUG: under the equivalence model the untracked ">> " insert is an - // ordinary new edit, and a new edit while a redo is pending clears the redo - // stack (the same rule tracked edits already follow). The all-tracked + // The untracked ">> " insert is an ordinary new edit, and a new edit while + // a redo is pending clears the redo stack (the same rule tracked edits + // already follow) — the parked "!" entry never replays anywhere, so its + // forward edit can never land at a stale offset. Matches the all-tracked // reference run: canRedo goes false after the ">> " edit, redo() is a // no-op at ">> note", undo-to-exhaustion yields "note", redo-to-exhaustion - // ">> note". Actual today: the pending redo entry survives the untracked - // edit and replays its forward edit at a stale offset, landing the "!" - // mid-word (">> n!ote"), and undo-to-exhaustion from there strands the - // untracked prefix at ">> note" instead of returning to "note". - test.failing( - 'an untracked edit while a redo is pending behaves like a tracked edit and clears the redo', - () => { - const d = doc('note'); - localEdit(d, 4, 4, '!'); // tracked: "note!" - d.undo(); - expect(d.getText()).toBe('note'); - expect(d.canRedo).toBe(true); + // ">> note". + test('an untracked edit while a redo is pending behaves like a tracked edit and clears the redo', () => { + const d = doc('note'); + localEdit(d, 4, 4, '!'); // tracked: "note!" + d.undo(); + expect(d.getText()).toBe('note'); + expect(d.canRedo).toBe(true); - remoteEdit(d, 0, 0, '>> '); // untracked prefix while redo is pending - expect(d.getText()).toBe('>> note'); + remoteEdit(d, 0, 0, '>> '); // untracked prefix while redo is pending + expect(d.getText()).toBe('>> note'); - // Mirrors the all-tracked reference: pushing a new edit clears redo, - // so the pending "!" never replays anywhere. - expect(d.canRedo).toBe(false); - d.redo(); - expect(d.getText()).toBe('>> note'); + // Mirrors the all-tracked reference: pushing a new edit clears redo, + // so the pending "!" never replays anywhere. + expect(d.canRedo).toBe(false); + d.redo(); + expect(d.getText()).toBe('>> note'); - // Exhaustion in both directions matches the reference timeline. - undoAll(d); - expect(d.getText()).toBe('note'); - redoAll(d); - expect(d.getText()).toBe('>> note'); - } - ); + // Exhaustion in both directions matches the reference timeline. + undoAll(d); + expect(d.getText()).toBe('note'); + redoAll(d); + expect(d.getText()).toBe('>> note'); + }); }); function insertEdit( diff --git a/packages/diffs/test/editorPublicApi.test.ts b/packages/diffs/test/editorPublicApi.test.ts index 9665df03a..90f9f855c 100644 --- a/packages/diffs/test/editorPublicApi.test.ts +++ b/packages/diffs/test/editorPublicApi.test.ts @@ -74,7 +74,7 @@ function insertText( line: number, character: number, text: string, - updateHistory = false + updateHistory = true ): void { editor.applyEdits( [ diff --git a/packages/diffs/test/editorTextDocument.test.ts b/packages/diffs/test/editorTextDocument.test.ts index 5e1e1e766..4b56cff77 100644 --- a/packages/diffs/test/editorTextDocument.test.ts +++ b/packages/diffs/test/editorTextDocument.test.ts @@ -1142,9 +1142,14 @@ describe('TextDocument', () => { expect(d.canUndo).toBe(false); }); - test('applyEdits default does not record undo', () => { + // History equivalence: applyEdits defaults to recording caller metadata as + // well as an undoable entry. Passing updateHistory=false still joins the + // timeline (see 'undo/redo across non-history edits' in + // editorEditStack.test.ts); it only omits that metadata. + test('applyEdits defaults to updating history metadata', () => { const d = doc('a'); - d.applyEdits([ + const selectionsBefore = [caret(0, 1)]; + const edits = [ { range: { start: { line: 0, character: 1 }, @@ -1152,10 +1157,20 @@ describe('TextDocument', () => { }, newText: 'b', }, - ]); + ]; + d.applyEdits(edits, undefined, selectionsBefore); expect(d.getText()).toBe('ab'); - expect(d.canUndo).toBe(false); - expect(d.undo()).toBeUndefined(); + expect(d.canUndo).toBe(true); + const undoResult = d.undo(); + expect(d.getText()).toBe('a'); + expect(undoResult?.[1]).toEqual(selectionsBefore); + d.redo(); + expect(d.getText()).toBe('ab'); + + const withoutMetadata = doc('a'); + withoutMetadata.applyEdits(edits, false, selectionsBefore); + expect(withoutMetadata.undo()?.[1]).toBeUndefined(); + expect(withoutMetadata.getText()).toBe('a'); }); test('undo and redo', () => {