Re: [PATCH/RFC 1/2] pull: pass the --no-ff-only flag through to merge, not fetch

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

 



On Thu, Dec 1, 2011 at 1:06 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Samuel Bronson <naesten@xxxxxxxxx> writes:
>
>> Hmm, yes, I had noticed that it was a tristate (merge.ff clearly is),
>> and I guess --no-ff-only is a pretty ugly flag. I do have to ask,
>> though: why give --ff these new values? Wouldn't it make more sense to
>> reuse the values accepted by merge.ff; namely, 'true' (the implied
>> default), 'false', and 'only'?
>
> The 'true' and 'false' values to merge.ff are carry-over from the days
> when it was a boolean, _not_ a tristate. If we were to make the UI more
> rational by making it clear that this is not a boolean, it is a good time
> for us to aim a bit higher than merely repeating the mistakes we made in
> the past due to historical accident. In other words, we could add a
> synonym for the "default" mode in addition to "--ff=true" (and for the
> "always merge" mode in addition to "--ff=false") that makes it clear that
> the value is _not_ a boolean [*1*]. If we were to go the "--ff=<value>"
> route, we have to add support for other ways to spell boolean 'true'
> (e.g. 'yes', '1', and 'on') anyway, so it is not that much extra work to
> do so, I would think.

Sure, that makes sense. I was just a little worried that you might be
(accidentally) proposing that --ff use a different set of names than
merge.ff for a moment there...

>> Otherwise, this looks like a very nice way to implement what I want: I
>> guess it is probably a mistake that the existing (documented) flags do
>> not behave in this way?
>
> Yeah, right now if you say "merge --ff-only --no-ff", we say these are
> mutually exclusive (which is true), but if you think about the tristate
> nature of the 'ff' option and spell it differently in your head, i.e.
> "merge --ff=only --ff=never", it is reasonable to argue that we should
> apply the usual "last one overrides" rule and behave as if "merge --no-ff"
> were given (for the purpose of "last one overrides", the configured
> defaults can be treated as if they come very early on the command line).
> After all "merge --no-ff --ff" does seem to use the "last one overrides"
> rule.

Yes, I totally agree that it would make more sense that way; I
certainly tried that before I even began to look at any of the code.

> [Footnote]
>
> *1* Perhaps 'allowed' instead of 'normal' (which I wrote out of thin-air;
> I do not have any strong preference on the actual values) may be a better
> choice for such a "this is not a boolean" spelling for the default mode.
--
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]