From 0c017188b4d41cc80c297e35097095026734b3d4 Mon Sep 17 00:00:00 2001
From: Christian Halstrick <christian.halstrick@sap.com>
Date: Tue, 31 Aug 2010 17:14:07 +0200
Subject: [PATCH] Improve MergeAlgorithm to produce smaller conflicts

The merge algorithm was reporting conflicts which where to big.

Example: The common base was "ABC", the "ours" version contained
"AB1C" (the addition of "1" after pos 2) and the "theirs" version also
contained "AB1C". We have two potentially conflicting edits in the
same region which happen to bring in exactly the same content. This
should not be a conflict - but was previously reported as
"AB<<<1===1>>>C".

This is fixed by checking every conflicting chunk whether the
conflicting regions have a common prefix or suffix and by removing
this regions from the conflict.

Change-Id: I4dc169b8ef7a66ec6b307e9a956feef906c9e15e
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
---
 .../jgit/merge/MergeAlgorithmTest.java        | 15 ++++++-
 .../eclipse/jgit/merge/MergeAlgorithm.java    | 39 +++++++++++++++++--
 2 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java
index d12fae7c1..1ccc00eca 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java
@@ -105,7 +105,7 @@ public class MergeAlgorithmTest extends TestCase {
 	 * @throws IOException
 	 */
 	public void testTwoConflictingModifications() throws IOException {
-		assertEquals(A + XXX_0 + B + Z + XXX_1 + Z + Z + XXX_2 + D + E + F + G
+		assertEquals(A + XXX_0 + B + XXX_1 + Z + XXX_2 + Z + D + E + F + G
 				+ H + I + J,
 				merge(base, replace_C_by_Z, replace_BC_by_ZZ));
 	}
@@ -118,7 +118,7 @@ public class MergeAlgorithmTest extends TestCase {
 	 * @throws IOException
 	 */
 	public void testOneAgainstTwoConflictingModifications() throws IOException {
-		assertEquals(A + XXX_0 + Z + Z + Z + XXX_1 + Z + C + Z + XXX_2 + E + F
+		assertEquals(A + Z + XXX_0 + Z + XXX_1 + C + XXX_2 + Z + E + F
 				+ G + H + I + J,
 				merge(base, replace_BCD_by_ZZZ, replace_BD_by_ZZ));
 	}
@@ -178,6 +178,17 @@ public class MergeAlgorithmTest extends TestCase {
 		assertEquals(A+B+C+D+E+F+G+H+I+XXX_0+Z+XXX_1+Y+XXX_2, merge(base, replace_J_by_Z, replace_J_by_Y));
 	}
 
+	/**
+	 * Check for a conflict where the second text was changed similar to the
+	 * first one, but the second texts modification covers one more line.
+	 *
+	 * @throws IOException
+	 */
+	public void testSameModification() throws IOException {
+		assertEquals(replace_C_by_Z,
+				merge(base, replace_C_by_Z, replace_C_by_Z));
+	}
+
 	private String merge(String commonBase, String ours, String theirs) throws IOException {
 		MergeResult r=MergeAlgorithm.merge(new RawText(Constants.encode(commonBase)), new RawText(Constants.encode(ours)), new RawText(Constants.encode(theirs)));
 		ByteArrayOutputStream bo=new ByteArrayOutputStream(50);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java
index deae82e76..2c8386b8c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java
@@ -195,11 +195,42 @@ public final class MergeAlgorithm {
 					theirsEndB += oursEdit.getEndA() - theirsEdit.getEndA();
 				}
 
+				// A conflicting region is found. Strip off common lines in
+				// in the beginning and the end of the conflicting region
+				int conflictLen = Math.min(oursEndB - oursBeginB, theirsEndB
+						- theirsBeginB);
+				int commonPrefix = 0;
+				while (commonPrefix < conflictLen
+						&& ours.equals(oursBeginB + commonPrefix, theirs,
+								theirsBeginB + commonPrefix))
+					commonPrefix++;
+				conflictLen -= commonPrefix;
+				int commonSuffix = 0;
+				while (commonSuffix < conflictLen
+						&& ours.equals(oursEndB - commonSuffix - 1, theirs,
+								theirsEndB - commonSuffix - 1))
+					commonSuffix++;
+				conflictLen -= commonSuffix;
+
+				// Add the common lines at start of conflict
+				if (commonPrefix > 0)
+					result.add(1, oursBeginB, oursBeginB + commonPrefix,
+							ConflictState.NO_CONFLICT);
+
 				// Add the conflict
-				result.add(1, oursBeginB, oursEndB,
-						ConflictState.FIRST_CONFLICTING_RANGE);
-				result.add(2, theirsBeginB, theirsEndB,
-						ConflictState.NEXT_CONFLICTING_RANGE);
+				if (conflictLen > 0) {
+					result.add(1, oursBeginB + commonPrefix, oursEndB
+							- commonSuffix,
+							ConflictState.FIRST_CONFLICTING_RANGE);
+					result.add(2, theirsBeginB + commonPrefix, theirsEndB
+							- commonSuffix,
+							ConflictState.NEXT_CONFLICTING_RANGE);
+				}
+
+				// Add the common lines at end of conflict
+				if (commonSuffix > 0)
+					result.add(1, oursEndB - commonSuffix, oursEndB,
+							ConflictState.NO_CONFLICT);
 
 				current = Math.max(oursEdit.getEndA(), theirsEdit.getEndA());
 				oursEdit = nextOursEdit;