Repository: yargs/yargs-parser
Pull request #83 (2017-03-08 18:11:24)

Merge with config when array
This is a fix for yargs/yargs#817.

markbirbeck: Shouldn't the coverage test just check the entire branch, rather than individual steps? I'm assuming that the coverage number appears to fall because I add my test in one commit before I add the code that passes that test in the next. This makes it easier to review.
bcoe: @markbirbeck thanks for the contribution (regarding coverage, this is a change in how coveralls works will not block the merging of this pull request).


markbirbeck: Thanks for the update @bcoe.
bcoe: @markbirbeck sorry to drop this on the floor for so long -- just working through a large backlog of issues now; having slept on this one. It comes to mind that we might want to make this a parser option that can be enabled or disabled. Here's a scenario I frequently run into:

* I'm configuring a cluster of servers, which are represented by an array; ['staging-www-1, 'staging-www-2] -- I might have this stored in a `config.json`.
* In production, I might wish to override this value with `['prod-www-1', 'prod-www-2']` -- with this new logic we'd instead end up with `['staging-www-1', 'staging-www-2', 'prod-www-1', 'prod-www-2']`?

Perhaps we simply add a new config option, `combine-arrays`, which defaults to `false`; if turned on it would combine merge arrays:

* if multiple arrays are given as arguments: `--arr 1 2 --arr 3 4`.
* or if a config file is provided along with an argument.
bcoe: @markbirbeck does my last comment make sense? still interested in landing this?

Patch:
diff --git a/.gitignore b/.gitignore
index 2454630..db9974f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,6 @@
+.idea
 .nyc_output
 node_modules
 .DS_Store
+package-lock.json
 ./test/fixtures/package.json
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3ad3df8..f9e7c62 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,27 @@
 
 All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.
 
+<a name="8.0.0"></a>
+# [8.0.0](https://github.com/yargs/yargs-parser/compare/v7.0.0...v8.0.0) (2017-10-05)
+
+
+### Bug Fixes
+
+* Ignore multiple spaces between arguments. ([#100](https://github.com/yargs/yargs-parser/issues/100)) ([d137227](https://github.com/yargs/yargs-parser/commit/d137227))
+
+
+### Features
+
+* allow configuration of prefix for boolean negation ([#94](https://github.com/yargs/yargs-parser/issues/94)) ([00bde7d](https://github.com/yargs/yargs-parser/commit/00bde7d))
+* reworking how numbers are parsed ([#104](https://github.com/yargs/yargs-parser/issues/104)) ([fba00eb](https://github.com/yargs/yargs-parser/commit/fba00eb))
+
+
+### BREAKING CHANGES
+
+* strings that fail `Number.isSafeInteger()` are no longer coerced into numbers. 
+
+
+
 <a name="7.0.0"></a>
 # [7.0.0](https://github.com/yargs/yargs-parser/compare/v6.0.1...v7.0.0) (2017-05-02)
 
diff --git a/index.js b/index.js
index 2ab9c2c..71d1a0c 100644
--- a/index.js
+++ b/index.js
@@ -270,9 +270,7 @@ function parse (args, opts) {
         }
       }
     } else {
-      argv._.push(
-        flags.strings['_'] || !isNumber(arg) ? arg : Number(arg)
-      )
+      argv._.push(maybeCoerceNumber('_', arg))
     }
   }
 
@@ -348,10 +346,8 @@ function parse (args, opts) {
   function setArg (key, val) {
     unsetDefaulted(key)
 
-    if (/-/.test(key) && !(flags.aliases[key] && flags.aliases[key].length) && configuration['camel-case-expansion']) {
-      var c = camelCase(key)
-      flags.aliases[key] = [c]
-      newAliases[c] = true
+    if (/-/.test(key) && configuration['camel-case-expansion']) {
+      addNewAlias(key, camelCase(key))
     }
 
     var value = processValue(key, val)
@@ -396,17 +392,23 @@ function parse (args, opts) {
     }
   }
 
+  function addNewAlias (key, alias) {
+    if (!(flags.aliases[key] && flags.aliases[key].length)) {
+      flags.aliases[key] = [alias]
+      newAliases[alias] = true
+    }
+    if (!(flags.aliases[alias] && flags.aliases[alias].length)) {
+      addNewAlias(alias, key)
+    }
+  }
+
   function processValue (key, val) {
     // handle parsing boolean arguments --foo=true --bar false.
     if (checkAllAliases(key, flags.bools) || checkAllAliases(key, flags.counts)) {
       if (typeof val === 'string') val = val === 'true'
     }
 
-    var value = val
-    if (!checkAllAliases(key, flags.strings) && !checkAllAliases(key, flags.coercions)) {
-      if (isNumber(val)) value = Number(val)
-      if (!isUndefined(val) && !isNumber(val) && checkAllAliases(key, flags.numbers)) value = NaN
-    }
+    var value = maybeCoerceNumber(key, val)
 
     // increment a count given as arg (either no value or value parsed as boolean)
     if (checkAllAliases(key, flags.counts) && (isUndefined(value) || typeof value === 'boolean')) {
@@ -421,6 +423,16 @@ function parse (args, opts) {
     return value
   }
 
+  function maybeCoerceNumber (key, value) {
+    if (!checkAllAliases(key, flags.strings) && !checkAllAliases(key, flags.coercions)) {
+      const shouldCoerceNumber = isNumber(value) && configuration['parse-numbers'] && (
+        Number.isSafeInteger(Math.floor(value))
+      )
+      if (shouldCoerceNumber || (!isUndefined(value) && checkAllAliases(key, flags.numbers))) value = Number(value)
+    }
+    return value
+  }
+
   // set args from config.json file, this should be
   // applied last so that defaults can be applied.
   function setConfig (argv) {
@@ -469,13 +481,13 @@ function parse (args, opts) {
       // if the value is an inner object and we have dot-notation
       // enabled, treat inner objects in config the same as
       // heavily nested dot notations (foo.bar.apple).
-      if (typeof value === 'object' && !Array.isArray(value) && configuration['dot-notation']) {
+      if (typeof value === 'object' && value !== null && !Array.isArray(value) && configuration['dot-notation']) {
         // if the value is an object but not an array, check nested object
         setConfigObject(value, fullKey)
       } else {
         // setting arguments via CLI takes precedence over
         // values within the config file.
-        if (!hasKey(argv, fullKey.split('.')) || (flags.defaulted[fullKey])) {
+        if (!hasKey(argv, fullKey.split('.')) || (flags.defaulted[fullKey]) || (flags.arrays[fullKey])) {
           setArg(fullKey, value)
         }
       }
@@ -513,13 +525,19 @@ function parse (args, opts) {
 
   function applyCoercions (argv) {
     var coerce
+    var applied = {}
     Object.keys(argv).forEach(function (key) {
-      coerce = checkAllAliases(key, flags.coercions)
-      if (typeof coerce === 'function') {
-        try {
-          argv[key] = coerce(argv[key])
-        } catch (err) {
-          error = err
+      if (!applied.hasOwnProperty(key)) { // If we haven't already coerced this option via one of its aliases
+        coerce = checkAllAliases(key, flags.coercions)
+        if (typeof coerce === 'function') {
+          try {
+            var value = coerce(argv[key])
+            ;([].concat(flags.aliases[key] || [], key)).forEach(ali => {
+              applied[ali] = argv[ali] = value
+            })
+          } catch (err) {
+            error = err
+          }
         }
       }
     })
@@ -558,9 +576,24 @@ function parse (args, opts) {
 
     if (!configuration['dot-notation']) keys = [keys.join('.')]
 
-    keys.slice(0, -1).forEach(function (key) {
-      if (o[key] === undefined) o[key] = {}
-      o = o[key]
+    keys.slice(0, -1).forEach(function (key, index) {
+      if (typeof o === 'object' && o[key] === undefined) {
+        o[key] = {}
+      }
+
+      if (typeof o[key] !== 'object' || Array.isArray(o[key])) {
+        // ensure that o[key] is an array, and that the last item is an empty object.
+        if (Array.isArray(o[key])) {
+          o[key].push({})
+        } else {
+          o[key] = [o[key], {}]
+        }
+
+        // we want to update the empty object at the end of the o[key] array, so set o to that object
+        o = o[key][o[key].length - 1]
+      } else {
+        o = o[key]
+      }
     })
 
     var key = keys[keys.length - 1]
@@ -602,8 +635,10 @@ function parse (args, opts) {
         flags.aliases[key].concat(key).forEach(function (x) {
           if (/-/.test(x) && configuration['camel-case-expansion']) {
             var c = camelCase(x)
-            flags.aliases[key].push(c)
-            newAliases[c] = true
+            if (c !== key && flags.aliases[key].indexOf(c) === -1) {
+              flags.aliases[key].push(c)
+              newAliases[c] = true
+            }
           }
         })
         flags.aliases[key].forEach(function (x) {
@@ -664,7 +699,6 @@ function parse (args, opts) {
   }
 
   function isNumber (x) {
-    if (!configuration['parse-numbers']) return false
     if (typeof x === 'number') return true
     if (/^0x[0-9a-f]+$/i.test(x)) return true
     return /^[-+]?(?:\d+(?:\.\d*)?|\.\d+)(e[-+]?\d+)?$/.test(x)
diff --git a/lib/tokenize-arg-string.js b/lib/tokenize-arg-string.js
index 23d39e1..6c8d23e 100644
--- a/lib/tokenize-arg-string.js
+++ b/lib/tokenize-arg-string.js
@@ -2,17 +2,23 @@
 module.exports = function (argString) {
   if (Array.isArray(argString)) return argString
 
+  argString = argString.trim()
+
   var i = 0
+  var prevC = null
   var c = null
   var opening = null
   var args = []
 
   for (var ii = 0; ii < argString.length; ii++) {
+    prevC = c
     c = argString.charAt(ii)
 
     // split on spaces unless we're in quotes.
     if (c === ' ' && !opening) {
-      i++
+      if (!(prevC === ' ')) {
+        i++
+      }
       continue
     }
 
diff --git a/package.json b/package.json
index 5d84e9f..9717e9e 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "yargs-parser",
-  "version": "7.0.0",
+  "version": "8.0.0",
   "description": "the mighty option parser used by yargs",
   "main": "index.js",
   "scripts": {
@@ -29,7 +29,7 @@
     "chai": "^3.5.0",
     "coveralls": "^2.11.12",
     "mocha": "^3.0.1",
-    "nyc": "^10.0.0",
+    "nyc": "^11.2.1",
     "standard": "^10.0.2",
     "standard-version": "^4.0.0"
   },


Test patch:
diff --git a/test/tokenize-arg-string.js b/test/tokenize-arg-string.js
index 9b9f9a0..2f7cffc 100644
--- a/test/tokenize-arg-string.js
+++ b/test/tokenize-arg-string.js
@@ -3,6 +3,7 @@
 var tokenizeArgString = require('../lib/tokenize-arg-string')
 
 require('chai').should()
+var expect = require('chai').expect
 
 describe('TokenizeArgString', function () {
   it('handles unquoted string', function () {
@@ -37,4 +38,13 @@ describe('TokenizeArgString', function () {
     args[1].should.equal('hello \'world\'')
     args[2].should.equal('--bar=foo "bar"')
   })
+
+  // https://github.com/yargs/yargs-parser/pull/100
+  // https://github.com/yargs/yargs-parser/pull/106
+  it('ignores unneeded spaces', function () {
+    var args = tokenizeArgString('  foo  bar  "foo  bar"  ')
+    args[0].should.equal('foo')
+    expect(args[1]).equal('bar')
+    expect(args[2]).equal('foo  bar')
+  })
 })
diff --git a/test/yargs-parser.js b/test/yargs-parser.js
index fa296b6..cf5f0ba 100644
--- a/test/yargs-parser.js
+++ b/test/yargs-parser.js
@@ -450,6 +450,18 @@ describe('yargs-parser', function () {
       argv.should.have.property('foo').and.deep.equal('baz')
     })
 
+    it('should combine values from config file and argv, if argv value is an array', function () {
+      var argv = parser(['--foo', 'bar'], {
+        config: ['settings'],
+        array: ['foo'],
+        default: {
+          settings: jsonPath
+        }
+      })
+
+      argv.should.have.property('foo').and.deep.equal(['bar', 'baz'])
+    })
+
     it('should use value from config file, if argv key is a boolean', function () {
       var argv = parser([], {
         config: ['settings'],
@@ -642,13 +654,15 @@ describe('yargs-parser', function () {
         configObjects: [{
           apple: 'apple',
           banana: 42,
-          foo: 'baz'
+          foo: 'baz',
+          gotcha: null
         }]
       })
 
       argv.should.have.property('apple', 'apple')
       argv.should.have.property('banana', 42)
       argv.should.have.property('foo', 'bar')
+      argv.should.have.property('gotcha', null)
     })
 
     it('should use value from config object, if argv value is using default value', function () {
@@ -1218,6 +1232,18 @@ describe('yargs-parser', function () {
         result.should.have.property('some-option').that.is.a('string').and.equals('val')
         result.should.have.property('someOption').that.is.a('string').and.equals('val')
       })
+
+      // https://github.com/yargs/yargs-parser/issues/95
+      it('should not duplicate option values when equivalent dashed aliases are provided', function () {
+        var result = parser(['--someOption', 'val'], {
+          alias: {
+            someOption: 'some-option'
+          }
+        })
+
+        result.should.have.property('some-option').that.is.a('string').and.equals('val')
+        result.should.have.property('someOption').that.is.a('string').and.equals('val')
+      })
     }
 
     describe('dashes and camelCase', function () {
@@ -1495,6 +1521,11 @@ describe('yargs-parser', function () {
       result['1'][0].should.equal('a')
       result['1'][1].should.equal('b')
     })
+
+    it('should create an array when passing an argument twice with same value', function () {
+      var result = parser(['-x', 'val1', '-x', 'val1'])
+      result.should.have.property('x').that.is.an('array').and.to.deep.equal(['val1', 'val1'])
+    })
   })
 
   describe('nargs', function () {
@@ -1955,6 +1986,17 @@ describe('yargs-parser', function () {
         })
         expect(parsed._[0]).to.equal('5')
       })
+
+      it('parses number if option explicitly set to number type', function () {
+        var parsed = parser(['--foo', '5', '--bar', '6'], {
+          number: 'bar',
+          configuration: {
+            'parse-numbers': false
+          }
+        })
+        expect(parsed['foo']).to.equal('5')
+        expect(parsed['bar']).to.equal(6)
+      })
     })
 
     describe('boolean negation', function () {
@@ -2429,6 +2471,25 @@ describe('yargs-parser', function () {
       })
       parsed.error.message.should.equal('foo is array: true')
     })
+
+    // see: https://github.com/yargs/yargs-parser/issues/76
+    it('only runs coercion functions once, even with aliases', function () {
+      var runcount = 0
+      var func = (arg) => {
+        runcount++
+        return undefined
+      }
+      parser([ '--foo', 'bar' ], {
+        alias: {
+          foo: ['f', 'foo-bar', 'bar'],
+          b: ['bar']
+        },
+        coerce: {
+          bar: func
+        }
+      })
+      runcount.should.equal(1)
+    })
   })
 
   // see: https://github.com/yargs/yargs-parser/issues/37
@@ -2440,4 +2501,49 @@ describe('yargs-parser', function () {
     })
     argv.a.should.deep.equal(['a.txt', 'b.txt'])
   })
+
+  // see: https://github.com/yargs/yargs/issues/963
+  it('does not magically convert numeric strings larger than Number.MAX_SAFE_INTEGER', () => {
+    const argv = parser([ '--foo', '93940495950949399948393' ])
+    argv.foo.should.equal('93940495950949399948393')
+  })
+
+  it('does not magically convert scientific notation larger than Number.MAX_SAFE_INTEGER', () => {
+    const argv = parser([ '--foo', '33e99999' ])
+    argv.foo.should.equal('33e99999')
+  })
+
+  it('converts numeric options larger than Number.MAX_SAFE_INTEGER to number', () => {
+    const argv = parser([ '--foo', '93940495950949399948393' ], {
+      number: ['foo']
+    })
+    argv.foo.should.equal(9.39404959509494e+22)
+  })
+
+  // see: https://github.com/yargs/yargs-parser/issues/101
+  describe('dot-notation array arguments combined with string arguments', function () {
+    it('parses correctly when dot-notation argument is first', function () {
+      var argv = parser([ '--foo.bar', 'baz', '--foo', 'bux' ])
+      Array.isArray(argv.foo).should.equal(true)
+      argv.foo[0].bar.should.equal('baz')
+      argv.foo[1].should.equal('bux')
+    })
+
+    it('parses correctly when dot-notation argument is last', function () {
+      var argv = parser([ '--foo', 'bux', '--foo.bar', 'baz' ])
+      Array.isArray(argv.foo).should.equal(true)
+      argv.foo[0].should.equal('bux')
+      argv.foo[1].bar.should.equal('baz')
+    })
+
+    it('parses correctly when there are multiple dot-notation arguments', function () {
+      var argv = parser([ '--foo.first', 'firstvalue', '--foo', 'bux', '--foo.bar', 'baz', '--foo.bla', 'banana' ])
+      Array.isArray(argv.foo).should.equal(true)
+      argv.foo.length.should.equal(4)
+      argv.foo[0].first.should.equal('firstvalue')
+      argv.foo[1].should.equal('bux')
+      argv.foo[2].bar.should.equal('baz')
+      argv.foo[3].bla.should.equal('banana')
+    })
+  })
 })