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

config: Explicitly handle help requests. #3292

Merged
merged 1 commit into from
May 17, 2024
Merged

Conversation

jholdstock
Copy link
Member

@jholdstock jholdstock commented May 17, 2024

Checking for --help as an explicit step before parsing any other configs makes the code more intuitive by removing a convoluted bit of error handling.

It also enables the IgnoreUnknown option to be used whilst parsing for help, which ensures the presence of --help will always result in the help message being printed. This fixes a minor inconsistency where the help message would be printed if the flag was placed before an invalid config, but placing it after would cause an invalid config error to be written instead. For example, dcrd --help --fakeflag vs dcrd --fakeflag --help.

Checking for --help as an explicit step before parsing any other configs
makes the code more intuitive by removing a convoluted bit of error
handling.

It also enables the IgnoreUnknown option to be used whilst parsing for
help, which ensures the presence of --help will always result in the
help message being printed. This fixes a minor inconsistency where the
help message would be printed if the flag was placed before an invalid
config, but placing it after would cause an invalid config error to be
written instead. For example, `dcrd --help --fakeflag` vs `dcrd
--fakeflag --help`.
@davecgh davecgh added this to the 2.1.0 milestone May 17, 2024
@davecgh davecgh added cli flag change Issues and/or pull requests that involve a change to the available CLI flags. and removed cli flag change Issues and/or pull requests that involve a change to the available CLI flags. labels May 17, 2024
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Looks reasonable. I'll do some testing before approving.

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Tested it works as expected:

$ ./dcrd --fake -h
Usage:
...
exit status 0
$ ./-h dcrd --fake
Usage:
...
exit status 0
$ ./dcrd --fake
unknown flag `fake'
Use dcrd -h to show usage

@davecgh davecgh merged commit a645aca into decred:master May 17, 2024
2 checks passed
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