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

Implemented 'Mandatory flags' per issue #44 #98

Closed
wants to merge 1 commit into from

Conversation

kppullin
Copy link

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

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

If this pull request is accepted I'll also update the docs : )

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.
@tj
Copy link
Owner

tj commented Oct 29, 2012

personally i prefer to leave this portion to the program logic, since it could be more complex, such as if --foo is used then --bar is required as well etc

@millermedeiros
Copy link

@visionmedia maybe a solution for the future is to allow objects on the .option() calls, like:

program
  .option({
    flags : '-f, --foo <file>',
    description : 'Foo option rocks',
    required : true,
    // specify that this option depends on other ones
    depends : '--bar, --ipsum',
    validate : function(value, program){
      // check if supplied argument is valid
      return value.indexOf('.') !== -1;
    }
  });

this would increase the flexibility and group all the logic related to arguments parsing/validation.

@tj
Copy link
Owner

tj commented Nov 5, 2012

meh I think that's getting too complex, it's easy to defer most of that to program logic

@thesmart
Copy link

Dude, seriously. I expected this feature to be part of commander. Wondering what is the point of commander if not to require arguments?

@tj
Copy link
Owner

tj commented Nov 15, 2012

don't use it if you dont want to? options are one thing but arguments can be parsed in a nearly infinite number of ways, I'd rather not handle that at all, frankly I want to get rid of sub-command support (as it is now) as well, and go with git-style sub commands

@tj
Copy link
Owner

tj commented Jun 13, 2013

leaving this up to the user, it's not hard to add a few conditionals

@tj tj closed this Jun 13, 2013
@EvanCarroll
Copy link

I don't agree with this position you've taken @visionmedia . It's one of those cases where 99% of the users want the barest most minimal functionality. It's a very ideological stance to take that because things can get more complex no accommodations should be made.

That said, I agree with your conclusion -- those that can't live with your decision should go elsewhere. Perhaps though you could make that more apparent in the docs. This is one of three bug reports that references this functionality. I imagine if there was an FAQ it'd be the foremost asked question. Would you accepted a doc-patch that said this feature would not be implemented, with a reference to optimist which seems to be all around more complete (a larger library)?

@tj
Copy link
Owner

tj commented Nov 11, 2013

it's trivial to do if (!foo) throw new Error('<foo> required'). If we added support for this then people would want a way to customize the output etc, which then becomes more obscure, gaining pretty much nothing. Almost always there's some form of logic applied to arguments anyway, validation, conditions based on the env etc...

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

Successfully merging this pull request may close these issues.

5 participants