use puka for escaping scripts

[nlf]

the current escaping of shell scripts is pretty naive and fails in spectacular fashion in windows in many scenarios, this switches out that implementation in favor of some slightly creative usage of puka to ensure that commands are handled a bit more consistently

[rhendric]

Hi! Puka's author here. I'd be interested to hear a little more about what motivated the escapeCmd logic here. Just from your tests, it looks like all those cases would be covered by ShellString.sh([cmd]).toString(isWindows && 'win32'), but it's very likely there are edge cases I'm not thinking of right now. (If there are, I'm curious if Puka ought to handle them differently somehow.)

[nlf]

it seemed like there was an unnecessary amount of double quoting going on that

> const cmd = `node -e 'require("fs").writeFileSync("cwd", process.cwd())'`
> const isWindows = true // for escapeCmd's sake
> // copy the puka require and escapeCmd function from lib/make-spawn-args.js
> console.log(escapeCmd(cmd))
node -e ^"require^(\^"fs\^"^).writeFileSync^(\^"cmd\^",^ process.cwd^(^)^)^"
> console.log(puka.ShellString.sh([cmd]).toString('win32'))
node -e ^^^"require^^^(\^^^"fs\^^^"^^^).writeFileSync^^^(\^^^"cmd\^^^",^^^ process.cwd^^^(^^^)^^^)^^^"
> child_process.spawnSync('cmd', ['/d', '/s', '/c', escapeCmd(cmd)], { stdio: 'inherit', windowsVerbatimArguments: true })
{
  status: 0,
  signal: null,
  output: [ null, null, null ],
  pid: 2316,
  stdout: null,
  stderr: null
}
> fs.readFileSync('./cwd', 'utf8')
'C:\\Users\\Nathan LaFreniere\\Projects\\run-script'
> > fs.unlinkSync('./cwd') // delete the file
> fs.readFileSync('./cwd', 'utf8') // just showing it's really deleted
Uncaught Error: ENOENT: no such file or directory, open './cwd'
    at Object.openSync (fs.js:476:3)
    at Object.readFileSync (fs.js:377:35) {
  errno: -4058,
  syscall: 'open',
  code: 'ENOENT',
  path: './cwd'
}
> child_process.spawnSync('cmd', ['/d', '/s', '/c', puka.ShellString.sh([cmd]).toString('win32')], { stdio: 'inherit', windowsVerbatimArguments: true }) // try using your suggestion
[eval]:1
^require^(\^fs\^^).writeFileSync^(\^cwd\^,^ process.cwd^(^)^)^
^

SyntaxError: Unexpected token '^'
    at new Script (vm.js:100:7)
    at createScript (vm.js:261:10)
    at Object.runInThisContext (vm.js:309:10)
    at internal/process/execution.js:77:19
    at [eval]-wrapper:6:22
    at evalScript (internal/process/execution.js:76:60)
    at internal/main/eval_string.js:23:3
{
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 8916,
  stdout: null,
  stderr: null
}

this was the only way I could find that seemed to work correctly for passing commands to cmd.exe using the various settings and flags that we use

[rhendric]

Ah, yes. I think you'll find that Puka's level of quoting is correct if the command calls a batch file instead of a true executable like node. (Try installing the echo-cli npm package, and running your test with echo-cli '<"' > cwd. I'm not on a Windows machine right now to test that, but I think the result of escapeCmd, which is echo-cli ^"^<\^"^" > cwd, will not result in <" successfully being written to a file called cwd—I think you'll get some sort of error instead.)

Windows is such a pain in this respect; the only way to get it right if you don't know whether your users will be calling one or the other is to walk the path and find the file referenced. I had Puka default to assuming batch files since that's what npm makes for the executables it installs, but calling node specifically maybe deserves to be a special case?