diff --git a/platform/diff-impl/src/com/intellij/diff/comparison/ByLine.java b/platform/diff-impl/src/com/intellij/diff/comparison/ByLine.java index 6abdc3781529..ae75bbaf8f33 100644 --- a/platform/diff-impl/src/com/intellij/diff/comparison/ByLine.java +++ b/platform/diff-impl/src/com/intellij/diff/comparison/ByLine.java @@ -15,7 +15,7 @@ */ package com.intellij.diff.comparison; -import com.intellij.diff.comparison.iterables.DiffIterableUtil.*; +import com.intellij.diff.comparison.iterables.DiffIterableUtil.ExpandChangeBuilder; import com.intellij.diff.comparison.iterables.FairDiffIterable; import com.intellij.diff.util.MergeRange; import com.intellij.diff.util.Range; @@ -24,6 +24,7 @@ import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.util.text.StringUtil; import com.intellij.util.containers.ContainerUtil; +import gnu.trove.Equality; import gnu.trove.TIntArrayList; import org.jetbrains.annotations.NotNull; @@ -33,7 +34,8 @@ import java.util.List; import static com.intellij.diff.comparison.ComparisonPolicy.IGNORE_WHITESPACES; import static com.intellij.diff.comparison.TrimUtil.trimEnd; import static com.intellij.diff.comparison.TrimUtil.trimStart; -import static com.intellij.diff.comparison.iterables.DiffIterableUtil.*; +import static com.intellij.diff.comparison.iterables.DiffIterableUtil.diff; +import static com.intellij.diff.comparison.iterables.DiffIterableUtil.fair; import static com.intellij.openapi.util.text.StringUtil.isWhiteSpace; public class ByLine { @@ -70,7 +72,7 @@ public class ByLine { if (policy == IGNORE_WHITESPACES) { FairDiffIterable changes = compareSmart(lines1, lines2, indicator); changes = optimizeLineChunks(lines1, lines2, changes, indicator); - return expandRanges(lines1, lines2, changes, indicator); + return correctChangesSecondStepIW(lines1, lines2, changes); } else { List iwLines1 = convertMode(lines1, IGNORE_WHITESPACES); @@ -109,6 +111,23 @@ public class ByLine { private static FairDiffIterable correctChangesSecondStep(@NotNull final List lines1, @NotNull final List lines2, @NotNull final FairDiffIterable changes) { + return doCorrectChangesSecondStep(lines1, lines2, changes, + Equality.CANONICAL); + } + + @NotNull + private static FairDiffIterable correctChangesSecondStepIW(@NotNull final List lines1, + @NotNull final List lines2, + @NotNull final FairDiffIterable changes) { + return doCorrectChangesSecondStep(lines1, lines2, changes, + (l1, l2) -> StringUtil.equals(l1.getContent(), l2.getContent())); + } + + @NotNull + private static FairDiffIterable doCorrectChangesSecondStep(@NotNull final List lines1, + @NotNull final List lines2, + @NotNull final FairDiffIterable changes, + @NotNull final Equality maximisingEquality) { /* * We want to fix invalid matching here: * @@ -153,7 +172,7 @@ public class ByLine { Line line2 = lines2.get(index2); if (!StringUtil.equalsIgnoreWhitespaces(sample, line1.getContent())) { - if (line1.equals(line2)) { + if (maximisingEquality.equals(line1, line2)) { flush(index1, index2); builder.markEqual(index1, index2); } @@ -194,13 +213,24 @@ public class ByLine { } private void alignExactMatching(TIntArrayList subLines1, TIntArrayList subLines2) { - if (subLines1.size() == subLines2.size()) return; - int n = Math.max(subLines1.size(), subLines2.size()); - if (n > 10) return; // we use brute-force algorithm (C_n_k). This will limit search space by ~250 cases. + boolean skipAligning = n > 10 || // we use brute-force algorithm (C_n_k). This will limit search space by ~250 cases. + subLines1.size() == subLines2.size(); // nothing to do + + if (skipAligning) { + int count = Math.min(subLines1.size(), subLines2.size()); + for (int i = 0; i < count; i++) { + int index1 = subLines1.get(i); + int index2 = subLines2.get(i); + if (lines1.get(index1).equals(lines2.get(index2))) { + builder.markEqual(index1, index2); + } + } + return; + } if (subLines1.size() < subLines2.size()) { - int[] matching = getBestMatchingAlignment(subLines1, subLines2, lines1, lines2); + int[] matching = getBestMatchingAlignment(subLines1, subLines2, lines1, lines2, maximisingEquality); for (int i = 0; i < subLines1.size(); i++) { int index1 = subLines1.get(i); int index2 = subLines2.get(matching[i]); @@ -210,7 +240,7 @@ public class ByLine { } } else { - int[] matching = getBestMatchingAlignment(subLines2, subLines1, lines2, lines1); + int[] matching = getBestMatchingAlignment(subLines2, subLines1, lines2, lines1, maximisingEquality); for (int i = 0; i < subLines2.size(); i++) { int index1 = subLines1.get(matching[i]); int index2 = subLines2.get(i); @@ -229,7 +259,8 @@ public class ByLine { private static int[] getBestMatchingAlignment(@NotNull final TIntArrayList subLines1, @NotNull final TIntArrayList subLines2, @NotNull final List lines1, - @NotNull final List lines2) { + @NotNull final List lines2, + @NotNull final Equality maximisingEquality) { assert subLines1.size() < subLines2.size(); final int size = subLines1.size(); @@ -264,7 +295,7 @@ public class ByLine { for (int i = 0; i < size; i++) { int index1 = subLines1.get(i); int index2 = subLines2.get(comb[i]); - if (lines1.get(index1).equals(lines2.get(index2))) weight++; + if (maximisingEquality.equals(lines1.get(index1), lines2.get(index2))) weight++; } if (weight > bestWeight) { @@ -319,21 +350,6 @@ public class ByLine { return Pair.create(bigLines, indexes); } - @NotNull - private static FairDiffIterable expandRanges(@NotNull List lines1, - @NotNull List lines2, - @NotNull FairDiffIterable iterable, - @NotNull ProgressIndicator indicator) { - List changes = new ArrayList<>(); - - for (Range ch : iterable.iterateChanges()) { - Range expanded = TrimUtil.expand(lines1, lines2, ch.start1, ch.start2, ch.end1, ch.end2); - if (!expanded.isEmpty()) changes.add(expanded); - } - - return fair(create(changes, lines1.size(), lines2.size())); - } - // // Lines // diff --git a/platform/diff-impl/tests/com/intellij/diff/comparison/LineComparisonUtilTest.kt b/platform/diff-impl/tests/com/intellij/diff/comparison/LineComparisonUtilTest.kt index 9144618851d3..2eab93eccb42 100644 --- a/platform/diff-impl/tests/com/intellij/diff/comparison/LineComparisonUtilTest.kt +++ b/platform/diff-impl/tests/com/intellij/diff/comparison/LineComparisonUtilTest.kt @@ -401,6 +401,7 @@ class LineComparisonUtilTest : ComparisonUtilTestBase() { // TODO (" _-------_ _ _ " - " _ _ _ ").trim() (" _-------_ _ _ " - " _ _ _ ").default() (" _ _--_-_------" - " _--_-_ ").trim() + (" _-------_ _ _ " - " _ _ _ ").ignore() testAll() } @@ -416,7 +417,6 @@ class LineComparisonUtilTest : ComparisonUtilTestBase() { lines() { ("====}_==== }_Y_====}" - "====}_Y_====}") (" _------_ _ " - " _ _ ").default() // result after second step correction - (" _ _-_-----" - " _-_ ").ignore() // result looks strange because of 'diff.unimportant.line.char.count' testAll() } } @@ -428,4 +428,59 @@ class LineComparisonUtilTest : ComparisonUtilTestBase() { testDefault() } } + + fun `test ignore whitespace policy applies two-step correction`() { + lines() { + ("1_ _ 1" - " 1") + ("-_-_ " - " ").default() + (" _-_---" - " ").trim() + ("-_-_ " - " ").ignore() + testAll() + } + + lines() { + (" 1_ _1" - " 1") + (" _-_-" - " ").default() + testAll() + } + + lines() { + ("X_ Y_X" - "Y ") + ("-_--_-" - "--").default() + ("-_ _-" - " ").trim() + testAll() + } + } + + fun `test regression - second step correction should be performed if there are no ambigous matchings`() { + lines { + ("}_ }" - " }_}") + ("-_--" - "--_-").default() + (" _ " - " _ ").trim() + testAll() + } + + lines { + (" }_}_ }" - "}_}_}") + ("--_ _--" - "-_ _-").default() + (" _ _ " - " _ _ ").trim() + testAll() + } + + lines() { + ("X_X __Y" - "X__Z") + (" _--__-" - " __-").default() + ("-_ __-" - " __-").trim() + testAll() + } + } + + fun `test regression - second step with too many possible matchings`() { + lines { + (" X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_ X" - "X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X_X ") + ("--_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _-_-_-_-_-_--" - "-_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _--").default() + (" _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _-_-_-_-_--" - " _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ").trim() + testAll() + } + } }