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: Show usage message on invalid cli flags. #3282

Merged
merged 1 commit into from
May 16, 2024

Conversation

jholdstock
Copy link
Member

This fixes a bug where the error handling code was not matching the intended behaviour described in its comment, and as a result the usage message was not being displayed when an invalid config flag was passed. dcrd was also exiting with code 0 instead of 1.

After fixing this bug the usage message was shown as expected and the exit code was correct, but the config error was being printed twice - once by dcrd itself, and once by the go-flags lib. This was fixed by changing the options passed into go-flags.

# Before
$ ./dcrd --fake-flag 
unknown flag `fake-flag'
exit status 0


# After
$ ./dcrd --fake-flag 
unknown flag `fake-flag'
Use dcrd -h to show usage
exit status 1

This fixes a bug where the error handling code was not matching the
intended behaviour described in its comment, and as a result the usage
message was not being displayed when an invalid config flag was passed.
dcrd was also exiting with code 0 instead of 1.

After fixing this bug the usage message was shown as expected and the
exit code was correct, but the config error was being printed twice -
once by dcrd itself, and once by the go-flags lib. This was
fixed by changing the options passed into go-flags.
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.

Nice find. I tested a bunch of combinations and this works as intended.

@davecgh davecgh added this to the 2.0.0 milestone May 16, 2024
@davecgh davecgh merged commit 318efaa into decred:master May 16, 2024
2 checks passed
@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
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