From 1bb30d7d99965b04aeaf7db3d099d751c6431bac Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Thu, 3 Aug 2023 00:44:56 +0300 Subject: [PATCH 01/10] Support custom flags from Option subclass in helpOption() --- lib/command.js | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/lib/command.js b/lib/command.js index 590a271dd..6cdf0bafd 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; } @@ -2050,16 +2048,17 @@ 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; + const helpOption = this.createOption(flags, description); + this._helpShortFlag = helpOption.short; + this._helpLongFlag = helpOption.long; return this; } From e8bea4aedd750418d094591d63699010f5c7b623 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Thu, 3 Aug 2023 13:04:15 +0300 Subject: [PATCH 02/10] Fix version() parameter type Borrowed from a3f0e28 that was supposed to land in the now-closed #1921. --- lib/command.js | 2 +- typings/index.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/command.js b/lib/command.js index 590a271dd..e014d957f 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1807,7 +1807,7 @@ Expecting one of '${allowedValues.join("', '")}'`); * * You can optionally supply the flags and description to override the defaults. * - * @param {string} str + * @param {string} [str] * @param {string} [flags] * @param {string} [description] * @return {this | string} `this` command for chaining, or version string if no arguments diff --git a/typings/index.d.ts b/typings/index.d.ts index 695c3bd25..73c606a5d 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -293,7 +293,7 @@ export class Command { * * You can optionally supply the flags and description to override the defaults. */ - version(str: string, flags?: string, description?: string): this; + version(str?: string, flags?: string, description?: string): this; /** * Define a command, implemented using an action handler. From e4d00db9dfb5e6620ab38b7c24070c5b16868562 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Wed, 2 Aug 2023 17:11:24 +0300 Subject: [PATCH 03/10] Change initial variable values in test for better error messages (cherry picked from commit 87db4ba81c33b0db8a0146afdcaac7e1a4dbcce8) --- tests/command.addHelpText.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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() From 9857b1eea2e64e42ab8710ab80cef98e0a71b354 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Fri, 4 Aug 2023 00:50:24 +0300 Subject: [PATCH 04/10] Store help option instance instead of just flags Analogous to 3c94dfd in #1933 for version option. Storing the Option instance of the help option instead of just its flags (_helpShortFlag, _helpLongFlag) is meaningful for cases when other properties of the instance need to be accessed (not currently the case, but could be in the future). --- lib/command.js | 10 ++++------ lib/help.js | 8 ++++---- tests/command.copySettings.test.js | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/command.js b/lib/command.js index 6cdf0bafd..e8aadb249 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1099,7 +1099,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 +2032,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,9 +2056,7 @@ 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); return this; } @@ -2123,7 +2121,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/tests/command.copySettings.test.js b/tests/command.copySettings.test.js index 79722d78b..0925c779c 100644 --- 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)', () => { From da1e153f2c9a7bceacdc644dd95fa3f48c3e183d Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Fri, 4 Aug 2023 01:57:12 +0300 Subject: [PATCH 05/10] Add missing _hasHelpOption check in copyInheritedSettings test --- tests/command.copySettings.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/command.copySettings.test.js b/tests/command.copySettings.test.js index 0925c779c..8b258f434 100644 --- a/tests/command.copySettings.test.js +++ b/tests/command.copySettings.test.js @@ -44,6 +44,7 @@ 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._helpOption.short).toBe('-Z'); From 35e95710f34717718e9471abe6b1d6453ead616f Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Fri, 4 Aug 2023 02:25:32 +0300 Subject: [PATCH 06/10] Do not use undefined long help option flag in legacy code --- lib/command.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/command.js b/lib/command.js index e014d957f..d0eaa6845 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1101,7 +1101,9 @@ Expecting one of '${allowedValues.join("', '")}'`); } // Fallback to parsing the help flag to invoke the help. - return this._dispatchSubcommand(subcommandName, [], [this._helpLongFlag]); + if (this._helpLongFlag) { + return this._dispatchSubcommand(subcommandName, [], [this._helpLongFlag]); + } } /** @@ -2034,7 +2036,9 @@ Expecting one of '${allowedValues.join("', '")}'`); } context.write(helpInformation); - this.emit(this._helpLongFlag); // deprecated + if (this._helpLongFlag) { + this.emit(this._helpLongFlag); // deprecated + } this.emit('afterHelp', context); getCommandAndParents(this).forEach(command => command.emit('afterAllHelp', context)); } From 97f3069d1fc60a36c7d5ce6d24ebc121506964a8 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Fri, 4 Aug 2023 04:12:12 +0300 Subject: [PATCH 07/10] Check for undefined in Option.is() Eliminates the need for the check in other places. (cherry picked from commit a12f5bec4ffa02fa84b3b67d746c616f0efb2130) --- lib/option.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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); } /** From 7335a9cb3b35d4712254c2f121e1453c6fc6e201 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Fri, 4 Aug 2023 10:15:55 +0300 Subject: [PATCH 08/10] Add missing obscured help flag tests --- tests/command.helpOption.test.js | 118 +++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) 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(); + }); +}); From a253ec6a73a209d58ea5f1d71c8c34afc65e7e2b Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sun, 13 Aug 2023 01:10:13 +0200 Subject: [PATCH 09/10] Revert "Fix version() parameter type" This reverts commit e8bea4aedd750418d094591d63699010f5c7b623. --- lib/command.js | 2 +- typings/index.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/command.js b/lib/command.js index d0eaa6845..25464793c 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1809,7 +1809,7 @@ Expecting one of '${allowedValues.join("', '")}'`); * * You can optionally supply the flags and description to override the defaults. * - * @param {string} [str] + * @param {string} str * @param {string} [flags] * @param {string} [description] * @return {this | string} `this` command for chaining, or version string if no arguments diff --git a/typings/index.d.ts b/typings/index.d.ts index 73c606a5d..695c3bd25 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -293,7 +293,7 @@ export class Command { * * You can optionally supply the flags and description to override the defaults. */ - version(str?: string, flags?: string, description?: string): this; + version(str: string, flags?: string, description?: string): this; /** * Define a command, implemented using an action handler. From 8dd417f31b5b289945e1b84ed7c71623ece29524 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sun, 13 Aug 2023 03:02:26 +0200 Subject: [PATCH 10/10] Fix help for commands with executable handler & only a short help flag --- lib/command.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/command.js b/lib/command.js index 25464793c..4fca7f998 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1101,9 +1101,9 @@ Expecting one of '${allowedValues.join("', '")}'`); } // Fallback to parsing the help flag to invoke the help. - if (this._helpLongFlag) { - return this._dispatchSubcommand(subcommandName, [], [this._helpLongFlag]); - } + return this._dispatchSubcommand(subcommandName, [], [ + this._helpLongFlag || this._helpShortFlag + ]); } /**