Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: address ambiguity between nargs of 1 and requiresArg (#1572)
  • Loading branch information
bcoe committed Mar 7, 2020
1 parent c36c571 commit a5edc32
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 4 deletions.
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -29,7 +29,7 @@
"string-width": "^4.2.0",
"which-module": "^2.0.0",
"y18n": "^4.0.0",
"yargs-parser": "^17.1.0"
"yargs-parser": "^18.0.0"
},
"devDependencies": {
"c8": "^7.0.0",
Expand Down
72 changes: 72 additions & 0 deletions test/yargs.js
Expand Up @@ -2428,4 +2428,76 @@ describe('yargs dsl tests', () => {
}, 5)
})
})

// See: https://github.com/yargs/yargs/issues/1098
it('should allow array and requires arg to be used in conjunction', () => {
const argv = yargs(['-i', 'item1', 'item2', 'item3'])
.option('i', {
alias: 'items',
type: 'array',
requiresArg: true
})
.argv
argv.items.should.eql(['item1', 'item2', 'item3'])
argv.i.should.eql(['item1', 'item2', 'item3'])
})

// See: https://github.com/yargs/yargs/issues/1570
describe('"nargs" with "array"', () => {
it('should not consume more than nargs items', () => {
const argv = yargs(['-i', 'item1', 'item2', '-i', 'item3', 'item4'])
.option('i', {
alias: 'items',
type: 'array',
nargs: 1
})
.argv
argv.items.should.eql(['item1', 'item3'])
argv.i.should.eql(['item1', 'item3'])
argv._.should.eql(['item2', 'item4'])
})

it('should apply nargs with higher precedence than requiresArg: true', () => {
const argv = yargs(['-i', 'item1', 'item2', '-i', 'item3', 'item4'])
.option('i', {
alias: 'items',
type: 'array',
nargs: 1,
requiresArg: true
})
.argv
argv.items.should.eql(['item1', 'item3'])
argv.i.should.eql(['item1', 'item3'])
argv._.should.eql(['item2', 'item4'])
})

// TODO: make this work with aliases, using a check similar to
// checkAllAliases() in yargs-parser.
it('should apply nargs with higher precedence than requiresArg()', () => {
const argv = yargs(['-i', 'item1', 'item2', '-i', 'item3', 'item4'])
.option('items', {
alias: 'i',
type: 'array',
nargs: 1
})
.requiresArg(['items'])
.argv
argv.items.should.eql(['item1', 'item3'])
argv.i.should.eql(['item1', 'item3'])
argv._.should.eql(['item2', 'item4'])
})

it('should raise error if not enough values follow nargs key', (done) => {
yargs
.option('i', {
alias: 'items',
type: 'array',
nargs: 1
})
.parse(['-i'], (err) => {
err.message.should.match(/Not enough arguments following: i/)
return done()
})
})
})
})
15 changes: 12 additions & 3 deletions yargs.js
Expand Up @@ -235,9 +235,18 @@ function Yargs (processArgs, cwd, parentRequire) {
return self
}

self.requiresArg = function (keys) {
argsert('<array|string>', [keys], arguments.length)
populateParserHintObject(self.nargs, false, 'narg', keys, 1)
self.requiresArg = function (keys, value) {
argsert('<array|string|object> [number]', [keys], arguments.length)
// If someone configures nargs at the same time as requiresArg,
// nargs should take precedent,
// see: https://github.com/yargs/yargs/pull/1572
// TODO: make this work with aliases, using a check similar to
// checkAllAliases() in yargs-parser.
if (typeof keys === 'string' && options.narg[keys]) {
return self
} else {
populateParserHintObject(self.requiresArg, false, 'narg', keys, NaN)
}
return self
}

Expand Down

0 comments on commit a5edc32

Please sign in to comment.