Option values should not be added to the command instance.

[badsyntax]

Adding the option values to the command object will only cause conflicts between the properties of the command object and of the command options.

Let's take this example:

program.option('--parse', 'parse');
program.option('--parse2', 'parse');

program.parse(['node', 'test', '--parse', '--parse2']);
console.log(program.parse); // undefined
console.log(program.parse2); // true

This seems like a pretty serious bug to me, certainly it's prevented me from naming my option 'parent' because of conflicts with this particular line: https://github.com/visionmedia/commander.js/blob/master/index.js#L155

Why can't the option values just be added to a new empty object?? Something like this;

program.option('--parse', 'parse');
program.option('--parse2', 'parse');

program.parse(['node', 'test', '--parse', '--parse2']);
console.log(program.input.parse); // true
console.log(program.input.parse2); // true

[JasonGiedymin]

Or you could have '--parse [my list of parses]' which you would then in your app split/parse appropriately

[tj]

it could be, it's just ugly and breaks backwards compat. we could do var opts = program....parse(args) to mitigate that though and maintain backwards compat without being super verbose

[badsyntax]

Ok guys, perhaps there's some misunderstanding here. Forget about parse, it was just an example.

Perhaps this code demonstrates the issue better:

var program = require('commander');

program.option('-o, --option');
program.option('-v, --version');
program.option('-a, --action');
program.option('-n, --normalize');
program.option('-e, --example');

program.parse(['node', 'test', '--option', '--version', '--action', '--normalize', '--example']);

console.log('option? %j', program.option);        // undefined
console.log('version? %j', program.version);      // undefined
console.log('action? %j', program.action);        // undefined
console.log('normalize? %j', program.normalize);  // undefined
console.log('example? %j', program.example);      // true

All of those console.log's should be outputting true. That is the crux of my bug report.

---

I understand it's pretty much impossible to fix this while keeping backwards compat. Tbh I don't understand why options were added to the command instance in the first place.

My understanding of the issue is as follows: options are saved on to the command instance, thus if I try to have an option with a name that matches a property/method of the Command object, then things break. The reason why I can't use one of ['option','version','action','normalize'] is because those are method names on the Command object, and things break with this logic.

It's so glaringly broken to me, I don't understand why others can't see this, or why no-one else has had issues with this. I have to assume there's something I'm missing here :/

[tj]

no I understand, it's a tradeoff, there's only a that short list of names you can't use, which yeah is certainly a "bug" but until we do a 3.0 like you say it can't really be fixed. Best we can do is return the options from the .parse() call

also a lot of those, such as normalize could be renamed to _normalize since it's just internal junk anyway

[badsyntax]

Oh I see now, thanks for clarifying. Personally I would certainly find it very useful if the options could be returned from .parse().

[tj]

damn looks like we return the left-over argv after already

[kbjr]

A fix for this doesn't necessarily have to break backward compat right away, options can be exposed in two places, and only remove them from the main program object when the next major release comes out. At least that way, people having this issue can start parsing those troublesome options (like --parse) now.

[yanickrochon]

I agree, options could be placed at two different places to keep BC. I too have a problem where I was wondering why this command :

node test add-role --name admin ---description "Admin role"

had args.description be a function!

The "list of names you can't use" makes things a little awkward and mixing parsed arguments with the command object is just wrong.

Or better yet, have the parsed arguments sent as second argument to the command action function.

command
    .description('Create and add a new role')
    .option('-n, --name <name>', 'the role name', String)
    .option('-d, --description <desc>', 'the role description', String)
    .action(function (command, args) {

    })
;

[cyrusdavid]

good point @yanickrochon!

[shadowspawn]

There is some good discussion in #404

[shadowspawn]

I have opened a Pull Request which allows storing option values separately rather than as command properties (access using .opts()), and passes the options (rather than the command) to the action handler.

See #1102

[shadowspawn]

Added . storeOptionsAsProperties() and . passCommandToAction() in Commander 4.1.

See: https://github.com/tj/commander.js#avoiding-option-name-clashes