From 1fd15e40cc7bf67c30849abc3030de8568246a4e Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 16 Jan 2022 15:50:00 +0100 Subject: [PATCH] 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 --- .../jgit/api/CherryPickCommandTest.java | 5 +- .../eclipse/jgit/api/MergeCommandTest.java | 6 +-- .../eclipse/jgit/api/RevertCommandTest.java | 2 +- .../jgit/merge/MergeMessageFormatterTest.java | 26 +++++++---- .../eclipse/jgit/api/CherryPickCommand.java | 2 +- .../org/eclipse/jgit/api/MergeCommand.java | 2 +- .../org/eclipse/jgit/api/RevertCommand.java | 4 +- .../jgit/merge/MergeMessageFormatter.java | 46 +++++++++++++++---- 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 { 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 { 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 { 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 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 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 conflictingPaths, - StringBuilder sb) { - sb.append("Conflicts:\n"); //$NON-NLS-1$ + private static void addConflictsMessage(Iterable 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'); } } -- 2.39.5