Repository: yargs/yargs
Pull request #1386 (2019-07-30 09:50:06)

feat: formalize existing callback argument to showHelp
This PR adds doc and test for an existing feature: Supplying a print callback to `usage.showHelp` instead of a `console` function name.

We use the callback in our code to explicitly specify the stdout/stderr stream dependencies of yargs - tests can use it instead of mocking the console.

Thanks for making this great library!


Patch:
diff --git a/README.md b/README.md
index f179081..679a3ee 100644
--- a/README.md
+++ b/README.md
@@ -80,7 +80,8 @@ require('yargs') // eslint-disable-line
   })
   .option('verbose', {
     alias: 'v',
-    default: false
+    type: 'boolean',
+    description: 'Run with verbose logging'
   })
   .argv
 ```
diff --git a/docs/advanced.md b/docs/advanced.md
index 9605cc5..8a0bb2a 100644
--- a/docs/advanced.md
+++ b/docs/advanced.md
@@ -439,7 +439,8 @@ yargs.parserConfiguration({
   "camel-case-expansion": true,
   "dot-notation": true,
   "parse-numbers": true,
-  "boolean-negation": true
+  "boolean-negation": true,
+  "deep-merge-config": false
 })
 ```
 
diff --git a/docs/api.md b/docs/api.md
index 5912252..d7eeb9e 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -137,10 +137,10 @@ var argv = require('yargs')
 Provide a synchronous function to coerce or transform the value(s) given on the
 command line for `key`.
 
-The coercion function should accept one argument, representing the parsed value
-from the command line, and should return a new value or throw an error. The
-returned value will be used as the value for `key` (or one of its aliases) in
-`argv`.
+The coercion function should accept one argument, representing the parsed value from
+the command line (an array if multiple values are parsed for the key), and should 
+return a new value or throw an error. The returned value will be used as the value for
+`key` (or one of its aliases) in `argv`.
 
 If the function throws, the error will be treated as a validation
 failure, delegating to either a custom [`.fail()`](#fail) handler or printing
@@ -914,7 +914,7 @@ To submit a new translation for yargs:
 <a name="middleware"></a>.middleware(callbacks, [applyBeforeValidation])
 ------------------------------------
 
-Define global middleware functions to be called first, in list order, for all cli command.  
+Define global middleware functions to be called first, in list order, for all cli command.
 
 The `callbacks` parameter can be a function or a list of functions.  Each callback gets passed a reference to argv.
 
@@ -1291,12 +1291,12 @@ Generate a bash completion script. Users of your application can install this
 script in their `.bashrc`, and yargs will provide completion shortcuts for
 commands and options.
 
-.showHelp(consoleLevel='error')
+.showHelp([consoleLevel | printCallback])
 ---------------------------
 
-Print the usage data using the [`console`](https://nodejs.org/api/console.html) function `consoleLevel` for printing.
+Print the usage data.
 
-Example:
+If no argument is provided, usage data is printed using `console.error`.
 
 ```js
 var yargs = require("yargs")
@@ -1304,12 +1304,18 @@ var yargs = require("yargs")
 yargs.showHelp(); //prints to stderr using console.error()
 ```
 
-Or, to print the usage data to `stdout` instead, you can specify the use of `console.log`:
+If a string is specified, usage data is printed using the [`console`](https://nodejs.org/api/console.html) function `consoleLevel`.
 
 ```js
 yargs.showHelp("log"); //prints to stdout using console.log()
 ```
 
+If a function is specified, it is called with a single argument - the usage data as a string.
+
+```js
+yargs.showHelp(s => myStream.write(s)); //prints to myStream
+```
+
 Later on, `argv` can be retrieved with `yargs.argv`.
 
 .showHelpOnFail(enable, [message])
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/apply-extends.js b/lib/apply-extends.js
index 864567a..20ded5e 100644
--- a/lib/apply-extends.js
+++ b/lib/apply-extends.js
@@ -16,7 +16,21 @@ function getPathToDefaultConfig (cwd, pathToExtend) {
   return path.resolve(cwd, pathToExtend)
 }
 
-function applyExtends (config, cwd) {
+function mergeDeep (config1, config2) {
+  const target = {}
+  const isObject = obj => obj && typeof obj === 'object' && !Array.isArray(obj)
+  Object.assign(target, config1)
+  for (let key of Object.keys(config2)) {
+    if (isObject(config2[key]) && isObject(target[key])) {
+      target[key] = mergeDeep(config1[key], config2[key])
+    } else {
+      target[key] = config2[key]
+    }
+  }
+  return target
+}
+
+function applyExtends (config, cwd, mergeExtends) {
   let defaultConfig = {}
 
   if (Object.prototype.hasOwnProperty.call(config, 'extends')) {
@@ -47,7 +61,7 @@ function applyExtends (config, cwd) {
 
   previouslyVisitedConfigs = []
 
-  return Object.assign({}, defaultConfig, config)
+  return mergeExtends ? mergeDeep(defaultConfig, config) : Object.assign({}, defaultConfig, config)
 }
 
 module.exports = applyExtends
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/lib/validation.js b/lib/validation.js
index 527a0d5..1a31e58 100644
--- a/lib/validation.js
+++ b/lib/validation.js
@@ -96,7 +96,7 @@ module.exports = function validation (yargs, usage, y18n) {
       if (specialKeys.indexOf(key) === -1 &&
         !positionalMap.hasOwnProperty(key) &&
         !yargs._getParseContext().hasOwnProperty(key) &&
-        !aliases.hasOwnProperty(key)
+        !self.isValidAndSomeAliasIsNotNew(key, aliases)
       ) {
         unknown.push(key)
       }
@@ -120,6 +120,21 @@ module.exports = function validation (yargs, usage, y18n) {
     }
   }
 
+  // check for a key that is not an alias, or for which every alias is new,
+  // implying that it was invented by the parser, e.g., during camelization
+  self.isValidAndSomeAliasIsNotNew = function isValidAndSomeAliasIsNotNew (key, aliases) {
+    if (!aliases.hasOwnProperty(key)) {
+      return false
+    }
+    const newAliases = yargs.parsed.newAliases
+    for (let a of [key, ...aliases[key]]) {
+      if (!newAliases.hasOwnProperty(a) || !newAliases[key]) {
+        return true
+      }
+    }
+    return false
+  }
+
   // validate arguments limited to enumerated choices
   self.limitedChoices = function limitedChoices (argv) {
     const options = yargs.getOptions()
diff --git a/yargs.js b/yargs.js
index 5b23a0f..a4ed28e 100644
--- a/yargs.js
+++ b/yargs.js
@@ -330,7 +330,7 @@ function Yargs (processArgs, cwd, parentRequire) {
     argsert('[object|string] [string|function] [function]', [key, msg, parseFn], arguments.length)
     // allow a config object to be provided directly.
     if (typeof key === 'object') {
-      key = applyExtends(key, cwd)
+      key = applyExtends(key, cwd, self.getParserConfiguration()['deep-merge-config'])
       options.configObjects = (options.configObjects || []).concat(key)
       return self
     }
@@ -504,7 +504,7 @@ function Yargs (processArgs, cwd, parentRequire) {
 
     // If an object exists in the key, add it to options.configObjects
     if (obj[key] && typeof obj[key] === 'object') {
-      conf = applyExtends(obj[key], rootPath || cwd)
+      conf = applyExtends(obj[key], rootPath || cwd, self.getParserConfiguration()['deep-merge-config'])
       options.configObjects = (options.configObjects || []).concat(conf)
     }
 
@@ -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)
     })
   })
 
diff --git a/test/fixtures/extends/config_deep.json b/test/fixtures/extends/config_deep.json
new file mode 100644
index 0000000..d4e1bdb
--- /dev/null
+++ b/test/fixtures/extends/config_deep.json
@@ -0,0 +1,12 @@
+{
+  "a": {
+    "b": 15,
+    "c": 12,
+    "d": [1,2,3],
+    "e": [1],
+    "f": "yes"
+  },
+  "test": {
+    "yes": 1
+  }
+}
diff --git a/test/usage.js b/test/usage.js
index f3c9725..3705271 100644
--- a/test/usage.js
+++ b/test/usage.js
@@ -810,6 +810,75 @@ describe('usage tests', () => {
       r.should.have.property('exit').and.equal(true)
     })
 
+    describe('with hyphens in options', () => {
+      it('fails when an invalid argument is provided', (done) => {
+        return yargs('--foo-bar')
+          .strict()
+          .fail((msg) => {
+            return done()
+          })
+          .argv
+      })
+
+      it('accepts valid options', () => {
+        const r = checkUsage(() => {
+          const opts = {
+            '--foo-bar': { description: 'foo bar option' },
+            '--bar-baz': { description: 'bar baz option' }
+          }
+
+          return yargs('--foo-bar --bar-baz')
+            .options(opts)
+            .strict()
+            .parse()
+        })
+
+        r.result.should.have.property('foo-bar', true)
+        r.result.should.have.property('fooBar', true)
+        r.result.should.have.property('bar-baz', true)
+        r.result.should.have.property('barBaz', true)
+      })
+
+      it('works with aliases', () => {
+        const r = checkUsage(() => {
+          const opts = {
+            '--foo-bar': { description: 'foo bar option', alias: 'f' },
+            '--bar-baz': { description: 'bar baz option', alias: 'b' }
+          }
+
+          return yargs('--foo-bar -b')
+            .options(opts)
+            .strict()
+            .parse()
+        })
+
+        r.result.should.have.property('foo-bar', true)
+        r.result.should.have.property('fooBar', true)
+        r.result.should.have.property('f', true)
+        r.result.should.have.property('bar-baz', true)
+        r.result.should.have.property('barBaz', true)
+        r.result.should.have.property('b', true)
+      })
+
+      it('accepts mixed options with values', () => {
+        const r = checkUsage(() => {
+          const opts = {
+            '--foo-bar': { description: 'foo bar option', demand: true },
+            '--baz': { description: 'baz option', demand: true }
+          }
+
+          return yargs('--foo-bar 150 --baz')
+            .options(opts)
+            .strict()
+            .parse()
+        })
+
+        r.result.should.have.property('foo-bar', 150)
+        r.result.should.have.property('fooBar', 150)
+        r.result.should.have.property('baz', true)
+      })
+    })
+
     it('should fail given an option argument without a corresponding description', () => {
       const r = checkUsage(() => {
         const opts = {
@@ -2345,7 +2414,27 @@ describe('usage tests', () => {
       ])
     })
 
-    it('should call the correct console.log method', () => {
+    it('should print the help using console.error when no arguments were specified', () => {
+      const r = checkUsage(() => {
+        const y = yargs(['--foo'])
+          .options('foo', {
+            alias: 'f',
+            describe: 'foo option'
+          })
+          .wrap(null)
+
+        y.showHelp()
+      })
+
+      r.errors.join('\n').split(/\n+/).should.deep.equal([
+        'Options:',
+        '  --help     Show help  [boolean]',
+        '  --version  Show version number  [boolean]',
+        '  --foo, -f  foo option'
+      ])
+    })
+
+    it('should call the correct console.log method when specified', () => {
       const r = checkUsage(() => {
         const y = yargs(['--foo'])
           .options('foo', {
@@ -2366,7 +2455,7 @@ describe('usage tests', () => {
       ])
     })
 
-    it('should provide a help string to handler when provided', (done) => {
+    it('should call the callback to print when specified', (done) => {
       const y = yargs(['--foo'])
         .options('foo', {
           alias: 'f',
@@ -2374,8 +2463,8 @@ describe('usage tests', () => {
         })
         .wrap(null)
 
-      y.showHelp(testHandler)
-      function testHandler (msg) {
+      y.showHelp(printCallback)
+      function printCallback (msg) {
         msg.split(/\n+/).should.deep.equal([
           'Options:',
           '  --help     Show help  [boolean]',
diff --git a/test/yargs.js b/test/yargs.js
index 29ba214..8be9e7a 100644
--- a/test/yargs.js
+++ b/test/yargs.js
@@ -1374,6 +1374,56 @@ describe('yargs dsl tests', () => {
         argv.c.should.equal(201)
         argv.z.should.equal(15)
       })
+
+      it('deep merge objects when extending when configured', () => {
+        const argv = yargs()
+          .parserConfiguration({ 'deep-merge-config': true })
+          .config({
+            extends: './test/fixtures/extends/config_deep.json',
+            a: {
+              b: 11,
+              d: [5, 2],
+              f: 'no',
+              g: {
+                h: 122
+              },
+              i: [1, 2]
+            }
+          })
+          .parse()
+
+        argv.a.b.should.equal(11)
+        argv.a.c.should.equal(12)
+        argv.a.d.should.deep.equal([5, 2])
+        argv.a.e.should.equal(1)
+        argv.a.f.should.equal('no')
+        argv.a.g.h.should.equal(122)
+        argv.a.i.should.deep.equal([1, 2])
+        argv.test.yes.should.equal(1)
+      })
+
+      it('do not merge objects by default when extending', () => {
+        const argv = yargs()
+          .config({
+            extends: './test/fixtures/extends/config_deep.json',
+            a: {
+              b: 11,
+              d: [5, 2],
+              f: 'no',
+              g: {
+                h: 122
+              }
+            }
+          })
+          .parse()
+
+        argv.a.b.should.equal(11)
+        argv.a.should.not.have.property('c')
+        argv.a.d.should.deep.equal([5, 2])
+        argv.a.should.not.have.property('e')
+        argv.a.f.should.equal('no')
+        argv.a.g.h.should.equal(122)
+      })
     })
   })