"Johannes Schindelin via GitGitGadget" <gitgitgad...@gmail.com>
writes:
> However, there is a much better way (that I was unaware of, at the time
> when I mentored Pratik to implement these options): OPT_PASSTHRU_ARGV.
> It is intended for exactly this use case, where command-line options
> want to be parsed into a separate `argv_array`.
>
> Let's use this feature.
>
> Incidentally, this also allows us to address a bug discovered by Phillip
> Wood, where the built-in rebase failed to understand that the `-C`
> option takes an optional argument.
The resulting code does show what is going on more clearly. I
wonder if we can do the same for -S as well? It needs to be passed
to other backends, so I guess it is not such a good idea.
> - .git_am_opt = STRBUF_INIT,
> + .git_am_opts = ARGV_ARRAY_INIT,
Yes, this is much nicer.
> @@ -856,10 +858,6 @@ int cmd_rebase(int argc, const char **argv, const char
> *prefix)
> { OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
> N_("GPG-sign commits"),
> PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> - OPT_STRING_LIST(0, "whitespace", &whitespace,
> - N_("whitespace"), N_("passed to 'git apply'")),
> - OPT_SET_INT('C', NULL, &opt_c, N_("passed to 'git apply'"),
> - REBASE_AM),
> OPT_BOOL(0, "autostash", &options.autostash,
> N_("automatically stash/stash pop before and after")),
> OPT_STRING_LIST('x', "exec", &exec, N_("exec"),
> @@ -884,6 +882,7 @@ int cmd_rebase(int argc, const char **argv, const char
> *prefix)
> N_("rebase all reachable commits up to the root(s)")),
> OPT_END(),
> };
> + int i;
>
> + for (i = 0; i < options.git_am_opts.argc; i++) {
Yes, this is very nice way to first collect and then inspect them
separately.
> + const char *option = options.git_am_opts.argv[i];
> + if (!strcmp(option, "--committer-date-is-author-date") ||
> + !strcmp(option, "--ignore-date") ||
> + !strcmp(option, "--whitespace=fix") ||
> + !strcmp(option, "--whitespace=strip"))
> + options.flags |= REBASE_FORCE;
> }
Overall very nicely done. Perhaps we'll see a test or two from
Philip?
Thanks.
