refactor: use stderr for CLI error output#3372
refactor: use stderr for CLI error output#3372sanjana2505006 wants to merge 1 commit intoapache:unstablefrom
Conversation
|
@git-hulk Whenever you get a chance, please review and let me know if any changes are needed |
| << new_opt << "--<config-key> <config-value>" | ||
| << "overwrite specific config option <config-key> to <config-value>" << std::endl; | ||
| static void PrintUsage(const char *program, std::ostream &os = std::cout) { | ||
| os << program << " implements the Redis protocol based on RocksDB" << std::endl |
There was a problem hiding this comment.
maybe we can just change it to std::cerr?
There was a problem hiding this comment.
I actually went with the std::ostream parameter because I wanted to stick to the standard way most CLI tools handle output. Usually, when someone asks for help explicitly (like with --help), they expect it on stdout so they can pipe it to grep or less. But if they just mess up a command, the usage warning should definitely go to stderr.
I also noticed that --version uses stdout right now, so this keeps things consistent.
Let me know if that makes sense! I'm happy to simplify it to just std::cerr if you'd prefer less boilerplate, but I figured this separation was worth the extra bit of code.
|
@PragmaTwice whenever you get a chance, please let me know your thoughts on this. If you still think we should just use |
I noticed that the CLI was printing error messages (like invalid arguments or config load failures) to
stdout, which isn't ideal for scripting or standard error handling. This PR redirects those critical errors tostderr.I also enhanced the PrintUsage function to accept an output stream, so
-h/--helpstill goes tostdout(as expected), but incorrect usage warnings go tostderr. The usage message itself has been slightly polished for better readability.Changes:
std::ostream&argument (defaults tocout).cerrand return exit code 1.cerr.<iostream>include for correctness.