Allow implies and conflicts to accept array values

[MattSturgeon]

Fixes #921 and fixes #845

npm test passes, but I haven't tried to run yargs with the changes, nor added any new tests (sorry).

I've also not updated the docs.

[bcoe]

@MattSturgeon should have checked open pulls before issues 😛

thanks for taking a stab at this; why don't you join the slack when you have a moment, and we can coordinate landing this feature.

[MattSturgeon]

// Kinda sucks having to replace the array object instead of just mutating it,
// plus we have to type conflicting[key] twice
conflicting[key] = conflicting[key].concat(value)

// About the same speed as concat + assign, but we get to keep the same array
// object. Still kinda ugly though
Array.prototype.push.apply(conflicting[key], value)

// Twice as fast, less hacky than push.apply, but even more verbose
for (var i = 0, l = implied[key].length; i < l; i++) {
    implied[key].push(value[i])
}

// Should be similar to above, but slightly cleaner
// I think this is my favourite.
for (var i in value) {
    implied[key].push(i)
}

// I didn't consider forEach() since it is known to be way slower than a for loop

[MattSturgeon]

Rebased on master using ES6 style, added a check for valid implies/conflicts values which throws a YError if passed - this causes a test to fail, so I'll need to update the tests.

FYI the diff reads much better with whitespace ignored because some of the indention had to change.

[bcoe]

the reason that tests are failing is that there's this weird edge-case feature where an argument can imply a certain # of positional arguments in _:

yargs.implies({
  foo: 4
})

So the second argument can be a number, or a string, or an array with your design. Rather than pulling in the YError object and throwing your own error, I would pull in the argsert library and do this:

  self.implies = function implies (key, value) {
    argsert('<string|object> [array|number|string]', [key, value], arguments.length)

    if (typeof key === 'object') {
      Object.keys(key).forEach((k) => {
        self.implies(k, key[k])
      })
    } else {
      yargs.global(key)
      if (!implied[key]) {
        implied[key] = []
      }
      if (Array.isArray(value)) {
        value.forEach((i) => self.implies(key, i))
      } else {
        implied[key].push(value)
      }
    }
  }```

[MattSturgeon]

Thanks for the suggestion - it looks much cleaner now.

[bcoe]

this is looking pretty good 👍 we're definitely going to want to update docs to reflect this new feature, and to add a test that shows it off.

[bcoe]

@MattSturgeon I've released your work to the next tag of yargs, I'd love your help testing (since the next release of yargs is pretty large).

npm i yargs@next --save

[Morishiri]

I've found an issue here.

If you specify array value for implies and this implications are failed then we are getting output multiplied by array length.

For example:

.option('all', {
    describe: 'My description',
    boolean: true,
    implies: ['svn-url', 'svn-branch', 'svn-user', 'svn-password']
})

without specified any implied option will result in this error message:

Implications failed:
    all -> svn-url all -> svn-branch all -> svn-user all -> svn-password all -> svn-url all -> svn-branch all -> svn-user all -> svn-password all -> svn-url all -> svn-branch all -> svn-user all -> svn-password all -> svn-url all -> svn-branch all -> svn-user all -> svn-password

I will try to create pull request as soon as I will find a moment to do so if you are willing to accept one.

[MattSturgeon]

@Morishiri you should probably open a new issue instead of commenting on this one. Are you saying the error message should be cleaned up and less verbose?

[Morishiri]

Yes, because now it shows all implications (not only failed) multiplied by implication count. I've created a pull request fixing this. #954 You may want to look into it.