On Fri, Jan 13, 2023 at 12:52:40AM +0900, Junio C Hamano wrote: > "Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> > > > > 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
