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

[Andrei Rybak <rybak.a.v@gmail.com>]

On 12/01/2023 11:36, Phillip Wood via GitGitGadget wrote:
> 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.
> 
> Reported-by: Olliver Schinagl <oliver@schinagl.nl>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>      rebase -i: allow a comment after a "break" command
>      
>      I'm open to suggestions for other ways to handle comments but copying
>      what we do to separate merge parents from the merge commit subject
>      seemed simplest.
>      
>      Should this print the comment when stopping for a break command?

Technically, the user can look up the command via `git status`, but it
would make sense to just give the user this information directly,
similar to how exec command prints "Executing: ..." in addition to the
existing break command's message "Stopped at ...".

> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1460%2Fphillipwood%2Fsequencer-allow-comment-after-break-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1460/phillipwood/sequencer-allow-comment-after-break-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1460
> 
>   Documentation/git-rebase.txt |  4 +++-
>   sequencer.c                  |  7 +++++--
>   t/lib-rebase.sh              |  2 +-
>   t/t3418-rebase-continue.sh   | 16 ++++++++++++++++
>   4 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index f9675bd24e6..511ace43db0 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -869,7 +869,9 @@ the files and/or the commit message, amend the commit, and continue
>   rebasing.
>   
>   To interrupt the rebase (just like an "edit" command would do, but without
> -cherry-picking any commit first), use the "break" command.
> +cherry-picking any commit first), use the "break" command. A "break"
> +command may be followed by a comment beginning with `#` followed by a
> +space.

A corresponding update to append_todo_help in rebase-interactive.c
would be helpful.

>   
>   If you just want to edit the commit message for a commit, replace the
>   command "pick" with the command "reword".

[...]

> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index 130e2f9b553..18d82869b38 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -266,6 +266,22 @@ test_expect_success 'the todo command "break" works' '
>   	test_path_is_file execed
>   '
>   
> +test_expect_success 'the todo command "break" accepts a comment' '
> +	rm -f execed &&
> +	test_write_lines "break # comment" "break #" "exec >execed" >expect &&
> +	write_script cat-todo.sh <<-\EOS &&
> +	GIT_SEQUENCE_EDITOR="grep ^\[^#\]" git rebase --edit-todo >actual
> +	EOS
> +	FAKE_LINES="exec_./cat-todo.sh break_#_comment b_# exec_>execed" \

It seems that helper set_cat_todo_editor could be used here, except that
tests in t3418-rebase-continue.sh use a global set_fake_editor at the
very top of the file, unlike tests in t3404-rebase-interactive.sh which
call set_fake_editor individually.  See also related commits 6a619ca03c
(t3404: remove uneeded calls to set_fake_editor, 2019-10-15) and
b2dbacbddf (t3404: set $EDITOR in subshell, 2019-10-15).

> +		git rebase -i HEAD &&
> +	test_cmp expect actual &&
> +	test_path_is_missing execed &&
> +	git rebase --continue &&
> +	test_path_is_missing execed &&
> +	git rebase --continue &&
> +	test_path_is_file execed
> +'
> +
>   test_expect_success '--reschedule-failed-exec' '
>   	test_when_finished "git rebase --abort" &&
>   	test_must_fail git rebase -x false --reschedule-failed-exec HEAD^ &&
> 
> base-commit: 8a4e8f6a67e7fc97048d4666eec38399b88e0e3b