From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.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: Fri, 12 Apr 2024 14:24:39 +0100 [thread overview]
Message-ID: <88bc0787-e7ae-49e5-99e8-97f6c55ea8c6@gmail.com> (raw)
In-Reply-To: <xmqqpluy1f9e.fsf@gitster.g>
Hi Junio
On 09/04/2024 20:56, Junio C Hamano wrote:
> 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.
I'll re-roll with a short error message plus some advice. I'll have a
think about adding advice for the other commands as well explaining how
the "merge" command should be used as well.
>> 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.
Sure
Thanks
Phillip
prev parent reply other threads:[~2024-04-12 13:24 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
2024-04-12 13:24 ` Phillip Wood [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=88bc0787-e7ae-49e5-99e8-97f6c55ea8c6@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=lists@haller-berlin.de \
--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.
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).