Bug 455785 - [CommonNavigator] CommonViewerSorter should use sortOnly
sorters when content is provided by different navigator contents
No longer check for trigger points in CommonViewerSorter when content is
provided by different contents so that sortOnly sorters are taken into
account when sorting.
Change-Id: I86ffd43684ed80f848ec52abf6c13663fd81ef01
Signed-off-by: Christian Mathis <chris.mh3@gmx.at>
diff --git a/bundles/org.eclipse.ui.navigator/src/org/eclipse/ui/navigator/CommonViewerSorter.java b/bundles/org.eclipse.ui.navigator/src/org/eclipse/ui/navigator/CommonViewerSorter.java
index aa01a7b..152514e 100644
--- a/bundles/org.eclipse.ui.navigator/src/org/eclipse/ui/navigator/CommonViewerSorter.java
+++ b/bundles/org.eclipse.ui.navigator/src/org/eclipse/ui/navigator/CommonViewerSorter.java
@@ -47,10 +47,6 @@
*/
public final class CommonViewerSorter extends TreePathViewerSorter {
- private static final int LEFT_UNDERSTANDS = 1;
- private static final int RIGHT_UNDERSTANDS = 2;
- private static final int BOTH_UNDERSTAND = LEFT_UNDERSTANDS | RIGHT_UNDERSTANDS;
-
private NavigatorContentService contentService;
private INavigatorSorterService sorterService;
@@ -110,29 +106,13 @@
if (sourceOfLvalue == sourceOfRvalue) {
sorter = sorterService.findSorter(sourceOfLvalue, parent, e1, e2);
} else {
+ // findSorter returns the sorter specified at the source or if it has a higher priority a sortOnly sorter that is registered for the parent
+ ViewerSorter lSorter = findApplicableSorter(sourceOfLvalue, parent, e1, e2);
+ ViewerSorter rSorter = findApplicableSorter(sourceOfRvalue, parent, e1, e2);
+ sorter = rSorter;
- boolean flags[] = new boolean[4];
- flags[0] = sourceOfLvalue.isTriggerPoint(e1);
- flags[1] = sourceOfLvalue.isTriggerPoint(e2);
- flags[2] = sourceOfRvalue.isTriggerPoint(e1);
- flags[3] = sourceOfRvalue.isTriggerPoint(e2);
-
- int whoknows = 0;
- whoknows = whoknows | (flags[0] & flags[1] ? LEFT_UNDERSTANDS : 0);
- whoknows = whoknows | (flags[2] & flags[3] ? RIGHT_UNDERSTANDS : 0);
-
- switch (whoknows) {
- case BOTH_UNDERSTAND:
- sorter = sourceOfLvalue.getSequenceNumber() < sourceOfRvalue.getSequenceNumber() ? sorterService
- .findSorter(sourceOfLvalue, parent, e1, e2)
- : sorterService.findSorter(sourceOfRvalue, parent, e1, e2);
- break;
- case LEFT_UNDERSTANDS:
- sorter = sorterService.findSorter(sourceOfLvalue, parent, e1, e2);
- break;
- case RIGHT_UNDERSTANDS:
- sorter = sorterService.findSorter(sourceOfRvalue, parent, e1, e2);
- break;
+ if (rSorter == null || (lSorter != null && sourceOfLvalue.getSequenceNumber() < sourceOfRvalue.getSequenceNumber())) {
+ sorter = lSorter;
}
}
@@ -147,6 +127,18 @@
return categoryDelta;
}
+ private ViewerSorter findApplicableSorter(INavigatorContentDescriptor descriptor, Object parent,
+ Object e1,
+ Object e2) {
+ ViewerSorter sorter = sorterService.findSorter(descriptor, parent, e1, e2);
+ if (!descriptor.isSortOnly()) { // for compatibility
+ if (!(descriptor.isTriggerPoint(e1) && descriptor.isTriggerPoint(e2))) {
+ return null;
+ }
+ }
+ return sorter;
+ }
+
@Override
public boolean isSorterProperty(Object element, String property) {
// Have to get the parent path from the content provider
diff --git a/tests/org.eclipse.ui.tests.navigator/plugin.xml b/tests/org.eclipse.ui.tests.navigator/plugin.xml
index b4164cf..8c1fe41 100644
--- a/tests/org.eclipse.ui.tests.navigator/plugin.xml
+++ b/tests/org.eclipse.ui.tests.navigator/plugin.xml
@@ -713,6 +713,67 @@
<override suppressedExtensionId="org.eclipse.ui.navigator.resourceContent"/>
</navigatorContent>
+ <navigatorContent
+ activeByDefault="false"
+ contentProvider="org.eclipse.ui.tests.navigator.extension.TestContentProviderBasicA"
+ id="org.eclipse.ui.tests.navigator.testContentBasic.a"
+ labelProvider="org.eclipse.ui.tests.navigator.extension.TestLabelProviderBlue"
+ name="Test Navigator Extension [Basic Content A]">
+ <triggerPoints>
+ <or>
+ <instanceof
+ value="org.eclipse.core.resources.IResource">
+ </instanceof>
+ <instanceof
+ value="org.eclipse.ui.tests.navigator.extension.TestExtensionTreeData">
+ </instanceof>
+ </or>
+ </triggerPoints>
+ <possibleChildren>
+ <instanceof
+ value="org.eclipse.ui.tests.navigator.extension.TreeContentA">
+ </instanceof>
+ </possibleChildren>
+ </navigatorContent>
+ <navigatorContent
+ activeByDefault="false"
+ contentProvider="org.eclipse.ui.tests.navigator.extension.TestContentProviderBasicB"
+ id="org.eclipse.ui.tests.navigator.testContentBasic.b"
+ labelProvider="org.eclipse.ui.tests.navigator.extension.TestLabelProviderBlue"
+ name="Test Navigator Extension [Basic Content B]">
+ <triggerPoints>
+ <or>
+ <instanceof
+ value="org.eclipse.core.resources.IResource">
+ </instanceof>
+ <instanceof
+ value="org.eclipse.ui.tests.navigator.extension.TestExtensionTreeData">
+ </instanceof>
+ </or></triggerPoints>
+ <possibleChildren>
+ <instanceof
+ value="org.eclipse.ui.tests.navigator.extension.TreeContentB">
+ </instanceof>
+ </possibleChildren>
+ </navigatorContent>
+ <navigatorContent
+ activeByDefault="false"
+ id="org.eclipse.ui.tests.navigator.testContentBasic.sortOnlySorter"
+ name="Test Navigator Extension [Basic Sortonly Sorter]"
+ sortOnly="true">
+ <commonSorter
+ class="org.eclipse.jface.viewers.ViewerSorter">
+ <parentExpression>
+ <or>
+ <instanceof
+ value="org.eclipse.core.resources.IResource">
+ </instanceof>
+ <instanceof
+ value="org.eclipse.ui.tests.navigator.extension.TestExtensionTreeData">
+ </instanceof>
+ </or></parentExpression>
+ </commonSorter>
+ </navigatorContent>
<navigatorContent
id="org.eclipse.ui.tests.navigator.testContentEmpty"
diff --git a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/NavigatorTestBase.java b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/NavigatorTestBase.java
index b8d6f04..42091cf 100644
--- a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/NavigatorTestBase.java
+++ b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/NavigatorTestBase.java
@@ -119,6 +119,9 @@
public static final String TEST_CONTENT_SORTER_RESOURCE_SORTONLY_OVERRIDE = "org.eclipse.ui.tests.navigator.testContentSorterResource.sortOnly.override";
public static final String TEST_CONTENT_SORTER_RESOURCE_OVERRIDE = "org.eclipse.ui.tests.navigator.testContentSorterResource.override";
public static final String TEST_CONTENT_SORTER_RESOURCE_OVERRIDE_SORTER = "org.eclipse.ui.tests.navigator.testContentSorterResource.override.sorter";
+ public static final String TEST_CONTENT_SORTER_BASIC_A = "org.eclipse.ui.tests.navigator.testContentBasic.a";
+ public static final String TEST_CONTENT_SORTER_BASIC_B = "org.eclipse.ui.tests.navigator.testContentBasic.b";
+ public static final String TEST_CONTENT_SORTER_BASIC_SORTONLY_SORTER = "org.eclipse.ui.tests.navigator.testContentBasic.sortOnlySorter";
public static final String TEST_CONTENT_REDLABEL = "org.eclipse.ui.tests.navigator.testContentRedLabel";
public static final String TEST_CONTENT_MISSINGLABEL = "org.eclipse.ui.tests.navigator.testContentMissingLabel";
diff --git a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/SorterTest.java b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/SorterTest.java
index b4c3377..7194181 100644
--- a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/SorterTest.java
+++ b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/SorterTest.java
@@ -382,6 +382,30 @@
}
+ public void testSorterSortOnlyMultiContent() throws Exception {
+
+ waitForModelObjects();
+
+ _contentService.bindExtensions(new String[] { TEST_CONTENT_SORTER_BASIC_SORTONLY_SORTER }, false);
+ _contentService.getActivationService().activateExtensions(
+ new String[] { TEST_CONTENT_SORTER_BASIC_SORTONLY_SORTER }, false);
+ _contentService.bindExtensions(new String[] { TEST_CONTENT_SORTER_BASIC_A }, false);
+ _contentService.getActivationService().activateExtensions(new String[] { TEST_CONTENT_SORTER_BASIC_A }, false);
+ _contentService.bindExtensions(new String[] { TEST_CONTENT_SORTER_BASIC_B }, false);
+ _contentService.getActivationService().activateExtensions(new String[] { TEST_CONTENT_SORTER_BASIC_B }, false);
+
+
+ TreeItem[] items = _viewer.getTree().getItems();
+
+ // the test content enables on the input of the viewer thus it is on
+ // top level
+ assertEquals("child1", items[0].getText());
+ assertEquals("child2", items[1].getText());
+ assertEquals("child3", items[2].getText());
+ assertEquals("child4", items[3].getText());
+
+ }
+
@Test
public void testSorterResource() throws Exception {
diff --git a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/extension/TestContentProviderBasicA.java b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/extension/TestContentProviderBasicA.java
new file mode 100644
index 0000000..9d5d755
--- /dev/null
+++ b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/extension/TestContentProviderBasicA.java
@@ -0,0 +1,98 @@
+package org.eclipse.ui.tests.navigator.extension;
+
+import org.eclipse.jface.viewers.ITreeContentProvider;
+import org.eclipse.jface.viewers.Viewer;
+
+/**
+ * @since 3.3
+ *
+ */
+public class TestContentProviderBasicA implements ITreeContentProvider {
+
+ private Object[] children = new Object[] { new TreeContentA("child3"), new TreeContentA("child1") };
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see org.eclipse.jface.viewers.IContentProvider#dispose()
+ */
+ public void dispose() {
+ // nothing
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.IContentProvider#inputChanged(org.eclipse.jface
+ * .viewers.Viewer, java.lang.Object, java.lang.Object)
+ */
+ public void inputChanged(Viewer viewer, Object oldInput, Object newInput) {
+ // nothing
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.ITreeContentProvider#getElements(java.lang.
+ * Object)
+ */
+ public Object[] getElements(Object inputElement) {
+ return getChildren(inputElement);
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.ITreeContentProvider#getChildren(java.lang.
+ * Object)
+ */
+ public Object[] getChildren(Object parentElement) {
+ return children;
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.ITreeContentProvider#getParent(java.lang.Object
+ * )
+ */
+ public Object getParent(Object element) {
+ return null;
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.ITreeContentProvider#hasChildren(java.lang.
+ * Object)
+ */
+ public boolean hasChildren(Object element) {
+ return true;
+ }
+
+}
+
+class TreeContentA {
+ private String name;
+
+ /**
+ *
+ */
+ public TreeContentA(String name) {
+ this.name = name;
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see java.lang.Object#toString()
+ */
+ public String toString() {
+ return name;
+ }
+}
diff --git a/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/extension/TestContentProviderBasicB.java b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/extension/TestContentProviderBasicB.java
new file mode 100644
index 0000000..240cb1f
--- /dev/null
+++ b/tests/org.eclipse.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/extension/TestContentProviderBasicB.java
@@ -0,0 +1,98 @@
+package org.eclipse.ui.tests.navigator.extension;
+
+import org.eclipse.jface.viewers.ITreeContentProvider;
+import org.eclipse.jface.viewers.Viewer;
+
+/**
+ * @since 3.3
+ *
+ */
+public class TestContentProviderBasicB implements ITreeContentProvider {
+
+ private Object[] children = new Object[] { new TreeContentA("child4"), new TreeContentA("child2") };
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see org.eclipse.jface.viewers.IContentProvider#dispose()
+ */
+ public void dispose() {
+ // nothing
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.IContentProvider#inputChanged(org.eclipse.jface
+ * .viewers.Viewer, java.lang.Object, java.lang.Object)
+ */
+ public void inputChanged(Viewer viewer, Object oldInput, Object newInput) {
+ // nothing
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.ITreeContentProvider#getElements(java.lang.
+ * Object)
+ */
+ public Object[] getElements(Object inputElement) {
+ return getChildren(inputElement);
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.ITreeContentProvider#getChildren(java.lang.
+ * Object)
+ */
+ public Object[] getChildren(Object parentElement) {
+ return children;
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.ITreeContentProvider#getParent(java.lang.Object
+ * )
+ */
+ public Object getParent(Object element) {
+ return null;
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * org.eclipse.jface.viewers.ITreeContentProvider#hasChildren(java.lang.
+ * Object)
+ */
+ public boolean hasChildren(Object element) {
+ return true;
+ }
+
+}
+
+class TreeContentB {
+ private String name;
+
+ /**
+ *
+ */
+ public TreeContentB(String name) {
+ this.name = name;
+ }
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see java.lang.Object#toString()
+ */
+ public String toString() {
+ return name;
+ }
+}
