fix nim-lang#13455 ; joinPath(a,b) now honors trailing slashes in b (…

…or a if b = "")

changelog.md

@@ -19,6 +19,7 @@
 - Global variable `lc` has been removed from sugar.nim.
 - `distinctBase` has been moved from sugar.nim to typetraits and now implemented as
   compiler type trait instead of macro. `distinctBase` in sugar module is now deprecated.
+- `joinPath(a,b)` now honors trailing slashes in `b` (or `a` if `b` = "")
 
 ### Breaking changes in the compiler
 

lib/pure/os.nim

@@ -120,14 +120,21 @@ proc normalizePathEnd(path: string, trailingSep = false): string =
 since((1, 1)):
   export normalizePathEnd
 
+template endsWith(a: string, b: set[char]): bool =
+  a.len > 0 and a[^1] in b
+
+proc joinPathImpl(result: var string, state: var int, tail: string) =
+  let trailingSep = tail.endsWith({DirSep, AltSep}) or tail.len == 0 and result.endsWith({DirSep, AltSep})
+  addNormalizePath(tail, result, state, DirSep)
+  normalizePathEnd(result, trailingSep=trailingSep)
+
 proc joinPath*(head, tail: string): string {.
   noSideEffect, rtl, extern: "nos$1".} =
   ## Joins two directory names to one.
   ##
-  ## If `head` is the empty string, `tail` is returned. If `tail` is the empty
-  ## string, `head` is returned with a trailing path separator. If `tail` starts
-  ## with a path separator it will be removed when concatenated to `head`.
-  ## Path separators will be normalized.
+  ## returns normalized path concatenation of `head` and `tail`, preserving
+  ## whether or not `tail` has a trailing slash (or, if tail if empty, whether
+  ## head has one).
   ##
   ## See also:
   ## * `joinPath(varargs) proc <#joinPath,varargs[string]>`_
@@ -146,11 +153,8 @@ proc joinPath*(head, tail: string): string {.
 
   result = newStringOfCap(head.len + tail.len)
   var state = 0
-  addNormalizePath(head, result, state, DirSep)
-  if result.len != 0 and result[^1] notin {DirSep, AltSep} and tail.len == 0:
-    result.add DirSep
-  else:
-    addNormalizePath(tail, result, state, DirSep)
+  joinPathImpl(result, state, head)
+  joinPathImpl(result, state, tail)
   when false:
     if len(head) == 0:
       result = tail
@@ -189,7 +193,7 @@ proc joinPath*(parts: varargs[string]): string {.noSideEffect,
   result = newStringOfCap(estimatedLen)
   var state = 0
   for i in 0..high(parts):
-    addNormalizePath(parts[i], result, state, DirSep)
+    joinPathImpl(result, state, parts[i])
 
 proc `/`*(head, tail: string): string {.noSideEffect.} =
   ## The same as `joinPath(head, tail) proc <#joinPath,string,string>`_.

tests/stdlib/tos.nim

@@ -353,14 +353,38 @@ block ospaths:
     doAssert relativePath(r"\\foo\bar\baz.nim", r"\foo") == r"\\foo\bar\baz.nim"
     doAssert relativePath(r"c:\foo.nim", r"\foo") == r"c:\foo.nim"
 
-  doAssert joinPath("usr", "") == unixToNativePath"usr/"
+  doAssert joinPath("usr", "") == unixToNativePath"usr"
   doAssert joinPath("", "lib") == "lib"
   doAssert joinPath("", "/lib") == unixToNativePath"/lib"
   doAssert joinPath("usr/", "/lib") == unixToNativePath"usr/lib"
-  doAssert joinPath("", "") == unixToNativePath""
-  doAssert joinPath("/" / "") == unixToNativePath"/"
+  doAssert joinPath("", "") == unixToNativePath"" # issue #13455
+  doAssert joinPath("", "/") == unixToNativePath"/"
+  doAssert joinPath("/", "/") == unixToNativePath"/"
+  doAssert joinPath("/", "") == unixToNativePath"/"
+  doAssert joinPath("/" / "") == unixToNativePath"/" # weird test case...
   doAssert joinPath("/", "/a/b/c") == unixToNativePath"/a/b/c"
   doAssert joinPath("foo/","") == unixToNativePath"foo/"
+  doAssert joinPath("foo","") == unixToNativePath"foo"
+  doAssert joinPath("foo/","abc") == unixToNativePath"foo/abc"
+  doAssert joinPath("foo//./","abc/.//") == unixToNativePath"foo/abc/"
+  doAssert joinPath("foo","abc") == unixToNativePath"foo/abc"
+  doAssert joinPath("","abc") == unixToNativePath"abc"
+
+  doAssert joinPath("gook/.","abc") == unixToNativePath"gook/abc"
+
+  # controversial: inconsistent with `joinPath("gook/.","abc")`
+  # on linux, `./foo` and `foo` are treated a bit differently for executables
+  # but not `./foo/bar` and `foo/bar`
+  doAssert joinPath(".", "/lib") == unixToNativePath"./lib"
+  doAssert joinPath(".","abc") == unixToNativePath"./abc"
+
+  # cases related to issue #13455
+  doAssert joinPath("foo", "", "") == "foo"
+  doAssert joinPath("foo", "") == "foo"
+  doAssert joinPath("foo/", "") == "foo/"
+  doAssert joinPath("foo/", ".") == "foo"
+  doAssert joinPath("foo", "./") == "foo/"
+  doAssert joinPath("foo", "", "bar/") == "foo/bar/"
 
 block getTempDir:
   block TMPDIR: