Community
Participate
Working Groups
ViewerSorter was deprecated in Bug 402464. 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?
I just got bit by this. I saw that ViewerSorter had been deprecated, and I took the time to go find all my instances of it and change them to ViewerComparators. Then I got hit with the ClassCastException mentioned by Patrick. I looked into what's required to implement a fix for this, but it doesn't look pretty. The extension point could be altered to accept ViewerComparator since it's a parent class, but then you run into the fact that the navigator API uses ViewerSorter all over the place: * CommonSorterDescriptor.createSorter() --> ViewerSorter * INavigatorSorterService.findSorterForParent(Object) --> ViewerSorter * INavigatorSorterService.findSorter(INavigatorContentDescriptor, Object, Object, Object) --> ViewerSorter * INavigatorSorterService.findAvailableSorters(INavigatorContentDescriptor) --> Map<String, ViewerSorter> So new API would need to be added, and the old API would have to wrap ViewerComparator's so they could be returned as ViewerSorter's by the deprecated API. Has anyone else had thoughts about how the work for this ticket might be implemented?
I'm putting 4.8 on this, this is really ugly issue, EGit also was hit by it (see bug 513876). We should check what can we do to fix this in 4.8 time frame.
(In reply to Daniel Wille from comment #1) > I just got bit by this. I saw that ViewerSorter had been deprecated, and I > took the time to go find all my instances of it and change them to > ViewerComparators. Then I got hit with the ClassCastException mentioned by > Patrick. > > I looked into what's required to implement a fix for this, but it doesn't > look pretty. Yep. > The extension point could be altered to accept ViewerComparator > since it's a parent class, but then you run into the fact that the navigator > API uses ViewerSorter all over the place: > > * CommonSorterDescriptor.createSorter() --> ViewerSorter > * INavigatorSorterService.findSorterForParent(Object) --> ViewerSorter > * INavigatorSorterService.findSorter(INavigatorContentDescriptor, Object, > Object, Object) --> ViewerSorter > * INavigatorSorterService.findAvailableSorters(INavigatorContentDescriptor) > --> Map<String, ViewerSorter> > > So new API would need to be added, and the old API would have to wrap > ViewerComparator's so they could be returned as ViewerSorter's by the > deprecated API. Yep. > Has anyone else had thoughts about how the work for this ticket might be > implemented? I agree with your conclusions above and I also don't think we can remove the ViewerSorter from common navigator framework yet, because this would be a breaking change. So we can mark all the methods as deprecated in 4.8, at same time provide the new API and in two years from that release we could try to perform a major (breaking) version change. See https://wiki.eclipse.org/Eclipse/API_Central/Deprecation_Policy for details. @Daniel: if you would do the major work, I would offer the review :-).
As a stop-gap measure, couldn't one accept a ViewerComparator in CommonSorterDescriptor.createSorter() and wrap it always? (Unless the created object already _is_ a ViewerSorter, of course.)
New Gerrit change created: https://git.eclipse.org/r/93613
(In reply to Thomas Wolf from comment #4) > As a stop-gap measure, couldn't one accept a ViewerComparator in > CommonSorterDescriptor.createSorter() and wrap it always? (Unless the > created object already _is_ a ViewerSorter, of course.) Good idea. This way clients using only declarative contributions to navigator can move to the new API. (In reply to Eclipse Genie from comment #5) > New Gerrit change created: https://git.eclipse.org/r/93613 I think this can be accepted for M7, but I have to ask for approval because the behavior of the extension point changes (relaxes the type bound for the navigatorContent/commonSorter extension).
I think we can split this bug into two: one for CommonSorterDescriptor (which can be changed in 4.7 once (and if) we get PMC approval) and another one for the rest of the API. I will file another one soon.
Gerrit change https://git.eclipse.org/r/93613 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=842abdce3ffbc3b685a2be7c71dafb5f7fc19766
The follow up bug on the required API changes is bug 514063.
(In reply to Andrey Loskutov from comment #9) > The follow up bug on the required API changes is bug 514063. Cool. Thanks, Andrey! (And thanks Alexander for the quick approval.)
New Gerrit change created: https://git.eclipse.org/r/93969
Gerrit change https://git.eclipse.org/r/93969 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a889b4ffeed1760a539e0e027c9b4e422ee4753a
