Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 28998

Summary: [Navigator] SortViewAction should handle customized ResourceSorter
Product: [Eclipse Project] Platform Reporter: Gunnar Wagenknecht <gunnar>
Component: UIAssignee: 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:

Description Gunnar Wagenknecht CLA 2003-01-04 05:05:44 EST
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).
Comment 1 Knut Radloff CLA 2003-01-13 14:18:46 EST
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.
Comment 2 Knut Radloff CLA 2003-01-13 14:44:20 EST
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.
Comment 3 Knut Radloff CLA 2003-01-27 13:29:25 EST
Don't make ResourceSorter subclassable.
Comment 4 Gunnar Wagenknecht CLA 2003-01-27 13:41:53 EST
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.
Comment 5 Gunnar Wagenknecht CLA 2003-01-27 13:43:45 EST
Reopend for investigation of an alternative to not subclassing ResourceSorter.
Comment 6 Knut Radloff CLA 2003-01-28 11:06:27 EST
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.
Comment 7 Gunnar Wagenknecht CLA 2003-01-29 11:38:39 EST
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?
Comment 8 Knut Radloff CLA 2003-01-29 12:23:31 EST
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.
Comment 9 Gunnar Wagenknecht CLA 2003-01-29 12:27:04 EST
Thanks.
Comment 10 Knut Radloff CLA 2003-01-29 12:51:40 EST
Fixed in > 20030129
Comment 11 Nick Edgar CLA 2003-01-31 12:17:41 EST
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.

Comment 12 Knut Radloff CLA 2003-01-31 13:34:51 EST
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.
Comment 13 Knut Radloff CLA 2003-01-31 13:35:34 EST
Please see my last comment
Comment 14 Nick Edgar CLA 2003-01-31 15:39:01 EST
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.
Comment 15 Knut Radloff CLA 2003-02-03 15:06:24 EST
Fixed in >20030203.