feat: apply default builder to command() and apply fail() handlers gl…

…obally (#583) BREAKING CHANGE: fail is now applied globally. BREAKING CHANGE: we now default to an empty builder function when command is executed with no builder.
yargs · Aug 12, 2016 · 0aaa68b · 0aaa68b
1 parent 927810c
commit 0aaa68b
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 13 deletions.
diff --git a/lib/command.js b/lib/command.js
@@ -34,9 +34,7 @@ module.exports = function (yargs, usage, validation) {
     handlers[parsedCommand.cmd] = {
       original: cmd,
       handler: handler,
-      // TODO: default to a noop builder in
-      // yargs@5.x
-      builder: builder,
+      builder: builder || {},
       demanded: parsedCommand.demanded,
       optional: parsedCommand.optional
     }
@@ -129,7 +127,7 @@ module.exports = function (yargs, usage, validation) {
     var currentContext = yargs.getContext()
     var parentCommands = currentContext.commands.slice()
     currentContext.commands.push(command)
-    if (commandHandler.builder && typeof commandHandler.builder === 'function') {
+    if (typeof commandHandler.builder === 'function') {
       // a function can be provided, which builds
       // up a yargs chain and possibly returns it.
       innerArgv = commandHandler.builder(yargs.reset(parsed.aliases))
@@ -145,7 +143,7 @@ module.exports = function (yargs, usage, validation) {
       } else {
         innerArgv = yargs.parsed.argv
       }
-    } else if (commandHandler.builder && typeof commandHandler.builder === 'object') {
+    } else if (typeof commandHandler.builder === 'object') {
       // as a short hand, an object can instead be provided, specifying
       // the options that a command takes.
       innerArgv = yargs.reset(parsed.aliases)

diff --git a/lib/usage.js b/lib/usage.js
@@ -34,9 +34,9 @@ module.exports = function (yargs, y18n) {
   var failureOutput = false
   self.fail = function (msg, err) {
     if (fails.length) {
-      fails.forEach(function (f) {
-        f(msg, err)
-      })
+      for (var i = fails.length - 1; i >= 0; --i) {
+        fails[i](msg, err)
+      }
     } else {
       if (yargs.getExitProcess()) setBlocking(true)
 
@@ -391,7 +391,7 @@ module.exports = function (yargs, y18n) {
 
   self.reset = function (globalLookup) {
     // do not reset wrap here
-    fails = []
+    // do not reset fails here
     failMessage = null
     failureOutput = false
     usage = undefined

diff --git a/test/completion.js b/test/completion.js
@@ -34,7 +34,7 @@ describe('Completion', function () {
           .argv
       })
 
-      r.logs.should.include('foo')
+      // should not suggest foo for completion unless foo is subcommand of apple
       r.logs.should.not.include('apple')
     })
 

diff --git a/test/usage.js b/test/usage.js
@@ -501,7 +501,7 @@ describe('usage tests', function () {
             try {
               return yargs('test')
                 .fail(function () {
-                  console.log('is not triggered')
+                  console.log('is triggered last')
                 })
                 .exitProcess(false)
                 .wrap(null)
@@ -511,15 +511,15 @@ describe('usage tests', function () {
                       console.log([error.name, error.message])
                     })
                     .exitProcess(false)
-
+                }, function (argv) {
                   throw new Error('foo')
                 })
                 .argv
             } catch (error) {
 
             }
           })
-          r.logs.should.deep.equal([['Error', 'foo']])
+          r.logs.should.deep.equal([['Error', 'foo'], 'is triggered last'])
           r.should.have.property('exit').and.be.false
         })
       })