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

requiredOption doesn't behave properly with default value #1135

Closed
notzenox opened this issue Dec 31, 2019 · 6 comments
Closed

requiredOption doesn't behave properly with default value #1135

notzenox opened this issue Dec 31, 2019 · 6 comments

Comments

@notzenox
Copy link

The code

program
  .requiredOption("-p --path <string>", "glob pattern of files/folders to delete", "../*")
  .parse(process.argv);

deleteFiles(program.path);

will run without the -p option set (because technically it has a value, default, but still) and delete whatever ../* resolves to.

I think requiredOption should either:

  1. Not have a default value parameter and keep it's current behaviour as-is.
  2. Accept a default value parameter but only use it if the option flag is set, i.e. using the above example node program.js won't run but node program.js -p will run with program.path == "../*" and of course node program.js -p /some/path/* will run with program.path == "/some/path/*"
@shadowspawn
Copy link
Collaborator

shadowspawn commented Jan 1, 2020

I considered default values when I was implementing requiredOption and wondered if the default value was ever useful. Basically, it is not useful to pass a hard-coded default with requiredOption.

The use case I came up with that persuaded me to keep default value is:

program
   .requiredOption('password <secret>', 'password, must be supplied or PASSWORD defined', process.env.PASSWORD);

I particularly like this as an elegant combination of requiredOption and environment variables for defining mandatory option values.

(For interest, a different but closely related issue got opened just yesterday: #1130. I'll be keeping an eye on how much confusion the current behaviour causes.)

@notzenox
Copy link
Author

notzenox commented Jan 1, 2020

If requiredOption would use the default value only when it would be used as a flag (--required-option) it could combine the best of both worlds:

  • it would still be a required option, because the program won't run without it being specified
  • environment variables, config files etc. could still be used, you'd just have to
    explicitly specify the required option

So no option - program doesn't run, option without value - use the default, option with value - use the value.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Jan 29, 2020

By design and as documented, .requiredOption() works like .option() except for making the option required. So if you specify the flag for an option with a <required-value> then you must supply a value.

The behaviour you describe is what you might think happens for option with an [optional-value]:

.requiredOption("-p --path [string]", "glob pattern of files/folders to delete", "../*")

but in fact the default value is assigned when the option is not supplied, as before! That behaviour does not seem particularly useful , but it has always been like that so would be a big breaking change.

@shadowspawn
Copy link
Collaborator

An answer was provided, and no further activity in a month. Closing this as resolved.

Feel free to open a new issue if it comes up again, with new information and renewed interest.

@shadowspawn
Copy link
Collaborator

For interest, I noticed a similar issue against argparse where the reporter expected the current Commander behaviour: nodeca/argparse#148

@shadowspawn
Copy link
Collaborator

PR #1652 adds .preset() for specifying the argument to use when optional argument not supplied.

program
   .addOption(new Option("-p --path [string]", "glob pattern of files/folders to delete")
      .preset("../*")
      .makeOptionMandatory());

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

No branches or pull requests

2 participants