Bug 133692

Comment 1 Alex Blewitt 2006-03-28 17:40:35 EST

Created attachment 37155 [details]
Adds OpenManifest to a plugin project

Adds an Open Manifest to a plugin project. This was created from HEAD so should be relatively up-to-date. I've tested it against 3.2M5 to check the functionality, but had problems with running from HEAD, so I'm not sure if I've merged the changes across well or whether there's a problem with the current version. However, I'm not going to get any more done tonight so thought it would be useful showing the first draft of the code in case anyone's got time to review it and make comments. How does one go about writing a test for this?

Comment 2 Wassim Melhem 2006-03-28 21:13:51 EST

Thanks Alex.  It's a useful function

I took a quick look.  In addition to the TODOs you had,
1. it does not seem to recognize fragment.xml
2. the package org.eclipse.pde.internal.ui.editor would make a better home for the action
3. the patch did not apply cleanly due to changes in PDEUIMessage that must have occurred in the meantime.

We would like to stabilize the translatable strings by M6, so that the translators can get to work.  It would therefore be good if you revise the patch before noon EST.  Otherwise, I'm not sure it will go in for 3.2

Comment 3 Alex Blewitt 2006-03-29 02:46:01 EST

No, at present, it doesn't recognize fragment.xml, just plugin.xml and MANIFEST.MF. Shouldn't be too hard to put in a check for those though.

The other package sounds like a reasonable place for it.

I created the changes to PDEUIMessages last night, so I'm wondering what would have happened in the meantime? There were only 3 new entries anyway:

+       public static String OpenManifestsAction_cannotFind;
+
+       public static String OpenManifestsAction_cannotOpen;
+
+       public static String OpenManifestsAction_title;

Unfortunately, I don't have the ability to do work on this during office hours, so the earliest I could hit the patch again is around 20:00 GMT+1, which is after 12:00 EST, unless I can get it done before I go into work. Would it be possible to apply the changes and then fix the fragment.xml loading code later?

Comment 4 Alex Blewitt 2006-03-29 03:08:58 EST

Created attachment 37180 [details]
OpenManifest patch with new action location

I've moved the action to the new package and updated the plugin.xml accordingly. I'm not sure why the plugin.xml has so many changes; I updated it to HEAD and so there may have been some formatting done (they look mostly like whitespace changes). Not implemented fragment.xml yet; will do it this evening.

Comment 5 Wassim Melhem 2006-03-29 03:40:57 EST

patch applied with minor changes.  thanks.

Comment 6 Alex Blewitt 2006-03-29 16:05:11 EST

Cool. I actually extended the implementation to check for feature.xml, plugin.xml and fragment.xml if you wanted that :-) 

I also discovered that I had to put two extensions in the plugin.xml; one for the filter type nature=org.eclipse.pde.PluginNature and one for nature=org.eclipse.pde.FeatureNature (otherwise the feature.xml didn't work). I'd have liked to have one point for both, but it didn't seem to have the ability for an 'or' filter. (The other approach would have been to change the WorkbenchProject IActionFilter to understand a comma-separated list of project natures instead of just one; but that would have required more changes.)

      <objectContribution
            objectClass="org.eclipse.core.resources.IProject"
            adaptable="true"
            id="org.eclipse.pde.ui.pluginProjectToolSet">
         <menu
               label="%org.eclipse.pde.ui.tools"
               id="org.eclipse.pde.ui.project.tools">
            <separator
                  name="group0">
            </separator>
         </menu>
         <filter
               name="projectNature"
		       value="org.eclipse.pde.FeatureNature">
         </filter>
         <action
               label="%OpenManifest.label"
               class="org.eclipse.pde.internal.ui.editor.OpenManifestAction"
               menubarPath="org.eclipse.pde.ui.project.tools/group0"
               enablesFor="+"
               id="org.eclipse.pde.ui.OpenManifest"> 
         </action> 
      </objectContribution>

The downside of this is if you have a selection with both features and plugins, it doesn't show the action since it's not common to all selected types.

I also changed OpenManifestAction to have fragment/features:

  || WorkspaceModelManager.hasFeatureManifest(project)
  || WorkspaceModelManager.hasFragmentManifest(project)))
and I thought this was neater on reflection:
  if (WorkspaceModelManager.hasBundleManifest(project))
    file = project.getFile("META-INF/MANIFEST.MF");
  else if (WorkspaceModelManager.hasPluginManifest(project))
    file = project.getFile("plugin.xml");
  else if (WorkspaceModelManager.hasFeatureManifest(project))
    file = project.getFile("feature.xml");
  else if (WorkspaceModelManager.hasFragmentManifest(project))
    file = project.getFile("fragment.xml");

I also caught the error when it couldn't find any manifests after my earlier patch :-)

BTW the resources are a bit odd; there's OpenManifestsAction_... and OpenManifestAction_... -- might be good to get rid of my plural name and put them all together in the same place.

Thanks for getting it in so quickly!

Alex.