Shift diff up to get to boundaries (microsoft#30087)

alexdima · alexdima · commit c8df6987f33d · 2017-07-04T16:05:20.000+02:00
diff --git a/src/vs/base/common/diff/diff.ts b/src/vs/base/common/diff/diff.ts
@@ -799,7 +799,7 @@ export class LcsDiff {
 		do {
 			mergedDiffs = false;
 
-			// Shift all the changes first
+			// Shift all the changes down first
 			for (let i = 0; i < changes.length; i++) {
 				const change = changes[i];
 				const originalStop = (i < changes.length - 1) ? changes[i + 1].originalStart : this.OriginalSequence.getLength();
@@ -834,9 +834,105 @@ export class LcsDiff {
 			changes = result;
 		} while (mergedDiffs);
 
+		// Shift changes back up until we hit empty or whitespace-only lines
+		for (let i = changes.length - 1; i >= 0; i--) {
+			const change = changes[i];
+
+			let originalStop = 0;
+			let modifiedStop = 0;
+			if (i > 0) {
+				const prevChange = changes[i - 1];
+				if (prevChange.originalLength > 0) {
+					originalStop = prevChange.originalStart + prevChange.originalLength;
+				}
+				if (prevChange.modifiedLength > 0) {
+					modifiedStop = prevChange.modifiedStart + prevChange.modifiedLength;
+				}
+			}
+
+			const checkOriginal = change.originalLength > 0;
+			const checkModified = change.modifiedLength > 0;
+
+			let bestDelta = 0;
+			let bestScore = this._boundaryScore(change.originalStart, change.originalLength, change.modifiedStart, change.modifiedLength);
+
+			for (let delta = 1; ; delta++) {
+				let originalStart = change.originalStart - delta;
+				let modifiedStart = change.modifiedStart - delta;
+
+				if (originalStart < originalStop || modifiedStart < modifiedStart) {
+					break;
+				}
+
+				if (checkOriginal && !this.OriginalElementsAreEqual(originalStart, originalStart + change.originalLength)) {
+					break;
+				}
+
+				if (checkModified && !this.ModifiedElementsAreEqual(modifiedStart, modifiedStart + change.modifiedLength)) {
+					break;
+				}
+
+				let score = this._boundaryScore(originalStart, change.originalLength, modifiedStart, change.modifiedLength);
+
+				if (score > bestScore) {
+					bestScore = score;
+					bestDelta = delta;
+				}
+			}
+
+			change.originalStart -= bestDelta;
+			change.modifiedStart -= bestDelta;
+		}
+
 		return changes;
 	}
 
+	private _OriginalIsBoundary(index: number): boolean {
+		if (index <= 0 || index >= this.OriginalSequence.getLength() - 1) {
+			return true;
+		}
+		return /^\s*$/.test(this.OriginalSequence.getElementHash(index));
+	}
+
+	private _OriginalRegionIsBoundary(originalStart: number, originalLength: number): boolean {
+		if (this._OriginalIsBoundary(originalStart) || this._OriginalIsBoundary(originalStart - 1)) {
+			return true;
+		}
+		if (originalLength > 0) {
+			let originalEnd = originalStart + originalLength;
+			if (this._OriginalIsBoundary(originalEnd - 1) || this._OriginalIsBoundary(originalEnd)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	private _ModifiedIsBoundary(index: number): boolean {
+		if (index <= 0 || index >= this.ModifiedSequence.getLength() - 1) {
+			return true;
+		}
+		return /^\s*$/.test(this.ModifiedSequence.getElementHash(index));
+	}
+
+	private _ModifiedRegionIsBoundary(modifiedStart: number, modifiedLength: number): boolean {
+		if (this._ModifiedIsBoundary(modifiedStart) || this._ModifiedIsBoundary(modifiedStart - 1)) {
+			return true;
+		}
+		if (modifiedLength > 0) {
+			let modifiedEnd = modifiedStart + modifiedLength;
+			if (this._ModifiedIsBoundary(modifiedEnd - 1) || this._ModifiedIsBoundary(modifiedEnd)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	private _boundaryScore(originalStart: number, originalLength: number, modifiedStart: number, modifiedLength: number): number {
+		let originalScore = (this._OriginalRegionIsBoundary(originalStart, originalLength) ? 1 : 0);
+		let modifiedScore = (this._ModifiedRegionIsBoundary(modifiedStart, modifiedLength) ? 1 : 0);
+		return (originalScore + modifiedScore);
+	}
+
 	/**
 	 * Concatenates the two input DiffChange lists and returns the resulting
 	 * list.
diff --git a/src/vs/editor/test/common/diff/diffComputer.test.ts b/src/vs/editor/test/common/diff/diffComputer.test.ts
@@ -539,4 +539,42 @@ suite('Editor Diff - DiffComputer', () => {
 		];
 		assertDiff(original, modified, expected, false, true);
 	});
+
+	test('pretty diff 3', () => {
+		var original = [
+			'class A {',
+			'	/**',
+			'	 * m1',
+			'	 */',
+			'	method1() {}',
+			'',
+			'	/**',
+			'	 * m3',
+			'	 */',
+			'	method3() {}',
+			'}',
+		];
+		var modified = [
+			'class A {',
+			'	/**',
+			'	 * m1',
+			'	 */',
+			'	method1() {}',
+			'',
+			'	/**',
+			'	 * m2',
+			'	 */',
+			'	method2() {}',
+			'',
+			'	/**',
+			'	 * m3',
+			'	 */',
+			'	method3() {}',
+			'}',
+		];
+		var expected = [
+			createLineInsertion(7, 11, 6)
+		];
+		assertDiff(original, modified, expected, false, true);
+	});
 });
