Community
Participate
Working Groups
Build Identifier: Build id: I20100921-1024 WorkbenchAdapterFactory.getAdapter implementation checks for the specific class name of IWorkbenchAdapter, and thus request for adapter for extension interfaces (IWorkbenchAdapter2, and IWorkbenchAdapter3) doesn't return any adapter, even though there is valid adapter present. Because of the above issue, if one override WorkbenchLabelProvider and uses getAdapter2 API, than it will always return null, even if the workbench adapter class associated with the requested object does implement IWorkbenchAdapter2. As a workaround, the user can register adapters himself for these extension interfaces, but ideally this support should be out of box. Reproducible: Always
Created attachment 186111 [details] 333590 v01 Just in-case it helps, I have added the patch which shows one of the approach using which this problem can be fixed.
You're right. The code in WorkbenchAdapter isn't too useful given the factory doesn't support IWorkbenchAdapter2. The question is whether it was a good idea to let WorkbenchAdapter implement IWorkbenchAdapter2 since there's no real benefit from it and clients that do need IWorkbenchAdapter2 functionality can register their own adapter. Having said that, since this is now API, it's probably best to 1. fix the factory 2. add the same code/support for IWorkbenchAdapter3 Boris, what's your take on this?
(In reply to comment #2) > Having said that, since this is now API, it's > probably best to > 1. fix the factory > 2. add the same code/support for IWorkbenchAdapter3 > > Boris, what's your take on this? The patch looks good to me (and it does implement support for IWorkbenchAdapter3 already). But shouldn't WorkbenchAdapterFactory.getAdapterList() be changed too to also return IWorkbenchAdapter2.class and IWorkbenchAdapter3.class?
Created attachment 186345 [details] 333590 v02 Boris, Updated WorkbenchAdapterFactory.getAdapterList() to add support for extension interfaces.
> But shouldn't > WorkbenchAdapterFactory.getAdapterList() be changed too to also return > IWorkbenchAdapter2.class and IWorkbenchAdapter3.class? Yes. The patch is not good as it misses to add the 'IWorkbenchAdapter3' support to 'WorkbenchAdapter'. Also, the additional isInstance(...) check should not be added.
(In reply to comment #5) > The patch is not good as it misses to add the 'IWorkbenchAdapter3' support to > 'WorkbenchAdapter'. Also, the additional isInstance(...) check should not be > added. I agree - WorkbenchAdapter should also implement IWorkbenchAdapter3, and the instanceof check is not needed.
Created attachment 186402 [details] 333590 v03 Boris/Dani, The initial instanceof check was added because, I wasn't sure, if we should be including changes in WorkbenchAdapter to implement IWA3 as part of this defect. Updated the patch to -> Include changes in WorkbenchAdapter to implement IWA3, the changes here are same as discussed earlier with Dani in defect 326695, but was excluded in the last because of this issue in WorkbenchAdapterFactory. -> Removed instanceof check, which is not required now.
Thanks for the updated patch. Committed patch to HEAD with small changes: - fixed indentation (uses spaces not tabs) - removed superfluous comment (//Get the workbench element.) Available in builds >= N20110111-2000.
