dthree / vorpal

Node's framework for interactive CLIs
http://vorpal.js.org
MIT License
5.64k stars 280 forks source link

Pass minimist options to command/options #19

Closed simov closed 9 years ago

simov commented 9 years ago

:wave:

Since you are using minimist to parse the command options you have to allow your users to pass minimist options as well.

For example I may want to pass {string: ['a', 'address']} for my connect command to indicate that the address option should be treated as string always:

app$ connect --address 0x890765a99f0c8a98415756df2d821093ad0d273f
{ options: { address: 7.822966968792672e+47 } }

I may want to be strict about each command option, that's why probably an options argument for the command function would be good.

dthree commented 9 years ago

Could you please clarify? Like perhaps give a code example of what you would expect.

simov commented 9 years ago
Vorpal
var vorpal = require('vorpal')
  , notereth = vorpal()

notereth
  .command('compile', 'Compiles a contract')
  .option('-a, --address', 'Contract address')
  .action(function (args, done) {
    console.log(args)
    done()
  })

notereth
  .delimiter('notereth$')
  .show()
notereth$ compile --address 0x890765a99f0c8a98415756df2d821093ad0d273f
{ options: { address: 7.822966968792672e+47 } }
Minimist
var minimist = require('minimist')
  , argv = minimist(process.argv.slice(2))
console.log(argv)
$ node test.js --address 0x890765a99f0c8a98415756df2d821093ad0d273f
{ _: [], address: 7.822966968792672e+47 }
Minimist + Options
var minimist = require('minimist')
  , argv = minimist(process.argv.slice(2), {string: ['a', 'address']})
console.log(argv)
$ node test.js --address 0x890765a99f0c8a98415756df2d821093ad0d273f
{ _: [], address: '0x890765a99f0c8a98415756df2d821093ad0d273f' }
simov commented 9 years ago

I'm expecting the hexadecimal hash address option to be a string, always. You can check out the minimist's options here.

dthree commented 9 years ago

Okay. Could I get your opinion on which you would prefer?

.option('-a, --address', 'desc', 'string')

or maybe:

vorpal.command('foo <bar>')
  .option('-a, --address', 'desc')
  .types({
    string: ['a', 'address'],
    boolean: ['bar']
  })
  // ...

I'm not going to support all of the minimist options, as Vorpal does a lot more than just use minimist, so they don't all apply, but type-casting, I can agree with.

simov commented 9 years ago

Ok, I haven't used any other minimist options so far, so probably they won't be needed, plus it may not blend well within your app.

1: How are you going to distinguish between description and type string?

.option('-a, --address', 'desc', 'string')

2: Question: Is the following types method going to work for all of the options in this command? If that's the case: :+1:

vorpal.command('foo <bar>')
  .option('-a, --address', 'desc')
  .option('-b, --boo', 'desc')
  .option('-p, --poo', 'desc')
  .types({
    string: ['a', 'address', 'b', 'boo']
  })
dthree commented 9 years ago

On 1, was just thinking it would always be an optional 3rd param. Don't really like it though.

On 2, yes - you got it. I like this one better as well. Does types sound like an intuitive method name?

simov commented 9 years ago

Sure, .types() sounds good. If it's documented, then no problem.

dthree commented 9 years ago

kk

dthree commented 9 years ago

Okay this should be good, and I added it to the wiki.

https://github.com/dthree/vorpal/wiki/API-|-vorpal.command#commandtypesobject

simov commented 9 years ago

Thanks :+1:

dthree commented 9 years ago

No probs :)

AljoschaMeyer commented 9 years ago

Documentation should probably also contain:


What is your opinion on adding further validation options, e.g. 'nonzero', 'positive', 'nonempty', 'wordcharacters', arbitrary regex, 'divisible by 23', 'among the 500 most common english words', ...

Drawing the line on which to include is pretty arbitrary. But the first few examples could be very handy.

simov commented 9 years ago

@AljoschaMeyer linking to the minimist's docs would be good idd, but all the rest regarding validation should be part of another module IMO. And most probably there is such module already, it just have to be hooked in here.

dthree commented 9 years ago

@AljoschaMeyer minimist basically supports string and boolean, and minimist is far more battle-tested than Vorpal is in the present. Doesn't seem necessary to get beyond that, and I intend Vorpal's API to have been invented based on necessity, not ideas.

So if I get several other people asking for complex typecasting, I'll add it. And then in that case, I would probably just make a method for fancy validation.

Right now boolean and string are supported. It's a wiki, so feel free to add clarifying data if you want :+1: You can also link to minimist for sure. Don't put the hard list, but mention minimist and link to it like this.