From 462c57ec8d94b23e21479394ee7880e0fc1bb832 Mon Sep 17 00:00:00 2001 From: Haamed Gheibi Date: Mon, 24 Jul 2023 17:50:34 -0700 Subject: [PATCH] Merge: Add diff3 style merge conflict formatter. Add base section to the merge conflict hunks. Bug: 442284 Change-Id: I977b43e7dd8119d6b72d11f09c4e8ec241750383 --- .../jgit/merge/MergeAlgorithmTest.java | 144 ++++++++++++++---- .../eclipse/jgit/merge/MergeAlgorithm.java | 12 ++ .../org/eclipse/jgit/merge/MergeChunk.java | 14 +- .../eclipse/jgit/merge/MergeFormatter.java | 64 ++++++++ .../jgit/merge/MergeFormatterPass.java | 72 +++++++-- 5 files changed, 258 insertions(+), 48 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 5f4331b04d..680a2d5d72 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 @@ -47,7 +47,10 @@ public class MergeAlgorithmTest { @Test public void testTwoConflictingModifications() throws IOException { assertEquals(t("aZdefghij"), - merge("abcdefghij", "abZdefghij", "aZZdefghij")); + merge("abcdefghij", "abZdefghij", "aZZdefghij", false)); + + assertEquals(t("aZdefghij"), + merge("abcdefghij", "abZdefghij", "aZZdefghij", true)); } /** @@ -60,7 +63,10 @@ public class MergeAlgorithmTest { @Test public void testOneAgainstTwoConflictingModifications() throws IOException { assertEquals(t("aZZefghij"), - merge("abcdefghij", "aZZZefghij", "aZcZefghij")); + merge("abcdefghij", "aZZZefghij", "aZcZefghij", false)); + + assertEquals(t("aZZefghij"), + merge("abcdefghij", "aZZZefghij", "aZcZefghij", true)); } /** @@ -72,7 +78,10 @@ public class MergeAlgorithmTest { @Test public void testNoAgainstOneModification() throws IOException { assertEquals(t("aZcZefghij"), - merge("abcdefghij", "abcdefghij", "aZcZefghij")); + merge("abcdefghij", "abcdefghij", "aZcZefghij", false)); + + assertEquals(t("aZcZefghij"), + merge("abcdefghij", "abcdefghij", "aZcZefghij", true)); } /** @@ -84,7 +93,10 @@ public class MergeAlgorithmTest { @Test public void testTwoNonConflictingModifications() throws IOException { assertEquals(t("YbZdefghij"), - merge("abcdefghij", "abZdefghij", "Ybcdefghij")); + merge("abcdefghij", "abZdefghij", "Ybcdefghij", false)); + + assertEquals(t("YbZdefghij"), + merge("abcdefghij", "abZdefghij", "Ybcdefghij", true)); } /** @@ -96,7 +108,10 @@ public class MergeAlgorithmTest { @Test public void testTwoComplicatedModifications() throws IOException { assertEquals(t("a"), - merge("abcdefghij", "aZZZZfZhZj", "abYdYYYYiY")); + merge("abcdefghij", "aZZZZfZhZj", "abYdYYYYiY", false)); + + assertEquals(t("a"), + merge("abcdefghij", "aZZZZfZhZj", "abYdYYYYiY", true)); } /** @@ -109,7 +124,9 @@ public class MergeAlgorithmTest { @Test public void testTwoModificationsWithSharedDelete() throws IOException { assertEquals(t("Cb}n}"), - merge("ab}n}n}", "ab}n}", "Cb}n}")); + merge("ab}n}n}", "ab}n}", "Cb}n}", false)); + + assertEquals(t("Cb}n}"), merge("ab}n}n}", "ab}n}", "Cb}n}", true)); } /** @@ -122,7 +139,11 @@ public class MergeAlgorithmTest { @Test public void testModificationsWithMiddleInsert() throws IOException { assertEquals(t("aBcd123123uvwxPq"), - merge("abcd123uvwxpq", "aBcd123123uvwxPq", "abcd123123uvwxpq")); + merge("abcd123uvwxpq", "aBcd123123uvwxPq", "abcd123123uvwxpq", + false)); + + assertEquals(t("aBcd123123uvwxPq"), merge("abcd123uvwxpq", + "aBcd123123uvwxPq", "abcd123123uvwxpq", true)); } /** @@ -135,7 +156,10 @@ public class MergeAlgorithmTest { @Test public void testModificationsWithMiddleDelete() throws IOException { assertEquals(t("Abz}z123Q"), - merge("abz}z}z123q", "Abz}z123Q", "abz}z123q")); + merge("abz}z}z123q", "Abz}z123Q", "abz}z123q", false)); + + assertEquals(t("Abz}z123Q"), + merge("abz}z}z123q", "Abz}z123Q", "abz}z123q", true)); } /** @@ -146,7 +170,10 @@ public class MergeAlgorithmTest { @Test public void testConflictAtStart() throws IOException { assertEquals(t("bcdefghij"), - merge("abcdefghij", "Zbcdefghij", "Ybcdefghij")); + merge("abcdefghij", "Zbcdefghij", "Ybcdefghij", false)); + + assertEquals(t("bcdefghij"), + merge("abcdefghij", "Zbcdefghij", "Ybcdefghij", true)); } /** @@ -157,7 +184,10 @@ public class MergeAlgorithmTest { @Test public void testConflictAtEnd() throws IOException { assertEquals(t("abcdefghi"), - merge("abcdefghij", "abcdefghiZ", "abcdefghiY")); + merge("abcdefghij", "abcdefghiZ", "abcdefghiY", false)); + + assertEquals(t("abcdefghi"), + merge("abcdefghij", "abcdefghiZ", "abcdefghiY", true)); } /** @@ -169,7 +199,10 @@ public class MergeAlgorithmTest { @Test public void testSameModification() throws IOException { assertEquals(t("abZdefghij"), - merge("abcdefghij", "abZdefghij", "abZdefghij")); + merge("abcdefghij", "abZdefghij", "abZdefghij", false)); + + assertEquals(t("abZdefghij"), + merge("abcdefghij", "abZdefghij", "abZdefghij", true)); } /** @@ -181,27 +214,36 @@ public class MergeAlgorithmTest { @Test public void testDeleteVsModify() throws IOException { assertEquals(t("ab<=Z>defghij"), - merge("abcdefghij", "abdefghij", "abZdefghij")); + merge("abcdefghij", "abdefghij", "abZdefghij", false)); + + assertEquals(t("ab<|c=Z>defghij"), + merge("abcdefghij", "abdefghij", "abZdefghij", true)); } @Test public void testInsertVsModify() throws IOException { - assertEquals(t("a"), merge("ab", "abZ", "aXY")); + assertEquals(t("a"), merge("ab", "abZ", "aXY", false)); + assertEquals(t("a"), merge("ab", "abZ", "aXY", true)); } @Test public void testAdjacentModifications() throws IOException { - assertEquals(t("ad"), merge("abcd", "aZcd", "abYd")); + assertEquals(t("ad"), merge("abcd", "aZcd", "abYd", false)); + assertEquals(t("ad"), merge("abcd", "aZcd", "abYd", true)); } @Test public void testSeparateModifications() throws IOException { - assertEquals(t("aZcYe"), merge("abcde", "aZcde", "abcYe")); + assertEquals(t("aZcYe"), merge("abcde", "aZcde", "abcYe", false)); + assertEquals(t("aZcYe"), merge("abcde", "aZcde", "abcYe", true)); } @Test public void testBlankLines() throws IOException { - assertEquals(t("aZc\nYe"), merge("abc\nde", "aZc\nde", "abc\nYe")); + assertEquals(t("aZc\nYe"), + merge("abc\nde", "aZc\nde", "abc\nYe", false)); + assertEquals(t("aZc\nYe"), + merge("abc\nde", "aZc\nde", "abc\nYe", true)); } /** @@ -214,11 +256,22 @@ public class MergeAlgorithmTest { */ @Test public void testTwoSimilarModsAndOneInsert() throws IOException { - assertEquals(t("aBcDde"), merge("abcde", "aBcde", "aBcDde")); - assertEquals(t("IAAAJCAB"), merge("iACAB", "IACAB", "IAAAJCAB")); - assertEquals(t("HIAAAJCAB"), merge("HiACAB", "HIACAB", "HIAAAJCAB")); + assertEquals(t("aBcDde"), merge("abcde", "aBcde", "aBcDde", false)); + assertEquals(t("aBcDde"), merge("abcde", "aBcde", "aBcDde", true)); + + assertEquals(t("IAAAJCAB"), merge("iACAB", "IACAB", "IAAAJCAB", false)); + assertEquals(t("IAAAJCAB"), merge("iACAB", "IACAB", "IAAAJCAB", true)); + + assertEquals(t("HIAAAJCAB"), + merge("HiACAB", "HIACAB", "HIAAAJCAB", false)); + assertEquals(t("HIAAAJCAB"), + merge("HiACAB", "HIACAB", "HIAAAJCAB", true)); + + assertEquals(t("AGADEFHIAAAJCAB"), + merge("AGADEFHiACAB", "AGADEFHIACAB", "AGADEFHIAAAJCAB", + false)); assertEquals(t("AGADEFHIAAAJCAB"), - merge("AGADEFHiACAB", "AGADEFHIACAB", "AGADEFHIAAAJCAB")); + merge("AGADEFHiACAB", "AGADEFHIACAB", "AGADEFHIAAAJCAB", true)); } /** @@ -232,18 +285,28 @@ public class MergeAlgorithmTest { @Test public void testTwoSimilarModsAndOneInsertAtEnd() throws IOException { Assume.assumeTrue(newlineAtEnd); - assertEquals(t("IAAJ"), merge("iA", "IA", "IAAJ")); - assertEquals(t("IAJ"), merge("iA", "IA", "IAJ")); - assertEquals(t("IAAAJ"), merge("iA", "IA", "IAAAJ")); + assertEquals(t("IAAJ"), merge("iA", "IA", "IAAJ", false)); + assertEquals(t("IAAJ"), merge("iA", "IA", "IAAJ", true)); + + assertEquals(t("IAJ"), merge("iA", "IA", "IAJ", false)); + assertEquals(t("IAJ"), merge("iA", "IA", "IAJ", true)); + + assertEquals(t("IAAAJ"), merge("iA", "IA", "IAAAJ", false)); + assertEquals(t("IAAAJ"), merge("iA", "IA", "IAAAJ", true)); } @Test public void testTwoSimilarModsAndOneInsertAtEndNoNewlineAtEnd() throws IOException { Assume.assumeFalse(newlineAtEnd); - assertEquals(t("I"), merge("iA", "IA", "IAAJ")); - assertEquals(t("I"), merge("iA", "IA", "IAJ")); - assertEquals(t("I"), merge("iA", "IA", "IAAAJ")); + assertEquals(t("I"), merge("iA", "IA", "IAAJ", false)); + assertEquals(t("I"), merge("iA", "IA", "IAAJ", true)); + + assertEquals(t("I"), merge("iA", "IA", "IAJ", false)); + assertEquals(t("I"), merge("iA", "IA", "IAJ", true)); + + assertEquals(t("I"), merge("iA", "IA", "IAAAJ", false)); + assertEquals(t("I"), merge("iA", "IA", "IAAAJ", true)); } /** @@ -254,22 +317,34 @@ public class MergeAlgorithmTest { @Test public void testEmptyTexts() throws IOException { // test modification against deletion - assertEquals(t(""), merge("A", "AB", "")); - assertEquals(t("<=AB>"), merge("A", "", "AB")); + assertEquals(t(""), merge("A", "AB", "", false)); + assertEquals(t(""), merge("A", "AB", "", true)); + + assertEquals(t("<=AB>"), merge("A", "", "AB", false)); + assertEquals(t("<|A=AB>"), merge("A", "", "AB", true)); // test unmodified against deletion - assertEquals(t(""), merge("AB", "AB", "")); - assertEquals(t(""), merge("AB", "", "AB")); + assertEquals(t(""), merge("AB", "AB", "", false)); + assertEquals(t(""), merge("AB", "AB", "", true)); + + assertEquals(t(""), merge("AB", "", "AB", false)); + assertEquals(t(""), merge("AB", "", "AB", true)); // test deletion against deletion - assertEquals(t(""), merge("AB", "", "")); + assertEquals(t(""), merge("AB", "", "", false)); + assertEquals(t(""), merge("AB", "", "", true)); } - private String merge(String commonBase, String ours, String theirs) throws IOException { + private String merge(String commonBase, String ours, String theirs, + boolean diff3) throws IOException { MergeResult r = new MergeAlgorithm().merge(RawTextComparator.DEFAULT, T(commonBase), T(ours), T(theirs)); ByteArrayOutputStream bo=new ByteArrayOutputStream(50); - fmt.formatMerge(bo, r, "B", "O", "T", UTF_8); + if (diff3) { + fmt.formatMergeDiff3(bo, r, "B", "O", "T", UTF_8); + } else { + fmt.formatMerge(bo, r, "B", "O", "T", UTF_8); + } return new String(bo.toByteArray(), UTF_8); } @@ -284,6 +359,9 @@ public class MergeAlgorithmTest { case '=': r.append("=======\n"); break; + case '|': + r.append("||||||| B\n"); + break; case '>': r.append(">>>>>>> T\n"); break; 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 abca813c11..b902492366 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java @@ -127,6 +127,8 @@ public final class MergeAlgorithm { // Let their complete content conflict with empty text result.add(1, 0, 0, ConflictState.FIRST_CONFLICTING_RANGE); + result.add(0, 0, base.size(), + ConflictState.BASE_CONFLICTING_RANGE); result.add(2, 0, theirs.size(), ConflictState.NEXT_CONFLICTING_RANGE); break; @@ -155,6 +157,8 @@ public final class MergeAlgorithm { // Let our complete content conflict with empty text result.add(1, 0, ours.size(), ConflictState.FIRST_CONFLICTING_RANGE); + result.add(0, 0, base.size(), + ConflictState.BASE_CONFLICTING_RANGE); result.add(2, 0, 0, ConflictState.NEXT_CONFLICTING_RANGE); break; } @@ -324,6 +328,14 @@ public final class MergeAlgorithm { result.add(1, oursBeginB + commonPrefix, oursEndB - commonSuffix, ConflictState.FIRST_CONFLICTING_RANGE); + + int baseBegin = Math.min(oursBeginB, theirsBeginB) + + commonPrefix; + int baseEnd = Math.min(base.size(), + Math.max(oursEndB, theirsEndB)) - commonSuffix; + result.add(0, baseBegin, baseEnd, + ConflictState.BASE_CONFLICTING_RANGE); + result.add(2, theirsBeginB + commonPrefix, theirsEndB - commonSuffix, ConflictState.NEXT_CONFLICTING_RANGE); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeChunk.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeChunk.java index ca998e30bc..7102aa6998 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeChunk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeChunk.java @@ -29,14 +29,22 @@ public class MergeChunk { NO_CONFLICT, /** - * This chunk does belong to a conflict and is the first one of the + * This chunk does belong to a conflict and is the ours section of the * conflicting chunks */ FIRST_CONFLICTING_RANGE, /** - * This chunk does belong to a conflict but is not the first one of the - * conflicting chunks. It's a subsequent one. + * This chunk does belong to a conflict and is the base section of the + * conflicting chunks + * + * @since 6.7 + */ + BASE_CONFLICTING_RANGE, + + /** + * This chunk does belong to a conflict and is the theirs section of + * the conflicting chunks. It's a subsequent one. */ NEXT_CONFLICTING_RANGE } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeFormatter.java index baf5d27714..a35b30eb01 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeFormatter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeFormatter.java @@ -82,6 +82,35 @@ public class MergeFormatter { new MergeFormatterPass(out, res, seqName, charset).formatMerge(); } + /** + * Formats the results of a merge of {@link org.eclipse.jgit.diff.RawText} + * objects in a Git conformant way using diff3 style. This method also + * assumes that the {@link org.eclipse.jgit.diff.RawText} objects being + * merged are line oriented files which use LF as delimiter. This method + * will also use LF to separate chunks and conflict metadata, therefore it + * fits only to texts that are LF-separated lines. + * + * @param out + * the output stream where to write the textual presentation + * @param res + * the merge result which should be presented + * @param seqName + * When a conflict is reported each conflicting range will get a + * name. This name is following the "<<<<<<< + * ", "|||||||" or ">>>>>>> " conflict + * markers. The names for the sequences are given in this list + * @param charset + * the character set used when writing conflict metadata + * @throws java.io.IOException + * if an IO error occurred + * @since 6.7 + */ + public void formatMergeDiff3(OutputStream out, + MergeResult res, List seqName, Charset charset) + throws IOException { + new MergeFormatterPass(out, res, seqName, charset, true).formatMerge(); + } + /** * Formats the results of a merge of exactly two * {@link org.eclipse.jgit.diff.RawText} objects in a Git conformant way. @@ -150,4 +179,39 @@ public class MergeFormatter { names.add(theirsName); formatMerge(out, res, names, charset); } + + /** + * Formats the results of a merge of three + * {@link org.eclipse.jgit.diff.RawText} objects in a Git conformant way, + * using diff-3 style. This convenience method accepts the names for the + * three sequences (base and the two merged sequences) as explicit + * parameters and doesn't require the caller to specify a List + * + * @param out + * the {@link java.io.OutputStream} where to write the textual + * presentation + * @param res + * the merge result which should be presented + * @param baseName + * the name ranges from the base should get + * @param oursName + * the name ranges from ours should get + * @param theirsName + * the name ranges from theirs should get + * @param charset + * the character set used when writing conflict metadata + * @throws java.io.IOException + * if an IO error occurred + * @since 6.7 + */ + @SuppressWarnings("unchecked") + public void formatMergeDiff3(OutputStream out, + MergeResult res, String baseName, String oursName, + String theirsName, Charset charset) throws IOException { + List names = new ArrayList<>(3); + names.add(baseName); + names.add(oursName); + names.add(theirsName); + formatMergeDiff3(out, res, names, charset); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeFormatterPass.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeFormatterPass.java index f09b343007..5e80b5fb23 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeFormatterPass.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeFormatterPass.java @@ -31,6 +31,8 @@ class MergeFormatterPass { private final boolean threeWayMerge; + private final boolean writeBase; // diff3-style requested + private String lastConflictingName; // is set to non-null whenever we are in // a conflict @@ -50,22 +52,47 @@ class MergeFormatterPass { */ MergeFormatterPass(OutputStream out, MergeResult res, List seqName, Charset charset) { + this(out, res, seqName, charset, false); + } + + /** + * @param out + * the {@link java.io.OutputStream} where to write the textual + * presentation + * @param res + * the merge result which should be presented + * @param seqName + * When a conflict is reported each conflicting range will get a + * name. This name is following the "<<<<<<< + * ", "|||||||" or ">>>>>>> " conflict + * markers. The names for the sequences are given in this list + * @param charset + * the character set used when writing conflict metadata + * @param writeBase + * base's contribution should be written in conflicts + */ + MergeFormatterPass(OutputStream out, MergeResult res, + List seqName, Charset charset, boolean writeBase) { this.out = new EolAwareOutputStream(out); this.res = res; this.seqName = seqName; this.charset = charset; this.threeWayMerge = (res.getSequences().size() == 3); + this.writeBase = writeBase; } void formatMerge() throws IOException { boolean missingNewlineAtEnd = false; for (MergeChunk chunk : res) { - RawText seq = res.getSequences().get(chunk.getSequenceIndex()); - writeConflictMetadata(chunk); - // the lines with conflict-metadata are written. Now write the chunk - for (int i = chunk.getBegin(); i < chunk.getEnd(); i++) - writeLine(seq, i); - missingNewlineAtEnd = seq.isMissingNewlineAtEnd(); + if (!isBase(chunk) || writeBase) { + RawText seq = res.getSequences().get(chunk.getSequenceIndex()); + writeConflictMetadata(chunk); + // the lines with conflict-metadata are written. Now write the + // chunk + for (int i = chunk.getBegin(); i < chunk.getEnd(); i++) + writeLine(seq, i); + missingNewlineAtEnd = seq.isMissingNewlineAtEnd(); + } } // one possible leftover: if the merge result ended with a conflict we // have to close the last conflict here @@ -77,16 +104,19 @@ class MergeFormatterPass { private void writeConflictMetadata(MergeChunk chunk) throws IOException { if (lastConflictingName != null - && chunk.getConflictState() != ConflictState.NEXT_CONFLICTING_RANGE) { - // found the end of an conflict + && !isTheirs(chunk) && !isBase(chunk)) { + // found the end of a conflict writeConflictEnd(); } - if (chunk.getConflictState() == ConflictState.FIRST_CONFLICTING_RANGE) { - // found the start of an conflict + if (isOurs(chunk)) { + // found the start of a conflict writeConflictStart(chunk); - } else if (chunk.getConflictState() == ConflictState.NEXT_CONFLICTING_RANGE) { - // found another conflicting chunk + } else if (isTheirs(chunk)) { + // found the theirs conflicting chunk writeConflictChange(chunk); + } else if (isBase(chunk)) { + // found the base conflicting chunk + writeConflictBase(chunk); } } @@ -113,6 +143,11 @@ class MergeFormatterPass { + lastConflictingName); } + private void writeConflictBase(MergeChunk chunk) throws IOException { + lastConflictingName = seqName.get(chunk.getSequenceIndex()); + writeln("||||||| " + lastConflictingName); //$NON-NLS-1$ + } + private void writeln(String s) throws IOException { out.beginln(); out.write((s + "\n").getBytes(charset)); //$NON-NLS-1$ @@ -125,4 +160,17 @@ class MergeFormatterPass { if (out.isBeginln()) out.write('\n'); } + + private boolean isBase(MergeChunk chunk) { + return chunk.getConflictState() == ConflictState.BASE_CONFLICTING_RANGE; + } + + private boolean isOurs(MergeChunk chunk) { + return chunk + .getConflictState() == ConflictState.FIRST_CONFLICTING_RANGE; + } + + private boolean isTheirs(MergeChunk chunk) { + return chunk.getConflictState() == ConflictState.NEXT_CONFLICTING_RANGE; + } } -- 2.39.5