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

added test and code #804

Closed
wants to merge 2 commits into from
Closed

added test and code #804

wants to merge 2 commits into from

Conversation

iluxame
Copy link

@iluxame iluxame commented May 30, 2017

I've rebased #502
@jdell64 Thanks for the feature! We need this feature badly so I've rebased your PR. No worries, you are still the author of your code.
Feel free to rebase your code and submit it in the original PR.
In such case we will not need this PR

@iluxame
Copy link
Author

iluxame commented May 30, 2017

@untitaker can you please take a look?

@untitaker
Copy link
Contributor

@iluxame I have proposed a few alternatives in that thread which I consider better since they add no extra API, but haven't recieved any feedback.

@iluxame
Copy link
Author

iluxame commented May 30, 2017

@untitaker Aha, sorry, somehow I missed it.
In my opinion its better to show help message with empty command.
in case of a sentence like "See command subcommand --help" user will need 2 commands instead of one. However from other point of view it will be clear to user that he didn't entered any meaningful input.

This patch actually aligns command user experience with group user experience. It seems that similar experience is less confusing. So I would prefer @jdell64 approach.

@iluxame
Copy link
Author

iluxame commented Jun 6, 2017

@untitaker
What do you think?

@davidism
Copy link
Member

Decision made to keep current "see --help" message in #502.

@davidism davidism closed this May 14, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants