This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 166658 - Declarative actions for 6.8
Summary: Declarative actions for 6.8
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Actions (show other bugs)
Version: 6.x
Hardware: All All
: P1 blocker (vote)
Assignee: Jaroslav Tulach
URL:
Keywords: API_REVIEW
: 18325 66836 (view as bug list)
Depends on: 183794
Blocks:
  Show dependency tree
 
Reported: 2009-06-06 00:31 UTC by Jaroslav Tulach
Modified: 2010-04-09 17:09 UTC (History)
4 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Sketch of @ActionReference, see especially @MimeAction registration definition and usage, this would be handy for defining @Shortcut, @Menu, etc. (9.17 KB, patch)
2009-06-10 18:39 UTC, Jaroslav Tulach
Details | Diff
Javadoc with new context interfaces and factory methods (467.12 KB, application/x-compressed)
2009-06-30 15:35 UTC, Jaroslav Tulach
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Tulach 2009-06-06 00:31:11 UTC
The goal is to eliminate need for use of SystemAction & similar and replace their common usages with something more 
declarative, easier to use and effective in the runtime.

http://wiki.netbeans.org/DeclarativeActionRegistration
Comment 1 Jaroslav Tulach 2009-06-06 00:32:59 UTC
Please review the plan to introduce @ActionRegistration as described at 
http://wiki.netbeans.org/DeclarativeActionRegistration
Comment 2 David Strupl 2009-06-08 16:19:30 UTC
DS01: "The actual usages in menu & co. will remain in layers". I don't think this is the right approach. I suggest that
the annotation somehow covers also  possibility to create the needed shadow links to the generated layer entry.
Comment 3 David Strupl 2009-06-08 16:24:34 UTC
DS02: @ActionRegistration(
...
   displayName="#Save",
   iconBase="org/netbeans/modules/openide/actions/save.png",
)

Can/will the processor check that the mentioned image file is present? Also can/will the processor check that the bundle
entry for the display name is present?
Comment 4 Jaroslav Tulach 2009-06-08 17:28:36 UTC
DS02: Bundle check is there. Icon check is not yet. Good idea.
DS01: I agree writing a link in a file that points to non-existing file that will be generated later is confusing. 
Thus let's add:
@interface ActionReference {
  String path(); // folder in sfs
  String id() = ""; // name of the .shadow file, by default taken from reference
  String refid() = ""; // path to original action, inferred if @ActionReference and @ActionDeclaration used at once
}
Comment 5 _ tboudreau 2009-06-08 18:44:32 UTC
Suggest including the stuff in spi.actions here - while this solves the general case of declarative registration, there is still a need for a simple replacement for 
a hand-coded CookieAction - for example, you are writing a Node and want to provide some actions for it by overriding getActions(boolean).
Comment 6 Jesse Glick 2009-06-09 17:17:28 UTC
Adding to DS1,

[JG01] Making these annotations work well with _registrations_ of actions to particular places in the GUI is the tricky
part (and must be solved for this API to be accepted). We should not do the straightforward (if not trivial) work in the
trunk and then hope that the hard part can be solved later. So I do not agree with there being a Phase I that ignores
the hard decisions, unless this is done only in a branch (so we can decide to defer for after 6.8, or rewrite the API
before merge).


[JG02] You should use SourceVersion.RELEASE_6, not SourceVersion.RELEASE_5, since you support -source 1.6.


[JG03] Accept ElementType.METHOD as well as TYPE in @AR. For non-context-sensitive actions, only need to change
calculation of id. (This logic should anyway be moved into a helper method in LayerGeneratingProcessor or LayerBuilder
or something.) For C-S actions, I guess you should accept a method with parameters? Whatever logic is in
@ProjectServiceProvider etc. should be factored out into helper methods.


[JG04] I think ANY is less expected and less often used than EACH (or ALL?) for C-S actions. E.g. if you declared

@AR(...)
public class CloseProjects implements ActionListener {
  public CloseProjects(Collection<Project> projects) {...}
  ...
}

the expectation is that this action will be available iff the selection consists of one or more nodes associated with a
Project, not if it is one Project node and one JDBC table node etc.


[JG05] For C-S actions, does the processor enforce that it is an ActionListener? Does it check that you are using <T>
rather than <? extends T>?


[JG06] ContextSelection should have the method isEnabledOnData implemented on it rather than using a switch statement.
Similarly, it could have a method ContextActionPerformer<Object> to get an injector.


[JG07] Consider putting @AR and associated code into a fresh module, perhaps api.actions. It is strange to be importing
"org.openide.awt" when this has little to do with AWT. openide.awt will become rather cluttered with all the new code,
which does not have much to do with the former contents (mostly utilities dealing with Swing components). And it would
be nice to have Tim's stuff (currently in spi.actions) be included, too.


[JG08] "Closeable" is incorrect. You meant "Closable". Similarly "Saveable" should be "Savable" and so on. In general in
English, 'e' should be inserted before a suffix only if the stem ended in a soft 'c' or 'g' ("Cloneable" is too late to
correct):

markable
replaceable
singable
pageable

and note that certain words have special forms from Latin, e.g. read -> legible.


[JG09] If you are going to copy code, please take some care to clean it up in the process. It is pretty strange to see a
brand-new class labeled

* @author Petr Hamernik
* @version 0.10, Apr 03, 1998

!


[JG10] As I have argued elsewhere, the proposed interface for Savable is bad; cookie-like interfaces should be designed
so that they need not be dynamically added to or removed from lookups, i.e. SaveCookie was misdesigned. There are
subtleties, especially to get Save All action to work in a more modern way (as has often been requested by Platform
users); look at issue #77210.
Comment 7 Jesse Glick 2009-06-09 17:34:55 UTC
To be clearer about JG01, the "tricky parts" are those things mentioned in

http://wiki.netbeans.org/DeclarativeRegistrationUsingAnnotations#section-DeclarativeRegistrationUsingAnnotations-ProblemsWithActions

I have some ideas how these issues might be resolved, but these ideas are unproven. We should not commit to a basic
infrastructure until it is clear that it will support the chosen techniques.

For example, the issue of having a data loader specify a list of well-known actions (rather than vice-versa) means that
we want some developer-friendly way to refer to a list of action IDs. If we simply use

  String id() default "";

in @AR and many actions use the generated ID then it is unclear how you are supposed to know exactly what that ID is!
Ideally any reference to an action ID would use a compiler-verified constant, so that the module defining the actions
could export these IDs as an API (without necessarily exporting the action impls themselves). Should this ID type be a
String? Or an Enum<?>? Or a Class<?>?
Comment 8 Jesse Glick 2009-06-10 16:20:32 UTC
[JG11] Probably context @ActionRegistration should be on the constructor rather than the class, making it intuitive that
you can alternately use a static method. (Inconsistent with @PSP but maybe that could be (compatibly) fixed too.) So you
could have either:

public class MyAction implements ActionListener {
  @AR(...) public MyAction(Collection<DataObject> ds) {...}
}

or

// anywhere
@AR(...) public static ActionListener myAction(Collection<DataObject> ds) {...}
Comment 9 Jaroslav Tulach 2009-06-10 18:39:10 UTC
Created attachment 83410 [details]
Sketch of @ActionReference, see especially @MimeAction registration definition and usage, this would be handy for defining @Shortcut, @Menu, etc.
Comment 10 rmichalsky 2009-06-10 21:07:49 UTC
[RM01] Phase II requires more work in apisupport than changing New Action Wizard templates. Annotation registrations are
not reflected in layer filesystem until project is compiled (and not yet at all in NB.org projects). We've talked about
this in person, but I realized further consequences, namely <this layer in context> node and NAW show wrong content,
resulting in buggy behavior in fairly common use-cases:

 a) Just added action will show up neither in layer nodes in project window nor in NAW.
 b) When adding two actions consecutively, you cannot place the 2-nd one after the 1-st one in wizard, as the 1-st won't
show up.
 c) After adding a menu (presuming menus will be finally registered by annotations as well), you cannot directly add an
action to it, as the menu won't show up in NAW.

To get this fixed we'd need either full-fledged Compile on Save or perhaps "process annotations on save" (?) or
something similar. Without it we'll end up with regressed and buggy apisupport projects for long. 

[RM02] ad JG01: +1, also from the POV of NAW solving currently messy context menu registrations would be the most
beneficial part, otherwise it is just another fancy way how to add an action atop of about 10 already existing ones. Why
not just state a path (or a list of paths) in layer file, where will be the action registered, e.g. "Loaders/text/html"
for mime-type based registration? Such a path then becomes sort of well-known ID for registration. The other side would
register this path as its, well, extension point, allowing validation and some generic processing. 

What I don't understand is (maybe a naive question), what would you need e.g DataObject to state its actions? The
vice-versa approach, i.e. action registers itself to DO, editor, project type, etc. seems more appropriate to me.

[RM03] ad custom presenters being out of scope: most of custom Presenter.Popup implementations are used for:

 a) showing a submenu in popup menu
 b) dynamically showing and hiding a menu item

Both cases could be IMHO expressed declaratively thus lowering number of necessary custom presenters to bearable
minimum. Showing static text/icon until item is clicked in not an option IMHO.
Comment 11 Jesse Glick 2009-06-11 00:35:43 UTC
-1 on @ActionReference.Meta; specialized annotations ought to be handled by specialized processors which perform proper
compile-time validation - that a MIME type is well-formed, a keyboard shortcut representation is valid, and so on. If
necessary there can be convenience APIs to make writing such processors as quick as possible.


To RM01 - I think this is a somewhat separate issue; we already needed a different way of displaying layer entries in 6.7.

That said, "process annotations on save" would be natural. The "on save" part is less important than the "process
annotations" part - i.e. support JSR 269 in NB. Long overdue, but there may be unforeseeable difficulties in its
implementation. Surely filed somewhere in java/source.


"action registers itself to DO, editor, project type, etc." - this does not work in general. Should OpenAction register
itself for *.form files? Or ReformatAction for *.xsd? Or SetAsMainAction for BPEL model projects? Impossible. Also see
(written for 6.7):

http://wiki.netbeans.org/DeclarativeRegistrationUsingAnnotations#section-DeclarativeRegistrationUsingAnnotations-ProblemsWithActions
Comment 12 Miloslav Metelka 2009-06-11 13:41:41 UTC
ad [RM03]:
 I also consider implementing of Presenter.Menu or Presenter.Popup due to checkbox menu item for editor actions. Altghough there's 
    Actions.connect(JCheckBoxMenuItem item, BooleanStateAction action, boolean popup)
I don't like the fact that BooleanStateAction extends SystemAction.

[MM01]
Since I'm just working on cleanup of editor actions (issue 166027): Do we plan to bring any kind of unification of editor actions with "system" actions? I 
guess not or not with this issue.

[MM02]
What would @Shortcut include? Please keep in mind that
  1) There are multiple shortcut profiles and action should show up in each of them.
  2) Action can have multi-key shortcut.
Comment 13 Jaroslav Tulach 2009-06-30 15:35:54 UTC
Created attachment 84192 [details]
Javadoc with new context interfaces and factory methods
Comment 14 Jaroslav Tulach 2009-06-30 15:40:38 UTC
I'd like to integrate tomorrow. All issues resolved, API created, apisupport changed, certain basic actions rewritten:

TCR1 - I kept SaveCookie as requested and rewrote SaveAction to work without nodes. However the logic of SaveAction is 
complex, it needs to seek the Lookup itself. Thus I needed to enhance Actions.context to delegate also to any 
ContextAwareAction. 

TCA2 - The original advice was to not expose .context(...) factory method in Java API because of its meaningless 
signature. Due to the support for SaveAction mentioned above I needed to change the signature so it makes sense now. 
Thus I included both context and callable methods in the API.
Comment 15 Jaroslav Tulach 2009-07-01 11:51:22 UTC
core-main#6dbe2ad21459
Comment 16 Quality Engineering 2009-07-01 17:13:06 UTC
Integrated into 'main-golden', will be available in build *200907011400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/6dbe2ad21459
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #166658: Merge of work on declarative actions
Comment 17 Jesse Glick 2009-12-11 12:38:49 UTC
*** Bug 18325 has been marked as a duplicate of this bug. ***
Comment 18 Jesse Glick 2010-01-23 09:53:54 UTC
*** Bug 66836 has been marked as a duplicate of this bug. ***