Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parseCommandString() method #1016

Closed
ehmicky opened this issue May 4, 2024 · 2 comments · Fixed by #1054
Closed

Add parseCommandString() method #1016

ehmicky opened this issue May 4, 2024 · 2 comments · Fixed by #1054

Comments

@ehmicky
Copy link
Collaborator

ehmicky commented May 4, 2024

Background

Behavior

execaCommand() takes a command string, splits it into an array of arguments (using spaces as delimiters), then forwards that array to execa().

It also trims whitespaces, allows consecutive spaces, and lets users escape spaces with a backslash.

import {execa, execaCommand} from 'execa';

await execaCommand('file one two\\ three');
// Is same as:
await execa('file', ['one', 'two three']);

Origin

The reason this method was introduced was to discourage the shell option. Many users used that option only to be able to pass the command and its arguments as a single string, as opposed to an array. However, the shell option comes with many pitfalls, including a risk for command injection. execaCommand() provides the single string syntax, but without using a shell.

Recent changes

However, since then, we've introduced the template string syntax. Unless the command or arguments are user-defined, it is actually now better to use the template string syntax instead of execaCommand(). The template string syntax has more features and provides with a simpler way to escape spaces.

import {execa, execaCommand} from 'execa';

await execa`file one ${'two three'}`;
await execaCommand('file one two\\ three');

So execaCommand() is now partially redundant, although it still has some use cases (e.g. user-defined input). Taking this new situation into account, this issue is a proposal for evolving execaCommand() to a new purpose.

Proposal

Rename execaCommand() to splitCommand(). It performs the same command/arguments splitting than execaCommand() does. However, it does not execute the command. Instead, it just returns an array with the file and its arguments. The intention is to pass that array to the template string syntax of execa.

Before:

import {execaCommand} from 'execa';

await execaCommand('file one two\\ three');

After:

import {execa, splitCommand} from 'execa';

await execa`${splitCommand('file one two\\ three')}`;

For the above case, this results in more verbose syntax. However, this comes with the following pros.

Advantages

Orthogonality

Unlike execaCommand(), splitCommand() can be used with $, i.e. can be used in scripts.

await $`${splitCommand('file one two\\ three')}`;

Escaping

execaCommand() uses backslashes to escape spaces. It cannot use ${} to escape spaces like the template string syntax does. On the other hand, splitCommand() allows mixing both.

const variableWithSpaces = 'four five';
await execa`${variableWithSpaces} ${splitCommand('one two\\ three')}`;

Subprocess interpolation

execaCommand() cannot re-use the result from another subprocess (while properly escaping it). splitCommand() does.

const result = await execa`...`;
await execa`${result} ${splitCommand('one two\\ three')}`;

Partial interpolation

execaCommand() splits the file and all its arguments. splitCommand() can apply to only the arguments (not the file), or to only some of the arguments.

await execa`echo four ${splitCommand('one two\\ three')}`;

Debugging

It is simpler to debug splitCommand(). Users will naturally just print the array being returned. There is less magic involved: users clearly understand what's happening in terms of parsing.

const commandArguments = splitCommand('one two\\ three');
console.log(commandArguments); // ['one', 'two three']
await execa`echo ${commandArguments}`;

Less confusion

Explaining the purpose of execaCommand(), as opposed to using the template string syntax, is not easy. I struggled a little when writing the new documentation.

I also saw some examples of execaCommand() being misused. For example, Storybook has a function which takes a command and an array of arguments as input. It joins it into a single string, then passes it to execaCommand(), as opposed to just pass the array of arguments directly to execa(). In other words they are doing something like:

await execaCommand([file, ...args].join(' '));

Instead of:

await execa(file, args);

With the side effect that any argument with spaces is now accidentally split into several arguments.

On the other hand, splitCommand()'s purpose is straightforward.


What do you think @sindresorhus?

@sindresorhus
Copy link
Owner

I agree with the general idea, but I think it needs a better name.

Some alternatives:

  • prepareCommand
  • parseCommandString

@ehmicky
Copy link
Collaborator Author

ehmicky commented May 4, 2024

parseCommandString() sounds good. 👍

@ehmicky ehmicky changed the title Add splitCommand() method Add parseCommandString() method May 4, 2024
This was referenced May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants