[m-rev.] for review: structured errors for getopt and getopt_io

[Paul Bone]

On Tue, Jun 10, 2014 at 02:20:17PM +1000, Julien Fischer wrote:
> diff --git a/library/getopt.m b/library/getopt.m
> index 6287e3b..9fc647a 100644
> --- a/library/getopt.m
> +++ b/library/getopt.m
> @@ -148,6 +148,27 @@
>      option_table(OptionType)::in, maybe_option_table(OptionType)::out,
>      set(OptionType)::out) 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.process_options_se(option_ops(OptionType)::in(option_ops),
> +    list(string)::in, list(string)::out, maybe_option_table_se(OptionType)::out)
> +    is det.
> +
> +:- pred getopt.process_options_se(option_ops(OptionType)::in(option_ops),
> +    list(string)::in, list(string)::out, list(string)::out,
> +    maybe_option_table_se(OptionType)::out) is det.
> +
> +% getopt.process_options_track_se(OptionOps, Args, OptionArgs,
> +%       NonOptionArgs, OptionTable0, Result, OptionsSet)
> +

What's this commented out code for?

> +:- pred getopt.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) is det.
> +
>  :- pred init_option_table(
>      pred(OptionType, option_data)::in(pred(out, out) is nondet),
>      option_table(OptionType)::out) is det.
> @@ -255,6 +276,59 @@
>      --->    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.
> +

These lines and others are too long.  (Yes the standard is currently 79 but
I'd prefer to make it 76 please).

> +:- type option_error_reason
> +    --->    unknown_type
> +            % No type for this option has been specified in the
> +            % `option_default'/2 predicate.
> +
> +    ;       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.
> +

third argument?

> diff --git a/library/getopt_io.m b/library/getopt_io.m
> index 84ea522..d4f54f7 100644
> --- a/library/getopt_io.m
> +++ b/library/getopt_io.m

Some (maybe all) of the problems above also occur in this file.

The implementation itself is fine, the only problems superficial.

Thanks.


-- 
Paul Bone