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

Have VfsPath::join() treat paths more unix-like #42

Closed
Property404 opened this issue Nov 26, 2022 · 3 comments · Fixed by #45
Closed

Have VfsPath::join() treat paths more unix-like #42

Property404 opened this issue Nov 26, 2022 · 3 comments · Fixed by #45

Comments

@Property404
Copy link
Contributor

Property404 commented Nov 26, 2022

Example, these are invalid:
path.root().join("..") - this should just return the root
path.join("/") - This should also return the root (this is how Path::join works)
path.join("a///b") - This should be the same as path.join("a/b") (this is also how Path::join works)

Do you think it's a good idea to change the behavior of VfsPath::join()? It would certainly be more convenient for what I'm working on.

I implemented the first suggestion in #41

Let me know and I can submit more PRs

@manuel-woelker
Copy link
Owner

Hi, thanks for the suggestions!

  • I commented on VfsPath::join(): treat '..' at root as valid #41 regarding path.root().join("..").
  • path.join("/") etc - having absolute paths be based on the root should be fine I think, looking forward to a PR!
  • path.join("a///b") - this one is a bit tricky, my gut instinct would be consider this as folders whose names happen to be the empty string. But this is not how OSes seem to treat this. I haven't found any hints as to why path separators are collapsed this way (other than preventing empty strings as filenames). I realize that this deviation from the unix standard might cause confusion, but OTOH I am still trying to figure out scenarios in which this handling of multiple slashes would be beneficial.

@Property404
Copy link
Contributor Author

Property404 commented Nov 30, 2022

In general, I think the big advantage of Unix collapsing slashes is that if you're concatenating two strings representing paths, you don't have to remember if the first one ended with a slash or not. Eg:

const DIRECTORY: &str = "usr/etc/wobbles/"; // or "usr/etc/wobble"
...
// Did DIRECTORY have a slash??
path.join(DIRECTORY+"/wobble.file")

I rely on this behavior a lot in other situations. Granted, in this particular example, you could do path.join(DIRECTORY);path.join("wobble.file"). But this could come up at higher levels of abstraction, like a shell, which is the primary reason for me wanting these changes

FWIW, both Unix and DOS paths treat consecutive slashes this way

@manuel-woelker
Copy link
Owner

Good points, align VFS behaviour with what OSes are doing sounds like the right thing to do. 👍

Feel free to add PRs regarding these points.

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

Successfully merging a pull request may close this issue.

2 participants