Fix: unexpected behavior of leading bang (#124)

mysticatea · Feb 3, 2018 · 35f86ae · 35f86ae
1 parent c76db0e
commit 35f86ae
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 3 deletions.
diff --git a/lib/match-tasks.js b/lib/match-tasks.js
@@ -43,7 +43,7 @@ function createFilter(pattern) {
     const spacePos = trimmed.indexOf(" ")
     const task = spacePos < 0 ? trimmed : trimmed.slice(0, spacePos)
     const args = spacePos < 0 ? "" : trimmed.slice(spacePos)
-    const matcher = new Minimatch(swapColonAndSlash(task))
+    const matcher = new Minimatch(swapColonAndSlash(task), { nonegate: true })
     const match = matcher.match.bind(matcher)
 
     return { match, task, args }

diff --git a/lib/run-task.js b/lib/run-task.js
@@ -82,6 +82,22 @@ function detectStreamKind(stream, std) {
     )
 }
 
+/**
+ * Ensure the output of shell-quote's `parse()` is acceptable input to npm-cli.
+ *
+ * The `parse()` method of shell-quote sometimes returns special objects in its
+ * output array, e.g. if it thinks some elements should be globbed. But npm-cli
+ * only accepts strings and will throw an error otherwise.
+ *
+ * See https://github.com/substack/node-shell-quote#parsecmd-env
+ *
+ * @param {object|string} arg - Item in the output of shell-quote's `parse()`.
+ * @returns {string} A valid argument for npm-cli.
+ */
+function cleanTaskArg(arg) {
+    return arg.pattern || arg.op || arg
+}
+
 //------------------------------------------------------------------------------
 // Interface
 //------------------------------------------------------------------------------
@@ -153,7 +169,7 @@ module.exports = function runTask(task, options) {
         else if (options.prefixOptions.indexOf("--silent") !== -1) {
             spawnArgs.push("--silent")
         }
-        Array.prototype.push.apply(spawnArgs, parseArgs(task))
+        Array.prototype.push.apply(spawnArgs, parseArgs(task).map(cleanTaskArg))
 
         cp = spawn(execPath, spawnArgs, spawnOptions)
 

diff --git a/test-workspace/package.json b/test-workspace/package.json
@@ -38,7 +38,9 @@
     "test-task:nest-append:run-s": "node ../bin/run-s/index.js test-task:append",
     "test-task:nest-append:run-p": "node ../bin/run-p/index.js test-task:append",
     "test-task:delayed": "node tasks/output-with-delay.js",
-    "test-task:yarn": "node ../bin/npm-run-all/index.js test-task:append:{a,b} --npm-path yarn"
+    "test-task:yarn": "node ../bin/npm-run-all/index.js test-task:append:{a,b} --npm-path yarn",
+    "!test": "node tasks/append1.js X",
+    "?test": "node tasks/append1.js Q"
   },
   "repository": {
     "type": "git",

diff --git a/test/pattern.js b/test/pattern.js
@@ -143,4 +143,71 @@ describe("[pattern] it should run matched tasks if glob like patterns are given.
             }
         })
     })
+
+    describe("\"!test-task:**\" should not match to anything", () => {
+        it("Node API", async () => {
+            try {
+                await nodeApi("!test-task:**")
+                assert(false, "should not match")
+            }
+            catch (err) {
+                assert((/not found/i).test(err.message))
+            }
+        })
+
+        it("npm-run-all command", async () => {
+            const stderr = new BufferStream()
+            try {
+                await runAll(["!test-task:**"], null, stderr)
+                assert(false, "should not match")
+            }
+            catch (_err) {
+                assert((/not found/i).test(stderr.value))
+            }
+        })
+
+        it("run-s command", async () => {
+            const stderr = new BufferStream()
+            try {
+                await runSeq(["!test-task:**"], null, stderr)
+                assert(false, "should not match")
+            }
+            catch (_err) {
+                assert((/not found/i).test(stderr.value))
+            }
+        })
+
+        it("run-p command", async () => {
+            const stderr = new BufferStream()
+            try {
+                await runPar(["!test-task:**"], null, stderr)
+                assert(false, "should not match")
+            }
+            catch (_err) {
+                assert((/not found/i).test(stderr.value))
+            }
+        })
+    })
+
+    describe("\"!test\" \"?test\" to \"!test\", \"?test\"", () => {
+        it("Node API", async () => {
+            await nodeApi(["!test", "?test"])
+            assert(result().trim() === "XQ")
+        })
+
+        it("npm-run-all command", async () => {
+            await runAll(["!test", "?test"])
+            assert(result().trim() === "XQ")
+        })
+
+        it("run-s command", async () => {
+            await runSeq(["!test", "?test"])
+            assert(result().trim() === "XQ")
+        })
+
+        it("run-p command", async () => {
+            await runPar(["!test", "?test"])
+            assert(result().trim() === "XQ" || result().trim() === "QX")
+        })
+    })
 })