-
Notifications
You must be signed in to change notification settings - Fork 994
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
working on a new API for commands that creates less redundant code #368
Conversation
650a021
to
8c51940
Compare
it('defaults to appropriate version # when yargs is installed normally', function (done) { | ||
testCmd('./normal-bin.js', [ '--version' ], function (buf) { | ||
buf.should.match(/9\.9\.9/) | ||
if (!isWindows()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need a new dependency for that? Why not just use process.platform !== 'win32'
?
Big fan of the new style, looking forward to taking advantage of it in a couple little tools I've written. Mostly it is the module style that I'll be using, will be able to more cleanly divide up my code. The globals I'll probably lean on in very simple tools and stay away from in more complex ones. |
}) | ||
.help() | ||
.argv | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it may be better to stick with the same structure of an example that we use above, i.e:
var argv = require('yargs')
.command('get', 'make a get HTTP request', {
url: {
default: 'http://yargs.js.org/'
}
})
.help()
.argv
The change in structure might confuse people (even though you mention that this is a yargs
instance), and knowing people don't always read text and skip to code, we might run into some trouble :o What do you think?
Edit: same goes for L484-487
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lrlna I've updated the documentation for command modules which I think are the most elegant approach we can direct people towards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bcoe 🆒, I like it.
working on a new API for commands that creates less redundant code
@bcoe a maybe-trivial clarification on the API: the intended purpose of the Example: #!/usr/bin/env node
import yargs from 'yargs';
import do_a from './do_a.js';
import do_b from './do_b.js';
yargs
.command('do_a', 'Does A', {/* a options */}, do_a)
.command('do_b', 'Does B', {/* b options */}, do_b); (I ask because in the example you save in a variable the |
Is there any way to specify the equivalent of the last argument to |
I'm working on a new API for commands that makes writing them less redundant:
builder
for describing command-specific help, and ahandler
that will get argv passed to it.global
options is being introduced, to eliminate the need for setting the same option over and over again on each command.foo <foo> [bar]
is being introduced.The definition of commands has now been split into a:
builder
: which is used to describe a command.handler
: which is used to handle a command when it is invoked.I would like to make it so that you could also provide a module to
command
, you would write a module like this:And would provide it to a command like this:
I think this is pretty killer, and splitting out the builder and handler makes this possible what do you think @nexdrew?
You can now specify
global
options using.global()
or theglobal: true
shorthand on options.These options will not be reset when passed to commands, by default
help
,version
, are treated this way (since this is the behavior folks usually want).see: #363, #362