This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 296739 - Evolution of the org.eclipse.ui.* extension points
Summary: Evolution of the org.eclipse.ui.* extension points
Status: RESOLVED FIXED
Alias: None
Product: e4
Classification: Eclipse Project
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact: Oleg Besedin CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-02 14:33 EST by Oleg Besedin CLA
Modified: 2012-10-03 00:31 EDT (History)
12 users (show)

See Also:


Attachments
Model element for "model snippets" (54.06 KB, patch)
2009-12-17 15:37 EST, Oleg Besedin CLA
no flags Details | Diff
Model Components (98.46 KB, text/plain)
2010-01-05 16:40 EST, Oleg Besedin CLA
no flags Details
"Show View" - first draft (88.84 KB, patch)
2010-01-07 16:21 EST, Oleg Besedin CLA
no flags Details | Diff
"Show Views" - now views have a desired view stack (31.96 KB, patch)
2010-01-12 10:02 EST, Oleg Besedin CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oleg Besedin CLA 2009-12-02 14:33:55 EST
For the existing org.eclipse.ui.* extension points, several possibilities are open:
	(a) create a model element for them in some form;
	(b) deprecate / ignore aside from compatibility;
	(c) continue to use existing extension point.

I'd like to open this bug to consider extension points that make sense as a model element - the "Type (a)" ext. points in the list above.

From looking at the list, the ext.points that might be considered as (a), are the following groups:

+ convenience grouping
	.actionSets / .actionSetPartAssociations 

+ extend parts that do not belong to me
	.dropActions
	.editorActions
	.popupMenus

+ declaring things that I have available
	.editors
	.views
	.newWizards / .exportWizards / .importWizards
	.preferecePages / .preferenceTransfer

+ describing elements that can affect multiple places in the workbench
	.decorators
	.markerSupport / .markerResolution / .markerImageProviders / .markerHelp

+ product-level definitions
	.intro / .helpSupport
Comment 1 Eric Moffatt CLA 2009-12-04 13:46:54 EST
How about:

- some form of the 'Command' contribution extensions.
- some form of the perspective/perspectiveExtensions.

When I say 'some form of' I just mean that we should take a look at the existing EP's and see if they either contain information we don't want to support in E4 and/or whether they can be augmented to allow the use of E4's new capabilities.

For example, we could augment a perspective definition to include an 'editorAreaId', allowing different perspectives to use different editor areas.
Comment 2 Oleg Besedin CLA 2009-12-14 16:54:19 EST
Let's use 
	org.eclipse.ui.views, and 
	org.ecilpse.ui.editors 

as concrete examples to figure out the basics. There are at least two dimensions here worth some discussion: should extensions be a part of the model, and how extensions should be described.
 
I. At run time where do we keep a list of available views / editors?

 - In the model?

Say, at the run time extensions are stored in the model. EMF "Application" element gets a list of "ViewerDescriptors" and a list of "EditorDescriptors".

Pros: everything is in the model. There is one place to worry about and only one place to learn.
Cons: model bloat. Suddenly, there are 3 new EMF classes with a bunch of strange attributes.

 - In a separate memory structure?

Pros: Model stays small and simple. Tie-ins to contributing bundles are more natural.
Cons: developer need to learn about this new separate piece. Whatever is the strategy of adding / removing elements from it, is it a different process from working with the model.

II. How do we populate it?

 - Using model snippets / model "diffs"

An XMI is produced somehow (by tool or by hand) which is a difference in the model contributed by this plugin. Extensions are supplied as model deltas.

Pros: generic mechanism that ties together multiple extensions. Very intuitive.
Cons: needs tooling, potentially error-prone due to complexities of diff/merge. 
Restrictions: For (I), extensions need to be stored in the model 

 - Using extension points 

Pros: well-undestood approach that does not require special tooling
Cons: no natural ties to the E4 UI model. Every extension is isolated.


From my viewpoit, I'd like to see E4 extensibiilty done by keeping most UI extensions in the model and using "model deltas" to describe extensions. However, while potentially most promising, it is a risky path and has drawbacks.

Any opinions?
Comment 3 Boris Bokowski CLA 2009-12-14 17:17:39 EST
(In reply to comment #2)
> Any opinions?

Yes ;-)

> From my viewpoint, I'd like to see E4 extensibiilty done by keeping most UI
> extensions in the model and using "model deltas" to describe extensions.
> However, while potentially most promising, it is a risky path and has
> drawbacks.

I like the consistency of this approach, and also how it will allow us to finally offer a way by which the UI elements of a complete application can be filtered, sorted, and otherwise manipulated. We've tried to do this in 3.x (e.g. Equinox transforms, activities) but never found a satisfactory solution. Now that we have a chance of solving this problem, I would like us to seize the opportunity.

I hope we will have time to provide at least minimal tooling for this, because tooling is important to make it usable. Maybe someone who is interested in the PDE side is able to help?
Comment 4 Eric Moffatt CLA 2009-12-15 10:05:10 EST
I see it the same way...the Model (MApplication?) contains an extension container for each extension. There is an specific model element for each extension that can either be populated by reading the PDE extensions or programmatically. See bug 296478 comments 21 & 23 for a description of a possible startup cycle.

One of the advantages of putting the elements into the model is the synergy it creates; any techniques we develop for the model are automatically available for our extension handling. For example it should be possible for someone to add extensions into the model by using a 'snippet' extension...
Comment 5 Oleg Besedin CLA 2009-12-17 15:37:34 EST
Created attachment 154703 [details]
Model element for "model snippets"

The patch adds a model element to keep model snippets. (Patch applied to CVS Head.)
Comment 6 Oleg Besedin CLA 2010-01-05 16:40:40 EST
Created attachment 155377 [details]
Model Components

As Remy and myself decided that "Snippets" can't be productively used for both model merges and extenions, I'll take the liberty to modify "Snippets" to fit better into model extensions. 

On the "public" side:

"org.eclipse.e4.workbench.model" extension point -> 
  components.xmi -> 
    list of ModelComponents


- We now have "ModelComponent" instead of "Snipptes" that serve as a holder for whatever elements plugin authors would want to add. The "Model Component" is exactly that - it is a piece of model that resides under some element with the specified ID. 

- As plugin author might need to add more than one ModelComponent, there is a container "ModelComponents". Having multiple components solves problem of references, such as adding a command and a handler for that command.

- [Likely to change] The XMI that describes model components is supplied via the extension point "org.eclipse.e4.workbench.model". (I'd like to change that to a header in the manifest.mf.)

For illustaration on how this works, try E4 IDE application with the launch configuration that include the photo demo. It should come up with the IDE menus + browser and preview part from the photo demo + the menu item and hander to create a new album.

(Patch applied to CVS Head.)
Comment 7 Remy Suen CLA 2010-01-06 10:13:31 EST
(In reply to comment #6)
> - [Likely to change] The XMI that describes model components is supplied via
> the extension point "org.eclipse.e4.workbench.model". (I'd like to change that
> to a header in the manifest.mf.)

So you mean like how localization is done by...

Bundle-Localization: fileName

...we would do something similar...

Eclipse-ModelExtension: fileName

...like that or...?

> For illustaration on how this works, try E4 IDE application with the launch
> configuration that include the photo demo.

Note that you need the org.eclipse.e4.ide.application bundle, I had to check this out from CVS.

I get an error opening components.xmi but I guess that's because EMF doesn't load the model in the workspace.
Comment 8 Oleg Besedin CLA 2010-01-06 10:44:31 EST
(In reply to comment #7)
> (In reply to comment #6)
> > - [Likely to change] The XMI that describes model components is supplied via
> > the extension point "org.eclipse.e4.workbench.model". (I'd like to change that
> > to a header in the manifest.mf.)
> So you mean like how localization is done by...
> Bundle-Localization: fileName
> ...we would do something similar...
> Eclipse-ModelExtension: fileName
> ...like that or...?

Yes, something like this. Probably:
 Eclipse-ModelComponents: <path_file1>, <path_file2>, ...

I am trying to avoid using the word "extension" so not to confuse with actually extending the UI model by providing new model element types. Hence, "components".

+ The upside is that this removes a level of indirection (no need for the extension point). 
+/- The [rather interesting] mixed bag is the treatment of non-singleton bundles: this way we could say that we accept model components from multiple versions of the same bundle - something that has been a hot discussion topic in the past. 
- The downside is that registry route has some useful conviniences and corner cases figured out.
Comment 9 Oleg Besedin CLA 2010-01-06 10:48:20 EST
(In reply to comment #7)
> I get an error opening components.xmi but I guess that's because EMF doesn't
> load the model in the workspace.

Yes, I've often seen this when the Eclipse environment I am using contains model definition that is different from what is checked out in the workspace. For me, I had to switch to using 3.6 environment for development and checking out everything from CVS.
Comment 10 Remy Suen CLA 2010-01-06 14:55:25 EST
Merging should work properly for the e4 IDE application now. There were two problems at hand.

1. It wasn't using the e4xmi extension.
2. The model extensions were loaded _after_ the reconciler was ran. I've moved the model extension processing code inside the ResourceHandler now so that the extensions would be processed before the reconciler ran. Note that at this point the application has no context so it is not possible for it to retrieve a Logger. Oleg, can you change that around when you fix that up when you get a chance and to also use @Inject instead of querying the context directly?
Comment 11 Remy Suen CLA 2010-01-06 22:03:28 EST
If I want a part to show up in two different perspectives, don't I have an id conflict? Or is the processor going to generate placeholders on the fly? Actually, that doesn't make sense, the renderer should take care of this I would think...?
Comment 12 Oleg Besedin CLA 2010-01-07 14:35:09 EST
(In reply to comment #9)
> (In reply to comment #7)
> > I get an error opening components.xmi but I guess that's because EMF doesn't
> > load the model in the workspace.
> Yes, I've often seen this when the Eclipse environment I am using contains
> model definition that is different from what is checked out in the workspace.
> For me, I had to switch to using 3.6 environment for development and checking
> out everything from CVS.

I'll ask about this on the newsgroup as it really starts to impact my work. In order to get IDs generated I can't use 3.6-based environment anymore.
Comment 13 Eric Moffatt CLA 2010-01-07 15:15:41 EST
(In reply to comment #11)
> If I want a part to show up in two different perspectives, don't I have an id
> conflict? Or is the processor going to generate placeholders on the fly?
> Actually, that doesn't make sense, the renderer should take care of this I
> would think...?

Depends on whether or not we force the MPlaceholder to have the same ID as the element that it references. At the moment they don't need to have an ID since the reference is hard, not done through any 'find' mechanism.

We might want to tweak the 'find' code to understand that when checking the id it should check the id of the referenced element if it's a placeholder. This means that when searching for a view by id we'd find the placeholder for it if it exists in the hierarchy.
Comment 14 Oleg Besedin CLA 2010-01-07 16:21:32 EST
Created attachment 155556 [details]
"Show View" - first draft

The patch adds basic ability to add view descriptors and "Show View" command. (The dialog for the "Show View" is a temporary oversimplification; the real one will be migrated from 3.x later.)

The Photo demo is ammended to contribute two views - "Library" and "Preview". Note that there is a bug in the renderer layout; as a result the application window needs to be resized for the newly added view to show up.

There are a number of obvious points to be be improved, but this is the start,

On the model side, patch adds PartDescriptor and PartDescirptorContainer and makes application a part descriptor container.

(Patch applied to CVS Head.)
Comment 15 Oleg Besedin CLA 2010-01-07 16:29:29 EST
(Opened bug 299083 for the layout problem.)
Comment 16 Remy Suen CLA 2010-01-08 08:12:04 EST
Oleg, I took a look at your EPS problem. It seems that your handler is being injected with the application's context when I'd expect it to be injected with the window's context. This happens in MenuItemRenderer's hookControllerLogic(MUIElement).
Comment 17 Remy Suen CLA 2010-01-08 08:21:04 EST
The cause is due to AbstractPartRenderer's getParentWithContext(MUIElement) method which only tries to retrieve the parent model. Because the 'parent' feature is tied to the 'children' feature, while the window is technically the parent of a menu, since the menu is not one of the window's "children", getParent() returns null and we ultimately end up using the application's context. I hope my explanation made sense.
Comment 18 Eric Moffatt CLA 2010-01-08 10:52:27 EST
Committed in >20100108. Changed this code to use EObject's eContainer() rather than relying on the UIElement's 'parent' state (since it's only useful for elements in the 'child' hierarchy).

I don't think that the use of EMF in this case is a bad idea since it will correctly cover any model that has elements it 'owns' that aren't in the basic 'child' tree structure. Can anyone see potential issues in doing this ?
Comment 19 Oleg Besedin CLA 2010-01-12 10:02:40 EST
Created attachment 155864 [details]
"Show Views" - now views have a desired view stack 

The patch adds a "category" attribute to the PartDescriptor. The attribute is to be used to find desired container for the new view. 

The patch changes IDE application to have the following predefined view stacks:

- org.eclipse.e4.primaryNavigationStack
- org.eclipse.e4.secondaryNavigationStack
- org.eclipse.e4.primaryDataStack
- org.eclipse.e4.secondaryDataStack

The patch changes views from the Photo demo to provide desired view stack.

There is a bug in redreding code: the new views are added, but not activated. Until that bug is fixed, one needs to click on the view tab for the view to show up.

To see this in action, as before, launh the product from the "org.eclipse.e4.ide.application" bundle; add the photo demo to the launch configuration; use Window -> Show Views menu item.
Comment 20 Paul Webster CLA 2010-01-12 10:13:36 EST
(In reply to comment #19)
> Created an attachment (id=155864) [details]
> "Show Views" - now views have a desired view stack 
> 
> The patch adds a "category" attribute to the PartDescriptor. The attribute is
> to be used to find desired container for the new view. 

Isn't the desired container an function of the perspective (layout), not the view descriptor itself (template to generate a part that can be instantiated)?

PW
Comment 21 Oleg Besedin CLA 2010-01-12 10:14:10 EST
Opened bug 299379 for the view activation problem.
Comment 22 Oleg Besedin CLA 2010-01-12 10:24:00 EST
(In reply to comment #20)
> Isn't the desired container an function of the perspective (layout), not the
> view descriptor itself (template to generate a part that can be instantiated)?

I think, in the end, the lookup for the destination stack should be limited to the current perspective. If stack is not found, one of the stacks on the perspective should be picked up as a default stack, of, if none exists, a new stack created. But at this time it just uses #findPart(), waiting for changes to perspective to be released.
Comment 23 Eric Moffatt CLA 2010-01-12 10:48:31 EST
If the app is using perspectives then (for now) we should expect that there's only one MPerspectiveStack. Searches might first do a 'findByType(MPerspectiveStack.class)' first and if one is found it could use its active child (an MPerspective) as the root of the search. If none is found then use the MWindow as the root.
Comment 24 Lars Vogel CLA 2012-10-03 00:31:00 EDT
I think this discussion is outdated. Closing as "Fixed". Please reopen if you disagree.