| Summary: | [Navigator] SortViewAction should handle customized ResourceSorter | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Gunnar Wagenknecht <gunnar> |
| Component: | UI | Assignee: | Knut Radloff <knut_radloff> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P2 | CC: | n.a.edgar |
| Version: | 2.1 | ||
| Target Milestone: | 2.1 M5 | ||
| Hardware: | PC | ||
| OS: | Windows 2000 | ||
| Whiteboard: | |||
This seems to be more a problem with the SortViewAction than the ResourceNavigator. I agree though that the SortViewAction should honor the navigator sorter and not set a new one. Actually ResourceNavigator.setSorter *expects* a new ResourceSorter. Also the doc for ResourceSorter states that it is not intended to be subclassed so technically I'm off the hook. What you are really asking for is to make ResourceSorter subclassable. Until I get more requests for this I would prefer you copied the sort by name/type code into your own sorter. Don't make ResourceSorter subclassable. This is not a good desicion. The ResourceNavigator provides an easy to extend navigator view. It already provides a set of "default" actions. If you disallow subclassing of important ResourceNavigator parts it reduces the possibility to create a smart resource view for your perspective. The point I'm asking for is to make the SortViewAction not setting a new ResourceSorter instance on every execution. Instead it should be smart enough to get the current ResourceSorter and modify its criteria like other common ResourceNavigator actions. Reopend for investigation of an alternative to not subclassing ResourceSorter. Please clarify "alternative to not subclassing ResourceSorter". Are you asking to make ResourceSorter subclassable or not? I did find a way to set the sort criteria without creating a new sorter. I could release this for M5. However, your point seems to be not to create a new sorter *and* to make the ResourceSorter subclassable. If you find a way to set the sort criteria without deleting the old sorter of the navigator, this would be fine. In general a desicion should take place if the ResourceSorter should made subclassable or not. If the desicion is to make the ResourceSorter not subclassable as a consequence the ResourceNavigator must be made not subclassable, too. An alternative would be, to define an interface "IResourceSorter" to give subclasses of ResourceNavigator a chance to use their own implementation of a ResourceSorter. Should we open another feature request for it or not? Even if the ResourceSorter is not subclassable it doesn't make the ResourceNavigator any less subclassable. It just means you can't customize the sorting aspect of it. I tend to agree though that it doesn't make sense for the ResourceSorter to be the only part that is not customizable. Entered bug 30521 for this. Thanks. Fixed in > 20030129 SortViewAction.run has: getNavigator().setSorter(sorter); viewer.refresh(); This will cause a double refresh since setSorter already refreshes. SortViewAction seems to copy some of the code in setSorter but not all of it (e.g. updating the preference). It should probably just call setSorter. The problem is that StructuredViewer.setSorter only does a refresh when a different sorter is set. Since I just modify the sorter I have to do a refresh in SortViewAction. The net result is that only one refresh happens. Ideally ResourceNavigator would do everything but set the sorter (like setFiltersPreference). In that case I could do the right thing in SortViewAction without relying on the behavior of StructuredViewer.setSorter and without ugly code. IResourceNavigator does not sufficiently spec what setSorter should do. I'm open for any suggestions on how to improve this. Please see my last comment Hmm, I see. Not pretty. At this stage, I'd just work around it in ResourceNavigator.setSorter, and force a refresh if the sorter is identical to the current one. The SortAction would then just call setSorter. Fixed in >20030203. |
When subclassing the ResourceNavigator most actions are intelligent to handle this case and take care of the new IResourceNavigator. The SortViewAction isn't that smart because it uses the ResourceSorter and doesn't care about the navigators sorter. IMHO the implementation is also not very resource sparing. public void run() { if (sort) getNavigator().setSorter(new ResourceSorter (ResourceSorter.TYPE)); else getNavigator().setSorter(new ResourceSorter (ResourceSorter.NAME)); } A better solution might be: public void run() { if (sort) getNavigator().getSorter().setCriteria(ResourceSorter.TYPE); else getNavigator().getSorter().setCriteria(ResourceSorter.NAME); } But the ResourceSorter has to be expanded to allow #setCriteria(int).