Subject: Re: [PATCH v2 2/2] rebase -i: improve error message when picking merge

From: Junio C Hamano <[email-masked]>
Date: Tue, 09 Apr 2024 12:56:45 -0700

Phillip Wood <[email-masked]> writes:

>>> +		return error(_("'%s' does not accept merge commits, "
>>> +			       "please use '%s' followed by '%s'"),
>>> +			     todo_command_info[command].str,
>>> +			     "merge -C", "break");
>> OK.  And when hitting the "break", they know that they are supposed
>> to say "git commit --amend" and then "git rebase --continue"?
>
> Yes. I guess we could add a hint to that effect if you think its worth it.

As I said elsewhere, I do not deal with non-linear stuff using
sequencer, so I _may_ be missing something obvious to the target
audience of this message.  But if I were dipping my toes to try
mucking with sequencer and edit the todo myself by inserting a random
merge commit there and got this message, I would have probably
appreciated if the message were a bit more explicit _why_ I would
want to use the 'break' there.  Otherwise, I probably would be lost
sitting in front of the shell command prompt.  If it were

	'pick' does not take a merge commit.  If you wanted to
	replay the merge, use 'merge -C' on the commit, and then
	'break' to give the control back to you so that you can
	do 'git commit --amend && git rebase --continue'.

that would have given me 70% of what I needed (the other 30% is why
I would want to "--amend", instead of just taking the result of
'merge -C' as-is, though).

We can formulate the message in such a way that a succinct first
part is sufficient for people who know the command, while the latter
more verbose and prescriptive part can be hidden behind the advice
mechanism.

> It feels funny to call error_merge_commit() for a command that we know
> supports merges but I can see that would make it easier to extend in
> the future.

Yes, I think that it is just a sign that the function is misnamed.
"Gee we have a merge commit, please tell me what you want to do with
it" is what the caller is asking, and the error messages are side
effects the caller does not have to care too much about.