Merge conflict messages: prefix conflict lines with a hash

C git also does so. Note that currently the comment character is
hard-coded as the hash '#' throughout JGit.

Bug: 548529
Change-Id: I4a5597694082a9e5b07412b365cfaf41fa034cfa
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

8 files changed, 65 insertions, 28 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CherryPickCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CherryPickCommandTest.java
index f4f0ecd689..0d38197d9a 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CherryPickCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CherryPickCommandTest.java
@@ -175,7 +175,8 @@ public class CherryPickCommandTest extends RepositoryTestCase {
 
 			assertEquals(CherryPickStatus.CONFLICTING, result.getStatus());
 			assertTrue(new File(db.getDirectory(), Constants.MERGE_MSG).exists());
-			assertEquals("side\n\nConflicts:\n\ta\n", db.readMergeCommitMsg());
+			assertEquals("side\n\n# Conflicts:\n#\ta\n",
+					db.readMergeCommitMsg());
 			assertTrue(new File(db.getDirectory(), Constants.CHERRY_PICK_HEAD)
 					.exists());
 			assertEquals(sideCommit.getId(), db.readCherryPickHead());
@@ -207,7 +208,7 @@ public class CherryPickCommandTest extends RepositoryTestCase {
 		String expected = "<<<<<<< master\na(master)\n=======\na(side)\n>>>>>>> 527460a side\n";
 		assertEquals(expected, read("a"));
 		assertTrue(new File(db.getDirectory(), Constants.MERGE_MSG).exists());
-		assertEquals("side\n\nConflicts:\n\ta\n", db.readMergeCommitMsg());
+		assertEquals("side\n\n# Conflicts:\n#\ta\n", db.readMergeCommitMsg());
 		assertFalse(new File(db.getDirectory(), Constants.CHERRY_PICK_HEAD)
 				.exists());
 		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
index 127c404230..64475f5d50 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
@@ -554,7 +554,7 @@ public class MergeCommandTest extends RepositoryTestCase {
 			git.merge().include(sideBranch)
 					.setStrategy(MergeStrategy.RESOLVE).call();
 
-			assertEquals("Merge branch 'side'\n\nConflicts:\n\ta\n",
+			assertEquals("Merge branch 'side'\n\n# Conflicts:\n#\ta\n",
 					db.readMergeCommitMsg());
 		}
 
@@ -1787,7 +1787,7 @@ public class MergeCommandTest extends RepositoryTestCase {
 							+ dateFormatter.formatDate(third
 									.getAuthorIdent()) + "\n\n\tthird commit\n",
 					db.readSquashCommitMsg());
-			assertEquals("\nConflicts:\n\tfile2\n", db.readMergeCommitMsg());
+			assertEquals("\n# Conflicts:\n#\tfile2\n", db.readMergeCommitMsg());
 
 			Status stat = git.status().call();
 			assertEquals(Sets.of("file2"), stat.getConflicting());
@@ -2013,7 +2013,7 @@ public class MergeCommandTest extends RepositoryTestCase {
 			git.merge().include(sideBranch).setStrategy(MergeStrategy.RESOLVE)
 					.setMessage("user message").call();
 
-			assertEquals("user message\n\nConflicts:\n\ta\n",
+			assertEquals("user message\n\n# Conflicts:\n#\ta\n",
 					db.readMergeCommitMsg());
 		}
 	}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RevertCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RevertCommandTest.java
index cfa8486ac5..d833f12d26 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RevertCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RevertCommandTest.java
@@ -232,7 +232,7 @@ public class RevertCommandTest extends RepositoryTestCase {
 			assertTrue(new File(db.getDirectory(), Constants.MERGE_MSG).exists());
 			assertEquals("Revert \"" + sideCommit.getShortMessage()
 					+ "\"\n\nThis reverts commit " + sideCommit.getId().getName()
-					+ ".\n\nConflicts:\n\ta\n",
+					+ ".\n\n# Conflicts:\n#\ta\n",
 					db.readMergeCommitMsg());
 			assertTrue(new File(db.getDirectory(), Constants.REVERT_HEAD)
 					.exists());
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeMessageFormatterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeMessageFormatterTest.java
index dedb56c7b0..a2576cc677 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeMessageFormatterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeMessageFormatterTest.java
@@ -13,6 +13,7 @@ import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.List;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdRef;
@@ -157,8 +158,8 @@ public class MergeMessageFormatterTest extends SampleDataRepositoryTestCase {
 	public void testFormatWithConflictsNoFooter() {
 		String originalMessage = "Header Line\n\nCommit body\n";
 		String message = formatter.formatWithConflicts(originalMessage,
-				Arrays.asList(new String[] { "path1" }));
-		assertEquals("Header Line\n\nCommit body\n\nConflicts:\n\tpath1\n",
+				List.of("path1"), '#');
+		assertEquals("Header Line\n\nCommit body\n\n# Conflicts:\n#\tpath1\n",
 				message);
 	}
 
@@ -166,8 +167,17 @@ public class MergeMessageFormatterTest extends SampleDataRepositoryTestCase {
 	public void testFormatWithConflictsNoFooterNoLineBreak() {
 		String originalMessage = "Header Line\n\nCommit body";
 		String message = formatter.formatWithConflicts(originalMessage,
-				Arrays.asList(new String[] { "path1" }));
-		assertEquals("Header Line\n\nCommit body\n\nConflicts:\n\tpath1\n",
+				List.of("path1"), '#');
+		assertEquals("Header Line\n\nCommit body\n\n# Conflicts:\n#\tpath1\n",
+				message);
+	}
+
+	@Test
+	public void testFormatWithConflictsCustomCharacter() {
+		String originalMessage = "Header Line\n\nCommit body";
+		String message = formatter.formatWithConflicts(originalMessage,
+				List.of("path1"), ';');
+		assertEquals("Header Line\n\nCommit body\n\n; Conflicts:\n;\tpath1\n",
 				message);
 	}
 
@@ -176,9 +186,9 @@ public class MergeMessageFormatterTest extends SampleDataRepositoryTestCase {
 		String originalMessage = "Header Line\n\nCommit body\n\nChangeId:"
 				+ " I123456789123456789123456789123456789\nBug:1234567\n";
 		String message = formatter.formatWithConflicts(originalMessage,
-				Arrays.asList(new String[] { "path1" }));
+				List.of("path1"), '#');
 		assertEquals(
-				"Header Line\n\nCommit body\n\nConflicts:\n\tpath1\n\n"
+				"Header Line\n\nCommit body\n\n# Conflicts:\n#\tpath1\n\n"
 						+ "ChangeId: I123456789123456789123456789123456789\nBug:1234567\n",
 				message);
 	}
@@ -188,9 +198,9 @@ public class MergeMessageFormatterTest extends SampleDataRepositoryTestCase {
 		String originalMessage = "Header Line\n\nCommit body\nBug:1234567\nMore Body\n\nChangeId:"
 				+ " I123456789123456789123456789123456789\nBug:1234567\n";
 		String message = formatter.formatWithConflicts(originalMessage,
-				Arrays.asList(new String[] { "path1" }));
+				List.of("path1"), '#');
 		assertEquals(
-				"Header Line\n\nCommit body\nBug:1234567\nMore Body\n\nConflicts:\n\tpath1\n\n"
+				"Header Line\n\nCommit body\nBug:1234567\nMore Body\n\n# Conflicts:\n#\tpath1\n\n"
 						+ "ChangeId: I123456789123456789123456789123456789\nBug:1234567\n",
 				message);
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java
index 7922f9e729..8ee1e965e9 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java
@@ -183,7 +183,7 @@ public class CherryPickCommand extends GitCommand<CherryPickResult> {
 					if (unmergedPaths != null) {
 						message = new MergeMessageFormatter()
 							.formatWithConflicts(srcCommit.getFullMessage(),
-										unmergedPaths);
+										unmergedPaths, '#');
 					} else {
 						message = srcCommit.getFullMessage();
 					}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java
index ef56d802c8..ce068b6306 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java
@@ -405,7 +405,7 @@ public class MergeCommand extends GitCommand<MergeResult> {
 							failingPaths, null);
 				}
 				String mergeMessageWithConflicts = new MergeMessageFormatter()
-						.formatWithConflicts(mergeMessage, unmergedPaths);
+						.formatWithConflicts(mergeMessage, unmergedPaths, '#');
 				repo.writeMergeCommitMsg(mergeMessageWithConflicts);
 				return new MergeResult(null, merger.getBaseCommitId(),
 						new ObjectId[] { headCommit.getId(),
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java
index 22ef4d0a32..02f581cbb6 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java
@@ -183,8 +183,8 @@ public class RevertCommand extends GitCommand<RevCommit> {
 								merger.getMergeResults(), failingPaths, null);
 					if (!merger.failed() && !unmergedPaths.isEmpty()) {
 						String message = new MergeMessageFormatter()
-						.formatWithConflicts(newMessage,
-								merger.getUnmergedPaths());
+								.formatWithConflicts(newMessage,
+										merger.getUnmergedPaths(), '#');
 						repo.writeRevertHead(srcCommit.getId());
 						repo.writeMergeCommitMsg(message);
 					}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeMessageFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeMessageFormatter.java
index f7966a267f..e0c083f55c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeMessageFormatter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeMessageFormatter.java
@@ -92,36 +92,62 @@ public class MergeMessageFormatter {
 	}
 
 	/**
-	 * Add section with conflicting paths to merge message.
+	 * Add section with conflicting paths to merge message. Lines are prefixed
+	 * with a hash.
 	 *
 	 * @param message
 	 *            the original merge message
 	 * @param conflictingPaths
 	 *            the paths with conflicts
 	 * @return merge message with conflicting paths added
+	 * @deprecated since 6.1; use
+	 *             {@link #formatWithConflicts(String, Iterable, char)} instead
 	 */
+	@Deprecated
 	public String formatWithConflicts(String message,
 			List<String> conflictingPaths) {
+		return formatWithConflicts(message, conflictingPaths, '#');
+	}
+
+	/**
+	 * Add section with conflicting paths to merge message.
+	 *
+	 * @param message
+	 *            the original merge message
+	 * @param conflictingPaths
+	 *            the paths with conflicts
+	 * @param commentChar
+	 *            comment character to use for prefixing the conflict lines
+	 * @return merge message with conflicting paths added
+	 * @since 6.1
+	 */
+	public String formatWithConflicts(String message,
+			Iterable<String> conflictingPaths, char commentChar) {
 		StringBuilder sb = new StringBuilder();
 		String[] lines = message.split("\n"); //$NON-NLS-1$
 		int firstFooterLine = ChangeIdUtil.indexOfFirstFooterLine(lines);
-		for (int i = 0; i < firstFooterLine; i++)
+		for (int i = 0; i < firstFooterLine; i++) {
 			sb.append(lines[i]).append('\n');
-		if (firstFooterLine == lines.length && message.length() != 0)
+		}
+		if (firstFooterLine == lines.length && message.length() != 0) {
 			sb.append('\n');
-		addConflictsMessage(conflictingPaths, sb);
-		if (firstFooterLine < lines.length)
+		}
+		addConflictsMessage(conflictingPaths, sb, commentChar);
+		if (firstFooterLine < lines.length) {
 			sb.append('\n');
-		for (int i = firstFooterLine; i < lines.length; i++)
+		}
+		for (int i = firstFooterLine; i < lines.length; i++) {
 			sb.append(lines[i]).append('\n');
+		}
 		return sb.toString();
 	}
 
-	private static void addConflictsMessage(List<String> conflictingPaths,
-			StringBuilder sb) {
-		sb.append("Conflicts:\n"); //$NON-NLS-1$
+	private static void addConflictsMessage(Iterable<String> conflictingPaths,
+			StringBuilder sb, char commentChar) {
+		sb.append(commentChar).append(" Conflicts:\n"); //$NON-NLS-1$
 		for (String conflictingPath : conflictingPaths) {
-			sb.append('\t').append(conflictingPath).append('\n');
+			sb.append(commentChar).append('\t').append(conflictingPath)
+					.append('\n');
 		}
 	}