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

Also flatten-duplicate-arrays when used with dot-notation #78

Closed
wants to merge 1 commit into from

Conversation

JeroenVdb
Copy link

Background: yargs/yargs#777

When using dot-notations I also want arrays to be flattened. This was the behaviour in Yargs 5 but not anymore in Yargs 6.

# GOOD
$ node good-yargs-parser.js --metric.list AA --metric.list BB --metricList CC --metricList DD
{ _: [],
  metric: { list: [ 'AA', 'BB' ] },
  metricList: [ 'CC', 'DD' ],
  '$0': 'yargstest.js' }

# BAD
$ node bad-yargs-parser.js --metric.list AA --metric.list BB --metricList CC --metricList DD
{ _: [],
  metric: { list: [ 'AA', ['BB'] ] },
  metricList: [ 'CC', 'DD' ],
  '$0': 'yargstest.js' }

@bcoe
Copy link
Member

bcoe commented Feb 18, 2017

@JeroenVdb at this time we don't yet fully support setting types for inner keys on objects, e.g.,

foo.bar: array, I think that adding this support could potentially lead to a combinatoric explosion of little tweaks we need to make in the parser -- I think I'd prefer that, if you want to enforce types on an object, you instead apply a coerce function on the top-level key:

something like this:

var argv = require('./')('--foo.a 99 --foo.b 33')
  .coerce('foo', function (obj) {
    obj.a = obj.a * 2
    obj.b = obj.b * 2
    return obj
  })
  .argv

console.log(argv)

Having said this!, I think you're correct in your analysis of yargs/yargs#777:_

It feels like --metric.list AA --metric.list BB should create an array on the list key, not two arrays.

Would you be willing to modify your patch to bring us back to the 5.x behavior?

@JeroenVdb
Copy link
Author

@bcoe I don't think I understand correct. Doesn't my patch restore the previous 5.X behaviour?

@bcoe
Copy link
Member

bcoe commented Feb 20, 2017

@JeroenVdb I just didn't like that we were starting to check foo.bar for object variables ... perhaps this is the minimal change to get behavior back to a reasonable spot though 👍

I'm going to be at Hack Illinois this coming weekend, working with students on yargs, expect a lot of things to land (also, this is a good time to get additional issues open).

@bcoe
Copy link
Member

bcoe commented Apr 15, 2017

@JeroenVdb closing in favor of: #86 (which I believe solves the more general case).

@bcoe bcoe closed this Apr 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants