-
Notifications
You must be signed in to change notification settings - Fork 995
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
feat: make positional arguments fully configurable #967
Conversation
lib/command.js
Outdated
@@ -239,64 +246,73 @@ module.exports = function command (yargs, usage, validation) { | |||
argv._ = argv._.slice(context.commands.length) // nuke the current commands | |||
const demanded = commandHandler.demanded.slice(0) | |||
const optional = commandHandler.optional.slice(0) | |||
const parseOptions = { |
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.
perhaps refactor the population of these parser options into a separate helper based on the command string, since they're also required for .positional()
.
@jeffijoe @ruimarinho @tkarls @atian25 @popomore @matatk @sloanlance I just took on this major refactor of how we parse commands in yargs to try to:
I would love your feedback -- I'm feeling pretty excited about this update to commands. |
@bcoe very nice, great work! |
cbc83f4
to
c9fb402
Compare
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.
Nice work :)
@@ -4,3 +4,4 @@ node_modules/ | |||
*.swp | |||
test.js | |||
coverage | |||
package-lock.json |
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.
😎
@popomore, @hax, @Morishiri, @zkat, @jeffijoe this work is now available on the candidate release of You can play with it by running:
|
Thanks for working on this; it is looking very promising. I have two questions...
|
@matatk try this on the most recent branch (npm yargs@next): const argv = require('./')
.command('$0 [port]', 'start the server', (yargs) => {
yargs
.positional('port', {
describe: 'port to bind on',
default: 5000
})
})
.argv I could imagine an alias for this that looks like this: const argv = require('./')
.usage('$0 [port]', (yargs) => {
yargs
.positional('port', {
describe: 'port to bind on',
default: 5000
})
})
.argv I wonder if this might be overkill though -- my feeling is it's probably better to encourage people to move towards using |
@bcoe that's brill; just what I was after—super-easy to configure positional arguments for the implicit default command and I get to refer to the positional arguments by name; thanks :-). |
@bcoe This looks great! Just be sure to xref in the docs for |
Introduces the
.positional()
method, which makes positional arguments fully configurable (but wait there's more, positional arguments will also be included separately in help output):TODO:
demanded
andarray
based on the command string.$0
to represent a default command (so that it's more intuitive that commands can be used to handle top-level operations).get translations forwe can do this post-hoc.Positionals:
(help wanted!).see: #541, #570, #959, etc
cc: @tkarls, @hax, @sloanlance, @iarna, etc.