diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/merge/DocumentMerger.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/merge/DocumentMerger.java index 5ac63e01724..0c86d9bd60c 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/merge/DocumentMerger.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/merge/DocumentMerger.java @@ -143,8 +143,9 @@ public Position getPosition(char type) { return fLeftPos; case MergeViewerContentProvider.RIGHT_CONTRIBUTOR: return fRightPos; + default: + return null; } - return null; } boolean isInRange(char type, int pos) { @@ -241,9 +242,7 @@ void setResolved(boolean r) { public boolean isResolved() { if (!fResolved && fDiffs != null) { - Iterator e= fDiffs.iterator(); - while (e.hasNext()) { - Diff d= e.next(); + for (Diff d : fDiffs) { if (!d.isResolved()) return false; } @@ -447,6 +446,12 @@ public void doDiff() throws CoreException { CompareContentViewerSwitchingPane.OPTIMIZED_ALGORITHM_USED, Boolean.FALSE); + Optional lDocIgnonerWhitespaceContributor = fInput + .createIgnoreWhitespaceContributor(lDoc); + + Optional rDocIgnonerWhitespaceContributor = fInput + .createIgnoreWhitespaceContributor(rDoc); + ArrayList newAllDiffs = new ArrayList<>(); for (RangeDifference es : e) { int ancestorStart= 0; @@ -497,6 +502,18 @@ public void doDiff() throws CoreException { && s.trim().length() == 0 && d.trim().length() == 0) { diff.fIsWhitespace= true; + + // Check if whitespace can be ignored by the contributor + if (s.length() > 0 && !lDocIgnonerWhitespaceContributor.isEmpty()) { + boolean isIgnored = lDocIgnonerWhitespaceContributor.get() + .isIgnoredWhitespace(es.leftStart(), es.leftLength()); + diff.fIsWhitespace = isIgnored; + } + if (diff.fIsWhitespace && d.length() > 0 && !rDocIgnonerWhitespaceContributor.isEmpty()) { + boolean isIgnored = rDocIgnonerWhitespaceContributor.get() + .isIgnoredWhitespace(es.rightStart(), es.rightLength()); + diff.fIsWhitespace = isIgnored; + } } // If the diff is of interest, record it and generate the token diffs @@ -677,9 +694,7 @@ public boolean useChange(Diff diff) { } private boolean useChange(int kind) { - if (kind == RangeDifference.NOCHANGE) - return false; - if (fInput.getCompareConfiguration().isChangeIgnored(kind)) + if ((kind == RangeDifference.NOCHANGE) || fInput.getCompareConfiguration().isChangeIgnored(kind)) return false; if (kind == RangeDifference.ANCESTOR) return fInput.isShowPseudoConflicts(); @@ -782,9 +797,7 @@ private void mergingTokenDiff(Diff baseDiff, for (; i < r.length; i++) { es= r[i]; try { - if (leftLine != leftDoc.getLineOfOffset(leftStart+sy.getTokenStart(es.leftStart()))) - break; - if (rightLine != rightDoc.getLineOfOffset(rightStart+sm.getTokenStart(es.rightStart()))) + if ((leftLine != leftDoc.getLineOfOffset(leftStart+sy.getTokenStart(es.leftStart()))) || (rightLine != rightDoc.getLineOfOffset(rightStart+sm.getTokenStart(es.rightStart())))) break; } catch (BadLocationException e) { // silently ignored @@ -972,10 +985,8 @@ public Diff findDiff(Position p, boolean left) { diffPos = diff.fRightPos; } // If the element falls within a diff, highlight that diff - if (diffPos.offset + diffPos.length >= p.offset && diff.fDirection != RangeDifference.NOCHANGE) - return diff; // Otherwise, highlight the first diff after the elements position - if (diffPos.offset >= p.offset) + if ((diffPos.offset + diffPos.length >= p.offset && diff.fDirection != RangeDifference.NOCHANGE) || (diffPos.offset >= p.offset)) return diff; } return null; @@ -999,9 +1010,7 @@ public int realToVirtualPosition(char contributor, int vpos) { int virtualPos= 0; // virtual position Point region= new Point(0, 0); - Iterator e= fAllDiffs.iterator(); - while (e.hasNext()) { - Diff diff= e.next(); + for (Diff diff : fAllDiffs) { Position pos= diff.getPosition(contributor); getLineRange(getDocument(contributor),pos, region); int realHeight= region.y; @@ -1034,9 +1043,7 @@ public int virtualToRealPosition(char contributor, int v) { int viewPos= 0; Point region= new Point(0, 0); - Iterator e= fAllDiffs.iterator(); - while (e.hasNext()) { - Diff diff= e.next(); + for (Diff diff : fAllDiffs) { Position pos= diff.getPosition(contributor); int viewHeight= getLineRange(getDocument(contributor), pos, region).y; int virtualHeight= diff.getMaxDiffHeight(); @@ -1061,9 +1068,7 @@ public int virtualToRealPosition(char contributor, int v) { public int getVirtualHeight() { int h= 1; if (fAllDiffs != null) { - Iterator e= fAllDiffs.iterator(); - while (e.hasNext()) { - Diff diff= e.next(); + for (Diff diff : fAllDiffs) { h+= diff.getMaxDiffHeight(); } } @@ -1076,9 +1081,7 @@ public int getVirtualHeight() { public int getRightHeight() { int h= 1; if (fAllDiffs != null) { - Iterator e= fAllDiffs.iterator(); - while (e.hasNext()) { - Diff diff= e.next(); + for (Diff diff : fAllDiffs) { h+= diff.getRightHeight(); } } @@ -1124,9 +1127,7 @@ public Diff findDiff(int viewportHeight, boolean synchronizedScrolling, Point si int yy, hh; int y= 0; if (fAllDiffs != null) { - Iterator e= fAllDiffs.iterator(); - while (e.hasNext()) { - Diff diff= e.next(); + for (Diff diff : fAllDiffs) { int h= synchronizedScrolling ? diff.getMaxDiffHeight() : diff.getRightHeight(); if (useChange(diff.getKind()) && !diff.fIsWhitespace) { diff --git a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java index 42c1b031eb9..81c9b1c4c2d 100644 --- a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java +++ b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -25,6 +26,7 @@ import java.net.URL; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.eclipse.compare.CompareConfiguration; import org.eclipse.compare.CompareViewerPane; @@ -33,11 +35,18 @@ import org.eclipse.compare.IStreamContentAccessor; import org.eclipse.compare.ITypedElement; import org.eclipse.compare.LabelContributionItem; +import org.eclipse.compare.contentmergeviewer.IIgnoreWhitespaceContributor; +import org.eclipse.compare.contentmergeviewer.ITokenComparator; import org.eclipse.compare.contentmergeviewer.TextMergeViewer; +import org.eclipse.compare.contentmergeviewer.TokenComparator; import org.eclipse.compare.internal.ChangeCompareFilterPropertyAction; import org.eclipse.compare.internal.IMergeViewerTestAdapter; import org.eclipse.compare.internal.MergeViewerContentProvider; import org.eclipse.compare.internal.Utilities; +import org.eclipse.compare.internal.merge.DocumentMerger; +import org.eclipse.compare.internal.merge.DocumentMerger.Diff; +import org.eclipse.compare.internal.merge.DocumentMerger.IDocumentMergerInput; +import org.eclipse.compare.rangedifferencer.RangeDifference; import org.eclipse.compare.structuremergeviewer.DiffNode; import org.eclipse.compare.structuremergeviewer.Differencer; import org.eclipse.core.runtime.CoreException; @@ -52,6 +61,7 @@ import org.eclipse.jface.text.IDocumentPartitioner; import org.eclipse.jface.text.IRegion; import org.eclipse.jface.text.ITypedRegion; +import org.eclipse.jface.text.Position; import org.eclipse.jface.text.Region; import org.eclipse.swt.SWT; import org.eclipse.swt.graphics.Image; @@ -476,6 +486,64 @@ public boolean canCacheFilteredRegions() { }, cc); } + + @Test + public void testCompareWithIgnoreWhitespaceContributor() throws Exception { + String leftTxt = "str\n= \"Hello\nWorld\""; + String rightTxt = "str\n\n= \"Hello\n\nWorld\""; // added newLine in offset 4 and 14 + + DiffNode testNode = new DiffNode(new EditableTestElement(leftTxt.getBytes()), + new EditableTestElement(rightTxt.getBytes())); + + CompareConfiguration cc = new CompareConfiguration(); + runInDialogWithIgnoreWhitespaceContributor(testNode, () -> { + try { + testDocumentMerger.doDiff(); + } catch (CoreException e) { + fail("Cannot do diff in Document Merger"); + } + + Diff firstDiff = testDocumentMerger.findDiff(new Position(4), false); // first different, not in literal + Diff secondDiff = testDocumentMerger.findDiff(new Position(14), false); // second different, in literal + + assertNotNull(firstDiff); + assertNotNull(secondDiff); + + assertEquals("Change direction is wrong", RangeDifference.RIGHT, firstDiff.getKind()); + assertEquals("Change direction is wrong", RangeDifference.RIGHT, secondDiff.getKind()); + + assertTrue("Change should be shown", testDocumentMerger.useChange(firstDiff)); // shows this diff in + // DocumentMerger + assertTrue("Change should be shown", testDocumentMerger.useChange(secondDiff)); // shows this diff in + // DocumentMerger + + cc.setProperty(CompareConfiguration.IGNORE_WHITESPACE, true);// IGNORE_WHITESPACE set to active + try { + testDocumentMerger.doDiff(); + } catch (CoreException e) { + fail("Cannot do diff in Document Merger"); + } + + firstDiff = testDocumentMerger.findDiff(new Position(4), false); + secondDiff = testDocumentMerger.findDiff(new Position(14), false); + + assertNotNull(firstDiff); + assertNotNull(secondDiff); + + assertEquals("Change direction is wrong", RangeDifference.RIGHT, firstDiff.getKind()); + assertEquals("Change direction is wrong", RangeDifference.RIGHT, secondDiff.getKind()); + + org.junit.Assert.assertFalse("Change should not be shown", testDocumentMerger.useChange(firstDiff)); // whitespace + // not + // in + // literal, do not show + // in DocumentMerger + assertTrue("Change should be shown", testDocumentMerger.useChange(secondDiff)); // whitespace in literal, + // show in DocumentMerger + + }, cc); + } + @Test public void testDocumentAsTypedElement() throws Exception { class DocumentAsTypedElement extends Document implements ITypedElement { @@ -643,4 +711,99 @@ protected IDocumentPartitioner getDocumentPartitioner() { return new DummyPartitioner(); } } + + private static DocumentMerger testDocumentMerger = null; + + private void runInDialogWithIgnoreWhitespaceContributor(Object input, Runnable runnable, + final CompareConfiguration cc) throws Exception { + Shell shell = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(); + Dialog dialog = new Dialog(shell) { + @Override + protected Control createDialogArea(Composite parent) { + Composite composite = (Composite) super.createDialogArea(parent); + viewer = new TestMergeViewer(composite, cc); + testDocumentMerger = createDocumentMerger(viewer, cc); + return composite; + } + }; + dialog.setBlockOnOpen(false); + dialog.open(); + viewer.setInput(input); + try { + runnable.run(); + } catch (WrappedException e) { + e.throwException(); + } + dialog.close(); + viewer = null; + } + + private static DocumentMerger createDocumentMerger(TestMergeViewer testMergeViewer, CompareConfiguration cc) { + return new DocumentMerger(new IDocumentMergerInput() { + + @Override + public Optional createIgnoreWhitespaceContributor(IDocument document) { + return Optional.of(new SimpleIgnoreWhitespaceContributor(document)); + } + + @Override + public IDocument getDocument(char contributor) { + IDocument document = Utilities.getDocument(contributor, testMergeViewer.getInput(), true, true); + if (document == null) { + return testMergeViewer.getAdapter(IMergeViewerTestAdapter.class).getDocument(contributor); + } + return document; + } + + @Override + public CompareConfiguration getCompareConfiguration() { + return cc; + } + + @Override + public Position getRegion(char contributor) { + return null; + } + + @Override + public boolean isIgnoreAncestor() { + return false; + } + + @Override + public boolean isThreeWay() { + return false; + } + + @Override + public ITokenComparator createTokenComparator(String line) { + return new TokenComparator(line); + } + + @Override + public boolean isHunkOnLeft() { + return false; + } + + @Override + public int getHunkStart() { + return 0; + } + + @Override + public boolean isPatchHunk() { + return false; + } + + @Override + public boolean isShowPseudoConflicts() { + return false; + } + + @Override + public boolean isPatchHunkOk() { + return false; + } + }); + } }