Bug 161286

I should also mention that the primary motivation for fixing the problem is issue 158765 - e.g. ergonomic IDE needs a 
way to analyse registered nodes and providing icon&co. declaratively with be of great help.

Created attachment 78924 [details]
API and its use throughout the IDE

There is no impl of LazyNode visible in the patch. I presume it delegates to original upon addNotify of children, or
getActions; and thereafter delegates all methods to original (e.g. getPasteTypes, ...).


[JG01] Do not make people write this stuff in their layer; define an annotation. For example, I would like to see

public class HudsonRootNode ... {
  ...
  @Node.Registration(location="UI/Services",
                     name="hudson",
                     displayName="org.netbeans.modules.hudson.ui.nodes.Bundle#LBL_HudsonNode",
                     icon="org/netbeans/modules/hudson/ui/resources/hudson.png",
                     position=488)
  public static Node getDefault() {...}
  ...
}

The processor would be trivial to implement.

But consider instead defining a more specialized annotation in an appropriate module specifically for the Services tab.
This would avoid the need to hardcode the UI/Runtime path and make code clearer. E.g.

  @ServicesNodeRegistration(name="hudson",
                            displayName="org.netbeans.modules.hudson.ui.nodes.Bundle#LBL_HudsonNode",
                            icon="org/netbeans/modules/hudson/ui/resources/hudson.png",
                            position=488)
  public static Node getDefault() {...}

Of course such an annotation would be less general, but we might want specialized annotations for other purposes anyway,
and I don't know of any other place in the IDE where this kind of registration of singleton nodes (as opposed to node
factories) would make sense. The NodeOp.factory method could stay where it is for possible future use - just the
annotation would be in a different module.

The only question is which module to put the annotation in. One possibility would be openide.windows. UI/Runtime/ is
defined by core.ide, the TopComponent (NbMainExplorer.MainTab.createEnvironmentTab) is implemented by o.n.core, and it
is registered in core.execution - i.e. a mess. Ideally the folder, TC, and annotation would all be defined in the same
module. A new module could be created for the purpose, or as the most appropriate-seeming existing module, core.ide
could be given a public API (with just the annotation) and the TC and its registration moved there. (AFAIK there is no
other use of NbMainExplorer and it could probably be killed.)


[JG02] Is there any purpose for having the public factory method? There is no situation where you would want to call
this method overload (you have to instantiate the original node just to pass it in!), so better to remove it.

Created attachment 78937 [details]
Patch with new classes in API as well as change of implementation

Re. JG01 - I happily provide annotation if there is sensible place to put it to. Creating new module however seems 
like overkill to me. Rather than that I prefer to keep just layer based registration.

Re. JG02 - The factory method is the only place to put the documentation to (in absence of the annotation). It used to 
be good habit to have both methods one visible in JavaDoc with all docs including layer one, one callable from layer. 
If JG01 is resolved, I am OK with deleting the public method.

JG01 - I suggest core.ide. Would that work? I could if necessary help with prep work - moving the current Services tab
infra to that module - which would be desirable independently of this issue.


JG02 - barring JG01, info could be in class Javadoc.


[JG03] LazyNode should behave sensibly if optional attributes are missing. At least it seems that shortDescription is
optional (e.g. HudsonRootNode does not set it currently). Not sure if setShortDescription(null) is legitimate.


[JG04] switchToOriginal should handle the case that map.get("original") returns null, e.g. after a CNFE is thrown by XMLFS.


[JG05] Careful with JUnit 4, it probably will not work correctly with NbTestCase, and we need to use NbTestCase so we
can add @RandomlyFails etc.

Re. core.ide - usually we want API to be in autoload module (although in case of compile time processor this may not 
be that important). core.ide can't be autoload. Moreover core.ide is in ideXY cluster, while the functionality is in 
platform. 

I don't think the module needs to be autoload. That is certainly appropriate for an API which is purely a bunch of Java
classes you can use. But this deals with a major added GUI element, which you need to decide to enable.

As to the cluster - NodeOp is indeed in the platform cluster, and that is fine. The Services tab is however defined,
intentionally, in the ide cluster. And all potential users of the registration would also be in the ide cluster or
higher; this GUI element is intended for use in an IDE. Putting the registration in a module in the platform cluster
would be incorrect.

Preparation: Services tab moved to core.ide. core-main #99eb1a5de1b6

Integrated into 'main-golden', will be available in build *200903281400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/99eb1a5de1b6
User: Jesse Glick <jglick@netbeans.org>
Log: #161286 prep: move Services tab into core.ide module.

Created attachment 79153 [details]
New patch with @annotations

I'd like to integrate the change soon.

JG02 still unaddressed in attached patch, JG03-05 as well I think though less important.


[JG06] Modules using the new annotation should have a <run-dependency>; they really expect the Services tab to be there.


[JG07] Replace

if (reg.position() != Integer.MAX_VALUE) {
    f.intvalue("position", reg.position());
}

with

f.position(reg.position());


[JG08] Do not use both instanceFile here, as that indicates that you want to bind the instance to the annotated element,
which is not the case; use only instanceAttribute. (An RFE for a method to create a nicely named file in a given folder
based on the element name would be accepted.)


[JG09] displayName in db.explorer.node.RootNode is malformed; use . not / as sep.


[JG10] The tab is called "Services", not "Service", so suggest "ServicesTabNodeRegistration".
                                                                       ^

[JG11] core.ide should dep on new openide.nodes.

Integrated into 'main-golden', will be available in build *200903311400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/491c989f09c8
User: Jesse Glick <jglick@netbeans.org>
Log: #161286 prep addendum: set icon for Services tab.

core-main#fcbe7ad3185b

Re. JG03 - imho the node behaves sensibly. setSD(null) seems to be allowed. Report a bug if you find otherwise.
Re. JG04 - the code handles such situation by reporting an exception and continuing. I am not sure what else you want 
it to do.
Re. JG05 - I do not plan to have unstable tests in this area.
Re. JG08 - You want me to not use an API that works and report you an issue to create another API that will work!? 
Sorry, I don't get the point. Feel free to do it yourself.

Otherwise I think I addressed your comments.

Integrated into 'main-golden', will be available in build *200904020200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/757b11c277c1
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #161286: Addressing JG06 and JG11. E.g. fixing runtime dependencies of those who use the new annotation

core-main#0109f9b6fc77 - fixing broken javadoc links

btw. Jesse: now when all modules using @ServicesTabNodeRegistration have runtime dependency on core.ide, the core.ide 
itself shall be autoload, shall it not?

core.ide defines a couple of other assorted UI elements. No strong opinion.

Integrated into 'main-golden', will be available in build *200904031400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/d5707b4a8011
User: Jesse Glick <jglick@netbeans.org>
Log: Dependencies added in 757b11c277c1 (for #161286) were invalid acc. to schema.
Correcting, and also making sure this mistake does not happen again.