From: Andrei Rybak Date: Thu, 12 Jan 2023 11:14:53 GMT Subject: Re: [PATCH] rebase -i: allow a comment after a "break" command Message-ID: In-Reply-To: On 12/01/2023 11:36, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood > > 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 "# " 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 > Signed-off-by: Phillip Wood > --- > 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