Community
Participate
Working Groups
JDT now offers a "Add project xxx to build path of yyy" when an import cannot be resolved. (Nice improvement). However if yyy is a PDE project, the plug-in dependency, rather than the build-path must be updated.
Move to JDT/UI
JDT does not know of PDE. Either the user has to know what he does or PDE has to track classpath changes and ask to update the plugin.xml when a new project dependency has been added. I think the first option is good enough. Another option is that PDE adds its own quick fix, similar to the 'add project dependency', suggesting instead to add a new plugin dependency. It would show along with the other suggestion (you can give it a higher relevance), so the smart user will realize the difference.
Thoughts on this one guys? As far as I know, there's no easy way to piggyback on JDT's proposal without duplicating a ton of code. The reasonable option seems listen to changes on the classpath and offer a quickfix, what do you think?
The Java similarities between PDE and JDT are perhaps obscuring the real issue. It could arise in CDT or even in more generic model-only contexts. The underlying quick fix is mend project xxx with the aid of project yyy. This could be resolved generically by extending the Nature API, with the ton of sharable JDT code placed where PDE can share it.
Thre's not really much code duplication is this case. Implementing your own quick fix proposal (IJavaCompletionProposal) that manipulates the Java project classpath in 'apply()' is trivial. A problem I see is that our quick fix performs a workspace search to find a project that contains the missing type. This shouldn't be duplicated; it would slow down the quick fix too much. But maybe PDE has a better way of doing that anyway: look if some plugin exports the package of the missing type. That way you would even find plugins that are not in the workspace. So if you have a fast way of looking up exported packages, I'd recommend adding your own quick fix. Maybe we can then think of a mechanism to prevent the original, PDE unspecific quick fix.
For 3.4 I plan to add a new extension point to solve this issue. See bug 196141.
Martin rocks!
*** Bug 101611 has been marked as a duplicate of this bug. ***
Moving off PDE. Back to inbox. Somebody else will have to give the JDT extension some love in my stead.
marking bugday, if noone bites I'll do this myself as this is very useful.
Cool, go get 'em Curtis. Once you finish this, I'll drink a bottle of Fort Garry Dark Ale in your honor. PDE is going to be so quickfix-tastic for 3.4M2
Created attachment 78387 [details] Work in progress Patch uses the extension point to add a quick fix processor for unresolved imports. If there is a plugin that exports the needed package a proposal is offered that adds it as a required bundle to the manifest file. Things that still have to be done: - Provide separate proposal for package import (vs required bundle) - Improve names/labels, NLS strings, including in the plugin.xml - Use enablement expressions and overrides in the extension definition - Various TODOs are left in the code, they should be finished or removed Things that could be done: - Make it work for 3.0 projects with no manifest - Validate the class name in the import, not just the package name - Improve UI when multiple bundles provide a valid fix
Curtis, this is looking good. With the current progress, this should definitely make it in for M2. A couple things. 1. Yes, want to allow others who override our FixProcessor to run (was one of the TODO comments) 2. If you want, you can refactor some of the subclasses into individual classes. 3. Forget what I said earlier about adding an import. The following code should work for adding a plug-in dependency no matter what version the plug-in is (including 3.0): IPluginImport pluginImport = <IPluginModelBase>.getPluginFactory().createImport(); pluginImport.setId(<id>); baseBundle.getPluginBase().add(pluginImport);
Created attachment 78426 [details] updated patch Hi Curtis, thanks a lot for the work. I played around with it to test and demo the new extension point. Here's an updated patch: - using enablement and overrides tags - offering an Undo change - improving some labels and added an image
(In reply to comment #13) I wasn't talking about others who override our processor, I was asking whether we should allow processors that we have overridden ourselves a second chance. For example, if we override the JDT processor, but do not find any exported packages that work, should be then allow the JDT processor to run? The getFixImportProposals method originally returned null if none were created (allowing overridden processors to get a chance to try). Martin changed it to return an empty array (preventing any other processors from running). I assume that this is what we want, but I figured I would ask and make sure. Thank you Martin for updating the patch, makes things look a lot better with the image and such. Also thanks for fixing bug 203322, kept entering xxx.* and wondering why my processor wasn't working :) I'm working on refactoring the classes, improving code/doc, adding package import option and a few other things.
Created attachment 78470 [details] Patch for review Finished major outstanding issues. Changes: - Supports adding a required-bundle or import-package entry (two separate proposals) - Refactored into two separate files using an abstract class to reduce code duplication - Updates labels, NLS'd strings - Proposal is not offered if an identical entry already exists - I'm sure there's more Issues: - The undo changes Martin added were not available to me, not sure if I broke them or what. - The labels might need to be changed because a decently long bundle name makes it difficult to tell the difference between the require-bundle and import-package proposals. - I NLS'd the strings, but I did it in way we do it in debug, not sure if it is the same. - If you add a require-bundle, there can still be a proposal to add the same thing as a import-package (not sure if this is a problem or not).
Can you put this in M2? (I'd like to show parts of it in the new and noteworthy)
Brian and I will review, we will try to target for M2. This indeed is a beautiful enhancement.
Martin, thanks to Curtis making such good progress on this bug, I don't see any problems getting this into M2 :)
Looking good. Looks like you got the hard stuff done :) I think a few very minor modifications will make it really cool. 1. Lets change the icon from the JDT library icon to the PDE Manifest icon (PDEPluginImages.DESC_PLUGIN_MF_OBJ). Note, we should use PDE's label provider to cache the icon. See org.eclipse.pde.internal.ui.editors.plugin.OverviewPage to see how it is done there. 2. We should not have multiple import-package resolutions if there are multiple plug-ins exporting the package. Import-Package is independent of the supplier, so we should only have it once no matter how many suppliers there are. On this same topic, I went back to review the OSGi specification and ask our runtime guru some questions. If a user uses Import-Package, when a package is loaded it will always resolve through that mechanism, even if you have multiple Require-Bundles who export the package. Therefore, if the user already has an Import-Package statement for a specific package, we can exit and contribute no quickfixes. If the package is not in the Import-Package statement, we should look at what packages the bundle already has access to. We can do this with the following code: IPluginModelBase base = PluginRegistry.findModel(project.getProject()); if (base != null) { BundleDescription desc = base.getBundleDescription(); if (desc != null) { StateHelper helper = Platform.getPlatformAdmin().getStateHelper(); ExportPackageDescription[] visiblePkgs = helper.getVisiblePackages(desc); When you iterate through the validPackagesIter, we should check to see if the ExportPackageDescription is included in the "already visible packages". If it is, then we can continue to the next one. If it is not, then we can create a Require-Bundle completion for each validPackage. After doing this, we know the package is not already in an Import-Package statement (otherwise we would have exited already), so if the proposals.size() > 0, we should also include one import package proposal. Curtis, let me know if this does not make sense. I can help clarify these things.
Curtis, thanks for the help in getting this into M2. Without you stepping up and finishing this bug, it never would have made it. Excellent job.
Verified in I20071211-0010.
