Quote editor.path argument

[rekmarks]

The editor.path string can contain spaces, e.g. /Applications/Visual Studio Code.app/.... This string is passed to execa(), which attempts to execute it as a command in the user's shell. On some systems, this string must be quoted, or else the shell will interpret the substring until the first space as the command, and either error or execute the wrong thing.

[legobeat]

From https://github.com/sindresorhus/execa/tree/v5.1.1#execacommandcommand-options:

If the file or an argument contains spaces, they must be escaped with backslashes. This matters especially if command is not a constant but a variable, for example with __dirname or process.cwd(). Except for spaces, no escaping/quoting is needed.

https://github.com/MetaMask/create-release-branch/actions/runs/7041542167/job/19164270553

[rekmarks]

@legobeat hmm, that's interesting. The weirdness here is that this has not been a problem for anyone who has used this tool, except myself as described above. Specifically, the error I got was:

/bin/sh: /Applications/Visual: No such file or directory
ErrorWithCause: Encountered an error while waiting for the release spec to be edited.
    at wrapError (.../node_modules/@metamask/create-release-branch/dist/misc-utils.js:86:23)
    at waitForUserToEditReleaseSpecification (.../node_modules/@metamask/create-release-branch/dist/release-specification.js:93:42)
...

I already tried my change locally and it fixes the error. However, according to this source, we should probably single-quote paths:

If you want to work with files with spaces or special characters in the filename, you may have to use quotes. For instance, if you wanted to create a file with a space in the name, you could use the following:
% cp /dev/null 'a file with spaces in the name'

I'm discussing with @mcmire where we're going to introduce quoting / escaping. I'm leaving this in draft until it's ready for another round of reviews.

[legobeat]

@legobeat hmm, that's interesting. The weirdness here is that this has not been a problem for anyone who has used this tool, except myself as described above. Specifically, the error I got was:

/bin/sh: /Applications/Visual: No such file or directory
ErrorWithCause: Encountered an error while waiting for the release spec to be edited.
    at wrapError (.../node_modules/@metamask/create-release-branch/dist/misc-utils.js:86:23)
    at waitForUserToEditReleaseSpecification (.../node_modules/@metamask/create-release-branch/dist/release-specification.js:93:42)
...

I already tried my change locally and it fixes the error. However, according to this source, we should probably single-quote paths:

If you want to work with files with spaces or special characters in the filename, you may have to use quotes. For instance, if you wanted to create a file with a space in the name, you could use the following:
% cp /dev/null 'a file with spaces in the name'

I'm discussing with @mcmire where we're going to introduce quoting / escaping. I'm leaving this in draft until it's ready for another round of reviews.

I'd wager that either quoting with single-quote or shell-escaping (with caveats - beware of shell-injection here) will work for arguments while escaping should be correct for the command itself?

[rekmarks]

@legobeat, single-quoting seems to work fine on my system:

/bin/zsh:

'echo' 'foo'
foo

/bin/sh:

'echo' 'foo'
foo

And this would be consistent with this source:

Single quotes: '...' removes the special meaning of every character between the quotes. Everything inside single quotes becomes a literal string. The only character that you can't safely enclose in single quotes is a single quote.

[legobeat]

@legobeat, single-quoting seems to work fine on my system:

/bin/zsh:

'echo' 'foo'
foo

/bin/sh:

'echo' 'foo'
foo

And this would be consistent with this source:

Single quotes: '...' removes the special meaning of every character between the quotes. Everything inside single quotes becomes a literal string. The only character that you can't safely enclose in single quotes is a single quote.

Why not support single-quotes?

[cryptodev-2s]

@legobeat @rekmarks we currently use execa 5.1.1 and on the recent version 8+ they have introduced the escaping Arguments are [automatically escaped](https://www.npmjs.com/package/execa#shell-syntax). They can contain any character, including spaces. This is the preferred method when executing single commands.
https://www.npmjs.com/package/execa
@mcmire we can consider upgrading the package

[legobeat]

@legobeat @rekmarks we currently use execa 5.1.1 and on the recent version 8+ they have introduced the escaping Arguments are [automatically escaped](https://www.npmjs.com/package/execa#shell-syntax). They can contain any character, including spaces. This is the preferred method when executing single commands. https://www.npmjs.com/package/execa we can consider upgrading the package

Just heads up that execa is ESM-only starting with v6.0.0, so this package would have to be migrated to ESM before upgrading that dependency further.
https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c

EDIT: Looks like it would actually be reasonable to stay on CommonJS and utilize dynamic imports. The mocking and spying in the tests would need to be rewritten somewhat to accommodate (or does Jest support running the tests as ESM while the package is otherwise CommonJS? 🤔 )

[rekmarks]

Closing in favor of #121.