ceylon-sdk icon indicating copy to clipboard operation
ceylon-sdk copied to clipboard

ceylon.file: Parent of relative paths

Open lucaswerkmeister opened this issue 10 years ago • 20 comments

When running in /a/b/c, what should the following print?

print(parsePath("foo").parent);
print(parsePath(".").parent);
print(parsePath("..").parent);
  1. foo, ., ..: Current behavior. Simple. But violates the assumption that path.parent.childPath(path.elementPaths.last) == path.
  2. /a/b/c, /a/b, /a: Correct, but ugly:
    • Path.absolute is no longer the only Path member that depends on system state (current working directory)
    • silent transformation from relative to absolute path
  3. ., .., ../..: Also correct, though in a way that could be considered lazy or cheaty. But I think it’s better than 2. Composes well with other paths if you normalize afterwards.
  4. null – change return type to Path? (suggested by @quintesse). Also correct, and allows users to detect when the end of a relative path has been reached. However, they’ll have to deal with the null possibility all the time (even for paths known to be absolute); I suspect that would be pretty annoying.

Note: if we go for 3, then

print(parsePath("foo/..").normalizedPath)

should also print . (currently prints empty string).

lucaswerkmeister avatar Sep 15 '15 14:09 lucaswerkmeister

Option 3. I don't see a justification for option 2's behavior of effectively converting to an absolute path using the cwd, and then re-relatavizing the result. Who's to say that we care about the cwd? (It's a relative path after all.)

jvasileff avatar Sep 15 '15 14:09 jvasileff

Option 4: Path.parent should either return Path? or have some other way to detect that there is no parent path.

Going for options 1 and 3 will make detecting that we've reached the end of our path a lot less nice. For Option 1 we could do:

variable Path path = ...;
while (path.parent != path) {
    // Do stuff
    path = path.parent;
}

For Option 3 that's not possible and we'd probably need to detect that our path contains only a single element. We could add a Path.hasParent but that hardly seems an improvement.

We can't do Option 2, remember that this is just string fiddling we're doing here, we might be creating a Path for a remote server or something.

quintesse avatar Sep 15 '15 16:09 quintesse

We can't do Option 2, remember that this is just string fiddling we're doing here, we might be creating a Path for a remote server or something.

We already have an absolute member. (If it wasn’t clear, I’m talking about ceylon.file::Path, not ceylon.net.uri::Path.)

lucaswerkmeister avatar Sep 15 '15 16:09 lucaswerkmeister

Yes @lucaswerkmeister , but that's you specifically asking for a mapping of that path to your current file system, you can't use that to deal with just any path you have no knowledge about. Even if we're not talking about remote file systems (ceylon.file might not be designed for that, but ceylon.net-uri is not meant for dealing with file systems) you might just be parsing parts of file paths to be concatenated later into a final path.

quintesse avatar Sep 15 '15 16:09 quintesse

absolute is different–it has to use some default base. If you re-imagine all functions that don't have to be absolute as implicitly absolute based on cwd, then you no longer have relative paths at all.

jvasileff avatar Sep 15 '15 16:09 jvasileff

Option 3 could perhaps be handled using the following rules:

  1. path is "." or ends with "/." -> add "."
  2. path is ".." or ends with "/.." -> add "/.."
  3. have a property, similar to .root, that determines if the path is ., let's say .current or .cwd (this would have to do .normalize first)

Then you could at least loop from, eg com/redhat/ceylon to . and use that as the stop condition. Of course if you make a mistake and continue looping you'll get ../../../../../../../../../../../../../../../../../../../../../../../../.. until you run out of memory.

So personally I'm more of a proponent of an explicit stop condition.

Edit: edited the rules (again x2)

quintesse avatar Sep 15 '15 16:09 quintesse

I’d use a very simple implementation for option 3: append .., then normalize. Of course, .normalizedPath needs to do the right thing, including your rules, but it should do that anyways, so IMO we might as well reuse that.

  1. have a property, similar to .root, that determines if the path is ., let's say .current or .cwd

+1, but it should return false for every absolute path, even if it happens to be the current working directory. So I’m not sure if .current or .cwd is the best name for it. (But I can’t think of a better name, either.)

lucaswerkmeister avatar Sep 15 '15 17:09 lucaswerkmeister

append .., then normalize

Well, the thing is that if someone passes foo/bar/../baz/one/two and calls .parent on it I'd expect it to return foo/bar/../baz/one and not foo/baz/one. Otherwise you can say goodbye to doing things like:

path.string[0:path.parent.string.size]

Yes, there might be other and better ways of doing this, but it's completely valid code and something I think people will expect to work.

In general my motto would be: leave people's paths alone unless they specifically ask for the change.

quintesse avatar Sep 15 '15 17:09 quintesse

Good point, not the entire path should be normalized. Perhaps the logic for normalizing a single “transition” could be shared…

lucaswerkmeister avatar Sep 15 '15 17:09 lucaswerkmeister

Oh and the stop condition isn't good enough because it doesn't deal with the root path, so you'd still need to test for path != root. So maybe a property that would return true when root || "." or something like that.

Btw, although I'm defining rules for Options 3 I still favor Options 1 much more (although instead of it returning Path? it might be something else as Path|Empty or whatever.)

quintesse avatar Sep 15 '15 17:09 quintesse

Perhaps the logic for normalizing a single “transition” could be shared…

Well remember that the condition probably has to return true as well for someone who explicitly passes a path like foo/bar/baz/../../.. so you'd have to normalize at that point anyway.

quintesse avatar Sep 15 '15 17:09 quintesse

Btw IMO Java handles this badly, asking for the parent of a path like foo/bar/. gives you foo/bar (the same for foo/bar/... It seems they literally just hack off the last part of the path.

quintesse avatar Sep 15 '15 17:09 quintesse

Perhaps the logic for normalizing a single “transition” could be shared…

Clarification: I meant shared between parent and normalizedPath, not shared in a Ceylon sense :D

lucaswerkmeister avatar Sep 15 '15 17:09 lucaswerkmeister

Btw IMO Java handles this badly, asking for the parent of a path like foo/bar/. gives you foo/bar (the same for foo/bar/... It seems they literally just hack off the last part of the path.

I guess you could justify that by saying that a path is just a series of slash-delimited anythings, and . and .. aren’t special as far as the Path API is concerned. Seems pretty silly to me though.

lucaswerkmeister avatar Sep 15 '15 17:09 lucaswerkmeister

Yes, you can't call it parent if what you really do is removeLastPart ;) And if you type cd ./.. in Linux or cd .\.. in Windows it does exactly what you would expect.

Edit: added cd examples

quintesse avatar Sep 15 '15 17:09 quintesse

Another question: what’s parsePath("a/b/c/./././././././.").parent? Should it be a/b, normalizing most of the path, or a/b/c/././…/./.., giving up and appending ..?

lucaswerkmeister avatar Sep 15 '15 17:09 lucaswerkmeister

See my "motto" :laughing:

But seriously I'd just leave it alone, who knows what kind of reason someone had to pass that specific path. If it's because they have dirty data they can always call .normalize on it.

But I won't mind either if people prefer something like:

  1. path ends with "/." -> remove and start over
  2. path ends with "/.." -> do backtracking until first non-dot(dot) path element (removing any /. as you go) and remove it. Also remove the last /... Start over.
  3. we're left with a path that doesn't end in /. or /.. -> remove the last part
  4. we're left with an empty path -> ".."

But it makes it more complex without a really good reason IMO. I prefer the KISS implementation.

quintesse avatar Sep 15 '15 17:09 quintesse

For the record, I think some variant of the original option 3 would be the most useful thing.

gavinking avatar Oct 02 '15 15:10 gavinking

However, it's worth noting that Path.parent currently just delegates through to java.nio.file.Path.getParent(), which has this comment:

Furthermore, this method does not eliminate special names such as "." and ".." that may be used in some implementations.

So that was by design.

gavinking avatar Oct 02 '15 15:10 gavinking

Anyway this issue is not blocking 1.2.

gavinking avatar Oct 02 '15 15:10 gavinking