Tracking issue for Path::relative_from stabilization

[brson]

Path::relative_from removes the common prefix from a path, e.g. /foo/bar relative_from /foo yields bar.

The old path_relative_from would calculate the relative path from two points on the file system, e.g. the relative path from /foo/bar to /foo/baz/qux is ../baz/qux. This is a useful operation.

Nominating because I think either relative_from is named wrong or is not useful.

[brson]

cc #23283

[aturon]

cc me

[aturon]

Note that the reason the new semantics was adopted has to do with symlinks (the same reason we do not normalize ..). We'll need to discuss this further, but for now it's plausible to leave this function unstable.

[pnkfelix]

Function is not yet stable, so this need not be milestoned. P-high, not 1.0.

[lilyball]

Path::relative_from made more sense back when we auto-normalized paths. The idea was if you joined the result to the first path, you'd get the second path. Since we don't normalize like that anymore, that behavior doesn't work.

It's still a potentially useful operation, but because of symlinks, there are two different behaviors that might be desired:

1. The current behavior where joining the result onto the first path unambiguously refers to the same thing the second path does, even if there's symlinks (which basically means base needs to be a prefix of self)
2. The old behavior where the result can start with ../ components. Symlinks mean traversing the base path and then traversing the returned relative path may not put you in the same directory that traversing theself` path does. But this operation is useful when either you're working with a path-based system that doesn't care about symlinks, or you've already resolved symlinks in the paths you're working with.

Based on that I think it's reasonable to provide both implementations. The former might want to be called something like strip_prefix() instead. the latter could continue to be called relative_from.

[withoutboats]

Nominating because I think either relative_from is named wrong or is not useful.

Just noting that I have found this function useful. I think it would probably make more sense to name it something like strip_prefix though.

[rgardner]

I too have found this function useful. What has to happen to stabilize this?

[JanLikar]

This should be stabilized or a similar function should be proposed.

[qmx]

since @mbrubeck pointed me here for my usecase too, here it comes:

let path = Path::new("/might/be/something/a");
assert_eq!(path.relative_from(Path::new("/might/be")), Path::new("something/a"));

I'm more than fine on renaming it to strip_prefix.

[aturon]

👍 for the strip_prefix proposal (or something like it). I'd have no problem stabilizing the function with such a clear name.

re: timing, after the 1.6 beta is cut (next week) we will triage all unstable APIs and select a batch for 1.7 stabilization. I'll make sure this one is up for discussion.

[huonw]

Maybe it could be trim instead of strip, to match the methods on str?

[alexcrichton]

🔔 This issue is now entering its final comment period for stabilization 🔔

The libs team likes the name strip_prefix, but the trim suggestion is also tempting!

[JanLikar]

str's trim strips both leading and trailing whitespaces, so I prefer strip.

[retep998]

Just pointing out that Windows treats .. as just removing the last component and completely disregards symlinks. Therefore relative_from or whatever it gets renamed to should be okay with .. on Windows at least.

[rgardner]

👍 to strip_prefix. It's more verbose than trim, but more clearly defines the operation being performed.

let path = Path::new("/home/user/code");
let home = env::home_dir().unwrap();
path.relative_from(home);
path.strip_prefix(home);
path.trim(home);

[huonw]

I wonder if this should return Result<&Path, StripPrefixError> instead of Option<&Path>. I say this because strip_prefix sounds quite definite (it is definitely stripping the prefix), and hence it is an error (rather than just a "meh, it happens") if it doesn't work.

[qmx]

+1 to explicit error

[withoutboats]

What would be the variants of StripPrefixError?

[huonw]

I was thinking struct StripPrefixError { _private: () } (possibly with more details about the input paths, I don't know), like, for instance, ParseBoolError.

[BurntSushi]

I like strip_prefix. Returning a Result seems prudent.

[aturon]

I'm on board with both the renaming and using Result here.

[alexcrichton]

The libs team discussed this during triage yesterday and the decision was to stabilize with the name strip_prefix and the tweak of returning a Result.

[softprops]

Is strip_prefix a different name for the same behavior found on strings?
https://doc.rust-lang.org/std/string/struct.String.html#method.trim_left_matches

[SimonSapin]

@softprops str::trim_left_matches zero or more instances of the pattern, as many as possible. Path::strip_prefix removes exactly one or returns Err if there isn’t a match. It also only cuts at path separators.