Bug 167323 - [Viewers]Open AbstractTreeViewer for external widget implementors
Summary: [Viewers]Open AbstractTreeViewer for external widget implementors
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3 M5   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2006-12-09 11:40 EST by Thomas Schindl CLA
Modified: 2007-02-01 05:49 EST (History)
6 users (show)

See Also:


Attachments
A patch to start discussion (67.34 KB, patch)
2006-12-13 04:12 EST, Thomas Schindl CLA
no flags Details | Diff
Open AbstractTreeViewer as much as need for subclassers (24.64 KB, patch)
2006-12-26 14:59 EST, Thomas Schindl CLA
no flags Details | Diff
Reapply able to head (23.68 KB, patch)
2007-01-14 09:53 EST, Thomas Schindl CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Schindl CLA 2006-12-09 11:40:18 EST
As reported by Ross Camara AbstractTreeViewer would work for nebula if we would open the API a bit. Boris and I decided that we will do this in 2 steps:
1. open the API
2. moving the Virtual-Support bits from TreeViewer to AbstractTreeViewer when the nebula grid provides virtual support.
Comment 1 Thomas Schindl CLA 2006-12-13 04:12:53 EST
Created attachment 55569 [details]
A patch to start discussion

I know Boris and I have thought about opening AbstractTreeViewer but after playing around a bit i think the better idea is to take the same approach like we did for TableViewer, so I started and finally it was not really a lot of work. The only thing discussable is the name of the new Viewer called VirtualAbstractTreeViewer, I think we should come up with an equal name for both of our widget-independent-columnbased-viewers.
Comment 2 Thomas Schindl CLA 2006-12-26 14:59:04 EST
Created attachment 56189 [details]
Open AbstractTreeViewer as much as need for subclassers

Ok guys this time a patch which opens AbstractTreeViewer and moves common things from TreeViewer to it so that subclassers can easily implement a viewer on top of it. 

I've also included a potential implementation for Grid and a test snippet which is copied from our snippets collection. Please report back as soon as possible whether the API provided using this patch is open enough to all of you.
Comment 3 Ross Camara CLA 2007-01-04 17:02:59 EST
One comment i had when implementing the GridViewers is that in both implementations i needed to override the getViewerRowFromItem(Widget) method. The problem here is that if we don't find the viewer row in the data map of the widget we return null throwing an NPE. In the TreeViewer you override the method also so that when no ViewerRow is found we create a new ViewerRow to return. I think we should just make this method abstract and not provide the base implementation. 
Comment 4 Thomas Schindl CLA 2007-01-05 02:33:16 EST
(In reply to comment #3)
> One comment i had when implementing the GridViewers is that in both
> implementations i needed to override the getViewerRowFromItem(Widget) method.
> The problem here is that if we don't find the viewer row in the data map of the
> widget we return null throwing an NPE. In the TreeViewer you override the
> method also so that when no ViewerRow is found we create a new ViewerRow to
> return. I think we should just make this method abstract and not provide the
> base implementation. 
> 

well this is not possible because if we declare this method abstract in AbstractTreeViewer we would break API contracts backwards. We can declare it abstract of course in ColumnViewer and AbstractTableViewer we can do that and it would certainly make sense IMHO. That's why I once proposed to insert a new Sub-Class below AbstractTreeViewer which declares the method as abstract which could not be declared abstract in AbstractTreeViewer because of backwards compatility. E.g. getItemAt() is also such a method!
Comment 5 Thomas Schindl CLA 2007-01-14 09:53:33 EST
Created attachment 56880 [details]
Reapply able to head
Comment 6 Boris Bokowski CLA 2007-01-17 17:09:05 EST
Slighlty modified patch released >20070117.  Chris, can you please have a look at HEAD to make sure I didn't break your subclasses?  I made two methods final that were previously not.

For 3.3, we don't have enough time to address the SWT.VIRTUAL case, this will have to wait and should be tracked with a new bug.
Comment 7 Ross Camara CLA 2007-01-17 18:25:02 EST
Verified with the latest version of cvs and our viewers still work. 
Comment 8 Chris Gross CLA 2007-01-19 09:35:29 EST
Tom, Boris - thanks for all the hard work.