Skip to content

Commit

Permalink
Options with multiple:true set must always return an array. Fixed a c…
Browse files Browse the repository at this point in the history
…ase where that was not so (where a scalar defaultValue was supplied and no option values passed on the command line). Fixes #24
  • Loading branch information
75lb committed Jan 19, 2017
1 parent a04ff8f commit c343257
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 26 deletions.
5 changes: 3 additions & 2 deletions lib/command-line-args.js
Expand Up @@ -34,8 +34,9 @@ module.exports = commandLineArgs
* ])
* ```
*/
function commandLineArgs (definitions, argv) {
definitions = new Definitions(definitions)
function commandLineArgs (optionDefinitions, argv) {
const definitions = new Definitions()
definitions.load(optionDefinitions)
argv = new Argv(argv)
argv.expandOptionEqualsNotation()
argv.expandGetoptNotation()
Expand Down
41 changes: 21 additions & 20 deletions lib/definitions.js
Expand Up @@ -12,10 +12,9 @@ const t = require('typical')
/**
* @alias module:definitions
*/
class Definitions {
constructor (definitions) {
this.list = []
arrayify(definitions).forEach(def => this.list.push(new Definition(def)))
class Definitions extends Array {
load (definitions) {
arrayify(definitions).forEach(def => this.push(new Definition(def)))
this.validate()
}

Expand All @@ -24,15 +23,15 @@ class Definitions {
* @returns {string}
*/
validate (argv) {
const someHaveNoName = this.list.some(def => !def.name)
const someHaveNoName = this.some(def => !def.name)
if (someHaveNoName) {
halt(
'NAME_MISSING',
'Invalid option definitions: the `name` property is required on each definition'
)
}

const someDontHaveFunctionType = this.list.some(def => def.type && typeof def.type !== 'function')
const someDontHaveFunctionType = this.some(def => def.type && typeof def.type !== 'function')
if (someDontHaveFunctionType) {
halt(
'INVALID_TYPE',
Expand All @@ -42,7 +41,7 @@ class Definitions {

let invalidOption

const numericAlias = this.list.some(def => {
const numericAlias = this.some(def => {
invalidOption = def
return t.isDefined(def.alias) && t.isNumber(def.alias)
})
Expand All @@ -53,7 +52,7 @@ class Definitions {
)
}

const multiCharacterAlias = this.list.some(def => {
const multiCharacterAlias = this.some(def => {
invalidOption = def
return t.isDefined(def.alias) && def.alias.length !== 1
})
Expand All @@ -64,7 +63,7 @@ class Definitions {
)
}

const hypenAlias = this.list.some(def => {
const hypenAlias = this.some(def => {
invalidOption = def
return def.alias === '-'
})
Expand All @@ -75,23 +74,23 @@ class Definitions {
)
}

const duplicateName = hasDuplicates(this.list.map(def => def.name))
const duplicateName = hasDuplicates(this.map(def => def.name))
if (duplicateName) {
halt(
'DUPLICATE_NAME',
'Two or more option definitions have the same name'
)
}

const duplicateAlias = hasDuplicates(this.list.map(def => def.alias))
const duplicateAlias = hasDuplicates(this.map(def => def.alias))
if (duplicateAlias) {
halt(
'DUPLICATE_ALIAS',
'Two or more option definitions have the same alias'
)
}

const duplicateDefaultOption = hasDuplicates(this.list.map(def => def.defaultOption))
const duplicateDefaultOption = hasDuplicates(this.map(def => def.defaultOption))
if (duplicateDefaultOption) {
halt(
'DUPLICATE_DEFAULT_OPTION',
Expand All @@ -106,8 +105,10 @@ class Definitions {
*/
createOutput () {
const output = {}
this.list.forEach(def => {
if (t.isDefined(def.defaultValue)) output[def.name] = def.defaultValue
this.forEach(def => {
if (t.isDefined(def.defaultValue)) {
output[def.name] = def.multiple ? arrayify(def.defaultValue) : def.defaultValue
}
if (Array.isArray(output[def.name])) {
output[def.name]._initial = true
}
Expand All @@ -121,23 +122,23 @@ class Definitions {
*/
get (arg) {
return option.short.test(arg)
? this.list.find(def => def.alias === option.short.name(arg))
: this.list.find(def => def.name === option.long.name(arg))
? this.find(def => def.alias === option.short.name(arg))
: this.find(def => def.name === option.long.name(arg))
}

getDefault () {
return this.list.find(def => def.defaultOption === true)
return this.find(def => def.defaultOption === true)
}

isGrouped () {
return this.list.some(def => def.group)
return this.some(def => def.group)
}

whereGrouped () {
return this.list.filter(containsValidGroup)
return this.filter(containsValidGroup)
}
whereNotGrouped () {
return this.list.filter(def => !containsValidGroup(def))
return this.filter(def => !containsValidGroup(def))
}

}
Expand Down
3 changes: 2 additions & 1 deletion test/class-argv.js
Expand Up @@ -31,7 +31,8 @@ runner.test('.expandGetoptNotation() with values', function () {
})

runner.test('.validate()', function () {
const definitions = new Definitions([
const definitions = new Definitions()
definitions.load([
{ name: 'one', type: Number }
])

Expand Down
9 changes: 6 additions & 3 deletions test/class-definitions.js
Expand Up @@ -6,17 +6,20 @@ const Definitions = require('../lib/definitions')
const runner = new TestRunner()

runner.test('.createOutput()', function () {
const definitions = new Definitions([ { name: 'one', defaultValue: 'eins' } ])
const definitions = new Definitions()
definitions.load([ { name: 'one', defaultValue: 'eins' } ])
a.deepStrictEqual(definitions.createOutput(), { one: 'eins' })
})

runner.test('.get()', function () {
const definitions = new Definitions([ { name: 'one', defaultValue: 'eins' } ])
const definitions = new Definitions()
definitions.load([ { name: 'one', defaultValue: 'eins' } ])
a.strictEqual(definitions.get('--one').name, 'one')
})

runner.test('.validate()', function () {
a.throws(function () {
const definitions = new Definitions([ { name: 'one' }, { name: 'one' } ]) // eslint-disable-line no-unused-vars
const definitions = new Definitions()
definitions.load([ { name: 'one' }, { name: 'one' } ])
})
})
15 changes: 15 additions & 0 deletions test/default-value.js
Expand Up @@ -48,3 +48,18 @@ runner.test('default value: falsy default values', function () {
two: false
})
})

runner.test('default value: is arrayifed if multiple set', function () {
const defs = [
{ name: 'one', defaultValue: 0, multiple: true }
]

let argv = []
a.deepStrictEqual(cliArgs(defs, argv), {
one: [ 0 ]
})
argv = [ '--one', '2' ]
a.deepStrictEqual(cliArgs(defs, argv), {
one: [ '2' ]
})
})

0 comments on commit c343257

Please sign in to comment.