Refactor help option and improve option parsing from parent when default command is invoked implicitly

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();
+  });
+});

tests/command.parseOptions.test.js

@@ -61,97 +61,97 @@ describe('parseOptions', () => {
   test('when empty args then empty results', () => {
     const program = createProgram();
     const result = program.parseOptions([]);
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
   });
 
   test('when only operands then results has all operands', () => {
     const program = createProgram();
     const result = program.parseOptions('one two three'.split(' '));
-    expect(result).toEqual({ operands: ['one', 'two', 'three'], unknown: [] });
+    expect(result).toEqual({ operands: ['one', 'two', 'three'], unknown: [], displayHelp: false });
   });
 
   test('when subcommand and operand then results has subcommand and operand', () => {
     const program = createProgram();
     const result = program.parseOptions('sub one'.split(' '));
-    expect(result).toEqual({ operands: ['sub', 'one'], unknown: [] });
+    expect(result).toEqual({ operands: ['sub', 'one'], unknown: [], displayHelp: false });
   });
 
   test('when program has flag then option removed', () => {
     const program = createProgram();
     const result = program.parseOptions('--global-flag'.split(' '));
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
   });
 
   test('when program has option with value then option removed', () => {
     const program = createProgram();
     const result = program.parseOptions('--global-value foo'.split(' '));
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
   });
 
   test('when program has flag before operand then option removed', () => {
     const program = createProgram();
     const result = program.parseOptions('--global-flag arg'.split(' '));
-    expect(result).toEqual({ operands: ['arg'], unknown: [] });
+    expect(result).toEqual({ operands: ['arg'], unknown: [], displayHelp: false });
   });
 
   test('when program has flag after operand then option removed', () => {
     const program = createProgram();
     const result = program.parseOptions('arg --global-flag'.split(' '));
-    expect(result).toEqual({ operands: ['arg'], unknown: [] });
+    expect(result).toEqual({ operands: ['arg'], unknown: [], displayHelp: false });
   });
 
   test('when program has flag after subcommand then option removed', () => {
     const program = createProgram();
     const result = program.parseOptions('sub --global-flag'.split(' '));
-    expect(result).toEqual({ operands: ['sub'], unknown: [] });
+    expect(result).toEqual({ operands: ['sub'], unknown: [], displayHelp: false });
   });
 
   test('when program has unknown option then option returned in unknown', () => {
     const program = createProgram();
     const result = program.parseOptions('--unknown'.split(' '));
-    expect(result).toEqual({ operands: [], unknown: ['--unknown'] });
+    expect(result).toEqual({ operands: [], unknown: ['--unknown'], displayHelp: false });
   });
 
   test('when program has unknown option before operands then all unknown in same order', () => {
     const program = createProgram();
     const result = program.parseOptions('--unknown arg'.split(' '));
-    expect(result).toEqual({ operands: [], unknown: ['--unknown', 'arg'] });
+    expect(result).toEqual({ operands: [], unknown: ['--unknown', 'arg'], displayHelp: false });
   });
 
   test('when program has unknown option after operand then option returned in unknown', () => {
     const program = createProgram();
     const result = program.parseOptions('arg --unknown'.split(' '));
-    expect(result).toEqual({ operands: ['arg'], unknown: ['--unknown'] });
+    expect(result).toEqual({ operands: ['arg'], unknown: ['--unknown'], displayHelp: false });
   });
 
   test('when program has flag after unknown option then flag removed', () => {
     const program = createProgram();
     const result = program.parseOptions('--unknown --global-flag'.split(' '));
-    expect(result).toEqual({ operands: [], unknown: ['--unknown'] });
+    expect(result).toEqual({ operands: [], unknown: ['--unknown'], displayHelp: false });
   });
 
   test('when subcommand has flag then flag returned as unknown', () => {
     const program = createProgram();
     const result = program.parseOptions('sub --sub-flag'.split(' '));
-    expect(result).toEqual({ operands: ['sub'], unknown: ['--sub-flag'] });
+    expect(result).toEqual({ operands: ['sub'], unknown: ['--sub-flag'], displayHelp: false });
   });
 
   test('when program has literal before known flag then option returned as operand', () => {
     const program = createProgram();
     const result = program.parseOptions('-- --global-flag'.split(' '));
-    expect(result).toEqual({ operands: ['--global-flag'], unknown: [] });
+    expect(result).toEqual({ operands: ['--global-flag'], unknown: [], displayHelp: false });
   });
 
   test('when program has literal before unknown option then option returned as operand', () => {
     const program = createProgram();
     const result = program.parseOptions('-- --unknown uuu'.split(' '));
-    expect(result).toEqual({ operands: ['--unknown', 'uuu'], unknown: [] });
+    expect(result).toEqual({ operands: ['--unknown', 'uuu'], unknown: [], displayHelp: false });
   });
 
   test('when program has literal after unknown option then literal preserved too', () => {
     const program = createProgram();
     const result = program.parseOptions('--unknown1 -- --unknown2'.split(' '));
-    expect(result).toEqual({ operands: [], unknown: ['--unknown1', '--', '--unknown2'] });
+    expect(result).toEqual({ operands: [], unknown: ['--unknown1', '--', '--unknown2'], displayHelp: false });
   });
 });
 
@@ -236,56 +236,56 @@ describe('Utility Conventions', () => {
   test('when program has combo known boolean short flags then arg removed', () => {
     const program = createProgram();
     const result = program.parseOptions(['-ab']);
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
     expect(program.opts()).toEqual({ aaa: true, bbb: true });
   });
 
   test('when program has combo unknown short flags then arg preserved', () => {
     const program = createProgram();
     const result = program.parseOptions(['-pq']);
-    expect(result).toEqual({ operands: [], unknown: ['-pq'] });
+    expect(result).toEqual({ operands: [], unknown: ['-pq'], displayHelp: false });
     expect(program.opts()).toEqual({ });
   });
 
   test('when program has combo known short option and required value then arg removed', () => {
     const program = createProgram();
     const result = program.parseOptions(['-cvalue']);
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
     expect(program.opts()).toEqual({ ccc: 'value' });
   });
 
   test('when program has combo known short option and optional value then arg removed', () => {
     const program = createProgram();
     const result = program.parseOptions(['-dvalue']);
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
     expect(program.opts()).toEqual({ ddd: 'value' });
   });
 
   test('when program has known combo short boolean flags and required value then arg removed', () => {
     const program = createProgram();
     const result = program.parseOptions(['-abcvalue']);
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
     expect(program.opts()).toEqual({ aaa: true, bbb: true, ccc: 'value' });
   });
 
   test('when program has known combo short boolean flags and optional value then arg removed', () => {
     const program = createProgram();
     const result = program.parseOptions(['-abdvalue']);
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
     expect(program.opts()).toEqual({ aaa: true, bbb: true, ddd: 'value' });
   });
 
   test('when program has known long flag=value then arg removed', () => {
     const program = createProgram();
     const result = program.parseOptions(['--ccc=value']);
-    expect(result).toEqual({ operands: [], unknown: [] });
+    expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
     expect(program.opts()).toEqual({ ccc: 'value' });
   });
 
   test('when program has unknown long flag=value then arg preserved', () => {
     const program = createProgram();
     const result = program.parseOptions(['--rrr=value']);
-    expect(result).toEqual({ operands: [], unknown: ['--rrr=value'] });
+    expect(result).toEqual({ operands: [], unknown: ['--rrr=value'], displayHelp: false });
     expect(program.opts()).toEqual({ });
   });