Community
Participate
Working Groups
If you have models BASIC and EXTENDED and EXTENDED depends on and derives classes from BASIC, these derived classes are not collected as children by BASICs ItemProviderAdapters. Since EXTENDEDs classes are not known at development time of BASIC there should be some runtime magic to collect appropriate child classes also in EXTENDED-like models.
In general I see two options: a) Provide a registry (as you mentioned) and an ext-point to populate it. I'm not sure if really every class has to be registered. It'd also be fine if I could register certain classes that would cause trouble otherwise. b) Use the local EPackage.Registry of the underlying ResourceSet to reflectively determine the set of derived child classes. I believe that all the needed things are in place except for the loop that augments the generated set of descriptors.
When this is solved, the template for the generated editor has to be adjusted to include something like factories.add(new ComposedAdapterFactory( ComposedAdapterFactory.Descriptor.Registry.INSTANCE)); , otherwise the editor will fall back on the ReflectiveItemProvider for children from EXTENDED models.
Created attachment 87876 [details] Changes to generate child creation extenders Quite a few tricky issues came up, including the need to be able to delegate resource loading to the extenders. I think this quite complete now...
Hi Ed, One thing that was really bothering me as I was looking over this: should we be restricting it to one set of extenders per package like this? In general, you could define any number of different item provider adapter factories for a single package, in order to visualize the model in different ways. But this patch allows you only to register an extender against a package URI. If someone tries to use multiple adapter factories for a single package, they're all forced to pick up the same extenders. I believe this is the first 1-to-1 linkage we've forced between an adapter factory and a package. I wonder if it wouldn't be better to have extensible item provider adapter factories register an ID, and then have the extenders register against those IDs, instead. This would allow for more flexibility for people who want to write their own, additional adapter factories. What do you think?
Wow. That was the opposite of insightful. With the existing design, a user could easily write an item provider adapter factory that picks up extenders for an arbitrary ID, just by changing the second argument to the ChildCreationExtenderManager constructor. So, my suggestion turns into a question: what would you think about changing some names (the "uri" attribute of extender in the extension point, the "namespace" attribute of ChildCreationExtenderManager) to make it sound as flexible as it actually is? We could even still generate the code to use the URI, if that's the most reasonable default. But, a more Eclipse-like dot-separated ID might make sense, too. I'm still doing a careful review. I'll let you know if I find any actual problems. ;)
I've finished the review. I found a few problems: ItemProviderAdapterFactory.javajet 316: In Java 1.4 case, must return Collection, not List (to match what IChildCreationExtender declares). For simplicity, probably should return Collection<?> in Java 5.0, too? GenPackageImpl.java: 4302: I'm not sure what the last condition is supposed to do, but it's always true since allUsedGenPackagesWithClassifiers will never contain a GenFeature. plugin.xmljet 58: Generating the extensions should be conditional on GenPackage.isChildCreationExtenders(), but isn't. childCreationExtenders.exsd 60: I believe meta.attribute should have a basedOn=":org.eclipse.emf.edit.provider.IChildCreationExtender" 82: should be: <extender (impressively, PDE caught the erroneous trailing >) Also, a few opportunities for improvement: ChildCreationExtenderManager.java 139: could collapse two lines: return getChildCreationExtenders().getDelegateResourceLocators(); ItemProviderAdapterFactory.javajet 402: Consider changing "affected" to "supported" or "extended" in the Javadoc. 407: CreationSwitch looks like an implementation detail to me. Why public? GenClassImpl.java 1873: getAllCreateChildFeaturesIncludingDelegation() and getCreateChildFeaturesIncludingDelegation() could share a private, static GenFeatureFilter. GenPackage.java 953: I thihnk getExtendedChildCreationData() really needs some Javadoc, as the return type is quite complicated. How about this: Returns a nested map structure describing the child creation extensions that this package provides to other packages. The result maps other packages to their classes that have child creation extension provided by this package. Each inner map maps from the class to the list of child creation data for these extensions.
I changed the logic to // It belongs to this package if the delegate feature is from this package, // or the class is from this package and either there is no delegate feature or the delegate feature is from some used package. // if (childCreationData.delegatedFeature != null && childCreationData.delegatedFeature.getGenPackage() == this || childCreationData.createClassifier.getGenPackage() == this && (childCreationData.delegatedFeature == null || allUsedGenPackagesWithClassifiers.contains(childCreationData.delegatedFeature.getGenPackage()))) { the plugin.xmljet logic also needs to be in the plugin.xmljet for the editor project (in case the edit and editor are the same project). I can't change this: getChildCreationExtenders(); return childCreationExtenders.getDelegateResourceLocators(); because the field actually has a different type (specialized list with extra methods) than the getter returns. I made the creation switch protected. For GenClassImpl, I created a private method to which both delegate.
The changes have been committed to CVS.
Fix available in HEAD: 2.4.0.I200801292000.
