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 191407 - @ActionRegistration equivalent for TopComponent.openAction
Summary: @ActionRegistration equivalent for TopComponent.openAction
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Window System (show other bugs)
Version: 7.0
Hardware: All All
: P2 normal (vote)
Assignee: Jaroslav Tulach
URL:
Keywords: API, API_REVIEW_FAST
: 53252 (view as bug list)
Depends on: 197161 200048
Blocks: 183794
  Show dependency tree
 
Reported: 2010-10-27 13:39 UTC by Jesse Glick
Modified: 2011-07-28 19:59 UTC (History)
5 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Rough patch (20.38 KB, application/octet-stream)
2010-11-19 21:41 UTC, Jaroslav Tulach
Details
Splitting the annotation to three independent ones (27.04 KB, patch)
2010-11-22 22:57 UTC, Jaroslav Tulach
Details | Diff
To integrate tomorrow (43.10 KB, patch)
2010-11-29 16:34 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2010-10-27 13:39:47 UTC
@ActionRegistration does not cover TopComponent.openAction; there are quite a few of these, so it probably should. Suggest something along the lines of

class TaskListTopComponent extends TopComponent {
  ...
  @OpenActionRegistration(iconBase="org/netbeans/modules/tasklist/ui/resources/taskList.png", displayName="#CTL_TaskListAction")
  public static synchronized TaskListTopComponent findInstance() {...}
  ...
}
Comment 1 Jaroslav Tulach 2010-11-03 15:32:47 UTC
Rather than just registering the top component open action, I suggest to create 

@Target({METHOD,CLASS})
@Retention(SOURCE)
@interface TopComponent.Registration {
  String id(); // id of the component, reused for the action too
  String mode(); // name of mode to put the component into
  String actionCategory() default ""; // by default don't generate an action at all
  boolean visible()
  String dockingStatus() default "maximized";
}

The processor for this action would generate not only the action but also .settings file as well as .wstcref file.
Comment 2 Jesse Glick 2010-11-03 16:32:04 UTC
[JG01] I assume you would still need some @ActionReference's? (Otherwise you would need to specify at least a Window submenu path - or is this just copied from actionCategory, assuming that has an implied "Window/" prefix? - and position.) If so, how would ActionProcessor find the ID?

I think it is better to keep the action annotation distinct from the component annotation; the former would be on findInstance and be accompanied by a regular @ActionID and @ActionReference's, whereas the latter would be on getDefault and just register the component.

Anyway a patch is needed to verify that whatever is proposed actually works.


[JG02] position attribute would be needed, to place the window within its mode.


[JG03] dockingStatus should be an enum. I guess this applies just to default-mode (see JG05)?


[JG04] Generating <module> from an annotation processor (for both *.settings and *.wstcref) is probably impossible. Probably the window system should just be fixed to not require this redundant information to begin with. I guess a ClassNotFoundException accomplishes the same thing, if the stack trace is suppressed.


[JG05] tc-ref2_2.dtd defines some other stuff - slide-in-status, previous-mode, docking-status@maximized-mode - can it be safely ignored? I don't understand the usage well enough to know which things are actually needed for initial declarations of TCs in layers, vs. which things are only needed by the window system when persisting customizations. I am guessing that docking-status can be omitted from the annotation entirely - modules generally declare windows docked, and users can convert them to sliding when desired.


[JG06] visible() should probably be called opened() for consistency with the XML form. Or initiallyOpened() may be more descriptive.
Comment 3 Stanislav Aubrecht 2010-11-05 08:23:46 UTC
re [JG05] - they're all optional attributes that are mainly used to serialize the window system state. i imagine some platform developers might want to use them to fine-tune the initial state of their topcomponents but they can switch to layer registration instead of the annotation.

re [JG06] - i'd prefer openAtStartup()
Comment 4 Jesse Glick 2010-11-05 12:45:40 UTC
JG05 - that's what I suspected. Good, so we can omit these things from the annotation unless and until someone requests them.


JG06 - yes, openAtStartup is clear.
Comment 5 Jaroslav Tulach 2010-11-19 21:41:02 UTC
Created attachment 103121 [details]
Rough patch
Comment 6 Jaroslav Tulach 2010-11-19 21:48:59 UTC
It turns out that implementing different registrations for @ActionID is not possible right now, so my patch needs to fix that.

By adding support for "preferredID" attribute (not yet properly documented) into openAction(Map), we can simplify the template for singletons. There is no longer any need for getDefault and findInstance. Enough to:

    @TopComponent.Registration(id="PrefTopComponent", mode="output", openAtStartup=true, actionDisplayName="#CTL_PrefAction")
    @ActionID(category="Windows", id="my.PrefTopComponent")
    @ActionReference(path="Menu/GoTo", position=300)
    public static synchronized PrefTopComponent getDefault() {

Probably the annotation shall be improved to handle also non-singleton components somehow...
Comment 7 Quality Engineering 2010-11-21 06:09:17 UTC
Integrated into 'main-golden', will be available in build *201011210001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/280214ceb7dc
User: Jesse Glick <jglick@netbeans.org>
Log: Prep for #191407.
Comment 8 Jaroslav Tulach 2010-11-22 22:57:40 UTC
Created attachment 103214 [details]
Splitting the annotation to three independent ones
Comment 9 Jaroslav Tulach 2010-11-22 23:01:54 UTC
The new patch would be used like:


@ConvertAsProperties(dtd = "-//org.netbeans.saas//Pref//EN",
autostore = false)
@TopComponent.Description(
    preferredID="PrefTopComponent", iconBase="", 
    persistenceType=TopComponent.PERSISTENCE_ALWAYS
)
@TopComponent.Registration(mode = "output", openAtStartup = true)
@ActionID(category = "Windows", id = "my.PrefTopComponent")
@ActionReference(path = "Menu/GoTo", position = 300)
@TopComponent.OpenActionRegistration(
    displayName = "#CTL_PrefTopComponent",
    preferredID = "PrefTopComponent"
)
public final class PrefTopComponent extends TopComponent {

by omitting preferredID in @TopComponent.OpenActionRegistration one can create actions that do not work with singleton components, but with unlimitted instances.
Comment 10 Jesse Glick 2010-11-22 23:29:59 UTC
(In reply to comment #9)
> by omitting preferredID in @TopComponent.OpenActionRegistration one can create
> actions that do not work with singleton components, but with unlimited
> instances.

I don't think this is useful, at least for open actions. An action to open a singleton TC generally follows a very standard pattern, and can be installed in the Window menu somewhere. Code to open non-singleton TCs would typically be activated by idiosyncratic conditions, not necessarily actionPerformed, and would typically need specialized code to create the TC, not just calling a default constructor.

BTW assert component != null now needs to be replaced by assert c != null. Or actionPerformed must be prepared for it to be null in case of instantiation problem.


"Rather than overriding this method" Javadoc should be copied into getPersistenceType and getIcon methods.


Do you plan to update NewTCIterator? An update to ActionRegistrationHinter would also be nice, at least for @OpenActionRegistration + @ActionID + @ActionReference.
Comment 11 Jaroslav Tulach 2010-11-29 16:34:35 UTC
Created attachment 103437 [details]
To integrate tomorrow
Comment 12 Jaroslav Tulach 2010-11-29 16:38:43 UTC
Right, opening multiple components without parameters is not that useful. They could use whatever they find in Utilities.actionGlobalContext(), but that is not really flexible. Having some form of DI would help. However the system is not flawed by itself. So I left the ability there.

Wizard changed. Generates singleton ready code. I also changed settings module to work with private factory methods (which makes the wizard generated code nicer, I think).

Javadoc sort of update.
Comment 13 Jesse Glick 2010-11-29 16:51:14 UTC
[JG07] "PREFFERED" is a typo.


[JG08] I'm confused by why findInstance still exists in the new template. Isn't it unused?

And does the instance field even need to exist - why can't create() just return a new instance each time, or just put all the annotations on the type element (and change readProperties to always work on 'this')?
Comment 14 Jaroslav Tulach 2010-11-29 17:58:00 UTC
Re. JG08: The findInstance is commented out for those who wish to create code that accesses the singleton instance. I can indeed remove it, but thought it is suggested practice used in other templates too to leave sample code in there. Removing the instance field will probably work too then.
Comment 15 Jesse Glick 2010-11-29 18:01:23 UTC
(In reply to comment #14)
> findInstance is commented out for those who wish to create code
> that accesses the singleton instance.

OK, but in that case should probably have default access rather than public to indicate that it is for use within neighboring code; and maybe improve Javadoc comment to explain why and when you might use such a method.

> Removing the instance field will probably work too then.

Even the commented-out findInstance does not use the instance field, so I'm not sure I understand the relationship.
Comment 16 Jaroslav Tulach 2010-11-29 21:28:21 UTC
OK, I removed the boiler plate code from the template.
Comment 17 Jaroslav Tulach 2010-11-30 06:24:57 UTC
core-main#7fd81e88d7cb
Comment 18 Quality Engineering 2010-12-01 06:37:56 UTC
Integrated into 'main-golden', will be available in build *201012010001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/755780184b9b
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: In preparation of #191407: Allow calls to non-public factory methods for consistency with methodvalue behavior
Comment 19 Jesse Glick 2011-07-28 19:59:53 UTC
*** Bug 53252 has been marked as a duplicate of this bug. ***