From befa00e5b2463520e89eabeec41f58acfca09ecc Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Wed, 23 Oct 2019 11:37:50 +0200 Subject: [PATCH] Have the cursor selection change event contain more information needed for cursorUndo (#82535) --- .../editor/browser/widget/codeEditorWidget.ts | 3 + src/vs/editor/common/controller/cursor.ts | 21 ++++++- .../editor/common/controller/cursorEvents.ts | 12 ++++ .../editor/contrib/cursorUndo/cursorUndo.ts | 31 +++++---- .../cursorUndo/test/cursorUndo.test.ts | 63 +++++++++++++++++++ src/vs/monaco.d.ts | 12 ++++ 6 files changed, 123 insertions(+), 19 deletions(-) create mode 100644 src/vs/editor/contrib/cursorUndo/test/cursorUndo.test.ts diff --git a/src/vs/editor/browser/widget/codeEditorWidget.ts b/src/vs/editor/browser/widget/codeEditorWidget.ts index 5bb0bef9be7..f81ecfa145f 100644 --- a/src/vs/editor/browser/widget/codeEditorWidget.ts +++ b/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 }; diff --git a/src/vs/editor/common/controller/cursor.ts b/src/vs/editor/common/controller/cursor.ts index dafbf41eb7e..80e20c21d7f 100644 --- a/src/vs/editor/common/controller/cursor.ts +++ b/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; diff --git a/src/vs/editor/common/controller/cursorEvents.ts b/src/vs/editor/common/controller/cursorEvents.ts index a873c2ed70c..eb3418b993f 100644 --- a/src/vs/editor/common/controller/cursorEvents.ts +++ b/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. */ diff --git a/src/vs/editor/contrib/cursorUndo/cursorUndo.ts b/src/vs/editor/contrib/cursorUndo/cursorUndo.ts index 5d26aae2665..aecd7026498 100644 --- a/src/vs/editor/contrib/cursorUndo/cursorUndo.ts +++ b/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 { diff --git a/src/vs/editor/contrib/cursorUndo/test/cursorUndo.test.ts b/src/vs/editor/contrib/cursorUndo/test/cursorUndo.test.ts new file mode 100644 index 00000000000..308484142e9 --- /dev/null +++ b/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)]); + }); + }); +}); diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 18e0a9abe18..46e4158c083 100644 --- a/src/vs/monaco.d.ts +++ b/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. */