Bug 182488 - Custom Actions for J2SE Project Node
Custom Actions for J2SE Project Node
Status: RESOLVED FIXED
Product: java
Classification: Unclassified
Component: Project
6.x
All All
: P3 (vote)
: 6.x
Assigned To: Jesse Glick
issues@java
http://hg.netbeans.org/core-main/rev/...
: API, API_REVIEW_FAST
: 108608 (view as bug list)
Depends on: 57874 167446 183564 191771
Blocks: 182455 207227
  Show dependency treegraph
 
Reported: 2010-03-22 13:14 UTC by Martin Schovanek
Modified: 2012-01-12 16:45 UTC (History)
8 users (show)

See Also:
Issue Type: TASK
:


Attachments
proposed patch (2.29 KB, patch)
2010-03-22 13:14 UTC, Martin Schovanek
Details | Diff
Running patch (61.43 KB, patch)
2010-03-26 03:07 UTC, Jesse Glick
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Schovanek 2010-03-22 13:14:52 UTC
Created attachment 95529 [details]
proposed patch

* Use Case:
Independent module wants to add an action into projects node popup menu.

* Proposed Solution:
Introduce well-known SFS folder: Projects/org-netbeans-modules-java-j2seproject/Actions.
The folder contains instance data objects representing SystemActions and JSeparator instances.

The same solution is used at org.netbeans.modules.apisupport.project.ui.SuiteActions for
BuildInstallersAction.

* Javadoc (see the patch):
http://bits.netbeans.org/dev/javadoc/org-netbeans-modules-projectuiapi/overview-summary.html
- Interface: ProjectTypeActions, Exported, Devel
...
Comment 1 Tomas Zezula 2010-03-22 17:38:19 UTC
Seems OK.
Only the category="devel" may be "official" as this is useful API contract even for others.
Comment 2 David Konecny 2010-03-22 22:11:25 UTC
For sake of consistency between project types (I'm thinking about J2SE and Java EE) I would prefer a (friend) API method somewhere:

public static List<Action> getPlugableActions(Project p) {
    List<Action> actions = Utilities.actionsForPath("Projects/"+p.getSomething()+"/Actions");
    if (!actions.isEmpty()) {
        actions.add(null);
    }
    return actions;
}

which all project types just call. Perhaps the method could be directly in CommonProjectActions but I do not know offhand how to implement p.getSomething() - ideally it would be value of AntBasedProjectType.getType() right?
Comment 3 Petr Jiricka 2010-03-23 14:18:03 UTC
> * Use Case:
> Independent module wants to add an action into projects node popup menu.

What are some concrete examples of plugins that would want to do that? 
In my experience, whatever is applicable to JavaSE, is very likely applicable to all Ant-based projects. So I agree with David that we want some more general API.

Though, if p.getSomething() is the value of AntBasedProjectType.getType() (which is Ant-specific), wouldn't the method need to be somewhere in the project.ant module, e.g. AntProjectHelper?
Comment 4 Martin Schovanek 2010-03-23 15:24:50 UTC
> For sake of consistency between project types (I'm thinking about J2SE and Java
> EE) I would prefer a (friend) API method somewhere:

Can we solve this in a separate issue? I would like to keep this change as simple as possible. Next, lookup the AntBasedProjectType, getting the type from project.xml and replacing dots by hyphen seems to me a little complicated.
Comment 5 David Konecny 2010-03-23 19:26:33 UTC
Re. "Can we solve this in a separate issue? I would like to keep this change as
simple as possible." - that's up to you. My experience with "separate issue" approach is that separate issue often never happens and later somebody files P2 defect for Java EE project type that an action cannot be plugged into popup menu and somebody(likely me) copy&pastes the code from J2SE and that's it. If you want to keep it simple then add CommonProjectActions.getPlugableActions(String antBasedProjectTypeLikeValue) for now. Just my two cents.
Comment 6 Petr Jiricka 2010-03-23 21:21:37 UTC
> CommonProjectActions.getPlugableActions

Typo, should be "getPluggableActions"
Comment 7 Jesse Glick 2010-03-23 23:10:53 UTC
I don't think the proposed patch is the right approach. We already have a Projects/Actions/ folder where arbitrary modules can add actions to projects, near the end. We also have Projects/Debugger_Actions_temporary/ and Projects/Profiler_Actions_temporary/ which have not been so temporary. apisupport.installer gets its own temporary folder, and autoproject defines another. Projects/o-n-m-maven/DependenciesActions/ seems to be another example. All of these lack the ability to insert actions at arbitrary places in the context menu, such as between Build and Clean (as has been requested recently), or to make branding customizations.

What is rather needed is for the _entire_ context menu of the root node to be defined declaratively (as mobility.project seems to do). Then getActions(boolean) would simply return Utilities.actionsForPath(...). This would be consistent with the main menus, the Projects tab background, and so on.

For compatibility, the widely used Projects/Actions/ would still need to be interpreted. This could perhaps be accomplished by creating Projects/$type/Actions/general.shadow -> Projects/Actions (assuming the relevant NamedServicesProvider impls recursively traverse shadows, which I think would be the case for RecognizeInstanceObjects since it uses FolderLookup). Actions which were actually intended to be registered to a particular project type (about half of the ones I see there now in nbms-and-javadoc layers.txt) would just be moved to a different folder, which in some cases could simplify them by removing the need to implement conditional visibility.

(More in keeping with the style of the MIME Lookup API and so on would be to _merge_ Projects/Actions/ with Projects/$type/Actions/ using MultiFileSystem, so that a registration could be either general or specific, and actions from both folders could be interleaved using the position attribute. It might however be difficult to decide where to put anything in Projects/Actions/ in this case, unless all the Projects/$type/Actions/ more or less standardized where common actions went. Deserves consideration.)

I am willing to try to do this for j2seproject if you are not sure how to best implement; assign to me in that case.
Comment 8 Jaroslav Tulach 2010-03-24 12:42:31 UTC
I guess Jesse's great plan sounds reasonable. The patch by Martin Schovánek is OK, but fixing this consistently is tempting. Registering all actions in layer is indeed move in good direction.

Y01 Don't play any tricks with MultiFS. Rather use the fact that FolderLookup understands .shadow (http://hg.netbeans.org/core-main/rev/81007b988a2a) and declare .shadow to Projects/Actions at the place where you want to include these shared actions.
Comment 9 Jesse Glick 2010-03-24 14:11:02 UTC
Y01 - again, this is certainly easier to implement, but it is also less powerful: it means that if you wish to register an action in Projects/Actions/ for all project types, you are constrained to pick a position only among those actions registered for all project types, which is conventionally near the bottom (between Delete and Properties). You could not, for example, register an action to appear just below New‣ for all project types - what I meant by "interleaved" in my last comment.

Note that if there were a common utility method to construct the context menu for a project root node, probably in projectuiapi, it could be initially implemented to simply call Utilities.actionsForPath("Projects/" + token + "/Actions"), where Projects/*/Actions/general.shadow -> Projects/Actions. Later we could decide to introduce Projects/any/Actions/ and make the utility method do a MFS merge, without loss of compatibility. (Projects/Actions/ would continue to use the private ordering it does now.)

Bug #167446 has a fuller discussion of the merging approach; I will make that into a more general issue (it began essentially identical to this issue).
Comment 10 Martin Schovanek 2010-03-24 17:23:53 UTC
Reassigning to Jesse, not sure how to implement correctly.
Comment 11 Jesse Glick 2010-03-24 19:47:01 UTC
For reference, the classes in the main repo which seem to load actions from folders under Projects/ are as follows:

ant.freeform/src/org/netbeans/modules/ant/freeform/Actions.java
apisupport.project/src/org/netbeans/modules/apisupport/project/ui/ModuleActions.java
apisupport.project/src/org/netbeans/modules/apisupport/project/ui/SuiteActions.java
cnd.classview/src/org/netbeans/modules/cnd/classview/model/ProjectNode.java
cnd.makeproject/src/org/netbeans/modules/cnd/makeproject/ui/MakeLogicalViewProvider.java
groovy.grailsproject/src/org/netbeans/modules/groovy/grailsproject/ui/GrailsLogicalViewProvider.java
j2ee.archive/src/org/netbeans/modules/j2ee/archive/ui/RootNode.java
j2ee.clientproject/src/org/netbeans/modules/j2ee/clientproject/ui/AppClientLogicalViewProvider.java
j2ee.earproject/src/org/netbeans/modules/j2ee/earproject/ui/J2eeArchiveLogicalViewProvider.java
j2ee.ejbjarproject/src/org/netbeans/modules/j2ee/ejbjarproject/ui/EjbJarLogicalViewProvider.java
javacard.project/src/org/netbeans/modules/javacard/project/JCLogicalViewProvider.java
java.j2seproject/src/org/netbeans/modules/java/j2seproject/ui/J2SELogicalViewProvider.java
maven/src/org/netbeans/modules/maven/nodes/DependenciesNode.java
maven/src/org/netbeans/modules/maven/nodes/MavenProjectNode.java
mobility.project/src/org/netbeans/modules/mobility/project/ui/actions/Actions.java
o.n.bluej/src/org/netbeans/bluej/nodes/BluejLogicalViewRootNode.java
php.project/src/org/netbeans/modules/php/project/ui/logicalview/PhpLogicalViewProvider.java
python.project/src/org/netbeans/modules/python/project/PythonLogicalView.java
ruby.merbproject/src/org/netbeans/modules/ruby/merbproject/ui/MerbLogicalViewProvider.java
ruby.project/src/org/netbeans/modules/ruby/rubyproject/ui/RubyLogicalViewProvider.java
ruby.railsprojects/src/org/netbeans/modules/ruby/railsprojects/ui/RailsLogicalViewProvider.java
uml.project/src/org/netbeans/modules/uml/project/ui/nodes/UMLPhysicalViewProvider.java
web.project/src/org/netbeans/modules/web/project/ui/WebLogicalViewProvider.java
Comment 12 Jesse Glick 2010-03-24 22:15:45 UTC
Working on it in a patch branch.
Comment 13 Jesse Glick 2010-03-26 03:07:50 UTC
Created attachment 95873 [details]
Running patch

Includes new APIs:

- CommonProjectActions.forType

- DynamicMenuContent.HIDE_WHEN_DISABLED

and declarative registrations of context menus for java.j2seproject and ant.freeform.
Comment 14 Jesse Glick 2010-03-26 03:12:56 UTC
Please review APIs. Time permitting, I will try to implement in maven and apisupport.project as well.
Comment 15 Jaroslav Tulach 2010-03-26 08:48:12 UTC
Meta comment: You can do 
$ hg update -C default
$ hg merge your_branch
$ hg branch your_branch
$ hg ci -m "Merge with default"
$ hg push
and then the http://hg.netbeans.org/repo/rev/tip will show your diff. I found that easier than attaching diffs to issues.

Y11 Actions shall be defined in Actions folder. Their usages in UI elements shall be via .shadow. I've noticed there is <file name="org-netbeans-modules-ant-freeform-Actions$Custom.instance"> in some folder, such usage shall fail the commit validation test that checks the above rule. Unless you disabled the check which I would found unfortunate.

Y12 Probably you want to add test for the DynamicMenuChange

Re. Y01. This all depends on "who knows better" http://wiki.apidesign.org/wiki/SuperVsInner in my approach the project definition remains in control (the folder shadow is in fact "inner"). My guess is that UI consistency point of view prefers "inner" too. So I am glad that we are starting with "general.shadow".
Comment 16 Tomas Zezula 2010-03-26 12:48:09 UTC
Seems good to me.
Comment 17 Jesse Glick 2010-03-26 12:49:45 UTC
(In reply to comment #15)
> Meta comment: You can do [...]

'hg pmerge && hg push' is simpler. Or you can just run 'hg pdiff' yourself if you install pbranch, or wait for

http://bitbucket.org/parren/hg-pbranch/issue/38/branch-diff-command-in-hgweb

to browse over HTTP.

> Y11 Unless you disabled the check

I did...

> which I would found unfortunate.

Can change if you care. Generally I am reluctant to create a global registration for an action which can only anyway apply to a single project type, since it could not be used in any other context. It would be more consistent, which might be useful for some purpose TBD - hypothetical future UI for context menu customizations, apisupport branding, etc.

> Y12 Probably you want to add test for [HIDE_WHEN_DISABLED]

Can try it.

(Originally tried to implement directly in actionsForPath but it seemed cleaner to make this an aspect of the presenter provider. We should also consider an optional action interface which produces an Action[] and splices in the result in the place of the original action, as this would greatly simplify implementation of meta-actions like Custom in the diff. As I have written elsewhere, it is unfortunate that our APIs initially assumed that every Action in a context menu is "real"; we should rather have had a list of factories, each of which would be passed a Lookup context and could then dynamically produce a list of either real Action's or JMenuItem's.)

> Re. Y01. in my approach the project
> definition remains in control

Even with merged folders you can perform any kind of override you like from the project's definition, so this is not a hard benefit (compared to the greater flexibility for third parties afforded by the merging approach).

> UI consistency point of view prefers "inner" too.

The UI could be consistent or inconsistent using either approach; it is up to developers to implement the UI they want. Consistency is actually somewhat easier to implement using the merging approach since there would be less redundancy in definitions - any inconsistencies from the "standard" in a project type would be visible in its layer.

Compare the situation with the MIME Lookup API, especially the context menu definitions for different editor kits. Context menus are consistent by default, since they share a generic definition (used as is by text/plain); but it is straightforward to insert additional items for a particular kit; or, if you really wanted to and with a bit more work, you could hide or reorder standard items.

> I am glad that we are starting with "general.shadow".

Starting with it because it is easier to implement on the API side, and it would have to be kept as an option even when using merging.
Comment 18 Jesse Glick 2010-04-01 16:11:45 UTC
Y11 - core-main #b92c4ffa1f03

Y12 - core-main #a3e7dda9d47a
Comment 19 Jesse Glick 2010-04-01 19:29:01 UTC
Implemented also for Maven projects. Not doing apisupport.project now.

core-main #a55a6e022b57
Comment 20 Quality Engineering 2010-04-03 05:31:40 UTC
Integrated into 'main-golden', will be available in build *201004030201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/a55a6e022b57
User: Jesse Glick <jglick@netbeans.org>
Log: Issue #182488: declarative registration of project root node actions.
Comment 21 Jesse Glick 2010-05-03 13:55:36 UTC
*** Bug 108608 has been marked as a duplicate of this bug. ***


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo