Re: [PATCH v5 2/3] pull: move default warning

From: Junio C Hamano <gitster@pobox.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: Git <git@vger.kernel.org>, "Elijah Newren" <newren@gmail.com>,
	"Jeff King" <peff@peff.net>, "Vít Ondruch" <vondruch@redhat.com>
Subject: Re: [PATCH v5 2/3] pull: move default warning
Date: Sun, 13 Dec 2020 12:58:41 -0800	[thread overview]
Message-ID: <xmqqpn3cep51.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: CAMP44s1v+ESk9y8WC8rKPb61KD_kUoM9F1gO-TZhEEgOzUAPTw@mail.gmail.com

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Fri, Dec 11, 2020 at 6:00 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> > Should actually be something like:
>> >
>> >         if (rebase_unspecified && !can_ff)
>> >                 die("Not a fast-forward; must either merge or rebase");
>>
>> The illustration I gave in the message you are responding to was
>> made in the context of patch 2/3; with patch 3/3 where can_ff
>> exists, it would not become like what you gave above.  It should
>> instead become
>>
>>         if (rebase_unspecified && !opt_ff && !can_ff) {
>>                 if (opt_verbosity >= 0 && advice_pull_non_ff)
>>                         show_advice_pull_non_ff();
>>                 die("not a fast-forward; must merge or rebase");
>>         }
>>
>> i.e. when we can fast-forward, we do not trigger the "you must
>> specify rebase/merge" message, and we do not trigger the "not a
>> fast-forward" error.
>
> It's not the !can_ff part I'm trying to highlight, it's the lack of
> advice *after* we have decided to flip the switch.
>
> As I said in another thread: I don't think we have any long
> condescending error in any other command.

Only the "not a fast-forward and you must choose between merge and
rebase" part (i.e. what I listed as the first part of three-part
message) is the error.

The rest, the message that teaches how to choose between merge and
rebase from command line and configuration (or how to choose
permanently not to make the choice between the two), is not an
error---it's called advice.

Even if we were to introduce the third choice (i.e. permanently not
to choose between rebase or merge and instead just error out without
getting advice), the advice must stay for those who didn't make any
choice among the three (i.e. merge, rebase or ff-only).

We have many of them.  An easy example to spot is a similar sized
onefor "git checkout HEAD^0".  Neither that one or the one under
discussion is particularly condescending.