Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is asZnUrl implemented correctly for FileReferences? #99

Open
kasperosterbye opened this issue Jul 27, 2022 · 9 comments
Open

Is asZnUrl implemented correctly for FileReferences? #99

kasperosterbye opened this issue Jul 27, 2022 · 9 comments

Comments

@kasperosterbye
Copy link

@Ducasse and I ran into a peculiar error today, not sure if the problem is mine or yours :-)

The situation an absolute path, for example '/Users/kasper/tmp'.
Assuming the directory tmp exist, I get '/Users/kasper/tmp' asFileReference isDirectory to be true.
However, '/Users/kasper/tmp' asFileReference asZnUrl isDirectoryPath returns false.

The reason for the difference is that the implementation of asZnUrl for file references are based on paths

AbstractFilePath>>asZnUrl
	^ self asAbsolute path asZnUrl

Basing the implementation on the path looses the information that '/Users/kasper/tmp' is a directory.

We have a chokepoint in our code which will allow us to encapsulate this, so we can work around our specific problem. But the question is if the implementation above should really be:

AbstractFilePath>>asZnUrl
	^ self isDirectory 
		ifTrue: [ ('file:',self pathString , '/') asZnUrl ]
		ifFalse: [ self asAbsolute path asZnUrl ]

I am sure you know a better implementation, but this seems to work - that is '/Users/kasper/tmp' asFileReference asZnUrl isDirectoryPath returns true

@svenvc
Copy link
Owner

svenvc commented Jul 29, 2022

This is difficult. I do not think there is a real solution here.

At least on macOS and other Unix based systems, a path to something can be both a file or a directory. Which one depends on the actual thing on the file system. It cannot be determined from the path alone, you have to ask the OS, look at the file system.

This is what FileReference does.

The trailing slash is a way to make clear this difference, just by looking at the path, that is what ZnUrl does. It should be possible to work with a URL without having access to the file system / OS itself.

Now, the suggested change could make sense, I don't know for sure. I need to think about this a bit. Input from others might be useful too.

@kasperosterbye
Copy link
Author

The trailing slash is a way to make clear this difference, just by looking at the path, that is what ZnUrl does. It should be possible to work with a URL without having access to the file system / OS itself.

The solution I ended up with in our code is a variant of the above:

	| znUrl |
	znUrl := aFileReference asZnUrl.
	aFileReference isDirectory 
		ifTrue: [ znUrl addPathSegment: $/ "a Character not a string!?" ].

So while one cannot tell a directory from a file in a pure path, here the starting point is a filereference, and thus we just needed a way to keep alive its directoryness.

We have our issue isolated, but thought it might be worth for you to consider.

@svenvc
Copy link
Owner

svenvc commented Jul 29, 2022

I understand. But my point is that doing #isDirectory on a FileReference is accessing OS/filesystem info that is not obvious from the path alone. This means you can only do it on a (local) machine where that directory actually exists (and is accessible). I could want to work with paths that do not (yet/locally) exist.

But I do not known what the best answer is. It probably depends on the use case.

@kasperosterbye
Copy link
Author

Indeed only on local machine.
The situation is that /User/kasper/tmp asFileReference isDirectory is true, whereas /User/kasper/foo asFileReference isDirectory is not (I do not have a foo directory).

Let the issue rest for a while - soliciting others input is also a good idea as you suggested.

@Ducasse
Copy link

Ducasse commented Jul 31, 2022

May be sven the solution is to provide a solution to configure the ZnUrl to be a directory when the use knows it.
Because in our case, we know that the basedirectory is a directory so we could set it using a setAsDirectory kind of message
and if ZnUrl use this information to resolve correctly then it would be already a step forward.

@svenvc
Copy link
Owner

svenvc commented Aug 1, 2022

May be sven the solution is to provide a solution to configure the ZnUrl to be a directory when the use knows it.
Because in our case, we know that the basedirectory is a directory so we could set it using a setAsDirectory kind of message
and if ZnUrl use this information to resolve correctly then it would be already a step forward.

There is such a method, it is called #closePath this adds a trailing slash to the path if there is not already one present, effectively making sure that #isDirectoryPath returns true.

@Ducasse
Copy link

Ducasse commented Aug 3, 2022

Ahhhhh I would have never guess it.
So this is good to know.

@svenvc
Copy link
Owner

svenvc commented Aug 3, 2022

Yeah, the name is maybe a bit strange, don't forget that URLs are for much more than just file references.

We could add an alias in the future, but maybe that is not really needed.

@kasperosterbye
Copy link
Author

Yeah, the name is maybe a bit strange, don't forget that URLs are for much more than just file references.

So true! But it is also for file references - which is while I suggested FileReference>>asZnUrl to localise the file specific behaviour so it will not bleed into ZnUrl as such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants