diff: fix two-step correction

* use correction with IGNORE_WHITESPACES policy
* fix cases of non-abmigous matching and when we refuse to check all possible variants
This commit is contained in:
Aleksey Pivovarov
2016-06-23 18:23:03 +03:00
parent ad396e7657
commit 35d02bf4ff
2 changed files with 98 additions and 27 deletions
@@ -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<Line> iwLines1 = convertMode(lines1, IGNORE_WHITESPACES);
@@ -109,6 +111,23 @@ public class ByLine {
private static FairDiffIterable correctChangesSecondStep(@NotNull final List<Line> lines1,
@NotNull final List<Line> lines2,
@NotNull final FairDiffIterable changes) {
return doCorrectChangesSecondStep(lines1, lines2, changes,
Equality.CANONICAL);
}
@NotNull
private static FairDiffIterable correctChangesSecondStepIW(@NotNull final List<Line> lines1,
@NotNull final List<Line> 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<Line> lines1,
@NotNull final List<Line> lines2,
@NotNull final FairDiffIterable changes,
@NotNull final Equality<Line> 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<Line> lines1,
@NotNull final List<Line> lines2) {
@NotNull final List<Line> lines2,
@NotNull final Equality<Line> 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<Line> lines1,
@NotNull List<Line> lines2,
@NotNull FairDiffIterable iterable,
@NotNull ProgressIndicator indicator) {
List<Range> 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
//
@@ -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()
}
}
}