Repository: yargs/yargs (js)

Issue:
super eager `help` command execution
I'm not sure if I'm doing something wrong here, but the `help` command is being executed if the word `help` is anywhere in the arguments text. I'm using it for a Slack app. My parser is configured like so:

```
const parser = require('yargs')
  .usage('Usage: /<command> <args>')
  .commandDir('../commands')
  .demand(1)
  .strict()
  .help()
  .alias('help', 'help?')
  .alias('help', 'h')
```

I expect `help` to run in only two cases: If a user does `/help?` or if they do `/<command> --help`, but not if there's just `/<command> something something help something`, which is currently triggering help text. Am I misconfiguring something or misunderstanding how the help auto command works?


Pull request:
fix: less eager help command execution
Now that yargs is being used to build chat bots in a few places, the eager help command feels like more of a hinderance than a _help_; It's relatively rare that help ends a sentence, but it's fairly frequent for help to show up somewhere in a natural language string (triggering help output in a chat bot).

This is a breaking change:

* we no longer allow a single character help command to trigger help output.
* a help command must now be at the end of a sequence of commands to trigger output.

fixes #676

BREAKING CHANGE: help command now only works if it's the las command in a list.

CC: @zkat 

Patch:
diff --git a/yargs.js b/yargs.js
index f333c0e..08569fc 100644
--- a/yargs.js
+++ b/yargs.js
@@ -693,7 +693,6 @@ function Yargs (processArgs, cwd, parentRequire) {
         if (parseOptions[pk][key] && !(pk in opts)) opts[pk] = parseOptions[pk][key]
       }
     })
-
     self.group(key, usage.getPositionalGroupName())
     return self.option(key, opts)
   }
@@ -982,17 +981,14 @@ 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 :)
-          let helpCmds = [helpOpt].concat(aliases[helpOpt] || [])
-          const multiCharHelpCmds = helpCmds.filter(k => k.length > 1)
-          if (multiCharHelpCmds.length) helpCmds = multiCharHelpCmds
-          // look for and strip any helpCmds from argv._
-          argv._ = argv._.filter((cmd) => {
-            if (~helpCmds.indexOf(cmd)) {
-              argv[helpOpt] = true
-              return false
-            }
-            return true
-          })
+          const helpCmds = [helpOpt]
+            .concat(aliases[helpOpt] || [])
+            .filter(k => k.length > 1)
+          // check if help should trigger and strip it from _.
+          if (~helpCmds.indexOf(argv._[argv._.length - 1])) {
+            argv._.pop()
+            argv[helpOpt] = true
+          }
         }
         // if there's a handler associated with a
         // command defer processing to it.


Test patch:
diff --git a/test/command.js b/test/command.js
index d9e9555..d468f90 100644
--- a/test/command.js
+++ b/test/command.js
@@ -682,10 +682,14 @@ describe('Command', () => {
         ''
       ]
 
-      helpCmd.logs.join('\n').split(/\n+/).should.deep.equal(expectedCmd)
+      // no help is output if help isn't last
+      // positional argument.
+      helpCmd.logs.should.eql([])
+      helpCmdSub.logs.should.eql([])
+      cmdHelpSub.logs.should.eql([])
+
+      // shows help if it is the last positional argument.
       cmdHelp.logs.join('\n').split(/\n+/).should.deep.equal(expectedCmd)
-      helpCmdSub.logs.join('\n').split(/\n+/).should.deep.equal(expectedSub)
-      cmdHelpSub.logs.join('\n').split(/\n+/).should.deep.equal(expectedSub)
       cmdSubHelp.logs.join('\n').split(/\n+/).should.deep.equal(expectedSub)
     })
   })
diff --git a/test/yargs.js b/test/yargs.js
index c41cb7e..a9f41a8 100644
--- a/test/yargs.js
+++ b/test/yargs.js
@@ -1728,27 +1728,6 @@ describe('yargs dsl tests', () => {
       ])
       h.result.should.have.property('_').and.deep.equal(['h'])
     })
-
-    it('uses single-char help alias as command if there are no multi-char aliases', () => {
-      const h = checkOutput(() => yargs('h')
-          .help('h').alias('h', '?')
-          .wrap(null)
-          .argv
-        )
-      const q = checkOutput(() => yargs('?')
-          .help('h').alias('h', '?')
-          .wrap(null)
-          .argv
-        )
-      const expected = [
-        'Options:',
-        '  --version  Show version number  [boolean]',
-        '  -h, -?     Show help  [boolean]',
-        ''
-      ]
-      h.logs[0].split('\n').should.deep.equal(expected)
-      q.logs[0].split('\n').should.deep.equal(expected)
-    })
   })
 
   describe('.coerce()', () => {