Warn about obscured help option

tj · Aug 3, 2023 · d03566b · d03566b
1 parent 1bb30d7
commit d03566b
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 12 deletions.
diff --git a/lib/command.js b/lib/command.js
@@ -12,6 +12,8 @@ const { suggestSimilar } = require('./suggestSimilar');
 
 // @ts-check
 
+const PRODUCTION = process.env.NODE_ENV === 'production';
+
 class Command extends EventEmitter {
   /**
    * Initialize a new `Command`.
@@ -521,6 +523,11 @@ Expecting one of '${allowedValues.join("', '")}'`);
 
     // register the option
     this.options.push(option);
+    this._checkForObscuredHelpOption((matchingOptionFlags) => (
+      `Help option '${this._helpOption.flags}' is obscured after adding option '${option.flags}'${matchingOptionFlags.length > 1
+        ? `
+- conflicts with options '${matchingOptionFlags.join("' and '")}'`
+        : ''}`));
 
     // handler for cli and env supplied values
     const handleOptionValue = (val, invalidValueMessage, valueSource) => {
@@ -1099,7 +1106,7 @@ 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]);
   }
 
   /**
@@ -2032,7 +2039,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
     }
     context.write(helpInformation);
 
-    this.emit(this._helpLongFlag); // deprecated
+    this.emit(this._helpOption.long); // deprecated
     this.emit('afterHelp', context);
     getCommandAndParents(this).forEach(command => command.emit('afterAllHelp', context));
   }
@@ -2056,13 +2063,37 @@ Expecting one of '${allowedValues.join("', '")}'`);
     this._helpFlags = flags = flags || this._helpFlags;
     this._helpDescription = description = description || this._helpDescription;
 
-    const helpOption = this.createOption(flags, description);
-    this._helpShortFlag = helpOption.short;
-    this._helpLongFlag = helpOption.long;
+    this._helpOption = this.createOption(flags, description);
+    this._checkForObscuredHelpOption((matchingOptionFlags) => (
+      `Newly added help option '${this._helpOption.flags}' is obscured
+- conflicts with ${matchingOptionFlags.length > 1 ? 'options' : 'option'} '${matchingOptionFlags.join("' and '")}'`));
 
     return this;
   }
 
+  /**
+   * @api private
+   */
+
+  _checkForObscuredHelpOption(makeMessage) {
+    if (!PRODUCTION && this._hasHelpOption) {
+      const shortMatchingOption = this._helpOption.short &&
+        this._findOption(this._helpOption.short);
+      if (shortMatchingOption || !this._helpOption.short) {
+        const longMatchingOption = this._helpOption.long &&
+          this._findOption(this._helpOption.long);
+        if (longMatchingOption || !this._helpOption.long) {
+          const matchingOptionFlags = (
+            shortMatchingOption === longMatchingOption
+              ? [shortMatchingOption]
+              : [shortMatchingOption, longMatchingOption]
+          ).filter(option => option).map(option => option.flags);
+          console.warn(makeMessage(matchingOptionFlags));
+        }
+      }
+    }
+  }
+
   /**
    * Output help information and exit.
    *
@@ -2123,7 +2154,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.)

diff --git a/lib/help.js b/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);
       }

diff --git a/tests/command.copySettings.test.js b/tests/command.copySettings.test.js
@@ -46,8 +46,8 @@ describe('copyInheritedSettings property tests', () => {
     cmd.copyInheritedSettings(source);
     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)', () => {