Repository: yargs/yargs (js)

Issue:
Crash on second call to help command
EDIT: The same error occurs doing anything a second time when using `help()`.

Version:

`6.6.0`

Error:

```
/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:899
          return k.length > 1
                  ^

TypeError: Cannot read property 'length' of undefined
    at /mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:899:19
    at Array.filter (native)
    at parseArgs (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:898:42)
    at Object.Yargs.self.parse (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:499:18)
    at SteamBot.exec (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:81:18)
    at SteamUser.SteamBot._client.on (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:35:12)
    at emitThree (events.js:116:13)
    at SteamUser.emit (events.js:195:7)
    at SteamUser._emitIdEvent (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/steam-user/components/utility.js:29:12)
    at SteamUser._handlers.(anonymous function) (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/steam-user/components/chat.js:282:9)
```

Code:

```js
'use strict'

// -- Dependencies -------------------------------------------------------------

// Node.js API
const path = require('path')

// Node packaged modules
const confit = require('confit')
const SteamUser = require('steam-user')
const yargs = require('yargs/yargs')

// Local modules
const dice = require('./plugins/cmds/dice')
const uptime = require('./plugins/cmds/uptime')

// -- Public Interface ---------------------------------------------------------

class SteamBot {
  constructor () {
    this._client = new SteamUser()
    this._client.on('friendOrChatMessage', (sender, msg, chat) => {
      this.exec(msg, { sender, chat })
    })
  }

  start (cb) {
    confit(path.resolve('config/')).create((err, config) => {
      if (err) return cb(err)

      // Build command parser
      this._parser = yargs()
      this._parser.command({
        'command': '!ch <cmd>', // config.get('cmd'),
        'description': 'test desc', // config.get('description'),
        'builder': yargs => {
          yargs.command(dice(this))
          yargs.command(uptime(this))
          yargs.help()
          yargs.version()
        }
      })

      this._client.logOn(config.get('login'))
      this._client.on('loggedOn', response => {
        this._client.setPersona(SteamUser.EPersonaState.Online)
        cb()
      })
    })
  }

  exec (cmd, context) {
    this._parser.parse(cmd, context, (err, argv, output) => {
      if (err && !output) throw err
      if (output) {
        this._client.chatMessage(context.chat, output)
      }
    })
  }
}

// -- Exports ------------------------------------------------------------------

module.exports = SteamBot

```

Steps to produce:

```
Scriptdaemon: !ch help // First call
CheevoBot:  !ch <cmd> // Description doesn't appear?

Commands:
  dice [roll]  Simulate a dice roll
  uptime       Retrieve bot uptime

Options:
  --help     Show help                                                 [boolean]
  --version  Show version number                                       [boolean]
Scriptdaemon: !ch help // Second call
CheevoBot is now Offline. // Crash
```

Pull request:
fix: running parse() multiple failed when help() enabled
there was a bug that caused an exception to be raised when `parse()` was called multiple times on the same yargs instance if `help()` is enabled.

fixes #776 

Patch:
diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index 0000000..91c3361
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,7 @@
+root = true
+
+[*.js]
+end_of_line = lf
+indent_style = space
+indent_size = 2
+insert_final_newline = true
diff --git a/README.md b/README.md
index 7f36555..ef2948e 100644
--- a/README.md
+++ b/README.md
@@ -980,6 +980,8 @@ $ node test.js
   '$0': 'test.js' }
 ```
 
+Note that a configuration object may extend from a JSON file using the `"extends"` property. When doing so, the `"extends"` value should be a path (relative or absolute) to the extended JSON file.
+
 <a name="conflicts"></a>.conflicts(x, y)
 ----------------------------------------------
 
@@ -1649,6 +1651,8 @@ as a configuration object.
 `cwd` can optionally be provided, the package.json will be read
 from this location.
 
+Note that a configuration stanza in package.json may extend from an identically keyed stanza in another package.json file using the `"extends"` property. When doing so, the `"extends"` value should be a path (relative or absolute) to the extended package.json file.
+
 .recommendCommands()
 ---------------------------
 
diff --git a/lib/apply-extends.js b/lib/apply-extends.js
new file mode 100644
index 0000000..ec751b0
--- /dev/null
+++ b/lib/apply-extends.js
@@ -0,0 +1,41 @@
+var fs = require('fs')
+var path = require('path')
+var assign = require('./assign')
+var YError = require('./yerror')
+
+var previouslyVisitedConfigs = []
+
+function checkForCircularExtends (path) {
+  if (previouslyVisitedConfigs.indexOf(path) > -1) {
+    throw new YError("Circular extended configurations: '" + path + "'.")
+  }
+}
+
+function getPathToDefaultConfig (cwd, pathToExtend) {
+  return path.resolve(cwd, pathToExtend)
+}
+
+function applyExtends (config, cwd, subKey) {
+  var defaultConfig = {}
+
+  if (config.hasOwnProperty('extends')) {
+    var pathToDefault = getPathToDefaultConfig(cwd, config.extends)
+
+    checkForCircularExtends(pathToDefault)
+
+    previouslyVisitedConfigs.push(pathToDefault)
+    delete config.extends
+
+    defaultConfig = JSON.parse(fs.readFileSync(pathToDefault, 'utf8'))
+    if (subKey) {
+      defaultConfig = defaultConfig[subKey] || {}
+    }
+    defaultConfig = applyExtends(defaultConfig, path.dirname(pathToDefault), subKey)
+  }
+
+  previouslyVisitedConfigs = []
+
+  return assign(defaultConfig, config)
+}
+
+module.exports = applyExtends
diff --git a/lib/validation.js b/lib/validation.js
index cc0ea65..3d3dbf5 100644
--- a/lib/validation.js
+++ b/lib/validation.js
@@ -135,6 +135,7 @@ module.exports = function (yargs, usage, y18n) {
         !descriptions.hasOwnProperty(key) &&
         !demandedOptions.hasOwnProperty(key) &&
         !positionalMap.hasOwnProperty(key) &&
+        !yargs._getParseContext().hasOwnProperty(key) &&
         !aliasLookup.hasOwnProperty(key)) {
         unknown.push(key)
       }
diff --git a/package.json b/package.json
index 0911614..ec5eb96 100644
--- a/package.json
+++ b/package.json
@@ -24,7 +24,7 @@
     "string-width": "^1.0.2",
     "which-module": "^1.0.0",
     "y18n": "^3.2.1",
-    "yargs-parser": "^4.2.1"
+    "yargs-parser": "^5.0.0"
   },
   "devDependencies": {
     "chai": "^3.4.1",
diff --git a/yargs.js b/yargs.js
index f39150d..a1e351f 100644
--- a/yargs.js
+++ b/yargs.js
@@ -9,6 +9,7 @@ const Validation = require('./lib/validation')
 const Y18n = require('y18n')
 const objFilter = require('./lib/obj-filter')
 const setBlocking = require('set-blocking')
+const applyExtends = require('./lib/apply-extends')
 const YError = require('./lib/yerror')
 
 var exports = module.exports = Yargs
@@ -304,6 +305,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)
       options.configObjects = (options.configObjects || []).concat(key)
       return self
     }
@@ -319,6 +321,7 @@ function Yargs (processArgs, cwd, parentRequire) {
     ;(Array.isArray(key) ? key : [key]).forEach(function (k) {
       options.config[k] = parseFn || true
     })
+
     return self
   }
 
@@ -469,11 +472,14 @@ function Yargs (processArgs, cwd, parentRequire) {
   self.pkgConf = function (key, path) {
     argsert('<string> [string]', [key, path], arguments.length)
     var conf = null
-    var obj = pkgUp(path)
+    // prefer cwd to require-main-filename in this method
+    // since we're looking for e.g. "nyc" config in nyc consumer
+    // rather than "yargs" config in nyc (where nyc is the main filename)
+    var obj = pkgUp(path || cwd)
 
     // If an object exists in the key, add it to options.configObjects
     if (obj[key] && typeof obj[key] === 'object') {
-      conf = obj[key]
+      conf = applyExtends(obj[key], path || cwd, key)
       options.configObjects = (options.configObjects || []).concat(conf)
     }
 
@@ -531,6 +537,10 @@ function Yargs (processArgs, cwd, parentRequire) {
     return parsed
   }
 
+  self._getParseContext = function () {
+    return parseContext || {}
+  }
+
   self._hasParseCallback = function () {
     return !!parseFn
   }
@@ -935,7 +945,7 @@ function Yargs (processArgs, cwd, parentRequire) {
     options.configuration = pkgUp()['yargs'] || {}
     const parsed = Parser.detailed(args, options)
     var argv = parsed.argv
-    if (parseContext) argv = assign(parseContext, argv)
+    if (parseContext) argv = assign(argv, parseContext)
     var aliases = parsed.aliases
 
     argv.$0 = self.$0
@@ -957,7 +967,7 @@ function Yargs (processArgs, cwd, parentRequire) {
         // consider any multi-char helpOpt alias as a valid help command
         // unless all helpOpt aliases are single-char
         // note that parsed.aliases is a normalized bidirectional map :)
-        var helpCmds = [helpOpt].concat(aliases[helpOpt])
+        var helpCmds = [helpOpt].concat(aliases[helpOpt] || [])
         var multiCharHelpCmds = helpCmds.filter(function (k) {
           return k.length > 1
         })


Test patch:
diff --git a/test/command.js b/test/command.js
index 9396e14..8c6df70 100644
--- a/test/command.js
+++ b/test/command.js
@@ -747,6 +747,21 @@ describe('Command', function () {
     counter.should.equal(2)
   })
 
+  // addresses: https://github.com/yargs/yargs/issues/776
+  it('allows command handler to be invoked repeatedly when help is enabled', function (done) {
+    var counter = 0
+    var y = yargs([])
+      .command('foo', 'the foo command', {}, function (argv) {
+        counter++
+      })
+      .help()
+    y.parse(['foo'], function () {})
+    y.parse(['foo'], function () {
+      counter.should.equal(2)
+      return done()
+    })
+  })
+
   // addresses https://github.com/yargs/yargs/issues/522
   it('does not require builder function to return', function () {
     var argv = yargs('yo')
diff --git a/test/fixtures/extends/circular_1.json b/test/fixtures/extends/circular_1.json
new file mode 100644
index 0000000..4cd6629
--- /dev/null
+++ b/test/fixtures/extends/circular_1.json
@@ -0,0 +1,4 @@
+{
+  "a": 44,
+  "extends": "./circular_2.json"
+}
\ No newline at end of file
diff --git a/test/fixtures/extends/circular_2.json b/test/fixtures/extends/circular_2.json
new file mode 100644
index 0000000..85b7de8
--- /dev/null
+++ b/test/fixtures/extends/circular_2.json
@@ -0,0 +1,4 @@
+{
+  "b": "any",
+  "extends": "./circular_1.json"
+}
\ No newline at end of file
diff --git a/test/fixtures/extends/config_1.json b/test/fixtures/extends/config_1.json
new file mode 100644
index 0000000..043fb87
--- /dev/null
+++ b/test/fixtures/extends/config_1.json
@@ -0,0 +1,5 @@
+{
+  "a": 30,
+  "b": 22,
+  "extends": "./config_2.json"
+}
\ No newline at end of file
diff --git a/test/fixtures/extends/config_2.json b/test/fixtures/extends/config_2.json
new file mode 100644
index 0000000..886f419
--- /dev/null
+++ b/test/fixtures/extends/config_2.json
@@ -0,0 +1,3 @@
+{
+  "z": 15
+}
\ No newline at end of file
diff --git a/test/fixtures/extends/packageA/package.json b/test/fixtures/extends/packageA/package.json
new file mode 100644
index 0000000..236e60f
--- /dev/null
+++ b/test/fixtures/extends/packageA/package.json
@@ -0,0 +1,6 @@
+{
+  "foo": {
+    "a": 80,
+    "extends": "../packageB/package.json"
+  }
+}
\ No newline at end of file
diff --git a/test/fixtures/extends/packageB/package.json b/test/fixtures/extends/packageB/package.json
new file mode 100644
index 0000000..473fda7
--- /dev/null
+++ b/test/fixtures/extends/packageB/package.json
@@ -0,0 +1,6 @@
+{
+  "foo": {
+    "a": 90,
+    "b": "riffiwobbles"
+  }
+}
\ No newline at end of file
diff --git a/test/validation.js b/test/validation.js
index a831dea..53c30bb 100644
--- a/test/validation.js
+++ b/test/validation.js
@@ -233,6 +233,43 @@ describe('validation tests', function () {
       argv._[0].should.equal('koala')
     })
 
+    // addresses: https://github.com/yargs/yargs/issues/791
+    it('should recognize context variables in strict mode', function (done) {
+      yargs()
+        .command('foo <y>')
+        .strict()
+        .parse('foo 99', {x: 33}, function (err, argv, output) {
+          expect(err).to.equal(null)
+          expect(output).to.equal('')
+          argv.y.should.equal(99)
+          argv.x.should.equal(33)
+          argv._.should.include('foo')
+          return done()
+        })
+    })
+
+    // addresses: https://github.com/yargs/yargs/issues/791
+    it('should recognize context variables in strict mode, when running sub-commands', function (done) {
+      yargs()
+        .command('request', 'request command', function (yargs) {
+          yargs
+            .command('get', 'sub-command')
+            .option('y', {
+              describe: 'y inner option'
+            })
+        })
+        .strict()
+        .parse('request get --y=22', {x: 33}, function (err, argv, output) {
+          expect(err).to.equal(null)
+          expect(output).to.equal('')
+          argv.y.should.equal(22)
+          argv.x.should.equal(33)
+          argv._.should.include('request')
+          argv._.should.include('get')
+          return done()
+        })
+    })
+
     it('fails when a required argument is missing', function (done) {
       yargs('-w 10 marsupial')
         .demand(1, ['w', 'b'])
diff --git a/test/yargs.js b/test/yargs.js
index 5d5771d..45719d2 100644
--- a/test/yargs.js
+++ b/test/yargs.js
@@ -1,16 +1,21 @@
-/* global context, describe, it, beforeEach */
+/* global context, describe, it, beforeEach, afterEach */
 
 var expect = require('chai').expect
 var fs = require('fs')
 var path = require('path')
 var checkOutput = require('./helpers/utils').checkOutput
-var yargs = require('../')
+var yargs
+var YError = require('../lib/yerror')
 
 require('chai').should()
 
 describe('yargs dsl tests', function () {
   beforeEach(function () {
-    yargs.reset()
+    yargs = require('../')
+  })
+
+  afterEach(function () {
+    delete require.cache[require.resolve('../')]
   })
 
   it('should use bin name for $0, eliminating path', function () {
@@ -768,6 +773,14 @@ describe('yargs dsl tests', function () {
       a1.foo.should.equal('bar')
       a1.context.should.equal('look at me go!')
     })
+
+    // see https://github.com/yargs/yargs/issues/724
+    it('overrides parsed value of argv with context object', function () {
+      var a1 = yargs.parse('-x=33', {
+        x: 42
+      })
+      a1.x.should.equal(42)
+    })
   })
 
   // yargs.parse(['foo', '--bar'], function (err, argv, output) {}
@@ -1155,6 +1168,42 @@ describe('yargs dsl tests', function () {
       argv.foo.should.equal(1)
       argv.bar.should.equal(2)
     })
+
+    describe('extends', function () {
+      it('applies default configurations when given config object', function () {
+        var argv = yargs
+          .config({
+            extends: './test/fixtures/extends/config_1.json',
+            a: 1
+          })
+          .argv
+
+        argv.a.should.equal(1)
+        argv.b.should.equal(22)
+        argv.z.should.equal(15)
+      })
+
+      it('protects against circular extended configurations', function () {
+        expect(function () {
+          yargs.config({extends: './test/fixtures/extends/circular_1.json'})
+        }).to.throw(YError)
+      })
+
+      it('handles aboslute paths', function () {
+        var absolutePath = path.join(process.cwd(), 'test', 'fixtures', 'extends', 'config_1.json')
+
+        var argv = yargs
+          .config({
+            a: 2,
+            extends: absolutePath
+          })
+          .argv
+
+        argv.a.should.equal(2)
+        argv.b.should.equal(22)
+        argv.z.should.equal(15)
+      })
+    })
   })
 
   describe('normalize', function () {
@@ -1411,6 +1460,20 @@ describe('yargs dsl tests', function () {
 
       argv.foo.should.equal('a')
     })
+
+    it('should apply default configurations from extended packages', function () {
+      var argv = yargs().pkgConf('foo', 'test/fixtures/extends/packageA').argv
+
+      argv.a.should.equal(80)
+      argv.b.should.equals('riffiwobbles')
+    })
+
+    it('should apply extended configurations from cwd when no path is given', function () {
+      var argv = yargs('', 'test/fixtures/extends/packageA').pkgConf('foo').argv
+
+      argv.a.should.equal(80)
+      argv.b.should.equals('riffiwobbles')
+    })
   })
 
   describe('skipValidation', function () {