sharkdp / hexyl

A command-line hex viewer
Apache License 2.0
8.93k stars 227 forks source link

Use `get_matches_safe` instead of `get_matches` for consistent exit point from program #31

Closed arnavb closed 5 years ago

arnavb commented 5 years ago

get_matches currently displays an error to the user on failure and calls process::exit. This means that if some sort of error occurs with CLI parsing, then the end of the main function is not reached. While this isn't necessarily a problem here, multiple exit points make the code harder to reason about in general. I've replaced get_matches with get_matches_safe, which instead of exiting the program returns a ClapResult. The error is then propagated upwards with the ? operator, allowing for a consistent exit point on error from the program.

Possible problems with this:

Please let me know your thoughts on this and whether I should make any changes.

sharkdp commented 5 years ago

Thank you very much.

  • Calling --help and --version will currently always be an error (the user won't see an error, but there will be an exit code of 1). I can however check if the error is of kind ErrorKind::HelpDisplayed or ErrorKind::VersionDisplayed and in those cases exit with a return code of 0. If you feel this is necessary, I will gladly make the appropriate changes to this PR.

Yes, we should do this. If you could open another PR for this, that'd be great. Thank you!