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

[Paul Bone]

On Sun, Jun 15, 2014 at 06:47:52PM +1000, Julien Fischer wrote:
> On Sun, 15 Jun 2014, Paul Bone wrote:
>
>> 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?
>
> It's not commented out code; it's the docuemntation for
> process_option_track_se.  (There's a similar thing for
> process_options_track above -- arguably the documentation should be
> better, but I don't intend to address that in this change.)
>
>>
>>> +:- 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.
>>> +

In that case I suggest placing it immediatly above the declration, and
indenting it:

    % getopt.process_options_track_se(OptionOps, Args, OptionArgs,
    %       NonOptionArgs, OptionTable0, Result, OptionsSet)
    %
:- 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.

Thanks.

-- 
Paul Bone