diff --git a/platform/diff-impl/src/com/intellij/diff/tools/fragmented/LineNumberConvertor.java b/platform/diff-impl/src/com/intellij/diff/tools/fragmented/LineNumberConvertor.java index 7d9d943ebb18..edd03b7f139b 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/fragmented/LineNumberConvertor.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/fragmented/LineNumberConvertor.java @@ -25,15 +25,15 @@ import java.util.TreeMap; public class LineNumberConvertor { // Master -> Slave - @NotNull private final TreeMap myFragments; + @NotNull private final TreeMap myFragments; // Slave -> Master - @NotNull private final TreeMap myInvertedFragments; + @NotNull private final TreeMap myInvertedFragments; @NotNull private final Corrector myCorrector = new Corrector(); - private LineNumberConvertor(@NotNull TreeMap fragments, - @NotNull TreeMap invertedFragments) { + private LineNumberConvertor(@NotNull TreeMap fragments, + @NotNull TreeMap invertedFragments) { myFragments = fragments; myInvertedFragments = invertedFragments; } @@ -83,35 +83,40 @@ public class LineNumberConvertor { * true: return 'good enough' position, even if exact matching is impossible */ private int convertRaw(boolean fromMaster, int value, boolean approximate) { - TreeMap fragments = fromMaster ? myFragments : myInvertedFragments; + TreeMap fragments = fromMaster ? myFragments : myInvertedFragments; if (approximate) { - Map.Entry floor = fragments.floorEntry(value); - if (floor == null) return 0; - if (floor.getValue() != -1) return floor.getValue() - floor.getKey() + value; + Map.Entry prevEntry = fragments.floorEntry(value); + if (prevEntry == null) return 0; + int start = prevEntry.getKey(); + Data data = prevEntry.getValue(); - Map.Entry floorHead = fragments.floorEntry(floor.getKey() - 1); - assert floorHead != null && floorHead.getValue() != -1; - - return floorHead.getValue() - floorHead.getKey() + floor.getKey(); + return Math.min(data.otherStart - start + value, data.otherStart + data.otherLength); } else { - Map.Entry floor = fragments.floorEntry(value); - if (floor == null || floor.getValue() == -1) return -1; - return floor.getValue() - floor.getKey() + value; + Map.Entry prevEntry = fragments.floorEntry(value); + if (prevEntry == null) return -1; + int start = prevEntry.getKey(); + Data data = prevEntry.getValue(); + + if (value >= start + data.length) return -1; + if (data.length != data.otherLength) return -1; + + return data.otherStart - start + value; } } public static class Builder { - @NotNull private final TreeMap myFragments = new TreeMap<>(); - @NotNull private final TreeMap myInvertedFragments = new TreeMap<>(); + @NotNull private final TreeMap myFragments = new TreeMap<>(); + @NotNull private final TreeMap myInvertedFragments = new TreeMap<>(); - public void put(int start, int newStart, int length) { - myFragments.put(start, newStart); - myFragments.put(start + length, -1); + public void put(int masterStart, int slaveStart, int length) { + put(masterStart, slaveStart, length, length); + } - myInvertedFragments.put(newStart, start); - myInvertedFragments.put(newStart + length, -1); + public void put(int masterStart, int slaveStart, int masterLength, int slaveLength) { + myFragments.put(masterStart, new Data(masterLength, slaveStart, slaveLength)); + myInvertedFragments.put(slaveStart, new Data(slaveLength, masterStart, masterLength)); } @NotNull @@ -120,6 +125,18 @@ public class LineNumberConvertor { } } + private static class Data { + public final int length; + public final int otherStart; + public final int otherLength; + + public Data(int length, int otherStart, int otherLength) { + this.length = length; + this.otherStart = otherStart; + this.otherLength = otherLength; + } + } + /* * myFragments allow to convert between Sm-So-Su, Mm-Mo-Mu, Em-Eo-Eu. * diff --git a/platform/diff-impl/src/com/intellij/diff/tools/fragmented/UnifiedDiffViewer.java b/platform/diff-impl/src/com/intellij/diff/tools/fragmented/UnifiedDiffViewer.java index bd93ecbedae8..9928ead710ba 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/fragmented/UnifiedDiffViewer.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/fragmented/UnifiedDiffViewer.java @@ -790,7 +790,7 @@ public class UnifiedDiffViewer extends ListenerDiffViewerBase { @NotNull private DiffUtil.DiffConfig getDiffConfig() { - return new DiffUtil.DiffConfig(getIgnorePolicy(), getHighlightPolicy()); + return new DiffUtil.DiffConfig(getTextSettings().getIgnorePolicy(), getHighlightPolicy()); } @NotNull @@ -800,13 +800,6 @@ public class UnifiedDiffViewer extends ListenerDiffViewerBase { return policy; } - @NotNull - private IgnorePolicy getIgnorePolicy() { - IgnorePolicy policy = getTextSettings().getIgnorePolicy(); - if (policy == IgnorePolicy.IGNORE_WHITESPACES_CHUNKS) return IgnorePolicy.IGNORE_WHITESPACES; - return policy; - } - // // Getters // @@ -1032,20 +1025,6 @@ public class UnifiedDiffViewer extends ListenerDiffViewerBase { super(getTextSettings()); } - @NotNull - @Override - protected IgnorePolicy getCurrentSetting() { - return getIgnorePolicy(); - } - - @NotNull - @Override - protected List getAvailableSettings() { - ArrayList settings = ContainerUtil.newArrayList(IgnorePolicy.values()); - settings.remove(IgnorePolicy.IGNORE_WHITESPACES_CHUNKS); - return settings; - } - @Override protected void onSettingsChanged() { rediff(); diff --git a/platform/diff-impl/src/com/intellij/diff/tools/fragmented/UnifiedFragmentBuilder.java b/platform/diff-impl/src/com/intellij/diff/tools/fragmented/UnifiedFragmentBuilder.java index 669de29bf048..1ed6836ea02c 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/fragmented/UnifiedFragmentBuilder.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/fragmented/UnifiedFragmentBuilder.java @@ -25,7 +25,6 @@ import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.List; -// This class works incorrectly with non-fair differences (when chunk of matched lines has different length in left/right files) class UnifiedFragmentBuilder { @NotNull private final List myFragments; @NotNull private final Document myDocument1; @@ -89,21 +88,15 @@ class UnifiedFragmentBuilder { int linesBefore = totalLines; int linesAfter; - if (lines1 >= 0) { - int startOffset = myDocument1.getLineStartOffset(startLine1); - int endOffset = myDocument1.getLineEndOffset(endLine1); - - appendTextSide(Side.LEFT, startOffset, endOffset, lines1, startLine1, -1); - } + int startOffset1 = myDocument1.getLineStartOffset(startLine1); + int endOffset1 = myDocument1.getLineEndOffset(endLine1); + appendTextSide(Side.LEFT, startOffset1, endOffset1, lines1, lines2, startLine1, -1); int linesBetween = totalLines; - if (lines2 >= 0) { - int startOffset = myDocument2.getLineStartOffset(startLine2); - int endOffset = myDocument2.getLineEndOffset(endLine2); - - appendTextSide(Side.RIGHT, startOffset, endOffset, lines2, -1, startLine2); - } + int startOffset2 = myDocument2.getLineStartOffset(startLine2); + int endOffset2 = myDocument2.getLineEndOffset(endLine2); + appendTextSide(Side.RIGHT, startOffset2, endOffset2, lines2, lines2, -1, startLine2); linesAfter = totalLines; @@ -122,43 +115,50 @@ class UnifiedFragmentBuilder { } private void appendTextMaster(int startLine1, int startLine2, int endLine1, int endLine2) { - int lines = myMasterSide.isLeft() ? endLine1 - startLine1 : endLine2 - startLine2; + // The slave-side line matching might be incomplete for non-fair line fragments (@see FairDiffIterable) + // If it ever became an issue, it could be fixed by explicit fair by-line comparing of "equal" regions - if (lines >= 0) { - int startOffset = myMasterSide.isLeft() ? myDocument1.getLineStartOffset(startLine1) : myDocument2.getLineStartOffset(startLine2); - int endOffset = myMasterSide.isLeft() ? myDocument1.getLineEndOffset(endLine1) : myDocument2.getLineEndOffset(endLine2); + int lines1 = endLine1 - startLine1; + int lines2 = endLine2 - startLine2; + int startOffset = myMasterSide.isLeft() ? myDocument1.getLineStartOffset(startLine1) : myDocument2.getLineStartOffset(startLine2); + int endOffset = myMasterSide.isLeft() ? myDocument1.getLineEndOffset(endLine1) : myDocument2.getLineEndOffset(endLine2); - appendText(myMasterSide, startOffset, endOffset, lines, startLine1, startLine2); - } + appendText(myMasterSide, startOffset, endOffset, lines1, lines2, startLine1, startLine2); } - private void appendTextSide(@NotNull Side side, int offset1, int offset2, int lines, int startLine1, int startLine2) { + private void appendTextSide(@NotNull Side side, int offset1, int offset2, int lines1, int lines2, int startLine1, int startLine2) { int linesBefore = totalLines; - appendText(side, offset1, offset2, lines, startLine1, startLine2); + appendText(side, offset1, offset2, lines1, lines2, startLine1, startLine2); int linesAfter = totalLines; - myChangedLines.add(new LineRange(linesBefore, linesAfter)); + if (linesBefore != linesAfter) myChangedLines.add(new LineRange(linesBefore, linesAfter)); } - private void appendText(@NotNull Side side, int offset1, int offset2, int lines, int startLine1, int startLine2) { - Document document = side.select(myDocument1, myDocument2); - - int newline = document.getTextLength() > offset2 + 1 ? 1 : 0; - TextRange base = new TextRange(myBuilder.length(), myBuilder.length() + offset2 - offset1 + newline); - TextRange changed = new TextRange(offset1, offset2 + newline); - myRanges.add(new HighlightRange(side, base, changed)); - - myBuilder.append(document.getCharsSequence().subSequence(offset1, offset2)); - myBuilder.append('\n'); + private void appendText(@NotNull Side side, int offset1, int offset2, int lines1, int lines2, int startLine1, int startLine2) { + int lines = side.select(lines1, lines2); + boolean notEmpty = lines >= 0; + int appendix = notEmpty ? 1 : 0; if (startLine1 != -1) { - myConvertor1.put(totalLines, startLine1, lines + 1); + myConvertor1.put(totalLines, startLine1, lines + appendix, lines1 + appendix); } if (startLine2 != -1) { - myConvertor2.put(totalLines, startLine2, lines + 1); + myConvertor2.put(totalLines, startLine2, lines + appendix, lines2 + appendix); } - totalLines += lines + 1; + if (notEmpty) { + Document document = side.select(myDocument1, myDocument2); + + int newline = document.getTextLength() > offset2 + 1 ? 1 : 0; + TextRange base = new TextRange(myBuilder.length(), myBuilder.length() + offset2 - offset1 + newline); + TextRange changed = new TextRange(offset1, offset2 + newline); + myRanges.add(new HighlightRange(side, base, changed)); + + myBuilder.append(document.getCharsSequence().subSequence(offset1, offset2)); + myBuilder.append('\n'); + + totalLines += lines + 1; + } } private static int getLineCount(@NotNull Document document) { diff --git a/platform/diff-impl/tests/com/intellij/diff/tools/fragmented/LineNumberConvertorTest.kt b/platform/diff-impl/tests/com/intellij/diff/tools/fragmented/LineNumberConvertorTest.kt index 37983d33c77a..f8562b3cd75c 100644 --- a/platform/diff-impl/tests/com/intellij/diff/tools/fragmented/LineNumberConvertorTest.kt +++ b/platform/diff-impl/tests/com/intellij/diff/tools/fragmented/LineNumberConvertorTest.kt @@ -112,20 +112,81 @@ class LineNumberConvertorTest : UsefulTestCase() { put(6, 5, 2) }, { - assertEquals(0, convertor.convertApproximate(0)) - assertEquals(2, convertor.convertApproximate(1)) - assertEquals(3, convertor.convertApproximate(2)) - assertEquals(3, convertor.convertApproximate(3)) - assertEquals(3, convertor.convertApproximate(4)) - assertEquals(3, convertor.convertApproximate(5)) - assertEquals(5, convertor.convertApproximate(6)) - assertEquals(6, convertor.convertApproximate(7)) - assertEquals(7, convertor.convertApproximate(8)) - assertEquals(7, convertor.convertApproximate(9)) + checkEmpty(-5, 0) + checkMatch(1, 2, 1) + checkEmpty(3, 5) + checkMatch(6, 5, 2) + checkEmpty(8, 20) + + checkApproximate(0, 0) + checkApproximate(1, 2) + checkApproximate(2, 3) + checkApproximate(3, 3) + checkApproximate(4, 3) + checkApproximate(5, 3) + checkApproximate(6, 5) + checkApproximate(7, 6) + checkApproximate(8, 7) + checkApproximate(9, 7) + checkApproximate(10, 7) + checkApproximate(11, 7) + + checkApproximateInv(0, 0) + checkApproximateInv(0, 1) + checkApproximateInv(1, 2) + checkApproximateInv(2, 3) + checkApproximateInv(2, 4) + checkApproximateInv(6, 5) + checkApproximateInv(7, 6) + checkApproximateInv(8, 7) + checkApproximateInv(8, 8) + checkApproximateInv(8, 9) + checkApproximateInv(8, 10) + checkApproximateInv(8, 11) } ) } + fun testNonFairRange() { + doTest( + { + put(1, 2, 1) + put(6, 5, 2, 4) + }, + { + checkEmpty(-5, 0) + checkMatch(1, 2, 1) + checkEmpty(3, 20) + + checkApproximate(0, 0) + checkApproximate(1, 2) + checkApproximate(2, 3) + checkApproximate(3, 3) + checkApproximate(4, 3) + checkApproximate(5, 3) + checkApproximate(6, 5) + checkApproximate(7, 6) + checkApproximate(8, 7) + checkApproximate(9, 8) + checkApproximate(10, 9) + checkApproximate(11, 9) + checkApproximate(12, 9) + + checkApproximateInv(0, 0) + checkApproximateInv(0, 1) + checkApproximateInv(1, 2) + checkApproximateInv(2, 3) + checkApproximateInv(2, 4) + checkApproximateInv(6, 5) + checkApproximateInv(7, 6) + checkApproximateInv(8, 7) + checkApproximateInv(8, 8) + checkApproximateInv(8, 9) + checkApproximateInv(8, 10) + checkApproximateInv(8, 11) + }) + } + // // Impl // @@ -144,25 +205,37 @@ class LineNumberConvertorTest : UsefulTestCase() { builder.put(left, right, length) } + fun put(left: Int, right: Int, lengthLeft: Int, lengthRight: Int) { + builder.put(left, right, lengthLeft, lengthRight) + } + fun finish(): Test = Test(builder.build()) } private class Test(val convertor: LineNumberConvertor) { - fun checkMatch(left: Int, right: Int, length: Int) { + fun checkMatch(left: Int, right: Int, length: Int = 1) { for (i in 0..length - 1) { assertEquals(right + i, convertor.convert(left + i)) assertEquals(left + i, convertor.convertInv(right + i)) } } - fun checkEmpty(start: Int, end: Int) { - for (i in start..end) { + fun checkApproximate(left: Int, right: Int) { + assertEquals(right, convertor.convertApproximate(left)) + } + + fun checkApproximateInv(left: Int, right: Int) { + assertEquals(left, convertor.convertApproximateInv(right)) + } + + fun checkEmpty(startLeft: Int, endLeft: Int) { + for (i in startLeft..endLeft) { assertEquals(-1, convertor.convert(i)) } } - fun checkEmptyInv(start: Int, end: Int) { - for (i in start..end) { + fun checkEmptyInv(startRight: Int, endRight: Int) { + for (i in startRight..endRight) { assertEquals(-1, convertor.convertInv(i)) } }