[m-rev.] for review: changes to option handling

[Peter Wang]

On Thu, 17 Sep 2020 07:15:01 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> This email has two purposes. The first is to seek a review from anyone
> of the attached diff and log for a change to getopt_io.m, including
> specifically about an XXX in the description of the new exported predicates.
> (The diff is with -b.)
> 
> The second is to seek opinions from everyone about the proposed
> next steps, one of which is also the motivation for the change to getopt_io.m.
> These are described in the attached file OPTIONS.
> 
> Zoltan.

> Let state be threaded through special option handlers.
> 
> library/getopt_io.m:
>     Add process_options_cookie/process_options_cookie_se, which
>     allow callers to specify special option handlers that have
>     state threaded through them.

I suggest "userdata" instead of cookie. Cookie tends to have the
connotation of a (short) opaque token that is given to you by the server
or API, which you must pass back unmodified for whatever purpose.
Whereas "userdata" describes a piece of data that is opaque to the API.

> 
>     Generalize the internal infrastructure of getopt_io.m to support this.
> 
>     Replace a multi-clause predicate with a single big disjunction
>     to make changes to the argument list easier.
> 
>     Put related arguments next to each other. Give some variables
>     more appropriate names. Use state variables where relevant.

> @@ -395,8 +421,28 @@
>  :- pred process_options_track(
>      option_ops_track(OptionType)::in(option_ops_track),
>      list(string)::in, list(string)::out, list(string)::out,
> -    option_table(OptionType)::in,
> -    maybe_option_table(OptionType)::out, set(OptionType)::out,
> +    option_table(OptionType)::in, maybe_option_table(OptionType)::out,
> +    set(OptionType)::out, io::di, io::uo) is det.
> +
> +    % process_options_cookie(OptionOps, Args, OptionArgs, NonOptionArgs,
> +    %   OptionTable0, Result, OptionsSet, !Cookie, !IO):
> +    %
> +    % This predicate is similar to process_options_track, but it also
> +    % threads a piece of state of a user-specified "cookie" type through
> +    % all the handlers of special options, so that each special handler
> +    % can read from and/or write to this state. Amongst other things,
> +    % this can be used by the caller to recover the *sequence* in which
> +    % special options are specified, information that is not present
> +    % in the (orderless) set of specified options.
> +    %
> +    % XXX Should we return the final value of CookieType
> +    % if the value we return for Result is error(...)?

I say yes.

I assume the rest of the changes are trivial so didn't look deeply.

> To keep getopt.m and getopt_io.m in sync, I propose that both
> should be generated automatically from a common source, named
> something like "getopt_template", which would look like this:
> 
> GETOPT_ONLY_START
> ... documentation or code specific to getopt
> GETOPT_ONLY_END
> GETOPT_IO_ONLY_START
> ... documentation or code specific to getopt_io
> GETOPT_IO_ONLY_END
> 
> A shell script using sed would generate both files by either
> including or not including text blocks like this, and also
> deleting ", io::di, io::uo" and ", !IO)" from the getopt.m versions
> of predicate declarations, clause heads and calls.
> 
> We could include this dependency in library/Mmakefile, or simply
> let developers run the script manually when modifications are needed.
> Any preferences, or objections to the whole scheme?

We already generate a few files in runtime/ with shell scripts and don't
check them in, so perhaps we should do that for the getopt modules.

> 
> ------------------------------------------------------------
> 
> The point of this change is to serve as the basis for making
> -O<n> options additive, as per Mantis #495.

Great!

> 
> The proposed scheme is as follows.
> 

That sounds fine.

Peter