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

Julien Fischer jfischer at opturion.com
Mon Jun 16 00:57:54 AEST 2014



On Mon, 16 Jun 2014, Paul Bone wrote:

> 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:

That would be inconsistent with the way the rest of the interfaces of
these modules is documented.  (Since this is inconsistent with most of
the rest of the standard library it should be changed, but not as part
of this change.  I'll look into it separately.)

Cheers,
Julien.



More information about the reviews mailing list