diff --git a/src/vs/editor/common/core/position.ts b/src/vs/editor/common/core/position.ts index ef06653bf0506b44965fb02d0b6afb99bc4d1459..ae173c66c68be25c69bcc2dccf50e483f22fb3c2 100644 --- a/src/vs/editor/common/core/position.ts +++ b/src/vs/editor/common/core/position.ts @@ -21,23 +21,29 @@ export class Position implements IEditorPosition { } public isBefore(other:IPosition): boolean { - if (this.lineNumber < other.lineNumber) { + return Position.isBefore(this, other); + } + public static isBefore(a:IPosition, b:IPosition): boolean { + if (a.lineNumber < b.lineNumber) { return true; } - if (other.lineNumber < this.lineNumber) { + if (b.lineNumber < a.lineNumber) { return false; } - return this.column < other.column; + return a.column < b.column; } public isBeforeOrEqual(other:IPosition): boolean { - if (this.lineNumber < other.lineNumber) { + return Position.isBeforeOrEqual(this, other); + } + public static isBeforeOrEqual(a:IPosition, b:IPosition): boolean { + if (a.lineNumber < b.lineNumber) { return true; } - if (other.lineNumber < this.lineNumber) { + if (b.lineNumber < a.lineNumber) { return false; } - return this.column <= other.column; + return a.column <= b.column; } public clone(): Position { diff --git a/src/vs/editor/common/model/editableTextModel.ts b/src/vs/editor/common/model/editableTextModel.ts index 888d082855d9de3e6559b202420c31e44aa6dc4b..8287f1951cc37a948dd82083c7e0049d9d6b4421 100644 --- a/src/vs/editor/common/model/editableTextModel.ts +++ b/src/vs/editor/common/model/editableTextModel.ts @@ -12,15 +12,8 @@ import {ILineEdit, ILineMarker, ModelLine} from 'vs/editor/common/model/modelLin import {DeferredEventsBuilder, TextModelWithDecorations} from 'vs/editor/common/model/textModelWithDecorations'; import {IMode} from 'vs/editor/common/modes'; -export interface IDeltaSingleEditOperation { - original: IValidatedEditOperation; - deltaStartLineNumber: number; - deltaStartColumn: number; - deltaEndLineNumber: number; - deltaEndColumn: number; -} - export interface IValidatedEditOperation { + sortIndex: number; identifier: editorCommon.ISingleEditOperationIdentifier; range: editorCommon.IEditorRange; rangeLength: number; @@ -155,6 +148,7 @@ export class EditableTextModel extends TextModelWithDecorations implements edito } return { + sortIndex: 0, identifier: operations[0].identifier, range: entireEditRange, rangeLength: this.getValueLengthInRange(entireEditRange), @@ -163,6 +157,22 @@ export class EditableTextModel extends TextModelWithDecorations implements edito }; } + private static _sortOpsAscending(a:IValidatedEditOperation, b:IValidatedEditOperation): number { + let r = Range.compareRangesUsingEnds(a.range, b.range); + if (r === 0) { + return a.sortIndex - b.sortIndex; + } + return r; + } + + private static _sortOpsDescending(a:IValidatedEditOperation, b:IValidatedEditOperation): number { + let r = Range.compareRangesUsingEnds(a.range, b.range); + if (r === 0) { + return b.sortIndex - a.sortIndex; + } + return -r; + } + public applyEdits(rawOperations:editorCommon.IIdentifiedSingleEditOperation[]): editorCommon.IIdentifiedSingleEditOperation[] { if (rawOperations.length === 0) { return []; @@ -173,6 +183,7 @@ export class EditableTextModel extends TextModelWithDecorations implements edito let op = rawOperations[i]; let validatedRange = this.validateRange(op.range); operations[i] = { + sortIndex: i, identifier: op.identifier, range: validatedRange, rangeLength: this.getValueLengthInRange(validatedRange), @@ -181,20 +192,19 @@ export class EditableTextModel extends TextModelWithDecorations implements edito }; } - // Sort operations - operations.sort((a, b) => { - return Range.compareRangesUsingEnds(a.range, b.range); - }); + // Sort operations ascending + operations.sort(EditableTextModel._sortOpsAscending); + + for (let i = 0, count = operations.length - 1; i < count; i++) { + let rangeEnd = operations[i].range.getEndPosition(); + let nextRangeStart = operations[i + 1].range.getStartPosition(); - // Operations can not overlap! - for (let i = operations.length - 2; i >= 0; i--) { - if (operations[i+1].range.getStartPosition().isBeforeOrEqual(operations[i].range.getEndPosition())) { + if (nextRangeStart.isBefore(rangeEnd)) { + // overlapping ranges throw new Error('Overlapping ranges are not allowed!'); } } - // console.log(JSON.stringify(operations, null, '\t')); - operations = this._reduceOperations(operations); let editableRange = this.getEditableRange(); @@ -208,8 +218,7 @@ export class EditableTextModel extends TextModelWithDecorations implements edito } // Delta encode operations - let deltaOperations = EditableTextModel._toDeltaOperations(operations); - let reverseRanges = EditableTextModel._getInverseEditRanges(deltaOperations); + let reverseRanges = EditableTextModel._getInverseEditRanges(operations); let reverseOperations: editorCommon.IIdentifiedSingleEditOperation[] = []; for (let i = 0; i < operations.length; i++) { reverseOperations[i] = { @@ -225,54 +234,59 @@ export class EditableTextModel extends TextModelWithDecorations implements edito return reverseOperations; } - private static _toDeltaOperation(base: IValidatedEditOperation, operation:IValidatedEditOperation): IDeltaSingleEditOperation { - let deltaStartLineNumber = operation.range.startLineNumber - (base ? base.range.endLineNumber : 0); - let deltaStartColumn = operation.range.startColumn - (deltaStartLineNumber === 0 ? base.range.endColumn : 0); - let deltaEndLineNumber = operation.range.endLineNumber - (base ? base.range.endLineNumber : 0); - let deltaEndColumn = operation.range.endColumn - (deltaEndLineNumber === 0 ? base.range.endColumn : 0); - - return { - original: operation, - deltaStartLineNumber: deltaStartLineNumber, - deltaStartColumn: deltaStartColumn, - deltaEndLineNumber: deltaEndLineNumber, - deltaEndColumn: deltaEndColumn - }; - } - /** * Assumes `operations` are validated and sorted ascending */ - public static _getInverseEditRanges(operations:IDeltaSingleEditOperation[]): editorCommon.IEditorRange[] { - let lineNumber = 0, - column = 0, - result:editorCommon.IEditorRange[] = []; + public static _getInverseEditRanges(operations:IValidatedEditOperation[]): editorCommon.IEditorRange[] { + let result:editorCommon.IEditorRange[] = []; + let prevOpEndLineNumber: number; + let prevOpEndColumn: number; + let prevOp:IValidatedEditOperation = null; for (let i = 0, len = operations.length; i < len; i++) { let op = operations[i]; - let startLineNumber = op.deltaStartLineNumber + lineNumber; - let startColumn = op.deltaStartColumn + (op.deltaStartLineNumber === 0 ? column : 0); + let startLineNumber: number; + let startColumn: number; + + if (prevOp) { + if (prevOp.range.endLineNumber === op.range.startLineNumber) { + startLineNumber = prevOpEndLineNumber; + startColumn = prevOpEndColumn + (op.range.startColumn - prevOp.range.endColumn); + } else { + startLineNumber = prevOpEndLineNumber + (op.range.startLineNumber - prevOp.range.endLineNumber); + startColumn = op.range.startColumn; + } + } else { + startLineNumber = op.range.startLineNumber; + startColumn = op.range.startColumn; + } + let resultRange: editorCommon.IEditorRange; - if (op.original.lines && op.original.lines.length > 0) { - // There is something to insert - if (op.original.lines.length === 1) { - // Single line insert - resultRange = new Range(startLineNumber, startColumn, startLineNumber, startColumn + op.original.lines[0].length); + if (op.lines && op.lines.length > 0) { + // the operation inserts something + let lineCount = op.lines.length; + let firstLine = op.lines[0]; + let lastLine = op.lines[lineCount - 1]; + + if (lineCount === 1) { + // single line insert + resultRange = new Range(startLineNumber, startColumn, startLineNumber, startColumn + firstLine.length); } else { - // Multi line insert - resultRange = new Range(startLineNumber, startColumn, startLineNumber + op.original.lines.length - 1, op.original.lines[op.original.lines.length - 1].length + 1); + // multi line insert + resultRange = new Range(startLineNumber, startColumn, startLineNumber + lineCount - 1, lastLine.length + 1); } } else { // There is nothing to insert resultRange = new Range(startLineNumber, startColumn, startLineNumber, startColumn); } - lineNumber = resultRange.endLineNumber; - column = resultRange.endColumn; + prevOpEndLineNumber = resultRange.endLineNumber; + prevOpEndColumn = resultRange.endColumn; result.push(resultRange); + prevOp = op; } return result; @@ -280,8 +294,9 @@ export class EditableTextModel extends TextModelWithDecorations implements edito private _applyEdits(operations:IValidatedEditOperation[]): void { - // Note the minus! - operations = operations.sort((a, b) => -Range.compareRangesUsingEnds(a.range, b.range)); + // Sort operations descending + operations.sort(EditableTextModel._sortOpsDescending); + this._withDeferredEvents((deferredEventsBuilder:DeferredEventsBuilder) => { let contentChangedEvents: editorCommon.IModelContentChangedEvent[] = []; @@ -516,14 +531,6 @@ export class EditableTextModel extends TextModelWithDecorations implements edito } } - public static _toDeltaOperations(operations:IValidatedEditOperation[]): IDeltaSingleEditOperation[] { - let result: IDeltaSingleEditOperation[] = []; - for (let i = 0; i < operations.length; i++) { - result[i] = EditableTextModel._toDeltaOperation(i > 0 ? operations[i-1] : null, operations[i]); - } - return result; - } - public undo(): editorCommon.IEditorSelection[] { if (this._isDisposed) { throw new Error('EditableTextModel.undo: Model is disposed'); diff --git a/src/vs/editor/test/common/model/editableTextModel.test.ts b/src/vs/editor/test/common/model/editableTextModel.test.ts index e75caaff62c9541a06dc11780000030de746a16b..03e3cfd4860352939586b4038fcbb5a7db2196dc 100644 --- a/src/vs/editor/test/common/model/editableTextModel.test.ts +++ b/src/vs/editor/test/common/model/editableTextModel.test.ts @@ -16,6 +16,7 @@ suite('EditorModel - EditableTextModel._getInverseEdits', () => { function editOp(startLineNumber: number, startColumn: number, endLineNumber: number, endColumn: number, rangeLength: number, text:string[]): IValidatedEditOperation { return { + sortIndex: 0, identifier: null, range: new Range(startLineNumber, startColumn, endLineNumber, endColumn), rangeLength: rangeLength, @@ -29,7 +30,7 @@ suite('EditorModel - EditableTextModel._getInverseEdits', () => { } function assertInverseEdits(ops:IValidatedEditOperation[], expected:Range[]): void { - var actual = EditableTextModel._getInverseEditRanges(EditableTextModel._toDeltaOperations(ops)); + var actual = EditableTextModel._getInverseEditRanges(ops); assert.deepEqual(actual, expected); } @@ -264,6 +265,7 @@ suite('EditorModel - EditableTextModel._toSingleEditOperation', () => { function editOp(startLineNumber: number, startColumn: number, endLineNumber: number, endColumn: number, rangeLength:number, text:string[]): IValidatedEditOperation { return { + sortIndex: 0, identifier: null, range: new Range(startLineNumber, startColumn, endLineNumber, endColumn), rangeLength: rangeLength, @@ -1165,7 +1167,162 @@ suite('EditorModel - EditableTextModel.applyEdits', () => { ); }); + test('issue #3980', () => { + testApplyEditsWithSyncedModels( + [ + 'class A {', + ' someProperty = false;', + ' someMethod() {', + ' this.someMethod();', + ' }', + '}', + ], + [ + editOp(1, 8, 1, 9, ['', '']), + editOp(3, 17, 3, 18, ['', '']), + editOp(3, 18, 3, 18, [' ']), + editOp(4, 5, 4, 5, [' ']), + ], + [ + 'class A', + '{', + ' someProperty = false;', + ' someMethod()', + ' {', + ' this.someMethod();', + ' }', + '}', + ] + ); + }); + + function testApplyEditsFails(original:string[], edits:IIdentifiedSingleEditOperation[]): void { + let model = new EditableTextModel([], TextModel.toRawText(original.join('\n'), TextModel.DEFAULT_CREATION_OPTIONS), null); + + let hasThrown = false; + try { + model.applyEdits(edits); + } catch(err) { + hasThrown = true; + } + assert.ok(hasThrown, 'expected model.applyEdits to fail.'); + + model.dispose(); + } + + test('touching edits: two inserts at the same position', () => { + testApplyEditsWithSyncedModels( + [ + 'hello world' + ], + [ + editOp(1, 1, 1, 1, ['a']), + editOp(1, 1, 1, 1, ['b']), + ], + [ + 'abhello world' + ] + ); + }); + + test('touching edits: insert and replace touching', () => { + testApplyEditsWithSyncedModels( + [ + 'hello world' + ], + [ + editOp(1, 1, 1, 1, ['b']), + editOp(1, 1, 1, 3, ['ab']), + ], + [ + 'babllo world' + ] + ); + }); + + test('overlapping edits: two overlapping replaces', () => { + testApplyEditsFails( + [ + 'hello world' + ], + [ + editOp(1, 1, 1, 2, ['b']), + editOp(1, 1, 1, 3, ['ab']), + ] + ); + }); + + test('overlapping edits: two overlapping deletes', () => { + testApplyEditsFails( + [ + 'hello world' + ], + [ + editOp(1, 1, 1, 2, ['']), + editOp(1, 1, 1, 3, ['']), + ] + ); + }); + + test('touching edits: two touching replaces', () => { + testApplyEditsWithSyncedModels( + [ + 'hello world' + ], + [ + editOp(1, 1, 1, 2, ['H']), + editOp(1, 2, 1, 3, ['E']), + ], + [ + 'HEllo world' + ] + ); + }); + test('touching edits: two touching deletes', () => { + testApplyEditsWithSyncedModels( + [ + 'hello world' + ], + [ + editOp(1, 1, 1, 2, ['']), + editOp(1, 2, 1, 3, ['']), + ], + [ + 'llo world' + ] + ); + }); + + test('touching edits: insert and replace', () => { + testApplyEditsWithSyncedModels( + [ + 'hello world' + ], + [ + editOp(1, 1, 1, 1, ['H']), + editOp(1, 1, 1, 3, ['e']), + ], + [ + 'Hello world' + ] + ); + }); + + test('touching edits: replace and insert', () => { + testApplyEditsWithSyncedModels( + [ + 'hello world' + ], + [ + editOp(1, 1, 1, 3, ['H']), + editOp(1, 3, 1, 3, ['e']), + ], + [ + 'Hello world' + ] + ); + }); test('change while emitting events 1', () => { diff --git a/src/vs/editor/test/common/model/editableTextModelAuto.test.ts b/src/vs/editor/test/common/model/editableTextModelAuto.test.ts index 7d07e85f404820cb0608d0b4b722e96f90049393..9617683842a8fc9beb5d0504d1660d2244c8fb3a 100644 --- a/src/vs/editor/test/common/model/editableTextModelAuto.test.ts +++ b/src/vs/editor/test/common/model/editableTextModelAuto.test.ts @@ -103,6 +103,35 @@ suite('EditorModel Auto Tests', () => { ] ); }); + + test('auto4', () => { + testApplyEditsWithSyncedModels( + [ + 'fefymj', + 'qum', + 'vmiwxxaiqq', + 'dz', + 'lnqdgorosf', + ], + [ + editOp(1, 3, 1, 5, ['hp']), + editOp(1, 7, 2, 1, ['kcg', '', 'mpx']), + editOp(2, 2, 2, 2, ['', 'aw', '']), + editOp(2, 2, 2, 2, ['vqr', 'mo']), + editOp(4, 2, 5, 3, ['xyc']), + ], + [ + 'fehpmjkcg', + '', + 'mpxq', + 'aw', + 'vqr', + 'moum', + 'vmiwxxaiqq', + 'dxycqdgorosf', + ] + ); + }); }); function getRandomInt(min: number, max: number): number { @@ -138,14 +167,15 @@ function generateEdits(content:string): ITestModelEdit[] { let offset = getRandomInt(0, maxOffset); let length = getRandomInt(0, maxOffset - offset); + let text = generateFile(true); result.push({ offset: offset, length: length, - text: generateFile(true) + text: text }); - maxOffset = offset - 1; + maxOffset = offset; cnt--; } @@ -166,37 +196,15 @@ class TestModel { public resultingContent: string; public edits: IIdentifiedSingleEditOperation[]; - constructor() { - this.initialContent = generateFile(false); - - let edits = generateEdits(this.initialContent); - let currentEditIdx = 0; + private static _generateOffsetToPosition(content:string): Position[] { + let result: Position[] = []; let lineNumber = 1; let column = 1; - let editStartPosition: Position = null; - this.edits = []; - for (let offset = 0, len = this.initialContent.length; currentEditIdx < edits.length && offset <= len; offset++) { - let ch = this.initialContent.charAt(offset); - - if (!editStartPosition) { - if (offset === edits[currentEditIdx].offset) { - editStartPosition = new Position(lineNumber, column); - } - } + for (let offset = 0, len = content.length; offset <= len; offset++) { + let ch = content.charAt(offset); - if (editStartPosition) { - if (offset === edits[currentEditIdx].offset + edits[currentEditIdx].length) { - this.edits.push({ - identifier: null, - range: new Range(editStartPosition.lineNumber, editStartPosition.column, lineNumber, column), - text: edits[currentEditIdx].text, - forceMoveMarkers: false - }); - currentEditIdx++; - editStartPosition = null; - } - } + result[offset] = new Position(lineNumber, column); if (ch === '\n') { lineNumber++; @@ -206,6 +214,27 @@ class TestModel { } } + return result; + } + + constructor() { + this.initialContent = generateFile(false); + + let edits = generateEdits(this.initialContent); + + let offsetToPosition = TestModel._generateOffsetToPosition(this.initialContent); + this.edits = []; + for (let i = 0; i < edits.length; i++) { + let startPosition = offsetToPosition[edits[i].offset]; + let endPosition = offsetToPosition[edits[i].offset + edits[i].length]; + this.edits.push({ + identifier: null, + range: new Range(startPosition.lineNumber, startPosition.column, endPosition.lineNumber, endPosition.column), + text: edits[i].text, + forceMoveMarkers: false + }); + } + this.resultingContent = this.initialContent; for (let i = edits.length - 1; i >= 0; i--) { this.resultingContent = ( @@ -247,7 +276,7 @@ if (GENERATE_TESTS) { let testModel = new TestModel(); - console.log(testModel.print()); + // console.log(testModel.print()); console.log('------END NEW TEST: ' + (number++)); @@ -259,7 +288,7 @@ if (GENERATE_TESTS) { ); // throw new Error('a'); } catch(err) { - console.log('bubu'); + console.log(err); console.log(testModel.print()); break; }