Skip to content

Commit

Permalink
feat: requiresArg is now simply an alias for nargs(1) (#1054)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: requiresArg now has significantly different error output, matching nargs.
  • Loading branch information
bcoe authored Jan 22, 2018
1 parent 2b56812 commit a3ddacc
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 57 deletions.
34 changes: 0 additions & 34 deletions lib/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,40 +54,6 @@ module.exports = function validation (yargs, usage, y18n) {
}
}

// make sure that any args that require an
// value (--foo=bar), have a value.
self.missingArgumentValue = function missingArgumentValue (argv) {
const options = yargs.getOptions()
if (options.requiresArg.length > 0) {
const missingRequiredArgs = []
const defaultValues = new Set([true, false, ''])

options.requiresArg.forEach((key) => {
// if the argument is not set in argv no need to check
// whether a right-hand-side has been provided.
if (!argv.hasOwnProperty(key)) return

const value = argv[key]
// if a value is explicitly requested,
// flag argument as missing if it does not
// look like foo=bar was entered.
if (defaultValues.has(value) ||
(Array.isArray(value) && !value.length)) {
missingRequiredArgs.push(key)
}
})

if (missingRequiredArgs.length > 0) {
usage.fail(__n(
'Missing argument value: %s',
'Missing argument values: %s',
missingRequiredArgs.length,
missingRequiredArgs.join(', ')
))
}
}
}

// make sure all the required arguments are present.
self.requiredArguments = function requiredArguments (argv) {
const demandedOptions = yargs.getDemandedOptions()
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"string-width": "^2.0.0",
"which-module": "^2.0.0",
"y18n": "^3.2.1",
"yargs-parser": "^8.1.0"
"yargs-parser": "^9.0.2"
},
"devDependencies": {
"chai": "^4.1.2",
Expand Down
8 changes: 4 additions & 4 deletions test/usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ describe('usage tests', () => {
' --version Show version number [boolean]',
' --foo, -f foo option',
' --bar, -b bar option',
'Missing argument value: foo'
'Not enough arguments following: f'
])
})

Expand Down Expand Up @@ -722,7 +722,7 @@ describe('usage tests', () => {
' --version Show version number [boolean]',
' --foo, -f foo option',
' --bar, -b bar option',
'Missing argument values: foo, bar'
'Not enough arguments following: bar'
])
})
})
Expand Down Expand Up @@ -754,7 +754,7 @@ describe('usage tests', () => {
' --version Show version number [boolean]',
' --foo, -f foo option',
' --bar, -b bar option',
'Missing argument value: foo'
'Not enough arguments following: f'
])
})
})
Expand All @@ -769,7 +769,7 @@ describe('usage tests', () => {
.argv
)

r.errors[1].should.equal('Missing argument values: foo, bar')
r.errors[1].should.equal('Not enough arguments following: bar')
})
})

Expand Down
6 changes: 3 additions & 3 deletions test/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ describe('validation tests', () => {
.option('w', {type: 'string', requiresArg: true})
.parse('-w', (err, argv, output) => {
expect(err).to.exist
expect(err).to.have.property('message', 'Missing argument value: w')
expect(err).to.have.property('message', 'Not enough arguments following: w')
return done()
})
})
Expand All @@ -451,7 +451,7 @@ describe('validation tests', () => {
.option('w', {type: 'boolean', requiresArg: true})
.parse('-w', (err, argv, output) => {
expect(err).to.exist
expect(err).to.have.property('message', 'Missing argument value: w')
expect(err).to.have.property('message', 'Not enough arguments following: w')
return done()
})
})
Expand All @@ -461,7 +461,7 @@ describe('validation tests', () => {
.option('w', {type: 'array', requiresArg: true})
.parse('-w', (err, argv, output) => {
expect(err).to.exist
expect(err).to.have.property('message', 'Missing argument value: w')
expect(err).to.have.property('message', 'Not enough arguments following: w')
return done()
})
})
Expand Down
1 change: 0 additions & 1 deletion test/yargs.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ describe('yargs dsl tests', () => {
defaultDescription: {},
choices: {},
coerce: {},
requiresArg: [],
skipValidation: [],
count: [],
normalize: [],
Expand Down
16 changes: 2 additions & 14 deletions yargs.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function Yargs (processArgs, cwd, parentRequire) {
groups = {}

const arrayOptions = [
'array', 'boolean', 'string', 'requiresArg', 'skipValidation',
'array', 'boolean', 'string', 'skipValidation',
'count', 'normalize', 'number'
]

Expand Down Expand Up @@ -204,7 +204,7 @@ function Yargs (processArgs, cwd, parentRequire) {

self.requiresArg = function (keys) {
argsert('<array|string>', [keys], arguments.length)
populateParserHintArray('requiresArg', keys)
populateParserHintObject(self.nargs, false, 'narg', keys, 1)
return self
}

Expand Down Expand Up @@ -965,17 +965,6 @@ function Yargs (processArgs, cwd, parentRequire) {
options.__ = y18n.__
options.configuration = pkgUp()['yargs'] || {}

// numbers are defaulted to `undefined`, which makes it impossible to verify requiresArg
// so _unlike_ other types, we will implicitly add them to narg when requiresArg is true
const numberTyped = new Set(options.number)
options.requiresArg
.filter(key => numberTyped.has(key))
.forEach(key => {
// if narg _and_ requiresArg are configured for an option,
// we should probably throw an error somewhere indicating conflicting configuration
options.narg[key] = 1
})

const parsed = Parser.detailed(args, options)
let argv = parsed.argv
if (parseContext) argv = Object.assign({}, argv, parseContext)
Expand Down Expand Up @@ -1118,7 +1107,6 @@ function Yargs (processArgs, cwd, parentRequire) {
self._runValidation = function runValidation (argv, aliases, positionalMap, parseErrors) {
if (parseErrors) throw new YError(parseErrors.message)
validation.nonOptionCount(argv)
validation.missingArgumentValue(argv)
validation.requiredArguments(argv)
if (strict) validation.unknownArguments(argv, aliases, positionalMap)
validation.customChecks(argv, aliases)
Expand Down

0 comments on commit a3ddacc

Please sign in to comment.