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

multi: Explicitly handle help requests. #469

Merged
merged 1 commit into from
May 18, 2024

Conversation

jholdstock
Copy link
Member

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, which happened to be unnecessarily duplicated in three places.

Moving it to a function in the internal package makes it reusable by multiple binaries.

This 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, vspd --help --fakeflag vs vspd --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, which happened to be unnecessarily duplicated in three places.

Moving it to a function in the internal package makes it reusable by
multiple binaries.

This 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, `vspd --help --fakeflag` vs `vspd
--fakeflag --help`.
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.

This reads well. I'll circle back after I've tested the equivalent changes made in the dcrd PR.

@jholdstock jholdstock merged commit 2bd340b into decred:master May 18, 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.

2 participants