Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for sharing and stand-alone parsing of subcommands. Warn about parse calls on commands added with .command() #1938

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4f5ab2d
Add missing checks for passThroughOptions
aweebit Aug 4, 2023
0ac6609
Add tests for illegal passThroughOptions
aweebit Aug 4, 2023
9d4c96a
Remove illegal passThroughOptions check deemed unnecessary
aweebit Aug 5, 2023
43d9faa
Use weaker wording: "broken" instead of "illegal"
aweebit Aug 5, 2023
43cc821
Unclutter error message in broken passThrough checks
aweebit Aug 5, 2023
7689506
Refactor _checkForBrokenPassThrough() to make it instance-aware
aweebit Aug 5, 2023
d1686db
Add subroutine for common parse call code
aweebit Aug 5, 2023
3de6161
Add support for sharing and stand-alone parsing of subcommands
aweebit Aug 5, 2023
680930d
Warn about parse calls on commands added with .command()
aweebit Aug 5, 2023
ea728ec
Merge branch 'release/12.x' into feature/parents-with-implicit-subcom…
aweebit Aug 5, 2023
5085ddc
Get rid of redundant currentParent
aweebit Aug 5, 2023
aa280af
Introduce _getCommandAndAncestors()
aweebit Aug 5, 2023
777a452
Use _getCommandAndAncestors() consistently
aweebit Aug 5, 2023
2005522
Introduce _getCommandAndAncestors()
aweebit Aug 5, 2023
da5a499
Use _getCommandAndAncestors() consistently
aweebit Aug 5, 2023
724d414
Merge branch 'feature/_getCommandAndAncestors' into feature/parents-w…
aweebit Aug 5, 2023
5828e16
Copy type for parents and JSDoc comment for parent to .d.ts file
aweebit Aug 5, 2023
8b11122
Use _getCommandAndAncestors() less aggressively
aweebit Aug 10, 2023
f7fd986
Remove NODE_ENV check
aweebit Aug 11, 2023
4d2415c
Merge branch 'feature/_getCommandAndAncestors' into feature/parents-w…
aweebit Aug 12, 2023
cea6e9d
Reword warning about parse calls on commands added with .command()
aweebit Aug 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Get rid of redundant currentParent
.currentParent is expected to never be different from .parent in
existing code using the library as intended.
  • Loading branch information
aweebit committed Aug 5, 2023
commit 5085ddca1d4acae2aa6eb5704cf5322acb0dbbb8
30 changes: 14 additions & 16 deletions lib/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -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[]} */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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' });
}
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/help.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
Expand Down