Skip to content

Use "natural" diff order by default in compare editor #2566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iloveeclipse merged 3 commits into from
Mar 15, 2026
+85 −25
Merged

Use "natural" diff order by default in compare editor #2566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e59757d
Updated TextMergeViewerTest with more pre/post conditions checks
iloveeclipse Mar 13, 2026
f7c29b1
Fix MirroredMergeViewerContentProvider not mirroring errors
iloveeclipse Mar 13, 2026
73dddd4
Use "natural" diff order by default in compare editor
iloveeclipse Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ndles/org.eclipse.compare/compare/org/eclipse/compare/internal/ComparePreferencePage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public static void initDefaults(IPreferenceStore store) {
store.setDefault(PATH_FILTER, ""); //$NON-NLS-1$
store.setDefault(ICompareUIConstants.PREF_NAVIGATION_END_ACTION, ICompareUIConstants.PREF_VALUE_PROMPT);
store.setDefault(ICompareUIConstants.PREF_NAVIGATION_END_ACTION_LOCAL, ICompareUIConstants.PREF_VALUE_LOOP);
store.setDefault(SWAPPED, false);
store.setDefault(SWAPPED, true);
}

public ComparePreferencePage() {
Expand Down
4 changes: 2 additions & 2 deletions ...ipse.compare/compare/org/eclipse/compare/internal/MirroredMergeViewerContentProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ public void setAncestorError(String errorMessage) {

@Override
public void setLeftError(String errorMessage) {
delegate.setLeftError(errorMessage);
delegate.setRightError(errorMessage);
}

@Override
public void setRightError(String errorMessage) {
delegate.setRightError(errorMessage);
delegate.setLeftError(errorMessage);
}
}
104 changes: 82 additions & 22 deletions team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import org.eclipse.compare.contentmergeviewer.TextMergeViewer;
import org.eclipse.compare.contentmergeviewer.TokenComparator;
import org.eclipse.compare.internal.ChangeCompareFilterPropertyAction;
import org.eclipse.compare.internal.ComparePreferencePage;
import org.eclipse.compare.internal.CompareUIPlugin;
import org.eclipse.compare.internal.IMergeViewerTestAdapter;
import org.eclipse.compare.internal.MergeViewerContentProvider;
import org.eclipse.compare.internal.Utilities;
Expand All @@ -56,6 +58,7 @@
import org.eclipse.jface.action.IContributionItem;
import org.eclipse.jface.action.ToolBarManager;
import org.eclipse.jface.dialogs.Dialog;
import org.eclipse.jface.preference.IPreferenceStore;
import org.eclipse.jface.text.Document;
import org.eclipse.jface.text.DocumentEvent;
import org.eclipse.jface.text.IDocument;
Expand All @@ -75,6 +78,15 @@

public class TextMergeViewerTest {

static boolean rightIsChanged() {
IPreferenceStore store = CompareUIPlugin.getDefault().getPreferenceStore();
return store.getBoolean(ComparePreferencePage.SWAPPED);
}

static boolean leftIsChanged() {
return !rightIsChanged();
}

/**
* Returns a boolean value indicating whether or not the contents
* of the given streams are considered to be equal. Closes both input streams.
Expand Down Expand Up @@ -179,7 +191,7 @@ public String getType() {
}
@Override
public InputStream getContents() {
return new ByteArrayInputStream(contents);
return contents == null ? null : new ByteArrayInputStream(contents);
}
@Override
protected Object clone() {
Expand Down Expand Up @@ -207,7 +219,7 @@ public boolean equals(Object obj) {
return false;
}
public Object getContentsAsString() {
return new String(contents);
return contents == null ? null : new String(contents);
}
}

Expand Down Expand Up @@ -286,31 +298,41 @@ public void testCopyRightToLeft() throws Exception {
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
String copiedText = "hi there";
DiffNode testNode = new DiffNode(parentNode, Differencer.CHANGE, null, new EditableTestElement("some text".getBytes()), new EditableTestElement(copiedText.getBytes()));
assertEquals("some text", ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertEquals(copiedText, ((EditableTestElement) testNode.getRight()).getContentsAsString());

runInDialog(testNode, () -> {
viewer.copy(false /* rightToLeft */);
viewer.copy(rightIsChanged() /* rightToLeft */);
saveViewerContents();
});
assertEquals(copiedText, ((EditableTestElement)testNode.getLeft()).getContentsAsString());
assertEquals(copiedText, ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertEquals(copiedText, ((EditableTestElement) testNode.getRight()).getContentsAsString());
}

@Test
public void testCopyLeftToRight() throws Exception {
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
String copiedText = "hi there";
DiffNode testNode = new DiffNode(parentNode, Differencer.CHANGE, null, new EditableTestElement(copiedText.getBytes()), new EditableTestElement("some text".getBytes()));
assertEquals(copiedText, ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertEquals("some text", ((EditableTestElement) testNode.getRight()).getContentsAsString());

runInDialog(testNode, () -> {
viewer.copy(true /* leftToRight */);
viewer.copy(leftIsChanged() /* leftToRight */);
saveViewerContents();
});
assertEquals(copiedText, ((EditableTestElement)testNode.getRight()).getContentsAsString());
assertEquals(copiedText, ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertEquals(copiedText, ((EditableTestElement) testNode.getRight()).getContentsAsString());
}

@Test
public void testCopyRightToEmptyLeft() throws Exception {
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
DiffNode testNode = new DiffNode(parentNode, Differencer.ADDITION, null, null, new EditableTestElement("hi there".getBytes()));
assertNull(testNode.getLeft());
assertEquals("hi there", ((EditableTestElement) testNode.getRight()).getContentsAsString());
runInDialog(testNode, () -> {
viewer.copy(false /* rightToLeft */);
viewer.copy(rightIsChanged() /* rightToLeft */);
saveViewerContents();
});
assertEquals(testNode.getRight(), testNode.getLeft());
Expand All @@ -320,8 +342,10 @@ public void testCopyRightToEmptyLeft() throws Exception {
public void testCopyLeftToEmptyRight() throws Exception {
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
DiffNode testNode = new DiffNode(parentNode, Differencer.DELETION, null, new EditableTestElement("hi there".getBytes()), null);
assertEquals("hi there", ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertNull(testNode.getRight());
runInDialog(testNode, () -> {
viewer.copy(true /* leftToRight */);
viewer.copy(leftIsChanged() /* leftToRight */);
saveViewerContents();
});
assertEquals(testNode.getRight(), testNode.getLeft());
Expand All @@ -331,8 +355,10 @@ public void testCopyLeftToEmptyRight() throws Exception {
public void testCopyEmptyLeftToRight() throws Exception {
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
DiffNode testNode = new DiffNode(parentNode, Differencer.ADDITION, null, null, new EditableTestElement("hi there".getBytes()));
assertNull(testNode.getLeft());
assertEquals("hi there", ((EditableTestElement) testNode.getRight()).getContentsAsString());
runInDialog(testNode, () -> {
viewer.copy(true /* leftToRight */);
viewer.copy(leftIsChanged() /* leftToRight */);
saveViewerContents();
});
assertNull(testNode.getLeft());
Expand All @@ -343,8 +369,10 @@ public void testCopyEmptyLeftToRight() throws Exception {
public void testCopyEmptyRightToLeft() throws Exception {
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
DiffNode testNode = new DiffNode(parentNode, Differencer.DELETION, null, new EditableTestElement("hi there".getBytes()), null);
assertEquals("hi there", ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertNull(testNode.getRight());
runInDialog(testNode, () -> {
viewer.copy(false /* rightToLeft */);
viewer.copy(rightIsChanged() /* rightToLeft */);
saveViewerContents();
});
assertNull(testNode.getLeft());
Expand All @@ -354,53 +382,81 @@ public void testCopyEmptyRightToLeft() throws Exception {
@Test
public void testModifyLeft() throws Exception {
DiffNode testNode = new DiffNode(new EditableTestElement("hi there".getBytes()), null);
assertEquals("hi there", ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertNull(testNode.getRight());

final String newText = "New text";
runInDialog(testNode, () -> {
IDocument doc = getDocument(true /* left */);
IDocument doc = getDocument(leftIsChanged() /* left */);
doc.set(newText);
saveViewerContents();
});
assertEquals(newText, ((EditableTestElement)testNode.getLeft()).getContentsAsString());
assertNull(testNode.getRight());
}

@Test
public void testModifyRight() throws Exception {
DiffNode testNode = new DiffNode(null, new EditableTestElement("hi there".getBytes()));
assertEquals("hi there", ((EditableTestElement) testNode.getRight()).getContentsAsString());
assertNull(testNode.getLeft());
final String newText = "New text";
runInDialog(testNode, () -> {
IDocument doc = getDocument(false /* right */);
IDocument doc = getDocument(rightIsChanged() /* right */);
doc.set(newText);
saveViewerContents();
});
assertEquals(newText, ((EditableTestElement)testNode.getRight()).getContentsAsString());
assertNull(testNode.getLeft());
}

@Test
public void testCopyEmptyRightToLeftAndModify() throws Exception {
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
DiffNode testNode = new DiffNode(parentNode, Differencer.ADDITION, null, null, new EditableTestElement("hi there".getBytes()));
int kind = leftIsChanged() ? Differencer.ADDITION : Differencer.DELETION;
DiffNode testNode = new DiffNode(parentNode, kind, null, new EditableTestElement(null), new EditableTestElement("hi there".getBytes()));
assertNull(((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertEquals("hi there", ((EditableTestElement) testNode.getRight()).getContentsAsString());

final String newText = "New text";
runInDialog(testNode, () -> {
viewer.copy(false /* rightToLeft */);
IDocument doc = getDocument(true /* left */);
viewer.copy(rightIsChanged() /* rightToLeft */);
IDocument doc = getDocument(leftIsChanged() /* left */);
doc.set(newText);
saveViewerContents();
});
assertEquals(newText, ((EditableTestElement)testNode.getLeft()).getContentsAsString());

if (leftIsChanged()) {
assertEquals(newText, ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertEquals("hi there", ((EditableTestElement) testNode.getRight()).getContentsAsString());
} else {
assertEquals(newText, ((EditableTestElement) testNode.getRight()).getContentsAsString());
assertEquals("hi there", ((EditableTestElement) testNode.getLeft()).getContentsAsString());
}
}

@Test
public void testCopyEmptyLeftToRightAndModify() throws Exception {
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
DiffNode testNode = new DiffNode(parentNode, Differencer.DELETION, null, new EditableTestElement("hi there".getBytes()), null);
int kind = leftIsChanged() ? Differencer.DELETION : Differencer.ADDITION;
DiffNode testNode = new DiffNode(parentNode, kind, null, new EditableTestElement("hi there".getBytes()), new EditableTestElement(null));
assertEquals("hi there", ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertNull(((EditableTestElement) testNode.getRight()).getContentsAsString());

final String newText = "New text";
runInDialog(testNode, () -> {
viewer.copy(true /* leftToRight */);
IDocument doc = getDocument(false /* right */);
viewer.copy(leftIsChanged() /* leftToRight */);
IDocument doc = getDocument(rightIsChanged() /* right */);
doc.set(newText);
saveViewerContents();
});
assertEquals(newText, ((EditableTestElement)testNode.getRight()).getContentsAsString());
if (leftIsChanged()) {
assertEquals("hi there", ((EditableTestElement) testNode.getLeft()).getContentsAsString());
assertEquals(newText, ((EditableTestElement) testNode.getRight()).getContentsAsString());
} else {
assertEquals("hi there", ((EditableTestElement) testNode.getRight()).getContentsAsString());
assertEquals(newText, ((EditableTestElement) testNode.getLeft()).getContentsAsString());
}
}

@Test
Expand All @@ -414,8 +470,12 @@ public void testCompareFilter() throws Exception {
leftString.getBytes());
final EditableTestElement rightElement = new EditableTestElement(
rightString.getBytes());
DiffNode testNode = new DiffNode(parentNode, Differencer.CHANGE, null,
leftElement, rightElement);
DiffNode testNode;
if (leftIsChanged()) {
testNode = new DiffNode(parentNode, Differencer.CHANGE, null, leftElement, rightElement);
} else {
testNode = new DiffNode(parentNode, Differencer.CHANGE, null, rightElement, leftElement);
}
final CompareConfiguration cc = new CompareConfiguration();
runInDialog(testNode, () -> {
Object adapter = viewer
Expand Down
Loading