Hi Phillip

On Mon, 22 Jul 2019 11:00:40 +0100 Phillip Wood <phillip.wood...@gmail.com> 
wrote:
> 
> [...]
> >
> > +   if (opts->ignore_whitespace) {
> > +           struct strbuf buf = STRBUF_INIT;
> > +
> > +           if (opts->strategy_opts)
> > +                   strbuf_addstr(&buf, opts->strategy_opts);
> > +
> > +           strbuf_addstr(&buf, " --ignore-space-change");
> > +           free(opts->strategy_opts);
> > +           opts->strategy_opts = strbuf_detach(&buf, NULL);
> > +   }
> > +
> 
> I think this would fit better in get_replay_opts()

Agreed, I'll move this to get_replay_opts().

> [...]
> 
> > @@ -489,6 +501,8 @@ int cmd_rebase__interactive(int argc, const char 
> > **argv, const char *prefix)
> >             { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, 
> > N_("key-id"),
> >                     N_("GPG-sign commits"),
> >                     PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> > +           OPT_BOOL(0, "ignore-whitespace", &opts.ignore_whitespace,
> > +                    N_("ignore changes in whitespace")),
> 
> As with the other patch is this actually going to be used by
> rebase--preserve-merges.sh?

I added this just for the completness. Is there any discussion on
dropping rebase--interactive as command and may be lib'fying it while
deprecating rebase--preserve-merges?

> [...]
> 
> > @@ -43,6 +43,7 @@ struct replay_opts {
> >     int verbose;
> >     int quiet;
> >     int reschedule_failed_exec;
> > +   int ignore_whitespace;
> 
> Is this new field used anywhere - we add -Xignore-space-change to
> replay_opts.xopts so why do we need this as well?

Ah! I just realised cmd_rebase__interactive use rebase_options and
not replay_options. I too think this field is not required.

Thanks
Rohit

Reply via email to