Hi Andrei
Thanks for taking a look at this
On 12/01/2023 11:14, Andrei Rybak wrote:
On 12/01/2023 11:36, Phillip Wood via GitGitGadget wrote:
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.
Reported-by: Olliver Schinagl <oliver@xxxxxxxxxxx>
Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
---
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 ...".
Yes I think that is probable a good idea.
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.
Thanks that's a good suggestion.
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).
I did look at using set_cat_todo_editor but it isn't that simple. It
cannot be used with set_fake_editor so I'd need to find another way to
supply the todo list and it always exits with a failure.
As well as checking that "break" accepts a comment this test also checks
that the comment is still present when the user re-edits the todo list
which is tricky to do with set_cat_todo_editor.
Best Wishes
Phillip
