On Tue, Dec 24, 2019 at 05:05:07AM -0500, Denton Liu wrote:
> > > 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.
