Re: [PATCH] rebase -i: allow a comment after a "break" command

[Jeff King <peff@peff.net>]

On Fri, Jan 13, 2023 at 12:52:40AM +0900, Junio C Hamano wrote:

> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > From: Phillip Wood <phillip.wood@dunelm.org.uk>
> >
> > When adding a "break" command to a rebase todo list it can be helpful to
> > add a comment as a reminder as to what the user was planning to do when
> > the rebase stopped. Anything following the command is interpreted as an
> > argument to the command and results in an error. Change this so that a
> > "break command may be followed by "# <comment>" in the same way as
> > a "merge" command. Requiring the comment to begin with "# " allows the
> > break command to start taking an argument in the future if that turns
> > out to be useful.
> 
> Why do we special case "break" and not give the same "comment is
> emitted when the control reaches the insn in the todo list" for
> others like "exec" or even "pick"?

I had somewhat the opposite thought. The "break" command is special in
that it is not doing anything useful except returning control to the
user. And hence producing a message is a useful add-on. So I expected
the patch to just allow:

  break this is a message the user will see

without any "#" at all.

That does close the door for further arguments, but I have trouble
imagining what they would be.

> Another comment with devil's advocate hat on is if we are better off
> not adding "# this comment is emitted" at all, and instead do
> 
>     pick ...
>     pick ...
>     exec echo this comment is emitted
>     break
>     pick ...

Yeah, I have certainly done something like that before. I think it is
just a matter of ergonomics. I'm pretty sure I have also forgotten to
put in the "break" afterwards.

Actually, before we had "break" I think I've done:

  x false this message will be visible when this fails

which is even more horrible. ;)

-Peff