kristofferhaugsbakk@xxxxxxxxxxxx writes:
> @@ -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 ***");
With the switch to "commented_addf", we'd terminate this line with
LF here, which means ...
> @@ -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");
... we'd want to terminate the line in these two other if/else if/else
arms for symmetry, so that ...
> }
> - strbuf_addstr(&ctx->message, "\n\nThis reverts commit ");
> + strbuf_addstr(&ctx->message, "\nThis reverts commit ");
... we can lose the termination of the previous line from here.
Makes sense.
> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
> index 411027fb58c..26d3cabb608 100755
> --- a/t/t3501-revert-cherry-pick.sh
> +++ b/t/t3501-revert-cherry-pick.sh
> @@ -228,6 +228,18 @@ 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 -c core.commentChar=% revert \
> + --edit --reference HEAD &&
> + git log -1 --format=%B HEAD >actual &&
> + printf "This reverts commit $(git show -s \
> + --pretty=reference HEAD^).\n\n" \
> + >expect &&
> + test_cmp expect actual
> +'
I guess this fails by leaving the "# *** SAY WHY" in the resulting
message, because the stripspace wants to see '%' to start commented
out lines to be stripped? If we inspect with this test what the
temporary file we give to the editor looks like to make sure that
'%' is used for commenting, that would be a more direct test, but
without going that far, at least can we have a comment describing
how this is expected to fail without the fix?
Thanks.
