Repository: yargs/yargs-parser
Pull request #23 (2016-04-08 11:30:21)

feat(configuration): Allow to pass configuration objects to yargs-parser
This was brought to by https://github.com/yargs/yargs/issues/435, about aliases not being set correctly to values set by `pkgConf`. (It also solves https://github.com/yargs/yargs/issues/423).

For aliases to be handled correctly, even considering transitive relationships, `pkgConf` values should be handled in yargs-parser, where most of the logic of aliases is. So, the solution here is to pass the object from the key in `package.json` to yargs-parser.

This is the first part of a solution that would later need yargs to pass the object to yargs-parser.

Here are some design considerations I made:
- I thought the best is to add a generic key to `opts` in yargs-parser, called `configObjects` that takes arbitrary objects and adds them to `argv`. So as to avoid coupling it with `pkgConf`. That way it makes sense to people who use yargs-parser by itself.
- Also, I thought it's fine for the `configObjects` values to have almost the same priority that values set by a config file, after all, they are another way of configuration. An issue with that is that values from `pkgConf` were previously added as 'defaults', so they could have been overridden by env values before, but not anymore. Could this be an breaking change?

Let me know what you think. This is a WIP, if you are ok with the changes I'll add more tests and documentation.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/5713392/badge)](https://coveralls.io/builds/5713392)

Coverage remained the same at 100.0% when pulling **3414df8b2bc09a01f562bb0494caeef6a1afa5f8 on elas7:config-objects** into **48b1e6a338f80417b3fd0fc3e39ae181332fdcb5 on yargs:master**.


Patch:
diff --git a/.npmignore b/.npmignore
deleted file mode 100644
index a399f75..0000000
--- a/.npmignore
+++ /dev/null
@@ -1,5 +0,0 @@
-.nyc_output
-node_modules
-.DS_Store
-yargs-logo.png
-./test/fixtures/package.json
diff --git a/index.js b/index.js
index 834f8da..2838f68 100644
--- a/index.js
+++ b/index.js
@@ -19,6 +19,7 @@ function parse (args, opts) {
     'boolean-negation': true
   }, opts.configuration)
   var defaults = opts.default || {}
+  var configObjects = opts.configObjects || []
   var envPrefix = opts.envPrefix
   var newAliases = {}
   // allow a i18n handler to be passed in, default to a fake one (util.format).
@@ -240,12 +241,12 @@ function parse (args, opts) {
         } else {
           next = args[i + 1]
 
-          if (next && !/^(-|--)[^-]/.test(next) &&
+          if (next !== undefined && !/^(-|--)[^-]/.test(next) &&
             !checkAllAliases(key, flags.bools) &&
             !checkAllAliases(key, flags.counts)) {
             setArg(key, next)
             i++
-          } else if (next && /true|false/.test(next)) {
+          } else if (/^(true|false)$/.test(next)) {
             setArg(key, next)
             i++
           } else {
@@ -263,10 +264,12 @@ function parse (args, opts) {
   // order of precedence:
   // 1. command line arg
   // 2. value from config file
-  // 3. value from env var
-  // 4. configured default value
+  // 3. value from config objects
+  // 4. value from env var
+  // 5. configured default value
   applyEnvVars(argv, true) // special case: check env vars that point to config file
   setConfig(argv)
+  setConfigObjects()
   applyEnvVars(argv, false)
   applyDefaultsAndAliases(argv, flags.aliases, defaults)
 
@@ -335,46 +338,48 @@ function parse (args, opts) {
       value = increment
     }
 
+    // Set normalized value when key is in 'normalize' and in 'arrays'
+    if (checkAllAliases(key, flags.normalize) && checkAllAliases(key, flags.arrays)) {
+      value = path.normalize(val)
+    }
+
     var splitKey = key.split('.')
     setKey(argv, splitKey, value)
 
-    // alias references an inner-value within
-    // a dot-notation object. see #279.
-    if (~key.indexOf('.') && flags.aliases[key]) {
+    // handle populating aliases of the full key
+    if (flags.aliases[key]) {
       flags.aliases[key].forEach(function (x) {
         x = x.split('.')
         setKey(argv, x, value)
       })
     }
 
-    ;(flags.aliases[splitKey[0]] || []).forEach(function (x) {
-      x = x.split('.')
+    // handle populating aliases of the first element of the dot-notation key
+    if (splitKey.length > 1 && configuration['dot-notation']) {
+      ;(flags.aliases[splitKey[0]] || []).forEach(function (x) {
+        x = x.split('.')
 
-      // handle populating dot notation for both
-      // the key and its aliases.
-      if (splitKey.length > 1) {
+        // expand alias with nested objects in key
         var a = [].concat(splitKey)
         a.shift() // nuke the old key.
         x = x.concat(a)
-      }
 
-      setKey(argv, x, value)
-    })
+        setKey(argv, x, value)
+      })
+    }
 
-    var keys = [key].concat(flags.aliases[key] || [])
-    for (var i = 0, l = keys.length; i < l; i++) {
-      if (flags.normalize[keys[i]]) {
-        keys.forEach(function (key) {
-          argv.__defineSetter__(key, function (v) {
-            val = path.normalize(v)
-          })
-
-          argv.__defineGetter__(key, function () {
-            return typeof val === 'string' ? path.normalize(val) : val
-          })
+    // Set normalize getter and setter when key is in 'normalize' but isn't an array
+    if (checkAllAliases(key, flags.normalize) && !checkAllAliases(key, flags.arrays)) {
+      var keys = [key].concat(flags.aliases[key] || [])
+      keys.forEach(function (key) {
+        argv.__defineSetter__(key, function (v) {
+          val = path.normalize(v)
         })
-        break
-      }
+
+        argv.__defineGetter__(key, function () {
+          return typeof val === 'string' ? path.normalize(val) : val
+        })
+      })
     }
   }
 
@@ -436,6 +441,14 @@ function parse (args, opts) {
     })
   }
 
+  // set all config objects passed in opts
+  function setConfigObjects () {
+    if (typeof configObjects === 'undefined') return
+    configObjects.forEach(function (configObject) {
+      setConfigObject(configObject)
+    })
+  }
+
   function applyEnvVars (argv, configOnly) {
     if (typeof envPrefix === 'undefined') return
 
@@ -494,7 +507,7 @@ function parse (args, opts) {
       o[key] = increment(o[key])
     } else if (o[key] === undefined && checkAllAliases(key, flags.arrays)) {
       o[key] = Array.isArray(value) ? value : [value]
-    } else if (o[key] === undefined || typeof o[key] === 'boolean') {
+    } else if (o[key] === undefined || checkAllAliases(key, flags.bools)) {
       o[key] = value
     } else if (Array.isArray(o[key])) {
       o[key].push(value)
diff --git a/package.json b/package.json
index c2429e4..228265f 100644
--- a/package.json
+++ b/package.json
@@ -34,5 +34,9 @@
   "dependencies": {
     "camelcase": "^2.1.1",
     "lodash.assign": "^4.0.6"
-  }
+  },
+  "files": [
+    "lib",
+    "index.js"
+  ]
 }


Test patch:
diff --git a/test/yargs-parser.js b/test/yargs-parser.js
index ca1476c..eb14c59 100644
--- a/test/yargs-parser.js
+++ b/test/yargs-parser.js
@@ -40,6 +40,12 @@ describe('yargs-parser', function () {
     parse.should.have.property('_').with.length(0)
   })
 
+  it('should set the value of a single long option to the next supplied value, even if the value is empty', function () {
+    var parse = parser(['--pow', ''])
+    parse.should.have.property('pow', '')
+    parse.should.have.property('_').with.length(0)
+  })
+
   it('should set the value of a single long option if an = was used', function () {
     var parse = parser(['--pow=xixxle'])
     parse.should.have.property('pow', 'xixxle')
@@ -121,6 +127,12 @@ describe('yargs-parser', function () {
     argv._[0].should.be.a('number')
   })
 
+  it('should set the value of a single short option to the next supplied value, even if the value is empty', function () {
+    var parse = parser(['-p', ''])
+    parse.should.have.property('p', '')
+    parse.should.have.property('_').with.length(0)
+  })
+
   it('should not set the next value as the value of a short option if that option is explicitly defined as a boolean', function () {
     var parse = parser([ '-t', 'moo' ], {
       boolean: 't'
@@ -141,6 +153,15 @@ describe('yargs-parser', function () {
     parse.should.have.property('_').and.deep.equal(['moo'])
   })
 
+  it('should not set boolean options values if the next value only contains the words "true" or "false"', function () {
+    var parse = parser(['--verbose', 'aaatrueaaa', 'moo', '-t', 'aaafalseaaa'], {
+      boolean: ['t', 'verbose']
+    })
+    parse.should.have.property('verbose', true).and.be.a('boolean')
+    parse.should.have.property('t', true).and.be.a('boolean')
+    parse.should.have.property('_').and.deep.equal(['aaatrueaaa', 'moo', 'aaafalseaaa'])
+  })
+
   it('should allow defining options as boolean in groups', function () {
     var parse = parser([ '-x', '-z', 'one', 'two', 'three' ], {
       boolean: ['x', 'y', 'z']
@@ -236,6 +257,19 @@ describe('yargs-parser', function () {
       a.s = ['', 'path', 'to', 'new', 'dir', '..', '..', ''].join(path.sep)
       a.s.should.equal(['', 'path', 'to', ''].join(path.sep))
     })
+
+    it('should normalize when key is also an array', function () {
+      var a = parser([ '-s', ['', 'tmp', '..', ''].join(path.sep), ['', 'path', 'to', 'new', 'dir', '..', '..', ''].join(path.sep) ], {
+        alias: {
+          s: ['save']
+        },
+        normalize: 's',
+        array: 's'
+      })
+      var expected = [path.sep, ['', 'path', 'to', ''].join(path.sep)]
+      a.should.have.property('s').and.deep.equal(expected)
+      a.should.have.property('save').and.deep.equal(expected)
+    })
   })
 
   describe('alias', function () {
@@ -571,6 +605,102 @@ describe('yargs-parser', function () {
     })
   })
 
+  describe('config objects', function () {
+    it('should load options from config object', function () {
+      var argv = parser([ '--foo', 'bar' ], {
+        configObjects: [{
+          apple: 'apple',
+          banana: 42,
+          foo: 'baz'
+        }]
+      })
+
+      argv.should.have.property('apple', 'apple')
+      argv.should.have.property('banana', 42)
+      argv.should.have.property('foo', 'bar')
+    })
+
+    it('should use value from config object, if argv value is using default value', function () {
+      var argv = parser([], {
+        configObjects: [{
+          apple: 'apple',
+          banana: 42,
+          foo: 'baz'
+        }],
+        default: {
+          foo: 'banana'
+        }
+      })
+
+      argv.should.have.property('apple', 'apple')
+      argv.should.have.property('banana', 42)
+      argv.should.have.property('foo', 'baz')
+    })
+
+    it('should use value from config object to all aliases', function () {
+      var argv = parser([], {
+        configObjects: [{
+          apple: 'apple',
+          banana: 42,
+          foo: 'baz'
+        }],
+        alias: {
+          a: ['apple'],
+          banana: ['b']
+        }
+      })
+
+      argv.should.have.property('apple', 'apple')
+      argv.should.have.property('a', 'apple')
+      argv.should.have.property('banana', 42)
+      argv.should.have.property('b', 42)
+      argv.should.have.property('foo', 'baz')
+    })
+
+    it('should load nested options from config object', function () {
+      var argv = parser(['--nested.foo', 'bar'], {
+        configObjects: [{
+          a: 'a',
+          nested: {
+            foo: 'baz',
+            bar: 'bar'
+          },
+          b: 'b'
+        }]
+      })
+
+      argv.should.have.property('a', 'a')
+      argv.should.have.property('b', 'b')
+      argv.should.have.property('nested').and.deep.equal({
+        foo: 'bar',
+        bar: 'bar'
+      })
+    })
+
+    it('should use nested value from config object, if argv value is using default value', function () {
+      var argv = parser([], {
+        configObjects: [{
+          a: 'a',
+          nested: {
+            foo: 'baz',
+            bar: 'bar'
+          },
+          b: 'b'
+        }],
+        default: {
+          'nested.foo': 'banana'
+        }
+      })
+
+      argv.should.have.property('a', 'a')
+      argv.should.have.property('b', 'b')
+      argv.should.have.property('nested').and.deep.equal({
+        foo: 'baz',
+        bar: 'bar'
+      })
+    })
+  })
+
   describe('dot notation', function () {
     it('should allow object graph traversal via dot notation', function () {
       var argv = parser([
@@ -866,7 +996,7 @@ describe('yargs-parser', function () {
     })
 
     describe('for boolean options', function () {
-      [true, false, undefined].forEach(function (def) {
+      [true, false, undefined, null].forEach(function (def) {
         describe('with explicit ' + def + ' default', function () {
           var opts = {
             default: {
@@ -1615,6 +1745,19 @@ describe('yargs-parser', function () {
 
         expect(parsed['alias.bar']).to.equal('abc')
       })
+
+      it('does not expand alias of first element of dot notation arguments', function () {
+        var parsed = parser(['--foo.bar', 'banana'], {
+          alias: {
+            'foo': ['f']
+          },
+          configuration: {
+            'dot-notation': false
+          }
+        })
+        expect(parsed['foo.bar']).to.equal('banana')
+        expect(parsed).not.to.include.keys('f.bar')
+      })
     })
 
     describe('parse numbers', function () {