From aac0b166463f4a2eeae17cbe1ca5dbe7aeaff2e1 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Fri, 4 May 2018 16:47:41 +0200 Subject: [PATCH] Fixes #23539 --- src/vs/editor/common/model.ts | 17 +- src/vs/editor/common/model/editStack.ts | 157 ++++++++++++++---- src/vs/editor/common/model/textModel.ts | 15 ++ .../common/services/modelServiceImpl.ts | 2 +- .../editor/contrib/format/formattingEdit.ts | 4 +- .../test/browser/controller/cursor.test.ts | 19 +++ src/vs/monaco.d.ts | 14 +- .../api/electron-browser/mainThreadEditor.ts | 4 +- .../browser/parts/editor/editorStatus.ts | 2 +- .../electron-browser/bulkEditService.ts | 2 +- 10 files changed, 185 insertions(+), 51 deletions(-) diff --git a/src/vs/editor/common/model.ts b/src/vs/editor/common/model.ts index fcd2b5ed666..e4a6ba3feed 100644 --- a/src/vs/editor/common/model.ts +++ b/src/vs/editor/common/model.ts @@ -596,11 +596,6 @@ export interface ITextModel { */ getEOL(): string; - /** - * Change the end of line sequence used in the text buffer. - */ - setEOL(eol: EndOfLineSequence): void; - /** * Get the minimum legal column for line at `lineNumber` */ @@ -1008,6 +1003,12 @@ export interface ITextModel { */ pushEditOperations(beforeCursorState: Selection[], editOperations: IIdentifiedSingleEditOperation[], cursorStateComputer: ICursorStateComputer): Selection[]; + /** + * Change the end of line sequence. This is the preferred way of + * changing the eol sequence. This will land on the undo stack. + */ + pushEOL(eol: EndOfLineSequence): void; + /** * Edit the model without adding the edits to the undo stack. * This can have dire consequences on the undo stack! See @pushEditOperations for the preferred way. @@ -1016,6 +1017,12 @@ export interface ITextModel { */ applyEdits(operations: IIdentifiedSingleEditOperation[]): IIdentifiedSingleEditOperation[]; + /** + * Change the end of line sequence without recording in the undo stack. + * This can have dire consequences on the undo stack! See @pushEOL for the preferred way. + */ + setEOL(eol: EndOfLineSequence): void; + /** * Undo edit operations until the first previous stop point created by `pushStackElement`. * The inverse edit operations will be pushed on the redo stack. diff --git a/src/vs/editor/common/model/editStack.ts b/src/vs/editor/common/model/editStack.ts index e05b7ad76b2..13a955cf522 100644 --- a/src/vs/editor/common/model/editStack.ts +++ b/src/vs/editor/common/model/editStack.ts @@ -5,7 +5,7 @@ 'use strict'; import { onUnexpectedError } from 'vs/base/common/errors'; -import { ICursorStateComputer, IIdentifiedSingleEditOperation } from 'vs/editor/common/model'; +import { ICursorStateComputer, IIdentifiedSingleEditOperation, EndOfLineSequence } from 'vs/editor/common/model'; import { Selection } from 'vs/editor/common/core/selection'; import { TextModel } from 'vs/editor/common/model/textModel'; @@ -14,13 +14,86 @@ interface IEditOperation { } interface IStackElement { - beforeVersionId: number; - beforeCursorState: Selection[]; + readonly beforeVersionId: number; + readonly beforeCursorState: Selection[]; + readonly afterCursorState: Selection[]; + readonly afterVersionId: number; - editOperations: IEditOperation[]; + undo(model: TextModel): void; + redo(model: TextModel): void; +} - afterCursorState: Selection[]; - afterVersionId: number; +class EditStackElement implements IStackElement { + public readonly beforeVersionId: number; + public readonly beforeCursorState: Selection[]; + public afterCursorState: Selection[]; + public afterVersionId: number; + + public editOperations: IEditOperation[]; + + constructor(beforeVersionId: number, beforeCursorState: Selection[]) { + this.beforeVersionId = beforeVersionId; + this.beforeCursorState = beforeCursorState; + this.afterCursorState = null; + this.afterVersionId = -1; + this.editOperations = []; + } + + public undo(model: TextModel): void { + // Apply all operations in reverse order + for (let i = this.editOperations.length - 1; i >= 0; i--) { + this.editOperations[i] = { + operations: model.applyEdits(this.editOperations[i].operations) + }; + } + } + + public redo(model: TextModel): void { + // Apply all operations + for (let i = 0; i < this.editOperations.length; i++) { + this.editOperations[i] = { + operations: model.applyEdits(this.editOperations[i].operations) + }; + } + } +} + +function getModelEOL(model: TextModel): EndOfLineSequence { + const eol = model.getEOL(); + if (eol === '\n') { + return EndOfLineSequence.LF; + } else { + return EndOfLineSequence.CRLF; + } +} + +class EOLStackElement implements IStackElement { + public readonly beforeVersionId: number; + public readonly beforeCursorState: Selection[]; + public readonly afterCursorState: Selection[]; + public afterVersionId: number; + + public eol: EndOfLineSequence; + + constructor(beforeVersionId: number, setEOL: EndOfLineSequence) { + this.beforeVersionId = beforeVersionId; + this.beforeCursorState = null; + this.afterCursorState = null; + this.afterVersionId = -1; + this.eol = setEOL; + } + + public undo(model: TextModel): void { + let redoEOL = getModelEOL(model); + model.setEOL(this.eol); + this.eol = redoEOL; + } + + public redo(model: TextModel): void { + let undoEOL = getModelEOL(model); + model.setEOL(this.eol); + this.eol = undoEOL; + } } export interface IUndoRedoResult { @@ -55,34 +128,60 @@ export class EditStack { this.future = []; } + public pushEOL(eol: EndOfLineSequence): void { + // No support for parallel universes :( + this.future = []; + + if (this.currentOpenStackElement) { + this.pushStackElement(); + } + + const prevEOL = getModelEOL(this.model); + let stackElement = new EOLStackElement(this.model.getAlternativeVersionId(), prevEOL); + + this.model.setEOL(eol); + + stackElement.afterVersionId = this.model.getVersionId(); + this.currentOpenStackElement = stackElement; + this.pushStackElement(); + } + public pushEditOperation(beforeCursorState: Selection[], editOperations: IIdentifiedSingleEditOperation[], cursorStateComputer: ICursorStateComputer): Selection[] { // No support for parallel universes :( this.future = []; + let stackElement: EditStackElement = null; + + if (this.currentOpenStackElement) { + if (this.currentOpenStackElement instanceof EditStackElement) { + stackElement = this.currentOpenStackElement; + } else { + this.pushStackElement(); + } + } + if (!this.currentOpenStackElement) { - this.currentOpenStackElement = { - beforeVersionId: this.model.getAlternativeVersionId(), - beforeCursorState: beforeCursorState, - editOperations: [], - afterCursorState: null, - afterVersionId: -1 - }; + stackElement = new EditStackElement(this.model.getAlternativeVersionId(), beforeCursorState); + this.currentOpenStackElement = stackElement; } const inverseEditOperation: IEditOperation = { operations: this.model.applyEdits(editOperations) }; - this.currentOpenStackElement.editOperations.push(inverseEditOperation); + stackElement.editOperations.push(inverseEditOperation); + stackElement.afterCursorState = EditStack._computeCursorState(cursorStateComputer, inverseEditOperation.operations); + stackElement.afterVersionId = this.model.getVersionId(); + return stackElement.afterCursorState; + } + + private static _computeCursorState(cursorStateComputer: ICursorStateComputer, inverseEditOperations: IIdentifiedSingleEditOperation[]): Selection[] { try { - this.currentOpenStackElement.afterCursorState = cursorStateComputer ? cursorStateComputer(inverseEditOperation.operations) : null; + return cursorStateComputer ? cursorStateComputer(inverseEditOperations) : null; } catch (e) { onUnexpectedError(e); - this.currentOpenStackElement.afterCursorState = null; + return null; } - - this.currentOpenStackElement.afterVersionId = this.model.getVersionId(); - return this.currentOpenStackElement.afterCursorState; } public undo(): IUndoRedoResult { @@ -93,13 +192,9 @@ export class EditStack { const pastStackElement = this.past.pop(); try { - // Apply all operations in reverse order - for (let i = pastStackElement.editOperations.length - 1; i >= 0; i--) { - pastStackElement.editOperations[i] = { - operations: this.model.applyEdits(pastStackElement.editOperations[i].operations) - }; - } + pastStackElement.undo(this.model); } catch (e) { + onUnexpectedError(e); this.clear(); return null; } @@ -118,20 +213,12 @@ export class EditStack { public redo(): IUndoRedoResult { if (this.future.length > 0) { - if (this.currentOpenStackElement) { - throw new Error('How is this possible?'); - } - const futureStackElement = this.future.pop(); try { - // Apply all operations - for (let i = 0; i < futureStackElement.editOperations.length; i++) { - futureStackElement.editOperations[i] = { - operations: this.model.applyEdits(futureStackElement.editOperations[i].operations) - }; - } + futureStackElement.redo(this.model); } catch (e) { + onUnexpectedError(e); this.clear(); return null; } diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts index 57a734fcd6e..aac822b74af 100644 --- a/src/vs/editor/common/model/textModel.ts +++ b/src/vs/editor/common/model/textModel.ts @@ -1143,6 +1143,21 @@ export class TextModel extends Disposable implements model.ITextModel { this._commandManager.pushStackElement(); } + public pushEOL(eol: model.EndOfLineSequence): void { + const currentEOL = (this.getEOL() === '\n' ? model.EndOfLineSequence.LF : model.EndOfLineSequence.CRLF); + if (currentEOL === eol) { + return; + } + try { + this._onDidChangeDecorations.beginDeferredEmit(); + this._eventEmitter.beginDeferredEmit(); + this._commandManager.pushEOL(eol); + } finally { + this._eventEmitter.endDeferredEmit(); + this._onDidChangeDecorations.endDeferredEmit(); + } + } + public pushEditOperations(beforeCursorState: Selection[], editOperations: model.IIdentifiedSingleEditOperation[], cursorStateComputer: model.ICursorStateComputer): Selection[] { try { this._onDidChangeDecorations.beginDeferredEmit(); diff --git a/src/vs/editor/common/services/modelServiceImpl.ts b/src/vs/editor/common/services/modelServiceImpl.ts index 4d5123785ab..d77dff222ac 100644 --- a/src/vs/editor/common/services/modelServiceImpl.ts +++ b/src/vs/editor/common/services/modelServiceImpl.ts @@ -418,7 +418,7 @@ export class ModelServiceImpl implements IModelService { // Otherwise find a diff between the values and update model model.pushStackElement(); - model.setEOL(textBuffer.getEOL() === '\r\n' ? EndOfLineSequence.CRLF : EndOfLineSequence.LF); + model.pushEOL(textBuffer.getEOL() === '\r\n' ? EndOfLineSequence.CRLF : EndOfLineSequence.LF); model.pushEditOperations( [], ModelServiceImpl._computeEdits(model, textBuffer), diff --git a/src/vs/editor/contrib/format/formattingEdit.ts b/src/vs/editor/contrib/format/formattingEdit.ts index 449a3fd5c64..4b11fd4eb56 100644 --- a/src/vs/editor/contrib/format/formattingEdit.ts +++ b/src/vs/editor/contrib/format/formattingEdit.ts @@ -26,7 +26,7 @@ export class FormattingEdit { } if (typeof newEol === 'number') { - editor.getModel().setEOL(newEol); + editor.getModel().pushEOL(newEol); } return singleEdits; @@ -40,8 +40,8 @@ export class FormattingEdit { } static execute(editor: ICodeEditor, _edits: TextEdit[]) { - let edits = FormattingEdit._handleEolEdits(editor, _edits); editor.pushUndoStop(); + let edits = FormattingEdit._handleEolEdits(editor, _edits); if (edits.length === 1 && FormattingEdit._isFullModelReplaceEdit(editor, edits[0])) { // We use replace semantics and hope that markers stay put... editor.executeEdits('formatEditsCommand', edits.map(edit => EditOperation.replace(Range.lift(edit.range), edit.text))); diff --git a/src/vs/editor/test/browser/controller/cursor.test.ts b/src/vs/editor/test/browser/controller/cursor.test.ts index 97612efe284..a66dc6fbcd9 100644 --- a/src/vs/editor/test/browser/controller/cursor.test.ts +++ b/src/vs/editor/test/browser/controller/cursor.test.ts @@ -1214,6 +1214,25 @@ suite('Editor Controller - Regression tests', () => { model.dispose(); }); + test('issue #23539: Setting model EOL isn\'t undoable', () => { + usingCursor({ + text: [ + 'Hello', + 'world' + ] + }, (model, cursor) => { + assertCursor(cursor, new Position(1, 1)); + model.setEOL(EndOfLineSequence.LF); + assert.equal(model.getValue(), 'Hello\nworld'); + + model.pushEOL(EndOfLineSequence.CRLF); + assert.equal(model.getValue(), 'Hello\r\nworld'); + + cursorCommand(cursor, H.Undo); + assert.equal(model.getValue(), 'Hello\nworld'); + }); + }); + test('issue #47733: Undo mangles unicode characters', () => { const languageId = new LanguageIdentifier('myMode', 3); class MyMode extends MockMode { diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 953562ce66b..17a205c0f2d 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -1505,10 +1505,6 @@ declare namespace monaco.editor { * @return EOL char sequence (e.g.: '\n' or '\r\n'). */ getEOL(): string; - /** - * Change the end of line sequence used in the text buffer. - */ - setEOL(eol: EndOfLineSequence): void; /** * Get the minimum legal column for line at `lineNumber` */ @@ -1744,6 +1740,11 @@ declare namespace monaco.editor { * @return The cursor state returned by the `cursorStateComputer`. */ pushEditOperations(beforeCursorState: Selection[], editOperations: IIdentifiedSingleEditOperation[], cursorStateComputer: ICursorStateComputer): Selection[]; + /** + * Change the end of line sequence. This is the preferred way of + * changing the eol sequence. This will land on the undo stack. + */ + pushEOL(eol: EndOfLineSequence): void; /** * Edit the model without adding the edits to the undo stack. * This can have dire consequences on the undo stack! See @pushEditOperations for the preferred way. @@ -1751,6 +1752,11 @@ declare namespace monaco.editor { * @return The inverse edit operations, that, when applied, will bring the model back to the previous state. */ applyEdits(operations: IIdentifiedSingleEditOperation[]): IIdentifiedSingleEditOperation[]; + /** + * Change the end of line sequence without recording in the undo stack. + * This can have dire consequences on the undo stack! See @pushEOL for the preferred way. + */ + setEOL(eol: EndOfLineSequence): void; /** * An event emitted when the contents of the model have changed. * @event diff --git a/src/vs/workbench/api/electron-browser/mainThreadEditor.ts b/src/vs/workbench/api/electron-browser/mainThreadEditor.ts index a938797cc00..377fed7a784 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadEditor.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadEditor.ts @@ -448,9 +448,9 @@ export class MainThreadTextEditor { } if (opts.setEndOfLine === EndOfLine.CRLF) { - this._model.setEOL(EndOfLineSequence.CRLF); + this._model.pushEOL(EndOfLineSequence.CRLF); } else if (opts.setEndOfLine === EndOfLine.LF) { - this._model.setEOL(EndOfLineSequence.LF); + this._model.pushEOL(EndOfLineSequence.LF); } let transformedEdits = edits.map((edit): IIdentifiedSingleEditOperation => { diff --git a/src/vs/workbench/browser/parts/editor/editorStatus.ts b/src/vs/workbench/browser/parts/editor/editorStatus.ts index 6476c972704..223738793be 100644 --- a/src/vs/workbench/browser/parts/editor/editorStatus.ts +++ b/src/vs/workbench/browser/parts/editor/editorStatus.ts @@ -1122,7 +1122,7 @@ export class ChangeEOLAction extends Action { const editorWidget = getEditorWidget(activeEditor); if (editorWidget && isWritableCodeEditor(editorWidget)) { const textModel = editorWidget.getModel(); - textModel.setEOL(eol.eol); + textModel.pushEOL(eol.eol); } } }); diff --git a/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts b/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts index 291e49fbb73..f075ff345d5 100644 --- a/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts +++ b/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts @@ -109,7 +109,7 @@ class EditTask implements IDisposable { } if (this._newEol !== undefined) { this._model.pushStackElement(); - this._model.setEOL(this._newEol); + this._model.pushEOL(this._newEol); this._model.pushStackElement(); } }