Re: [PATCH] merge: handle --ff/--no-ff/--ff-only as a tri-state option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:

> You allow --no-ff-only but ignore it, which I think is incorrect.  In
>
>     git merge --ff-only --no-ff-only [...]
>
> , the --no-ff-only should presumably cancel the effect of the previous
> --ff-only (i.e., be equivalent to "--ff").

Ideally, if we were starting from scratch and living in the "you
pick one out of three" world, we should forbid "--no-ff-only".  The
"--no-ff" option is spelled as if it is a negation of "--ff", and it
did start as such before "--ff-only" was introduced, but "--no-ff"
would have been better named "--always-create-merge", which is what
the option really means.

And in the tristate world, with mutually exclusive "--A", "--B", and
"--C" options, "--no-C" does not mean "I want to do A" at all.

If the existing code had allowed with "--no-ff-only" to defeat
configured merge.ff=only from the command line, then there may have
been users who are used to that behaviour, and we cannot break them,
but luckily or unluckily it does not work, so...

>> @@ -1157,14 +1181,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>>  		show_diffstat = 0;
>>  
>>  	if (squash) {
>> -		if (!allow_fast_forward)
>> +		if (fast_forward == FF_NO)
>>  			die(_("You cannot combine --squash with --no-ff."));
>>  		option_commit = 0;
>>  	}
>>  
>
> So there is still a problem with setting merge.ff=false, namely that it
> prevents the use of --squash.  That's not good.  (I realize that you are
> not to blame for this pre-existing behavior.)
>
> How should --squash and the ff-related options interact?

Interesting point.

>     git merge --ff --squash
>     git merge --no-ff --squash
>
> I think these should just squash.
>
>     git merge --ff-only --squash
>
> I think this should definitely squash.  But perhaps it should require
> that HEAD be an ancestor of the branch to be merged?
>
>     git merge --squash --ff
>     git merge --squash --no-ff
>     git merge --squash --ff-only
>
> Should these do the same as the versions with the option order reversed?

As "--squash" is about _not_ moving the head but only updating the
working tree and the index, I personally think it should be treated
as an error if any of these "ff" options is explicitly given from
the command line.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]