rust-vfs
rust-vfs copied to clipboard
Use camino paths or support non-UTF-8 paths?
Hi there -- thanks for writing this crate! I'd like to use it for some of my own crates that interact with the file system.
I'm wondering if you think using camino for paths would be interesting -- since you already restrict yourself to paths that can be encoded as UTF-8.
Alternatively, you could also go in the direction of supporting non-UTF-8 paths by using Path
, OsString
etc.
(I also noticed some path handling that might be funky on Windows, such as potentially not handling backslash path separators properly -- either camino
or std::path::Path
will help with that since they internally handle those separators.)
(I'm happy to do this, and also to fix up Windows support.)
Having looked at the impl a bit, I think I'm going to use camino to keep things simple. There's definitely some use cases that may benefit from non-UTF-8 path support, but they may wish to have a separate impl for this.
Hi, thanks for your comments and suggestions. The primary use case for this crate is abstracting over application files or assets. This means that the paths are under the control of the application and should all be UTF-8. The API currently only supports UTF-8 paths. I am not sure what benefits camino would offer. As far as I can tell, camino "frontloads" non-Unicode path errors, which is something that rust-vfs should already do out of the box.
Thanks! The main thing camino
would provide is clearer handling around paths, and proper Windows path handling.
For example:
- https://docs.rs/vfs/0.5.1/vfs/filesystem/trait.FileSystem.html currently takes
&str
-- it could take&Utf8Path
instead. - https://docs.rs/vfs/0.5.1/vfs/path/struct.VfsPath.html#method.join currently takes
&str
-- it could take&Utf8Path
(orimpl AsRef<Utf8Path>
) instead. - Internal implementations like https://docs.rs/vfs/0.5.1/src/vfs/impls/overlay.rs.html#72 currently look for
/
manually -- they could use the path methods instead, which would also provide Windows path handling for free.
Let me know what you think!
Thanks for the examples!
I feel that something like root.join("foo").
is more ergonomic than root.join(camino::Utf8Path::new("foo"))
. I also think that if anyone wants to use camino they could either do something like root.join(camino::Utf8Path::new("foo").as_ref())
or provide an extension trait that takes a &Utf8Path
Having the VfsPath
methods take impl AsRef<str>
instead of &str
might be the another solution that could work. I have to admit I am a bit hesitant to risk longer compile times due to method monomorphization caused by impl Parameters. I might to be falling into the "premature optimization" trap, though...
Regarding using camino internally for path handling sounds like an idea, although again I am kind of reluctant to add a dependency for an rfind. A better solution might be to refactor that logic out into an internal function or similar.
Windows path handling should work fine with the current setup. I am not currently seeing any issues on my Windows development machine. Is there anything in particular that stood out?
Thanks again for your feedback - much appreciated!
I feel that something like root.join("foo"). is more ergonomic than root.join(camino::Utf8Path::new("foo")). I also think that if anyone wants to use camino they could either do something like root.join(camino::Utf8Path::new("foo").as_ref()) or provide an extension trait that takes a &Utf8Path
Yeah I think impl AsRef<Utf8Path>
would work best for that use case -- that's what camino's join()
takes as well. That works with strings as well as paths, so it's super transparent.
Having the VfsPath methods take impl AsRef
instead of &str might be the another solution that could work. I have to admit I am a bit hesitant to risk longer compile times due to method monomorphization caused by impl Parameters. I might to be falling into the "premature optimization" trap, though...
I haven't noticed a real difference tbh, and I tend to care about compile times.
Regarding using camino internally for path handling sounds like an idea, although again I am kind of reluctant to add a dependency for an rfind. A better solution might be to refactor that logic out into an internal function or similar.
Totally understood. I'm pretty biased here since I'm one of the maintainers of camino, but people have generally really appreciated the library and it already has >800k downloads and >30 dependents on crates.io in just a few months.
Windows path handling should work fine with the current setup. I am not currently seeing any issues on my Windows development machine. Is there anything in particular that stood out?
Most of the internal path processing code uses forward slashes -- in some cases people on Windows might pass in backslash-separated paths (I have a use case which does that, in fact).
Thanks again for engaging :)