Re: [PATCH v2 2/3] sequencer: comment `--reference` subject line properly
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 13, 2024, 01:07 UTC
- Message-ID
- <xmqqbjyk2b70.fsf@gitster.g>
- In-Reply-To
- <710c5b1a3f6bf8dc112ff13f27a8b2165274488d.1731406513.git.code@khaugsbakk.name>
kristofferhaugsbakk@fastmail.com writes:
Show 8 quoted lines
> @@ -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 ...
Show 10 quoted lines
> @@ -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.
Show 19 quoted lines
> 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.