Refactor help option implementation to hold actual Option (#2006)

Readme.md

@@ -904,6 +904,8 @@ program
   .helpOption('-e, --HELP', 'read more information');
 ```
 
+(Or use `.addHelpOption()` to add an option you construct yourself.)
+
 ### .helpCommand()
 
 A help command is added by default if your command has subcommands. You can explicitly turn on or off the implicit help command with `.helpCommand(true)` and `.helpCommand(false)`.

lib/command.js

@@ -7,7 +7,7 @@ const process = require('process');
 const { Argument, humanReadableArgName } = require('./argument.js');
 const { CommanderError } = require('./error.js');
 const { Help } = require('./help.js');
-const { Option, splitOptionFlags, DualOptions } = require('./option.js');
+const { Option, DualOptions } = require('./option.js');
 const { suggestSimilar } = require('./suggestSimilar');
 
 class Command extends EventEmitter {
@@ -66,11 +66,8 @@ class Command extends EventEmitter {
     };
 
     this._hidden = false;
-    this._hasHelpOption = true;
-    this._helpFlags = '-h, --help';
-    this._helpDescription = 'display help for command';
-    this._helpShortFlag = '-h';
-    this._helpLongFlag = '--help';
+    /** @type {(Option | null | undefined)} */
+    this._helpOption = undefined; // Lazy created on demand. May be null if help option is disabled.
     this._addImplicitHelpCommand = undefined; // undecided whether true or false yet, not inherited
     /** @type {Command} */
     this._helpCommand = undefined; // lazy initialised, inherited
@@ -87,11 +84,7 @@ class Command extends EventEmitter {
    */
   copyInheritedSettings(sourceCommand) {
     this._outputConfiguration = sourceCommand._outputConfiguration;
-    this._hasHelpOption = sourceCommand._hasHelpOption;
-    this._helpFlags = sourceCommand._helpFlags;
-    this._helpDescription = sourceCommand._helpDescription;
-    this._helpShortFlag = sourceCommand._helpShortFlag;
-    this._helpLongFlag = sourceCommand._helpLongFlag;
+    this._helpOption = sourceCommand._helpOption;
     this._helpCommand = sourceCommand._helpCommand;
     this._helpConfiguration = sourceCommand._helpConfiguration;
     this._exitCallback = sourceCommand._exitCallback;
@@ -1189,7 +1182,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
 
     // Fallback to parsing the help flag to invoke the help.
     return this._dispatchSubcommand(subcommandName, [], [
-      this._helpLongFlag || this._helpShortFlag
+      this._getHelpOption()?.long ?? this._getHelpOption()?.short ?? '--help'
     ]);
   }
 
@@ -2001,7 +1994,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
         return humanReadableArgName(arg);
       });
       return [].concat(
-        (this.options.length || this._hasHelpOption ? '[options]' : []),
+        (this.options.length || (this._helpOption !== null) ? '[options]' : []),
         (this.commands.length ? '[command]' : []),
         (this.registeredArguments.length ? args : [])
       ).join(' ');
@@ -2122,35 +2115,69 @@ Expecting one of '${allowedValues.join("', '")}'`);
     }
     context.write(helpInformation);
 
-    if (this._helpLongFlag) {
-      this.emit(this._helpLongFlag); // deprecated
+    if (this._getHelpOption()?.long) {
+      this.emit(this._getHelpOption().long); // deprecated
     }
     this.emit('afterHelp', context);
     this._getCommandAndAncestors().forEach(command => command.emit('afterAllHelp', context));
   }
 
   /**
-   * You can pass in flags and a description to override the help
-   * flags and help description for your command. Pass in false to
-   * disable the built-in help option.
+   * You can pass in flags and a description to customise the built-in help option.
+   * Pass in false to disable the built-in help option.
    *
-   * @param {(string | boolean)} [flags]
+   * @example
+   * program.helpOption('-?, --help' 'show help'); // customise
+   * program.helpOption(false); // disable
+   *
+   * @param {(string | boolean)} flags
    * @param {string} [description]
    * @return {Command} `this` command for chaining
    */
 
   helpOption(flags, description) {
+    // Support disabling built-in help option.
     if (typeof flags === 'boolean') {
-      this._hasHelpOption = flags;
+      if (flags) {
+        this._helpOption = this._helpOption ?? undefined; // preserve existing option
+      } else {
+        this._helpOption = null; // disable
+      }
       return this;
     }
-    this._helpFlags = flags || this._helpFlags;
-    this._helpDescription = description || this._helpDescription;
 
-    const helpFlags = splitOptionFlags(this._helpFlags);
-    this._helpShortFlag = helpFlags.shortFlag;
-    this._helpLongFlag = helpFlags.longFlag;
+    // Customise flags and description.
+    flags = flags ?? '-h, --help';
+    description = description ?? 'display help for command';
+    this._helpOption = this.createOption(flags, description);
+
+    return this;
+  }
 
+  /**
+   * Lazy create help option.
+   * Returns null if has been disabled with .helpOption(false).
+   *
+   * @returns {(Option | null)} the help option
+   * @package internal use only
+   */
+  _getHelpOption() {
+    // Lazy create help option on demand.
+    if (this._helpOption === undefined) {
+      this.helpOption(undefined, undefined);
+    }
+    return this._helpOption;
+  }
+
+  /**
+   * Supply your own option to use for the built-in help option.
+   * This is an alternative to using helpOption() to customise the flags and description etc.
+   *
+   * @param {Option} option
+   * @return {Command} `this` command for chaining
+   */
+  addHelpOption(option) {
+    this._helpOption = option;
     return this;
   }
 
@@ -2212,8 +2239,9 @@ Expecting one of '${allowedValues.join("', '")}'`);
    */
 
   _outputHelpIfRequested(args) {
-    const helpOption = this._hasHelpOption && args.find(arg => arg === this._helpLongFlag || arg === this._helpShortFlag);
-    if (helpOption) {
+    const helpOption = this._getHelpOption();
+    const helpRequested = helpOption && args.find(arg => helpOption.is(arg));
+    if (helpRequested) {
       this.outputHelp();
       // (Do not have all displayed text available so only passing placeholder.)
       this._exit(0, 'commander.helpDisplayed', '(outputHelp)');

lib/help.js

@@ -63,19 +63,19 @@ class Help {
 
   visibleOptions(cmd) {
     const visibleOptions = cmd.options.filter((option) => !option.hidden);
-    // Implicit help
-    const showShortHelpFlag = cmd._hasHelpOption && cmd._helpShortFlag && !cmd._findOption(cmd._helpShortFlag);
-    const showLongHelpFlag = cmd._hasHelpOption && !cmd._findOption(cmd._helpLongFlag);
-    if (showShortHelpFlag || showLongHelpFlag) {
-      let helpOption;
-      if (!showShortHelpFlag) {
-        helpOption = cmd.createOption(cmd._helpLongFlag, cmd._helpDescription);
-      } else if (!showLongHelpFlag) {
-        helpOption = cmd.createOption(cmd._helpShortFlag, cmd._helpDescription);
-      } else {
-        helpOption = cmd.createOption(cmd._helpFlags, cmd._helpDescription);
+    // Built-in help option.
+    const helpOption = cmd._getHelpOption();
+    if (helpOption && !helpOption.hidden) {
+      // Automatically hide conflicting flags. Bit dubious but a historical behaviour that is convenient for single-command programs.
+      const removeShort = helpOption.short && cmd._findOption(helpOption.short);
+      const removeLong = helpOption.long && cmd._findOption(helpOption.long);
+      if (!removeShort && !removeLong) {
+        visibleOptions.push(helpOption); // no changes needed
+      } else if (helpOption.long && !removeLong) {
+        visibleOptions.push(cmd.createOption(helpOption.long, helpOption.description));
+      } else if (helpOption.short && !removeShort) {
+        visibleOptions.push(cmd.createOption(helpOption.short, helpOption.description));
       }
-      visibleOptions.push(helpOption);
     }
     if (this.sortOptions) {
       visibleOptions.sort(this.compareOptions);

lib/option.js

@@ -324,5 +324,4 @@ function splitOptionFlags(flags) {
 }
 
 exports.Option = Option;
-exports.splitOptionFlags = splitOptionFlags;
 exports.DualOptions = DualOptions;

package.json

@@ -62,7 +62,7 @@
     "@typescript-eslint/parser": "^6.7.5",
     "eslint": "^8.30.0",
     "eslint-config-standard": "^17.0.0",
-    "eslint-config-standard-with-typescript": "^39.1.1",
+    "eslint-config-standard-with-typescript": "^40.0.0",
     "eslint-plugin-import": "^2.26.0",
     "eslint-plugin-jest": "^27.1.7",
     "eslint-plugin-n": "^16.2.0",