Closed cameel closed 2 years ago
Hi @cameel. I would like to work on this issue.
@Midhun07 This one is actually taken. @spiyer99 is working in it and has already submitted a PR (still work in progress though). I tried to assign him earlier to make this apparent but github won't let me - I think I can only do that if someone has commented in the issue.
This is a continuation of the CLI refactor from #11518. Getting rid of
exit()
was suggested in the review.Currently
CommandLineParser
uses the mechanism inherited fromCommandLineInterface
to report errors - it prints a message to the error stream and returnsfalse
.exit()
which terminates the application.serr()
, returning). Throwing exceptions would be more concise.Here's my proposal on how to refactor this:
1) Modify
CommandLineInterface
so that the return value from the parser only indicates whether it should continue processing (true
) or immediately exit without error (false
).CommandLineParser
to make them not useexit()
.CommandLineInterface
. The parser should only signal that printing was requested on the command line, not perform the action. This would be more in line with the purpose of the class and would let us get rid of thesout()
stream form it. 2) Define an exception type that represents a command line validation error.serr()
and returnsfalse
with throwing that exception.BOOST_THROW_EXCEPTION
macro.serr()
stream from the parser.CommandLineInterface
and handle it by printing the message to stderr and exiting with an error code.boost::program_options::error
fromCommandLineParser::parse()
toCommandLineInterface
too. They're the same kind of error and should really be handled the same way.