Bugzilla – Full Text Bug Listing
| Summary: | [CommonNavigator] [Discussion] Navigator extensions only operate at one level | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Michael Valenta <Michael.Valenta> |
| Component: | UI | Assignee: | Michael D. Elder <mdelder> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | dmisic, jcorchis, n.a.edgar |
| Version: | 3.2 | ||
| Target Milestone: | 3.2 M5 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
|
Description
Michael Valenta
I would expect the label provider of a Navigator Extension to be consulted for any elements that come from that Navigator Extension's content provider. Likewise, the content provider should be consulted first to get the children of a parent element it provided (in addition to allowing other extensions to augment this). In a former life, the Common Navigator framework supported functionality exactly as you specify. The problem with this approach is that link with editor fails for any content which is not specified in the enablement (that is, for a request to set the selection for an item that has not been shown in the tree). Previously, tricks were played with weak hash maps to ensure that only the first level of content was necessary in the enablement clause. Once the content provider contributed that child, the same label provider would always be consulted first, and if no label was present, others could override it. The content provider was also the first to be invoked for children for that element. However, whenever someone tried to do a selectAndReveal() on an object that had not yet been shown in the tree and who was not described by the enablement clause, the setAndReveal() would fail (since it could not identify the correct content provider to invoke for getParent()). This lead to some intermittent failures that were hard to debug. As a result, the decesion was made when porting down to WTP that it would require all elements to be specified. If we can come up with a solution for selectAndReveal(), I am fine with adding this functionality back into the framework. As for consulting the label provider of the contributing extension first, this is a change that could be made to the framework with minimal (or no) impact to the client extensions. First off, we should clarify what the intent is of the <enablement> element. Does it describe the kinds of elements provided by the extension's content provider, or does it describe the parent element (i.e. from some other extension) under which the extension's content provider can provide (additional) elements? I was assuming the latter, but from your description it sounds more like the former. This needs to be clarified in the extension point schema. Either way, I would expect the framework to keep track of which extension provided which elements, and talk directly to that extension's label provider for those elements' labels. I would not expect <enablement> to come into play at all for labels, unless there is some way of overriding labels provided by another extension. And in that case, we should probably separate out the label provider into a separate extension point, as is done for actions. I'm fine with keeping <enablement> (or whatever element) for describing the output elements so that editor linking, Show In, and other select/reveal cases work OK. We'll need to clarify the semantics if multiple extensions have overlapping <enablement> clauses though. It's also unclear to me what adaptable="true" means in this case. Normally it means "this applies to any element that can adapt to the class specified", but there's no way of mapping from an adapter result (i.e. an IResource) back to the adapter (i.e. the model element) via the basic IAdapter mechanism, so I don't see how it helps in the select/reveal case. For the resource extension case, I'd expect its output to be described simply as IResource, and for this to match all flavours of resources. Is there a reason for the current extension specifying IWorkspaceRoot, IProject, IFolder and IFile separately? On a related note, I am also concerned about conflicting enablement rules. What happens if two extensions indicate that they want to handle IResource? Who wins? Or do they both win? Similarly, what happens if one indicates IResource and another indicates IFile? Does the IFile contributor win since it is more specific? I tend to agree with Nick that we should consider separating these pieces out. I also think that there is commonality between the enablement rule of a navigator extension and the adapterFactory extension in Runtime (i.e. adapterFactors handle the IFile/IResource case nicely). Basically, we have an IWorkbenchAdapter API that has overlap with how enablement rules work for the navigatorExtension (with the advantage that the workbench adapter solves some of the problems we are discussing here). I would like to understand why that mechanism doesn't meet the needs of the Common Navigator or conversly, what advantages the approach taken for the Common Navigator has over the adaptable mechanism. One difference is that you can only obtain one IWorkbenchAdapter from a given resource or resource mapping, so there's only one way of presenting it. The Common Navigator is more open-ended. Different navigator extensions could have different ways of showing the same resource(s). Of course, it's unclear what this means to the user if this actually happens. But I just wanted to illustrate this fundamental difference between an adapter-based approach and an extension-based approach. Yes, I see what you mean. However, I have a specific issue which works for the adapter mechanism and not for enablement rules. I am using instances of ModelProvider to root the contributions from multiple models in the same CommonViewer. Enablement rules work OK when each model provides a content extension for their specific model provider class. But what if one or more do not. I would like to register a default extension against the ModelProvider class so any subclass that does not have one registered would default to use the one registered against ModelProvider. I get this behavior with the adaptable mechanism but I suspect I wouldn't with enablement rules. I am open to alternatives on this. One possibility would be to have an override mechanism whereby a navigatorContent extension can explicitly indicate that it overrides another. That would handle my case but has the disadvantage that one of the parties must know about the conflict.This may not be possible in all cases. I think my last two comments (comment 5 and comment 7) are really part of a separate problem. I have entered bug 115012 for this. I'm OK with keeping the enablement rules as well but, like Nick, I am concerned about how conflicting content providers will be managed. To illustrate the problem more concretely, I've pasted the extension I created for the example I am working on. There two issues I have: 1) I need to specify 8 objectClass enablement rules. It turns out this isn't any worse than what I needed to do for my adapterFactories either. The problem is that EMF generates models without a common superclass. This isn't such a big deal for adapter factories because EMF generaly doesn't use them (they have their own concept of an adapter factory). 2) In actuality, there is a common superclass to EMF objects. It is EObject but I didn't feel that I should use that class in my enablement rule. The problem is that I did have to use the EMF Resource class in my enablement rule because of the way my example model was being presented. I can easily see other models doing the same thing which will lead to serious problems. As it turns out, in this case it would be possible to provide a general EObject/Resource content provider since EMF provides a general mechanism for handling those anyway. The problem is that, if this wasn't provided at the EMF level, we could easily end up with a case where multiple extensions are trying to provide the same content (or similar content). Perhaps we need to have a notion of primary vs. complimentary providers. In any situation, there can only be one primary provider but any number of comlimentary ones. We would then need a scheme to pick when there are multiple primaries. One mechanisms is discussed in bug 115012. Another possibility is to say that the content provider that provided the content for the level above takes priority over any others. This would handle my case since the resource came from the LibraryModelProvider. <extension point="org.eclipse.ui.navigator.navigatorContent"> <navigatorContent enabledByDefault="true" actionProvider="org.eclipse.team.examples.library.adapt.NavigatorExtensionActio nProvider" contentProvider="org.eclipse.team.examples.library.adapt.NavigatorExtensionCont entProvider" id="org.eclipse.team.examples.library.navigatorExtension" labelProvider="org.eclipse.team.examples.library.adapt.NavigatorExtensionLabelP rovider" name="Library Example"> <enablement> <or> <objectClass name="org.eclipse.team.examples.library.adapt.LibraryModelProvider"> </objectClass> <objectClass name="org.eclipse.example.library.Book"> </objectClass> <objectClass name="org.eclipse.example.library.BookCopy"> </objectClass> <objectClass name="org.eclipse.example.library.BookCategory"> </objectClass> <objectClass name="org.eclipse.example.library.Library"> </objectClass> <objectClass name="org.eclipse.example.library.Writer"> </objectClass> <objectClass name="org.eclipse.example.library.util.Category"> </objectClass> <objectClass name="org.eclipse.emf.ecore.resource.Resource"> </objectClass> </or> </enablement> </navigatorContent> </extension> Sorry for all the comments but I think it is worthwhile mentioning one more thing. For the Team case I am working on, my viewer has a root content provider whose children are one or more ModelProviders. The expectation is that each model provider would have a navigator extension associated with it and would have 100% control of all content shown under their model provider, regardless of how deep the tree is. This is what I would like to do in the Common Navigator but it does not seem possible. In response to Nick (comment 3 above): The <enablement> method currently serves the dual purpose of identifying (1) the trigger points that will invoke the content provider defined by the extension and (2) the label provider that will be used to provide a label for that content. As previously state, I agree that we could bring back some of the functionality that was removed for the WTP contribution to coordinate the label provider with a contribution. I have opened bug 115123 to track the update of the schema doc. I have opened bug 115125 to track the extraction of the label provider as a first class extension. In response to comment 4, the adaptable="true" attribute is absorbed from the ActionExpression semantics. Currently, if the enablement object is IAdaptable, it will try to adapt it to the stated object (e.g. IResource). The mapping from ModelElement --> IResource does not come into play for selectAndReveal(). selectAndReveal() does not use the enablement expressions; it just calls setSelection() with the parameters. The adaptable="true" part of the expression clause for IFolder is necessary for elements like IPackageFragmentRoot (Java source folders) since the Java extension (for the Common Navigator) is designed to only contribute Java content. Although, adaptable=true for IFile/IResource is not necessary. In response to comment 5, "conflicting enablement rules" only applies for the label provider case. When the rules overlap, both extensions are invoked for child content. Keep in mind that the enablement expressions are merely ActionExpressions for the M3 deliverable. There is a requirement to port this over to the more generic and extensible org.eclipse.core.expressions framework for M4. I have opened bug 115130 for tracking this requirement. ... more responses to come. In response to 6-9: It sounds almost like you have a set of content extensions, but you only want one of them to win. How do your ModelProviders decide which one should win? We have a situation in WTP that may approximate your scenario (so please tell me if it does): The J2EE extension is fully EMF based; EMF of course by default uses EObject as the root interface (but this is configurable). In the WTP J2EE models, no extra configuration was used, so they are all rooted in EObject. There were two customizations (one in the framework and one in the J2EE extension) to make integrating EMF omdels easier. The first customization was for any extension that was contributing an EMF model. A special element called "emfEnablement" specified a "package" attribute. Whenever an extension specified the emfEnablement tag, it could identify every object from the EMF model just by using the EMF ePackage string name. Of course this customization had to be removed for the port down to Platform, but I hope I can bring something back like this with a property tester for generic EObject that could tap into the org.eclipse.core.expressions framework (recall this is the direction the enablement tag is heading). The second customization was used in conjunction with another WTP concept called a "Function Group". We had approached the platform group at some point to port this concept down, but nothing ever happened with it. In WTP, there was a contributed "DynamicAdapterFactory" that allowed extensions to override the AdapterFactory mechanism used on the EObject to provide a customized ItemProvider based on the "context/situation" where the object was being presented. The flow: (1) An extension to the Common Navigator taps into the "trigger points" in a basic resource hierarchy (IProject + specific natures). When the Common Navigator Extension is instantiated, it would create a DynamicAdapterFactory to adapt EObjects to an Item Provider (for others reading this discussion, ItemProviders are used to get an image and label for an EObject). (2) The Common Navigator extension contributed the root EMF object (in this case the J2EE deployment descriptor) (3) A seperate extension was tapped into the DynamicAdapterFactory extension mechanism to specify an AdapterFactory for a given EMF package (if memory serves). When the Common Navigator extension receives a call to getChildren(<root EMF model object>), it delegates through to the DynamicAdapterFactory to fetch the correct ItemProvider. Once it has the ItemProvider, it can return the text for the EObject. (4) Behind the scenes, the DynamicAdapterFactory examines all available AdapterFactory extensions defined and picks the "best choice" to return. The "best choice" was determined by the Function Group concept (sort of like a Capability that is tied to the state of a specific project, extensions can be turned on and off automagically just based on the state of the natures or classpath containers attached to the project). This scheme allows an EMF object to be rendered with a basic approach (say for instance in WTP) and then allow other vendors to override and "improve" the default rendering in commercial products. But the jist of this is that it allows your "override once and only once" scenario. The DynamicAdapterFactory mechanism evolved somewhat independent of the Common Navigator and is specifically EMF based. IMHO, I don't think it makes sense to try to replicate the framework in a more generic way. I could see a great argument here to formalize the DynamicAdapterFactory mechanism and push that down into EMF. Thoughts? I think an EMF property tester for enablement rules makes sense. Your discussion about Funtion Group seems to be related to discussion that have occurred about user role or context (i.e. what the user is currently doin affects what menus and views are shown and can configure those views). This is a very complex issue. The problem of varying what is shown to the user based on what they are doing is complex. Function Groups provided an effective yet simple solution to this problem. However, Function Groups are not based on user-activity or capability; instead they are based on the state of a given project. Extensions for editors, navigational viewers, operations, wizard pages, etc, would all enable or disable as a single collective unit based on the state of the project. The solution was built using smaller, easier to manage units (instead of a single overarching solution) which made it easier to implement and maintain. It might be worth considering porting down Function Groups to solve the "case of the overridable content provider" (sounds like a Scooby Do cartoon to me). What about actionHandlers? Certainly, other action handlers may want to "override" one or more of the actions, global or otherwise, provided by resource content provider, but keep the remaining action contributions from the resource action handler. The dropHandlers are okay since the user is prompted to choose the handler in the case of multiple handlers, but this won't work for context menus. I think that this defect has resulted in some good changes to the framework, but to help focus any remaining issues, I am closing the defect and requesting clients to open one or more focused defects if there are still concerns about a particular capability or feature. Thank you. |