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

Standardize "requires a subcommand" error message #299

Merged
merged 2 commits into from
Jun 26, 2020
Merged

Standardize "requires a subcommand" error message #299

merged 2 commits into from
Jun 26, 2020

Conversation

JacobEvelyn
Copy link
Contributor

Closes #298

raise BadCommandLine,"Command '#{name}' requires a subcommand"
end
elsif have_subcommands?
if have_subcommands?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure that reordering these conditionals is safe but please double-check that!

Copy link
Owner

Choose a reason for hiding this comment

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

It seems to be breaking this test:

https://github.com/davetron5000/gli/blob/main/test/tc_subcommands.rb#L149

(sorry about the weird test framework—Given/When/Then don't do anything so the test is vanilla Test::Unit really)

That test is wanting to see that if the command has a subcommand, but extra args were given, the message is "Unknown command". I guess that error check has precedence, so I think this if statement does need to be kinda (annoyingly) complex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah whoops! I think I can just reorder the if and elsif parts of the conditional—let me know if that seems right now.

(I'm trying out GitHub's "make a code change and PR right from the browser" feature but a downside is I can't run tests. Let me know if this process is too annoying for you.)

@davetron5000
Copy link
Owner

This version passes the tests. I don't feel strongly about having a failing test that this PR fixes since it's mostly about wording and the code gets executed by the tests. Thanks!

@davetron5000 davetron5000 merged commit abbe8fb into davetron5000:main Jun 26, 2020
@davetron5000
Copy link
Owner

released as 2.19.2

@JacobEvelyn
Copy link
Contributor Author

Thank you!

@JacobEvelyn JacobEvelyn deleted the patch-1 branch June 26, 2020 13:02
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.

"requires a subcommand" error without examples
2 participants