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

Add program.opts namespace for all options #35

Closed
wants to merge 2 commits into from

Conversation

russpos
Copy link

@russpos russpos commented Dec 16, 2011

Add opts namespace, which holds all options and values. Unlike the Commander object itself, it will have values for all options. Addresses open issues #29 and #19.

Includes updates to existing test suite.

…Commander object itself, it will have values for all options.
@@ -88,6 +88,7 @@ function Command(name) {
this.commands = [];
this.options = [];
this.args = [];
this.opts = {};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we change it to .options im down

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't that conflict with how it's currently using .options (as an Array of option objects)?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arrays can have props, iirc that one is private anyway

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to defeat the purpose of namespacing them. Take this example:

program
  .option('-a, --anchovies', 'Add anchovies?')
  .option('-c, --cheese [type]', 'optionally specify the type of cheese', 'mozzarella');

program.parse(['node', 'test']);
console.log('Expected: ', program.opts);
console.log('Not expected: ', program.options);

Ouput:

Expected:  { anchovies: false, cheese: 'mozzarella' }
Not expected:  [ { flags: '-a, --anchovies',
    required: 0,
    optional: 0,
    bool: true,
    short: '-a',
    long: '--anchovies',
    description: 'Add anchovies?' },
{ flags: '-c, --cheese [type]',
    required: 0,
    optional: -14,
    bool: true,
    short: '-c',
    long: '--cheese',
    description: 'optionally specify the type of cheese' },
anchovies: false,
cheese: 'mozzarella' ]

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's just rename the internal one to _options

@georgesnelling
Copy link

Any decision on fixing this? I hit it when I tried to create an option named --version which failed in a very weird way because conflicted with an internal method. Seems pretty bad to have these namespaces conflict. And creating an option named --version seems like a pretty common case, as are several others. No opinion about this particular fix, but would like to see the namespaces separated somehow. Thanks TJ.

@tj
Copy link
Owner

tj commented Apr 14, 2012

@georgesnelling --version is mapped to .version(val) so people can see what version your program is, you dont have to handle that one manually

@georgesnelling
Copy link

Headsmack! I've been using it so long that I didn't bother reading the docs when I hit the problem, but went straight to issues. It does fail in a rather confusing way if you miss that nice feature, but that's a corner case. Thanks for all your work. -g

@georgesnelling
Copy link

Played with this a bit more. Aesthetically, shouldn't the module throw if the user enters an option that is reserved?

@SomeKittens
Copy link
Collaborator

Can you bring this up to date with Commander?

@thethomaseffect
Copy link
Collaborator

Closed as out of date, thanks for the pull though!

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