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

default value interferes with passing context to sub-commands #724

Closed
bcoe opened this issue Dec 2, 2016 · 3 comments
Closed

default value interferes with passing context to sub-commands #724

bcoe opened this issue Dec 2, 2016 · 3 comments
Labels

Comments

@bcoe
Copy link
Member

bcoe commented Dec 2, 2016

setting a default value for a global option overrides the context object.

yargs()
          .command('batman', 'batman command', function (yargs) {
            yargs.command('robin', 'robin command', {}, function (_argv) {
              argv = _argv
            })
          })
          .option('state', {
            default: undefined,
            global: true
          })
          .parse('batman robin --what', {
            state: 'grumpy but rich'
          }, function (_err, argv, _output) {})
@bcoe bcoe added the bug label Dec 2, 2016
@maxrimue
Copy link
Member

maxrimue commented Jan 4, 2017

@bcoe I think I tracked down the problem. It seems like argv will always take precedence over any context passed to .parse(). For example, this will fail:

var parsed = yargs.parse('--foo bar', {
  foo: 'not bar'
})

parsed.foo.should.equal('not bar')

If I'm not totally wrong here, it seems that the problem lies in L875, where assign is called to merge context and argv, but in a way that argv takes precedence. Changing the order in this statement made the aforementioned test case pass.

Isn't the context object meant to take precedence over argv?

Edit: Never mind, finally got this 😄 I guess the problem is that when we merge argv and context, the default values are already in argv.

@maxrimue
Copy link
Member

@bcoe I would propose we add a new parameter for the parser like: Parser.detailed(args, options, state) so that the parser can apply state in precedence over default values and give back the argv like that. What do you think? Would take on this when I know what to do.

@bcoe
Copy link
Member Author

bcoe commented Jan 20, 2017

@maxrimue before we add any more configuration options; I think I'd be tempted to just swap the order of precedence. I argue that context should take precedence.

Would you be okay with making this one of the breaking changes in #742 (CC: @nexdrew).

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

No branches or pull requests

2 participants