Re: [PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()

[Stefan Beller <sbeller@google.com>]

On Tue, Jun 2, 2015 at 11:48 PM, Paul Tan <pyokagan@gmail.com> wrote:
> Certain git commands, such as git-pull, are simply wrappers around other
> git commands like git-fetch, git-merge and git-rebase. As such, these
> wrapper commands will typically need to "pass through" command-line
> options of the commands they wrap.
>
> Implement the parse_opt_pass_argv_array() parse-options callback, which
> will reconstruct all the provided command-line options into an
> argv_array, such that it can be passed to another git command. This is
> useful for passing command-line options that can be specified multiple
> times.
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>
> Notes:
>     v2
>
>     * This function is a requirement for the rewrite of git-am to handle
>       passing git-apply's options to git-apply. Since it would be
>       implemented anyway I thought it would be good if git-pull could take
>       advantage of it as well to handle --strategy and --strategy-option.
>
>  parse-options-cb.c | 32 ++++++++++++++++++++++++++++++++
>  parse-options.h    |  1 +
>  2 files changed, 33 insertions(+)
>
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> index 5b1dbcf..7330506 100644
> --- a/parse-options-cb.c
> +++ b/parse-options-cb.c
> @@ -4,6 +4,7 @@
>  #include "commit.h"
>  #include "color.h"
>  #include "string-list.h"
> +#include "argv-array.h"
>
>  /*----- some often used options -----*/
>
> @@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)
>
>         return 0;
>  }
> +
> +/**
> + * For an option opt, recreate the command-line option, appending it to
> + * opt->value which must be a argv_array. This is useful when we need to pass
> + * the command-line option, which can be specified multiple times, to another
> + * command.
> + */
> +int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int unset)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +       struct argv_array *opt_value = opt->value;
> +
> +       if (opt->long_name) {
> +               strbuf_addstr(&sb, unset ? "--no-" : "--");
> +               strbuf_addstr(&sb, opt->long_name);
> +               if (arg) {
> +                       strbuf_addch(&sb, '=');
> +                       strbuf_addstr(&sb, arg);
> +               }
> +       } else if (opt->short_name && !unset) {
> +               strbuf_addch(&sb, '-');
> +               strbuf_addch(&sb, opt->short_name);
> +               if (arg)
> +                       strbuf_addstr(&sb, arg);
> +       } else
> +               return -1;
> +
> +       argv_array_push(opt_value, sb.buf);
> +       strbuf_release(&sb);
> +       return 0;
> +}
> diff --git a/parse-options.h b/parse-options.h
> index 1d21398..b663f87 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *, const char *, int);
>  extern int parse_opt_string_list(const struct option *, const char *, int);
>  extern int parse_opt_noop_cb(const struct option *, const char *, int);
>  extern int parse_opt_pass_strbuf(const struct option *, const char *, int);
> +extern int parse_opt_pass_argv_array(const struct option *, const char *, int);
>
>  #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', "verbose", (var), (h))
>  #define OPT__QUIET(var, h)    OPT_COUNTUP('q', "quiet",   (var), (h))
> --
> 2.1.4
>

This patch looks very similar to the first one, so I wonder if the
combination of these
could be written as:

static int parse_opt_pass_strbuf_internal(const struct option *opt,
const char *arg, int unset, struct strbuf *sb)
{
       if (opt->long_name) {
               strbuf_addstr(sb, unset ? "--no-" : "--");
               strbuf_addstr(sb, opt->long_name);
               if (arg) {
                       strbuf_addch(sb, '=');
                       strbuf_addstr(sb, arg);
               }
       } else if (opt->short_name && !unset) {
               strbuf_addch(sb, '-');
               strbuf_addch(sb, opt->short_name);
               if (arg)
                       strbuf_addstr(sb, arg);
       } else
               return -1;
      return 0;
}

/**
 * For an option opt, recreates the command-line option in opt->value which
 * must be an strbuf. This is useful when we need to pass the command-line
 * option to another command.
 */
int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)
{
       struct strbuf *sb = opt->value;
       strbuf_reset(sb);
       return parse_opt_pass_strbuf_internal(...);
}

/**
 * For an option opt, recreate the command-line option, appending it to
 * opt->value which must be a argv_array. This is useful when we need to pass
 * the command-line option, which can be specified multiple times, to another
 * command.
 */
int parse_opt_pass_argv_array(const struct option *opt, const char
*arg, int unset)
{
       struct strbuf sb = STRBUF_INIT;
       struct argv_array *opt_value = opt->value;

       int ret = parse_opt_pass_strbuf_internal(...)

       argv_array_push(opt_value, sb.buf);
       strbuf_release(&sb);
       return 0;
}