https://issues.dlang.org/show_bug.cgi?id=13825

--- Comment #1 from Thibaut CHARLES <cromfr@gmail.com> ---
my current workaround is to call buildNormalizedPath on each argument of relativePath, but there is still an issue if the first path starts with "../"

--

https://issues.dlang.org/show_bug.cgi?id=13825

hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hsteoh@quickfur.ath.cx

--

https://issues.dlang.org/show_bug.cgi?id=13825

Seb <greensunny12@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |bootcamp
                 CC|                            |greensunny12@gmail.com

--

https://issues.dlang.org/show_bug.cgi?id=13825

dechcaudron+dlang.issue.tracking@protonmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |dechcaudron+dlang.issue.tra
                   |                            |cking@protonmail.com

--- Comment #2 from dechcaudron+dlang.issue.tracking@protonmail.com ---
Hi Thibaut,

I'm taking this one. The same behavior is exhibited on the range returned by "asRelativePath". At a first glance, the proposed fix will indeed be to indeed call "buildNormalizedPath" on each of the arguments, but I need to look into memory allocation considerations.

Regarding your last comment:
> but there is still an issue if the first path starts with "../"

Even though the docs do not explicitly mention so, I believe the first argument is expected to be an absolute path (otherwise, you would be changing an out-of-context relative path into another relative path, which looks like a bad idea to me). Perhaps it would be a good idea to add a contract on both parameters (but that is a breaking change that I assume would require a deprecation period, let's see what the maintainers say). Thus, a first argument that starts with ".." would fall out of this category.

Regards,
Dechcaudron

--

https://issues.dlang.org/show_bug.cgi?id=13825

--- Comment #3 from Dechcaudron <dechcaudron+issues.dlang@pm.me> ---
Okay, so I've been mulling on it for the last day. Before I move in to make any changes, here are some questions I'd like to get some discussion going around:

1) Does it make sense that the first argument to the `relativePath` and `asRelativePath` functions can be already relative? Personally I find that to be a bad idea, but you may have a valid counterpoint (please, provide use case).

2) Currently, the "second argument must be an absolute path" claim is exposed in the functions' documentation, and only in `relativePath` is it enforced via throwing an exception (`asRelativePath` merely assumes so). Regardless of the outcome of question 1), would if make sense to remove the exception from `relativePath` and add a check for `isAbsolute(path)` as an input contract? I understand this is a breaking change, so please see 4).

3) The root cause of this issue does not seem to be other than the arguments not being normalized. As the reporter stated, calling `buildNormalizedPath` on both arguments fixes the issue (bar for the first argument starting with "../", see 1)). A quick and dirty fix would be to naively call such function on the arguments within both `relativePath` and `asRelativePath`, but that would be wasteful if the paths are already normalized. It makes sense to me that we add a contract on both input arguments to require they are normalized. As of today, we do not have any `isNormalized` function, but it could be added. Would that make sense? Again, breaking changes, please see 4).

4) Since 2) and 3) both imply breaking changes in the behavior of the function
(or rather, as I am suggesting, the ability to terminate the program via
`assert` when the arguments violate the stated requirements) I understand the
way to go would be to begin a deprecation process. Since the signature of the
function does not need to change, the process is not as straightforward as I'd
like. I would propose the following:
  1. Create the `relativePath(bool useContracts, CaseSensitive cs =
CaseSensitive.osDefault)` template, which `static if`s `useContracts` to
determine whether to define the `relativePath` function with the new behavior
if true, or to define the deprecated function with the legacy behavior if
false. The legacy function would call the template function with `false`, to
trigger the deprecation warning. Proceed the same way with `asRelativePath`.
  2. Once the deprecation period expires, move the definition of the new
behavior with contracts to the legacy function declaration, deprecate the
`useContracts = true` declaration (which by then would forward to the legacy
signature function) and remove the `useContracts = false` declaration.
  3. Once the deprecation period for the template expires, remove it and keep
the legacy signature function with contracts.
  4. My grand-grandkids will close the issue.

Thoughts?

--

https://issues.dlang.org/show_bug.cgi?id=13825

Dechcaudron <dechcaudron+issues.dlang@pm.me> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|nobody@puremagic.com        |dechcaudron+issues.dlang@pm
                   |                            |.me

--

https://issues.dlang.org/show_bug.cgi?id=13825

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P3

--