Re: [PATCH v3 1/3] pull --rebase: add --[no-]autostash flag

From: Mehul Jain <mehul.jain2029@gmail.com>
Date: Mar 4, 2016, 05:37 UTC
Message-ID: <CA+DCAeSRN=nqhnv14kvbnxExiSBPnA3hLP8hY0fbd+YgTR3TNw@mail.gmail.com>
In-Reply-To: <vpqd1rbiifi.fsf@anie.imag.fr>

On Thu, Mar 3, 2016 at 10:54 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 15 quoted lines

> Mehul Jain <mehul.jain2029@gmail.com> writes:
>
>> If rebase.autoStash configuration variable is
>> set, there is no way to override it for
>> "git pull --rebase" from the command line.
>>
>> Teach "git pull --rebase" the --[no]autostash
>> command line flag which overrides the current
>> value of rebase.autostash, if set. As "git rebase"
>> understands the --[no]autostash option, it's
>> just a matter of passing the option to underlying
>> "git rebase" when "git pull --rebase" is called.
>
> We normally wrap text with a bit less than 80 columns. Yours is wrappet
> at 50 columns which makes it look weird.

OK. I will change it.

Show 7 quoted lines

>> +     else {
>> +             /* If --[no-]autostash option is called without --rebase */
>> +             if (opt_autostash == 0)
>> +                     die(_("--no-autostash option is only valid with --rebase."));
>> +             else if (opt_autostash == 1)
>
> The else is not needed since the other branch dies.

I'm bit confused here. Which "else" you are talking about. I think both the "else" and "else if" are needed here because:

- for the first "else", it is necessary that the case is only executed
when --rebase option is not given. If "else" is removed then in some case
where user calls "git pull --rebase --autostash" will lead to the execution of
"else if (opt_autostash == 1)"  case.

- Also removal of  "else if (opt_autostash == 1)" is not the right thing. As
the possibility of opt_autostash = -1 is there and this change may lead to
the execution of "die(_("--no-autostash ... "));" in case user calls "git pull".

Though I agree with Eric on combining the "if and else if" cases.

Thanks, Mehul