Re: [PATCH v5 2/3] pull: move default warning
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 13, 2020, 20:58 UTC
- Message-ID
- <xmqqpn3cep51.fsf@gitster.c.googlers.com>
- In-Reply-To
- <CAMP44s1v+ESk9y8WC8rKPb61KD_kUoM9F1gO-TZhEEgOzUAPTw@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 28 quoted lines
> 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.