Repository: yargs/yargs-parser
Pull request #202 (2019-09-25 17:08:53)

feat!: rework `collect-unknown-options` into `unknown-options-as-args`, providing more comprehensive functionality
@bcoe: since I don't seem to be able to re-open #181, I'm creating a new pull request.  It turns out that the issue I was having with the command parsing after the previous pull request was that it uses an array arg, rather than the `_` array.  I've updated my feature from `collect-unknown-options` to `unknown-options-as-args` so that it can also cover this case.

I've tested this in the script I plan to use it in, and it works as I would expect now.

bcoe: @henderea thank you for digging into this, I've been dragging my feet for too long on the next release of yargs, because I was hoping to loop back around to try to dig into this issue.

I've been the victim of my of my libraries' own success these days, and have been spread a bit thin on code review I'm afraid 😦... thanks for your hard work.

@mleguen, could I perhaps loop you in to help review this.
mleguen: @bcoe my pleasure, but I am afraid I will not be available for a review before wednesday (my weekly OSS day at work).
mleguen: Wow! Just finished reading the extensive discussion in yargs/yargs#1243, #181 and #202 :-)...
Time to start reviewing!
henderea: @mleguen On the topic of the forEach stuff, I was largely following the pattern of the existing code.  The comment I put about Node 6 compatibility on the `Object.values` stuff was also copied from the existing code.

I'll make the requested refactoring to reduce the `configuration['unknown-options-as-args'] && isUnknownOption(arg)` into `isUnknownOptionAsArg(arg)` soon.
mleguen: @henderea OK, I can see now the source of this complicated use of Array.foreach in place of Array.some is checkAllAliases.

I will submit a PR to improve this.
henderea: @mleguen: I've committed the suggested refactoring of `isUnknownOptionAsArg(arg)`, and also have converted my 2 usages of `forEach` with a boolean to `some`.  I left `checkAllAliases` alone, though, but I can include that in this PR if that would make it easier for everyone.
henderea: According to https://node.green/, it seems that support for `array.some` goes back just as far as `array.forEach`, so I think it should be fine on v6.

Patch:
diff --git a/README.md b/README.md
index 1110bbc..5f1ccb9 100644
--- a/README.md
+++ b/README.md
@@ -386,26 +386,26 @@ node example.js --test-field 1
 { _: [], testField: 1 }
 ```
 
-### collect unknown options
+### unknown options as args
 
 * default: `false`
-* key: `collect-unknown-options`
+* key: `unknown-options-as-args`
 
-Should unknown options be collected into `_`?  An unknown option is one that is not
+Should unknown options be treated like regular arguments?  An unknown option is one that is not
 configured in `opts`.
 
 _If disabled_
 
 ```sh
-node example.js --unknown-option --known-option 2
-{ _: [], unknownOption: true, knownOption: 2 }
+node example.js --unknown-option --known-option 2 --string-option --unknown-option2
+{ _: [], unknownOption: true, knownOption: 2, stringOption: '', unknownOption2: true }
 ```
 
 _If enabled_
 
 ```sh
-node example.js --unknown-option --known-option 2
-{ _: ['--unknown-option'], knownOption: 2 }
+node example.js --unknown-option --known-option 2 --string-option --unknown-option2
+{ _: ['--unknown-option'], knownOption: 2, stringOption: '--unknown-option2' }
 ```
 
 ## Special Thanks
diff --git a/index.js b/index.js
index 08f9852..a5f7ae4 100644
--- a/index.js
+++ b/index.js
@@ -27,7 +27,7 @@ function parse (args, opts) {
     'halt-at-non-option': false,
     'strip-aliased': false,
     'strip-dashed': false,
-    'collect-unknown-options': false
+    'unknown-options-as-args': false
   }, opts.configuration)
   var defaults = opts.default || {}
   var configObjects = opts.configObjects || []
@@ -143,7 +143,7 @@ function parse (args, opts) {
     var next
     var value
 
-    if (configuration['collect-unknown-options'] && isUnknownOption(arg)) {
+    if (isUnknownOptionAsArg(arg)) {
       argv._.push(arg)
     // -- separated by =
     } else if (arg.match(/^--.+=/) || (
@@ -361,7 +361,7 @@ function parse (args, opts) {
     // and terminates when one is observed.
     var available = 0
     for (ii = i + 1; ii < args.length; ii++) {
-      if (!args[ii].match(/^-[^0-9]/)) available++
+      if (!args[ii].match(/^-[^0-9]/) || isUnknownOptionAsArg(args[ii])) available++
       else break
     }
 
@@ -384,7 +384,7 @@ function parse (args, opts) {
 
     if (checkAllAliases(key, flags.bools) && !(/^(true|false)$/.test(next))) {
       argsToSet.push(true)
-    } else if (isUndefined(next) || (/^-/.test(next) && !negative.test(next))) {
+    } else if (isUndefined(next) || (/^-/.test(next) && !negative.test(next) && !isUnknownOptionAsArg(next))) {
       // for keys without value ==> argsToSet remains an empty []
       // set user default value, if available
       if (defaults.hasOwnProperty(key)) {
@@ -393,7 +393,7 @@ function parse (args, opts) {
     } else {
       for (var ii = i + 1; ii < args.length; ii++) {
         next = args[ii]
-        if (/^-/.test(next) && !negative.test(next)) break
+        if (/^-/.test(next) && !negative.test(next) && !isUnknownOptionAsArg(next)) break
         i = ii
         argsToSet.push(processValue(key, next))
       }
@@ -761,27 +761,20 @@ function parse (args, opts) {
   }
 
   function hasAnyFlag (key) {
-    var isSet = false
     // XXX Switch to [].concat(...Object.values(flags)) once node.js 6 is dropped
     var toCheck = [].concat(...Object.keys(flags).map(k => flags[k]))
 
-    toCheck.forEach(function (flag) {
-      if (flag[key]) isSet = flag[key]
+    return toCheck.some(function (flag) {
+      return flag[key]
     })
-
-    return isSet
   }
 
   function hasFlagsMatching (arg, ...patterns) {
-    var hasFlag = false
     var toCheck = [].concat(...patterns)
-    toCheck.forEach(function (pattern) {
+    return toCheck.some(function (pattern) {
       var match = arg.match(pattern)
-      if (match && hasAnyFlag(match[1])) {
-        hasFlag = true
-      }
+      return match && hasAnyFlag(match[1])
     })
-    return hasFlag
   }
 
   // based on a simplified version of the short flag group parsing logic
@@ -809,6 +802,10 @@ function parse (args, opts) {
     return hasAllFlags
   }
 
+  function isUnknownOptionAsArg (arg) {
+    return configuration['unknown-options-as-args'] && isUnknownOption(arg)
+  }
+
   function isUnknownOption (arg) {
     // ignore negative numbers
     if (arg.match(negative)) { return false }


Test patch:
diff --git a/test/yargs-parser.js b/test/yargs-parser.js
index 27f8074..349549b 100644
--- a/test/yargs-parser.js
+++ b/test/yargs-parser.js
@@ -2690,12 +2690,12 @@ describe('yargs-parser', function () {
       })
     })
 
-    describe('collect-unknown-options = true', function () {
+    describe('unknown-options-as-args = true', function () {
       it('should ignore unknown options in long format separated by =', function () {
         const argv = parser('--known-arg=1 --unknown-arg=2', {
           number: ['known-arg'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2708,7 +2708,7 @@ describe('yargs-parser', function () {
         const argv = parser('--no-known-arg --no-unknown-arg', {
           boolean: ['known-arg'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2721,7 +2721,7 @@ describe('yargs-parser', function () {
         const argv = parser('--known-arg a --unknown-arg b', {
           string: ['known-arg'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2734,7 +2734,7 @@ describe('yargs-parser', function () {
         const argv = parser('-k.arg=a -u.arg=b', {
           string: ['k.arg'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2748,7 +2748,7 @@ describe('yargs-parser', function () {
         const argv = parser('-k.arg 1 -u.arg 2', {
           number: ['k.arg'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2762,7 +2762,7 @@ describe('yargs-parser', function () {
         const argv = parser('-k=a -u=b', {
           string: ['k'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2774,7 +2774,7 @@ describe('yargs-parser', function () {
         const argv = parser('-k- -u-', {
           string: ['k'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2786,7 +2786,7 @@ describe('yargs-parser', function () {
         const argv = parser('-k 1 -u 2', {
           number: ['k'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2794,11 +2794,51 @@ describe('yargs-parser', function () {
           'k': 1
         })
       })
+      it('should allow an unknown arg to be used as the value of another flag in short form', function () {
+        const argv = parser('-k -u', {
+          string: ['k'],
+          narg: { 'k': 1 },
+          configuration: {
+            'unknown-options-as-args': true
+          }
+        })
+        argv.should.deep.equal({
+          _: [],
+          'k': '-u'
+        })
+      })
+      it('should allow an unknown arg to be used as the value of another flag in long form', function () {
+        const argv = parser('--known-arg --unknown-arg', {
+          string: ['known-arg'],
+          narg: { 'known-arg': 1 },
+          configuration: {
+            'unknown-options-as-args': true
+          }
+        })
+        argv.should.deep.equal({
+          _: [],
+          'knownArg': '--unknown-arg',
+          'known-arg': '--unknown-arg'
+        })
+      })
+      it('should allow an unknown arg to be used as the value of another flag in array form', function () {
+        const argv = parser('--known-arg --unknown-arg1 --unknown-arg2', {
+          array: ['known-arg'],
+          configuration: {
+            'unknown-options-as-args': true
+          }
+        })
+        argv.should.deep.equal({
+          _: [],
+          'knownArg': ['--unknown-arg1', '--unknown-arg2'],
+          'known-arg': ['--unknown-arg1', '--unknown-arg2']
+        })
+      })
       it('should ignore unknown options in short format followed by a number', function () {
         const argv = parser('-k1 -u2', {
           number: ['k'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2810,7 +2850,7 @@ describe('yargs-parser', function () {
         const argv = parser('-k/1/ -u/2/', {
           string: ['k'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2822,7 +2862,7 @@ describe('yargs-parser', function () {
         const argv = parser('-kuv', {
           boolean: ['k', 'v'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2833,7 +2873,7 @@ describe('yargs-parser', function () {
         const argv = parser('-kv', {
           boolean: ['k', 'v'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({
@@ -2846,7 +2886,7 @@ describe('yargs-parser', function () {
         const argv = parser('-k -33', {
           boolean: ['k'],
           configuration: {
-            'collect-unknown-options': true
+            'unknown-options-as-args': true
           }
         })
         argv.should.deep.equal({