From 2273e245c6f1345f6b865028c6730b6e6c6e1aaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kat=20March=C3=A1n?= <kzm@zkat.tech>
Date: Fri, 13 Apr 2018 16:30:35 -0700
Subject: [PATCH] fix(windows): get escape-related patch over the finish line

---
 index.js      | 32 +++++++++++++++++++++++++++-----
 test/index.js | 15 ++++-----------
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/index.js b/index.js
index b0221f8..dabcbc8 100644
--- a/index.js
+++ b/index.js
@@ -9,6 +9,8 @@ const parseArgs = require('./parse-args.js')
 const path = require('path')
 const which = promisify(require('which'))
 
+const isWindows = process.platform === 'win32'
+
 module.exports = npx
 module.exports.parseArgs = parseArgs
 function npx (argv) {
@@ -207,10 +209,30 @@ function getNpmCache (opts) {
 
 module.exports._buildArgs = buildArgs
 function buildArgs (specs, prefix, opts) {
+  // NOTE: Why the weird escape condition? Because the `child.spawn` npm always
+  // runs in shell mode when on Windows (it's the default, to prevent weird
+  // behavior). That means that arguments to npm need to be escaped (but as
+  // strings, not as paths).
+  //
+  // The regular unix invocation doesn't need escaping, because it doesn't run
+  // in shell mode.
   const args = ['install'].concat(specs)
-  args.push('--global', '--prefix', process.platform === 'win32' ? `"${prefix}"` : prefix)
-  if (opts.cache) args.push('--cache', opts.cache)
-  if (opts.userconfig) args.push('--userconfig', opts.userconfig)
+  args.push(
+    '--global',
+    '--prefix', isWindows ? child.escapeArg(prefix) : prefix
+  )
+  if (opts.cache) {
+    args.push(
+      '--cache',
+      isWindows ? child.escapeArg(opts.cache) : opts.cache
+    )
+  }
+  if (opts.userconfig) {
+    args.push(
+      '--userconfig',
+      isWindows ? child.escapeArg(opts.userconfig) : opts.userconfig
+    )
+  }
   args.push('--loglevel', 'error', '--json')
 
   return args
@@ -222,7 +244,7 @@ function installPackages (specs, prefix, opts) {
   return findNodeScript(opts.npm, {isLocal: true}).then(npmPath => {
     if (npmPath) {
       args.unshift(
-        process.platform === 'win32'
+        isWindows
           ? child.escapeArg(opts.npm)
           : opts.npm
       )
@@ -231,7 +253,7 @@ function installPackages (specs, prefix, opts) {
       return opts.npm
     }
   }).then(npmPath => {
-    return process.platform === 'win32' ? child.escapeArg(npmPath, true) : npmPath
+    return isWindows ? child.escapeArg(npmPath, true) : npmPath
   }).then(npmPath => {
     return child.spawn(npmPath, args, {
       stdio: opts.installerStdio
diff --git a/test/index.js b/test/index.js
index 389073c..632f2ff 100644
--- a/test/index.js
+++ b/test/index.js
@@ -141,24 +141,17 @@ test('installPackages unit', t => {
           })
         }
       },
-      escapeArg (arg) {
-        if (arg === '/f@ke_/path to/node') {
-          return '\'/f@ke_/path to/node\''
-        } else if (arg === 'C:\\f@ke_\\path to\\node') {
-          return '"C:\\f@ke_\\path to\\node"'
-        }
-        return arg
-      }
+      escapeArg: child.escapeArg
     }
   })._installPackages
-  return installPkgs(['installme@latest', 'file:foo'], 'myprefix', {
+  return installPkgs(['installme@latest', 'file:foo'], 'my prefix', {
     npm: NPM_PATH
   }).then(deets => {
     t.deepEqual(deets[1], [
-      NPM_PATH,
+      isWindows ? `"${NPM_PATH}"` : NPM_PATH,
       'install', 'installme@latest', 'file:foo',
       '--global',
-      '--prefix', isWindows ? '"myprefix"' : 'myprefix',
+      '--prefix', isWindows ? '"my prefix"' : "'my prefix'",
       '--loglevel', 'error',
       '--json'
     ], 'args to spawn were correct for installing requested package')