-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Required arguments for options and commands are not enforced #230
Comments
I agree, will look into this for a future release! |
So what are required arguments for exactly? |
@foxx @thethomaseffect Can you guys see if this works for you? I needed similar functionality so I thought I might spin up a quick pull request. |
I see a lot of open pull-requests.. Will this be merged if I spend time implementing it? |
@NameFILIP Hopefully. If you want to ensure your PR is merged, make sure it's wanted (already shown in this case), and has proper tests & documentation. |
Is anyone actually working towards this? |
My 2 cents: #462 |
Just come up against this again, seems to be quite a few PRs, are any due to be merged by core dev? |
Any word on this @tj? |
just came across this issue and didn't really get why it's still not implemented. I saw different reasons for not doing it, but the main problem that current implementation is not really consistent:
what the reason to define required option and don't perform any validation? Of course, it's possible to implement custom validation, but then you need to align error messages with standard validation messages like ' |
Any update on this? |
I immediately regret using this library. |
Any update? :) |
+1 |
1 similar comment
+1 |
+1 |
Just came across this thread and I'm wondering the same thing. How is there not support for required arguments and why the reluctance? :( |
+1 |
2 similar comments
+1 |
+1 |
+1 The fact that this still isn't a thing almost hurts. If you want to be a command-line library this should be one of the core features. |
After battling with commander.js for some time, I've finally switched to yargs. It has all the features one commonly needs to build CLI tools, including support for required options. Here's an example with some basic functionality: yargs
.command(
'create',
'Create a new storage version',
{
message: {
alias: 'm',
describe: 'Storage version description',
demandOption: true,
requiresArg: true,
type: 'string',
nargs: 1
},
location: {
alias: 'l',
describe:
'Directory where storage versions are kept, grouped by storage area',
default: 'storage/versions',
requiresArg: true,
type: 'string',
nargs: 1
},
storage: {
alias: 's',
describe: 'Storage area',
choices: ['local', 'sync'],
default: 'local',
requiresArg: true,
type: 'string',
nargs: 1
}
},
function(argv) {
createVersion(argv);
}
)
.demandCommand(1)
.help()
.alias('help', 'h')
.version()
.alias('version', 'v')
.strict().argv; |
For simple scenarios, following code works well for me: program
.description('Makes any type of pie.')
.arguments('<type>')
.action(function (type) {
// Bake that pie.
});
program.parse(process.argv);
if (!process.argv.slice(2).length) {
program.outputHelp(() => program.help());
} |
AWS CLI
Last line mainly.
Suggestion 1: Additional boolean parameter at the end: program.option('-o', '--option <value>', 'A mandatory option.', true) Suggestion 2: Fancy string syntax -> Add (a) star(s) somewhere with the value program.option('-o', '--option <*value>', 'A mandatory option.') |
Not a fan of the asterisk. I prefer appending |
I rather prefeer the usage of a property or function call which is more explicit and readable: program.option('-o', '--option <*value>', 'A mandatory option.').required program.option.required('-o', '--option <*value>', 'A mandatory option.') |
Relatedly, required arguments should be enforced. It should do more than produce nothing. See above comment for an example use case. |
Can be better to add options in case something else has to be passed in future:
This also can distinguish from default value, as well as can accept just an object for all settings, this is convenient if options come from somewhere else:
|
Thanks for the aws example @mrgrain. I tracked down the associated help, and aws explicitly marks all the optional options. Not what I want to do, but interesting.
|
Syntax:
I am currently liking |
I prefer that too. |
Yes, they do it pretty much the other way around and documentation-wise the square brackets are certainly common. But for now anyway of requiring an option to be present would be nice. One can always add their own message styling. I'm very much like |
I do also like the idea in principle of supporting an options/settings parameter to allow for future additions: #230 (comment) But I think it needs to be a new method as
|
I added an issue to get some visibility on possible API, vote for preferred API there: #1038 |
Implementing |
Added |
v4.0.0 has been released. There were some other things raised in various comments, but the main issue discussed of allowing a mandatory option has been added. Feel free to open a new issue for other aspects, with new information and renewed interest. |
@shadowspawn That's already a start but the other main issue of this thread was mandatory arguments, which still isn't a thing :( The current workaround being if (program.args.length < 2) {
program.outputHelp(() => program.help());
} |
Missing arguments generate an error from Commander v5: #995 #1149 Using the example code from: #230 (comment)
|
So I’ve been trying to use the .requiredOption() feature, but it simply is not work for me: "commander": "^10.0.0”, test.js
It returns no error. Thanks for any help. |
A quick answer @mikkimichaelis : the short and long option go in the same string parameter, and the short option is a single character. So try: program.requiredOption('-m, --meetingId <string>', 'Specifies the meeting.id of target meeting'); (If you tried logging out |
Wow! Thanks so much for the quick response and guidance! Yes, you are exactly right, but you knew that :-) |
It would be pretty awesome to have support for required arguments. Although there is support for required argument values, there doesn't appear to be anything for ensuring the argument has been specified.
For example;
I'd like to make it so an error is raised if
-e
has not been specified like so;The text was updated successfully, but these errors were encountered: