Hi Junio
On 08/04/2024 23:29, Junio C Hamano wrote:
"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
+ case TODO_EDIT:
+ 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.
item->commit = lookup_commit_reference(r, &commit_oid);
- return item->commit ? 0 : -1;
+ if (!item->commit)
+ return -1;
+ if (is_rebase_i(opts) && item->command != TODO_MERGE &&
+ item->commit->parents && item->commit->parents->next)
+ return error_merge_commit(item->command);
This is good for now, but we may see command other than TODO_MERGE
learn how to handle a merge commit, and when that happens, I wonder
what we want to do here. One thought is to do this:
if (is_rebase_i(opts) && is_merge_commit(item->commit))
return error_merge_commit(item);
and teach error_merge_commit() to silently return 0 on TODO_MERGE.
Other commands, when they learn how to deal with a merge commit,
will then update their entries in error_merge_commit().
f
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.
Would we want to customize the message from error_merge_commit() to
make it closer to cut-and-paste ready? For that, something like
int error_merge_commit(struct todo_item *item)
{
switch (item->command) {
case TODO_PICK:
return error(_("'%s'" is bad, plase use "
'%s %s'"),
todo_command_info[item->command].str,
"merge -C",
oid_to_hex(item->commit->oid));
...
}
}
might go in a more friendly way.
If we want something the user can cut and paste then we would need to
suggest a second parent for the merge. We cannot just use the existing
parent as it may be rebased which means tracking all the commits we see
while parsing the list. If the second parent is rebased we need to make
sure the user labels it so we can use that label here. For that reason
(and the fact that I don't think can we really tell what the user was
hoping to achieve) I think the best we can do is point the user in the
direction of the "merge" command even though that leaves them to figure
out what to do with that command.
Best Wishes
Phillip
