Skip to content

Fix positional arguments not populated when halt-at-non-option is set#2524

Open
ordinary9843 wants to merge 1 commit into
yargs:mainfrom
ordinary9843:fix/halt-at-non-option-positionals
Open

Fix positional arguments not populated when halt-at-non-option is set#2524
ordinary9843 wants to merge 1 commit into
yargs:mainfrom
ordinary9843:fix/halt-at-non-option-positionals

Conversation

@ordinary9843
Copy link
Copy Markdown

When halt-at-non-option is configured, yargs-parser places all positional arguments into argv['--'] instead of argv._. The populatePositionals function in command.ts reads only argv._.length for its required-argument count check and uses argv._ exclusively when mapping positionals to their named keys. This means that even when the correct number of positionals is provided, the validation reports "Not enough non-option arguments: got 0, need at least 1" and no positional values are populated.

This fix detects when halt-at-non-option is active and argv['--'] has content, then merges those entries into argv._ before validation and mapping run. Both the count check and the positional population now see the correct arguments.

Fixes #2423

Comment thread lib/command.ts
Comment on lines +551 to +553
// When halt-at-non-option is configured, yargs-parser places positional
// arguments in argv['--'] instead of argv._. Move them into argv._ so
// positional validation and mapping work correctly.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out the -- storage is on purpose.

#2423 (comment)

I fear simply moving the arguments between properties will interact with other behaviours and combinations of options. In particular if caller has set "populate--": true.

Aside: the ongoing cost we pay for letting yargs users configure yargs-parser!

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 this pull request may close these issues.

halt-at-non-option is not working as expected.

2 participants