Bug 52271

Petr suggested to have a new FileSystemAction that would behave
non-recursively, or have some setter for recursivity on that action
(it would have to not extend SharedClassObject).

Radek, do you think this is feasible to add to filesystems? Delegate
to e.g. FileSystem.getActions(boolean recursive)?

I believe I have already suggested that this (as well as the current
somewhat ugly situation with Find in Files) could be resolved nicely
with a single (cookie-style) interface that could be placed into
lookup of a node such as

public interface NonRecursiveFolder {
    FileObject getFolder();
}

which would have similar effects to having a (folder) FileObject, or
DataFolder, in your lookup, except that clients which might otherwise
operate on it recursively (such as search or VCS) would be expected to
behave nonrecursively. Package nodes could then add this interface
(instead of DataFolder) to lookup and be done. No need to change the
FileSystemAction API.

What Jesse proposes is what came first to my mind, but then I felt
that this is a bit inside out. The node then says I'm a nonRecursive
node, what ever it means and will hope that the actions will then
behave correctly. 
I would preffer to add correct actions to correct nodes, which is IMO
more straightforward and easier to understand. 
 

I think that there is often a need to recognize type of a node, not
just whether it is recursive or not. I think that an attribute "node
type" would be useful in many situations. I just do not know whether
the "node type" attribute would replace the recursiveness attribute or
whether recursiveness should be an additional information besides
"node type".

"Node type" ~ Node.lookup.

I don't see anything too strange about the proposed lookup addition.
We already have DataObject in lookup, which basically means that the
node represents a folder and everything recursively in it. The
proposed cookie would mean that the node represents a folder and all
files directly in it but no subfolders. We have always used the system
whereby a node's lookup tries to declare what it represents, and
actions adjust their behavior accordingly - this is nothing new.

Sure. Back to the cookie land :-).

My concern was a bit philosophical. It is not the folder what is
non-recursive it is the node or even it is the operation on the node
what is woking in non-rcursive mode. But surely OK and  let's go with
a cookie in FS.

Maybe we should implement some more general contract that maps nodes
to files, say the Node has in its lookup an instance of

public interface FileMapper {
    FileMapperFile [] getFiles();
}

public interface FileMapperFile {
    FileObject getFile();  // file or folder
    boolean isFolderContent(); //  to define folder's content
    boolean recurseFolder(); // to recurse
}

Re. Hrebejk's "It is not the folder [t]hat is non-recursive it is the
node" - exactly, which is why the cookie would be only in the node's
lookup, not e.g. in some DataFolder lookup where it would not make any
sense. "or even it is the operation on the node [t]hat is wo[r]king in
[a] non-r[e]cursive mode" - perhaps, but probably we want *all*
operations on a package node to operate in non-recursive mode, not
just selected actions. Adding nonrecursive-style actions would let the
node specify which actions should behave in which way, but on the
other hand it forces the node to know something about other parts of
the system (VCS, Search). Unfortunately it seems java/project would
still need an explicit dependency on org.openidex.util for other
reasons - default behavior of search is too slow and needs to be
overridden; and need to exclude nonsharable/invisible files from
search. No strong opinion here.

To msandor: what would be the purpose of your proposed interfaces?
Seems rather complicated and I don't know of any use cases which would
need it. BTW you can already add multiple files to your lookup if you
want to.

jesse: The purpose is to map nodes to set of files/folders. Idea
behind two interfaces is to defer child enumeration to time when they
are needed and to cover all scenarios how a node can map to filesystem.

But which actual nodes need such a thing? And which actual nodes need
to defer creation of enumerated files?

Nodes with large number of children maybe.

msandor: But I mean which nodes in the IDE? What module? What window?
I cannot think of such a case offhand.

See additional use case in issue #51132 - a project should have the
option to adjust how it invokes the compiler depending on whether you
are looking at a package or folder representation. Such an issue
cannot be resolved by providing specialized actions, since the action
comes from another place - you need to have some kind of info
available in the Node, i.e. its lookup, and the Node should not have
to be aware of the project's exact needs.

This does not necessarily belong to filesystems. Upon discussions with
several people we suggest to place a marker interface into
openide/loaders.

Therefore I suggest a new interface:

package org.openide.loaders;

/**
 * Marker interface for folders that have non-recursive content.
 * When an implementation of this interface is contained in the
 * lookup of a node, actions on that node should be performed
 * non-recursively.
 */
public interface NonRecursiveFolder {
    FileObject getFolder();
}

-----

The getFolder() method is not really necessary specifically for
package nodes (they have the DataFolder (or FileObject) in lookup), on
the other hand the interface says that some folder is non-recursive,
so it should say which one.

This is an API change, so I kindly ask for a fast API review.

Scheduling for 4.1.

To quote Jesse: "what would be the purpose of your proposed interface?
which actual nodes need to use it?" 

The only usecase I could find from reading the discussion above is a
communication between java package view and cvs, find, and possibly
other modules that need to operate on package and not recursively on
DataFolder. If this is true, then create interface JavaPackage and put
it into the module that provides the java packages and not loaders.

The only reason why such interface should be in loaders is if it
needed some kind of support from FileSystemAction. Does it? Can I see
some sample code that will use the interface? A test showing what
should work? Without it I have to say I do not believe the change is
in an integrateble state.

> what would be the purpose of your proposed interface?

To define that actions on Java package nodes should be performed
non-recursively.

> which actual nodes need to use it?

org.netbeans.spi.java.project.support.ui.PackageViewChildren.PackageNode

> then create interface JavaPackage and put it into the module that
> provides the java packages and not loaders

The problem with this is, that the modules that provide the actions
(VCS, search, ...) do not depend on java module. And that dependency
is not desired. This kind of nodes can be possibly implemented in
other modules as well, therefore it's good to have that interface on
some public place.

> The only reason why such interface should be in loaders is if it
> needed some kind of support from FileSystemAction.

The actions that are provided through FileSystemAction will use it.
But also possibly other actions will use it (search, compile,...).

> Can I see some sample code that will use the interface?

Under construction, I'm going to attach the diff here...

> A test showing what should work?

We do not have any test that would verify the behavior of VCS actions,
that would be quite complex. It will be verified manually that the VCS
actions on package nodes will run non-recursively.

Created attachment 19528 [details]
The implementation in VCS modules, which check the NonRecursiveFolder in Node's lookup

Yarda: my comments "what actual nodes...?" were in reference to
msandor's side comment about a more complex API. I support
mentlicher's proposed simple API in org.openide.loaders.

Re. VcsFSCommandsAction.java: do you really need to search for
multiple instance of NonRecursiveFolder on a single node? Probably just

  nodes[i].getLookup().lookup(NonRecursiveFolder.class)

would suffice.

I know Jesse's comments were for Maros, my comments are for Jesse. It
is good that you support new interface in loaders, but if there is no
other use for it then being provided by java package view, then it is
wrong solution.

A lot previous comments here were about "non-recursive folder", but
(until shown otherwise) they in fact talked about "folder as java
package". Trying to generalize such concept will lead to misuses, that
is for sure. In first phase only javapackages will provide such
interface and only vcs will use it. In second phase more modules will
change their behaviour relying on presence of such interface, while
they will think it means java package (the only know usecase for
them). The third phase will start as soon as someone else decides to
provide the interface from its node (and mean non-recursive folder,
what ever it is supposed to be, but not java package). Then a lot of
functionality will get broken as certain actions will get enabled in
situations they will have never been designed for. This chaos can be
prevented, but not by introducing strange interface in loaders which
have no meaning for it.

Either define what is the loader's interface semantics, or if you fail
to do so, establish either contract from vcs to java or from java to vcs.

> > A test showing what should work?

> We do not have any test that would verify the 
> behavior of VCS actions,
> that would be quite complex. 

The fact that VCS is not used to write test, does not mean it is
impossible or complex. In fact it is pretty easy. [if the interface
stays in current shape], then just create Node that has this interface
and a DataFolder and sure FileSystemAction for that Node (after
calling createContextAware and getting presenter), correctly
propagates to your actions the setRecursionBanned(true) call.

There is nothing complex on this and it is at least polite if you are
trying to mess other apis with interfaces that do not belong there.

We can make that simple test. The complex thing that I had in mind was
to verify that the actual actions will run non-recursively.

So replying to Yarda:

Yes, the only intended use case is from the Java package node
(wherever that happens to be implemented - currently java/project).
But the semantics from the perspective of the client is "folder with
direct contents and not subpackages". The clients do *not* in general
care that this is a Java package; they simply do not want to recurse.
If it happened that somewhere else in the IDE, some folder was
displayed which had nothing to do with Java but which did not
represent subfolders, then this cookie would be desirable on that node
too. That is why I think the currently proposed name is appropriate.

It is not true that VCS is the only client: as mentioned before, there
are several proposed clients -

1. VCS, to use e.g. "cvs ... -l ..." on the folder.

2. Search, to not recurse. Can currently pass a special search info,
which works but requires the package node to depend on the search API,
which seems unnecessary. With the proposed API, that dep could be removed.

3. Refactoring (perhaps - at UI discretion), to avoid e.g. renaming
subpackages in a package view, *but* offer to rename subpackages in a
tree view.

4. Maybe other random stuff - e.g. Internationalize could notice that
its node selection was not recursive.

Note that some actions (VCS, Search) really do not care at all about
Java. Others (Refactor, Internationalize) do - these can still use the
same interface but they have to additionally check
ClassPath.getClassPath to see if the folder is in fact a Java package
(and what its package is, etc.).

BTW: I originally suggested this interface in Filesystems, not
Datasystems, because it does not really have anything to do with the
rest of the org.openide.loaders package; it is simply a cookie using
FileObject. If we want to ultimately deprecate and remove Datasystems,
this interface ought not be affected. (For *recursive* folders we will
need to start putting FileObject, not DataFolder, into lookup - some
client code in project-related modules already looks for FileObject as
a cookie, but the conversion is not complete since FolderNode's do not
provide it yet.) So I would suggest finding a better home for it - if
not Filesystems, somewhere else that we do not plan to eventually
deprecate (projects/queries??).

I still believe the interface does belong to java and should be 
called JavaPackage, but as nobody else agreed with me, treat this as 
minority opinion. 
 
The problem is imho that openide/loaders do not have concept of 
non-recursive folder. No reason for them to provide such interface. 
Other problem is that I do not believe that the interface means 
non-recursive folder. The only usage we have is "java package" and I 
am convinced that it will stay that way, and if not, we will only 
get problems due to misuses as described in my post above. 
 
Btw. notice that we do not need public interface at all, we can just 
add 
pu. st. boolean PackageView.isJavaPackage (Node n); 
and handle that internally without public interface. 

To Jesse:
> I would suggest finding a better home for it - if not Filesystems,
> somewhere else that we do not plan to eventually deprecate
> (projects/queries??).

O.K., I'm for projects/queries. I did not suggest that, because that
interface is different from other queries in that module.
But if it makes no problem, we can put it into org/netbeans/api/queries.
Alternatively we can write this as a query, but it would add an
unnecessary overhead IMO.

For reference - for everything else than cvs (which needs cvs -l on
folder) implementation described in issue 43233 can be successfully
used. This is to support my (minority) opinion that this interface
should be in PackageView.

Re. PackageView.isJavaPackage(Node) - seems inconsistent with the
general lookup pattern in NB. The node represents a nonrecursive
folder, rather than a recursive folder. Recursive folders have always
been in node lookups; why not nonrecursive folders?

There is no reason that I can see to believe that there is anything
Java-specific here; "directory but not subdirectories" is a pretty
generic concept. Yarda warns of problems arising from misuse but does
not give even a hypothetical example.

org.netbeans.api.queries is an inappropriate package; this is not a
query. Could use some other package in the same module. I really only
suggested projects/queries because it's there and because
org.openide.loaders is targeted for eventual deprecation and
org.openide.filesystems was rejected and because we don't want to make
a new module for something so trivial.

Re. possible use cases: I forgot issue #51132, so

5. compile.single action implementations.

Do you have an idea about which folder will be most appropriate in
projects/queries module? I need to resolve this soon.
I suggest org/netbeans/api/fileinfo.
It can possibly contain other classes describing files/folders in the
future (if there will be a need for other specific info about files).
Comments/suggestions?

Proposed package name is OK with me; wait for a second (or third...)
opinion.

I think 'fileinfo' is a reasonable name here. And agree with
projects/queries - though not ideal, it looks like we'd hardly find
better place. Should be well documented.

Thank you. So if no one objects, I'm going to create
projects/queries/src/org/netbeans/api/fileinfo/ folder with the
suggested NonRecursiveFolder interface in ~ 3 - 4 hours. Thanks.

Moving to projects component, there's no "queries" subcomponent
unfortunately.

Fixed in trunk:

/cvs/projects/queries/apichanges.xml,v  <--  apichanges.xml
new revision: 1.3; previous revision: 1.2

/cvs/projects/queries/manifest.mf,v  <--  manifest.mf
new revision: 1.7; previous revision: 1.6

/cvs/projects/queries/nbproject/project.xml,v  <--  project.xml
new revision: 1.5; previous revision: 1.4

RCS file:
/cvs/projects/queries/src/org/netbeans/api/fileinfo/NonRecursiveFolder.java,v

/cvs/projects/queries/src/org/netbeans/api/fileinfo/NonRecursiveFolder.java,v
 <--  NonRecursiveFolder.java
initial revision: 1.1

/cvs/ide/golden/public-packages.txt,v  <--  public-packages.txt
new revision: 1.10; previous revision: 1.9

/cvs/java/project/nbproject/project.xml,v  <--  project.xml
new revision: 1.19; previous revision: 1.18

/cvs/java/project/src/org/netbeans/spi/java/project/support/ui/PackageViewChildren.java,v
 <--  PackageViewChildren.java
new revision: 1.49; previous revision: 1.48

*** Issue 43233 has been marked as a duplicate of this issue. ***