Community
Participate
Working Groups
According to the Javadoc ViewerSorter should not be used, and ViewerComparator should be used instead. Once we move JFace to Java 1.5, I suggest to add the @Deprecate flag to the class. This makes it easier to people to see that they should avoid using this class.
Gerrit review request https://git.eclipse.org/r/14791
This patch leads to 3 errors in the Problems view: Description Resource Path Location Type Javadoc: The type ViewerSorter is deprecated TreePathViewerSorter.java /org.eclipse.jface/src/org/eclipse/jface/viewers line 72 Java Problem Javadoc: The type ViewerSorter is deprecated TreePathViewerSorter.java /org.eclipse.jface/src/org/eclipse/jface/viewers line 51 Java Problem Javadoc: The type ViewerSorter is deprecated TreePathViewerSorter.java /org.eclipse.jface/src/org/eclipse/jface/viewers line 32 Java Problem
Will you accept the patch to deprecate ViewerSorter if the Problems View gets updated to use ViewerComparator?
I mean this patch caused 3 compile errors in jface in org/eclipse/jface/viewers/TreePathViewerSorter It's API so we can delete it, we'll need to deal with those errors. PW
(In reply to comment #4) > I mean this patch caused 3 compile errors in jface in > org/eclipse/jface/viewers/TreePathViewerSorter It's API so we can delete > it, we'll need to deal with those errors. > > PW We should not change code just because we deprecate a certain member. The problem is only in Javadoc and only because that setting is set to error and includes deprecation, which for code is set to 'Warning'. I suggest to either exclude deprecation checking in Javadoc or report malformed Javadoc as warning instead of error.
New change pushed, following Danis suggestion (exclude deprecation checking in Javadoc). One question, if the Javadoc says ViewerSort should not be used, should derived class as TreePathViewerSorter not also deprecated?
(In reply to comment #4) > I mean this patch caused 3 compile errors in jface in > org/eclipse/jface/viewers/TreePathViewerSorter It's API so we can delete > it, we'll need to deal with those errors. This should read "it's API so we can't delete it" PW
> This should read "it's API so we can't delete it" New patch set updated with the suggestion of Dani.
(In reply to comment #8) > > New patch set updated with the suggestion of Dani. I can't rebase patch set 4, it says it's in conflict. It also doesn't include any changes to prefs, won't I still get my compile errors? PW
Gerrit change https://git.eclipse.org/r/14791 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=af75c21231400b8ba248273f201baecc61a34a49
.
> ** @deprecated see <code>ViewerSorter</code> for more information. >... > public class TreePathViewerSorter extends ViewerSorter { That doesn't make sense and violates our rules about API deprecation. ViewerSorter doesn't tell what clients of TreePathViewerSorter should do. If you want to deprecate TreePathViewerSorter, you have to implement a new class TreePathViewerComparator and port at least one client as a proof that it really works.
New Gerrit change created: https://git.eclipse.org/r/62144
Gerrit change https://git.eclipse.org/r/62144 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=787676ebf424281a586499d427a8a15577b9cf4e
(In reply to Markus Keller from comment #12) > > ** @deprecated see <code>ViewerSorter</code> for more information. > >... > > public class TreePathViewerSorter extends ViewerSorter { > > That doesn't make sense and violates our rules about API deprecation. > > ViewerSorter doesn't tell what clients of TreePathViewerSorter should do. If > you want to deprecate TreePathViewerSorter, you have to implement a new > class TreePathViewerComparator and port at least one client as a proof that > it really works. Thanks Markus, I reverted the change for TreePathViewerSorter and opened Bug 483839 for its replacement.
In our project we replaced all use of ViewerSorter with ViewerComparator. But we have one class which is now an instance of ViewerComparator that is used in the org.eclipse.ui.navigator.navigatorContent extension point in the navigatorContent/commonSorter element. That class is instantiated by the platform and it triggers a ClassCastException: java.lang.ClassCastException: org.eclipse.myproject.ui.MyViewerComparator cannot be cast to org.eclipse.jface.viewers.ViewerSorter at org.eclipse.ui.internal.navigator.sorters.CommonSorterDescriptor$1.run(CommonSorterDescriptor.java:97) Should that extension point be deprecated? Or should it accept both ViewerComparator and ViewerSorter? Can it be fixed in both Neon and Mars.2?
(In reply to Patrick Tasse from comment #16) > In our project we replaced all use of ViewerSorter with ViewerComparator. > > But we have one class which is now an instance of ViewerComparator that is > used in the org.eclipse.ui.navigator.navigatorContent extension point in the > navigatorContent/commonSorter element. > > That class is instantiated by the platform and it triggers a > ClassCastException: > > java.lang.ClassCastException: org.eclipse.myproject.ui.MyViewerComparator > cannot be cast to org.eclipse.jface.viewers.ViewerSorter > at > org.eclipse.ui.internal.navigator.sorters.CommonSorterDescriptor$1. > run(CommonSorterDescriptor.java:97) > > Should that extension point be deprecated? Or should it accept both > ViewerComparator and ViewerSorter? Can it be fixed in both Neon and Mars.2? Can you open a new bug for this?
Created Bug 484248 and linked it with Bug 482137.
