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

[Julien Fischer]

On Mon, 16 Jun 2014, Julien Fischer wrote:

>
>
> 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.)

On second thoughts, it's probably best to delete the comment in this
case.  We've already said that the predicates in this section are
identical to those in the previous one except that they return
structured errors instead of strings.

Cheers,
Julien.