Re: [RFC 2/3] merge: Add hints to tell users about "git merge --abort"

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 5, 2014, 18:29 UTC
Message-ID: <xmqqa9d4ijmu.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CADgNjak3aqPDV0iZYc8b6QJ9y+6bUd28n0UJOm6WjufQhjfuwA@mail.gmail.com>

Andrew Wong <andrew.kw.w@gmail.com> writes:

Show 30 quoted lines

> On Wed, Feb 26, 2014 at 3:38 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Andrew Wong wrote:
>>
>>> --- a/builtin/merge.c
>>> +++ b/builtin/merge.c
>>> @@ -909,7 +909,8 @@ static int suggest_conflicts(int renormalizing)
>>>       fclose(fp);
>>>       rerere(allow_rerere_auto);
>>>       printf(_("Automatic merge failed; "
>>> -                     "fix conflicts and then commit the result.\n"));
>>> +                     "fix conflicts and then commit the result.\n"
>>> +                     "To abort the merge, use \"git merge --abort\".\n"));
>>
>> Seems reasonable, but I worry about the command growing too noisy.
>>
>> Could this be guarded by an advice.<something> setting?  (See advice.*
>> in git-config(1) for what I mean.)
>
> I was planning to use advice.resolveConflict, but as I went through
> merge.c, I noticed there could be a few other situations where we
> could print out the same message:
> 1. when prepare_to_commit() fails, due to hook error, editor error, or
> empty commit message
> 2. "git commit --no-commit"
>
> This means contexts are no longer only about "resolving conflict", so
> I was thinking of renaming advice.resolveConflict to something like
> advice.mergeHints.
>
> Any thoughts?

I have no strong opinion on the naming, other than that I doubt this particular new "how to abort" message is worth the headache associated with the "rename" which involves transition planning of deprecating the old, supporting both for a while and then removing the old.

The existing message above in "suggest-conflicts" is about hinting the user to first resolve the conflict before attempting to continue, and that is perfectly in line with the existing use of advice.resolveConfict in die_conflict() in git-pull that tells the user there is an unresolved conflict.

On the other hand, the additional "how to abort" message does not have to be limited to "you have conflicted paths in the index" case.

If the user said "git merge" while another "git merge" is still outstanding, we would want to say "You have not concluded your previous merge" and die, and you presumably want to add the same "how to abort" message there. Such a codepath is unlikely to be covered by existing advice.resolveConflict, and it sounds more natural, at least to me, to use a separate variable to squelch only the new "how to abort" part.