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

Additional help topic commands now return "subcommand is required" error #1056

Closed
ob-stripe opened this issue Mar 10, 2020 · 3 comments
Closed

Comments

@ob-stripe
Copy link

#922 changed the behavior of all non-runnable commands, including additional help topic commands.

They now return the new ErrSubCommandRequired error, which by default results in subcommand is required being displayed and exiting with status 1. Both are incorrect for additional help topic commands, whose entire purpose is to display a help message.

Can we restore the previous behavior for additional help topic commands? Naively, something like this:

	if !c.Runnable() {
-		return ErrSubCommandRequired
+		if c.IsAdditionalHelpTopicCommand() {
+			return flag.ErrHelp
+		} else {
+			return ErrSubCommandRequired
+		}
	}

cc @bruceadowns

@bruceadowns
Copy link
Contributor

At first glance, I'm in agreement with refining the change to not consider an additional help topic commands an error.

It would help to see some examples and their expected behavior.

@ob-stripe
Copy link
Author

ob-stripe commented Mar 17, 2020

Hi @bruceadowns, thanks and sorry for the delayed reply.

A concrete example would be the resources command in the stripe CLI utility: https://github.com/stripe/stripe-cli/blob/master/pkg/cmd/resources.go

This command doesn't define a Run function or any subcommands (i.e. it's an "additional help topic" command). With cobra 0.0.5, the command behaves as expected:

$ go run cmd/stripe/main.go resources
Available commands:
  3d_secure
  ...
  webhook_endpoints

Use "stripe [command] --help" for more information about a command.

$ echo $?
0

After upgrading to cobra 0.0.6, a subcommand is required error is output and the process exits with status 1 instead:

$ go run cmd/stripe/main.go resources
 Available commands:
  3d_secure
  ...
  webhook_endpoints

Use "stripe [command] --help" for more information about a command.
subcommand is required
exit status 1

$ echo $?
1

Does that make sense? I can provide a minimal self-contained example project if you'd prefer.

If you agree that it makes sense, should I open a PR with the patch I proposed in the first comment, or would you have a more involved solution in mind?

@bruceadowns
Copy link
Contributor

That makes perfect sense, thanks for the clarification. I'd recommend opening a PR, using your proposed solution, with any relatable tests.

@ob-stripe ob-stripe mentioned this issue Mar 18, 2020
jharshman added a commit to jharshman/cobra that referenced this issue Mar 26, 2020
Issue Reference: spf13#1056

when a command was not runnable. This caused all commands w/o a run
function set to error w/ that message and a status code of 1.

This change reverts the addition of that new error. Similar
functionality can be accomplished by leveraging RunE.
jharshman added a commit to jharshman/cobra that referenced this issue Mar 26, 2020
Issue Reference: spf13#1056

spf13#922 introduced a new error
type that emitted when a command was not runnable. This caused
all commands w/o a run function set to error w/ that message and a status code of 1.

This change reverts the addition of that new error. Similar
functionality can be accomplished by leveraging RunE.
jharshman added a commit that referenced this issue Mar 27, 2020
Issue Reference: #1056

#922 introduced a new error
type that emitted when a command was not runnable. This caused
all commands w/o a run function set to error w/ that message and a status code of 1.

This change reverts the addition of that new error. Similar
functionality can be accomplished by leveraging RunE.
muscliary pushed a commit to muscliary/cobra that referenced this issue Sep 12, 2023
Issue Reference: spf13/cobra#1056

spf13/cobra#922 introduced a new error
type that emitted when a command was not runnable. This caused
all commands w/o a run function set to error w/ that message and a status code of 1.

This change reverts the addition of that new error. Similar
functionality can be accomplished by leveraging RunE.
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 a pull request may close this issue.

3 participants