From a440783b10bbc7bb6a6965364eac9c34ed35f857 Mon Sep 17 00:00:00 2001 From: Myk Melez Date: Wed, 26 Apr 2017 17:04:42 -0700 Subject: [PATCH 1/4] test a default option with a value equal to its default value --- test/partial.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/partial.js b/test/partial.js index 2e96aaf..a0c5053 100644 --- a/test/partial.js +++ b/test/partial.js @@ -45,6 +45,18 @@ runner.test('partial: defaultOption 2', function () { }) }) +runner.test('partial: defaultOption with value equal to defaultValue', function () { + const definitions = [ + { name: 'file', type: String, defaultOption: true, defaultValue: 'file1' }, + ] + const argv = [ 'file1', '--two=3', '--four', '5' ] + const options = commandLineArgs(definitions, { argv, partial: true }) + a.deepStrictEqual(options, { + file: 'file1', + _unknown: [ '--two', '3', '--four', '5' ] + }) +}) + runner.test('partial: multiple', function () { const definitions = [ { name: 'files', type: String, multiple: true } From e1e01a0995e2bf05a2f81ad73a79d954bb5cf153 Mon Sep 17 00:00:00 2001 From: Lloyd Brookes Date: Mon, 1 May 2017 15:29:46 +0100 Subject: [PATCH 2/4] Setting a non-multiple defaultOption with a value equal to its defaultValue corrected, fixes #48 --- example/mocha.js | 2 - lib/command-line-args.js | 9 ++-- lib/definitions.js | 6 ++- lib/grouped-output.js | 12 +++-- lib/output.js | 102 +++++++++++++++++++++++---------------- lib/value-arg.js | 30 ++++++++++++ test/class-output.js | 20 +++++--- test/default-value.js | 6 +-- test/grouping.js | 3 +- test/partial.js | 2 +- 10 files changed, 124 insertions(+), 68 deletions(-) create mode 100644 lib/value-arg.js diff --git a/example/mocha.js b/example/mocha.js index 36356a8..0250396 100644 --- a/example/mocha.js +++ b/example/mocha.js @@ -24,12 +24,10 @@ Example output: $ mocha example/mocha.js --value 3 --no-colors - Array #indexOf() ✓ should pass when the supplied value is between 1 and 3 - 1 passing (7ms) */ diff --git a/lib/command-line-args.js b/lib/command-line-args.js index f7558cb..4ff15ee 100644 --- a/lib/command-line-args.js +++ b/lib/command-line-args.js @@ -30,8 +30,6 @@ function commandLineArgs (optionDefinitions, options) { const Definitions = require('./definitions') const option = require('./option') const Argv = require('./argv') - const Output = require('./output') - const GroupedOutput = require('./grouped-output') const definitions = new Definitions() definitions.load(optionDefinitions) @@ -41,16 +39,17 @@ function commandLineArgs (optionDefinitions, options) { argv.expandGetoptNotation() argv.validate(definitions, options) - const output = definitions.isGrouped() ? new GroupedOutput(definitions, options) : new Output(definitions, options) + const OutputClass = definitions.isGrouped() ? require('./grouped-output') : require('./output') + const output = new OutputClass(definitions, options) let optionName - argv.forEach(arg => { + for (const arg of argv) { if (option.isOption(arg)) { optionName = output.set(arg) ? undefined : arg } else { optionName = output.set(optionName, arg) ? undefined : optionName } - }) + } return output.toObject() } diff --git a/lib/definitions.js b/lib/definitions.js index 1623fc8..bc97600 100644 --- a/lib/definitions.js +++ b/lib/definitions.js @@ -14,10 +14,15 @@ const t = require('typical') */ class Definitions extends Array { load (definitions) { + this.clear() arrayify(definitions).forEach(def => this.push(new Definition(def))) this.validate() } + clear () { + this.length = 0 + } + /** * validate option definitions * @returns {string} @@ -123,7 +128,6 @@ class Definitions extends Array { whereNotGrouped () { return this.filter(def => !containsValidGroup(def)) } - } function halt (name, message) { diff --git a/lib/grouped-output.js b/lib/grouped-output.js index d579801..87f6a3f 100644 --- a/lib/grouped-output.js +++ b/lib/grouped-output.js @@ -5,24 +5,26 @@ const Output = require('./output') class GroupedOutput extends Output { toObject () { + const superOutput = super.toObject() + delete superOutput._unknown const grouped = { - _all: this.output + _all: superOutput } if (this.unknown.length) grouped._unknown = this.unknown this.definitions.whereGrouped().forEach(def => { arrayify(def.group).forEach(groupName => { grouped[groupName] = grouped[groupName] || {} - if (t.isDefined(this.output[def.name])) { - grouped[groupName][def.name] = this.output[def.name] + if (t.isDefined(this.output[def.name].value)) { + grouped[groupName][def.name] = this.output[def.name].value } }) }) this.definitions.whereNotGrouped().forEach(def => { - if (t.isDefined(this.output[def.name])) { + if (t.isDefined(this.output[def.name].value)) { if (!grouped._none) grouped._none = {} - grouped._none[def.name] = this.output[def.name] + grouped._none[def.name] = this.output[def.name].value } }) return grouped diff --git a/lib/output.js b/lib/output.js index 1597e71..759b782 100644 --- a/lib/output.js +++ b/lib/output.js @@ -2,25 +2,35 @@ const t = require('typical') const arrayify = require('array-back') +class OutputValue { + constructor (value) { + this.value = value + this.hasDefaultArrayValue = false + this.isDefaultValue = false + this.isSuppliedValue = false + this.valueSource = 'unknown' + } +} + class Output { - constructor (defs, options) { + constructor (definitions, options) { this.options = options || {} this.output = {} - this.hasDefaultArrayValue = {} this.unknown = [] - const Definitions = require('./definitions') - this.definitions = new Definitions() - this.definitions.load(defs) + this.definitions = definitions this._assignDefaultValues() } _assignDefaultValues () { this.definitions.forEach(def => { if (t.isDefined(def.defaultValue)) { - this.output[def.name] = def.multiple ? arrayify(def.defaultValue) : def.defaultValue if (def.multiple) { - this.hasDefaultArrayValue[def.name] = true + this.output[def.name] = new OutputValue(arrayify(def.defaultValue)) + this.output[def.name].hasDefaultArrayValue = true + } else { + this.output[def.name] = new OutputValue(def.defaultValue) } + this.output[def.name].valueSource = 'default' } }) } @@ -31,11 +41,8 @@ class Output { set (optionArg, value) { /* if the value marker is present at the beginning, strip it */ const option = require('./option') - const reBeginsWithValueMarker = new RegExp('^' + option.VALUE_MARKER) - const isOptionValueNotationValue = reBeginsWithValueMarker.test(value) - value = isOptionValueNotationValue - ? value.replace(reBeginsWithValueMarker, '') - : value + const ValueArg = require('./value-arg') + const valueArg = new ValueArg(value) /* lookup the definition.. if no optionArg (--option) was supplied, use the defaultOption */ let def @@ -43,15 +50,20 @@ class Output { def = this.definitions.get(optionArg) } else { def = this.definitions.getDefault() - if (def) { - /* if it's not a `multiple` and the defaultOption has already been set, move on */ - if (!def.multiple && t.isDefined(this.output[def.name]) && this.output[def.name] !== def.defaultValue) { - if (t.isDefined(value)) this.unknown.push(value) - return true - /* in the case we're setting an --option=value value on a multiple defaultOption, tag the value onto the previous unknown */ - } else if (def.multiple && isOptionValueNotationValue && t.isDefined(this.output[def.name])) { - if (t.isDefined(value) && this.unknown.length) { - this.unknown[this.unknown.length - 1] += `=${value}` + const currentValue = this.output[def && def.name] && this.output[def && def.name] + + /* check for and handle unknown values */ + if (def && currentValue && t.isDefined(currentValue.value)) { + if (def.multiple) { + /* in the case we're setting an --option=value value on a multiple defaultOption, tag the value onto the previous unknown */ + if (valueArg.isOptionValueNotationValue() && valueArg.isDefined() && this.unknown.length) { + this.unknown[this.unknown.length - 1] += `=${valueArg.value}` + return true + } + } else { + /* currentValue has already been set by argv,log this value as unknown and move on */ + if (currentValue.valueSource === 'argv') { + if (valueArg.isDefined()) this.unknown.push(valueArg.value) return true } } @@ -61,58 +73,64 @@ class Output { /* if there's no definition or defaultOption, do nothing and continue */ if (!def) { if (t.isDefined(optionArg)) this.unknown.push(optionArg) - if (t.isDefined(value)) { - if (isOptionValueNotationValue) { - this.unknown[this.unknown.length - 1] += `=${value}` + if (valueArg.isDefined()) { + if (valueArg.isOptionValueNotationValue()) { + this.unknown[this.unknown.length - 1] += `=${valueArg.value}` } else { - this.unknown.push(value) + this.unknown.push(valueArg.value) } } return true } const name = def.name + this.output[name] = this.output[name] || new OutputValue() + const outputValue = this.output[name] /* if not already initialised, set a `multiple` value to a new array. */ - if (def.multiple && !t.isDefined(this.output[name])) this.output[name] = [] + if (def.multiple && !t.isDefined(outputValue.value)) outputValue.value = [] /* for boolean types, set value to `true`. For all other types run value through setter function. */ if (def.isBoolean()) { - value = true - } else if (t.isDefined(value)) { - value = def.type ? def.type(value) : value + valueArg.value = true + } else if (valueArg.isDefined()) { + valueArg.value = def.type ? def.type(valueArg.value) : valueArg.value } - if (t.isDefined(value)) { - if (Array.isArray(this.output[name])) { - if (this.hasDefaultArrayValue[name]) { - this.output[name] = [ value ] - delete this.hasDefaultArrayValue[name] + /* set output value */ + if (valueArg.isDefined()) { + outputValue.valueSource = 'argv' + if (Array.isArray(outputValue.value)) { + if (outputValue.hasDefaultArrayValue) { + outputValue.value = [ valueArg.value ] + outputValue.hasDefaultArrayValue = false } else { - this.output[name].push(value) + outputValue.value.push(valueArg.value) } return false } else { - this.output[name] = value + outputValue.value = valueArg.value return true } } else { - if (!Array.isArray(this.output[name])) this.output[name] = null + if (!Array.isArray(outputValue.value) && outputValue.valueSource === 'unknown') outputValue.value = null return false } } get (name) { - return this.output[name] + return this.output[name] && this.output[name].value } toObject () { - let output + let output = Object.assign({}, this.output) if (this.options.partial && this.unknown.length) { - output = Object.assign({}, this.output) output._unknown = this.unknown - } else { - output = this.output + } + for (const prop in output) { + if (prop !== '_unknown') { + output[prop] = output[prop].value + } } return output } diff --git a/lib/value-arg.js b/lib/value-arg.js new file mode 100644 index 0000000..d2a7c65 --- /dev/null +++ b/lib/value-arg.js @@ -0,0 +1,30 @@ +'use strict' +const t = require('typical') +const option = require('./option') +const reBeginsWithValueMarker = new RegExp('^' + option.VALUE_MARKER) + +class ValueArg { + constructor (value) { + this.initialValue = value + this.value = this.stripMarker(value) + } + + isOptionValueNotationValue () { + return reBeginsWithValueMarker.test(this.initialValue) + } + + /** + * if the value marker is present at the beginning, strip it + */ + stripMarker (value) { + return this.isOptionValueNotationValue() + ? value.replace(reBeginsWithValueMarker, '') + : value + } + + isDefined () { + return t.isDefined(this.value) + } +} + +module.exports = ValueArg diff --git a/test/class-output.js b/test/class-output.js index 82b54de..b30884e 100644 --- a/test/class-output.js +++ b/test/class-output.js @@ -2,21 +2,23 @@ const TestRunner = require('test-runner') const a = require('assert') const Output = require('../lib/output') +const Definitions = require('../lib/definitions') const runner = new TestRunner() runner.test('output.set(name): initial value', function () { - let definitions = [ + let definitions = new Definitions() + definitions.load([ { name: 'one', type: Number } - ] + ]) let output = new Output(definitions) a.strictEqual(output.get('one'), undefined) output.set('--one') a.strictEqual(output.get('one'), null) - definitions = [ + definitions.load([ { name: 'one', type: Boolean } - ] + ]) output = new Output(definitions) a.strictEqual(output.get('one'), undefined) output.set('--one') @@ -24,9 +26,10 @@ runner.test('output.set(name): initial value', function () { }) runner.test('output.set(name, value)', function () { - const definitions = [ + const definitions = new Definitions() + definitions.load([ { name: 'one', type: Number, defaultValue: 1 } - ] + ]) const output = new Output(definitions) a.strictEqual(output.get('one'), 1) output.set('--one', '2') @@ -34,9 +37,10 @@ runner.test('output.set(name, value)', function () { }) runner.test('output.set(name, value): multiple', function () { - const definitions = [ + const definitions = new Definitions() + definitions.load([ { name: 'one', type: Number, multiple: true, defaultValue: [ 1 ] } - ] + ]) const output = new Output(definitions) a.deepStrictEqual(output.get('one'), [ 1 ]) output.set('--one', '2') diff --git a/test/default-value.js b/test/default-value.js index 3272d21..ef17b67 100644 --- a/test/default-value.js +++ b/test/default-value.js @@ -73,7 +73,7 @@ runner.test('default value: is arrayifed if multiple set', function () { runner.test('default value: combined with defaultOption', function () { const defs = [ - { name: 'path', defaultOption: true, defaultValue: './' }, + { name: 'path', defaultOption: true, defaultValue: './' } ] let argv = [ '--path', 'test' ] @@ -92,7 +92,7 @@ runner.test('default value: combined with defaultOption', function () { runner.test('default value: combined with multiple and defaultOption', function () { const defs = [ - { name: 'path', multiple: true, defaultOption: true, defaultValue: './' }, + { name: 'path', multiple: true, defaultOption: true, defaultValue: './' } ] let argv = [ '--path', 'test1', 'test2' ] @@ -119,7 +119,7 @@ runner.test('default value: combined with multiple and defaultOption', function runner.test('default value: array default combined with multiple and defaultOption', function () { const defs = [ - { name: 'path', multiple: true, defaultOption: true, defaultValue: [ './' ] }, + { name: 'path', multiple: true, defaultOption: true, defaultValue: [ './' ] } ] let argv = [ '--path', 'test1', 'test2' ] diff --git a/test/grouping.js b/test/grouping.js index 6d85cd0..367e115 100644 --- a/test/grouping.js +++ b/test/grouping.js @@ -12,7 +12,8 @@ const definitions = [ const runner = new TestRunner() runner.test('groups', function () { - a.deepStrictEqual(commandLineArgs(definitions, { argv: [ '--one', '1', '--two', '2', '--three', '3' ] }), { + const output = commandLineArgs(definitions, { argv: [ '--one', '1', '--two', '2', '--three', '3' ] }) + a.deepStrictEqual(output, { a: { one: '1', two: '2' diff --git a/test/partial.js b/test/partial.js index a0c5053..4de5ca5 100644 --- a/test/partial.js +++ b/test/partial.js @@ -47,7 +47,7 @@ runner.test('partial: defaultOption 2', function () { runner.test('partial: defaultOption with value equal to defaultValue', function () { const definitions = [ - { name: 'file', type: String, defaultOption: true, defaultValue: 'file1' }, + { name: 'file', type: String, defaultOption: true, defaultValue: 'file1' } ] const argv = [ 'file1', '--two=3', '--four', '5' ] const options = commandLineArgs(definitions, { argv, partial: true }) From 36af5e537a060694e2b21e22d86179e53a2a6acc Mon Sep 17 00:00:00 2001 From: Lloyd Brookes Date: Mon, 1 May 2017 22:36:23 +0100 Subject: [PATCH 3/4] refactor, more tests --- lib/argv.js | 4 +- lib/command-line-args.js | 11 ++- lib/option.js | 34 ++------ lib/output.js | 142 ++++++++++++++++++++-------------- lib/value-arg.js | 18 +---- test/class-output.js | 65 ++++++++++++++-- test/exceptions.js | 2 + test/partial.js | 12 +++ test/type-boolean-multiple.js | 18 ----- test/type-boolean.js | 11 +++ test/type-number-multiple.js | 32 -------- test/type-number.js | 35 ++++++++- test/type-other-multiple.js | 31 -------- test/type-other.js | 25 ++++++ 14 files changed, 244 insertions(+), 196 deletions(-) delete mode 100644 test/type-boolean-multiple.js delete mode 100644 test/type-number-multiple.js delete mode 100644 test/type-other-multiple.js diff --git a/lib/argv.js b/lib/argv.js index bd0c060..6e1e367 100644 --- a/lib/argv.js +++ b/lib/argv.js @@ -33,7 +33,7 @@ class Argv extends Array { if (this.some(optEquals.test.bind(optEquals))) { const expandedArgs = [] this.forEach(arg => { - const matches = arg.match(optEquals.re) + const matches = arg.match(optEquals) if (matches) { expandedArgs.push(matches[1], option.VALUE_MARKER + matches[2]) } else { @@ -53,7 +53,7 @@ class Argv extends Array { const combinedArg = option.combined const hasGetopt = this.some(combinedArg.test.bind(combinedArg)) if (hasGetopt) { - findReplace(this, combinedArg.re, arg => { + findReplace(this, combinedArg, arg => { arg = arg.slice(1) return arg.split('').map(letter => '-' + letter) }) diff --git a/lib/command-line-args.js b/lib/command-line-args.js index 4ff15ee..a0d2d32 100644 --- a/lib/command-line-args.js +++ b/lib/command-line-args.js @@ -28,9 +28,7 @@ module.exports = commandLineArgs function commandLineArgs (optionDefinitions, options) { options = options || {} const Definitions = require('./definitions') - const option = require('./option') const Argv = require('./argv') - const definitions = new Definitions() definitions.load(optionDefinitions) const argv = new Argv() @@ -43,11 +41,16 @@ function commandLineArgs (optionDefinitions, options) { const output = new OutputClass(definitions, options) let optionName + const option = require('./option') for (const arg of argv) { if (option.isOption(arg)) { - optionName = output.set(arg) ? undefined : arg + optionName = output.setFlag(arg) ? undefined : arg } else { - optionName = output.set(optionName, arg) ? undefined : optionName + if (optionName) { + optionName = output.setOptionValue(optionName, arg) ? undefined : optionName + } else { + optionName = output.setValue(arg) ? undefined : optionName + } } } diff --git a/lib/option.js b/lib/option.js index 428b998..bd10618 100644 --- a/lib/option.js +++ b/lib/option.js @@ -1,32 +1,14 @@ 'use strict' -/** - * A module for testing for and extracting names from options (e.g. `--one`, `-o`) - * - * @module option - * @private - */ - -class Arg { - constructor (re) { - this.re = re - } - +class ArgRegExp extends RegExp { name (arg) { - return arg.match(this.re)[1] + return arg.match(this)[1] } - test (arg) { - return this.re.test(arg) - } -} - -const option = { - short: new Arg(/^-([^\d-])$/), - long: new Arg(/^--(\S+)/), - combined: new Arg(/^-([^\d-]{2,})$/), - isOption (arg) { return this.short.test(arg) || this.long.test(arg) }, - optEquals: new Arg(/^(--\S+?)=(.*)/), - VALUE_MARKER: '552f3a31-14cd-4ced-bd67-656a659e9efb' // must be unique } -module.exports = option +exports.short = new ArgRegExp('^-([^\\d-])$') +exports.long = new ArgRegExp('^--(\\S+)') +exports.combined = new ArgRegExp('^-([^\\d-]{2,})$') +exports.isOption = arg => exports.short.test(arg) || exports.long.test(arg) +exports.optEquals = new ArgRegExp('^(--\\S+?)=(.*)') +exports.VALUE_MARKER = '552f3a31-14cd-4ced-bd67-656a659e9efb' // must be unique diff --git a/lib/output.js b/lib/output.js index 759b782..53303e3 100644 --- a/lib/output.js +++ b/lib/output.js @@ -31,50 +31,73 @@ class Output { this.output[def.name] = new OutputValue(def.defaultValue) } this.output[def.name].valueSource = 'default' + } else { + /* guarantees a `multiple` returns an array, even without a defaultValue */ + if (def.multiple) this.output[def.name] = new OutputValue([]) } }) } - /** - * Return `true` when an option value was set and is not a multiple. Return `false` if option was a multiple or if a value was not yet set. - */ - set (optionArg, value) { - /* if the value marker is present at the beginning, strip it */ - const option = require('./option') + setFlag (optionArg) { + const def = this.definitions.get(optionArg) + + if (def) { + this.output[def.name] = this.output[def.name] || new OutputValue() + const outputValue = this.output[def.name] + + /* for boolean types, set value to `true`. For all other types run value through setter function. */ + if (def.isBoolean()) { + if (Array.isArray(outputValue.value)) { + outputValue.value.push(true) + } else { + outputValue.value = true + } + return true + } else { + if (!Array.isArray(outputValue.value) && outputValue.valueSource === 'unknown') outputValue.value = null + return false + } + } else { + this.unknown.push(optionArg) + return true + } + } + + setOptionValue (optionArg, value) { const ValueArg = require('./value-arg') const valueArg = new ValueArg(value) - /* lookup the definition.. if no optionArg (--option) was supplied, use the defaultOption */ - let def - if (t.isDefined(optionArg)) { - def = this.definitions.get(optionArg) - } else { - def = this.definitions.getDefault() - const currentValue = this.output[def && def.name] && this.output[def && def.name] + const def = this.definitions.get(optionArg) - /* check for and handle unknown values */ - if (def && currentValue && t.isDefined(currentValue.value)) { - if (def.multiple) { - /* in the case we're setting an --option=value value on a multiple defaultOption, tag the value onto the previous unknown */ - if (valueArg.isOptionValueNotationValue() && valueArg.isDefined() && this.unknown.length) { - this.unknown[this.unknown.length - 1] += `=${valueArg.value}` - return true + if (def) { + this.output[def.name] = this.output[def.name] || new OutputValue() + const outputValue = this.output[def.name] + + /* set output value */ + if (valueArg.isDefined()) { + /* run value through setter function. */ + valueArg.value = def.type ? def.type(valueArg.value) : valueArg.value + outputValue.valueSource = 'argv' + if (Array.isArray(outputValue.value)) { + if (outputValue.hasDefaultArrayValue) { + outputValue.value = [ valueArg.value ] + outputValue.hasDefaultArrayValue = false + } else { + outputValue.value.push(valueArg.value) } + return false } else { - /* currentValue has already been set by argv,log this value as unknown and move on */ - if (currentValue.valueSource === 'argv') { - if (valueArg.isDefined()) this.unknown.push(valueArg.value) - return true - } + outputValue.value = valueArg.value + return true } + } else { + if (!Array.isArray(outputValue.value) && outputValue.valueSource === 'unknown') outputValue.value = null + return false } - } - - /* if there's no definition or defaultOption, do nothing and continue */ - if (!def) { - if (t.isDefined(optionArg)) this.unknown.push(optionArg) + } else { + /* handle unknowns in the case there's no definition or defaultOption */ if (valueArg.isDefined()) { - if (valueArg.isOptionValueNotationValue()) { + if (valueArg.isOptionValueNotationValue) { this.unknown[this.unknown.length - 1] += `=${valueArg.value}` } else { this.unknown.push(valueArg.value) @@ -82,39 +105,44 @@ class Output { } return true } + } - const name = def.name - this.output[name] = this.output[name] || new OutputValue() - const outputValue = this.output[name] - - /* if not already initialised, set a `multiple` value to a new array. */ - if (def.multiple && !t.isDefined(outputValue.value)) outputValue.value = [] + /** + * Return `true` when an option value was set and is not a multiple. Return `false` if option was a multiple or if a value was not yet set. + */ + setValue (value) { + const ValueArg = require('./value-arg') + const valueArg = new ValueArg(value) - /* for boolean types, set value to `true`. For all other types run value through setter function. */ - if (def.isBoolean()) { - valueArg.value = true - } else if (valueArg.isDefined()) { - valueArg.value = def.type ? def.type(valueArg.value) : valueArg.value - } + /* use the defaultOption */ + const def = this.definitions.getDefault() - /* set output value */ - if (valueArg.isDefined()) { - outputValue.valueSource = 'argv' - if (Array.isArray(outputValue.value)) { - if (outputValue.hasDefaultArrayValue) { - outputValue.value = [ valueArg.value ] - outputValue.hasDefaultArrayValue = false + /* handle unknown values in the case a value was already set on a defaultOption */ + if (def) { + const currentValue = this.output[def.name] + if (valueArg.isDefined() && currentValue && t.isDefined(currentValue.value)) { + if (def.multiple) { + /* in the case we're setting an --option=value value on a multiple defaultOption, tag the value onto the previous unknown */ + if (valueArg.isOptionValueNotationValue && this.unknown.length) { + this.unknown[this.unknown.length - 1] += `=${valueArg.value}` + return true + } } else { - outputValue.value.push(valueArg.value) + /* currentValue has already been set by argv,log this value as unknown and move on */ + if (currentValue.valueSource === 'argv') { + this.unknown.push(valueArg.value) + return true + } } - return false - } else { - outputValue.value = valueArg.value - return true } + return this.setOptionValue(`--${def.name}`, value) } else { - if (!Array.isArray(outputValue.value) && outputValue.valueSource === 'unknown') outputValue.value = null - return false + if (valueArg.isOptionValueNotationValue) { + this.unknown[this.unknown.length - 1] += `=${valueArg.value}` + } else { + this.unknown.push(valueArg.value) + } + return true } } diff --git a/lib/value-arg.js b/lib/value-arg.js index d2a7c65..a29d55b 100644 --- a/lib/value-arg.js +++ b/lib/value-arg.js @@ -5,21 +5,9 @@ const reBeginsWithValueMarker = new RegExp('^' + option.VALUE_MARKER) class ValueArg { constructor (value) { - this.initialValue = value - this.value = this.stripMarker(value) - } - - isOptionValueNotationValue () { - return reBeginsWithValueMarker.test(this.initialValue) - } - - /** - * if the value marker is present at the beginning, strip it - */ - stripMarker (value) { - return this.isOptionValueNotationValue() - ? value.replace(reBeginsWithValueMarker, '') - : value + this.isOptionValueNotationValue = reBeginsWithValueMarker.test(value) + /* if the value marker is present at the value beginning, strip it */ + this.value = value ? value.replace(reBeginsWithValueMarker, '') : value } isDefined () { diff --git a/test/class-output.js b/test/class-output.js index b30884e..9c9312b 100644 --- a/test/class-output.js +++ b/test/class-output.js @@ -6,14 +6,14 @@ const Definitions = require('../lib/definitions') const runner = new TestRunner() -runner.test('output.set(name): initial value', function () { +runner.test('output.setFlag(name): initial value', function () { let definitions = new Definitions() definitions.load([ { name: 'one', type: Number } ]) let output = new Output(definitions) a.strictEqual(output.get('one'), undefined) - output.set('--one') + output.setFlag('--one') a.strictEqual(output.get('one'), null) definitions.load([ @@ -21,28 +21,79 @@ runner.test('output.set(name): initial value', function () { ]) output = new Output(definitions) a.strictEqual(output.get('one'), undefined) - output.set('--one') + output.setFlag('--one') a.strictEqual(output.get('one'), true) }) -runner.test('output.set(name, value)', function () { +runner.test('output.setOptionValue(name, value)', function () { const definitions = new Definitions() definitions.load([ { name: 'one', type: Number, defaultValue: 1 } ]) const output = new Output(definitions) a.strictEqual(output.get('one'), 1) - output.set('--one', '2') + output.setOptionValue('--one', '2') a.strictEqual(output.get('one'), 2) }) -runner.test('output.set(name, value): multiple', function () { +runner.test('output.setOptionValue(name, value): multiple, defaultValue', function () { const definitions = new Definitions() definitions.load([ { name: 'one', type: Number, multiple: true, defaultValue: [ 1 ] } ]) const output = new Output(definitions) a.deepStrictEqual(output.get('one'), [ 1 ]) - output.set('--one', '2') + output.setOptionValue('--one', '2') a.deepStrictEqual(output.get('one'), [ 2 ]) }) + +runner.test('output.setOptionValue(name, value): multiple 2', function () { + const definitions = new Definitions() + definitions.load([ + { name: 'one', type: Number, multiple: true } + ]) + const output = new Output(definitions) + a.deepStrictEqual(output.get('one'), [ ]) + output.setOptionValue('--one', '2') + a.deepStrictEqual(output.get('one'), [ 2 ]) + output.setOptionValue('--one', '3') + a.deepStrictEqual(output.get('one'), [ 2, 3 ]) +}) + +runner.test('output.setValue(value): no defaultOption', function () { + const definitions = new Definitions() + definitions.load([ + { name: 'one', type: Number } + ]) + const output = new Output(definitions) + a.deepStrictEqual(output.get('one'), undefined) + output.setValue('2') + a.deepStrictEqual(output.get('one'), undefined) + a.deepStrictEqual(output.unknown, [ '2' ]) +}) + +runner.test('output.setValue(value): with defaultOption', function () { + const definitions = new Definitions() + definitions.load([ + { name: 'one', type: Number, defaultOption: true } + ]) + const output = new Output(definitions) + a.deepStrictEqual(output.get('one'), undefined) + output.setValue('2') + a.deepStrictEqual(output.get('one'), 2) + a.deepStrictEqual(output.unknown, []) +}) + +runner.test('output.setValue(value): multiple', function () { + const definitions = new Definitions() + definitions.load([ + { name: 'one', multiple: true, defaultOption: true } + ]) + const output = new Output(definitions) + a.deepStrictEqual(output.get('one'), []) + output.setValue('1') + a.deepStrictEqual(output.get('one'), [ '1' ]) + output.setValue('2') + a.deepStrictEqual(output.get('one'), [ '1', '2' ]) + +}) diff --git a/test/exceptions.js b/test/exceptions.js index 50b73a6..2733c10 100644 --- a/test/exceptions.js +++ b/test/exceptions.js @@ -172,3 +172,5 @@ runner.test('err-invalid-definition: multiple defaultOption', function () { a.strictEqual(err.name, 'DUPLICATE_DEFAULT_OPTION') } }) + +runner.test('err-invalid-defaultOption: defaultOption on a Boolean type') diff --git a/test/partial.js b/test/partial.js index 4de5ca5..e55f3b2 100644 --- a/test/partial.js +++ b/test/partial.js @@ -57,6 +57,18 @@ runner.test('partial: defaultOption with value equal to defaultValue', function }) }) +runner.test('partial: defaultOption with value equal to defaultValue 2', function () { + const definitions = [ + { name: 'file', type: String, defaultOption: true, defaultValue: 'file1' } + ] + const argv = [ '--file', '--file=file1', '--two=3', '--four', '5' ] + const options = commandLineArgs(definitions, { argv, partial: true }) + a.deepStrictEqual(options, { + file: 'file1', + _unknown: [ '--two', '3', '--four', '5' ] + }) +}) + runner.test('partial: multiple', function () { const definitions = [ { name: 'files', type: String, multiple: true } diff --git a/test/type-boolean-multiple.js b/test/type-boolean-multiple.js deleted file mode 100644 index 74e6822..0000000 --- a/test/type-boolean-multiple.js +++ /dev/null @@ -1,18 +0,0 @@ -'use strict' -const TestRunner = require('test-runner') -const commandLineArgs = require('../') -const a = require('assert') - -const runner = new TestRunner() - -const definitions = [ - { name: 'array', type: Boolean, multiple: true } -] - -runner.test('type-boolean-multiple: 1', function () { - const argv = [ '--array', '--array', '--array' ] - const result = commandLineArgs(definitions, { argv }) - a.deepStrictEqual(result, { - array: [ true, true, true ] - }) -}) diff --git a/test/type-boolean.js b/test/type-boolean.js index d663bc5..ed58702 100644 --- a/test/type-boolean.js +++ b/test/type-boolean.js @@ -57,3 +57,14 @@ runner.test('type-boolean: global Boolean overridden', function () { { one: true } ) }) + +runner.test('type-boolean-multiple: 1', function () { + const definitions = [ + { name: 'array', type: Boolean, multiple: true } + ] + const argv = [ '--array', '--array', '--array' ] + const result = commandLineArgs(definitions, { argv }) + a.deepStrictEqual(result, { + array: [ true, true, true ] + }) +}) diff --git a/test/type-number-multiple.js b/test/type-number-multiple.js deleted file mode 100644 index 634f287..0000000 --- a/test/type-number-multiple.js +++ /dev/null @@ -1,32 +0,0 @@ -'use strict' -const TestRunner = require('test-runner') -const commandLineArgs = require('../') -const a = require('assert') - -const optionDefinitions = [ - { name: 'array', type: Number, multiple: true } -] - -const runner = new TestRunner() - -runner.test('number multiple: 1', function () { - const argv = [ '--array', '1', '2', '3' ] - const result = commandLineArgs(optionDefinitions, { argv }) - a.deepStrictEqual(result, { - array: [ 1, 2, 3 ] - }) - a.notDeepStrictEqual(result, { - array: [ '1', '2', '3' ] - }) -}) - -runner.test('number multiple: 2', function () { - const argv = [ '--array', '1', '--array', '2', '--array', '3' ] - const result = commandLineArgs(optionDefinitions, { argv }) - a.deepStrictEqual(result, { - array: [ 1, 2, 3 ] - }) - a.notDeepStrictEqual(result, { - array: [ '1', '2', '3' ] - }) -}) diff --git a/test/type-number.js b/test/type-number.js index c7bb879..db9bab8 100644 --- a/test/type-number.js +++ b/test/type-number.js @@ -3,13 +3,12 @@ const TestRunner = require('test-runner') const commandLineArgs = require('../') const a = require('assert') -const optionDefinitions = [ - { name: 'one', type: Number } -] - const runner = new TestRunner() runner.test('type-number: different values', function () { + const optionDefinitions = [ + { name: 'one', type: Number } + ] a.deepStrictEqual( commandLineArgs(optionDefinitions, { argv: [ '--one', '1' ] }), { one: 1 } @@ -25,3 +24,31 @@ runner.test('type-number: different values', function () { const result = commandLineArgs(optionDefinitions, { argv: [ '--one', 'asdf' ] }) a.ok(isNaN(result.one)) }) + +runner.test('number multiple: 1', function () { + const optionDefinitions = [ + { name: 'array', type: Number, multiple: true } + ] + const argv = [ '--array', '1', '2', '3' ] + const result = commandLineArgs(optionDefinitions, { argv }) + a.deepStrictEqual(result, { + array: [ 1, 2, 3 ] + }) + a.notDeepStrictEqual(result, { + array: [ '1', '2', '3' ] + }) +}) + +runner.test('number multiple: 2', function () { + const optionDefinitions = [ + { name: 'array', type: Number, multiple: true } + ] + const argv = [ '--array', '1', '--array', '2', '--array', '3' ] + const result = commandLineArgs(optionDefinitions, { argv }) + a.deepStrictEqual(result, { + array: [ 1, 2, 3 ] + }) + a.notDeepStrictEqual(result, { + array: [ '1', '2', '3' ] + }) +}) diff --git a/test/type-other-multiple.js b/test/type-other-multiple.js deleted file mode 100644 index 816aa7a..0000000 --- a/test/type-other-multiple.js +++ /dev/null @@ -1,31 +0,0 @@ -'use strict' -const TestRunner = require('test-runner') -const commandLineArgs = require('../') -const a = require('assert') - -const definitions = [ - { - name: 'file', - multiple: true, - type: function (file) { - return file - } - } -] - -const runner = new TestRunner() - -runner.test('type-other-multiple: different values', function () { - a.deepStrictEqual( - commandLineArgs(definitions, { argv: [ '--file', 'one.js' ] }), - { file: [ 'one.js' ] } - ) - a.deepStrictEqual( - commandLineArgs(definitions, { argv: [ '--file', 'one.js', 'two.js' ] }), - { file: [ 'one.js', 'two.js' ] } - ) - a.deepStrictEqual( - commandLineArgs(definitions, { argv: [ '--file' ] }), - { file: [] } - ) -}) diff --git a/test/type-other.js b/test/type-other.js index 04e5d0d..77ee10a 100644 --- a/test/type-other.js +++ b/test/type-other.js @@ -38,3 +38,28 @@ runner.test('type-other: broken custom type function', function () { commandLineArgs(definitions, { argv: [ '--file', 'one.js' ] }) }) }) + +runner.test('type-other-multiple: different values', function () { + const definitions = [ + { + name: 'file', + multiple: true, + type: function (file) { + return file + } + } + ] + + a.deepStrictEqual( + commandLineArgs(definitions, { argv: [ '--file', 'one.js' ] }), + { file: [ 'one.js' ] } + ) + a.deepStrictEqual( + commandLineArgs(definitions, { argv: [ '--file', 'one.js', 'two.js' ] }), + { file: [ 'one.js', 'two.js' ] } + ) + a.deepStrictEqual( + commandLineArgs(definitions, { argv: [ '--file' ] }), + { file: [] } + ) +}) From 2ce90aa9f01d00a5f0a3a86c7ecfe881fc610739 Mon Sep 17 00:00:00 2001 From: Lloyd Brookes Date: Tue, 2 May 2017 23:03:55 +0100 Subject: [PATCH 4/4] more tests, ensure 100% branch coverage --- .gitignore | 1 + lib/definition.js | 6 +--- lib/grouped-output.js | 15 +++++---- lib/output.js | 48 +++++++++------------------ test/bad-input.js | 18 +++++++++++ test/class-output.js | 1 - test/grouping.js | 75 +++++++++++++++++++++++++++++++++++++++---- 7 files changed, 112 insertions(+), 52 deletions(-) diff --git a/.gitignore b/.gitignore index bb48ed1..f71c9e2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ tmp .coveralls.yml +coverage diff --git a/lib/definition.js b/lib/definition.js index e6959e5..2879663 100644 --- a/lib/definition.js +++ b/lib/definition.js @@ -229,11 +229,7 @@ class OptionDefinition { } isBoolean (value) { - if (this.type) { - return this.type === Boolean || (t.isFunction(this.type) && this.type.name === 'Boolean') - } else { - return false - } + return this.type === Boolean || (t.isFunction(this.type) && this.type.name === 'Boolean') } } diff --git a/lib/grouped-output.js b/lib/grouped-output.js index 87f6a3f..383e243 100644 --- a/lib/grouped-output.js +++ b/lib/grouped-output.js @@ -1,5 +1,4 @@ 'use strict' -const t = require('typical') const arrayify = require('array-back') const Output = require('./output') @@ -13,18 +12,20 @@ class GroupedOutput extends Output { if (this.unknown.length) grouped._unknown = this.unknown this.definitions.whereGrouped().forEach(def => { - arrayify(def.group).forEach(groupName => { + const outputValue = this.output[def.name] + for (const groupName of arrayify(def.group)) { grouped[groupName] = grouped[groupName] || {} - if (t.isDefined(this.output[def.name].value)) { - grouped[groupName][def.name] = this.output[def.name].value + if (outputValue && outputValue.isDefined()) { + grouped[groupName][def.name] = outputValue.value } - }) + } }) this.definitions.whereNotGrouped().forEach(def => { - if (t.isDefined(this.output[def.name].value)) { + const outputValue = this.output[def.name] + if (outputValue && outputValue.isDefined()) { if (!grouped._none) grouped._none = {} - grouped._none[def.name] = this.output[def.name].value + grouped._none[def.name] = outputValue.value } }) return grouped diff --git a/lib/output.js b/lib/output.js index 53303e3..2579696 100644 --- a/lib/output.js +++ b/lib/output.js @@ -6,10 +6,12 @@ class OutputValue { constructor (value) { this.value = value this.hasDefaultArrayValue = false - this.isDefaultValue = false - this.isSuppliedValue = false this.valueSource = 'unknown' } + + isDefined () { + return t.isDefined(this.value) + } } class Output { @@ -69,40 +71,22 @@ class Output { const def = this.definitions.get(optionArg) - if (def) { - this.output[def.name] = this.output[def.name] || new OutputValue() - const outputValue = this.output[def.name] + this.output[def.name] = this.output[def.name] || new OutputValue() + const outputValue = this.output[def.name] - /* set output value */ - if (valueArg.isDefined()) { - /* run value through setter function. */ - valueArg.value = def.type ? def.type(valueArg.value) : valueArg.value - outputValue.valueSource = 'argv' - if (Array.isArray(outputValue.value)) { - if (outputValue.hasDefaultArrayValue) { - outputValue.value = [ valueArg.value ] - outputValue.hasDefaultArrayValue = false - } else { - outputValue.value.push(valueArg.value) - } - return false - } else { - outputValue.value = valueArg.value - return true - } + /* run value through setter function. */ + valueArg.value = def.type(valueArg.value) + outputValue.valueSource = 'argv' + if (Array.isArray(outputValue.value)) { + if (outputValue.hasDefaultArrayValue) { + outputValue.value = [ valueArg.value ] + outputValue.hasDefaultArrayValue = false } else { - if (!Array.isArray(outputValue.value) && outputValue.valueSource === 'unknown') outputValue.value = null - return false + outputValue.value.push(valueArg.value) } + return false } else { - /* handle unknowns in the case there's no definition or defaultOption */ - if (valueArg.isDefined()) { - if (valueArg.isOptionValueNotationValue) { - this.unknown[this.unknown.length - 1] += `=${valueArg.value}` - } else { - this.unknown.push(valueArg.value) - } - } + outputValue.value = valueArg.value return true } } diff --git a/test/bad-input.js b/test/bad-input.js index 250ebbe..8616343 100644 --- a/test/bad-input.js +++ b/test/bad-input.js @@ -28,3 +28,21 @@ runner.test('bad-input: handles arrays with relative paths', function () { colours: [ '../what', '../ever' ] }) }) + +runner.test('bad-input: empty string', function () { + const definitions = [ + { name: 'one', type: String }, + { name: 'two', type: Number }, + { name: 'three', type: Number, multiple: true }, + { name: 'four', type: String }, + { name: 'five', type: Boolean } + ] + const argv = [ '--one', '', '', '--two', '0', '--three=', '', '--four=', '--five=' ] + a.deepStrictEqual(commandLineArgs(definitions, { argv }), { + one: '', + two: 0, + three: [ 0, 0 ], + four: '', + five: true + }) +}) diff --git a/test/class-output.js b/test/class-output.js index 9c9312b..a931507 100644 --- a/test/class-output.js +++ b/test/class-output.js @@ -95,5 +95,4 @@ runner.test('output.setValue(value): multiple', function () { a.deepStrictEqual(output.get('one'), [ '1' ]) output.setValue('2') a.deepStrictEqual(output.get('one'), [ '1', '2' ]) - }) diff --git a/test/grouping.js b/test/grouping.js index 367e115..e415094 100644 --- a/test/grouping.js +++ b/test/grouping.js @@ -3,16 +3,16 @@ const TestRunner = require('test-runner') const commandLineArgs = require('../') const a = require('assert') -const definitions = [ - { name: 'one', group: 'a' }, - { name: 'two', group: 'a' }, - { name: 'three', group: 'b' } -] - const runner = new TestRunner() runner.test('groups', function () { - const output = commandLineArgs(definitions, { argv: [ '--one', '1', '--two', '2', '--three', '3' ] }) + const definitions = [ + { name: 'one', group: 'a' }, + { name: 'two', group: 'a' }, + { name: 'three', group: 'b' } + ] + const argv = [ '--one', '1', '--two', '2', '--three', '3' ] + const output = commandLineArgs(definitions, { argv }) a.deepStrictEqual(output, { a: { one: '1', @@ -57,3 +57,64 @@ runner.test('groups: multiple and _none', function () { } }) }) + +runner.test('groups: nothing set', function () { + const definitions = [ + { name: 'one', group: 'a' }, + { name: 'two', group: 'a' }, + { name: 'three', group: 'b' } + ] + const argv = [ ] + const output = commandLineArgs(definitions, { argv }) + a.deepStrictEqual(output, { + a: {}, + b: {}, + _all: {} + }) +}) + +runner.test('groups: nothing set with one ungrouped', function () { + const definitions = [ + { name: 'one', group: 'a' }, + { name: 'two', group: 'a' }, + { name: 'three' } + ] + const argv = [ ] + const output = commandLineArgs(definitions, { argv }) + a.deepStrictEqual(output, { + a: {}, + _all: {} + }) +}) + +runner.test('groups: two ungrouped, one set', function () { + const definitions = [ + { name: 'one', group: 'a' }, + { name: 'two', group: 'a' }, + { name: 'three' }, + { name: 'four' } + ] + const argv = [ '--three', '3' ] + const output = commandLineArgs(definitions, { argv }) + a.deepStrictEqual(output, { + a: {}, + _all: { three: '3' }, + _none: { three: '3' } + }) +}) + +runner.test('groups: two ungrouped, both set', function () { + const definitions = [ + { name: 'one', group: 'a' }, + { name: 'two', group: 'a' }, + { name: 'three' }, + { name: 'four' } + ] + const argv = [ '--three', '3', '--four', '4' ] + const output = commandLineArgs(definitions, { argv }) + a.deepStrictEqual(output, { + a: {}, + _all: { three: '3', four: '4' }, + _none: { three: '3', four: '4' } + }) +})