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

Adding .choices(['a', 'b', 'c]) to a variadic option removes its variadic-ness #1447

Closed
MikeyBurkman opened this issue Jan 26, 2021 · 2 comments

Comments

@MikeyBurkman
Copy link

Version: 7.0.0

I was super happy about the recent addition of choices() to options, but when I add choices to a variadic option, the option changes to a single value:

program
  .command('services')
  .addOption(
    new Option(
      '-e, --env <names...>',
      'The environment(s) to look up'
    ).choices(['dev', 'stg', 'prod'])
  )
  .action((args) => console.log(args));
foo services -e dev -e stg

Without .choices(), env becomes an array of ['dev', 'stg']. However, after adding choices, it becomes a single value 'stg'.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Jan 26, 2021

Short version: yes, this is currently a known limitation.

Long version

I had realised there were some limitations like this when adding the variadic support.

.choices() is implemented the same way as custom option processing functions supplied by the user. Commander does not know whether a custom function is a simple coercion (like .choices()) or a complex function that accumulates values. All variadic does when there is a custom function is call it multiple times and leaves it up to the custom function to decide what to return. This works fairly well with the complex custom functions but not much use with the simple coerce functions!

To get .choices() working with variadic we can either extend the choices implementation to understand about variadic, or add a way to tell apart simple and complex custom option processing functions. (I like the second way as a general solution, but possibly too complex. Say adding .coerce() on Option to use for adding simple custom functions. Perhaps easier to support variadic inside choices.)

@shadowspawn shadowspawn self-assigned this Jan 31, 2021
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Jan 31, 2021
@shadowspawn shadowspawn removed their assignment Feb 5, 2021
@shadowspawn
Copy link
Collaborator

.choices() now plays well with variadic from Commander 7.1.0

@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants