Help in handling weird join behavior

[vitiral]

@soc left a comment here and I wrote a quick script to demonstrate the problem:

Joining: a with b
Result : a/b
Joining: a/ with b
Result : a/b
Joining: /a with b
Result : /a/b
Joining: a with /b
Result : /b
Joining: /a with /b
Result : /b

This is indeed bizare and an issue I think path_abs might be able to help with.

The issue with "joining not compiling" is that the API for most path operations takes in AsRef<Path>. I think you would agree that both RelativePath and AbsolutePath should be AsRef<Path>, right? This makes the issue harder as I'm not sure of a way to disallow a certain type in rust (please let me know if there is a way).

[vitiral]

I think a really simple solution is to simply strip the Root components in the second item, before passing it to rust. I would like to know more about why rust chose to do it this way first though.

Maybe the reason is because stripping a prefix is much less clear. Joining \a with \\?\C:\b\c is not so clearly equal to a/b/c. It's hard to decide what to do in that case except return an error, which join cannot do.

[vitiral]

The simplest thing would be to just do what the user told you to: \a with \\?\C:\b\c would just be \a\\?\C:\b\c. Obviously this is an invalid path, but shrug.

[soc]

The issue with "joining not compiling" is that the API for most path operations takes in AsRef.

I think join shouldn't, because it is not meaningful to join an absolute path to something. So don't take Path, only a PathRel, or something.

[soc]

Ammonite Ops gets a lot of things right (ignore the DSL stuff).

[vitiral]

The issue is that I don't think I can define AsRef<PathRel> for &str or String, which is basically required to have ergonomics here.

How do I get the following to work _without accepting either &str or AsRef<Path>?

abs.join("foo")
abs.join(Path::new("foo"))
abs.join(PathRel::new("foo"))

Or are you saying that there should be runtime errors when attempting to create PathRel with a root directory?

[vitiral]

Alternative: there could be PathAbs::join_abs(&self, PathRel) which only accepts a relative path. Then we could have a macro path_rel!("foo/bar") which just issues a compiler error if the string starts with /.

So

abs.join_abs(path_rel!("foo"))
abs.join_abs(path_rel!("/foo")) // <---- compiler error

[vitiral]

This would technically be a breaking change though :(

Oh well, very few ppl are probably using join_abs.

[soc]

I'm not sure I have fully understood this specific issue. Basically the argument of join should always considered to be relative, and PathRel::new should ignore leading slashes completely, because you have already specified by writing down its name that you want a relative path.

Whether supporting conversions from types which don't make that distinction makes sense ... I think it would be necessary to write more example code, or look at existing projects to figure out whether the convenience is worth it.

My current impression is that Rust's Path/PathBuf is too broken to allow implicit conversions, and people should convert their existing Rust paths explicitly (or rather replace them with e. g. your crate altogether).

[vitiral]

what is your opinion of Path::new(r"\a").join(r"\\?\C:\b\c")? What should that return?

[soc]

I think this is a good case for not accepting raw strings, and having different PathRel::from methods instead, which treat absolute paths in various way (bailing out, reinterpreting as relative, ...).

Also, I think it makes sense to only accept a single format for paths at this level, and sort out the given string means only at the last possible point, when passing things to native APIs.

[vitiral]

But PathRel::from_str(&str) will return a Result right? That is a slightly annoying API but I guess it is tenable. What do you think about the macro to check a const &str path at compile time?

[soc]

Mhh, I don't have an opinion on that; my position is that Rust's macros are rather terrible from a cost/benefit point of view and hope that they get fixed. So I'm avoiding introducing new ones myself.

[vitiral]

Okay, I'm not totally sure on this issue. I'm very tempted to just join it as the user requests and then validate that it is a "valid" path by not allowing root or prefixes to be in the middle and returning an error if they are.

fn join_abs(&self, right: AsRef<Path>) -> path_abs::Result<PathAbs> {
}

This already returns a PathAbs so I can return an error if I need to, but it doesn't require the user to make the right separately and deal with any related errors.

Adding a PathRel would probably be useful to do this kind of validation up-front, but I don't think the typesystem adds that many ergonomics here. If we did that there could maybe be a PathAbs::join_rel(&self, right: PathRel) -> PathAbs as well which accomplishes what you want, but I'm not sold on it's value in most cases.

[ExpHP]

The point of the standard library's behavior is that lhs.join(rhs) returns "what rhs means to something situated in lhs." (or likewise, if you were to change directory to lhs, what does rhs refer to?)

Use cases of this are all over the place in CLI args and config files, but here's one you might be familiar with:

# semantics of CARGO_MANIFEST_DIR.join(path)
[dependencies]
a-local-dep = { path = "my/local-dep" } # relative to Cargo.toml
another-dep = { path = "/home/lampam/rust/another-dep" } # absolute

---

Before it was added to path_abs, I used to implement path.absolute() as current_dir()?.join(path).

[vitiral]

The next version includes the concat and append functions, which handle this behavior.

[vitiral]

@ExpHP thanks for explaining the logic, that is actually really helpful! I might open a PR to add that reasoning to the std docs some day :D