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

Mandatory flags #44

Closed
grahamb opened this issue Jan 16, 2012 · 17 comments

Comments

Projects
None yet
@grahamb
Copy link

commented Jan 16, 2012

Commander supports required values for flags (.option('-f --foo <foo>', 'the value of foo)) but there doesn't appear to be a built-in way of making a flag itself required/mandatory.

Example:

var program = require('commander');

program
    .version('0.0.1')
    .option('-f --foo <foo>', 'Foo is required')
    .parse(process.argv);


console.log(program.foo);

Output:

$ node test.js -f bar
// outputs "bar", good

$ node test.js -f
// outputs "error: option `-f --foo <foo>' argument missing", good

$ node test.js
// outputs "undefined", not good; should output "error: required option `-f -foo <foo>' is missing" or similar
@fhellwig

This comment has been minimized.

Copy link

commented May 30, 2012

I totally agree. Try the ls command with an invalid option:

$ ls -X
ls: illegal option -- X
usage: ls [-ABCFGHLOPRSTUWabcdefghiklmnopqrstuwx1] [file ...]

Seems like a reasonable requirement for commander.

@sebmck

This comment has been minimized.

Copy link

commented Jun 5, 2012

+1

Having an asterisks at the end of the first option argument would be a good mandatory/required indicator.

kppullin added a commit to kppullin/commander.js that referenced this issue Oct 28, 2012

Implemented 'Mandatory flags' per issue tj#44
Implemented 'Mandatory flags' based on the thread:
tj#44 .

Similar to the fix from this pull request:
tj#70 , but is additive
and does not break the existing semantics of 'required' arguments.
@kilianc

This comment has been minimized.

Copy link

commented Jan 30, 2013

+1

1 similar comment
@Gurpartap

This comment has been minimized.

Copy link

commented Feb 2, 2013

+1

@kilianc

This comment has been minimized.

Copy link

commented Feb 2, 2013

Also, when a required parameter is missing, we should show the usage after the warning.

@ravi

This comment has been minimized.

Copy link

commented Jun 2, 2013

+1

4 similar comments
@Morriz

This comment has been minimized.

Copy link

commented Jun 3, 2013

+1

@farzher

This comment has been minimized.

Copy link

commented Jul 21, 2013

+1

@klyngbaek

This comment has been minimized.

Copy link

commented Sep 25, 2013

+1

@johngeorgewright

This comment has been minimized.

Copy link

commented Mar 14, 2014

+1

@tj

This comment has been minimized.

Copy link
Owner

commented Mar 15, 2014

I believe I mentioned this in a different issue, but it doesn't really make sense, that means the value is required, not the flag

@tj tj closed this Mar 15, 2014

@ravi

This comment has been minimized.

Copy link

commented Mar 15, 2014

@visionmedia do you mean to say that '--foo ' already implies that '--foo' is a required arg? Also, what about command line args that are flags (and so do not have a value)? I have a feeling I am misunderstanding your response. Can you explain?

@tj

This comment has been minimized.

Copy link
Owner

commented Mar 15, 2014

oh sorry I misread the original post, there was another issue which mentioned that --foo <bar> should imply that the flag should be required. Regarding this issue I think it could be doable but I don't think we could really come up with a great API for it. The required flags often vary as far as combinations go, so IMO it's leaky to try to come up with an API, I just plop in assert()s

imrehg added a commit to imrehg/node-bitpay-client that referenced this issue Jul 12, 2014

bitpay-cli: email is only required for login command
Move the "-e|--email" option to the "login" command since no other
command requires it.

A tricky part is to get the option values out of the commander library.
Apparently top level ("program" options) are accessed through the
program's name (here e.g. "bitpay.optionname"), while command-specific
options are passed to the .action function (in this patch e.g.
"env.optionname"). This means one has to be mindful whether an option
is global (accessible all through the "program") or just within the option
and retrieve the option value accordingly.

Also changed "[email]" to "<email>", meaning that the email value is a
required parameter of "-e|--email" option (ie. "bitpay login -e" is not
a valid command when using "<email>" in the settings, while it would be
valid using the previous "[email]" form. This affects the other options
as well, but that's beyond the scope of this patch.

Unfortunately as I see commander does not allow (yet) for making flags
required (i.e. to make it invalid to call "bitpay login" without "-e"),
so internal checks are still needed. See more at the upstream issue
tracker of tj/commander.js/issues/44

Signed-off-by: Gergely Imreh <imrehg@gmail.com>
@danse

This comment has been minimized.

Copy link

commented Aug 20, 2015

If i get this correctly, there is not a way to have mandatory command line arguments still. This seems pretty important, especially given how devastating undefined values can be in a Javascript program

@mralexgray

This comment has been minimized.

Copy link

commented Sep 15, 2015

+1, why was this closed?

@marcelofarias

This comment has been minimized.

Copy link

commented Oct 29, 2015

Submitted #462. Mentioned here: #230.

@axelpale

This comment has been minimized.

Copy link

commented Sep 27, 2017

+1, but see the docs about how to solve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.