diff --git a/lib/command.js b/lib/command.js index 590a271dd..1cc39133a 100644 --- a/lib/command.js +++ b/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; } @@ -500,6 +498,24 @@ Expecting one of '${allowedValues.join("', '")}'`); return new Option(flags, description); } + /** + * Check for option flag conflicts. + * Register option if no conflicts found. + * Throw otherwise. + * + * @param {Option} option + * @api private + */ + + _registerOption(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 '")}'` + : ''}`)); + } + /** * Add an option. * @@ -507,6 +523,8 @@ Expecting one of '${allowedValues.join("', '")}'`); * @return {Command} `this` command for chaining */ addOption(option) { + this._registerOption(option); + const oname = option.name(); const name = option.attributeName(); @@ -521,9 +539,6 @@ Expecting one of '${allowedValues.join("', '")}'`); this.setOptionValueWithSource(name, option.defaultValue, 'default'); } - // register the option - this.options.push(option); - // handler for cli and env supplied values const handleOptionValue = (val, invalidValueMessage, valueSource) => { // val is null for optional option used without an optional-argument. @@ -1101,7 +1116,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 + ]); } /** @@ -1820,7 +1837,7 @@ Expecting one of '${allowedValues.join("', '")}'`); description = description || 'output the version number'; const versionOption = this.createOption(flags, description); this._versionOptionName = versionOption.attributeName(); - this.options.push(versionOption); + this._registerOption(versionOption); this.on('option:' + versionOption.name(), () => { this._outputConfiguration.writeOut(`${str}\n`); this._exit(0, 'commander.version', str); @@ -2034,7 +2051,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,20 +2069,45 @@ 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); + 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 (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. * @@ -2124,7 +2168,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 index 14e0fb9f3..a112b59fe 100644 --- 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/lib/option.js b/lib/option.js index d61fc5f2f..613dd0748 100644 --- a/lib/option.js +++ b/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); } /** diff --git a/tests/command.addHelpText.test.js b/tests/command.addHelpText.test.js index e0bac2895..7103b0861 100644 --- a/tests/command.addHelpText.test.js +++ b/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() diff --git a/tests/command.copySettings.test.js b/tests/command.copySettings.test.js index 79722d78b..8b258f434 100644 --- a/tests/command.copySettings.test.js +++ b/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)', () => { diff --git a/tests/command.helpOption.test.js b/tests/command.helpOption.test.js index 00e068c94..8f074c6f1 100644 --- a/tests/command.helpOption.test.js +++ b/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(); + }); +});