Hi Paul,
On 2015-05-18 17:05, Paul Tan wrote:
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 0b771b9..a4d9c92 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -11,16 +11,64 @@
> #include "argv-array.h"
> #include "run-command.h"
>
> +/**
> + * Given an option opt, where opt->value points to a char* and opt->defval is a
> + * string, sets opt->value to the evaluation of "--$defval=$arg". If `arg` is
> + * NULL, then opt->value is set to "--$defval". If unset is true, then
> + * opt->value is set to "--no-$defval".
> + */
> +static int parse_opt_passthru(const struct option *opt, const char
> *arg, int unset)
How about adding this to parse-options-cb.c in a separate patch? I guess the description could say something like:
/**
* Given an option opt, recreate the command-line option, as strbuf. This is useful
* when a command needs to parse a command-line option in order to pass it to yet
* another command. This callback can be used in conjunction with the
* PARSE_OPT_(OPTARG|NOARG|NONEG) options, but not with PARSE_OPT_LASTARG_DEFAULT
* because it expects `defval` to be the name of the command-line option to
* reconstruct.
*
* The result will be stored in opt->value, which is expected to be a pointer to an
* strbuf.
*/
Implied by my suggested description, I also propose to put the re-recreated command-line option into a strbuf instead of a char *, to make memory management easier (read: avoid memory leaks).
You might also want to verify that arg is `NULL` when `unset != 0`. Something like this:
int parse_opt_passthru(const struct option *opt, const char *arg, int unset)
{
struct strbuf *buf = opt->value;
assert(opt->defval);
strbuf_reset(buf);
if (unset) {
assert(!arg);
strbuf_addf(buf, "--no-%s", opt->defval);
}
else {
strbuf_addf(buf, "--%s", opt->defval);
if (arg)
strbuf_addf(buf, "=%s", arg);
}
return 0;
}
> static struct option pull_options[] = {
> + /* Shared options */
> + OPT__VERBOSITY(&opt_verbosity),
> + { OPTION_CALLBACK, 0, "progress", &opt_progress, NULL,
> + N_("force progress reporting"),
> + PARSE_OPT_NOARG, parse_opt_passthru},
I *think* there is a '(intptr_t) "progress"' missing...
> /**
> + * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level.
> + */
> +static void argv_push_verbosity(struct argv_array *arr)
> +{
> + int verbosity;
> +
> + for (verbosity = opt_verbosity; verbosity > 0; verbosity--)
> + argv_array_push(arr, "-v");
> +
> + for (verbosity = opt_verbosity; verbosity < 0; verbosity++)
> + argv_array_push(arr, "-q");
> +}
Hmm... I guess this is *really* nit-picky, but I'd rather use "i" instead of "verbosity" because the second loop is about quietness instead of verbosity ;-)
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html