Have the cursor selection change event contain more information needed for cursorUndo (#82535)

src/vs/editor/browser/widget/codeEditorWidget.ts

@@ -1371,6 +1371,9 @@ export class CodeEditorWidget extends Disposable implements editorBrowser.ICodeE
 			const e2: ICursorSelectionChangedEvent = {
 				selection: e.selections[0],
 				secondarySelections: e.selections.slice(1),
+				modelVersionId: e.modelVersionId,
+				oldSelections: e.oldSelections,
+				oldModelVersionId: e.oldModelVersionId,
 				source: e.source,
 				reason: e.reason
 			};

src/vs/editor/common/controller/cursor.ts

@@ -37,6 +37,18 @@ export class CursorStateChangedEvent {
 	 * The primary selection is always at index 0.
 	 */
 	readonly selections: Selection[];
+	/**
+	 * The new model version id that `selections` apply to.
+	 */
+	readonly modelVersionId: number;
+	/**
+	 * The old selections.
+	 */
+	readonly oldSelections: Selection[] | null;
+	/**
+	 * The model version id the that `oldSelections` apply to.
+	 */
+	readonly oldModelVersionId: number;
 	/**
 	 * Source of the call that caused the event.
 	 */
@@ -46,8 +58,11 @@ export class CursorStateChangedEvent {
 	 */
 	readonly reason: CursorChangeReason;
 
-	constructor(selections: Selection[], source: string, reason: CursorChangeReason) {
+	constructor(selections: Selection[], modelVersionId: number, oldSelections: Selection[] | null, oldModelVersionId: number, source: string, reason: CursorChangeReason) {
 		this.selections = selections;
+		this.modelVersionId = modelVersionId;
+		this.oldSelections = oldSelections;
+		this.oldModelVersionId = oldModelVersionId;
 		this.source = source;
 		this.reason = reason;
 	}
@@ -540,7 +555,9 @@ export class Cursor extends viewEvents.ViewEventEmitter implements ICursors {
 			|| oldState.cursorState.length !== newState.cursorState.length
 			|| newState.cursorState.some((newCursorState, i) => !newCursorState.modelState.equals(oldState.cursorState[i].modelState))
 		) {
-			this._onDidChange.fire(new CursorStateChangedEvent(selections, source || 'keyboard', reason));
+			const oldSelections = oldState ? oldState.cursorState.map(s => s.modelState.selection) : null;
+			const oldModelVersionId = oldState ? oldState.modelVersionId : 0;
+			this._onDidChange.fire(new CursorStateChangedEvent(selections, newState.modelVersionId, oldSelections, oldModelVersionId, source || 'keyboard', reason));
 		}
 
 		return true;

src/vs/editor/common/controller/cursorEvents.ts

@@ -72,6 +72,18 @@ export interface ICursorSelectionChangedEvent {
 	 * The secondary selections.
 	 */
 	readonly secondarySelections: Selection[];
+	/**
+	 * The model version id.
+	 */
+	readonly modelVersionId: number;
+	/**
+	 * The old selections.
+	 */
+	readonly oldSelections: Selection[] | null;
+	/**
+	 * The model version id the that `oldSelections` refer to.
+	 */
+	readonly oldModelVersionId: number;
 	/**
 	 * Source of the call that caused the event.
 	 */

src/vs/editor/contrib/cursorUndo/cursorUndo.ts

@@ -48,7 +48,6 @@ export class CursorUndoRedoController extends Disposable implements IEditorContr
 
 	private _undoStack: CursorState[];
 	private _redoStack: CursorState[];
-	private _prevState: CursorState | null;
 
 	constructor(editor: ICodeEditor) {
 		super();
@@ -57,38 +56,36 @@ export class CursorUndoRedoController extends Disposable implements IEditorContr
 
 		this._undoStack = [];
 		this._redoStack = [];
-		this._prevState = null;
 
 		this._register(editor.onDidChangeModel((e) => {
 			this._undoStack = [];
 			this._redoStack = [];
-			this._prevState = null;
 		}));
 		this._register(editor.onDidChangeModelContent((e) => {
 			this._undoStack = [];
 			this._redoStack = [];
-			this._prevState = null;
-			this._pushStateIfNecessary();
 		}));
-		this._register(editor.onDidChangeCursorSelection(() => this._pushStateIfNecessary()));
-	}
-
-	private _pushStateIfNecessary(): void {
-		const newState = new CursorState(this._editor.getSelections()!);
-
-		if (!this._isCursorUndoRedo && this._prevState) {
-			const isEqualToLastUndoStack = (this._undoStack.length > 0 && this._undoStack[this._undoStack.length - 1].equals(this._prevState));
+		this._register(editor.onDidChangeCursorSelection((e) => {
+			if (this._isCursorUndoRedo) {
+				return;
+			}
+			if (!e.oldSelections) {
+				return;
+			}
+			if (e.oldModelVersionId !== e.modelVersionId) {
+				return;
+			}
+			const prevState = new CursorState(e.oldSelections);
+			const isEqualToLastUndoStack = (this._undoStack.length > 0 && this._undoStack[this._undoStack.length - 1].equals(prevState));
 			if (!isEqualToLastUndoStack) {
-				this._undoStack.push(this._prevState);
+				this._undoStack.push(prevState);
 				this._redoStack = [];
 				if (this._undoStack.length > 50) {
 					// keep the cursor undo stack bounded
 					this._undoStack.shift();
 				}
 			}
-		}
-
-		this._prevState = newState;
+		}));
 	}
 
 	public cursorUndo(): void {

src/vs/editor/contrib/cursorUndo/test/cursorUndo.test.ts

@@ -0,0 +1,63 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import { Selection } from 'vs/editor/common/core/selection';
+import { withTestCodeEditor } from 'vs/editor/test/browser/testCodeEditor';
+import { CursorUndo, CursorUndoRedoController } from 'vs/editor/contrib/cursorUndo/cursorUndo';
+import { Handler } from 'vs/editor/common/editorCommon';
+import { CoreNavigationCommands, CoreEditingCommands } from 'vs/editor/browser/controller/coreCommands';
+
+suite('FindController', () => {
+
+	const cursorUndoAction = new CursorUndo();
+
+	test('issue #82535: Edge case with cursorUndo', () => {
+		withTestCodeEditor([
+			''
+		], {}, (editor) => {
+
+			editor.registerAndInstantiateContribution(CursorUndoRedoController.ID, CursorUndoRedoController);
+
+			// type hello
+			editor.trigger('test', Handler.Type, { text: 'hello' });
+
+			// press left
+			CoreNavigationCommands.CursorLeft.runEditorCommand(null, editor, {});
+
+			// press Delete
+			CoreEditingCommands.DeleteRight.runEditorCommand(null, editor, {});
+			assert.deepEqual(editor.getValue(), 'hell');
+			assert.deepEqual(editor.getSelections(), [new Selection(1, 5, 1, 5)]);
+
+			// press left
+			CoreNavigationCommands.CursorLeft.runEditorCommand(null, editor, {});
+			assert.deepEqual(editor.getSelections(), [new Selection(1, 4, 1, 4)]);
+
+			// press Ctrl+U
+			cursorUndoAction.run(null!, editor, {});
+			assert.deepEqual(editor.getSelections(), [new Selection(1, 5, 1, 5)]);
+		});
+	});
+
+	test('issue #82535: Edge case with cursorUndo (reverse)', () => {
+		withTestCodeEditor([
+			''
+		], {}, (editor) => {
+
+			editor.registerAndInstantiateContribution(CursorUndoRedoController.ID, CursorUndoRedoController);
+
+			// type hello
+			editor.trigger('test', Handler.Type, { text: 'hell' });
+			editor.trigger('test', Handler.Type, { text: 'o' });
+			assert.deepEqual(editor.getValue(), 'hello');
+			assert.deepEqual(editor.getSelections(), [new Selection(1, 6, 1, 6)]);
+
+			// press Ctrl+U
+			cursorUndoAction.run(null!, editor, {});
+			assert.deepEqual(editor.getSelections(), [new Selection(1, 6, 1, 6)]);
+		});
+	});
+});

src/vs/monaco.d.ts

@@ -2396,6 +2396,18 @@ declare namespace monaco.editor {
 		 * The secondary selections.
 		 */
 		readonly secondarySelections: Selection[];
+		/**
+		 * The model version id.
+		 */
+		readonly modelVersionId: number;
+		/**
+		 * The old selections.
+		 */
+		readonly oldSelections: Selection[] | null;
+		/**
+		 * The model version id the that `oldSelections` refer to.
+		 */
+		readonly oldModelVersionId: number;
 		/**
 		 * Source of the call that caused the event.
 		 */