Repository: yargs/yargs-parser
Pull request #39 (2016-07-15 21:10:51)

fix(count): do not increment a default value
Based on https://github.com/yargs/yargs/issues/500

This was a bit tricky to figure out because default values were already being applied in line 274 but were then being overwritten immediately thereafter in a weird attempt to use the `increment` function to set a default value of 0 when no explicit default value is given. When an explicit default value _was_ given, the `increment` function would then inadvertently add/concat a `1` to the value. 😕 

Therefore the fix is comprised of these changes:

1. Let `applyDefaultsAndAliases` do its job of setting explicit defaults, see line 274
2. Apply a default of `0` to a count only when (a) no arg is given and (b) no explicit default is set, see new line 278
3. Only use the `increment` function when a count arg is actually given (not when setting a default value), see new line 339
4. When setting a default value for a count, use the value as given (don't use `increment` and don't wrap it in an array), see new line 519
5. Since `increment` is now only used for when an arg is given, use a default of `1` instead of `0`, see new line 674

Through testing, I also found that previous functionality treated all arg values for a count flag equally (they all increment the count, regardless of value). This has been preserved, codified in a new test. We can consider changing this later if we want.

Hopefully this makes the logic a little more straightforward. 😎 

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

Coverage remained the same at 100.0% when pulling **ff1c627af218a91d57266fc5f0f7a8b40143ce00 on fix-count-with-default** into **2418bbcc60ffea5906e36c6536cacd96e925b522 on master**.


Patch:
diff --git a/index.js b/index.js
index 0abbb3b..35c0272 100644
--- a/index.js
+++ b/index.js
@@ -273,8 +273,9 @@ function parse (args, opts) {
   applyEnvVars(argv, false)
   applyDefaultsAndAliases(argv, flags.aliases, defaults)
 
+  // for any counts either not in args or without an explicit default, set to 0
   Object.keys(flags.counts).forEach(function (key) {
-    setArg(key, defaults[key])
+    if (!hasKey(argv, key.split('.'))) setArg(key, 0)
   })
 
   notFlags.forEach(function (key) {
@@ -334,7 +335,8 @@ function parse (args, opts) {
       if (!isUndefined(val) && !isNumber(val) && checkAllAliases(key, flags.numbers)) value = NaN
     }
 
-    if (checkAllAliases(key, flags.counts)) {
+    // increment a count given as arg (either no value or value parsed as boolean)
+    if (checkAllAliases(key, flags.counts) && (isUndefined(value) || typeof value === 'boolean')) {
       value = increment
     }
 
@@ -514,7 +516,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 || checkAllAliases(key, flags.bools)) {
+    } else if (o[key] === undefined || checkAllAliases(key, flags.bools) || checkAllAliases(key, flags.counts)) {
       o[key] = value
     } else if (Array.isArray(o[key])) {
       o[key].push(value)
@@ -665,8 +667,11 @@ function combineAliases (aliases) {
   return combined
 }
 
+// this function should only be called when a count is given as an arg
+// it is NOT called to set a default value
+// thus we can start the count at 1 instead of 0
 function increment (orig) {
-  return orig !== undefined ? orig + 1 : 0
+  return orig !== undefined ? orig + 1 : 1
 }
 
 function Parser (args, opts) {


Test patch:
diff --git a/test/yargs-parser.js b/test/yargs-parser.js
index e290110..a482743 100644
--- a/test/yargs-parser.js
+++ b/test/yargs-parser.js
@@ -1263,6 +1263,60 @@ describe('yargs-parser', function () {
       parsed.verbose.should.equal(2)
       parsed.should.have.property('_').and.deep.equal(['moomoo'])
     })
+
+    it('should use a default value as is when no arg given', function () {
+      var parsed = parser([], {
+        count: 'v',
+        default: { v: 3 }
+      })
+      parsed.v.should.equal(3)
+
+      parsed = parser([], {
+        count: 'v',
+        default: { v: undefined }
+      })
+      expect(parsed.v).to.be.undefined
+
+      parsed = parser([], {
+        count: 'v',
+        default: { v: null }
+      })
+      expect(parsed.v).to.be.null
+
+      parsed = parser([], {
+        count: 'v',
+        default: { v: false }
+      })
+      parsed.v.should.equal(false)
+
+      parsed = parser([], {
+        count: 'v',
+        default: { v: 'hello' }
+      })
+      parsed.v.should.equal('hello')
+    })
+
+    it('should ignore a default value when arg given', function () {
+      var parsed = parser(['-vv', '-v', '-v'], {
+        count: 'v',
+        default: { v: 1 }
+      })
+      parsed.v.should.equal(4)
+    })
+
+    it('should increment regardless of arg value', function () {
+      var parsed = parser([
+        '-v',
+        '-v=true',
+        '-v', 'true',
+        '-v=false',
+        '-v', 'false',
+        '--no-v',
+        '-v=999',
+        '-v=foobar'
+      ], { count: 'v' })
+      parsed.v.should.equal(8)
+    })
   })
 
   describe('array', function () {