Repository: yargs/yargs
Pull request #825 (2017-03-13 05:30:02)

Number commands don't work
casting argv._ elements back to string when evaluating for commands to
allow non-string commands to be correctly found and matched

Resolves https://github.com/yargs/yargs/issues/824

sputnik13: why is an increase in coverage considered a fail...  that's puzzling to me
bcoe: @sputnik13 you're good; coveralls.io switched to looking at branch coverage rather than line coverage which has broken a few things for us.

@nickmerwin I think you'll find quite a few projects bump into this confusion -- do you have a blog post I could reference and tweet, etc.
nickmerwin: @bcoe we've added a toggle to the repo settings if you'd prefer to disable this:

![image](https://cloud.githubusercontent.com/assets/1129/23885939/2bacb17e-0834-11e7-984b-9f6c3e7e0554.png)

Blog post is coming shortly.
nickmerwin: @bcoe we've decided to make this opt-in for existing repos so as to minimize confusion; branches will still be analyzed, but not factored into aggregate % unless the setting is checked.
bcoe: -


On Tuesday, March 14, 2017, Nick Merwin <notifications@github.com> wrote:

> @bcoe <https://github.com/bcoe> we've decided to make this opt-in for
> existing repos so as to minimize confusion; branches will still be
> analyzed, but not factored into aggregate % unless the setting is checked.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/yargs/yargs/pull/825#issuecomment-286471043>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAL4MVDjusyunvb1XJs30jWLbZOHFe5kks5rlrtxgaJpZM4Ma3N9>
> .
>

sputnik13: @bcoe any chance we can get reviews on this?  I'm not familiar with your team's processes so I'm just wondering whether nobody's looking at it because of the test failure.
bcoe: @nickmerwin probably a good call 👍 appreciate the hard work.

@sputnik13 will review soon, appreciate the pull request; with regards to process, there are a few folks who contribute with code reviews, but unfortunately most of us are doing this evenings and weekends -- so the turnaround can take a few days.

With regards to this issue, there's a related ticket that I'd like to fix:

https://github.com/yargs/yargs/issues/823

So I was planning on looking a bit closer at the command parsing logic, to see if we could solve a few categories of bugs at once 😄 

sputnik13: @bcoe understood, and I appreciate the quick responses, I understand it can be a challenge to work on projects on your own time.  Please let me know if there's anything I can do to help.

Patch:
diff --git a/yargs.js b/yargs.js
index a340911..5648588 100644
--- a/yargs.js
+++ b/yargs.js
@@ -567,7 +567,7 @@ function Yargs (processArgs, cwd, parentRequire) {
         self.demand(key, demand)
       }
 
-      if ('demandOption' in opt) {
+      if (opt.demandOption) {
         self.demandOption(key, typeof opt.demandOption === 'string' ? opt.demandOption : undefined)
       }
 
@@ -983,7 +983,8 @@ function Yargs (processArgs, cwd, parentRequire) {
         var handlerKeys = command.getCommands()
         if (handlerKeys.length) {
           var firstUnknownCommand
-          for (var i = 0, cmd; (cmd = argv._[i]) !== undefined; i++) {
+          for (var i = 0, cmd; argv._[i] !== undefined; i++) {
+            cmd = String(argv._[i])
             if (~handlerKeys.indexOf(cmd) && cmd !== completionCommand) {
               setPlaceholderKeys(argv)
               return command.runCommand(cmd, self, parsed)


Test patch:
diff --git a/test/command.js b/test/command.js
index 0af0929..4d50af7 100644
--- a/test/command.js
+++ b/test/command.js
@@ -1317,4 +1317,14 @@ describe('Command', function () {
     called.should.equal(false)
     r.errors.should.match(/Missing required argument/)
   })
+
+  it('should support numeric commands', function () {
+    var output = []
+    yargs('1')
+      .command('1', 'numeric command', function (yargs) {
+        output.push('1')
+      })
+      .argv
+    output.should.include('1')
+  })
 })
diff --git a/test/yargs.js b/test/yargs.js
index 45719d2..078bee9 100644
--- a/test/yargs.js
+++ b/test/yargs.js
@@ -1823,8 +1823,14 @@ describe('yargs dsl tests', function () {
     it('allows an error to be handled by fail() handler', function () {
       var msg
       var err
+      var jsonErrMessage
       yargs('--json invalid')
         .coerce('json', function (arg) {
+          try {
+            JSON.parse(arg)
+          } catch (err) {
+            jsonErrMessage = err.message
+          }
           return JSON.parse(arg)
         })
         .fail(function (m, e) {
@@ -1832,7 +1838,7 @@ describe('yargs dsl tests', function () {
           err = e
         })
         .argv
-      expect(msg).to.match(/Unexpected token i/)
+      expect(msg).to.equal(jsonErrMessage)
       expect(err).to.exist
     })