Bug 395795 - [patch] Template with Incremental Project Builder generates IObjectActionDelegate instead of handler
Summary: [patch] Template with Incremental Project Builder generates IObjectActionDele...
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.3   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.3 M5   Edit
Assignee: Curtis Windatt CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2012-12-05 03:33 EST by maarten meijer CLA
Modified: 2013-01-08 17:36 EST (History)
1 user (show)

See Also:


Attachments
Areas of change in wizard (81.04 KB, image/png)
2013-01-03 06:52 EST, maarten meijer CLA
no flags Details
Areas of change in wizard page (56.00 KB, image/png)
2013-01-03 06:52 EST, maarten meijer CLA
no flags Details
Areas of change in plugin.xml (85.73 KB, image/png)
2013-01-03 06:53 EST, maarten meijer CLA
no flags Details
Areas of change in generated code (32.08 KB, image/png)
2013-01-03 06:53 EST, maarten meijer CLA
no flags Details
mylyn/context/zip (11.64 KB, application/octet-stream)
2013-01-03 06:56 EST, maarten meijer CLA
no flags Details
mylyn/context/zip (260.82 KB, application/octet-stream)
2013-01-07 12:17 EST, maarten meijer CLA
no flags Details
Removed IObjectActionDelegate and added Handler and o.e.u.menus for plugin.xml (38.85 KB, patch)
2013-01-07 12:23 EST, maarten meijer CLA
curtis.windatt.public: iplog+
Details | Diff
mylyn/context/zip (260.82 KB, application/octet-stream)
2013-01-07 12:23 EST, maarten meijer CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description maarten meijer CLA 2012-12-05 03:33:07 EST
The template incremental project builder generates a code skeleton that contains a Sample Popup Menu Action ToggleNatureAction that implements IObjectActionDelegate.

The preferred way since Eclipse 3.7 is to use a handler and commands.
It should be easy to link the toggle handler code similar to the current action to two Menu commands under the "Configure" item using the popup:org.eclipse.ui.projectConfigure?after=additions URL.

One of the commands is "Enable Sample Builder" , the other is "Disable Sample Builder" as its easy to use the org.eclipse.core.resources.projectNature property in a visibleWhen expression.

This will be a tremendous productivity gain for all people that have to make builders. Using org.eclipse.ui.projectConfigure as location will declutter the popup menu.
Comment 1 maarten meijer CLA 2012-12-05 03:33:45 EST
Willing to make contribution for this.
Comment 2 Curtis Windatt CLA 2012-12-05 10:23:52 EST
It would be great to have any of the templates updated to use commands.  We will probably have to update the Eclipse versions the template is valid for.
Comment 3 maarten meijer CLA 2013-01-03 06:28:51 EST
required git repos are:
git://git.eclipse.org/gitroot/pde/eclipse.pde.ui.git
git://git.eclipse.org/gitroot/platform/eclipse.platform.releng.git
plus a clean 4.3.x SDK as platform target
Comment 4 maarten meijer CLA 2013-01-03 06:52:30 EST
Created attachment 225167 [details]
Areas of change in wizard
Comment 5 maarten meijer CLA 2013-01-03 06:52:52 EST
Created attachment 225168 [details]
Areas of change in wizard page
Comment 6 maarten meijer CLA 2013-01-03 06:53:16 EST
Created attachment 225169 [details]
Areas of change in plugin.xml
Comment 7 maarten meijer CLA 2013-01-03 06:53:44 EST
Created attachment 225170 [details]
Areas of change in generated code
Comment 8 maarten meijer CLA 2013-01-03 06:56:27 EST
I would like some guidance on how to approach this bug

I think the work can be done mostly in project *org.eclipse.pde.ui.templates*

I need to create two new classes in *org.eclipse.pde.internal.ui.templates.ide*
BuilderCommandNewWizard.java
BuilderCommandtemplate.java

and then some new templates in a new folder templates_3.6 or existing folder templates_3.5 (what should be earliest version to use?)
builder/java/$builderClassName$.java
builder/java/$natureClassName$.java
builder/java/AddRemoveNatureHandler.java

Then the code would create extensions as follows:

   <extension
         point="org.eclipse.ui.commands">
      <command
            categoryId="sample.category"
            defaultHandler="sample.handler.AddRemoveIesNatureHandler"
            description="Add/Remove Sample Nature enables the validator"
            id="sample.command.addRemoveNature"
            name="Enable/Disable Sample Builder">
      </command>
   </extension>

     <extension
         point="org.eclipse.ui.menus">
      <menuContribution
            locationURI="popup:org.eclipse.ui.projectConfigure?after=additions">
         <command
               commandId="sample.command.addRemoveNature"
               icon="platform:/plugin/sample.plugin/icons/ies.gif"
               label="Disable Sample Builder"
               style="push">
            <visibleWhen
                  checkEnabled="false">
               <with
                     variable="selection">
                  <count
                        value="1">
                  </count>
                  <iterate>
                     <and>
                        <instanceof
                              value="org.eclipse.core.resources.IProject">
                        </instanceof>
                        <test
                              property="org.eclipse.core.resources.projectNature"
                              value="sample.builderNature">
                        </test>
                     </and>
                  </iterate>
               </with>
            </visibleWhen>
         </command>
         <command
               commandId="sample.command.addRemoveNature"
               icon="platform:/plugin/sample.plugin/icons/ies.gif"
               label="Enable Sample Builder"
               style="push">
            <visibleWhen
                  checkEnabled="false">
               <with
                     variable="selection">
                  <count
                        value="1">
                  </count>
                  <iterate>
                     <and>
                        <instanceof
                              value="org.eclipse.core.resources.IProject">
                        </instanceof>
                        <not>
                           <test
                                 property="org.eclipse.core.resources.projectNature"
                                 value="sample.builderNatureNature">
                           </test>
                        </not>
                     </and>
                  </iterate>
               </with>
            </visibleWhen>
         </command>
      </menuContribution>
   </extension>
   
   Also some documentation cleanup would be needed (see images)
Comment 9 maarten meijer CLA 2013-01-03 06:56:30 EST
Created attachment 225171 [details]
mylyn/context/zip
Comment 10 Curtis Windatt CLA 2013-01-03 14:59:15 EST
(In reply to comment #8)
> I would like some guidance on how to approach this bug
> 
> I think the work can be done mostly in project *org.eclipse.pde.ui.templates*
> 
> I need to create two new classes in
> *org.eclipse.pde.internal.ui.templates.ide*
> BuilderCommandNewWizard.java
> BuilderCommandtemplate.java
> 
> and then some new templates in a new folder templates_3.6 or existing folder
> templates_3.5 (what should be earliest version to use?)
> builder/java/$builderClassName$.java
> builder/java/$natureClassName$.java
> builder/java/AddRemoveNatureHandler.java
> 

I think you are on the right track.  You will need to modify the template at org.eclipse.pde.internal.ui.templates.ide.BuilderTemplate.  This will allow you to create the additional content in the created project.

It is unecessary to move the content from templates_3.0 unless you are using API that is not available until later versions.  The commands you plan to use I assume require 3.1 APIs from org.eclipse.core.commands.  However, PDE UI does not support anything less than Eclipse 3.1 when creating a new project (see the option on the first page of the new plug-in wizard).
Comment 11 maarten meijer CLA 2013-01-07 07:26:02 EST
(In reply to comment #10)
> (In reply to comment #8)
> > I would like some guidance on how to approach this bug
> >
> > I think the work can be done mostly in project *org.eclipse.pde.ui.templates*
> >
> > I need to create two new classes in
> > *org.eclipse.pde.internal.ui.templates.ide*
> > BuilderCommandNewWizard.java
> > BuilderCommandtemplate.java
> >
> > and then some new templates in a new folder templates_3.6 or existing folder
> > templates_3.5 (what should be earliest version to use?)
> > builder/java/$builderClassName$.java
> > builder/java/$natureClassName$.java
> > builder/java/AddRemoveNatureHandler.java
> >
> 
> I think you are on the right track.  You will need to modify the template at
> org.eclipse.pde.internal.ui.templates.ide.BuilderTemplate.  This will allow you
> to create the additional content in the created project.
> 
> It is unecessary to move the content from templates_3.0 unless you are using API
> that is not available until later versions.  The commands you plan to use I
> assume require 3.1 APIs from org.eclipse.core.commands.  However, PDE UI does
> not support anything less than Eclipse 3.1 when creating a new project (see the
> option on the first page of the new plug-in wizard).
So a better option UI wise would be not to have a separate template, but to modify the existing template UI so that you can choose between having an a popup menu action and a popup menu command and handler with a radio button.
Then the rest would stay the same.
Comment 12 Curtis Windatt CLA 2013-01-07 09:57:13 EST
(In reply to comment #11)
> So a better option UI wise would be not to have a separate template, but to
> modify the existing template UI so that you can choose between having an a
> popup menu action and a popup menu command and handler with a radio button.
> Then the rest would stay the same.

We don't need to retain the old action code.  The Command API has been available since 3.1.  Just modify the existing template to create a command and handler instead of contributing an action.
Comment 13 maarten meijer CLA 2013-01-07 11:30:49 EST
OK please assign to me so I can contribute a patch
Comment 14 Curtis Windatt CLA 2013-01-07 11:35:34 EST
(In reply to comment #13)
> OK please assign to me so I can contribute a patch

Because you are the reporter you can edit the assignee and attach files.  I changed the assignee for you.
Comment 15 maarten meijer CLA 2013-01-07 12:17:59 EST
Created attachment 225294 [details]
mylyn/context/zip
Comment 16 maarten meijer CLA 2013-01-07 12:23:03 EST
Created attachment 225295 [details]
Removed IObjectActionDelegate and added Handler and o.e.u.menus for plugin.xml

All strings externalized where needed
Comment 17 maarten meijer CLA 2013-01-07 12:23:07 EST
Created attachment 225296 [details]
mylyn/context/zip
Comment 18 Curtis Windatt CLA 2013-01-07 12:26:29 EST
I will try to review for M5, but with 4.2.2 being released at the same time it might have to wait until M6.
Comment 19 Curtis Windatt CLA 2013-01-08 17:36:25 EST
Fixed in master:
http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=3bd94f514dc85a3744abc69f06580f820dbc3e7b

The patch included a fair number of unecessary changes (modifying the template plug-ins natures, new wizards, etc).  In the future if you could avoid including them in the patch it would be appreciated.

Thanks for improving this, the templates don't get much attention.