ArgParser: option value recognition should be syntactic, not registry-based
- Dominant language
- C++
- Stars
- 2k
- Forks
- 874
- Avg merge
- 6d 15h
- Merged PRs (30d)
- 46
Description
`ArgParser` decides whether a token following an option is that option's value by looking the token up in the current command's option table (`ArgParser::Command::is_registered_option()`). Two consequences follow.
### 1. A parent command's option swallows a subcommand's option
Because the lookup is scoped to a single command, an option of a parent command does not recognise an option belonging to a subcommand, and takes it as its own value:
```
$ traffic_ctl config reload -f -m
# --format is set to "-m"; --monitor is silently dropped
$ traffic_ctl config reload -f -D ip_allow.id=foo
Error: Unknown command, option or args: 'ip_allow.id=foo'
```
`--format`/`-f` and `--run-root` are declared on the top level command, while `-m` and `-D` belong to `config reload`. The reverse direction is safe only because each level strips its own options before recursing into its subcommands, so this is a one-directional accident rather than a property of the design.
### 2. Making the lookup recursive trades one problem for another
Walking the subcommand tree inside `is_registered_option()` fixes the above, but couples parent value parsing to the entire command tree: adding an option to any subcommand retroactively narrows the values that unrelated parent options accept. A new `--json` on `config get` would turn `traffic_ctl --format --json` from a value into a usage error.
### What argparse does
`ArgParser`'s arities are documented to mirror the `nargs` values of Python's `argparse`, which resolves this syntactically and never consults the option table. A token following an option is refused as that option's value when it starts with a prefix character, is longer than one character, contains no space, and does not look like a negative number (`-\.?\d`):
```
-t -5 -> value '-5' -t -x -> error: expected one argument
-t -5.5 -> value '-5.5' -t -zzz -> error (not a registered option either)
-t - -> value '-' -t -- -> error
```
The negative-number exception is itself withdrawn if any registered option looks numeric. The documented escape for a dash-prefixed value is `--opt=value`.
`argparse` also scopes subcommands: an option of the parent parser may not appear after the subcommand name at all (`reload -f json` gives `unrecognized arguments: -f json`). That is the structural reason it rarely meets this case. `ArgParser` scans the whole argv at every level, which is more permissive and is what creates the ambiguity.
### Proposal
Adopt the syntactic rule, keep `ArgParser`'s existing `--` escape, and decide explicitly how numeric values are treated, since several ATS options take them. This subsumes every case above with no tree walk.
Found while reviewing #13570, which fixed the same class of problem within a single command.
Contributor guide
Assessment
This issue has not been assessed yet.