Merge Editor Apply Edit Bugfix

src/vs/editor/common/diff/algorithms/diffAlgorithm.ts

@@ -15,6 +15,14 @@ export class SequenceDiff {
 		public readonly seq1Range: OffsetRange,
 		public readonly seq2Range: OffsetRange
 	) { }
+
+	public reverse(): SequenceDiff {
+		return new SequenceDiff(this.seq2Range, this.seq1Range);
+	}
+
+	public toString(): string {
+		return `${this.seq1Range} <-> ${this.seq2Range}`;
+	}
 }
 
 /**
@@ -34,11 +42,22 @@ export class OffsetRange {
 	public get length(): number {
 		return this.endExclusive - this.start;
 	}
+
+	public toString() {
+		return `[${this.start}, ${this.endExclusive})`;
+	}
 }
 
 export interface ISequence {
 	getElement(offset: number): number;
 	get length(): number;
+
+	/**
+	 * The higher the score, the better that offset can be used to split the sequence.
+	 * Is used to optimize insertions.
+	 * Must not be negative.
+	*/
+	getBoundaryScore?(length: number): number;
 }
 
 export class SequenceFromIntArray implements ISequence {

src/vs/editor/common/diff/algorithms/dynamicProgrammingDiffing.ts

@@ -32,23 +32,28 @@ export class DynamicProgrammingDiffing implements IDiffAlgorithm {
 					} else {
 						extendedSeqScore = lcsLengths.get(s1 - 1, s2 - 1);
 					}
+					if (s1 > 0 && s2 > 0 && directions.get(s1 - 1, s2 - 1) === 3) {
+						// Prefer consecutive diagonals
+						extendedSeqScore += 0.1;
+					}
 					extendedSeqScore += (equalityScore ? equalityScore(s1, s2) : 1);
 				} else {
 					extendedSeqScore = -1;
 				}
 
 				const newValue = Math.max(horizontalLen, verticalLen, extendedSeqScore);
 
-				if (newValue === horizontalLen) {
+				if (newValue === extendedSeqScore) {
+					// Prefer diagonals
+					const prevLen = s1 > 0 && s2 > 0 ? lengths.get(s1 - 1, s2 - 1) : 0;
+					lengths.set(s1, s2, prevLen + 1);
+					directions.set(s1, s2, 3);
+				} else if (newValue === horizontalLen) {
 					lengths.set(s1, s2, 0);
 					directions.set(s1, s2, 1);
 				} else if (newValue === verticalLen) {
 					lengths.set(s1, s2, 0);
 					directions.set(s1, s2, 2);
-				} else {
-					const prevLen = s1 > 0 && s2 > 0 ? lengths.get(s1 - 1, s2 - 1) : 0;
-					lengths.set(s1, s2, prevLen + 1);
-					directions.set(s1, s2, 3);
 				}
 
 				lcsLengths.set(s1, s2, newValue);

src/vs/editor/common/diff/algorithms/joinSequenceDiffs.ts

@@ -5,6 +5,13 @@
 
 import { ISequence, OffsetRange, SequenceDiff } from 'vs/editor/common/diff/algorithms/diffAlgorithm';
 
+export function optimizeSequenceDiffs(sequence1: ISequence, sequence2: ISequence, sequenceDiffs: SequenceDiff[]): SequenceDiff[] {
+	let result = sequenceDiffs;
+	result = joinSequenceDiffs(sequence1, sequence2, result);
+	result = shiftSequenceDiffs(sequence1, sequence2, result);
+	return result;
+}
+
 /**
  * This function fixes issues like this:
  * ```
@@ -27,20 +34,104 @@ export function joinSequenceDiffs(sequence1: ISequence, sequence2: ISequence, se
 		const lastResult = result[result.length - 1];
 		const cur = sequenceDiffs[i];
 
-		if (cur.seq1Range.start - lastResult.seq1Range.endExclusive === 1) {
-			if (cur.seq1Range.isEmpty) {
-				if (sequence2.getElement(cur.seq2Range.start - 1) === sequence2.getElement(cur.seq2Range.endExclusive - 1)) {
-					result[result.length - 1] = new SequenceDiff(lastResult.seq1Range, new OffsetRange(
-						lastResult.seq2Range.start,
-						cur.seq2Range.endExclusive - 1
-					));
-					continue;
+		if (cur.seq1Range.isEmpty) {
+			let all = true;
+			const length = cur.seq1Range.start - lastResult.seq1Range.endExclusive;
+			for (let i = 1; i <= length; i++) {
+				if (sequence2.getElement(cur.seq2Range.start - i) !== sequence2.getElement(cur.seq2Range.endExclusive - i)) {
+					all = false;
+					break;
 				}
 			}
+
+			if (all) {
+				// Merge previous and current diff
+				result[result.length - 1] = new SequenceDiff(lastResult.seq1Range, new OffsetRange(
+					lastResult.seq2Range.start,
+					cur.seq2Range.endExclusive - length
+				));
+				continue;
+			}
 		}
 
 		result.push(cur);
 	}
 
 	return result;
 }
+
+// align character level diffs at whitespace characters
+// import { IBar } from "foo";
+// import { I[Arr, I]Bar } from "foo";
+// ->
+// import { [IArr, ]IBar } from "foo";
+
+// import { ITransaction, observableValue, transaction } from 'vs/base/common/observable';
+// import { ITransaction, observable[FromEvent, observable]Value, transaction } from 'vs/base/common/observable';
+// ->
+// import { ITransaction, [observableFromEvent, ]observableValue, transaction } from 'vs/base/common/observable';
+
+// collectBrackets(level + 1, levelPerBracketType);
+// collectBrackets(level + 1, levelPerBracket[ + 1, levelPerBracket]Type);
+// ->
+// collectBrackets(level + 1, [levelPerBracket + 1, ]levelPerBracketType);
+
+export function shiftSequenceDiffs(sequence1: ISequence, sequence2: ISequence, sequenceDiffs: SequenceDiff[]): SequenceDiff[] {
+	if (!sequence1.getBoundaryScore || !sequence2.getBoundaryScore) {
+		return sequenceDiffs;
+	}
+
+	for (let i = 0; i < sequenceDiffs.length; i++) {
+		const diff = sequenceDiffs[i];
+		if (diff.seq1Range.isEmpty) {
+			const seq2PrevEndExclusive = (i > 0 ? sequenceDiffs[i - 1].seq2Range.endExclusive : -1);
+			const seq2NextStart = (i + 1 < sequenceDiffs.length ? sequenceDiffs[i + 1].seq2Range.start : sequence2.length + 1);
+			sequenceDiffs[i] = shiftDiffToBetterPosition(diff, sequence1, sequence2, seq2NextStart, seq2PrevEndExclusive);
+		} else if (diff.seq2Range.isEmpty) {
+			const seq1PrevEndExclusive = (i > 0 ? sequenceDiffs[i - 1].seq1Range.endExclusive : -1);
+			const seq1NextStart = (i + 1 < sequenceDiffs.length ? sequenceDiffs[i + 1].seq1Range.start : sequence1.length + 1);
+			sequenceDiffs[i] = shiftDiffToBetterPosition(diff.reverse(), sequence2, sequence1, seq1NextStart, seq1PrevEndExclusive).reverse();
+		}
+	}
+
+	return sequenceDiffs;
+}
+
+function shiftDiffToBetterPosition(diff: SequenceDiff, sequence1: ISequence, sequence2: ISequence, seq2NextStart: number, seq2PrevEndExclusive: number) {
+	// don't touch previous or next!
+	let deltaBefore = 1;
+	while (diff.seq1Range.start - deltaBefore > seq2PrevEndExclusive &&
+		sequence2.getElement(diff.seq2Range.start - deltaBefore) ===
+		sequence2.getElement(diff.seq2Range.endExclusive - deltaBefore)) {
+		deltaBefore++;
+	}
+	deltaBefore--;
+
+	let deltaAfter = 1;
+	while (diff.seq1Range.start + deltaAfter < seq2NextStart &&
+		sequence2.getElement(diff.seq2Range.start + deltaAfter) ===
+		sequence2.getElement(diff.seq2Range.endExclusive + deltaAfter)) {
+		deltaAfter++;
+	}
+	deltaAfter--;
+
+	let bestDelta = 0;
+	let bestScore = -1;
+	// find best scored delta
+	for (let delta = -deltaBefore; delta < deltaAfter; delta++) {
+		const seq2OffsetStart = diff.seq2Range.start + delta;
+		const seq2OffsetEndExclusive = diff.seq2Range.endExclusive + delta;
+		const seq1Offset = diff.seq1Range.start + delta;
+
+		const score = sequence1.getBoundaryScore!(seq1Offset) + sequence2.getBoundaryScore!(seq2OffsetStart) + sequence2.getBoundaryScore!(seq2OffsetEndExclusive);
+		if (score > bestScore) {
+			bestScore = score;
+			bestDelta = delta;
+		}
+	}
+
+	if (bestDelta !== 0) {
+		return new SequenceDiff(diff.seq1Range.delta(bestDelta), diff.seq2Range.delta(bestDelta));
+	}
+	return diff;
+}

src/vs/editor/common/diff/standardLinesDiffComputer.ts

@@ -4,11 +4,12 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { assertFn, checkAdjacentItems } from 'vs/base/common/assert';
+import { CharCode } from 'vs/base/common/charCode';
 import { Position } from 'vs/editor/common/core/position';
 import { Range } from 'vs/editor/common/core/range';
 import { SequenceFromIntArray, OffsetRange, SequenceDiff, ISequence } from 'vs/editor/common/diff/algorithms/diffAlgorithm';
 import { DynamicProgrammingDiffing } from 'vs/editor/common/diff/algorithms/dynamicProgrammingDiffing';
-import { joinSequenceDiffs } from 'vs/editor/common/diff/algorithms/joinSequenceDiffs';
+import { optimizeSequenceDiffs } from 'vs/editor/common/diff/algorithms/joinSequenceDiffs';
 import { MyersDiffAlgorithm } from 'vs/editor/common/diff/algorithms/myersDiffAlgorithm';
 import { ILinesDiff, ILinesDiffComputer, ILinesDiffComputerOptions, LineRange, LineRangeMapping, RangeMapping } from 'vs/editor/common/diff/linesDiffComputer';
 
@@ -109,8 +110,11 @@ export class StandardLinesDiffComputer implements ILinesDiffComputer {
 		const sourceSlice = new Slice(originalLines, diff.seq1Range);
 		const targetSlice = new Slice(modifiedLines, diff.seq2Range);
 
-		const originalDiffs = this.myersDiffingAlgorithm.compute(sourceSlice, targetSlice);
-		const diffs = joinSequenceDiffs(sourceSlice, targetSlice, originalDiffs);
+		const originalDiffs = sourceSlice.length + targetSlice.length < 500
+			? this.dynamicProgrammingDiffing.compute(sourceSlice, targetSlice)
+			: this.myersDiffingAlgorithm.compute(sourceSlice, targetSlice);
+
+		const diffs = optimizeSequenceDiffs(sourceSlice, targetSlice, originalDiffs);
 		const result = diffs.map(
 			(d) =>
 				new RangeMapping(
@@ -217,6 +221,24 @@ class Slice implements ISequence {
 		return this.elements.length;
 	}
 
+	public getBoundaryScore(length: number): number {
+		//   a   b   c   ,           d   e   f
+		// 11  0   0   12  15  6   13  0   0   11
+
+		const prevCategory = getCategory(length > 0 ? this.elements[length - 1] : -1);
+		const nextCategory = getCategory(length < this.elements.length ? this.elements[length] : -1);
+
+		let score = 0;
+		if (prevCategory !== nextCategory) {
+			score += 10;
+		}
+
+		score += getCategoryBoundaryScore(prevCategory);
+		score += getCategoryBoundaryScore(nextCategory);
+
+		return score;
+	}
+
 	public translateOffset(offset: number): Position {
 		// find smallest i, so that lineBreakOffsets[i] > offset using binary search
 
@@ -239,3 +261,38 @@ class Slice implements ISequence {
 		return Range.fromPositions(this.translateOffset(range.start), this.translateOffset(range.endExclusive));
 	}
 }
+
+const enum CharBoundaryCategory {
+	Word = 0,
+	End = 1,
+	Other = 2,
+	Space = 3,
+}
+
+function getCategoryBoundaryScore(category: CharBoundaryCategory): number {
+	return category;
+}
+
+function getCategory(charCode: number): CharBoundaryCategory {
+	if (isSpace(charCode)) {
+		return CharBoundaryCategory.Space;
+	} else if (isWordChar(charCode)) {
+		return CharBoundaryCategory.Word;
+	} else if (charCode === -1) {
+		return CharBoundaryCategory.End;
+	} else {
+		return CharBoundaryCategory.Other;
+	}
+}
+
+function isWordChar(charCode: number): boolean {
+	return (
+		(charCode >= CharCode.a && charCode <= CharCode.z)
+		|| (charCode >= CharCode.A && charCode <= CharCode.Z)
+		|| (charCode >= CharCode.Digit0 && charCode <= CharCode.Digit9)
+	);
+}
+
+function isSpace(charCode: number): boolean {
+	return charCode === CharCode.Space || charCode === CharCode.Tab || charCode === CharCode.LineFeed || charCode === CharCode.CarriageReturn;
+}

src/vs/workbench/contrib/mergeEditor/browser/model/modifiedBaseRange.ts

@@ -239,9 +239,15 @@ function editsToLineRangeEdit(range: LineRange, sortedEdits: RangeEdit[], textMo
 
 	const lines = splitLines(text);
 	if (startsLineBefore) {
+		if (lines[0] !== '') {
+			return undefined;
+		}
 		lines.shift();
 	}
 	if (endsLineAfter) {
+		if (lines[lines.length - 1] !== '') {
+			return undefined;
+		}
 		lines.pop();
 	}
 	return new LineRangeEdit(range, lines);