Community
Participate
Working Groups
3.1 M4 The WorkbenchAdvisor is currently overloaded with the responsibility to configure the workbench, all workbench windows, and the windows' action bars. This leads to either a large implementing advisor class, or to clients delegating to other objects in an ad-hoc way (e.g. the IDE's WorkbenchActionBuilder). Implementors often get state management wrong, keeping window-level state as fields on the (singleton) workbench advisor. The window-level responsibilities in WorkbenchAdvisor should be split out into a window-level advisor, WorkbenchWindowAdvisor. Action bar configuration split out as well into an ActionBarAdvisor. This will lead to a 1:1 correspondence between the advisors and the configurers. In addition, this opens up the possibility of allowing different window configurations within the same application (e.g. main window, help window, update window). Currently Help and Update have to resort to using a JFace ApplicationWindow to avoid unwanted menu contributions. These changes must not break any existing WorkbenchAdvisor implementations. Backwards compatibility should be provided via a CompatibilityWorkbenchWindowAdvisor, which will delegate to the old WorkbenchAdvisor methods.
ActionBarAdvisor should also provide a default implementaton of fillActionBars that implements the best practices for handling the FILL_PROXY flags, instantiating actions, and actually populating the action bars. It may also provide conveniences for managing window-level actions.
First cut released. Readme example fixed up, working on Hyperbola and IDE. Still need to figure out how to move getInitialPerspectiveId() and getDefaultPageInput() over to WorkbenchWindowAdvisor -- the workbench assumes these are global in some places. Maybe getMainPreferencePageId() too.
Will also have to fix up the PDE RCP templates.
CC'ing Wassim for the required template work. I can provide a fixed up template for the default example.
Refactored IDEWorkbenchAdvisor, adding IDEWorkbenchWindowAdvisor, and changing WorkbenchActionBuilder to extend ActionBarAdvisor.
The main changes have been released for M5. Still want to look at possibility of moving getDefaultPageInput() and getInitialPerspectiveId() to window advisor for M6. Filed bug 84971 for the PDE template work.
And also the possibility of allowing different window advisors for different windows.
While you're at it can you fix it so the developer doesn't have to create a perspective factory and default perspective unless they really need one? That would simplify the minimal RCP app.
I put a note about the changes here: http://www.eclipsepowered.org/archives/2005/02/15/workbenchadvisor-refactored/ . It pretty much repeats what was said here and asks for comments (to come back here).
The perspective can be null, see bug 71150, however you usually should provide one so I've left getInitialPerspectiveId() as abstract.
That's not quite the same thing. I wasn't asking for "no perspective" (I'm not sure what that would even mean, how can you open windows and have toolbar actions?) I was asking for a default (stub, empty, do-nothing) perspective. If it ever makes any sense to omit it then getInitialPerspectiveId() should not be abstract. It's not a big deal but I would argue that the default implementation should get the id of some kind of stub perspective. I was thinking that simple RCP apps that don't need more than one perspective, maybe most RCP apps, could just use the default one, and we could simplify the doc for learning how to build RCP apps. I know, I know, it's not hard to make one but it's one thing you could do to lower the bar for adoption.
Re Comment #11 > I would argue that the default implementation should get the id of some kind of stub perspective. Seems strange. What purpose would this stub perspective serve? An RCP application needs to contribute at least one perspective to do anything useful, no? What views would be visible in this perspective by default? Would the stub perspective contain an editor area?
We also wouldn't want the stub perspective showing up in the list of available perspectives (assuming that's made visible by the app). If getting off the ground is the main concern, the PDE templates provides a good start.
Views can be contributed to a perspective with the org.eclipse.ui.perspectiveExtensions extension point. Editor area defaults to on. It's not a big deal, just struck me as odd when I was writing the tutorial.
Didn't this (split api) get completed in 3.1M5a?
I'm still pondering the points in comment #6, in addition to the idea of allowing different types of window advisor in the same app.
An unfortunate side-effect of this split is that it is technically hard to have more than one type of WorkbenchWindowAdvisor - but the APIs make this seem possible. The WorkbenchAdvisor needs information about which type of WindowAdvisor to create and currently it doesn't have access to either the page input or perspective id when createWorkbenchWindowAdvisor() is called.
> it doesn't have access to either the page input or perspective id when createWorkbenchWindowAdvisor() is called I'm aware of this limitation, and am planning on making this info available, either to the create method directly, or via get methods on the IWorkbenchWindowConfigurer. Any preference?
Advantage of method parameters are that they are explicit but the advisor will have to cache them if they are needed later. If they are available via the configurer they are automatically cached. I don't have a strong preference.
> it doesn't have access to either the page input or perspective id when createWorkbenchWindowAdvisor() is called It turns out this is harder to handle than I thought. The page input and perspective ID are available and can be passed when the window is first opened, however when a window is restored from a previous session, this info is not available before the window is created (and is no longer considered an argument to the window creation). I'm going to have to leave this for 3.2. I've filed bug 88816 for this. Closing this PR with M5 tag (no advisor changes were in M6).
Marking as verified. You're soaking in it.
