git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "Stefan Haller" <lists@haller-berlin.de>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Patrick Steinhardt" <ps@pks.im>,
	"Rubén Justo" <rjusto@gmail.com>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH v2 2/2] rebase -i: improve error message when picking merge
Date: Tue, 09 Apr 2024 12:56:45 -0700	[thread overview]
Message-ID: <xmqqpluy1f9e.fsf@gitster.g> (raw)
In-Reply-To: <1b74f6f9-f4b9-4909-82b3-26f19b7a1347@gmail.com> (Phillip Wood's message of "Tue, 9 Apr 2024 16:04:33 +0100")

Phillip Wood <phillip.wood123@gmail.com> 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.


  reply	other threads:[~2024-04-09 19:57 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-26 10:58 [PATCH] rebase -i: improve error message when picking merge Phillip Wood via GitGitGadget
2024-04-03 13:42 ` phillip.wood123
2024-04-04  6:08   ` Patrick Steinhardt
2024-04-04  6:08 ` Patrick Steinhardt
2024-04-04 15:29   ` phillip.wood123
2024-04-04 19:44 ` Rubén Justo
2024-04-05  9:30   ` phillip.wood123
2024-04-06 14:24     ` Rubén Justo
2024-04-07 13:55       ` phillip.wood123
2024-04-08 14:16 ` [PATCH v2 0/2] " Phillip Wood via GitGitGadget
2024-04-08 14:16   ` [PATCH v2 1/2] rebase -i: pass struct replay_opts to parse_insn_line() Phillip Wood via GitGitGadget
2024-04-09  4:03     ` Patrick Steinhardt
2024-04-08 14:16   ` [PATCH v2 2/2] rebase -i: improve error message when picking merge Phillip Wood via GitGitGadget
2024-04-08 22:29     ` Junio C Hamano
2024-04-09  4:03       ` Patrick Steinhardt
2024-04-09  5:08         ` Junio C Hamano
2024-04-09  6:04           ` Patrick Steinhardt
2024-04-09 15:04       ` Phillip Wood
2024-04-09 19:56         ` Junio C Hamano [this message]
2024-04-12 13:24           ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqpluy1f9e.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=lists@haller-berlin.de \
    --cc=phillip.wood123@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    --cc=rjusto@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).