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 negative option makes Commander.js to return "true" when option is not specified #979

Closed
slavafomin opened this issue Jun 20, 2019 · 9 comments

Comments

@slavafomin
Copy link

slavafomin commented Jun 20, 2019

Hello!

I'm encountering yet another strange behavior of Commander.js.

When I add the following option:
command.option('--clean', 'cleans the destination build directory');

And then run my program this way: program, the Commander returns clean option as undefined.

However, when I add an additional negative option:

command.option(`--clean`, 'cleans the destination build directory');
command.option(`--no-clean`, 'prevents cleaning the destination directory');

It starts to return true for the clean option when executed the same as above instead of the undefined.

Why is that? I want the option to be always undefined if it's not specified via CLI.

@shadowspawn
Copy link
Collaborator

There is special processing for --no-foo which makes foo true by default. See: https://github.com/tj/commander.js#other-option-types-negatable-boolean-and-flagvalue

When the feature was added long ago, it was assumed it would be used without the ordinary flag. There are currently issues with trying to use both --no-foo and --foo. See #939.

@slavafomin
Copy link
Author

@shadowspawn thank you for explanation. But is there a way to disable this special behavior for --no- prefixed options?

@shadowspawn
Copy link
Collaborator

Short version: no. (Not that I am aware of.)

@shadowspawn
Copy link
Collaborator

See #795 for the new behaviour if it is accepted (which is likely).

@slavafomin
Copy link
Author

The existence of the negated option shouldn't make true the default value for non-negated one. This is just a bad design and a very non-intuitive behavior.

If we say "user can ask to make pizza without a cheese" it doesn't mean, that if she didn't ask it, we have to always make pizza with cheese. It's a twisted logic.

The much better API would be:

  • if option is specified without a value, i.e. --cheese it would be resolved as true
  • If it's specified with --no prefix, i.e. --no-cheese it should resolve to false
  • if it's omitted, then it should be undefined
  • if value is specified, e.g.: --cheese cheddar, then it should return a string cheddar.

With such approach, the developer don't have to define negated option specifically at all, instead, every option could be negated. I believe such API would be straightforward, intuitive and easy to work with.

And if we really want to have a default value, then the developer should provide it, we shouldn't make this choice for her, e.g.: .option('cheese').defaultsTo('mozzarella') or .option('cut').defaultsTo(true).

@usmonster
Copy link
Contributor

Totally agree, and as mentioned above, this will very likely be fixed in the next major version. Feel free to subscribe to #795. :)

@slavafomin
Copy link
Author

@usmonster great to hear that we are on the same page on this. Thank you for you contribution! Looking forward for this to be merged.

@shadowspawn
Copy link
Collaborator

This issue will be resolved when v3.0.0 is released. Available now as a prerelease. See #1001

@shadowspawn
Copy link
Collaborator

Support for combined --foo and --no-foo has shipped in v3 (thanks @usmonster): https://github.com/tj/commander.js/releases/tag/v3.0.0

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

3 participants