Hi Tony On 31/10/2023 05:09, Tony Tung via GitGitGadget wrote:
From: Tony Tung <tonytung@xxxxxxxxx> Instead of using the hardcoded `# `, use the user-defined comment_line_char. Adds a test to prevent regressions.
Well spotted and thanks for fixing this. Normally we wrap the commit message at ~72 chars.
Signed-off-by: Tony Tung <tonytung@xxxxxxxxx> --- sequencer.c | 5 +++-- t/t3404-rebase-interactive.sh | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index d584cac8ed9..8c6666d5e43 100644 --- a/sequencer.c +++ b/sequencer.c @@ -6082,8 +6082,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_char, + "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/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 8ea2bf13026..076dca87871 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1839,6 +1839,45 @@ test_expect_success '--update-refs adds label and update-ref commands' ' ) '
Thank you for taking the time to add a test. I think it could be simplified though as all we really need to do is check that the expected comment is present in the todo list. Something like (untested)
test_expect_success '--update-refs works with core.commentChar' ' git worktree add new-branch && test_when_finished "git worktree remove new-branch" && test_config core.commentchar : && write_script fake-editor.sh <<-\EOF && grep "^: Ref refs/heads/new-branch checked out at .*new-branch" "$1" && # no need to rebase >"$1" EOF ( test_set_editor "$(pwd)/fake-editor.sh" && git rebase -i --update-refs HEAD^ ) ' Best Wishes Phillip
+test_expect_success '--update-refs works with core.commentChar' ' + git checkout -b update-refs-with-commentchar no-conflict-branch && + test_config core.commentChar : && + git branch -f base HEAD~4 && + git branch -f first HEAD~3 && + git branch -f second HEAD~3 && + git branch -f third HEAD~1 && + git commit --allow-empty --fixup=third && + git branch -f is-not-reordered && + git commit --allow-empty --fixup=HEAD~4 && + git branch -f shared-tip && + git checkout update-refs && + ( + write_script fake-editor.sh <<-\EOF && + grep "^[^:]" "$1" + exit 1 + EOF + test_set_editor "$(pwd)/fake-editor.sh" && + + cat >expect <<-EOF && + pick $(git log -1 --format=%h J) J + fixup $(git log -1 --format=%h update-refs) fixup! J : empty + update-ref refs/heads/second + update-ref refs/heads/first + pick $(git log -1 --format=%h K) K + pick $(git log -1 --format=%h L) L + fixup $(git log -1 --format=%h is-not-reordered) fixup! L : empty + update-ref refs/heads/third + pick $(git log -1 --format=%h M) M + update-ref refs/heads/no-conflict-branch + update-ref refs/heads/is-not-reordered + update-ref refs/heads/update-refs-with-commentchar + EOF + + test_must_fail git rebase -i --autosquash --update-refs primary shared-tip >todo && + test_cmp expect todo + ) +' + test_expect_success '--update-refs adds commands with --rebase-merges' ' git checkout -b update-refs-with-merge no-conflict-branch && git branch -f base HEAD~4 &&