Skip to content

ArgParser: option value recognition should be syntactic, not registry-based #13597

Description

@brbzull0

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.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions