Re: [RFC PATCH 7/7] merge: teach --autostash option

From: Denton Liu <liu.denton@gmail.com>
Date: Dec 24, 2019, 10:12 UTC
Message-ID: <20191224101214.GA2744494@generichostname>
In-Reply-To: <20191224100507.GB2743221@generichostname>

On Tue, Dec 24, 2019 at 05:05:07AM -0500, Denton Liu wrote:

Show 23 quoted lines

> > >  	OPT_PASSTHRU_ARGV('s', "strategy", &opt_strategies, N_("strategy"),
> > >  		N_("merge strategy to use"),
> > >  		0),
> > > @@ -671,6 +671,10 @@ static int run_merge(void)
> > >  	argv_array_pushv(&args, opt_strategy_opts.argv);
> > >  	if (opt_gpg_sign)
> > >  		argv_array_push(&args, opt_gpg_sign);
> > > +	if (opt_autostash == 0)
> > > +		argv_array_push(&args, "--no-autostash");
> > > +	else if (opt_autostash == 1)
> > > +		argv_array_push(&args, "--autostash");
> > 
> > Or shorter:
> > 
> > 	argv_array_pushf(&args, "%s-autostash", opt_autostash ? "-" : "--no");
> > 
> > Ah, but that would mishandle `-1`. I bet I will be puzzled by this
> > again. Maybe it would make sense to mention in a code comment that it
> > can be `-1` in which case we leave it to `rebase` to use the config
> > settings to determine whether or not to autostash.
> 
> I copied this over from the rebase case. I'll add a comment there as
> well.

Actually, on another thought, this happens in a couple of places in builtin/pull.c. I'm not sure that it's worth the noise of commenting it every place where this happens. I dunno. I'll send a re-roll out and if someone else brings it up again, I'll add the comments in.