Re: [GSoC][PATCH v2 1/1] rebase -i: add --ignore-whitespace flag

[Rohit Ashiwal <rohit.ashiwal265@gmail.com>]

Hi Phillip

On Mon, 22 Jul 2019 11:00:40 +0100 Phillip Wood <phillip.wood123@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