Skip to content

Commit

Permalink
Fix argument parsing for volta run to properly handle flags
Browse files Browse the repository at this point in the history
Clap's parsing for variable-length argument lists has precedence rules for what
happens when a potential vararg overlaps with a known flag value (see https://docs.rs/clap/latest/clap/struct.Arg.html#method.allow_hyphen_values):

1. If we are already in a list of variable args, the args take precedence and any
   potential flags are ignored (flags are treated as part of the vararg)
2. If we are _not_ already in a list of variable args, then known flags take
   precedence and are parsed as such, rather than starting the vararg list

As a result of this precedence, with `volta run` set up to have `args` be
separate from `command`, we could run into parsing issues with commands that have
flags that overlap with Volta's. For example:

```
volta run node -h
```

When parsing this, Clap would see `node` as the `command`, and then it would
parse `-h` as _Volta's_ `-h` (help) flag and print Volta's help. This is because
the variable-length `args` hadn't started yet (we hadn't run into any unknown
positional arguments), so Clap treated `-h` as a known flag with higher
precedence. This is exactly the opposite of what we actually want for `volta run`

Instead, what we want is that everything after the `command` argument should
be treated as part of the arguments to the command; regardless of whether the
flag overlaps with Volta's flags.

To achieve that, we combine `command` and `args` into a _single_ vararg, so that
by virtue of having a command in the first place, the vararg has started parsing
and Clap will treat any flags as more argument values, rather than as flags.
  • Loading branch information
charlespierce committed Aug 16, 2024
1 parent bd1b4ce commit 8cf3dea
Showing 1 changed file with 15 additions and 7 deletions.
22 changes: 15 additions & 7 deletions src/command/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ pub(crate) struct Run {
#[arg(long = "env", value_name = "NAME=value", num_args = 1)]
envs: Vec<String>,

/// The command to run
command: OsString,

/// Arguments to pass to the command
#[arg(allow_hyphen_values = true, trailing_var_arg = true)]
args: Vec<OsString>,
/// The command to run, along with any arguments
#[arg(
allow_hyphen_values = true,
trailing_var_arg = true,
value_name = "COMMAND",
required = true
)]
command_and_args: Vec<OsString>,
}

impl Command for Run {
Expand All @@ -59,7 +61,13 @@ impl Command for Run {
let envs = self.parse_envs();
let platform = self.parse_platform(session)?;

match execute_tool(&self.command, &self.args, &envs, platform, session).into_result() {
// Safety: At least one value is required for `command_and_args`, so there must be at
// least one value in the list. If no value is provided, Clap will show a "required
// argument missing" message and this function won't be called.
let command = &self.command_and_args[0];
let args = &self.command_and_args[1..];

match execute_tool(command, args, &envs, platform, session).into_result() {
Ok(()) => {
session.add_event_end(ActivityKind::Run, ExitCode::Success);
Ok(ExitCode::Success)
Expand Down

0 comments on commit 8cf3dea

Please sign in to comment.