Community
Participate
Working Groups
3.4 I20080513-2000 TreeViewer and TableViewer are marked as 'This class is not intended to be subclassed by clients.' Maybe this was always marked as this and I never realized, but it dosen't make sense to have this restriction. There are also at least 20 subtypes of TreeViewer in SDK alone. One reason to subclass the viewers is to implement protected void handleLabelProviderChanged(LabelProviderChangedEvent event) Label changes are broadcasted in terms of resources, but the viewer might show model elements like Java elements and has to do a mapping to evaluate the elements to update. Other use cases are fine tuning of protected void handleInvalidSelection(...) See the CommonViewer for more use cases.
(In reply to comment #0) > Maybe this was always marked as this and I never realized Yes it was always marked like this. :-)
Boris, any chance we get this for 3.5? Every class in the TableViewer hierarchy allows subclassing except ColumnViewer and TableViewer. I can understand this for the ColumnViewer which is an implementation detail but subclassing TableViewer is really needed. NOTE: ColumnViewer is missing the correct API tooling tags.
The problem with this is, while I realize that there are advanced use cases that currently require subclassing, I don't want to encourage clients to create subclasses gratuitously. For example, the Javadoc for TableViewer does not explain the interface for subclassers. It would be easier for me to say yes to a more narrow API. How about something like this: TableViewer.setViewerStrategy(ViewerStrategy strategy) public abstract class ViewerStrategy { protected ... handleLabelProviderChanged(...); protected ... handleInvalidSelection(...) } How many methods are we talking about? How much control over what happens do you need in methods like handleInvalidSelection (firing of events?) - would you be able to move what you are doing today into a separate class without access to protected methods?
There are lots of methods: look e.g. at org.eclipse.jdt.internal.ui.browsing.PackagesViewTableViewer and all its super classes in the hierarchy. org.eclipse.search.internal.ui.SearchResultViewer is even worse. The strategy also has performance impacts as it introduces indirections. TreeViewer has many more subclasses than TableViewer and each of them overrides/extends other features i.e. the strategy will be quite big. I think we can't expect that all the owners of those classes go and refactor their code, so I guess either we open up the Tree/TableViewer or we say no and clients will most likely live with that fact and see the warning in their code.
Per bug 234345 comment 4, please extend this discussion to include the protected internalRefresh(Widget, Object, boolean, boolean) and the internalRemove(Object[]) methods in AbstractTreeViewer (which is not marked with @noextend) that are currently tagged with: "EXPERIMENTAL. Not to be used except by JDT. This method was added to support JDT's explorations into grouping by working sets, which requires viewers to support multiple equal elements. See bug 76482 for more details. This support will likely be removed in Eclipse 3.2 in favor of proper support for multiple equal elements." I myself am pretty neutral about this topic since the generic viewers have been "good enough" for me so far. We did have somebody on IRC a few weeks ago express the desire to subclass TreeViewer and I couldn't really give a good response to that outside of saying "the javadocs says you shouldn't do that".
Hitesh is now responsible for watching bugs in the [Viewers] component area.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
