[m-rev.] for review / comment: structured errors for getopt
Julien Fischer
jfischer at opturion.com
Sun Mar 30 01:26:11 AEDT 2014
On Wed, 26 Mar 2014, Julien Fischer wrote:
>>> @@ -255,12 +276,78 @@
>>> ; string(string)
>>> ; maybe_string(maybe(string)).
>>>
>>> -:- type option_table(OptionType) == map(OptionType, option_data).
>>> +:- type option_table(OptionType) == map(OptionType, option_data).
>>>
>>> :- type maybe_option_table(OptionType)
>>> ---> ok(option_table(OptionType))
>>> ; error(string).
>>>
>>> +:- type maybe_option_table_se(OptionType)
>>> + ---> ok(option_table(OptionType))
>>> + ; error(option_error(OptionType)).
>>> +
>>> +:- type option_error(OptionType)
>>> + ---> unrecognized(string)
>>> + % An option that is not recognized appeared on the command
>>> line.
>>> + % The argument gives the option as it appeared on the command
>>> line.
>>> +
>>> + % The first two arguments of the remaining constructors of this type
>>> + % identify the option enumeration value and the string that appeared
>>> on the
>>> + % command line for that option respectively.
>>> +
>>> + ; unknown_type(OptionType, string)
>>> + % No type has been specified for the option.
>>> +
>>> + ; requires_argument(OptionType, string)
>>> + % The option requires an argument but it occurred on the
>>> command
>>> + % line without one.
>> ...
>>
>> I suggest factoring out the first two arguments.
>>
>> :- type option_error(OptionType)
>> ---> unrecognized(string)
>> ; option_error(OptionType, string, option_error_reason).
>>
>> :- type option_error_reason
>> ---> unknown_type
>> ; requires_argument
>> ; does_not_allow_argument(string)
>> ; ...
Here is an updated diff.
Cheers,
Julien.
diff --git a/library/getopt_io.m b/library/getopt_io.m
index 84ea522..8bee7f0 100644
--- a/library/getopt_io.m
+++ b/library/getopt_io.m
@@ -153,6 +153,27 @@
option_table(OptionType)::in, maybe_option_table(OptionType)::out,
set(OptionType)::out, io::di, io::uo) is det.
+% Variants of the above that return structured errors.
+% These behave as the above versions except that any error values returned are
+% members of the option_error/1 type rather than strings.
+
+:- pred getopt_io.process_options_se(option_ops(OptionType)::in(option_ops),
+ list(string)::in, list(string)::out, maybe_option_table_se(OptionType)::out,
+ io::di, io::uo) is det.
+
+:- pred getopt_io.process_options_se(option_ops(OptionType)::in(option_ops),
+ list(string)::in, list(string)::out, list(string)::out,
+ maybe_option_table_se(OptionType)::out, io::di, io::uo) is det.
+
+% getopt_io.process_options_track(OptionOps, Args, OptionArgs,
+% NonOptionArgs, OptionTable0, Result, OptionsSet)
+
+:- pred getopt_io.process_options_track_se(
+ option_ops_track(OptionType)::in(option_ops_track),
+ list(string)::in, list(string)::out, list(string)::out,
+ option_table(OptionType)::in, maybe_option_table_se(OptionType)::out,
+ set(OptionType)::out, io::di, io::uo) is det.
+
:- pred init_option_table(
pred(OptionType, option_data)::in(pred(out, out) is nondet),
option_table(OptionType)::out) is det.
@@ -255,12 +276,81 @@
; string(string)
; maybe_string(maybe(string)).
-:- type option_table(OptionType) == map(OptionType, option_data).
+:- type option_table(OptionType) == map(OptionType, option_data).
:- type maybe_option_table(OptionType)
---> ok(option_table(OptionType))
; error(string).
+:- type maybe_option_table_se(OptionType)
+ ---> ok(option_table(OptionType))
+ ; error(option_error(OptionType)).
+
+:- type option_error(OptionType)
+ ---> unrecognized_option(string)
+ % An option that is not recognized appeared on the command line.
+ % The argument gives the option as it appeared on the command line.
+
+ ; option_error(OptionType, string, option_error_reason).
+ % An error occurred with a specific option. The first two arguments
+ % identify the option enumeration value and the string that appeared
+ % on the command line for that option respectively. The third
+ % argument describes the nature of the error with that option.
+
+:- type option_error_reason
+ ---> unknown_type
+ % No type has been specified for the option.
+
+ ; requires_argument
+ % The option requires an argument but it occurred on the command
+ % line without one.
+
+ ; does_not_allow_argument(string)
+ % The option does not allow an argument but it was provided with
+ % one on the command line.
+ % The argument gives the contents of the argument position
+ % on the command line.
+
+ ; cannot_negate
+ % The option cannot be negated but its negated form appeared on
+ % the command line.
+
+ ; special_handler_failed
+ % The special option handler predicate for the option failed.
+
+ ; special_handler_missing
+ % A special option handler predicate was not provided for the option.
+
+ ; special_handler_error(string)
+ % The special option handler predicate for the option returned an
+ % error.
+ % The argument is a string describing the error.
+
+ ; requires_numeric_argument(string)
+ % The option requires a numeric argument but it occurred on the
+ % command line with a non-numeric argument.
+ % The third argument gives the argument as it appeared on the
+ % command line.
+
+ ; file_special_cannot_open(string, io.error)
+ % The option is a file_special option whose argument is the file
+ % named by the first argument.
+ % Attempting to open this file resulted in the I/O error given
+ % by the second argument.
+
+ ; file_special_cannot_read(string, io.error)
+ % The option is a file_special option whose argument is the file
+ % named by the first argument.
+ % Attempting to read from this file resulted in the I/O error given
+ % by the second argument.
+
+ ; file_special_contains_non_option_args(string).
+ % The option is a file_special option whose argument is the file
+ % named by the argument. This file contained some non-option
+ % arguments.
+
+:- func getopt_io.option_error_to_string(option_error(OptionType)) = string.
+
% The following three predicates search the option table for
% an option of the specified type; if it is not found, they
% report an error by calling error/1.
@@ -357,11 +447,47 @@ init_option_table_multi(OptionDefaultsPred, OptionTable) :-
), OptionDefaultsList),
map.from_assoc_list(OptionDefaultsList, OptionTable).
-getopt_io.process_options(OptionOps, Args0, NonOptionArgs, Result, !IO) :-
- getopt_io.process_options(OptionOps, Args0, _OptionArgs, NonOptionArgs,
+process_options(OptionOps, Args0, NonOptionArgs, Result, !IO) :-
+ process_options_se(OptionOps, Args0, NonOptionArgs, Result0, !IO),
+ (
+ Result0 = ok(OptionTable),
+ Result = ok(OptionTable)
+ ;
+ Result0 = error(Error),
+ Msg = option_error_to_string(Error),
+ Result = error(Msg)
+ ).
+
+process_options(OptionOps, Args0, OptionArgs, NonOptionArgs, Result, !IO) :-
+ process_options_se(OptionOps, Args0, OptionArgs, NonOptionArgs, Result0,
+ !IO),
+ (
+ Result0 = ok(OptionTable),
+ Result = ok(OptionTable)
+ ;
+ Result0 = error(Error),
+ Msg = option_error_to_string(Error),
+ Result = error(Msg)
+ ).
+
+process_options_track(OptionOps, Args0, OptionArgs, NonOptionArgs,
+ OptionTable0, Result, OptionsSet, !IO) :-
+ process_options_track_se(OptionOps, Args0, OptionArgs, NonOptionArgs,
+ OptionTable0, Result0, OptionsSet, !IO),
+ (
+ Result0 = ok(OptionTable),
+ Result = ok(OptionTable)
+ ;
+ Result0 = error(Error),
+ Msg = option_error_to_string(Error),
+ Result = error(Msg)
+ ).
+
+getopt_io.process_options_se(OptionOps, Args0, NonOptionArgs, Result, !IO) :-
+ getopt_io.process_options_se(OptionOps, Args0, _OptionArgs, NonOptionArgs,
Result, !IO).
-getopt_io.process_options(OptionOps, Args0, OptionArgs, NonOptionArgs, Result,
+getopt_io.process_options_se(OptionOps, Args0, OptionArgs, NonOptionArgs, Result,
!IO) :-
(
OptionOps = option_ops(Short, Long, Defaults),
@@ -385,7 +511,7 @@ getopt_io.process_options(OptionOps, Args0, OptionArgs, NonOptionArgs, Result,
[], RevOptionArgs, OptionTable0, Result, set.init, _OptionsSet, !IO),
OptionArgs = list.reverse(RevOptionArgs).
-getopt_io.process_options_track(OptionOps, Args0, OptionArgs, NonOptionArgs,
+getopt_io.process_options_track_se(OptionOps, Args0, OptionArgs, NonOptionArgs,
OptionTable0, Result, OptionsSet, !IO) :-
OptionOps = option_ops_track(Short, Long, Special),
Internal = option_ops_internal(Short, Long, track(Special)),
@@ -396,7 +522,7 @@ getopt_io.process_options_track(OptionOps, Args0, OptionArgs, NonOptionArgs,
:- pred getopt_io.process_arguments(list(string)::in, list(string)::out,
option_ops_internal(OptionType)::in(option_ops_internal), list(string)::in,
list(string)::out, option_table(OptionType)::in,
- maybe_option_table(OptionType)::out,
+ maybe_option_table_se(OptionType)::out,
set(OptionType)::in, set(OptionType)::out, io::di, io::uo) is det.
getopt_io.process_arguments([], [], _, OptionArgs, OptionArgs,
@@ -425,8 +551,8 @@ getopt_io.process_arguments([Option | Args0], Args, OptionOps,
Args = Args0
)
;
- ErrorMsg = "unrecognized option `-" ++ Option ++ "'",
- Result = error(ErrorMsg),
+ Error = unrecognized_option(Option),
+ Result = error(Error),
OptionArgs = OptionArgs0,
Args = Args0
)
@@ -452,14 +578,14 @@ getopt_io.process_arguments([Option | Args0], Args, OptionOps,
[Option | OptionArgs0], OptionArgs,
OptionTable0, Result, !OptionsSet, !IO)
;
- ErrorMsg = "unknown type for option `" ++ Option ++ "'",
- Result = error(ErrorMsg),
+ Error = option_error(Flag, Option, unknown_type),
+ Result = error(Error),
OptionArgs = OptionArgs0,
Args = Args0
)
;
- ErrorMsg = "unrecognized option `" ++ OptionName ++ "'",
- Result = error(ErrorMsg),
+ Error = unrecognized_option(OptionName),
+ Result = error(Error),
OptionArgs = OptionArgs0,
Args = Args0
)
@@ -484,8 +610,8 @@ getopt_io.process_arguments([Option | Args0], Args, OptionOps,
Args = Args0
)
;
- ErrorMsg = "unrecognized option `-" ++ ShortOptions ++ "'",
- Result = error(ErrorMsg),
+ Error = unrecognized_option("-" ++ ShortOptions),
+ Result = error(Error),
OptionArgs = OptionArgs0,
Args = Args0
)
@@ -524,7 +650,7 @@ getopt_io.process_arguments([Option | Args0], Args, OptionOps,
maybe(string)::in, list(string)::in, list(string)::out,
option_ops_internal(OptionType)::in(option_ops_internal), list(string)::in,
list(string)::out, option_table(OptionType)::in,
- maybe_option_table(OptionType)::out,
+ maybe_option_table_se(OptionType)::out,
set(OptionType)::in, set(OptionType)::out, io::di, io::uo) is det.
getopt_io.handle_long_option(Option, Flag, OptionData, MaybeOptionArg0,
@@ -556,16 +682,16 @@ getopt_io.handle_long_option(Option, Flag, OptionData, MaybeOptionArg0,
( MissingArg = yes ->
Args = Args0,
OptionArgs = OptionArgs1,
- ErrorMsg = "option `" ++ Option ++ "' needs an argument",
- Result = error(ErrorMsg)
+ Error = option_error(Flag, Option, requires_argument),
+ Result = error(Error)
;
getopt_io.need_arg(OptionData, no),
- MaybeOptionArg = yes(_)
+ MaybeOptionArg = yes(ArgVal)
->
Args = Args0,
OptionArgs = OptionArgs1,
- ErrorMsg = "option `" ++ Option ++ "' does not allow an argument",
- Result = error(ErrorMsg)
+ Error = option_error(Flag, Option, does_not_allow_argument(ArgVal)),
+ Result = error(Error)
;
getopt_io.process_option(OptionData, Option, Flag, MaybeOptionArg,
OptionOps, OptionTable0, Result1, !OptionsSet, !IO),
@@ -586,7 +712,7 @@ getopt_io.handle_long_option(Option, Flag, OptionData, MaybeOptionArg0,
option_ops_internal(OptionType)::in(option_ops_internal), list(string)::in,
list(string)::out, list(string)::in, list(string)::out,
option_table(OptionType)::in,
- maybe_option_table(OptionType)::out,
+ maybe_option_table_se(OptionType)::out,
set(OptionType)::in, set(OptionType)::out, io::di, io::uo) is det.
getopt_io.handle_short_options([], _, Args, Args, OptionArgs, OptionArgs,
@@ -623,15 +749,15 @@ getopt_io.handle_short_options([Opt | Opts0], OptionOps, Args0, Args,
)
;
string.char_to_string(Opt, OptString),
- ErrorMsg = "unknown type for option `-" ++ OptString ++ "'",
- Result = error(ErrorMsg),
+ Error = option_error(Flag, "-" ++ OptString, unknown_type),
+ Result = error(Error),
OptionArgs = OptionArgs0,
Args = Args0
)
;
string.char_to_string(Opt, OptString),
- ErrorMsg = "unrecognized option `-" ++ OptString ++ "'",
- Result = error(ErrorMsg),
+ Error = unrecognized_option("-" ++ OptString),
+ Result = error(Error),
OptionArgs = OptionArgs0,
Args = Args0
).
@@ -658,7 +784,7 @@ getopt_io.get_short_option_arg(Opts, Arg, Args0, Args,
:- pred getopt_io.process_option(option_data::in, string::in, OptionType::in,
maybe(string)::in, option_ops_internal(OptionType)::in(option_ops_internal),
option_table(OptionType)::in,
- maybe_option_table(OptionType)::out,
+ maybe_option_table_se(OptionType)::out,
set(OptionType)::in, set(OptionType)::out, io::di, io::uo) is det.
getopt_io.process_option(bool(_), _Option, Flag, MaybeArg, _OptionOps,
@@ -682,7 +808,7 @@ getopt_io.process_option(int(_), Option, Flag, MaybeArg, _OptionOps,
map.set(Flag, int(IntArg), !OptionTable),
Result = ok(!.OptionTable)
;
- getopt_io.numeric_argument(Option, Arg, Result)
+ getopt_io.numeric_argument(Flag, Option, Arg, Result)
)
;
MaybeArg = no,
@@ -708,7 +834,7 @@ getopt_io.process_option(maybe_int(_), Option, Flag, MaybeArg, _OptionOps,
map.set(Flag, maybe_int(yes(IntArg)), !OptionTable),
Result = ok(!.OptionTable)
;
- getopt_io.numeric_argument(Option, Arg, Result)
+ getopt_io.numeric_argument(Flag, Option, Arg, Result)
)
;
MaybeArg = no,
@@ -769,7 +895,7 @@ getopt_io.process_option(int_special, Option, Flag, MaybeArg, OptionOps,
getopt_io.process_special(Option, Flag, int(IntArg),
OptionOps, OptionTable0, Result, !OptionsSet)
;
- getopt_io.numeric_argument(Option, Arg, Result)
+ getopt_io.numeric_argument(Flag, Option, Arg, Result)
)
;
MaybeArg = no,
@@ -797,7 +923,7 @@ getopt_io.process_option(maybe_string_special, Option, Flag, MaybeArg,
error("maybe_string_special argument expected " ++
"in getopt_io.process_option")
).
-getopt_io.process_option(file_special, _Option, _Flag, MaybeArg, OptionOps,
+getopt_io.process_option(file_special, Option, Flag, MaybeArg, OptionOps,
OptionTable0, Result, !OptionsSet, !IO) :-
(
MaybeArg = yes(FileName),
@@ -815,19 +941,22 @@ getopt_io.process_option(file_special, _Option, _Flag, MaybeArg, OptionOps,
Result = Result0
;
Args = [_ | _],
- Result = error(FileName ++
- " contains non-option arguments")
+ Reason = file_special_contains_non_option_args(FileName),
+ Error = option_error(Flag, Option, Reason),
+ Result = error(Error)
)
;
- ReadRes = error(_, Error),
- io.error_message(Error, Msg),
- Result = error("cannot read " ++ FileName ++ ": " ++ Msg)
+ ReadRes = error(_, IO_Error),
+ Reason = file_special_cannot_read(FileName, IO_Error),
+ Error = option_error(Flag, Option, Reason),
+ Result = error(Error)
),
io.seen(!IO)
;
- SeeRes = error(Error),
- io.error_message(Error, Msg),
- Result = error("cannot open " ++ FileName ++ ": " ++ Msg)
+ SeeRes = error(IO_Error),
+ Reason = file_special_cannot_open(FileName, IO_Error),
+ Error = option_error(Flag, Option, Reason),
+ Result = error(Error)
)
;
MaybeArg = no,
@@ -836,7 +965,7 @@ getopt_io.process_option(file_special, _Option, _Flag, MaybeArg, OptionOps,
:- pred process_negated_option(string::in, OptionType::in,
option_ops_internal(OptionType)::in(option_ops_internal),
- option_table(OptionType)::in, maybe_option_table(OptionType)::out,
+ option_table(OptionType)::in, maybe_option_table_se(OptionType)::out,
set(OptionType)::in, set(OptionType)::out) is det.
process_negated_option(Option, Flag, OptionOps, OptionTable0, Result,
@@ -873,45 +1002,24 @@ process_negated_option(Option, Flag, OptionOps, OptionTable0, Result,
getopt_io.process_special(Option, Flag, maybe_string(no),
OptionOps, OptionTable0, Result, !OptionsSet)
;
- OptionData = int_special,
- ErrorMsg = "cannot negate option `" ++ Option ++ "' --" ++
- "only boolean, maybe and accumulating options can be negated",
- Result = error(ErrorMsg)
- ;
- OptionData = string_special,
- ErrorMsg = "cannot negate option `" ++ Option ++ "' --" ++
- "only boolean, maybe and accumulating options can be negated",
- Result = error(ErrorMsg)
- ;
- OptionData = int(_),
- ErrorMsg = "cannot negate option `" ++ Option ++ "' --" ++
- "only boolean, maybe and accumulating options can be negated",
- Result = error(ErrorMsg)
- ;
- OptionData = string(_),
- ErrorMsg = "cannot negate option `" ++ Option ++ "' --" ++
- "only boolean, maybe and accumulating options can be negated",
- Result = error(ErrorMsg)
- ;
- OptionData = special,
- ErrorMsg = "cannot negate option `" ++ Option ++ "' --" ++
- "only boolean, maybe and accumulating options can be negated",
- Result = error(ErrorMsg)
- ;
- OptionData = file_special,
- ErrorMsg = "cannot negate option `" ++ Option ++ "' --" ++
- "only boolean, maybe and accumulating options can be negated",
- Result = error(ErrorMsg)
+ ( OptionData = int_special
+ ; OptionData = string_special
+ ; OptionData = int(_)
+ ; OptionData = string(_)
+ ; OptionData = special
+ ; OptionData = file_special
+ ),
+ Error = option_error(Flag, Option, cannot_negate),
+ Result = error(Error)
)
;
- string.append_list(["unknown type for option `", Option, "'"],
- ErrorMsg),
- Result = error(ErrorMsg)
+ Error = option_error(Flag, Option, unknown_type),
+ Result = error(Error)
).
:- pred getopt_io.process_special(string::in, OptionType::in, special_data::in,
option_ops_internal(OptionType)::in(option_ops_internal),
- option_table(OptionType)::in, maybe_option_table(OptionType)::out,
+ option_table(OptionType)::in, maybe_option_table_se(OptionType)::out,
set(OptionType)::in, set(OptionType)::out) is det.
getopt_io.process_special(Option, Flag, OptionData, OptionOps,
@@ -922,11 +1030,18 @@ getopt_io.process_special(Option, Flag, OptionData, OptionOps,
(
Handler(Flag, OptionData, OptionTable0, Result0)
->
- Result = Result0
+ (
+ Result0 = ok(OptionTable),
+ Result = ok(OptionTable)
+ ;
+ Result0 = error(HandlerMsg),
+ Reason = special_handler_error(HandlerMsg),
+ Error = option_error(Flag, Option, Reason),
+ Result = error(Error)
+ )
;
- string.append_list(["the handler of option `",
- Option, "' failed"], ErrorMsg),
- Result = error(ErrorMsg)
+ Error = option_error(Flag, Option, special_handler_failed),
+ Result = error(Error)
)
;
MaybeHandler = track(TrackHandler),
@@ -935,16 +1050,23 @@ getopt_io.process_special(Option, Flag, OptionData, OptionOps,
NewOptionsSet)
->
set.union(NewOptionsSet, !OptionsSet),
- Result = Result0
+ (
+ Result0 = ok(OptionTable),
+ Result = ok(OptionTable)
+ ;
+ Result0 = error(TrackHandlerMsg),
+ Reason = special_handler_error(TrackHandlerMsg),
+ Error = option_error(Flag, Option, Reason),
+ Result = error(Error)
+ )
;
- string.append_list(["the handler of option `",
- Option, "' failed"], ErrorMsg),
- Result = error(ErrorMsg)
+ Error = option_error(Flag, Option, special_handler_failed),
+ Result = error(Error)
)
;
MaybeHandler = none,
- ErrorMsg = "option `" ++ Option ++ "' has no handler",
- Result = error(ErrorMsg)
+ Error = option_error(Flag, Option, special_handler_missing),
+ Result = error(Error)
).
%-----------------------------------------------------------------------------%
@@ -964,13 +1086,68 @@ getopt_io.need_arg(string_special, yes).
getopt_io.need_arg(maybe_string_special, yes).
getopt_io.need_arg(file_special, yes).
-:- pred getopt_io.numeric_argument(string::in, string::in,
- maybe_option_table(OptionType)::out) is det.
+:- pred getopt_io.numeric_argument(OptionType::in, string::in, string::in,
+ maybe_option_table_se(OptionType)::out) is det.
+
+getopt_io.numeric_argument(Flag, Option, Arg, Result) :-
+ Reason = requires_numeric_argument(Arg),
+ Error = option_error(Flag, Option, Reason),
+ Result = error(Error).
+
+%-----------------------------------------------------------------------------%
-getopt_io.numeric_argument(Option, Arg, Result) :-
- ErrorMsg = "option `" ++ Option ++
- "' requires a numeric argument; `" ++ Arg ++ "' is not numeric",
- Result = error(ErrorMsg).
+option_error_to_string(Error) = String :-
+ (
+ Error = unrecognized_option(OptionName),
+ string.format("unrecognized option `%s'", [s(OptionName)], String)
+ ;
+ Error = option_error(_, OptionName, Reason),
+ (
+ Reason = unknown_type,
+ string.format("unknown type for option `%s'",
+ [s(OptionName)], String)
+ ;
+ Reason = requires_argument,
+ string.format("option `%s' needs an argument",
+ [s(OptionName)], String)
+ ;
+ Reason = does_not_allow_argument(_),
+ string.format("option `%s' does not allow an argument",
+ [s(OptionName)], String)
+ ;
+ Reason = cannot_negate,
+ string.format("cannot negate option `%s' -- " ++
+ "only boolean, maybe and accumulating options can can be negated",
+ [s(OptionName)], String)
+ ;
+ Reason = special_handler_failed,
+ string.format("the handler of option `%s' failed",
+ [s(OptionName)], String)
+ ;
+ Reason = special_handler_missing,
+ string.format("option `%s' has no handler",
+ [s(OptionName)], String)
+ ;
+ Reason = special_handler_error(String)
+ ;
+ Reason = requires_numeric_argument(Arg),
+ string.format(
+ "option `%s' requires a numeric argument; `%s' is not numeric",
+ [s(OptionName), s(Arg)], String)
+ ;
+ Reason = file_special_cannot_open(FileName, IO_Error),
+ io.error_message(IO_Error, Msg),
+ string.format("cannot open %s: %s", [s(FileName), s(Msg)], String)
+ ;
+ Reason = file_special_cannot_read(FileName, IO_Error),
+ io.error_message(IO_Error, Msg),
+ string.format("cannot read %s: %s", [s(FileName), s(Msg)], String)
+ ;
+ Reason = file_special_contains_non_option_args(FileName),
+ string.format("%s contains non-option arguments",
+ [s(FileName)], String)
+ )
+ ).
%-----------------------------------------------------------------------------%
More information about the reviews
mailing list