From 4f5ab2de1ba232893ed312816859c7eb9f7c6a98 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 02:20:56 +0300 Subject: [PATCH 01/18] Add missing checks for passThroughOptions --- lib/command.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/command.js b/lib/command.js index 590a271dd..ac8189866 100644 --- a/lib/command.js +++ b/lib/command.js @@ -272,6 +272,8 @@ class Command extends EventEmitter { this.commands.push(cmd); cmd.parent = this; + this._checkForIllegalPassThroughOptions(cmd, this); + return this; } @@ -721,6 +723,9 @@ Expecting one of '${allowedValues.join("', '")}'`); */ enablePositionalOptions(positional = true) { this._enablePositionalOptions = !!positional; + this.commands.forEach((command) => { + this._checkForIllegalPassThroughOptions(command, this); + }); return this; } @@ -735,12 +740,22 @@ Expecting one of '${allowedValues.join("', '")}'`); */ passThroughOptions(passThrough = true) { this._passThroughOptions = !!passThrough; - if (!!this.parent && passThrough && !this.parent._enablePositionalOptions) { - throw new Error('passThroughOptions can not be used without turning on enablePositionalOptions for parent command(s)'); - } + this._checkForIllegalPassThroughOptions(this, this.parent); return this; } + /** + * @param {Command} command + * @param {Command | null} parent + * @api private + */ + + _checkForIllegalPassThroughOptions(command, parent) { + if (parent && command._passThroughOptions && !parent._enablePositionalOptions) { + throw new Error(`passThroughOptions cannot be used for '${command._name}' without turning on enablePositionalOptions for parent command${parent._name ? ` '${parent._name}'` : ''}`); + } + } + /** * Whether to store option values as properties on command object, * or store separately (specify false). In both cases the option values can be accessed using .opts(). From 0ac660905f0dfd4ec1ea6d31ddbf808ad5a4ac5d Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 02:35:29 +0300 Subject: [PATCH 02/18] Add tests for illegal passThroughOptions --- tests/command.positionalOptions.test.js | 35 ++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/command.positionalOptions.test.js b/tests/command.positionalOptions.test.js index d130b7835..169866d20 100644 --- a/tests/command.positionalOptions.test.js +++ b/tests/command.positionalOptions.test.js @@ -362,13 +362,36 @@ describe('program with action handler and positionalOptions and subcommand', () // ------------------------------------------------------------------------------ -test('when program not positional and turn on passthrough in subcommand then error', () => { - const program = new commander.Command(); - const sub = program.command('sub'); +describe('illegal passThroughOptions', () => { + test('when program not positional and turn on passThroughOptions in subcommand then error', () => { + const program = new commander.Command(); + const sub = program.command('sub'); + + expect(() => { + sub.passThroughOptions(); + }).toThrow(); + }); + + test('when program not positional and add subcommand with passThroughOptions then error', () => { + const program = new commander.Command(); + const sub = new commander.Command('sub') + .passThroughOptions(); - expect(() => { - sub.passThroughOptions(); - }).toThrow(); + expect(() => { + program.addCommand(sub); + }).toThrow(); + }); + + test('when program has subcommand with passThroughOptions and reset to non-positional then error', () => { + const program = new commander.Command() + .enablePositionalOptions(); + program.command('sub') + .passThroughOptions(); + + expect(() => { + program.enablePositionalOptions(false); + }).toThrow(); + }); }); // ------------------------------------------------------------------------------ From 9d4c96a4fc40590dced784d24f26661d183fb512 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 03:14:33 +0300 Subject: [PATCH 03/18] Remove illegal passThroughOptions check deemed unnecessary --- lib/command.js | 3 --- tests/command.positionalOptions.test.js | 11 ----------- 2 files changed, 14 deletions(-) diff --git a/lib/command.js b/lib/command.js index ac8189866..e01586a1d 100644 --- a/lib/command.js +++ b/lib/command.js @@ -723,9 +723,6 @@ Expecting one of '${allowedValues.join("', '")}'`); */ enablePositionalOptions(positional = true) { this._enablePositionalOptions = !!positional; - this.commands.forEach((command) => { - this._checkForIllegalPassThroughOptions(command, this); - }); return this; } diff --git a/tests/command.positionalOptions.test.js b/tests/command.positionalOptions.test.js index 169866d20..73dd7a877 100644 --- a/tests/command.positionalOptions.test.js +++ b/tests/command.positionalOptions.test.js @@ -381,17 +381,6 @@ describe('illegal passThroughOptions', () => { program.addCommand(sub); }).toThrow(); }); - - test('when program has subcommand with passThroughOptions and reset to non-positional then error', () => { - const program = new commander.Command() - .enablePositionalOptions(); - program.command('sub') - .passThroughOptions(); - - expect(() => { - program.enablePositionalOptions(false); - }).toThrow(); - }); }); // ------------------------------------------------------------------------------ From 43d9faa402345f0424ee642203535b2c1aa25bb2 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 03:15:42 +0300 Subject: [PATCH 04/18] Use weaker wording: "broken" instead of "illegal" --- lib/command.js | 6 +++--- tests/command.positionalOptions.test.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/command.js b/lib/command.js index e01586a1d..d190512ed 100644 --- a/lib/command.js +++ b/lib/command.js @@ -272,7 +272,7 @@ class Command extends EventEmitter { this.commands.push(cmd); cmd.parent = this; - this._checkForIllegalPassThroughOptions(cmd, this); + this._checkForBrokenPassThrough(cmd, this); return this; } @@ -737,7 +737,7 @@ Expecting one of '${allowedValues.join("', '")}'`); */ passThroughOptions(passThrough = true) { this._passThroughOptions = !!passThrough; - this._checkForIllegalPassThroughOptions(this, this.parent); + this._checkForBrokenPassThrough(this, this.parent); return this; } @@ -747,7 +747,7 @@ Expecting one of '${allowedValues.join("', '")}'`); * @api private */ - _checkForIllegalPassThroughOptions(command, parent) { + _checkForBrokenPassThrough(command, parent) { if (parent && command._passThroughOptions && !parent._enablePositionalOptions) { throw new Error(`passThroughOptions cannot be used for '${command._name}' without turning on enablePositionalOptions for parent command${parent._name ? ` '${parent._name}'` : ''}`); } diff --git a/tests/command.positionalOptions.test.js b/tests/command.positionalOptions.test.js index 73dd7a877..82011e136 100644 --- a/tests/command.positionalOptions.test.js +++ b/tests/command.positionalOptions.test.js @@ -362,7 +362,7 @@ describe('program with action handler and positionalOptions and subcommand', () // ------------------------------------------------------------------------------ -describe('illegal passThroughOptions', () => { +describe('broken passThrough', () => { test('when program not positional and turn on passThroughOptions in subcommand then error', () => { const program = new commander.Command(); const sub = program.command('sub'); From 43cc821cb89da5b804713a9472b860187c090722 Mon Sep 17 00:00:00 2001 From: aweebit Date: Sat, 5 Aug 2023 03:42:38 +0300 Subject: [PATCH 05/18] Unclutter error message in broken passThrough checks Co-authored-by: John Gee --- lib/command.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/command.js b/lib/command.js index d190512ed..65559112a 100644 --- a/lib/command.js +++ b/lib/command.js @@ -749,7 +749,7 @@ Expecting one of '${allowedValues.join("', '")}'`); _checkForBrokenPassThrough(command, parent) { if (parent && command._passThroughOptions && !parent._enablePositionalOptions) { - throw new Error(`passThroughOptions cannot be used for '${command._name}' without turning on enablePositionalOptions for parent command${parent._name ? ` '${parent._name}'` : ''}`); + throw new Error(`passThroughOptions cannot be used for '${command._name}' without turning on enablePositionalOptions for parent command(s)`); } } From 7689506699b9db61a121baea40ba455e8d89c78d Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 04:01:21 +0300 Subject: [PATCH 06/18] Refactor _checkForBrokenPassThrough() to make it instance-aware --- lib/command.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/command.js b/lib/command.js index 65559112a..e4b5d1699 100644 --- a/lib/command.js +++ b/lib/command.js @@ -272,7 +272,7 @@ class Command extends EventEmitter { this.commands.push(cmd); cmd.parent = this; - this._checkForBrokenPassThrough(cmd, this); + cmd._checkForBrokenPassThrough(); return this; } @@ -737,19 +737,17 @@ Expecting one of '${allowedValues.join("', '")}'`); */ passThroughOptions(passThrough = true) { this._passThroughOptions = !!passThrough; - this._checkForBrokenPassThrough(this, this.parent); + this._checkForBrokenPassThrough(); return this; } /** - * @param {Command} command - * @param {Command | null} parent * @api private */ - _checkForBrokenPassThrough(command, parent) { - if (parent && command._passThroughOptions && !parent._enablePositionalOptions) { - throw new Error(`passThroughOptions cannot be used for '${command._name}' without turning on enablePositionalOptions for parent command(s)`); + _checkForBrokenPassThrough() { + if (this.parent && this._passThroughOptions && !this.parent._enablePositionalOptions) { + throw new Error(`passThroughOptions cannot be used for '${this._name}' without turning on enablePositionalOptions for parent command(s)`); } } From d1686db1fb417551af88b89f4b6ccc9f1c3d252e Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 04:02:53 +0300 Subject: [PATCH 07/18] Add subroutine for common parse call code Borrowed from #1917 and #1919. --- lib/command.js | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/lib/command.js b/lib/command.js index e4b5d1699..ce9030701 100644 --- a/lib/command.js +++ b/lib/command.js @@ -897,6 +897,21 @@ Expecting one of '${allowedValues.join("', '")}'`); return userArgs; } + /** + * @param {boolean} async + * @param {Function} userArgsCallback + * @param {string[]} [argv] + * @param {Object} [parseOptions] + * @param {string} [parseOptions.from] + * @return {Command|Promise} + * @api private + */ + + _parseSubroutine(async, userArgsCallback, argv, parseOptions) { + const userArgs = this._prepareUserArgs(argv, parseOptions); + return userArgsCallback(userArgs); + } + /** * Parse `argv`, setting options and invoking commands when defined. * @@ -915,10 +930,10 @@ Expecting one of '${allowedValues.join("', '")}'`); */ parse(argv, parseOptions) { - const userArgs = this._prepareUserArgs(argv, parseOptions); - this._parseCommand([], userArgs); - - return this; + return this._parseSubroutine(false, (userArgs) => { + this._parseCommand([], userArgs); + return this; + }, argv, parseOptions); } /** @@ -941,10 +956,10 @@ Expecting one of '${allowedValues.join("', '")}'`); */ async parseAsync(argv, parseOptions) { - const userArgs = this._prepareUserArgs(argv, parseOptions); - await this._parseCommand([], userArgs); - - return this; + return this._parseSubroutine(true, async(userArgs) => { + await this._parseCommand([], userArgs); + return this; + }, argv, parseOptions); } /** From 3de6161485e1c98361d2425776684cde79a54235 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 04:05:42 +0300 Subject: [PATCH 08/18] Add support for sharing and stand-alone parsing of subcommands --- lib/command.js | 46 +++++++++++++++++++++++++++++++--------------- lib/help.js | 4 ++-- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/lib/command.js b/lib/command.js index ce9030701..ee31ec371 100644 --- a/lib/command.js +++ b/lib/command.js @@ -25,7 +25,17 @@ class Command extends EventEmitter { this.commands = []; /** @type {Option[]} */ this.options = []; - this.parent = null; + /** @type {Command[]} */ + this.parents = []; + /** + * Last added parent command, + * or parent command in last parse call. + * + * @type {Command | null} + */ + this.currentParent = null; + /** @type {Command | null} */ + this.parent = null; // last added parent command (legacy) this._allowUnknownOption = false; this._allowExcessArguments = true; /** @type {Argument[]} */ @@ -154,7 +164,8 @@ class Command extends EventEmitter { cmd._executableFile = opts.executableFile || null; // Custom name for executable file, set missing to null to match constructor if (args) cmd.arguments(args); this.commands.push(cmd); - cmd.parent = this; + cmd.parents.push(this); + cmd.currentParent = cmd.parent = this; cmd.copyInheritedSettings(this); if (desc) return this; @@ -271,7 +282,8 @@ class Command extends EventEmitter { if (opts.noHelp || opts.hidden) cmd._hidden = true; // modifying passed command due to existing implementation this.commands.push(cmd); - cmd.parent = this; + cmd.parents.push(this); + cmd.currentParent = cmd.parent = this; cmd._checkForBrokenPassThrough(); return this; @@ -746,7 +758,9 @@ Expecting one of '${allowedValues.join("', '")}'`); */ _checkForBrokenPassThrough() { - if (this.parent && this._passThroughOptions && !this.parent._enablePositionalOptions) { + if (this._passThroughOptions && this.parents.some( + (parent) => !parent._enablePositionalOptions + )) { throw new Error(`passThroughOptions cannot be used for '${this._name}' without turning on enablePositionalOptions for parent command(s)`); } } @@ -1103,7 +1117,7 @@ Expecting one of '${allowedValues.join("', '")}'`); if (subCommand._executableHandler) { this._executeSubCommand(subCommand, operands.concat(unknown)); } else { - return subCommand._parseCommand(operands, unknown); + return subCommand._parseCommand(operands, unknown, this); } }); return hookResult; @@ -1281,7 +1295,9 @@ Expecting one of '${allowedValues.join("', '")}'`); * @api private */ - _parseCommand(operands, unknown) { + _parseCommand(operands, unknown, currentParent) { + this.currentParent = currentParent ?? null; + const parsed = this.parseOptions(unknown); this._parseOptionsEnv(); // after cli, so parseArg not called on both cli and env this._parseOptionsImplied(); @@ -1323,18 +1339,18 @@ Expecting one of '${allowedValues.join("', '")}'`); let actionResult; actionResult = this._chainOrCallHooks(actionResult, 'preAction'); actionResult = this._chainOrCall(actionResult, () => this._actionHandler(this.processedArgs)); - if (this.parent) { + if (currentParent) { actionResult = this._chainOrCall(actionResult, () => { - this.parent.emit(commandEvent, operands, unknown); // legacy + currentParent.emit(commandEvent, operands, unknown); // legacy }); } actionResult = this._chainOrCallHooks(actionResult, 'postAction'); return actionResult; } - if (this.parent && this.parent.listenerCount(commandEvent)) { + if (currentParent?.listenerCount(commandEvent)) { checkForUnknownOptions(); this._processArguments(); - this.parent.emit(commandEvent, operands, unknown); // legacy + currentParent.emit(commandEvent, operands, unknown); // legacy } else if (operands.length) { if (this._findCommand('*')) { // legacy default command return this._dispatchSubcommand('*', operands, unknown); @@ -1390,7 +1406,7 @@ Expecting one of '${allowedValues.join("', '")}'`); _checkForMissingMandatoryOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. - for (let cmd = this; cmd; cmd = cmd.parent) { + for (let cmd = this; cmd; cmd = cmd.currentParent) { cmd.options.forEach((anOption) => { if (anOption.mandatory && (cmd.getOptionValue(anOption.attributeName()) === undefined)) { cmd.missingMandatoryOptionValue(anOption); @@ -1437,7 +1453,7 @@ Expecting one of '${allowedValues.join("', '")}'`); */ _checkForConflictingOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. - for (let cmd = this; cmd; cmd = cmd.parent) { + for (let cmd = this; cmd; cmd = cmd.currentParent) { cmd._checkForConflictingLocalOptions(); } } @@ -1774,7 +1790,7 @@ Expecting one of '${allowedValues.join("', '")}'`); .filter(option => option.long) .map(option => option.long); candidateFlags = candidateFlags.concat(moreFlags); - command = command.parent; + command = command.currentParent; } while (command && !command._enablePositionalOptions); suggestion = suggestSimilar(flag, candidateFlags); } @@ -1795,7 +1811,7 @@ Expecting one of '${allowedValues.join("', '")}'`); const expected = this._args.length; const s = (expected === 1) ? '' : 's'; - const forSubcommand = this.parent ? ` for '${this.name()}'` : ''; + const forSubcommand = this.currentParent ? ` for '${this.name()}'` : ''; const message = `error: too many arguments${forSubcommand}. Expected ${expected} argument${s} but got ${receivedArgs.length}.`; this.error(message, { code: 'commander.excessArguments' }); } @@ -2212,7 +2228,7 @@ function incrementNodeInspectorPort(args) { function getCommandAndParents(startCommand) { const result = []; - for (let command = startCommand; command; command = command.parent) { + for (let command = startCommand; command; command = command.currentParent) { result.push(command); } return result; diff --git a/lib/help.js b/lib/help.js index 14e0fb9f3..67c22c458 100644 --- a/lib/help.js +++ b/lib/help.js @@ -101,7 +101,7 @@ class Help { if (!this.showGlobalOptions) return []; const globalOptions = []; - for (let parentCmd = cmd.parent; parentCmd; parentCmd = parentCmd.parent) { + for (let parentCmd = cmd.currentParent; parentCmd; parentCmd = parentCmd.currentParent) { const visibleOptions = parentCmd.options.filter((option) => !option.hidden); globalOptions.push(...visibleOptions); } @@ -241,7 +241,7 @@ class Help { cmdName = cmdName + '|' + cmd._aliases[0]; } let parentCmdNames = ''; - for (let parentCmd = cmd.parent; parentCmd; parentCmd = parentCmd.parent) { + for (let parentCmd = cmd.currentParent; parentCmd; parentCmd = parentCmd.currentParent) { parentCmdNames = parentCmd.name() + ' ' + parentCmdNames; } return parentCmdNames + cmdName + ' ' + cmd.usage(); From 680930d0869d5b9fd17213449bfe68c7f857449e Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 04:44:09 +0300 Subject: [PATCH 09/18] Warn about parse calls on commands added with .command() --- lib/command.js | 11 +++++++++++ tests/command.addCommand.test.js | 1 + 2 files changed, 12 insertions(+) diff --git a/lib/command.js b/lib/command.js index ee31ec371..cdc9a5709 100644 --- 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`. @@ -21,6 +23,9 @@ class Command extends EventEmitter { constructor(name) { super(); + /** @type {boolean} */ + this._implicitlyCreated = false; + /** @type {Command[]} */ this.commands = []; /** @type {Option[]} */ @@ -155,6 +160,7 @@ class Command extends EventEmitter { const [, name, args] = nameAndArgs.match(/([^ ]+) *(.*)/); const cmd = this.createCommand(name); + cmd._implicitlyCreated = true; if (desc) { cmd.description(desc); cmd._executableHandler = true; @@ -922,6 +928,11 @@ Expecting one of '${allowedValues.join("', '")}'`); */ _parseSubroutine(async, userArgsCallback, argv, parseOptions) { + const methodName = async ? 'parseAsync' : 'parse'; + if (!PRODUCTION && this._implicitlyCreated) { + console.warn(`Called .${methodName}() on subcommand '${this._name}' added with .command(). +Call on top-level command instead`); + } const userArgs = this._prepareUserArgs(argv, parseOptions); return userArgsCallback(userArgs); } diff --git a/tests/command.addCommand.test.js b/tests/command.addCommand.test.js index f8f86b682..09e94c3c5 100644 --- a/tests/command.addCommand.test.js +++ b/tests/command.addCommand.test.js @@ -30,6 +30,7 @@ test('when commands added using .addCommand and .command then internals similar' expect(cmd2.parent).toBe(program2); for (const key of Object.keys(cmd1)) { + if (key === '_implicitlyCreated') continue; // expected to differ switch (typeof cmd1[key]) { case 'string': case 'boolean': From 5085ddca1d4acae2aa6eb5704cf5322acb0dbbb8 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 10:10:09 +0300 Subject: [PATCH 10/18] Get rid of redundant currentParent .currentParent is expected to never be different from .parent in existing code using the library as intended. --- lib/command.js | 30 ++++++++++++++---------------- lib/help.js | 4 ++-- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/command.js b/lib/command.js index cdc9a5709..35410283a 100644 --- a/lib/command.js +++ b/lib/command.js @@ -38,9 +38,7 @@ class Command extends EventEmitter { * * @type {Command | null} */ - this.currentParent = null; - /** @type {Command | null} */ - this.parent = null; // last added parent command (legacy) + this.parent = null; this._allowUnknownOption = false; this._allowExcessArguments = true; /** @type {Argument[]} */ @@ -171,7 +169,7 @@ class Command extends EventEmitter { if (args) cmd.arguments(args); this.commands.push(cmd); cmd.parents.push(this); - cmd.currentParent = cmd.parent = this; + cmd.parent = this; cmd.copyInheritedSettings(this); if (desc) return this; @@ -289,7 +287,7 @@ class Command extends EventEmitter { this.commands.push(cmd); cmd.parents.push(this); - cmd.currentParent = cmd.parent = this; + cmd.parent = this; cmd._checkForBrokenPassThrough(); return this; @@ -1306,8 +1304,8 @@ Call on top-level command instead`); * @api private */ - _parseCommand(operands, unknown, currentParent) { - this.currentParent = currentParent ?? null; + _parseCommand(operands, unknown, parent) { + this.parent = parent ?? null; const parsed = this.parseOptions(unknown); this._parseOptionsEnv(); // after cli, so parseArg not called on both cli and env @@ -1350,18 +1348,18 @@ Call on top-level command instead`); let actionResult; actionResult = this._chainOrCallHooks(actionResult, 'preAction'); actionResult = this._chainOrCall(actionResult, () => this._actionHandler(this.processedArgs)); - if (currentParent) { + if (parent) { actionResult = this._chainOrCall(actionResult, () => { - currentParent.emit(commandEvent, operands, unknown); // legacy + parent.emit(commandEvent, operands, unknown); // legacy }); } actionResult = this._chainOrCallHooks(actionResult, 'postAction'); return actionResult; } - if (currentParent?.listenerCount(commandEvent)) { + if (parent?.listenerCount(commandEvent)) { checkForUnknownOptions(); this._processArguments(); - currentParent.emit(commandEvent, operands, unknown); // legacy + parent.emit(commandEvent, operands, unknown); // legacy } else if (operands.length) { if (this._findCommand('*')) { // legacy default command return this._dispatchSubcommand('*', operands, unknown); @@ -1417,7 +1415,7 @@ Call on top-level command instead`); _checkForMissingMandatoryOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. - for (let cmd = this; cmd; cmd = cmd.currentParent) { + for (let cmd = this; cmd; cmd = cmd.parent) { cmd.options.forEach((anOption) => { if (anOption.mandatory && (cmd.getOptionValue(anOption.attributeName()) === undefined)) { cmd.missingMandatoryOptionValue(anOption); @@ -1464,7 +1462,7 @@ Call on top-level command instead`); */ _checkForConflictingOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. - for (let cmd = this; cmd; cmd = cmd.currentParent) { + for (let cmd = this; cmd; cmd = cmd.parent) { cmd._checkForConflictingLocalOptions(); } } @@ -1801,7 +1799,7 @@ Call on top-level command instead`); .filter(option => option.long) .map(option => option.long); candidateFlags = candidateFlags.concat(moreFlags); - command = command.currentParent; + command = command.parent; } while (command && !command._enablePositionalOptions); suggestion = suggestSimilar(flag, candidateFlags); } @@ -1822,7 +1820,7 @@ Call on top-level command instead`); const expected = this._args.length; const s = (expected === 1) ? '' : 's'; - const forSubcommand = this.currentParent ? ` for '${this.name()}'` : ''; + const forSubcommand = this.parent ? ` for '${this.name()}'` : ''; const message = `error: too many arguments${forSubcommand}. Expected ${expected} argument${s} but got ${receivedArgs.length}.`; this.error(message, { code: 'commander.excessArguments' }); } @@ -2239,7 +2237,7 @@ function incrementNodeInspectorPort(args) { function getCommandAndParents(startCommand) { const result = []; - for (let command = startCommand; command; command = command.currentParent) { + for (let command = startCommand; command; command = command.parent) { result.push(command); } return result; diff --git a/lib/help.js b/lib/help.js index 67c22c458..14e0fb9f3 100644 --- a/lib/help.js +++ b/lib/help.js @@ -101,7 +101,7 @@ class Help { if (!this.showGlobalOptions) return []; const globalOptions = []; - for (let parentCmd = cmd.currentParent; parentCmd; parentCmd = parentCmd.currentParent) { + for (let parentCmd = cmd.parent; parentCmd; parentCmd = parentCmd.parent) { const visibleOptions = parentCmd.options.filter((option) => !option.hidden); globalOptions.push(...visibleOptions); } @@ -241,7 +241,7 @@ class Help { cmdName = cmdName + '|' + cmd._aliases[0]; } let parentCmdNames = ''; - for (let parentCmd = cmd.currentParent; parentCmd; parentCmd = parentCmd.currentParent) { + for (let parentCmd = cmd.parent; parentCmd; parentCmd = parentCmd.parent) { parentCmdNames = parentCmd.name() + ' ' + parentCmdNames; } return parentCmdNames + cmdName + ' ' + cmd.usage(); From aa280afbf8b0fde08566ebd9c20cc85df2a62c6a Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 11:32:16 +0300 Subject: [PATCH 11/18] Introduce _getCommandAndAncestors() Replaces getCommandAndParents(): - turned into an instance method - used "ancestors" instead of "parents" because it is more specific and to avoid confusion with the recently introduced parents array --- lib/command.js | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/command.js b/lib/command.js index 35410283a..a0f3d1176 100644 --- a/lib/command.js +++ b/lib/command.js @@ -122,6 +122,19 @@ class Command extends EventEmitter { return this; } + /** + * @returns {Command[]} + * @api private + */ + + _getCommandAndAncestors() { + const result = []; + for (let command = this; command; command = command.parent) { + result.push(command); + } + return result; + } + /** * Define a command. * @@ -853,7 +866,7 @@ Expecting one of '${allowedValues.join("', '")}'`); getOptionValueSourceWithGlobals(key) { // global overwrites local, like optsWithGlobals let source; - getCommandAndParents(this).forEach((cmd) => { + this._getCommandAndAncestors().forEach((cmd) => { if (cmd.getOptionValueSource(key) !== undefined) { source = cmd.getOptionValueSource(key); } @@ -1256,7 +1269,7 @@ Call on top-level command instead`); _chainOrCallHooks(promise, event) { let result = promise; const hooks = []; - getCommandAndParents(this) + this._getCommandAndAncestors() .reverse() .filter(cmd => cmd._lifeCycleHooks[event] !== undefined) .forEach(hookedCommand => { @@ -1627,7 +1640,7 @@ Call on top-level command instead`); */ optsWithGlobals() { // globals overwrite locals - return getCommandAndParents(this).reduce( + return this._getCommandAndAncestors().reduce( (combinedOptions, cmd) => Object.assign(combinedOptions, cmd.opts()), {} ); @@ -2072,7 +2085,7 @@ Call on top-level command instead`); } const context = this._getHelpContext(contextOptions); - getCommandAndParents(this).reverse().forEach(command => command.emit('beforeAllHelp', context)); + this._getCommandAndAncestors().reverse().forEach(command => command.emit('beforeAllHelp', context)); this.emit('beforeHelp', context); let helpInformation = this.helpInformation(context); @@ -2086,7 +2099,7 @@ Call on top-level command instead`); this.emit(this._helpLongFlag); // deprecated this.emit('afterHelp', context); - getCommandAndParents(this).forEach(command => command.emit('afterAllHelp', context)); + this._getCommandAndAncestors().forEach(command => command.emit('afterAllHelp', context)); } /** @@ -2229,18 +2242,4 @@ function incrementNodeInspectorPort(args) { }); } -/** - * @param {Command} startCommand - * @returns {Command[]} - * @api private - */ - -function getCommandAndParents(startCommand) { - const result = []; - for (let command = startCommand; command; command = command.parent) { - result.push(command); - } - return result; -} - exports.Command = Command; From 777a4528a6160171740fc795b517c7264d736d26 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 11:54:44 +0300 Subject: [PATCH 12/18] Use _getCommandAndAncestors() consistently --- lib/command.js | 15 +++++++-------- lib/help.js | 16 ++++++++-------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/command.js b/lib/command.js index a0f3d1176..405da30f2 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1428,13 +1428,13 @@ Call on top-level command instead`); _checkForMissingMandatoryOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. - for (let cmd = this; cmd; cmd = cmd.parent) { + this._getCommandAndAncestors().forEach((cmd) => { cmd.options.forEach((anOption) => { if (anOption.mandatory && (cmd.getOptionValue(anOption.attributeName()) === undefined)) { cmd.missingMandatoryOptionValue(anOption); } }); - } + }); } /** @@ -1475,9 +1475,9 @@ Call on top-level command instead`); */ _checkForConflictingOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. - for (let cmd = this; cmd; cmd = cmd.parent) { + this._getCommandAndAncestors().forEach((cmd) => { cmd._checkForConflictingLocalOptions(); - } + }); } /** @@ -1806,14 +1806,13 @@ Call on top-level command instead`); if (flag.startsWith('--') && this._showSuggestionAfterError) { // Looping to pick up the global options too let candidateFlags = []; - let command = this; - do { + for (const command of this._getCommandAndAncestors()) { const moreFlags = command.createHelp().visibleOptions(command) .filter(option => option.long) .map(option => option.long); candidateFlags = candidateFlags.concat(moreFlags); - command = command.parent; - } while (command && !command._enablePositionalOptions); + if (command._enablePositionalOptions) break; + } suggestion = suggestSimilar(flag, candidateFlags); } diff --git a/lib/help.js b/lib/help.js index 14e0fb9f3..7f1c2c354 100644 --- a/lib/help.js +++ b/lib/help.js @@ -101,10 +101,10 @@ class Help { if (!this.showGlobalOptions) return []; const globalOptions = []; - for (let parentCmd = cmd.parent; parentCmd; parentCmd = parentCmd.parent) { - const visibleOptions = parentCmd.options.filter((option) => !option.hidden); + cmd._getCommandAndAncestors().slice(1).forEach((ancestorCmd) => { + const visibleOptions = ancestorCmd.options.filter((option) => !option.hidden); globalOptions.push(...visibleOptions); - } + }); if (this.sortOptions) { globalOptions.sort(this.compareOptions); } @@ -240,11 +240,11 @@ class Help { if (cmd._aliases[0]) { cmdName = cmdName + '|' + cmd._aliases[0]; } - let parentCmdNames = ''; - for (let parentCmd = cmd.parent; parentCmd; parentCmd = parentCmd.parent) { - parentCmdNames = parentCmd.name() + ' ' + parentCmdNames; - } - return parentCmdNames + cmdName + ' ' + cmd.usage(); + let ancestorCmdNames = ''; + cmd._getCommandAndAncestors().slice(1).forEach((ancestorCmd) => { + ancestorCmdNames = ancestorCmd.name() + ' ' + ancestorCmdNames; + }); + return ancestorCmdNames + cmdName + ' ' + cmd.usage(); } /** From 200552285c42a8dcecb19118be505ae8e8e7ff9d Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 11:32:16 +0300 Subject: [PATCH 13/18] Introduce _getCommandAndAncestors() Replaces getCommandAndParents(): - turned into an instance method - used "ancestors" instead of "parents" because it is more precise (cherry picked from commit aa280afbf8b0fde08566ebd9c20cc85df2a62c6a) --- lib/command.js | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/command.js b/lib/command.js index 590a271dd..dce158bcc 100644 --- a/lib/command.js +++ b/lib/command.js @@ -109,6 +109,19 @@ class Command extends EventEmitter { return this; } + /** + * @returns {Command[]} + * @api private + */ + + _getCommandAndAncestors() { + const result = []; + for (let command = this; command; command = command.parent) { + result.push(command); + } + return result; + } + /** * Define a command. * @@ -825,7 +838,7 @@ Expecting one of '${allowedValues.join("', '")}'`); getOptionValueSourceWithGlobals(key) { // global overwrites local, like optsWithGlobals let source; - getCommandAndParents(this).forEach((cmd) => { + this._getCommandAndAncestors().forEach((cmd) => { if (cmd.getOptionValueSource(key) !== undefined) { source = cmd.getOptionValueSource(key); } @@ -1208,7 +1221,7 @@ Expecting one of '${allowedValues.join("', '")}'`); _chainOrCallHooks(promise, event) { let result = promise; const hooks = []; - getCommandAndParents(this) + this._getCommandAndAncestors() .reverse() .filter(cmd => cmd._lifeCycleHooks[event] !== undefined) .forEach(hookedCommand => { @@ -1577,7 +1590,7 @@ Expecting one of '${allowedValues.join("', '")}'`); */ optsWithGlobals() { // globals overwrite locals - return getCommandAndParents(this).reduce( + return this._getCommandAndAncestors().reduce( (combinedOptions, cmd) => Object.assign(combinedOptions, cmd.opts()), {} ); @@ -2022,7 +2035,7 @@ Expecting one of '${allowedValues.join("', '")}'`); } const context = this._getHelpContext(contextOptions); - getCommandAndParents(this).reverse().forEach(command => command.emit('beforeAllHelp', context)); + this._getCommandAndAncestors().reverse().forEach(command => command.emit('beforeAllHelp', context)); this.emit('beforeHelp', context); let helpInformation = this.helpInformation(context); @@ -2036,7 +2049,7 @@ Expecting one of '${allowedValues.join("', '")}'`); this.emit(this._helpLongFlag); // deprecated this.emit('afterHelp', context); - getCommandAndParents(this).forEach(command => command.emit('afterAllHelp', context)); + this._getCommandAndAncestors().forEach(command => command.emit('afterAllHelp', context)); } /** @@ -2179,18 +2192,4 @@ function incrementNodeInspectorPort(args) { }); } -/** - * @param {Command} startCommand - * @returns {Command[]} - * @api private - */ - -function getCommandAndParents(startCommand) { - const result = []; - for (let command = startCommand; command; command = command.parent) { - result.push(command); - } - return result; -} - exports.Command = Command; From da5a4992f7948ea05407b9fa6bb99062bb5b2659 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 11:54:44 +0300 Subject: [PATCH 14/18] Use _getCommandAndAncestors() consistently (cherry picked from commit 777a4528a6160171740fc795b517c7264d736d26) --- lib/command.js | 15 +++++++-------- lib/help.js | 16 ++++++++-------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/command.js b/lib/command.js index dce158bcc..b1c5622dc 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1378,13 +1378,13 @@ Expecting one of '${allowedValues.join("', '")}'`); _checkForMissingMandatoryOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. - for (let cmd = this; cmd; cmd = cmd.parent) { + this._getCommandAndAncestors().forEach((cmd) => { cmd.options.forEach((anOption) => { if (anOption.mandatory && (cmd.getOptionValue(anOption.attributeName()) === undefined)) { cmd.missingMandatoryOptionValue(anOption); } }); - } + }); } /** @@ -1425,9 +1425,9 @@ Expecting one of '${allowedValues.join("', '")}'`); */ _checkForConflictingOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. - for (let cmd = this; cmd; cmd = cmd.parent) { + this._getCommandAndAncestors().forEach((cmd) => { cmd._checkForConflictingLocalOptions(); - } + }); } /** @@ -1756,14 +1756,13 @@ Expecting one of '${allowedValues.join("', '")}'`); if (flag.startsWith('--') && this._showSuggestionAfterError) { // Looping to pick up the global options too let candidateFlags = []; - let command = this; - do { + for (const command of this._getCommandAndAncestors()) { const moreFlags = command.createHelp().visibleOptions(command) .filter(option => option.long) .map(option => option.long); candidateFlags = candidateFlags.concat(moreFlags); - command = command.parent; - } while (command && !command._enablePositionalOptions); + if (command._enablePositionalOptions) break; + } suggestion = suggestSimilar(flag, candidateFlags); } diff --git a/lib/help.js b/lib/help.js index 14e0fb9f3..7f1c2c354 100644 --- a/lib/help.js +++ b/lib/help.js @@ -101,10 +101,10 @@ class Help { if (!this.showGlobalOptions) return []; const globalOptions = []; - for (let parentCmd = cmd.parent; parentCmd; parentCmd = parentCmd.parent) { - const visibleOptions = parentCmd.options.filter((option) => !option.hidden); + cmd._getCommandAndAncestors().slice(1).forEach((ancestorCmd) => { + const visibleOptions = ancestorCmd.options.filter((option) => !option.hidden); globalOptions.push(...visibleOptions); - } + }); if (this.sortOptions) { globalOptions.sort(this.compareOptions); } @@ -240,11 +240,11 @@ class Help { if (cmd._aliases[0]) { cmdName = cmdName + '|' + cmd._aliases[0]; } - let parentCmdNames = ''; - for (let parentCmd = cmd.parent; parentCmd; parentCmd = parentCmd.parent) { - parentCmdNames = parentCmd.name() + ' ' + parentCmdNames; - } - return parentCmdNames + cmdName + ' ' + cmd.usage(); + let ancestorCmdNames = ''; + cmd._getCommandAndAncestors().slice(1).forEach((ancestorCmd) => { + ancestorCmdNames = ancestorCmd.name() + ' ' + ancestorCmdNames; + }); + return ancestorCmdNames + cmdName + ' ' + cmd.usage(); } /** From 5828e160da136608bc46871d3a3e60fabefabb19 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 5 Aug 2023 12:51:48 +0300 Subject: [PATCH 15/18] Copy type for parents and JSDoc comment for parent to .d.ts file A corresponding type test for parents has been added. --- typings/index.d.ts | 5 +++++ typings/index.test-d.ts | 1 + 2 files changed, 6 insertions(+) diff --git a/typings/index.d.ts b/typings/index.d.ts index 695c3bd25..8faa66798 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -281,6 +281,11 @@ export class Command { processedArgs: any[]; readonly commands: readonly Command[]; readonly options: readonly Option[]; + parents: Command[]; + /** + * Last added parent command, + * or parent command in last parse call. + */ parent: Command | null; constructor(name?: string); diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 734036fad..d3639b530 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -30,6 +30,7 @@ expectType(program.args); expectType(program.processedArgs); expectType(program.commands); expectType(program.options); +expectType(program.parents); expectType(program.parent); // version From 8b11122d3965c42af19988933da01b3c93a8ec4c Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Thu, 10 Aug 2023 03:22:55 +0300 Subject: [PATCH 16/18] Use _getCommandAndAncestors() less aggressively Only call when all elements are to be iterated. Resort to manual iteration via .parent in other cases. --- lib/command.js | 7 ++++--- lib/help.js | 8 ++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/command.js b/lib/command.js index b1c5622dc..d0868fd7e 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1756,13 +1756,14 @@ Expecting one of '${allowedValues.join("', '")}'`); if (flag.startsWith('--') && this._showSuggestionAfterError) { // Looping to pick up the global options too let candidateFlags = []; - for (const command of this._getCommandAndAncestors()) { + let command = this; + do { const moreFlags = command.createHelp().visibleOptions(command) .filter(option => option.long) .map(option => option.long); candidateFlags = candidateFlags.concat(moreFlags); - if (command._enablePositionalOptions) break; - } + command = command.parent; + } while (command && !command._enablePositionalOptions); suggestion = suggestSimilar(flag, candidateFlags); } diff --git a/lib/help.js b/lib/help.js index 7f1c2c354..560801bd3 100644 --- a/lib/help.js +++ b/lib/help.js @@ -101,10 +101,10 @@ class Help { if (!this.showGlobalOptions) return []; const globalOptions = []; - cmd._getCommandAndAncestors().slice(1).forEach((ancestorCmd) => { + for (let ancestorCmd = cmd.parent; ancestorCmd; ancestorCmd = ancestorCmd.parent) { const visibleOptions = ancestorCmd.options.filter((option) => !option.hidden); globalOptions.push(...visibleOptions); - }); + } if (this.sortOptions) { globalOptions.sort(this.compareOptions); } @@ -241,9 +241,9 @@ class Help { cmdName = cmdName + '|' + cmd._aliases[0]; } let ancestorCmdNames = ''; - cmd._getCommandAndAncestors().slice(1).forEach((ancestorCmd) => { + for (let ancestorCmd = cmd.parent; ancestorCmd; ancestorCmd = ancestorCmd.parent) { ancestorCmdNames = ancestorCmd.name() + ' ' + ancestorCmdNames; - }); + } return ancestorCmdNames + cmdName + ' ' + cmd.usage(); } From f7fd9866c0cbbb8f6a43c750ddcdf29fb19ccf85 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Fri, 11 Aug 2023 20:51:26 +0200 Subject: [PATCH 17/18] Remove NODE_ENV check Motivation: https://github.com/tj/commander.js/pull/1955 --- lib/command.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/command.js b/lib/command.js index 405da30f2..43fcff330 100644 --- a/lib/command.js +++ b/lib/command.js @@ -12,8 +12,6 @@ const { suggestSimilar } = require('./suggestSimilar'); // @ts-check -const PRODUCTION = process.env.NODE_ENV === 'production'; - class Command extends EventEmitter { /** * Initialize a new `Command`. @@ -940,7 +938,7 @@ Expecting one of '${allowedValues.join("', '")}'`); _parseSubroutine(async, userArgsCallback, argv, parseOptions) { const methodName = async ? 'parseAsync' : 'parse'; - if (!PRODUCTION && this._implicitlyCreated) { + if (this._implicitlyCreated) { console.warn(`Called .${methodName}() on subcommand '${this._name}' added with .command(). Call on top-level command instead`); } From cea6e9dfcd1a738d25949a547d4b68b4253bdc22 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Sat, 12 Aug 2023 21:32:39 +0200 Subject: [PATCH 18/18] Reword warning about parse calls on commands added with .command() Made it less strict: the library user might know better. --- lib/command.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/command.js b/lib/command.js index 804442f71..a3c705050 100644 --- a/lib/command.js +++ b/lib/command.js @@ -939,8 +939,8 @@ Expecting one of '${allowedValues.join("', '")}'`); _parseSubroutine(async, userArgsCallback, argv, parseOptions) { const methodName = async ? 'parseAsync' : 'parse'; if (this._implicitlyCreated) { - console.warn(`Called .${methodName}() on subcommand '${this._name}' added with .command(). -Call on top-level command instead`); + console.warn(`Called .${methodName}() on subcommand '${this._name}' added with .command() +- meant to call on the top-level command?`); } const userArgs = this._prepareUserArgs(argv, parseOptions); return userArgsCallback(userArgs);