Re: [PATCH] refuse to merge during a merge

Clemens Buchacher <drizzd@aon.at> writes:

I think the new condition that you added to stop the merge is more in line with the original intent of the check. We never wanted to check "do we still have unmerged entries?" but wanted to see "is another merge in progress?"; not checking MERGE_HEAD was a simple omission.

But I do not necessarily agree with the combined check nor with the new message. I think it would be more sensible to split the codepath like this:

	if (we see MERGE_HEAD)
		die("You have not concluded your merge (MERGE_HEAD exists).");
	if (the index is unmerged)
		die("You are in the middle of a conflicted merge (index unmerged).");

Then in a _later_ patch, you could try to be more helpful by paying more attention to the context. E.g.

	if (we see MERGE_HEAD) {
        	figure out what was attempted by looking at
                MERGE_MSG and other cues;
		die("You have not concluded your merge with %s.\n"
		    "Perhaps you would want 'git reset' to recover?"
                    that);
	}
	if (the index is unmerged)
		die("You are in the middle of a conflicted merge");

The point is that combining the checks makes it harder to later give more appropriate diagnosis and suggestion to the end user.

For example, "git merge" may learn "git merge --abort" like other commands that have "attempt, stop, let the user fix up to conclude" modes of operations (i.e. rebase and am), and we may suggest to use that to recover in the message, instead of 'git reset'. But that can only be used if we stopped because we saw MERGE_HEAD; you definitely do not want to suggest "git merge --abort" if the index is unmerged due to a conflicted rebase in progress.

Note that I am not suggesting you to blow this up to one large patch by adding fancier "what were we doing" logic; I am perfectly OK with the minimum "detect MERGE_HEAD and refuse". I am only saying that I am unhappy with the way two different error conditions are conflated.

Personally, I'd suggest not to give "you can do this to recover" message.