fixes for stratify.path (#187)

d3 · Dec 9, 2021 · 296e8c4 · 296e8c4
1 parent 7ca356e
commit 296e8c4
Show file tree

Hide file tree

Showing 2 changed files with 146 additions and 23 deletions.
diff --git a/src/stratify.js b/src/stratify.js
@@ -112,34 +112,34 @@ export default function() {
   return stratify;
 }
 
-// To normalize a path, we coerce to a string, strip trailing slash if present,
-// and add leading slash if missing. This requires counting the number of
-// preceding backslashes which may be used to escape the forward slash: an odd
-// number indicates an escaped forward slash.
+// To normalize a path, we coerce to a string, strip the trailing slash if any
+// (as long as the trailing slash is not immediately preceded by another slash),
+// and add leading slash if missing.
 function normalize(path) {
   path = `${path}`;
-  let i = path.length - 1;
-  if (path[i] === "/") {
-    let k = 0;
-    while (i > 0 && path[--i] === "\\") ++k;
-    if ((k & 1) === 0) path = path.slice(0, -1);
-  }
+  let i = path.length;
+  if (slash(path, i - 1) && !slash(path, i - 2)) path = path.slice(0, -1);
   return path[0] === "/" ? path : `/${path}`;
 }
 
 // Walk backwards to find the first slash that is not the leading slash, e.g.:
 // "/foo/bar" ⇥ "/foo", "/foo" ⇥ "/", "/" ↦ "". (The root is special-cased
-// because the id of the root must be a truthy value.) The slash may be escaped,
-// which again requires counting the number of preceding backslashes. Note that
-// normalized paths cannot end with a slash except for the root.
+// because the id of the root must be a truthy value.)
 function parentof(path) {
   let i = path.length;
-  while (i > 2) {
-    if (path[--i] === "/") {
-      let j = i, k = 0;
-      while (j > 0 && path[--j] === "\\") ++k;
-      if ((k & 1) === 0) break;
-    }
+  if (i < 2) return "";
+  while (--i > 1) if (slash(path, i)) break;
+  return path.slice(0, i);
+}
+
+// Slashes can be escaped; to determine whether a slash is a path delimiter, we
+// count the number of preceding backslashes escaping the forward slash: an odd
+// number indicates an escaped forward slash.
+function slash(path, i) {
+  if (path[i] === "/") {
+    let k = 0;
+    while (i > 0 && path[--i] === "\\") ++k;
+    if ((k & 1) === 0) return true;
   }
-  return path.slice(0, i < 3 ? i - 1 : i);
+  return false;
 }
diff --git a/test/stratify-test.js b/test/stratify-test.js
@@ -464,6 +464,129 @@ it("stratify.path(path) returns the root node", () => {
   });
 });
 
+it("stratify.path(path) correctly handles single-character folders", () => {
+  const root = stratify().path(d => d.path)([
+    {path: "/"},
+    {path: "/d"},
+    {path: "/d/123"}
+  ]);
+  assert(root instanceof hierarchy);
+  assert.deepStrictEqual(noparent(root), {
+    id: "/",
+    depth: 0,
+    height: 2,
+    data: {path: "/"},
+    children: [
+      {
+        id: "/d",
+        depth: 1,
+        height: 1,
+        data: {path: "/d"},
+        children: [
+          {
+            id: "/d/123",
+            depth: 2,
+            height: 0,
+            data: {path: "/d/123"}
+          }
+        ]
+      }
+    ]
+  });
+});
+
+it("stratify.path(path) correctly handles empty folders", () => {
+  const root = stratify().path(d => d.path)([
+    {path: "/"},
+    {path: "//"},
+    {path: "///"}
+  ]);
+  assert(root instanceof hierarchy);
+  assert.deepStrictEqual(noparent(root), {
+    id: "/",
+    depth: 0,
+    height: 2,
+    data: {path: "/"},
+    children: [
+      {
+        id: "//",
+        depth: 1,
+        height: 1,
+        data: {path: "//"},
+        children: [
+          {
+            id: "///",
+            depth: 2,
+            height: 0,
+            data: {path: "///"}
+          }
+        ]
+      }
+    ]
+  });
+});
+
+it("stratify.path(path) correctly handles single-character folders with trailing slashes", () => {
+  const root = stratify().path(d => d.path)([
+    {path: "/"},
+    {path: "/d/"},
+    {path: "/d/123/"}
+  ]);
+  assert(root instanceof hierarchy);
+  assert.deepStrictEqual(noparent(root), {
+    id: "/",
+    depth: 0,
+    height: 2,
+    data: {path: "/"},
+    children: [
+      {
+        id: "/d",
+        depth: 1,
+        height: 1,
+        data: {path: "/d/"},
+        children: [
+          {
+            id: "/d/123",
+            depth: 2,
+            height: 0,
+            data: {path: "/d/123/"}
+          }
+        ]
+      }
+    ]
+  });
+});
+
+it("stratify.path(path) correctly handles imputed single-character folders", () => {
+  const root = stratify().path(d => d.path)([
+    {path: "/"},
+    {path: "/d/123"}
+  ]);
+  assert(root instanceof hierarchy);
+  assert.deepStrictEqual(noparent(root), {
+    id: "/",
+    depth: 0,
+    height: 2,
+    data: {path: "/"},
+    children: [
+      {
+        id: "/d",
+        depth: 1,
+        height: 1,
+        data: null,
+        children: [
+          {
+            id: "/d/123",
+            depth: 2,
+            height: 0,
+            data: {path: "/d/123"}
+          }
+        ]
+      }
+    ]
+  });
+});
+
 it("stratify.path(path) allows slashes to be escaped", () => {
   const root = stratify().path(d => d.path)([
     {path: "/"},
@@ -650,9 +773,9 @@ it("stratify.path(path) implicitly trims trailing slashes", () => {
   });
 });
 
-it("stratify.path(path) trims at most one trailing slash", () => {
+it("stratify.path(path) does not trim trailing slashes preceded by a slash", () => {
   const root = stratify().path(d => d.path)([
-    {path: "/aa///"},
+    {path: "/aa//"},
     {path: "/b"}
   ]);
   assert(root instanceof hierarchy);
@@ -684,7 +807,7 @@ it("stratify.path(path) trims at most one trailing slash", () => {
                 id: "/aa//",
                 depth: 3,
                 height: 0,
-                data: {path: "/aa///"},
+                data: {path: "/aa//"},
               }
             ]
           }