[Issue 13825] relativePath not handling "." and ".." correctly

d-bugmail at puremagic.com d-bugmail at puremagic.com
Sun Oct 14 10:06:35 UTC 2018


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

--- Comment #3 from Dechcaudron <dechcaudron+issues.dlang at 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?

--


More information about the Digitalmars-d-bugs mailing list