Introduce `AbsolutePath::RelativeTo`

[ForNeVeR]

Currently, this method is available on the LocalPath but not on the AbsolutePath.

[ronimizy]

hi, would like to contribute to this issue implementation
got couple of questions:

1. should there be a methods for both AbsolutePath and LocalPath?
2. result type for either of them should be a LocalPath? (logically it seems for me that is should be a RelativePath but readme says that it is WIP)

[ForNeVeR]

1. should there be a methods for both AbsolutePath and LocalPath?

Yes. So, this task is about adding a method for AbsolutePath.

2. result type for either of them should be a LocalPath?

Yep.

(logically it seems for me that is should be a RelativePath but readme says that it is WIP)

Not quite correct (or, let's say, this is a potential topic for discussion).

You see: in a multi-root file system (such as one that's used by Windows), it's not always possible to construct a relative path between two points. For example, there's no relative path between C:\ and D:\.

In the single-root Unix file system, the relative path always exists though.

So, in my mind, the future design of this would be to introduce an additional method RelativePath? StrictlyRelativeTo(LocalPath/AbsolutePath other) that would return null if it's impossible to construct a relative path.

And leave the current approach of LocalPath RelativeTo(LocalPath/AbsolutePath other) that returns a relative path if possible (in most cases), and an absolute path if impossible (if paths happen to be placed on different file system trees).

[ronimizy]

Yes. So, this task is about adding a method for AbsolutePath.

hm, it seems that i phrased it badly) i meant, should there be an overloads for both AbsolutePath.RelativeTo(AbsolutePath) and AbsolutePath.RelativeTo(LocalPath)?

You see: in a multi-root file system (such as one that's used by Windows), it's not always possible to construct a relative path between two points. For example, there's no relative path between C:\ and D:.

That's a nice observation that i haven't considered (as a non windows user) :)
Should we handle it ourselves with some special logic, or just let the Path.GetRelativePath do its thing?
I would appreciate if you can explain to me the behaviour of this method when paths from different roots are supplied on windows (as i don't have a windows machine available to test it myself). I tried to call it with C:\folder1 and D:\folder2 values and got ../D:\folder2, but as i do it on macOS, this result is probably incorrect when talking about execution on windows machine.

So, in my mind, the future design of this would be to introduce an additional method RelativePath? StrictlyRelativeTo(LocalPath/AbsolutePath other) that would return null if it's impossible to construct a relative path.

mb TryGet approach would be nicer?

[ForNeVeR]

should there be an overloads for both AbsolutePath.RelativeTo(AbsolutePath) and AbsolutePath.RelativeTo(LocalPath)?

No, let's make the LocalPath::RelativeTo method accept a LocalPath, and AbsolutePath::RelativeTo method accept a AbsolutePath. Makes more sense like this to me.

You see: in a multi-root file system (such as one that's used by Windows), it's not always possible to construct a relative path between two points. For example, there's no relative path between C:\ and D:.

That's a nice observation that i haven't considered (as a non windows user) :) Should we handle it ourselves with some special logic, or just let the Path.GetRelativePath do its thing?

I imagine we'll get our own variant of Path.GetRelativePath sooner or later, but for now Path.GetRelativePath would work well for this purpose. It implements the behavior I outlined: sometimes it may return an absolute path.

I would appreciate if you can explain to me the behaviour of this method when paths from different roots are supplied on windows (as i don't have a windows machine available to test it myself). I tried to call it with C:\folder1 and D:\folder2 values and got ../D:\folder2, but as i do it on macOS, this result is probably incorrect when talking about execution on windows machine.

Yeah, Path.GetRelativePath has a platfom-dependent behavior. I.e. Path.GetRelativePath(@"C:\", @"D:\") returns a @"D:\" on Windows.

So, in my mind, the future design of this would be to introduce an additional method RelativePath? StrictlyRelativeTo(LocalPath/AbsolutePath other) that would return null if it's impossible to construct a relative path.

mb TryGet approach would be nicer?

Yeah, perhaps. Naming and behavior of this method will be a topic of the future, when we'll get a RelativePath.

[ronimizy]

hm, i think that i encountered a bug in PathStrings while writing a test for added functionality

test itself is pretty simple, the expected part is taken from Path.GetRelativePath output.

[Theory]
[InlineData("/usr/bin", "/etc/bin", "../../usr/bin")]
public void RelativeToReturnsCorrectRelativePath(string from, string to, string expected)
{
    if (OperatingSystem.IsWindows()) return;

    var fromPath = new AbsolutePath(from);
    var toPath = new AbsolutePath(to);

    LocalPath relativePath = fromPath.RelativeTo(toPath);

    Assert.Equal(expected, relativePath.Value);
}

but the actual value is "usr/bin" which is not correct because "../../usr/bin" is not equivalent to it

[ForNeVeR]

Can confirm, it's broken.

It supports ../foo, but when it sees ../../, it thinks it should go one level up from ../ which is an empty path.

So, an even number of .. gives an empty path, and an odd number of .. gives one ...

This is a separate issue, but I can see how it blocks you from adding tests for relative paths. Will check soon.

[ronimizy]

i going to look at the method, to get some familiarity with it at least :)

[ronimizy]

I would like to point out as well, that there is an error in readme and in one of the PathStrings.Normalize samples.
"foo/../." should resolve not in "foo" but in "."

[ForNeVeR]

Yeah, I'll fix that as well.