Bugzilla – Full Text Bug Listing
| Summary: | Provide more structure, safety, and convenience for ID-based references between extension points (id hell) | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Berthold Daum <berthold.daum> | ||||||||||||||||||||||||||||||||
| Component: | UI | Assignee: | Chris Aniszczyk <caniszczyk> | ||||||||||||||||||||||||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||||||||||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||||||||||||||||||
| Priority: | P3 | CC: | berthold.daum, caniszczyk, contact, ed, irbull, jan.diederich, jiri.klement, KetanPadegaonkar, markus.kell.r, pwebster, remy.suen, xavier.mehaut | ||||||||||||||||||||||||||||||||
| Version: | 3.3 | Keywords: | noteworthy | ||||||||||||||||||||||||||||||||
| Target Milestone: | 3.4 M6 | ||||||||||||||||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||||||||||||
| Whiteboard: | id hell | ||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||
This is an excellent idea and we were planning to do that for early 3.4. While this would help to address most of the ID "hell", there will remain cases that we cannot address it in the schemas, e.g. all those "magical" IDs that are declared in code such as menu IDs, etc. in the workbench. *** Bug 117130 has been marked as a duplicate of this bug. *** *** Bug 78483 has been marked as a duplicate of this bug. *** Created attachment 63844 [details]
Schema definition for proposed extension point idFactory
(In reply to comment #1) > This is an excellent idea and we were planning to do that for early 3.4. > > While this would help to address most of the ID "hell", there will remain cases > that we cannot address it in the schemas, e.g. all those "magical" IDs that are > declared in code such as menu IDs, etc. in the workbench. > For all those "magical"id parts contained in the Java code and manifest definitions, the construction of an ID factory could help. The following rather elaborate proposal shows how it could work. Let's consider the construction of menu path. With an extension point org.eclipse.core.idFactory (schema is appended) we could define: <extension point="org.eclipse.core.idFactory"> <idSet id="org.eclipse.ui.menuPath" name="Menu Path" separator="/"> <manifest attributeName="path" elementPath="actionSet/menu" point="org.eclipse.ui.actionSets"> <append attributeName="id" subpath="."> <append attributeName="name" subpath="separator"/> <append attributeName="name" subpath="groupMarker"/> <appendDefault value="additions"/> </append> </manifest> <java annotationType="MenuPath"/> </idSet> </extension> The "java" subelement refers to an annotation type defined as public @interface MenuPath { public String value(); } When defining menus, for example in an ActionBarAdvisor, we could use this annotation type to add IDs to the above idSet: @MenuPath(IWorkbenchActionConstants.M_HELP+"/"+IWorkbenchActionConstants.HELP_END) protected void fillMenuBar(IMenuManager menuBar) { MenuManager helpMenu = new MenuManager("&Help", IWorkbenchActionConstants.M_HELP); menuBar.add(helpMenu); helpMenu.add(getAction(ActionFactory.HELP_CONTENTS.getId())); helpMenu.add(new Separator(IWorkbenchActionConstants.HELP_END)); helpMenu.add(getAction(ActionFactory.ABOUT.getId())); ... } ---------- Another example generating IDs used as "relative" ID in perspective extensions: <extension point="org.eclipse.core.idFactory"> <idSet id="org.eclipse.ui.areaId" name="Area ID"> <manifest attributeName="id" elementPath="view" point="org.eclipse.ui.views"/> <java annotationType="AreaID"/> </idSet> </extension> The "java" subelement refers to an annotation type defined as public @interface AreaID { public String value(); } All we need here is to add an annotation of this type to IPageLayout public static String ID_EDITOR_AREA = "org.eclipse.ui.editorss"; //$NON-NLS-1$ @AreaID(ID_EDITOR_AREA) ---------- How are those manufactured IDs used? Basically in the same way as all other IDs provided by the property "satisfiedBy". Using the notation org.eclipse.core.idFactory:xxxxxx where xxxxx is the ID of a specific idSet, we can refer to that specific idSet and propose its values whenever a ref-attribute is used. Therefore, the "Browse" button mentioned under (1c) would not only list those configuration elements that own an attribute "id" but also all declared idSets org.eclipse.core.idFactory:xxxxxx. The Java part (collecting the IDs defined by the declared annotation types) would probably need some help from the JDT developers. Berthold, thanks for putting a lot of thought into this. Using annotations in Java code may be problematic though, since this will require plug-ins using them to upgrade their Execution Environment to J2SE-1.5. I am not sure what the decision on that will be for 3.4. However, this should not stop us from pursuing the declarative portion in 3.4. (In reply to comment #6) > Berthold, thanks for putting a lot of thought into this. Pure selfishness :) > Using annotations in Java code may be problematic though, since this will > require plug-ins using them to upgrade their Execution Environment to J2SE-1.5. > > I am not sure what the decision on that will be for 3.4. > > However, this should not stop us from pursuing the declarative portion in 3.4. > By allowing for the definitions of literals in the idFactory extension point the Java part can be dropped altogether, e.g. <extension point="org.eclipse.core.idFactory"> <idSet id="org.eclipse.ui.areaId" name="Area ID"> <manifest attributeName="id" elementPath="view" point="org.eclipse.ui.views"/> <literal value="org.eclipse.ui.editorss"/> </idSet> </extension> Created attachment 64350 [details]
Schema definition for proposed extension point idFactory
(In reply to comment #1) > While this would help to address most of the ID "hell", there will remain cases > that we cannot address it in the schemas, e.g. all those "magical" IDs that are > declared in code such as menu IDs, etc. in the workbench. True, without changing code in the platform those IDs wouldn't show up. In 3.4 we will hopefully be in a position to define some of the basic menu structure (file, edit, window, etc) in a declaration (which would make some of it available). PW *** Bug 218593 has been marked as a duplicate of this bug. *** Created attachment 89585 [details]
ID Search attribute v01
This is a proof of concept patch (i.e bad constant names, no edge condition checking)
Just like a java class attribute, it depends on adding appInfo to a string attribute. ex:
<appInfo>
<meta.attribute kind="xml_path" basedOn="org.eclipse.ui.perspectives/perspective/@id"/>
</appInfo>
This added to the targetID attribute of perspective elements in org.eclipse.ui.perspectives will list perpsective IDs when the Browse button is clicked.
For org.eclipse.ui.handlers - handler commandId attribute the basedOn string looks like "org.eclipse.ui.commands/command/@id"
This is the PDEEditor part of the story ... there's no schema editor support for adding this appInfo marker.
PW
Created attachment 89861 [details]
Schema examples v01
using the new attribute type in perspectiveExtensions - targetID and handlers - commandId
PW
investigating... Created attachment 89892 [details]
ID Search attribute v02
This builds on top of Paul's patch by:
-making the 'Browse' button of the new IdAttributeRow be on the same line as the other widgets
-prevents NPEs if the xml_path that has been entered in the exsd is not correct
The openReference() method is still not implemented but it should ideally open the plugin.xml of the declaring plug-in of the id in question.
Created attachment 89945 [details]
org.eclipse.pde.patch
Here is an updated patch for you to play with Paul. The only thing you will have to change is updating your sample schemas to use "identifier" instead of "xml_path"
- This patches adds support for the notion of an 'identifier' throughout the schema models.
- The schema editor has support in defining an 'identifier' via a filtered selection dialog
- The extension editor has support to browse these identifiers and jump to the defining plug-in
I will work on cleaning up the patch a bit more tomorrow so we can possibly have something ready for the next I-build. I'm also working on actually adding schema-model related tests to PDE.
Created attachment 89946 [details]
mylyn/context/zip
Created attachment 89948 [details]
Schema examples v02
First let me say this is awesome!
Updated the schema examples to work with Chris' latest patch. commands, handlers, bindings, contexts, and perspectiveExtensions.
Very cool.
2 comments on the schema editor part:
The dependsOn field only seems to hold the last value filled in ... i.e. if I switch to a previous identifier field, it still shows the old value. AFAICT the xml is correct, though.
The attribute search dialog won't let you keep typing to narrow it ... i.e. id (which is everything :-) - perspective
Thanx a lot Chris ... did I mention this is awesome? Try and datafill a keybindging now :-)
PW
Created attachment 89999 [details]
org.eclipse.pde.patch
Ok, I think we're good for launch. I cleaned up the code, created an image for identifier attributes and added some validation to the mix.
I think we're at the stage where we can release this to the public for testing and stabilization. What do you say Paul, let's dance?
Created attachment 90000 [details]
mylyn/context/zip
Created attachment 90001 [details]
sample screenshot #1
Created attachment 90002 [details]
sample screenshot #2
WOW ! This is *really* awesome :) Created attachment 90010 [details]
org.eclipse.pde.patch
Paul, try out this patch. The new attribute type 'identifier' is also allowed to specify restrictions in combination with the reference path. So for example, you can update the perspectiveExtensions example with a restriction of "*" to include that as a valid identifier. Restrictions in combination with the reference path will be used to compute valid identifiers.
Paul, so something like:
<attribute name="targetID" type="string" use="required">
<annotation>
<documentation>
the unique identifier of the perspective (as specified in the registry) into which the contribution is made. If the value is set to "*" the extension is applied to all perspectives.
</documentation>
<appinfo>
<meta.attribute kind="identifier" basedOn="org.eclipse.ui.perspectives/perspective/@id"/>
</appinfo>
</annotation>
<simpleType>
<restriction base="string">
<enumeration value="*">
</enumeration>
</restriction>
</simpleType>
</attribute>
This will cause "*" come up in the list also along with other valid identifiers.
Ok, this will go into the next I-build to gather feedback from the community.
> 20080218
Created attachment 90011 [details]
mylyn/context/zip
Created attachment 90013 [details]
Schema examples v03
Works with PDE HEAD, released to showcase the PDE feature >20080218
PW
To the I-Build I downloaded both: the Win32 and the Linux version, but both archives (zip and tar) seem to be broken. Can someone look at this? I don't see any issues. verified in I20080327-2251 |
Rationale: The ID-based "wiring" between plug-ins is currently one of the most error-prone and tedious areas when developing Eclipse plug-ins and Eclipse RCP applications. 1. Enhancements to the extension point schema (exsd). a) Allow for a new attribute type "ref". Attributes of that type will refer to the id-Attribute of other configuration elements. b) For attributes of type "ref" provide a property called "satisfiedBy". This property will specify which configuration elements qualify as an id-source for the attribute. c) For the property "satisfiedBy" provide a "Browse" button. On demand this button will list all known configuration elements that own an attribute "id". Example: We show an example for extension point org.eclipse.ui.perspectiveExtensions. Extensions to this point can, for example, refer to a given view by ID. We change the type of attribute "id" from "string" to "ref" and add the attribute "satisfiedBy". This attribute specifies as an id-source the configuration element "view" in extension point "org.eclipse.ui.views". These configuration element has an "id" attribute and thus qualifies as an id-source. <element name="view"> <complexType> <attribute name="id" type="ref" use="required" satisfiedBy="org.eclipse.ui.views/view"> <annotation> <documentation> the unique identifier of the view which will be added to the perspective layout. </documentation> </annotation> </attribute> .... </element> 2. Enhancement to the "Extensions" section of the Plug-in manifest editor. For attributes declared in the schema as of type "ref" provide a "Browse" button. On demand this button will list the IDs of all configuration elements matching the path specified in the schema's "satisfiedBy" property. 3. Automatically check for dangling "ref" attributes in a given scope. This tool should be provided both for features and RCP product configurations. A warning should be given for each dangling reference. This check should be made optional/configurable via the Preferences.