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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 1, 2011, 18:06 UTC
Message-ID: <7vvcq0np35.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAJYzjmep7sKxiSNhMzAX2DRYJhANDQkPL5pX4HOZ9CssJxcWbw@mail.gmail.com>

Samuel Bronson <naesten@gmail.com> writes:

Show 5 quoted lines

> 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.

> 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.

[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.