Bugzilla – Full Text Bug Listing
| Summary: | [Preferences] Need ability to get at property page details without using internal code | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Brian Fitzpatrick <bfitzpat> |
| Component: | UI | Assignee: | Tod Creasey <Tod_Creasey> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | minor | ||
| Priority: | P3 | CC: | bokowski, lchan, pwebster, remy.suen |
| Version: | 3.3.1 | Keywords: | api, helpwanted |
| Target Milestone: | 3.4 M6 | Flags: | bokowski:
review+
|
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | hasPatch | ||
| Bug Depends on: | |||
| Bug Blocks: | 201125 | ||
| Attachments: | |||
Adding Linda Could you attach a patch with your suggestions - it is easier for me to review then. Created attachment 82282 [details]
Patch for an utility class that currently uses internal property page API.
Attached is the patch for a DTP utility class that encapsulates all its calls to the property page internal API. We are looking for similar public utility methods from the Platform UI.
Could you attach a patch what you want applied to the platform please? Please see the attached patch for the 3 utility methods and implementation that we would like to have in platform. I'm afraid I don't know the Platform UI code well enough to suggest the proper platform ui package and class to apply this to. It would be great if Platform UI can provide a public API utility class with the 3 methods similar to the attached patch, but of course in the org.eclipse.ui namespace. From our perspective, we simply want to be able to use those methods currently in org.eclipse.ui.internal.dialogs, without getting the internal discouraged access warnings. Could you attach the class then please. It is much harder to apply something with all of the + and - lines if I can't use the patch mechanism. Created attachment 84779 [details]
Sample Utility class.
Attached is the class file described in the comments.
Created attachment 86844 [details]
A datatools free version
Would this meet your requirements?
Yes, that looks like it would address the problem nicely. Thanks! Linda, do you agree? The first 2 methods look fine. Thanks. Re: the third method, public static IWorkbenchPropertyPage createPropertyPage(Object element) can we add a second argument to specify the type of specialized property page instance that the method should create? This would then handle the use case where there are specialized propertyPageContributions for the same adapter element. This second argument may be null, which means any type of IWorkbenchPropertyPage. And otherwise return null? You will get whatever type you specified when you specified in the extension point so the most that would add is a type check that you could do yourself. In the case where there are multiple propertyPageContributions for the specified adapter element... if the type check doesn't pass, the iterator while loop would continue to try the next propertyPageContribution, until it creates the first matching type. And of course, if none of the applicable propertyPageContributions creates the specified type, returns null. This functionality is needed within the method, as the caller has no access to getApplicableContributors . The sample implementation in Comment #7 shows the above logic. I can see that use case but I don't think we would want that as API as it is a pretty narrow requirement. A contributorsFor: method that returns a collection for you to introspect is likely better for a framework. (In reply to comment #13) Hi Tod, I'm open to use other API method(s) to create a specialized property page. So what type will the suggested contributorsFor: method return? If they are of IPropertyPageContributor, will it then have new methods to introspect the specialized type of IWorkbenchPropertyPage that it can create? And will it also provide a createPage API method for the caller to create a new property page instance? Linda I guess my question is why you want to check on java type instead of id? It seems like you could get pages you didn't expect if they happen to be a subclass. I guess I need a better description of your use case because I think the solution you are suggesting is only useful for your current implementation and not generally useful. Are you looking for a specific page? Is it so that you can select the page or embed it in some other composite? (In reply to comment #15) (Sorry for the delayed reply; I was out of the office.) In my use case, an adapter element (a DTP IConnectionProfile object in this case) returns 6 contributors, each for a different type of property page. One of the contributors is an adopter of the DTP Connectivity Profile UI framework, which must contribute a subclass of ProfilePropertyPage. My UI framework is only interested in the DTP connectivity ui adoptors, and can embed any type of ProfilePropertyPage. It is thus interested in only this type of contributors. As illustrated in the 2 sample extension manifest below from different database plugins, a different extension gets used depending on the type of IConnectionProfile. So the pageId would vary and is defined by an extension. Thus checking on the Java type (in this case, ProfilePropertyPage) is needed. <extension point="org.eclipse.ui.propertyPages"> <page class="org.eclipse.datatools.connectivity.apache.derby.internal.ui.connection.DerbyEmbeddedDBPropertyPage" id="org.eclipse.datatools.connectivity.db.derby.profileProperties" name="%DERBY_EMBEDDED_DB_PROPERTY_PAGE_NAME"> <filter name="org.eclipse.datatools.profile.property.id" value="org.eclipse.datatools.connectivity.db.derby.embedded.connectionProfile"/> <enabledWhen> <instanceof value="org.eclipse.datatools.connectivity.IConnectionProfile"> </instanceof> </enabledWhen> </page> </extension> <extension point="org.eclipse.ui.propertyPages"> <page class="org.eclipse.datatools.enablement.mysql.internal.ui.connection.MySQLDBProfilePropertyPage" id="org.eclipse.datatools.enablement.mysql.profileProperties" name="%profile.mysql.properties" objectClass="org.eclipse.datatools.connectivity.IConnectionProfile"> <filter name="org.eclipse.datatools.profile.property.id" value="org.eclipse.datatools.enablement.mysql.connectionProfile"/> </page> </extension> I still don't think this pattern is something we want in API Why are you looking these contributors up anyways? Are you not using our dialog? Are you doing something special with these pages? Would something in PreferenceNode be helpful to you? i.e. the classname vairable being exposed? >> Why are you looking these contributors up anyways? Are you not using our dialog? Are you doing something special with these pages? I'm providing an adapter for the ProfilePropertyPage (from the DTP Connectivity ui framework) to adapt to the OpenDataAccess PropertyPage UI API. So it needs to check on the type of page in a contribution, to decide whether it can be adapted. Then wraps it into an ODA property page. The wrapped ODA page is then provided to an ODA PropertyPage host, which will then add the page to its Preference dialog. This way, an ODA host can consume any extended ProfilePropertyPage and ODA property pages, using the same ODA API. >> Would something in PreferenceNode be helpful to you? i.e. the classname vairable being exposed? It depends on when this info is available. I need to first check on the type of page in a contribution to determine whether it can be adapted. And thus before the page gets added to a Preference dialog. All I would be giving you is the name of the class which we know as soon as we load the extension. We would try not to load the class as this would cause unneccessary plug-in activations. Obviously, just the class name is not sufficient, as they would vary. But I suppose I can load the class to check its type.
Can you please elaborate then on the API to get a class name, and how that can work together with creating a property page of a specific type?
public static IWorkbenchPropertyPage createPropertyPage(Object element)
If we gave you code instead to get a list of classnames (which I can get from the RegistryPageContributions) then you could load the class and check (or even do something that does not require class loading). Anything we do that requires us to load classes that are not neccessary in the end could be a big performance hit. Ok. Just to clarify what the proposed API methods would be... so after determining which classname is the appropriate one from the list of classnames returned by an API method, which API to use then to create an IWorkbenchPropertyPage for that classname and adaptable element? Created attachment 89183 [details]
Illustratrive class
So here is what you would be after then.
Truth be told this is a really wild usecase. Is there really no other way that you can do this? Looking up pages that you know nothing of, wrapping them and then opening them in another dialog seems pretty complex to me.
Will IPropertyPageContributor be available in an API package, instead of an internal package in 3.3? If so, this would work fine.
Thanks much for working this out with me.
RE: the use case, I'm afraid that it is necessary given the fact that there are 2 frameworks involved, each with its own set of API due to their differences in nature. And a 3rd consumer component that prefers to consume all their pages with a single set of API.
>> Looking up pages (adopters of DTP Connectivity UI framework) that you (DTP ODA UI framework) know nothing of, wrapping them (as ODA pages) and
then (e.g. BIRT) opening them in a (BIRT) dialog.
Created attachment 89249 [details]
Version without IPropertyPageContributor
We won't be making any more API for this case then we need to - it already is something I am not sure us a good idea so IPropertyPageContributor won't be added. Here is a version that uses Object.
So let me get your use case straight.
Plu
1) You have a custom properties dialog.
2) You want all of the property pages that adapt to IConnectionProfile .
3) Your dialog needs subclasses of ProfilePropertyPage
4) These ProfilePropertyPages need to adapt to another type OpenDataAccess PropertyPage
Have I got it straight? I think I don't because you can do this now with the current API and some instanceof checks.
Adding Boris as this API needs a second look Like Tod, I don't understand your original use case. Sorry, but could you try explaining it at the level that Tod used in comment 25? Would it make sense to talk about this on the phone, or using IM, rather than Bugzilla? Since further design is needed to address the third use case (i.e. creating a specific type of property page),
in the meantime, can you please go ahead and commit the utility methods for the first 2 use cases? I believe these 2 use cases have no issues all along; they are:
* public static boolean hasContributors(Object element);
* public static PreferenceDialog createPreferenceDialog(Shell parentShell,
ISelection selection, Object element);
Created attachment 89658 [details]
Updated version of PreferencesUtil to cover your use case
Boris and I both prefer that we go for a solution that re-uses existing classes. org.eclipse.ui.dialogs.PreferencesUtil is the obvious choice.
I think for opening the dialog createPropertyDialogOn(Shell shell,
final IAdaptable element, String propertyPageId, String[] displayedIds, Object data) already covers what you need.
So there are two more issues:
1) Doing the check for contributors
2) This introspection you need to do to get property pages you can wrap to your other type of page.
This code here will return an array of IPreferenceNode which will allow you to create the page if you wish to introspect it.
Tod, We've tested with your latest patch 89658. And it works just fine. Looking forward to use it in a M6 integration build. Thanks again for all your help and patience. Linda Thanks for your help in getting us to understand and your patience with us trying to get it right. +1 - the proposed solution adds minimal API and fits in with our existing structure. Just to be clear, it still does not solve the problem of potentially causing too many plug-in activations. I would recommend that you keep an eye for unwanted plug-in activations and file a new bug if this turns out to be a problem. Tod - the new method propertiesContributorsFor() still needs Javadoc (and an @since tag...) While you are at it, could you also correct the Javadoc for createPropertyDialogOn()? It should probably say "a particular property page" instead of "a particular preference page". Last version released for build >20080219 Verified in I20080323-2000 |
Hi... In DTP and ODA, we need the ability to get at property page details without using internal API. We're trying to clean up a variety of access warnings and this is the last major one to clean up. The list of classes we'd like access to either through a public utility or by making certain internal classes public includes: * PropertyPageContributorManager - we want to use this to determine if a given object has property pages (in our case it's typically connection profiles, but it could be anything visible in the DSE) public static boolean hasContributors(Object selected) { if (selected == null || !(selected instanceof IAdaptable)) return false; Collection contributors = PropertyPageContributorManager. getManager().getApplicableContributors(selected); return contributors != null && contributors.size() > 0; } * PropertyPageManager and PropertyDialog - these we use with the PropertyPageContributorManager to pop up a dialog with the applicable property pages. We need a handle to the dialog so we can manage it's size and position using mementos. Object selected = getSelectedObject(); if (selected == null || !(selected instanceof IAdaptable)) return; IAdaptable adaptable = (IAdaptable) selected; PropertyPageManager pageManager = new PropertyPageManager(); String title = ConnectivityUIPlugin.getDefault().getResourceString( "properties.dialog"); //$NON-NLS-1$ // load pages for the selection // fill the manager with contributions from the matching contributors PropertyPageContributorManager.getManager().contribute(pageManager, adaptable); PropertyDialog propertyDialog = new PropertyDialog(mViewer.getControl() .getShell(), pageManager, mViewer.getSelection()); propertyDialog.create();