Refactor _hasHelpOption, add explicit lazy getter

Author: shadowspawn

lib/command.js

@@ -66,8 +66,8 @@ class Command extends EventEmitter {
     };
 
     this._hidden = false;
+    /** @type {Option | undefined | null} */
     this._helpOption = undefined; // Lazy created on demand.
-    this._hasHelpOption = true;
     this._addImplicitHelpCommand = undefined; // Deliberately undefined, not decided whether true or false
     this._helpCommandName = 'help';
     this._helpCommandnameAndArgs = 'help [command]';
@@ -85,8 +85,7 @@ class Command extends EventEmitter {
    */
   copyInheritedSettings(sourceCommand) {
     this._outputConfiguration = sourceCommand._outputConfiguration;
-    this._helpOption = sourceCommand.helpOption();
-    this._hasHelpOption = sourceCommand._hasHelpOption;
+    this._helpOption = sourceCommand._helpOption;
     this._helpCommandName = sourceCommand._helpCommandName;
     this._helpCommandnameAndArgs = sourceCommand._helpCommandnameAndArgs;
     this._helpCommandDescription = sourceCommand._helpCommandDescription;
@@ -1097,7 +1096,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
 
     // Fallback to parsing the help flag to invoke the help.
     return this._dispatchSubcommand(subcommandName, [], [
-      this.helpOption().long ?? this.helpOption().short ?? '--help'
+      this._getHelpOption().long ?? this._getHelpOption().short ?? '--help'
     ]);
   }
 
@@ -1902,7 +1901,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(' ');
@@ -2023,8 +2022,8 @@ Expecting one of '${allowedValues.join("', '")}'`);
     }
     context.write(helpInformation);
 
-    if (this.helpOption().long) {
-      this.emit(this.helpOption().long); // deprecated
+    if (this._getHelpOption().long) {
+      this.emit(this._getHelpOption().long); // deprecated
     }
     this.emit('afterHelp', context);
     this._getCommandAndAncestors().forEach(command => command.emit('afterAllHelp', context));
@@ -2033,50 +2032,53 @@ Expecting one of '${allowedValues.join("', '")}'`);
   /**
    * 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.
-   * Lastly, can call with no arguments to return the built-in help option.
    *
    * @example
    * program.helpOption('-?, --help' 'show help'); // customise
    * program.helpOption(false); // disable
-   * const helpOption = program.helpOption(); // getter
    *
-   * @param {string | boolean} [flags]
+   * @param {string | boolean} flags
    * @param {string} [description]
-   * @return {Command | Option} `this` command for chaining (or help option if no arguments)
+   * @return {Command} `this` command for chaining
    */
 
   helpOption(flags, description) {
-    // Getter, lazy create help option on demand.
-    if (flags === undefined && description === undefined) {
-      if (this._helpOption === undefined) {
-        this._helpOption = this.createOption('-h, --help', 'display help for command');
-      }
-      return this._helpOption;
-    }
-
     // Support disabling built-in help option.
     if (typeof flags === 'boolean') {
-      this._hasHelpOption = flags;
+      this._helpOption = flags ? undefined : null;
       return this;
     }
 
     // Customise flags and description.
-    flags = flags ?? this.helpOption().flags;
-    description = description ?? this.helpOption().description;
+    flags = flags ?? this._getHelpOption().flags;
+    description = description ?? this._getHelpOption().description;
     this._helpOption = this.createOption(flags, description);
 
     return this;
   }
 
+  /**
+   *
+   * @returns {Option} the help option
+   */
+  _getHelpOption() {
+    // Lazy create help option on demand.
+    if (this._helpOption === undefined) {
+      this._helpOption = this.createOption('-h, --help', 'display help for command');
+    }
+    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|undefined} option
-   * @return {Command|Option} `this` command for chaining
+   * @param {Option} option
+   * @return {Command} `this` command for chaining
    */
   addHelpOption(option) {
     this._helpOption = option;
+    return this;
   }
 
   /**
@@ -2139,8 +2141,8 @@ Expecting one of '${allowedValues.join("', '")}'`);
  */
 
 function outputHelpIfRequested(cmd, args) {
-  const helpOption = cmd.helpOption();
-  const helpRequested = cmd._hasHelpOption && args.find(arg => helpOption.is(arg));
+  const helpOption = cmd._getHelpOption();
+  const helpRequested = helpOption && args.find(arg => helpOption.is(arg));
   if (helpRequested) {
     cmd.outputHelp();
     // (Do not have all displayed text available so only passing placeholder.)

lib/help.js

@@ -69,8 +69,8 @@ class Help {
   visibleOptions(cmd) {
     const visibleOptions = cmd.options.filter((option) => !option.hidden);
     // Built-in help option.
-    if (cmd._hasHelpOption && !cmd.helpOption().hidden) {
-      const helpOption = cmd.helpOption();
+    const helpOption = cmd._getHelpOption();
+    if (helpOption && !helpOption.hidden) {
       // Automatically hide conflicting flags. (Bit magical and messy, but preserve for now!)
       const removeShort = helpOption.short && cmd._findOption(helpOption.short);
       const removeLong = helpOption.long && cmd._findOption(helpOption.long);

tests/command.chain.test.js

@@ -130,9 +130,16 @@ describe('Command methods that should return this for chaining', () => {
     expect(result).toBe(program);
   });
 
-  test('when call .helpOption() then returns this', () => {
+  test('when call .helpOption(flags) then returns this', () => {
     const program = new Command();
-    const result = program.helpOption(false);
+    const flags = '-h, --help';
+    const result = program.helpOption(flags);
+    expect(result).toBe(program);
+  });
+
+  test('when call .addHelpOption() then returns this', () => {
+    const program = new Command();
+    const result = program.addHelpOption(new Option('-h, --help'));
     expect(result).toBe(program);
   });
 

tests/command.copySettings.test.js

@@ -31,11 +31,10 @@ describe('copyInheritedSettings property tests', () => {
   test('when copyInheritedSettings then copies helpOption(false)', () => {
     const source = new commander.Command();
     const cmd = new commander.Command();
-    expect(cmd._hasHelpOption).toBeTruthy();
 
     source.helpOption(false);
     cmd.copyInheritedSettings(source);
-    expect(cmd._hasHelpOption).toBeFalsy();
+    expect(cmd._getHelpOption()).toBe(null);
   });
 
   test('when copyInheritedSettings then copies helpOption(flags, description)', () => {
@@ -44,11 +43,12 @@ describe('copyInheritedSettings property tests', () => {
 
     source.helpOption('-Z, --zz', 'ddd');
     cmd.copyInheritedSettings(source);
-    const helpOption = cmd.helpOption();
-    expect(helpOption.flags).toBe('-Z, --zz');
-    expect(helpOption.description).toBe('ddd');
-    expect(helpOption.short).toBe('-Z');
-    expect(helpOption.long).toBe('--zz');
+    expect(cmd._getHelpOption()).toBe(source._getHelpOption());
+    // const helpOption = cmd._getHelpOption();
+    // expect(helpOption.flags).toBe('-Z, --zz');
+    // expect(helpOption.description).toBe('ddd');
+    // expect(helpOption.short).toBe('-Z');
+    // expect(helpOption.long).toBe('--zz');
   });
 
   test('when copyInheritedSettings then copies addHelpCommand(name, description)', () => {