[m-rev.] for review / comment: structured errors for getopt

[Julien Fischer]

Hi Paul,

On Wed, 2 Apr 2014, Paul Bone wrote:

> On Tue, Mar 25, 2014 at 05:21:34PM +1100, Julien Fischer wrote:
>>
>> For review / feedback.
>>
>> I'm not intending to push this to github in its current form; in requesting a review
>> I am mainly seeking feedback on the following things:
>>
>> (1) the overall proposal.
>
> It's good, I'm in favor of using the type system rather than storing
> information like this in strings.
>
> Do you have plans for deprecating the original API and renaming the _se
> predicates to the normal ones?

In the *very* long term that is the plan, but that particular API has
been around for a very long time and a lot of software uses it.

>> (2) in particular, the names of the constructors of the option_error/1 type and
>>     their documentation.
>
> See below.
>
>>
>> diff --git a/library/getopt_io.m b/library/getopt_io.m
>> index 84ea522..69cac88 100644
>> --- a/library/getopt_io.m
>> +++ b/library/getopt_io.m
>> +
>> +:- type option_error(OptionType)
>> +    --->    unrecognized(string)
>> +            % An option that is not recognized appeared on the command line.
>> +            % The argument gives the option as it appeared on the command line.
>> +
>> +    % The first two arguments of the remaining constructors of this type
>> +    % identify the option enumeration value and the string that appeared on the
>> +    % command line for that option respectively.
>> +
>> +    ;       unknown_type(OptionType, string)
>> +            % No type has been specified for the option.
>> +
>
> I don't understand this.  My guess is that a type is whether an option's
> value is an int, string, bool etc, but this isn't absolutly clear.  I'm more
> confused by "specified", what is an example of how a type, at runtime, could
> fail to be specified?  Maybe a quick example should be in the comment here.

It means that no type has been specified for the option in the
"option_default" predicate that you need to provide to getopt -- I'll
try rewording it.

>> +    ;       requires_argument(OptionType, string)
>> +            % The option requires an argument but it occurred on the command
>> +            % line without one.
>> +
>> +    ;       does_not_allow_argument(OptionType, string, string)
>> +            % The option does not allow an argument but it was provided with
>> +            % one on the command line.
>> +            % The third argument gives the contents of the argument position
>> +            % on the command line.
>> +
>> +    ;       cannot_negate(OptionType, string)
>> +            % The option cannot be negated but its negated form appeared on
>> +            % the command line.
>> +
>> +    ;       special_handler_failed(OptionType, string)
>> +            % The special option handler predicate for the option failed.
>> +
>> +    ;       special_handler_missing(OptionType, string)
>> +            % A special option handler predicate was not provided for the option.
>> +
>> +    ;       special_handler_error(OptionType, string, string)
>> +            % The special option handler predicate for the option returned an
>> +            % error.
>> +            % The third argument is a string describing the error.
>> +
>> +    ;       requires_numeric_argument(OptionType, string, 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.
>> +
>> +    ;       file_special_cannot_open(OptionType, string, string, io.error)
>> +            % The option is a file_special option whose argument is the file
>> +            % named by the third argument.
>> +            % Attempting to open this file resulted in the I/O error given
>> +            % by the fourth argument.
>> +
>> +    ;       file_special_cannot_read(OptionType, string, string, io.error)
>> +            % The option is a file_special option whose argument is the file
>> +            % named by the third argument.
>> +            % Attempting to read from this file resulted in the I/O error given
>> +            % by the fourth argument.
>> +
>> +    ;       file_special_contains_non_option_args(OptionType, string, string).
>> +            % The option is a file_special option whose argument is the file
>> +            % named by the third argument.  This file contained some non-option
>> +            % arguments.
>
> This isn't a strong opinion, but it may be reasonable to fold these three
> constructors together, and then either in the 3rd argument or in a new
> argument specify at what stage reading the file failed.

I thought about that, but have look at the revised diff I posted the
other day addressing Peter Wang's comments.

Cheers,
Julien.