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

Data overload #515

Closed
wants to merge 3 commits into from
Closed

Data overload #515

wants to merge 3 commits into from

Conversation

ericuldall
Copy link

New features allow the following:

#!/usr/bin/env node
var program = require('commander');

program
  .version('0.0.1')
  .description('Test')
  .usage('[options] string')
  .option('-t, --type [value]', 'Type')
  .option('-f, --format', 'Format')
  .parse(process.argv);

var type = program.get('type'); //if exists return type, else returns false
var options = program.get(); //return all options, or empty object

var has = program.has('type', function(type){
    console.log(type); //will return if type exists
});
console.log(has); //boolean

var has_all = program.has_all(['type', 'format'], function(type, format){
    console.log(type, format); //will return if both exist
});
console.log(has_all); //boolean

}

@zhiyelee
Copy link
Collaborator

hold on this. New added get has will overwrite custom command with same name. #404

@ericuldall
Copy link
Author

So commands will have to be moved out of the objects main scope as well, for this to be viable?

@ericuldall
Copy link
Author

UPDATE: I'm not seeing any conflict with custom commands. Can you provide an example of this case?

@ericuldall
Copy link
Author

I've tested this with the following scenario:

cmd: mycmd
subcmd: mycmd-get

mycmd get -y

works as expected. Am I missing something?

@zhiyelee
Copy link
Collaborator

oh, sorry, it's about options.

Just want to give some feedback here to help others avoid a little pitfall: you cannot use --once as an argument, probably because the name collides with some internals of commander. Interestingly, the once argument will revert the expected behavior: myprog --once will set commander.once=undefined, so a falsy value, while running myprog will set it to a function as expected.

I don't want to introduce more these cases until we fix this issue.

@ericuldall
Copy link
Author

I think my pull request fixes that issue.

Options no longer get stored to the object directly.

Before you would have had:

program.once

Now you get:

program._data.once

The following is tested and working:

#!/usr/bin/env node
var program = require('commander');

program
  .version('0.0.1')
  .description('Test')
  .usage('[options] <file>|string')
  .option('-y, --yes', 'Something')
  .option('-o, --once', 'Once')
  .parse(process.argv);


program.has_all(['yes', 'once'], function(yes, once){
    console.log(yes, once);
});

@shadowspawn
Copy link
Collaborator

I am still investigating options. For now, I'll note that get() and has() are similar in name and purpose to the new Mapobject, and should probably behave in similar way.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map

@shadowspawn
Copy link
Collaborator

I am closing this in favour of #951, which builds on work in this Pull Request and aims for backwards compatibility.

Thank you for your contributions.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Nov 26, 2019

I have opened a new 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants