From 94304b9f48e9c68f3214e25527c3aac865d3ce63 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Mon, 25 Nov 2024 21:13:11 +0100 Subject: [PATCH 1/3] sequencer: comment checked-out branch properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git rebase --update-ref` does not insert commands for dependent/sub- branches which are checked out.[1] Instead it leaves a comment about that fact. The comment char is hardcoded (#). In turn the comment line gets interpreted as an invalid command when `core.commentChar`/ `core.commentString` is in use. † 1: See 900b50c242 (rebase: add --update-refs option, 2022-07-19) Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- sequencer.c | 5 +++-- t/t3400-rebase.sh | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index 353d804999b88d..1b6fd86f70bc4b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -6382,8 +6382,9 @@ static int add_decorations_to_list(const struct commit *commit, /* If the branch is checked out, then leave a comment instead. */ if ((path = branch_checked_out(decoration->name))) { item->command = TODO_COMMENT; - strbuf_addf(ctx->buf, "# Ref %s checked out at '%s'\n", - decoration->name, path); + strbuf_commented_addf(ctx->buf, comment_line_str, + "Ref %s checked out at '%s'\n", + decoration->name, path); } else { struct string_list_item *sti; item->command = TODO_UPDATE_REF; diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index 09f230eefb2cbc..7c47af6dcd9d45 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -456,4 +456,23 @@ test_expect_success 'rebase when inside worktree subdirectory' ' ) ' +test_expect_success 'git rebase --update-ref with core.commentChar and branch on worktree' ' + test_when_finished git branch -D base topic2 && + test_when_finished git checkout main && + test_when_finished git branch -D wt-topic && + test_when_finished git worktree remove wt-topic && + git checkout main && + git checkout -b base && + git checkout -b topic2 && + test_commit msg2 && + git worktree add wt-topic && + git checkout base && + test_commit msg3 && + git checkout topic2 && + GIT_SEQUENCE_EDITOR="cat >actual" git -c core.commentChar=% \ + rebase -i --update-refs base && + test_grep "% Ref refs/heads/wt-topic checked out at" actual && + test_grep "% Ref refs/heads/topic2 checked out at" actual +' + test_done From 515d034f8d922602b9d417cdc02768757aa6f6c1 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Mon, 25 Nov 2024 21:13:12 +0100 Subject: [PATCH 2/3] sequencer: comment `--reference` subject line properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git revert --reference ` 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 Signed-off-by: Junio C Hamano --- 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 1b6fd86f70bc4b..d26299cdea2be5 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 411027fb58c7df..b84fdfe8a321e4 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="head -n4 >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 && From 7e2f377b03b0d3593df73a094f97c27b219b02f5 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Mon, 25 Nov 2024 21:13:13 +0100 Subject: [PATCH 3/3] sequencer: comment commit messages properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rebase todo editor has commands like `fixup -c` which affects the commit messages of the rebased commits.[1] For example: pick hash1 fixup hash2 fixup -c hash3 This says that hash2 and hash3 should be squashed into hash1 and that hash3’s commit message should be used for the resulting commit. So the user is presented with an editor where the two first commit messages are commented out and the third is not. However this does not work if `core.commentChar`/`core.commentString` is in use since the comment char is hardcoded (#) in this `sequencer.c` function. As a result the first commit message will not be commented out. † 1: See 9e3cebd97cb (rebase -i: add fixup [-C | -c] command, 2021-01-29) Signed-off-by: Phillip Wood Co-authored-by: Phillip Wood Reported-by: Taylor Blau Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- sequencer.c | 12 ++++++++---- t/t3437-rebase-fixup-options.sh | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/sequencer.c b/sequencer.c index d26299cdea2be5..42a6f257cbb665 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1941,10 +1941,10 @@ static int seen_squash(struct replay_ctx *ctx) static void update_comment_bufs(struct strbuf *buf1, struct strbuf *buf2, int n) { - strbuf_setlen(buf1, 2); + strbuf_setlen(buf1, strlen(comment_line_str) + 1); strbuf_addf(buf1, _(nth_commit_msg_fmt), n); strbuf_addch(buf1, '\n'); - strbuf_setlen(buf2, 2); + strbuf_setlen(buf2, strlen(comment_line_str) + 1); strbuf_addf(buf2, _(skip_nth_commit_msg_fmt), n); strbuf_addch(buf2, '\n'); } @@ -1963,8 +1963,12 @@ static void update_squash_message_for_fixup(struct strbuf *msg) size_t orig_msg_len; int i = 1; - strbuf_addf(&buf1, "# %s\n", _(first_commit_msg_str)); - strbuf_addf(&buf2, "# %s\n", _(skip_first_commit_msg_str)); + strbuf_add_commented_lines(&buf1, _(first_commit_msg_str), + strlen(_(first_commit_msg_str)), + comment_line_str); + strbuf_add_commented_lines(&buf2, _(skip_first_commit_msg_str), + strlen(_(skip_first_commit_msg_str)), + comment_line_str); s = start = orig_msg = strbuf_detach(msg, &orig_msg_len); while (s) { const char *next; diff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh index 7929e2e2e3a8fc..a4b90e881e3a7b 100755 --- a/t/t3437-rebase-fixup-options.sh +++ b/t/t3437-rebase-fixup-options.sh @@ -127,6 +127,21 @@ test_expect_success 'fixup -C with conflicts gives correct message' ' test_cmp expected-author actual-author ' +test_expect_success 'conflicting fixup -C after fixup with custom comment string' ' + test_config core.commentString COMMENT && + test_when_finished "test_might_fail git rebase --abort" && + git checkout --detach A3 && + test_must_fail env FAKE_LINES="1 fixup 2 fixup_-C 4" git rebase -i A && + echo resolved >A && + git add A && + FAKE_COMMIT_AMEND=edited git rebase --continue && + test_commit_message HEAD <<-\EOF + A3 + + edited + EOF +' + test_expect_success 'skipping fixup -C after fixup gives correct message' ' test_when_finished "test_might_fail git rebase --abort" && git checkout --detach A3 &&