Allow touching edits in editableTextModel (#3980)

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 {

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');

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', () => {
 

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;
 		}