Repository: yargs/yargs
Pull request #1418 (2019-08-31 13:48:16)

Removed "default: false" on verbose.
Proposal to remove "default: false" on verbose in the README.

I was mildly confused when I copied the example and called it with `-v`, expecting no value to default the value to true. However it appears as if the default value overrides both the "not passed" value, and the "passed with no value" parameter. This makes me believe "default: false" should never be used, as the default of default is effectively false.

At the very least I believe it shouldn't be part of the example in the README, especially on conjunction with the verbose parameter, which makes a program that uses verbose differently than any program I have ever seen.


Patch:
diff --git a/README.md b/README.md
index f179081..09a12ef 100644
--- a/README.md
+++ b/README.md
@@ -80,7 +80,8 @@ require('yargs') // eslint-disable-line
   })
   .option('verbose', {
     alias: 'v',
-    default: false
+    type: 'count',
+    description: 'Run with verbose logging. Additition 'v's (-vv, -vvv, etc) to run with even more verbose logging.'
   })
   .argv
 ```
diff --git a/index.js b/index.js
index b36531b..2db543e 100644
--- a/index.js
+++ b/index.js
@@ -32,7 +32,6 @@ function singletonify (inst) {
         return inst.$0
       })
       Argv.__defineGetter__('parsed', () => {
-        console.warn('In future major releases of yargs, "parsed" will be a private field. Use the return value of ".parse()" or ".argv" instead')
         return inst.parsed
       })
     }
diff --git a/lib/command.js b/lib/command.js
index 7024310..02bc7ea 100644
--- a/lib/command.js
+++ b/lib/command.js
@@ -174,7 +174,7 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
     let numFiles = currentContext.files.length
     const parentCommands = currentContext.commands.slice()
 
-    // what does yargs look like after the buidler is run?
+    // what does yargs look like after the builder is run?
     let innerArgv = parsed.argv
     let innerYargs = null
     let positionalMap = {}
@@ -230,14 +230,22 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
 
       innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false)
 
-      const handlerResult = isPromise(innerArgv)
-        ? innerArgv.then(argv => commandHandler.handler(argv))
-        : commandHandler.handler(innerArgv)
+      let handlerResult
+      if (isPromise(innerArgv)) {
+        handlerResult = innerArgv.then(argv => commandHandler.handler(argv))
+      } else {
+        handlerResult = commandHandler.handler(innerArgv)
+      }
 
       if (isPromise(handlerResult)) {
-        handlerResult.catch(error =>
-          yargs.getUsageInstance().fail(null, error)
-        )
+        yargs.getUsageInstance().cacheHelpMessage()
+        handlerResult.catch(error => {
+          try {
+            yargs.getUsageInstance().fail(null, error)
+          } catch (err) {
+            // fail's throwing would cause an unhandled rejection.
+          }
+        })
       }
     }
 
diff --git a/lib/usage.js b/lib/usage.js
index fed6d9f..92bf378 100644
--- a/lib/usage.js
+++ b/lib/usage.js
@@ -148,6 +148,7 @@ module.exports = function usage (yargs, y18n) {
 
   const defaultGroup = 'Options:'
   self.help = function help () {
+    if (cachedHelpMessage) return cachedHelpMessage
     normalizeAliases()
 
     // handle old demanded API
@@ -403,6 +404,13 @@ module.exports = function usage (yargs, y18n) {
     })
   }
 
+  // if yargs is executing an async handler, we take a snapshot of the
+  // help message to display on failure:
+  let cachedHelpMessage
+  self.cacheHelpMessage = function () {
+    cachedHelpMessage = this.help()
+  }
+
   // given a set of keys, place any keys that are
   // ungrouped under the 'Options:' grouping.
   function addUngroupedKeys (keys, aliases, groups) {
diff --git a/yargs.js b/yargs.js
index 5b23a0f..794cf02 100644
--- a/yargs.js
+++ b/yargs.js
@@ -544,10 +544,12 @@ function Yargs (processArgs, cwd, parentRequire) {
     argsert('[string|array] [function|boolean|object] [function]', [args, shortCircuit, _parseFn], arguments.length)
     freeze()
     if (typeof args === 'undefined') {
-      const parsed = self._parseArgs(processArgs)
+      const argv = self._parseArgs(processArgs)
+      const tmpParsed = self.parsed
       unfreeze()
       // TODO: remove this compatibility hack when we release yargs@15.x:
-      return (this.parsed = parsed)
+      self.parsed = tmpParsed
+      return argv
     }
 
     // a context object can optionally be provided, this allows


Test patch:
diff --git a/test/command.js b/test/command.js
index 9c394ce..d3a5628 100644
--- a/test/command.js
+++ b/test/command.js
@@ -1426,36 +1426,69 @@ describe('Command', () => {
     })
   })
 
-  // addresses https://github.com/yargs/yargs/issues/510
-  it('fails when the promise returned by the command handler rejects', (done) => {
-    const error = new Error()
-    yargs('foo')
-      .command('foo', 'foo command', noop, (yargs) => Promise.reject(error))
-      .fail((msg, err) => {
-        expect(msg).to.equal(null)
-        expect(err).to.equal(error)
-        done()
+  describe('async', () => {
+    // addresses https://github.com/yargs/yargs/issues/510
+    it('fails when the promise returned by the command handler rejects', (done) => {
+      const error = new Error()
+      yargs('foo')
+        .command('foo', 'foo command', noop, (yargs) => Promise.reject(error))
+        .fail((msg, err) => {
+          expect(msg).to.equal(null)
+          expect(err).to.equal(error)
+          done()
+        })
+        .parse()
+    })
+
+    it('succeeds when the promise returned by the command handler resolves', (done) => {
+      const handler = new Promise((resolve, reject) => {
+        setTimeout(() => {
+          return resolve(true)
+        }, 5)
       })
-      .parse()
-  })
+      const parsed = yargs('foo hello')
+        .command('foo <pos>', 'foo command', () => {}, (yargs) => handler)
+        .fail((msg, err) => {
+          return done(Error('should not have been called'))
+        })
+        .parse()
 
-  it('succeeds when the promise returned by the command handler resolves', (done) => {
-    const handler = new Promise((resolve, reject) => {
-      setTimeout(() => {
-        return resolve(true)
-      }, 5)
-    })
-    const parsed = yargs('foo hello')
-      .command('foo <pos>', 'foo command', () => {}, (yargs) => handler)
-      .fail((msg, err) => {
-        return done(Error('should not have been called'))
+      handler.then(called => {
+        called.should.equal(true)
+        parsed.pos.should.equal('hello')
+        return done()
       })
-      .parse()
+    })
 
-    handler.then(called => {
-      called.should.equal(true)
-      parsed.pos.should.equal('hello')
-      return done()
+    // see: https://github.com/yargs/yargs/issues/1144
+    it('displays error and appropriate help message when handler fails', (done) => {
+      const y = yargs('foo')
+        .command('foo', 'foo command', (yargs) => {
+          yargs.option('bar', {
+            describe: 'bar option'
+          })
+        }, (argv) => {
+          return Promise.reject(Error('foo error'))
+        })
+        .exitProcess(false)
+      // the bug reported in #1144 only happens when
+      // usage.help() is called, this does not occur when
+      // console output is suppressed. tldr; we capture
+      // the log output:
+      let errorLog = ''
+      y._getLoggerInstance().error = (out) => {
+        if (out) errorLog += `${out}\n`
+      }
+      y.parse()
+      // the promise returned by the handler rejects immediately,
+      // so we can wait for a small number of ms:
+      setTimeout(() => {
+        // ensure the appropriate help is displayed:
+        errorLog.should.include('bar option')
+        // ensure error was displayed:
+        errorLog.should.include('foo error')
+        return done()
+      }, 5)
     })
   })