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

Bug 98108

Summary: [Viewers] JFace StructuredViewer.preservingSelection ends up using Object.equals instead of comparer for comparing elements
Product: [Eclipse Project] Platform Reporter: Ken Larson <bugzilla>
Component: UIAssignee: Boris Bokowski <bokowski>
Status: VERIFIED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: eclipse, jason_brennan, tom.schindl
Version: 3.1Keywords: helpwanted
Target Milestone: 3.4 M6   
Hardware: PC   
OS: All   
Whiteboard:

Description Ken Larson CLA 2005-06-02 10:01:06 EDT 
StructuredViewer has a method setComparer(IElementComparer) to provide an
alternate means of comparing elements.  In my case, equals is a very deep (slow)
comparison, so I use this feature to make my lists faster.  In particular, I am
using TableViewer.

However, when doing a StructuredViewer.refresh, when there is a selection in the
table, the function preservingSelection does:

if (!newSelection.equals(oldSelection))

And if you look at StructuredSelection.equals, it in turn compares the elements
using Object.equals.

I think it would be nice if this comparison could be done using the
IElementComparer of the StructuredViewer.  I have subclassed TableViewer to kind
of hack this in, but it would be nice if this functionality were already there.
Comment 1 Tod Creasey CLA 2005-06-02 10:12:46 EDT 
The problem with any change to this is Hashtable lookup - we cache element using
a Hashtable which uses hashCode (which is spec'ed to be consistent with equals).
Comment 2 Ken Larson CLA 2005-06-02 10:34:54 EDT 
The IElementComparer provides an alternate hash code mechanism.

The StructuredViewer is already set up to completely avoid using the elements'
equals and hashcode methods, using:

	public void setComparer(IElementComparer comparer) {
		this.comparer = comparer;
		if (elementMap != null) {
			elementMap = new CustomHashtable(elementMap, comparer);
		}
	}

The CustomHashtable (I believe) avoids using the Object.equals.

So it seems to me that the selection could and should use the same way of
storing and looking up elements as the list itself.
Comment 3 Tod Creasey CLA 2005-06-02 10:39:41 EDT 
OK point taken.
Comment 4 Boris Bokowski CLA 2006-06-12 22:03:35 EDT 
We should add a constructor to StructuredSelection that takes an IElementComparer. See TreeSelection, we pass in the IElementComparer when the viewer has a comparer.  However, I think we can only use the element comparer in equals() if the element comparer of the other selection is identical to this selection's comparer. Should be fixed in TreeSelection.equals() as well to be consistent.
Comment 5 Boris Bokowski CLA 2007-04-27 20:49:08 EDT 
Deferring
Comment 6 Boris Bokowski CLA 2008-03-24 03:31:19 EDT 
Fix released to HEAD.
Comment 7 Jason Brennan CLA 2008-03-28 14:01:06 EDT 
Verified in I20080327-2251
Comment 8 Boris Bokowski CLA 2008-03-28 22:28:50 EDT 
Marking as verified.