Closed rvesse closed 7 years ago
The following commit shows another variation on this approach:
https://github.com/jontodd/airline/commit/66d85bb4157bf44acf8fd1f144e920e3990402c8
My inclination is to go with hybrid of the various approaches. Basically define a new ErrorHandler
Interface and have the parsers route all errors through this via a ParseState.withError()
method. We can then have several Basic implementations provided based on the behaviours already described. Yet this would also allow end users to define entirely custom logic as desired. It also may mean that we don't need a @SuppressErrors
annotation or argument to options.
One thing to be careful of if we allow for suppressing Errors is that when we suppress an error we should make sure that we have consumed the token that led to the error so that we can continue parsing and don't get stuck in an infinite loop. It may be possible to add logic to detect this case into our error handlers e.g. If the handler received an error which is of an identical type to the preceding error and the message and cause trees are equivalent we bailout and throw an error regardless
Now implemented as follows:
ParserErrorHandler
interfaceFailFast
implementationFailAll
and CollectAll
implementations that collect errors and either throw or store themParseResult<T>
class and parseWithResult()
methods on SingleCommand
and Cli
HelpOption.showHelpIfErrors()
which takes in a ParseResult
and automatically shows help if any errors occurredSo showing help on errors is not automatic but is now much more easily done.
Currently if parsing fails for whatever reason we throw an exception so any logic that a developer might have written to provide help for the command is most likely ignored. Also because they won't have received an instance of their command class even if they had used something like the
HelpOption
and the user specified--help
at the command line they have no way of inspecting this value short of manually inspecting the command line inputs.In some of the existing places where I use airline currently I end up creating special logic to do just this when really it's something that the library should do automatically.
It would also be nice if there is a way to get all the parser Errors in one go and present these to the user.
This problem dates all the way back to the original library and one possible solution was suggested there but was never actually adopted. See https://github.com/airlift/airline/issues/22
Given all the changes that have happened since then that approach almost certainly won't directly apply onto the current code but the general approach should work. I would be tempted to actually generalise this further and have a
@SupressErrors
or similar annotation/arguments to annotations that could be used to indicate that the presence of an option should suppress throwing errors. Also given that we have a configurable parsing infrastructure we may wish to make the behaviour configurable:The best thing to do might be to define an enumeration of possible behaviours and provide that as a field on the
@Parser
annotation and then improve the parser to behave accordingly.