Skip to content

Commit

Permalink
keep significant white spaces in multiline string literals (eclipse-p…
Browse files Browse the repository at this point in the history 
…latform#1068)

Co-authored-by: Tobias Melcher <tobias.melcher@sap.com>
  • Loading branch information
@tobiasmelcher @tobias-melcher
tobiasmelcher and tobias-melcher committed Jan 4, 2024
1 parent 9e3779f commit ef98ac4
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 28 deletions.
57 changes: 29 additions & 28 deletions ...undles/org.eclipse.compare/compare/org/eclipse/compare/internal/merge/DocumentMerger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -241,9 +242,7 @@ void setResolved(boolean r) {

public boolean isResolved() {
if (!fResolved && fDiffs != null) {
Iterator<Diff> e= fDiffs.iterator();
while (e.hasNext()) {
Diff d= e.next();
for (Diff d : fDiffs) {
if (!d.isResolved())
return false;
}
Expand Down Expand Up @@ -447,6 +446,12 @@ public void doDiff() throws CoreException {
CompareContentViewerSwitchingPane.OPTIMIZED_ALGORITHM_USED,
Boolean.FALSE);

Optional<IIgnoreWhitespaceContributor> lDocIgnonerWhitespaceContributor = fInput
.createIgnoreWhitespaceContributor(lDoc);

Optional<IIgnoreWhitespaceContributor> rDocIgnonerWhitespaceContributor = fInput
.createIgnoreWhitespaceContributor(rDoc);

ArrayList<Diff> newAllDiffs = new ArrayList<>();
for (RangeDifference es : e) {
int ancestorStart= 0;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -999,9 +1010,7 @@ public int realToVirtualPosition(char contributor, int vpos) {
int virtualPos= 0; // virtual position
Point region= new Point(0, 0);

Iterator<Diff> 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;
Expand Down Expand Up @@ -1034,9 +1043,7 @@ public int virtualToRealPosition(char contributor, int v) {
int viewPos= 0;
Point region= new Point(0, 0);

Iterator<Diff> 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();
Expand All @@ -1061,9 +1068,7 @@ public int virtualToRealPosition(char contributor, int v) {
public int getVirtualHeight() {
int h= 1;
if (fAllDiffs != null) {
Iterator<Diff> e= fAllDiffs.iterator();
while (e.hasNext()) {
Diff diff= e.next();
for (Diff diff : fAllDiffs) {
h+= diff.getMaxDiffHeight();
}
}
Expand All @@ -1076,9 +1081,7 @@ public int getVirtualHeight() {
public int getRightHeight() {
int h= 1;
if (fAllDiffs != null) {
Iterator<Diff> e= fAllDiffs.iterator();
while (e.hasNext()) {
Diff diff= e.next();
for (Diff diff : fAllDiffs) {
h+= diff.getRightHeight();
}
}
Expand Down Expand Up @@ -1124,9 +1127,7 @@ public Diff findDiff(int viewportHeight, boolean synchronizedScrolling, Point si
int yy, hh;
int y= 0;
if (fAllDiffs != null) {
Iterator<Diff> 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) {
Expand Down
163 changes: 163 additions & 0 deletions team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
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;
import java.io.InputStream;
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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<IIgnoreWhitespaceContributor> 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;
}
});
}
}

0 comments on commit ef98ac4

Please sign in to comment.