[PATCH v3 2/3] sequencer: comment `--reference` subject line properly
From: <hidden>
Date: 2024-11-24 20:56:55
Subsystem:
the rest · Maintainer:
Linus Torvalds
From: Kristoffer Haugsbakk <redacted>
`git revert --reference <commit>` leaves behind a comment in the
first line:[1]
# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***
Meaning that the commit will just consist of the next line if the user
exits the editor directly:
This reverts commit <--format=reference commit>
But the comment char here is hardcoded (#). Which means that the
comment line will inadvertently be included in the commit message if
`core.commentChar`/`core.commentString` is in use.
† 1: See 43966ab3156 (revert: optionally refer to commit in the
"reference" format, 2022-05-26)
Signed-off-by: Kristoffer Haugsbakk <redacted>
---
Notes (series):
v3:
• Review feedback: check more in the test by inspecting the
proposed commit message.
Link: https://lore.kernel.org/git/4c623fcf-01dd-4056-80c1-b3c860ab7f87@gmail.com/ (local)
• Message:
• Rewrite message now that we are testing something different
• consistency with the other two messages (see previous)
v2:
• `strbuf_commented_addf` adds a newline, unlike the previous function.
We need to remove a newline from the final `strbuf_addstr` with `This
reverts commits` and add a newline to each of the other
branches (`else if` and `else`).
sequencer.c | 9 +++++----
t/t3501-revert-cherry-pick.sh | 14 ++++++++++++++
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 1b6fd86f70b..d26299cdea2 100644
--- a/sequencer.c
+++ b/sequencer.c@@ -2341,8 +2341,8 @@ static int do_pick_commit(struct repository *r, next = parent; next_label = msg.parent_label; if (opts->commit_use_reference) { - strbuf_addstr(&ctx->message, - "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***"); + strbuf_commented_addf(&ctx->message, comment_line_str, + "*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***"); } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) && /* * We don't touch pre-existing repeated reverts, because
@@ -2352,12 +2352,13 @@ static int do_pick_commit(struct repository *r, !starts_with(orig_subject, "Revert \"")) { strbuf_addstr(&ctx->message, "Reapply \""); strbuf_addstr(&ctx->message, orig_subject); + strbuf_addstr(&ctx->message, "\n"); } else { strbuf_addstr(&ctx->message, "Revert \""); strbuf_addstr(&ctx->message, msg.subject); - strbuf_addstr(&ctx->message, "\""); + strbuf_addstr(&ctx->message, "\"\n"); } - strbuf_addstr(&ctx->message, "\n\nThis reverts commit "); + strbuf_addstr(&ctx->message, "\nThis reverts commit "); refer_to_commit(opts, &ctx->message, commit); if (commit->parents && commit->parents->next) {
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 411027fb58c..43476236131 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh@@ -228,6 +228,20 @@ test_expect_success 'identification of reverted commit (--reference)' ' test_cmp expect actual ' +test_expect_success 'git revert --reference with core.commentChar' ' + test_when_finished "git reset --hard to-ident" && + git checkout --detach to-ident && + GIT_EDITOR="cat | head -4 >actual" git -c core.commentChar=% revert \ + --edit --reference HEAD && + cat <<-EOF >expect && + % *** SAY WHY WE ARE REVERTING ON THE TITLE LINE *** + + This reverts commit $(git show -s --pretty=reference HEAD^). + + EOF + test_cmp expect actual +' + test_expect_success 'identification of reverted commit (revert.reference)' ' git checkout --detach to-ident && git -c revert.reference=true revert --no-edit HEAD &&
--
2.47.0