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

[Paul Bone]

On Mon, Jun 16, 2014 at 01:21:21AM +1000, Julien Fischer wrote:
>
>
> 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.)

That's fine.

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

That's also fine :-).

Thanks.


-- 
Paul Bone