Use createOption() in helpOption() to support custom option flag extraction (+ various improvements)

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');
 
 // @ts-check
@@ -67,16 +67,13 @@ 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';
     this._addImplicitHelpCommand = undefined; // Deliberately undefined, not decided whether true or false
+
+    this._helpConfiguration = {};
     this._helpCommandName = 'help';
     this._helpCommandnameAndArgs = 'help [command]';
     this._helpCommandDescription = 'display help for command';
-    this._helpConfiguration = {};
+    this.helpOption('-h, --help', 'display help for command');
   }
 
   /**
@@ -89,15 +86,6 @@ 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._helpCommandName = sourceCommand._helpCommandName;
-    this._helpCommandnameAndArgs = sourceCommand._helpCommandnameAndArgs;
-    this._helpCommandDescription = sourceCommand._helpCommandDescription;
-    this._helpConfiguration = sourceCommand._helpConfiguration;
     this._exitCallback = sourceCommand._exitCallback;
     this._storeOptionsAsProperties = sourceCommand._storeOptionsAsProperties;
     this._combineFlagAndOptionalValue = sourceCommand._combineFlagAndOptionalValue;
@@ -106,6 +94,16 @@ class Command extends EventEmitter {
     this._showHelpAfterError = sourceCommand._showHelpAfterError;
     this._showSuggestionAfterError = sourceCommand._showSuggestionAfterError;
 
+    this._helpConfiguration = sourceCommand._helpConfiguration;
+    this._helpCommandName = sourceCommand._helpCommandName;
+    this._helpCommandnameAndArgs = sourceCommand._helpCommandnameAndArgs;
+    this._helpCommandDescription = sourceCommand._helpCommandDescription;
+    if (sourceCommand._hasHelpOption) {
+      this.helpOption(sourceCommand._helpFlags, sourceCommand._helpDescription);
+    } else {
+      this._hasHelpOption = false;
+    }
+
     return this;
   }
 
@@ -1101,7 +1099,9 @@ Expecting one of '${allowedValues.join("', '")}'`);
     }
 
     // Fallback to parsing the help flag to invoke the help.
-    return this._dispatchSubcommand(subcommandName, [], [this._helpLongFlag]);
+    return this._dispatchSubcommand(subcommandName, [], [
+      this._helpOption.long || this._helpOption.short
+    ]);
   }
 
   /**
@@ -2034,7 +2034,9 @@ Expecting one of '${allowedValues.join("', '")}'`);
     }
     context.write(helpInformation);
 
-    this.emit(this._helpLongFlag); // deprecated
+    if (this._helpOption.long) {
+      this.emit(this._helpOption.long); // deprecated
+    }
     this.emit('afterHelp', context);
     getCommandAndParents(this).forEach(command => command.emit('afterAllHelp', context));
   }
@@ -2050,16 +2052,15 @@ Expecting one of '${allowedValues.join("', '")}'`);
    */
 
   helpOption(flags, description) {
-    if (typeof flags === 'boolean') {
-      this._hasHelpOption = flags;
+    if (flags !== undefined && typeof flags !== 'string') {
+      this._hasHelpOption = !!flags;
       return this;
     }
-    this._helpFlags = flags || this._helpFlags;
-    this._helpDescription = description || this._helpDescription;
+    this._hasHelpOption = true;
+    this._helpFlags = flags = flags || this._helpFlags;
+    this._helpDescription = description = description || this._helpDescription;
 
-    const helpFlags = splitOptionFlags(this._helpFlags);
-    this._helpShortFlag = helpFlags.shortFlag;
-    this._helpLongFlag = helpFlags.longFlag;
+    this._helpOption = this.createOption(flags, description);
 
     return this;
   }
@@ -2124,7 +2125,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
  */
 
 function outputHelpIfRequested(cmd, args) {
-  const helpOption = cmd._hasHelpOption && args.find(arg => arg === cmd._helpLongFlag || arg === cmd._helpShortFlag);
+  const helpOption = cmd._hasHelpOption && args.find(arg => arg === cmd._helpOption.long || arg === cmd._helpOption.short);
   if (helpOption) {
     cmd.outputHelp();
     // (Do not have all displayed text available so only passing placeholder.)

lib/help.js

@@ -71,14 +71,14 @@ 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);
+    const showShortHelpFlag = cmd._hasHelpOption && cmd._helpOption.short && !cmd._findOption(cmd._helpOption.short);
+    const showLongHelpFlag = cmd._hasHelpOption && !cmd._findOption(cmd._helpOption.long);
     if (showShortHelpFlag || showLongHelpFlag) {
       let helpOption;
       if (!showShortHelpFlag) {
-        helpOption = cmd.createOption(cmd._helpLongFlag, cmd._helpDescription);
+        helpOption = cmd.createOption(cmd._helpOption.long, cmd._helpDescription);
       } else if (!showLongHelpFlag) {
-        helpOption = cmd.createOption(cmd._helpShortFlag, cmd._helpDescription);
+        helpOption = cmd.createOption(cmd._helpOption.short, cmd._helpDescription);
       } else {
         helpOption = cmd.createOption(cmd._helpFlags, cmd._helpDescription);
       }

lib/option.js

@@ -220,13 +220,13 @@ class Option {
   /**
    * Check if `arg` matches the short or long flag.
    *
-   * @param {string} arg
+   * @param {string | undefined} arg
    * @return {boolean}
    * @api private
    */
 
   is(arg) {
-    return this.short === arg || this.long === arg;
+    return arg !== undefined && (this.short === arg || this.long === arg);
   }
 
   /**

tests/command.addHelpText.test.js

@@ -181,7 +181,7 @@ describe('context checks with full parse', () => {
   });
 
   test('when help requested then context.error is false', () => {
-    let context = {};
+    let context;
     const program = new commander.Command();
     program
       .exitOverride()
@@ -193,7 +193,7 @@ describe('context checks with full parse', () => {
   });
 
   test('when help for error then context.error is true', () => {
-    let context = {};
+    let context;
     const program = new commander.Command();
     program
       .exitOverride()
@@ -206,7 +206,7 @@ describe('context checks with full parse', () => {
   });
 
   test('when help on program then context.command is program', () => {
-    let context = {};
+    let context;
     const program = new commander.Command();
     program
       .exitOverride()
@@ -218,7 +218,7 @@ describe('context checks with full parse', () => {
   });
 
   test('when help on subcommand and "before" subcommand then context.command is subcommand', () => {
-    let context = {};
+    let context;
     const program = new commander.Command();
     program
       .exitOverride();
@@ -231,7 +231,7 @@ describe('context checks with full parse', () => {
   });
 
   test('when help on subcommand and "beforeAll" on program then context.command is subcommand', () => {
-    let context = {};
+    let context;
     const program = new commander.Command();
     program
       .exitOverride()

tests/command.copySettings.test.js

@@ -44,10 +44,11 @@ describe('copyInheritedSettings property tests', () => {
 
     source.helpOption('-Z, --zz', 'ddd');
     cmd.copyInheritedSettings(source);
+    expect(cmd._hasHelpOption).toBeTruthy();
     expect(cmd._helpFlags).toBe('-Z, --zz');
     expect(cmd._helpDescription).toBe('ddd');
-    expect(cmd._helpShortFlag).toBe('-Z');
-    expect(cmd._helpLongFlag).toBe('--zz');
+    expect(cmd._helpOption.short).toBe('-Z');
+    expect(cmd._helpOption.long).toBe('--zz');
   });
 
   test('when copyInheritedSettings then copies addHelpCommand(name, description)', () => {

tests/command.helpOption.test.js

@@ -131,3 +131,121 @@ describe('helpOption', () => {
     }).toThrow("error: unknown command 'UNKNOWN'");
   });
 });
+
+describe('obscured help flags', () => {
+  test('when obscured default help short flag parsed then outputHelp() not called', () => {
+    const program = new commander.Command();
+    program.outputHelp = jest.fn().mockImplementation(
+      program.outputHelp.bind(program)
+    );
+    program
+      .exitOverride()
+      .option('-h');
+    expect(() => {
+      program.parse(['-h'], { from: 'user' });
+    }).not.toThrow();
+    expect(program.outputHelp).not.toHaveBeenCalled();
+  });
+
+  test('when obscured default help long flag parsed then outputHelp() not called', () => {
+    const program = new commander.Command();
+    program.outputHelp = jest.fn().mockImplementation(
+      program.outputHelp.bind(program)
+    );
+    program
+      .exitOverride()
+      .option('--help');
+    expect(() => {
+      program.parse(['--help'], { from: 'user' });
+    }).not.toThrow();
+    expect(program.outputHelp).not.toHaveBeenCalled();
+  });
+
+  test('when both default help flags obscured and short flag parsed then outputHelp() not called', () => {
+    const program = new commander.Command();
+    program.outputHelp = jest.fn().mockImplementation(
+      program.outputHelp.bind(program)
+    );
+    program
+      .exitOverride()
+      .option('-h, --help');
+    expect(() => {
+      program.parse(['-h'], { from: 'user' });
+    }).not.toThrow();
+    expect(program.outputHelp).not.toHaveBeenCalled();
+  });
+
+  test('when both default help flags obscured and long flag parsed then outputHelp() not called', () => {
+    const program = new commander.Command();
+    program.outputHelp = jest.fn().mockImplementation(
+      program.outputHelp.bind(program)
+    );
+    program
+      .exitOverride()
+      .option('-h, --help');
+    expect(() => {
+      program.parse(['--help'], { from: 'user' });
+    }).not.toThrow();
+    expect(program.outputHelp).not.toHaveBeenCalled();
+  });
+
+  test('when obscured custom help short flag parsed then outputHelp() not called', () => {
+    const program = new commander.Command();
+    program.outputHelp = jest.fn().mockImplementation(
+      program.outputHelp.bind(program)
+    );
+    program
+      .exitOverride()
+      .helpOption('-c, --custom-help')
+      .option('-c');
+    expect(() => {
+      program.parse(['-c'], { from: 'user' });
+    }).not.toThrow();
+    expect(program.outputHelp).not.toHaveBeenCalled();
+  });
+
+  test('when obscured custom help long flag parsed then outputHelp() not called', () => {
+    const program = new commander.Command();
+    program.outputHelp = jest.fn().mockImplementation(
+      program.outputHelp.bind(program)
+    );
+    program
+      .exitOverride()
+      .helpOption('-c, --custom-help')
+      .option('--custom-help');
+    expect(() => {
+      program.parse(['--custom-help'], { from: 'user' });
+    }).not.toThrow();
+    expect(program.outputHelp).not.toHaveBeenCalled();
+  });
+
+  test('when both custom help flags obscured and short flag parsed then outputHelp() not called', () => {
+    const program = new commander.Command();
+    program.outputHelp = jest.fn().mockImplementation(
+      program.outputHelp.bind(program)
+    );
+    program
+      .exitOverride()
+      .helpOption('-c, --custom-help')
+      .option('-c, --custom-help');
+    expect(() => {
+      program.parse(['-c'], { from: 'user' });
+    }).not.toThrow();
+    expect(program.outputHelp).not.toHaveBeenCalled();
+  });
+
+  test('when both custom help flags obscured and long flag parsed then outputHelp() not called', () => {
+    const program = new commander.Command();
+    program.outputHelp = jest.fn().mockImplementation(
+      program.outputHelp.bind(program)
+    );
+    program
+      .exitOverride()
+      .helpOption('-c, --custom-help')
+      .option('-c, --custom-help');
+    expect(() => {
+      program.parse(['--custom-help'], { from: 'user' });
+    }).not.toThrow();
+    expect(program.outputHelp).not.toHaveBeenCalled();
+  });
+});