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 248933

Summary: Provide API for file separator
Product: platform Reporter: Vladimir Kvashin <vkvashin>
Component: FilesystemsAssignee: Jaroslav Havlin <jhavlin>
Status: NEW ---    
Severity: normal CC: issues
Priority: P3 Keywords: API_REVIEW_FAST
Version: 8.1   
Hardware: All   
OS: All   
Issue Type: DEFECT Exception Reporter:
Attachments: Proposed change
Proposed change v2
Alternative patch v1

Description Vladimir Kvashin 2014-11-26 17:11:15 UTC
After remote file system is introduced, one can not use File.separator when constructing paths any more: if local system is Windows, then File.separator is "\\" (backslash), while for remote paths "/" should be used.

So FileSystem API should have a method that returns a proper separator.
Comment 1 Vladimir Kvashin 2014-11-26 17:18:32 UTC
Created attachment 150733 [details]
Proposed change
Comment 2 Jaroslav Havlin 2014-11-27 15:33:35 UTC
Created attachment 150746 [details]
Proposed change v2

Slightly updated patch.

 - JavaDoc and apichanges.xml entry added.
 - FileStateInvalidException is logged in FileUtil.getNativeFileSeparator(FO)

I'm just not sure whether the handling of FileStateInvalidException in 
FileUtil.getNativeFileSeparator(FO) (ignore and return default "local" value)
is always correct, and thus whether the convenience method should be introduced.
Let's wait for other opinions.

Thank you very much for the patch.
Comment 3 Jaroslav Havlin 2014-11-28 09:15:43 UTC
We were discussing the new method with Ondra, who pointed out that we should provide correct implementation of getNativeFileSeparator() for existing filesystems. But:

 - What about filesystem that doesn't have underlying native filesystem with
   directory structure (XML filesystem). Does having native file separator
   make sense for them?
 - Even on Windows slash (/) is used as file separator in file names for
   LocalFile system, e.g. in method findResource(String) and similar ones.
   So where and how the native file separator would be used?
 - Probably the FileSystem implementation should abstract access to the
   native filesystem and handle its directory structure and path format
   internally.

I'm sorry for not seeing this questions earlier.
Comment 4 Vladimir Kvashin 2014-11-28 10:42:21 UTC
(In reply to Jaroslav Havlin from comment #3)
> ...
>    So where and how the native file separator would be used?
> ...

Sorry for not stating my thoughts clear enough.
My intention was to use it 
1) when constructing a path that is shown to user 
2) when constructing a path that is to be passed to an external utility
In both cases, on Windows, this should be "\", on other systems - "/".

For use case that led to this issue creation, jar or system file system does not matter at all; however, in my opinion, it would be logical if these file system had "/" as native file separator (for example, if I launch jar -t on Windows, it shows "/" slashes.)

So I would propose that 
- org.openide.filesystems.FileSystem returns "/"
- o.n.m..masterfs.filebasedfs.FileBasedFileSystem returns File.separator
- remote file system for *nix and MacOS returns "/"

Or we can make this method abstract and implement in each descendant (there are quite a few).
Comment 5 Jaroslav Havlin 2014-11-28 11:59:47 UTC
(In reply to Vladimir Kvashin from comment #4)
> Sorry for not stating my thoughts clear enough.
No problem. Thank you for explanation.

> 1) when constructing a path that is shown to user 
In local filesystem, FileUtil.getDisplayName(FileObject) is used in this scenario.
It doesn't work correctly for remote files?

If some modification is needed, I would prefer just improving FileUtil.getDisplayName(). Maybe it could look for FileObject's attribute "org.openide.filesystems.FileObject.displayName" and use it as display name if it is found.


> 2) when constructing a path that is to be passed to an external utility
In local filesystem, FileUtil.toFile(FileObject) is used in this case. We might add some more general alternative, e.g. FileUtil.toFilePath(FileObject).

But, if we want to pass the native path to some utility, doesn't we need to be aware of remote filesystems anyway (or the remote nature of the task)? If so, we would have some general API, which is useful only when working with remote files. And thous it would be sufficient to have this information in Remote Filesystem API.
Comment 6 Jaroslav Havlin 2014-11-28 12:17:50 UTC
Note: Knowing the native separator is useful when constructing paths to not-existing files.
Comment 7 Vladimir Kvashin 2014-12-01 13:00:34 UTC
(In reply to Jaroslav Havlin from comment #5)
(Just writing down what was said when discussing this in Prague)

> ...
> If some modification is needed, I would prefer just improving
> FileUtil.getDisplayName().
That's probably OK if we just display it, but not OK if we fill a user editable field with path

> ...
> > 2) when constructing a path that is to be passed to an external utility
> In local filesystem, FileUtil.toFile(FileObject) is used in this case. We
> might add some more general alternative, e.g.
> FileUtil.toFilePath(FileObject).
I'm OK with that as an API, however in this case we need some SPI for a particular FileSystem to override the behaviour.

> But, if we want to pass the native path to some utility, doesn't we need to
> be aware of remote filesystem anyway (or the remote nature of the task)? If
> so, we would have some general API, which is useful only when working with
> remote files. And thous it would be sufficient to have this information in
> Remote Filesystem API.
This can make less possibilities for code reuse. A logic that constructs a path and launches a utility and processes its output should not necessarily "know" whether it works in local context or remote.
Now we work on supporting version control over remote file system.
In order to have more code reuse for local and remote, we need some more general method of constructing the path.
Comment 8 Jaroslav Tulach 2014-12-01 14:43:55 UTC
Y01 Isn't it finally a time to add FileObject.getFileObject(String relativePath, boolean onlyExisting) method? That method could by default construct Invalid file object and deliver an event, when it becomes valid. Various filesystems could override it. The (invalid) FileObject could be passed into FileUtil.toFile for example.

Y02 RemoteFS is closely tight to nativeexecution API. Should not the path be obtained in a "native execution context"? It makes little sense to get the "general" path, it only makes sense in a particular "native execution context". Should not native execution API be enhanced with an ability to convert a FileObject to proper path?
Comment 9 Vladimir Kvashin 2014-12-05 10:53:52 UTC
(In reply to Jaroslav Tulach from comment #8)
> Y01 Isn't it finally a time to add FileObject.getFileObject(String
> relativePath, boolean onlyExisting) method?
> ...

I don't see how this resolves the issue. The issue is that (if we are on Windows) in one NetBeans instance some paths (local) should be shown using "\" separators while other (remote, jar, etc) should be shown with "/" separator. But NetBeans FileSystem always uses "/".


> Y02 RemoteFS is closely tight to nativeexecution API. 
> ...

You are right: if a module depends on nativeexecution and remote "semi-public" API, such module has no such issue at all.

But there are lots of UIs and other functionality that aren't tightly tied with nativeexecution, they work in terms of FileSystem API. And lots of UIs show paths. I think that's good for a such UI to not depend on nativeexecution (less dependencies always better).

Well, we can do without that. But we'll have more dependencies then. And having less dependencies is always better.
Comment 10 Jaroslav Havlin 2014-12-10 10:35:58 UTC
Created attachment 151004 [details]
Alternative patch v1

> Y01 Isn't it finally a time to add
> FileObject.getFileObject(String relativePath, boolean onlyExisting) method? 
> That method could by default construct Invalid file object and deliver an 
> event, when it becomes valid. Various filesystems could override it. The
> (invalid) FileObject could be passed into FileUtil.toFile for example.
It would be useful, but it seems to be quite a complex change (for now).


What about probably the simplest possible solution - using FileObject's attribute "NativeFileSeparator". In masterfs FileObjets, it would be java.io.File.separator. In remote FileObjects, it would be the separator used by the remote OS. For other FileObjects with no underlying native file, it would be undefined (null).

Then we could call someFileObject.getAttribute("NativeFileSeparator") and use the value or handle the returned null somehow.

(The patch contains only the change in masterfs. It would we similar in remote FS.)