Re: [PATCH 1/1] rebase: really just passthru the `git am` options
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 13, 2018, 13:05 UTC
- Message-ID
- <xmqqlg5xunzh.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <dc36a450680b1854abbb9bb06180001ce68c3f3b.1542112703.git.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 10 quoted lines
> 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.
Show 18 quoted lines
> @@ -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.
Show 7 quoted lines
> + 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.