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

Have help command call help directly for subcommands, when possible #1864

Merged
merged 2 commits into from Apr 21, 2023

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Apr 15, 2023

Pull Request

Problem

The implementation of the help command like my-util help foo is a bit fragile. The implementation invokes the subcommand passing this._helpLongFlag. This can fail if:

  • the current command does not have a long flag
  • the subcommand uses different help flags
  • the subcommand has the help option disabled

See: #1863 (help option disabled)

Solution

Call .help() directly for a known (non-external) subcommand. Just use the help flags as a fallback for external subcommands.

This takes a little extra code, but I have never been happy about passing through the help flags so I think it is worth it.

ChangeLog

  • Fixed: help command works when help option is disabled

@shadowspawn shadowspawn marked this pull request as ready for review April 16, 2023 00:29
if (!subcommandName) {
this.help();
}
const subCommand = this._findCommand(subcommandName);
Copy link
Collaborator

@abetomo abetomo Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If subcommandName is undefined, this._findCommand(subcommandName) also returns undefined.

How about the following code?

if (!subcommandName) {
  this.help();
} else {
  const subCommand = this._findCommand(subcommandName);
  ...
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The help method calls exit, so the following code will not be reached if subcommandName is undefined.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense.

Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@shadowspawn shadowspawn merged commit 63abdac into tj:develop Apr 21, 2023
11 checks passed
@shadowspawn shadowspawn deleted the feature/robust-help-command branch April 21, 2023 22:05
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Apr 21, 2023
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants