From f82235962a4fd4af744aec28e4d858f8ac67f146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 7 Sep 2023 11:25:06 +0200 Subject: [PATCH 01/10] ignore / if the delimiter is something else (note: I don't think this is the correct fix! But at least it gives a unit test) closes #1849 --- src/transforms/tree.js | 4 ++-- test/output/treeDelimiter.svg | 41 +++++++++++++++++++++++++++++++++++ test/plots/index.ts | 1 + test/plots/tree-delimiter.ts | 21 ++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 test/output/treeDelimiter.svg create mode 100644 test/plots/tree-delimiter.ts diff --git a/src/transforms/tree.js b/src/transforms/tree.js index 44d42cf474..7095b7a51f 100644 --- a/src/transforms/tree.js +++ b/src/transforms/tree.js @@ -169,7 +169,7 @@ function nodeData(field) { function normalizer(delimiter = "/") { return `${delimiter}` === "/" ? (P) => P // paths are already slash-separated - : (P) => P.map(replaceAll(delimiter, "/")); // TODO string.replaceAll when supported + : (P) => P.map(replaceAll("/", "\\/")).map(replaceAll(delimiter, "/")); // TODO string.replaceAll when supported } function replaceAll(search, replace) { @@ -272,7 +272,7 @@ function parentValue(evaluate) { function nameof(path) { let i = path.length; while (--i > 0) if (slash(path, i)) break; - return path.slice(i + 1); + return path.slice(i + 1).replaceAll("\\/", "/"); } // Slashes can be escaped; to determine whether a slash is a path delimiter, we diff --git a/test/output/treeDelimiter.svg b/test/output/treeDelimiter.svg new file mode 100644 index 0000000000..119ce5729d --- /dev/null +++ b/test/output/treeDelimiter.svg @@ -0,0 +1,41 @@ + + + + + + + + + + + /foo + /foo/bar + /foo/baz + /foo/bar/http:\/\/www.example.com + /foo/bar/https:\/\/www.example.com\/posts\/1 + /foo/baz/https:\/\/www.example.com\/posts\/2 + + + http://www.example.com/foo/bar/http:\/\/www.example.com + https://www.example.com/posts/1/foo/bar/https:\/\/www.example.com\/posts\/1 + https://www.example.com/posts/2/foo/baz/https:\/\/www.example.com\/posts\/2 + + + foo/foo + bar/foo/bar + baz/foo/baz + + \ No newline at end of file diff --git a/test/plots/index.ts b/test/plots/index.ts index 35ded36808..9998e92373 100644 --- a/test/plots/index.ts +++ b/test/plots/index.ts @@ -298,6 +298,7 @@ export * from "./title.js"; export * from "./traffic-horizon.js"; export * from "./travelers-covid-drop.js"; export * from "./travelers-year-over-year.js"; +export * from "./tree-delimiter.js"; export * from "./uniform-random-difference.js"; export * from "./untyped-date-bin.js"; export * from "./us-congress-age-color-explicit.js"; diff --git a/test/plots/tree-delimiter.ts b/test/plots/tree-delimiter.ts new file mode 100644 index 0000000000..f71bc13364 --- /dev/null +++ b/test/plots/tree-delimiter.ts @@ -0,0 +1,21 @@ +import * as Plot from "@observablehq/plot"; + +export async function treeDelimiter() { + return Plot.plot({ + axis: null, + height: 100, + margin: 10, + marginLeft: 40, + marginRight: 190, + marks: [ + Plot.tree( + [ + "foo;bar;http://www.example.com", + "foo;bar;https://www.example.com/posts/1", + "foo;baz;https://www.example.com/posts/2" + ], + {delimiter: ";"} + ) + ] + }); +} From 22fb3f101613b730006f3602e77cef15e2d4a9fa Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 7 Sep 2023 08:48:20 -0700 Subject: [PATCH 02/10] avoid string.replaceAll --- src/transforms/tree.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/transforms/tree.js b/src/transforms/tree.js index 7095b7a51f..43f3d1bc5e 100644 --- a/src/transforms/tree.js +++ b/src/transforms/tree.js @@ -169,7 +169,15 @@ function nodeData(field) { function normalizer(delimiter = "/") { return `${delimiter}` === "/" ? (P) => P // paths are already slash-separated - : (P) => P.map(replaceAll("/", "\\/")).map(replaceAll(delimiter, "/")); // TODO string.replaceAll when supported + : (P) => P.map(slashEscape).map(replaceAll(delimiter, "/")); // TODO string.replaceAll when supported +} + +function slashEscape(string) { + return string.replace(/\//g, "\\/"); +} + +function slashUnescape(string) { + return string.replace(/\\\//g, "/"); } function replaceAll(search, replace) { @@ -272,7 +280,7 @@ function parentValue(evaluate) { function nameof(path) { let i = path.length; while (--i > 0) if (slash(path, i)) break; - return path.slice(i + 1).replaceAll("\\/", "/"); + return slashUnescape(path.slice(i + 1)); } // Slashes can be escaped; to determine whether a slash is a path delimiter, we From 5aea07dd3b64c9601981dd044e9f2f7079622992 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 7 Sep 2023 09:02:59 -0700 Subject: [PATCH 03/10] add delimiter test --- test/output/treeDelimiter.svg | 41 ----------------------------------- test/plots/tree-delimiter.ts | 33 ++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 45 deletions(-) delete mode 100644 test/output/treeDelimiter.svg diff --git a/test/output/treeDelimiter.svg b/test/output/treeDelimiter.svg deleted file mode 100644 index 119ce5729d..0000000000 --- a/test/output/treeDelimiter.svg +++ /dev/null @@ -1,41 +0,0 @@ - - - - - - - - - - - /foo - /foo/bar - /foo/baz - /foo/bar/http:\/\/www.example.com - /foo/bar/https:\/\/www.example.com\/posts\/1 - /foo/baz/https:\/\/www.example.com\/posts\/2 - - - http://www.example.com/foo/bar/http:\/\/www.example.com - https://www.example.com/posts/1/foo/bar/https:\/\/www.example.com\/posts\/1 - https://www.example.com/posts/2/foo/baz/https:\/\/www.example.com\/posts\/2 - - - foo/foo - bar/foo/bar - baz/foo/baz - - \ No newline at end of file diff --git a/test/plots/tree-delimiter.ts b/test/plots/tree-delimiter.ts index f71bc13364..f5ae067f7f 100644 --- a/test/plots/tree-delimiter.ts +++ b/test/plots/tree-delimiter.ts @@ -3,19 +3,44 @@ import * as Plot from "@observablehq/plot"; export async function treeDelimiter() { return Plot.plot({ axis: null, - height: 100, + height: 120, margin: 10, marginLeft: 40, marginRight: 190, marks: [ Plot.tree( [ - "foo;bar;http://www.example.com", - "foo;bar;https://www.example.com/posts/1", - "foo;baz;https://www.example.com/posts/2" + "foo;bar;https://example.com", + "foo;bar;https://example.com/posts/1", + "foo;baz;https://example.com/posts/2", + "foo;bar\\;baz;https://example2.com", // “bar;baz” should be a single node + "foo;bar/baz;https://example4.com", // "bar/baz" should be a single node, distinct from “bar;baz” + "foo;bar\\/baz;https://example3.com" // “bar\/baz” should be a single node ], {delimiter: ";"} ) ] }); } + +export async function treeDelimiter2() { + return Plot.plot({ + axis: null, + height: 120, + margin: 10, + marginLeft: 40, + marginRight: 190, + marks: [ + Plot.tree( + [ + "foo/bar/https:\\/\\/example.com", + "foo/bar/https:\\/\\/example.com\\/posts\\/1", + "foo/baz/https:\\/\\/example.com\\/posts\\/2", + "foo/bar;baz/https:\\/\\/example2.com", // “bar;baz” should be a single node + "foo/bar\\/baz/https:\\/\\/example4.com", // "bar/baz" should be a single node, distinct from “bar;baz” + "foo/bar\\\\/baz/https:\\/\\/example3.com" // “bar\/baz” should be a single node + ] + ) + ] + }); +} From 0aae97cda336065e97f8736530775be15209f4ca Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 7 Sep 2023 12:00:49 -0700 Subject: [PATCH 04/10] \/ needs three (six) backslashes! --- test/plots/tree-delimiter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plots/tree-delimiter.ts b/test/plots/tree-delimiter.ts index f5ae067f7f..bc734fbca2 100644 --- a/test/plots/tree-delimiter.ts +++ b/test/plots/tree-delimiter.ts @@ -38,7 +38,7 @@ export async function treeDelimiter2() { "foo/baz/https:\\/\\/example.com\\/posts\\/2", "foo/bar;baz/https:\\/\\/example2.com", // “bar;baz” should be a single node "foo/bar\\/baz/https:\\/\\/example4.com", // "bar/baz" should be a single node, distinct from “bar;baz” - "foo/bar\\\\/baz/https:\\/\\/example3.com" // “bar\/baz” should be a single node + "foo/bar\\\\\\/baz/https:\\/\\/example3.com" // “bar\/baz” should be a single node ] ) ] From 847e8ecabdb75fcd785e32f330d72f05f02b79dd Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 7 Sep 2023 23:11:05 -0700 Subject: [PATCH 05/10] pass tests, but still not complete --- src/transforms/tree.js | 27 ++++++++---- test/output/treeDelimiter.svg | 80 ++++++++++++++++++++++++++++++++++ test/output/treeDelimiter2.svg | 80 ++++++++++++++++++++++++++++++++++ test/plots/tree-delimiter.ts | 40 +++++++++-------- 4 files changed, 200 insertions(+), 27 deletions(-) create mode 100644 test/output/treeDelimiter.svg create mode 100644 test/output/treeDelimiter2.svg diff --git a/src/transforms/tree.js b/src/transforms/tree.js index 43f3d1bc5e..097f02b347 100644 --- a/src/transforms/tree.js +++ b/src/transforms/tree.js @@ -169,20 +169,29 @@ function nodeData(field) { function normalizer(delimiter = "/") { return `${delimiter}` === "/" ? (P) => P // paths are already slash-separated - : (P) => P.map(slashEscape).map(replaceAll(delimiter, "/")); // TODO string.replaceAll when supported + : (P) => P.map(slashDelimiter(delimiter)); } -function slashEscape(string) { - return string.replace(/\//g, "\\/"); +function slashDelimiter(delimiter) { + const search = new RegExp(`(\\\\*)(${regexEscape(delimiter)}|/)`, "g"); + return (value) => + value == null + ? null + : value.replace( + search, + (match, a, b) => + b === delimiter + ? a.length & 1 + ? `${a.slice(1)}${delimiter}` // drop one backslash + : `${a}/` // replace delimiter with slash + : a.length & 1 + ? `${a}\\\\/` // add two backslashes to escape backslash + : `${a}\\/` // add one backslash to escape slash + ); } function slashUnescape(string) { - return string.replace(/\\\//g, "/"); -} - -function replaceAll(search, replace) { - search = new RegExp(regexEscape(search), "g"); - return (value) => (value == null ? null : `${value}`.replace(search, replace)); + return string.replace(/\\\//g, "/").replace(/\\\\/g, "\\"); // TODO count backslashes properly } function regexEscape(string) { diff --git a/test/output/treeDelimiter.svg b/test/output/treeDelimiter.svg new file mode 100644 index 0000000000..f806f3cc9d --- /dev/null +++ b/test/output/treeDelimiter.svg @@ -0,0 +1,80 @@ + + + + + + + + + + + + + + + + + + + + + + + + /foo + /foo/a + /foo/b + /foo/c;c + /foo/e\\;e + /foo/f\/f + /foo/g\\\/g + /foo/d\\ + /foo/a/\/\/example + /foo/a/\/\/example\/1 + /foo/b/\/\/example\/2 + /foo/c;c/\/\/example2 + /foo/e\\;e/\/\/example2 + /foo/f\/f/\/\/example4 + /foo/g\\\/g/\/\/example3 + /foo/d\\/d + /foo/d\\/\d + /foo/d\\/d/\/\/example2 + /foo/d\\/\d/\/\/example2 + + + //example/foo/a/\/\/example + //example/1/foo/a/\/\/example\/1 + //example/2/foo/b/\/\/example\/2 + //example2/foo/c;c/\/\/example2 + //example2/foo/e\\;e/\/\/example2 + //example4/foo/f\/f/\/\/example4 + //example3/foo/g\\\/g/\/\/example3 + //example2/foo/d\\/d/\/\/example2 + //example2/foo/d\\/\d/\/\/example2 + + + foo/foo + a/foo/a + b/foo/b + c;c/foo/c;c + e\;e/foo/e\\;e + f/f/foo/f\/f + g\/g/foo/g\\\/g + d\/foo/d\\ + d/foo/d\\/d + \d/foo/d\\/\d + + \ No newline at end of file diff --git a/test/output/treeDelimiter2.svg b/test/output/treeDelimiter2.svg new file mode 100644 index 0000000000..f806f3cc9d --- /dev/null +++ b/test/output/treeDelimiter2.svg @@ -0,0 +1,80 @@ + + + + + + + + + + + + + + + + + + + + + + + + /foo + /foo/a + /foo/b + /foo/c;c + /foo/e\\;e + /foo/f\/f + /foo/g\\\/g + /foo/d\\ + /foo/a/\/\/example + /foo/a/\/\/example\/1 + /foo/b/\/\/example\/2 + /foo/c;c/\/\/example2 + /foo/e\\;e/\/\/example2 + /foo/f\/f/\/\/example4 + /foo/g\\\/g/\/\/example3 + /foo/d\\/d + /foo/d\\/\d + /foo/d\\/d/\/\/example2 + /foo/d\\/\d/\/\/example2 + + + //example/foo/a/\/\/example + //example/1/foo/a/\/\/example\/1 + //example/2/foo/b/\/\/example\/2 + //example2/foo/c;c/\/\/example2 + //example2/foo/e\\;e/\/\/example2 + //example4/foo/f\/f/\/\/example4 + //example3/foo/g\\\/g/\/\/example3 + //example2/foo/d\\/d/\/\/example2 + //example2/foo/d\\/\d/\/\/example2 + + + foo/foo + a/foo/a + b/foo/b + c;c/foo/c;c + e\;e/foo/e\\;e + f/f/foo/f\/f + g\/g/foo/g\\\/g + d\/foo/d\\ + d/foo/d\\/d + \d/foo/d\\/\d + + \ No newline at end of file diff --git a/test/plots/tree-delimiter.ts b/test/plots/tree-delimiter.ts index bc734fbca2..f9492c2005 100644 --- a/test/plots/tree-delimiter.ts +++ b/test/plots/tree-delimiter.ts @@ -3,19 +3,22 @@ import * as Plot from "@observablehq/plot"; export async function treeDelimiter() { return Plot.plot({ axis: null, - height: 120, + height: 150, margin: 10, marginLeft: 40, marginRight: 190, marks: [ Plot.tree( [ - "foo;bar;https://example.com", - "foo;bar;https://example.com/posts/1", - "foo;baz;https://example.com/posts/2", - "foo;bar\\;baz;https://example2.com", // “bar;baz” should be a single node - "foo;bar/baz;https://example4.com", // "bar/baz" should be a single node, distinct from “bar;baz” - "foo;bar\\/baz;https://example3.com" // “bar\/baz” should be a single node + "foo;a;//example", // foo → a → //example + "foo;a;//example/1", // foo → a → //example/1 + "foo;b;//example/2", // foo → b → //example/2 + "foo;c\\;c;//example2", // foo → c;c → //example2 + "foo;d\\\\;d;//example2", // foo → d\ → d → //example3 + "foo;d\\\\;\\d;//example2", // foo → d\ → \d → //example3 + "foo;e\\\\\\;e;//example2", // foo → e\;e → //example3 + "foo;f/f;//example4", // foo → f/f → //example4 + "foo;g\\/g;//example3" // foo → g\/g → //example3 ], {delimiter: ";"} ) @@ -26,21 +29,22 @@ export async function treeDelimiter() { export async function treeDelimiter2() { return Plot.plot({ axis: null, - height: 120, + height: 150, margin: 10, marginLeft: 40, marginRight: 190, marks: [ - Plot.tree( - [ - "foo/bar/https:\\/\\/example.com", - "foo/bar/https:\\/\\/example.com\\/posts\\/1", - "foo/baz/https:\\/\\/example.com\\/posts\\/2", - "foo/bar;baz/https:\\/\\/example2.com", // “bar;baz” should be a single node - "foo/bar\\/baz/https:\\/\\/example4.com", // "bar/baz" should be a single node, distinct from “bar;baz” - "foo/bar\\\\\\/baz/https:\\/\\/example3.com" // “bar\/baz” should be a single node - ] - ) + Plot.tree([ + "foo/a/\\/\\/example", // foo → a → //example + "foo/a/\\/\\/example\\/1", // foo → a → //example/1 + "foo/b/\\/\\/example\\/2", // foo → b → //example/2 + "foo/c;c/\\/\\/example2", // foo → c;c → //example2 + "foo/d\\\\/d/\\/\\/example2", // foo → d\ → d → //example3 + "foo/d\\\\/\\d/\\/\\/example2", // foo → d\ → \d → //example3 + "foo/e\\\\;e/\\/\\/example2", // foo → e\;e → //example3 + "foo/f\\/f/\\/\\/example4", // foo → f/f → //example4 + "foo/g\\\\\\/g/\\/\\/example3" // foo → g\/g → //example3 + ]) ] }); } From 6473d2ab89232d03cbcdf5671d3e11ff8182e547 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Mon, 11 Sep 2023 11:25:55 -0700 Subject: [PATCH 06/10] proper escape/unescape --- src/transforms/tree.js | 80 ++++++++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 23 deletions(-) diff --git a/src/transforms/tree.js b/src/transforms/tree.js index 097f02b347..f9ffe6b615 100644 --- a/src/transforms/tree.js +++ b/src/transforms/tree.js @@ -167,35 +167,69 @@ function nodeData(field) { } function normalizer(delimiter = "/") { - return `${delimiter}` === "/" + delimiter = `${delimiter}`; + if (delimiter.length !== 1) throw new Error(`multi-character delimiter`); + return delimiter === "/" ? (P) => P // paths are already slash-separated - : (P) => P.map(slashDelimiter(delimiter)); + : (P) => P.map((p) => slashDelimiter(p, delimiter)); } -function slashDelimiter(delimiter) { - const search = new RegExp(`(\\\\*)(${regexEscape(delimiter)}|/)`, "g"); - return (value) => - value == null - ? null - : value.replace( - search, - (match, a, b) => - b === delimiter - ? a.length & 1 - ? `${a.slice(1)}${delimiter}` // drop one backslash - : `${a}/` // replace delimiter with slash - : a.length & 1 - ? `${a}\\\\/` // add two backslashes to escape backslash - : `${a}\\/` // add one backslash to escape slash - ); -} +const CODE_BACKSLASH = 92; +const CODE_SLASH = 47; -function slashUnescape(string) { - return string.replace(/\\\//g, "/").replace(/\\\\/g, "\\"); // TODO count backslashes properly +function slashDelimiter(input, delimiter) { + const CODE_DELIMITER = delimiter.charCodeAt(0); + let afterBackslash = false; + for (let i = 0, n = input.length; i < n; ++i) { + const code = input.charCodeAt(i); + switch (code) { + case CODE_BACKSLASH: + if (!afterBackslash) { + afterBackslash = true; + continue; + } + // eslint-disable-next-line no-fallthrough + default: + if (afterBackslash) { + if (code === CODE_DELIMITER) { + (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash + } else if (code === CODE_SLASH) { + (input = input.slice(0, i) + "\\\\" + input.slice(i)), (i += 2), (n += 2); // add two backslashes + } + afterBackslash = false; + } else if (code === CODE_DELIMITER) { + input = input.slice(0, i) + "/" + input.slice(i + 1); // replace delimiter with slash + } else if (code === CODE_SLASH) { + (input = input.slice(0, i) + "\\" + input.slice(i)), ++i, ++n; // add a backslash + } + break; + } + } + return input; } -function regexEscape(string) { - return `${string}`.replace(/[\\^$*+?.()|[\]{}]/g, "\\$&"); +function slashUnescape(input) { + let afterBackslash = false; + for (let i = 0, n = input.length; i < n; ++i) { + const code = input.charCodeAt(i); + switch (code) { + case CODE_BACKSLASH: + if (!afterBackslash) { + afterBackslash = true; + continue; + } + // eslint-disable-next-line no-fallthrough + default: + if (afterBackslash) { + if (code === CODE_SLASH || code === CODE_BACKSLASH) { + (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash + } + afterBackslash = false; + } + break; + } + } + return input; } function isNodeValue(option) { From 0e46d7add1e460a46922e1589a11f8b0bfd126d2 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Tue, 12 Sep 2023 08:11:29 -0700 Subject: [PATCH 07/10] refactor logic --- src/transforms/tree.js | 48 +++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/src/transforms/tree.js b/src/transforms/tree.js index f9ffe6b615..f69a828e4f 100644 --- a/src/transforms/tree.js +++ b/src/transforms/tree.js @@ -182,28 +182,27 @@ function slashDelimiter(input, delimiter) { let afterBackslash = false; for (let i = 0, n = input.length; i < n; ++i) { const code = input.charCodeAt(i); + if (code === CODE_BACKSLASH && !afterBackslash) { + afterBackslash = true; + continue; + } switch (code) { - case CODE_BACKSLASH: - if (!afterBackslash) { - afterBackslash = true; - continue; - } - // eslint-disable-next-line no-fallthrough - default: + case CODE_DELIMITER: if (afterBackslash) { - if (code === CODE_DELIMITER) { - (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash - } else if (code === CODE_SLASH) { - (input = input.slice(0, i) + "\\\\" + input.slice(i)), (i += 2), (n += 2); // add two backslashes - } - afterBackslash = false; - } else if (code === CODE_DELIMITER) { + (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash + } else { input = input.slice(0, i) + "/" + input.slice(i + 1); // replace delimiter with slash - } else if (code === CODE_SLASH) { - (input = input.slice(0, i) + "\\" + input.slice(i)), ++i, ++n; // add a backslash + } + break; + case CODE_SLASH: + if (afterBackslash) { + (input = input.slice(0, i) + "\\\\" + input.slice(i)), (i += 2), (n += 2); // add two backslashes + } else { + (input = input.slice(0, i) + "\\" + input.slice(i)), ++i, ++n; // add backslash } break; } + afterBackslash = false; } return input; } @@ -212,22 +211,19 @@ function slashUnescape(input) { let afterBackslash = false; for (let i = 0, n = input.length; i < n; ++i) { const code = input.charCodeAt(i); + if (code === CODE_BACKSLASH && !afterBackslash) { + afterBackslash = true; + continue; + } switch (code) { case CODE_BACKSLASH: - if (!afterBackslash) { - afterBackslash = true; - continue; - } - // eslint-disable-next-line no-fallthrough - default: + case CODE_SLASH: if (afterBackslash) { - if (code === CODE_SLASH || code === CODE_BACKSLASH) { - (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash - } - afterBackslash = false; + (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash } break; } + afterBackslash = false; } return input; } From f9e23393da16097850cc41085e158a682594c36c Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Tue, 12 Sep 2023 08:13:10 -0700 Subject: [PATCH 08/10] lift delimiter code --- src/transforms/tree.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/transforms/tree.js b/src/transforms/tree.js index f69a828e4f..c62eab4d22 100644 --- a/src/transforms/tree.js +++ b/src/transforms/tree.js @@ -168,17 +168,16 @@ function nodeData(field) { function normalizer(delimiter = "/") { delimiter = `${delimiter}`; + if (delimiter === "/") return (P) => P; // paths are already slash-separated if (delimiter.length !== 1) throw new Error(`multi-character delimiter`); - return delimiter === "/" - ? (P) => P // paths are already slash-separated - : (P) => P.map((p) => slashDelimiter(p, delimiter)); + const delimiterCode = delimiter.charCodeAt(0); + return (P) => P.map((p) => slashDelimiter(p, delimiterCode)); } const CODE_BACKSLASH = 92; const CODE_SLASH = 47; -function slashDelimiter(input, delimiter) { - const CODE_DELIMITER = delimiter.charCodeAt(0); +function slashDelimiter(input, delimiterCode) { let afterBackslash = false; for (let i = 0, n = input.length; i < n; ++i) { const code = input.charCodeAt(i); @@ -187,7 +186,7 @@ function slashDelimiter(input, delimiter) { continue; } switch (code) { - case CODE_DELIMITER: + case delimiterCode: if (afterBackslash) { (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash } else { From 7d3204df3727f8408208db5aa183808020dc40b4 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Tue, 12 Sep 2023 08:14:04 -0700 Subject: [PATCH 09/10] tweak error message --- src/transforms/tree.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transforms/tree.js b/src/transforms/tree.js index c62eab4d22..090fb6283a 100644 --- a/src/transforms/tree.js +++ b/src/transforms/tree.js @@ -169,7 +169,7 @@ function nodeData(field) { function normalizer(delimiter = "/") { delimiter = `${delimiter}`; if (delimiter === "/") return (P) => P; // paths are already slash-separated - if (delimiter.length !== 1) throw new Error(`multi-character delimiter`); + if (delimiter.length !== 1) throw new Error("delimiter must be exactly one character"); const delimiterCode = delimiter.charCodeAt(0); return (P) => P.map((p) => slashDelimiter(p, delimiterCode)); } From 5df247a0d817a68b4967bb9c191bba1717de55c4 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Tue, 12 Sep 2023 08:46:00 -0700 Subject: [PATCH 10/10] refactor logic slightly --- src/transforms/tree.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/transforms/tree.js b/src/transforms/tree.js index 090fb6283a..067fc03229 100644 --- a/src/transforms/tree.js +++ b/src/transforms/tree.js @@ -178,14 +178,16 @@ const CODE_BACKSLASH = 92; const CODE_SLASH = 47; function slashDelimiter(input, delimiterCode) { + if (delimiterCode === CODE_BACKSLASH) throw new Error("delimiter cannot be backslash"); let afterBackslash = false; for (let i = 0, n = input.length; i < n; ++i) { - const code = input.charCodeAt(i); - if (code === CODE_BACKSLASH && !afterBackslash) { - afterBackslash = true; - continue; - } - switch (code) { + switch (input.charCodeAt(i)) { + case CODE_BACKSLASH: + if (!afterBackslash) { + afterBackslash = true; + continue; + } + break; case delimiterCode: if (afterBackslash) { (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash @@ -209,13 +211,13 @@ function slashDelimiter(input, delimiterCode) { function slashUnescape(input) { let afterBackslash = false; for (let i = 0, n = input.length; i < n; ++i) { - const code = input.charCodeAt(i); - if (code === CODE_BACKSLASH && !afterBackslash) { - afterBackslash = true; - continue; - } - switch (code) { + switch (input.charCodeAt(i)) { case CODE_BACKSLASH: + if (!afterBackslash) { + afterBackslash = true; + continue; + } + // eslint-disable-next-line no-fallthrough case CODE_SLASH: if (afterBackslash) { (input = input.slice(0, i - 1) + input.slice(i)), --i, --n; // remove backslash