Re: [PATCH v3 1/2] sequencer: extract revert message formatting into shared function
- From
Siddharth Asthana <siddharthasthana31@gmail.com>
- Date
- Mar 6, 2026, 04:55 UTC
- Message-ID
- <272a8ce4-08a7-490d-901a-ca8b5e72eb41@gmail.com>
- In-Reply-To
- <xmqqcy1s8p81.fsf@gitster.g>
On 26/02/26 03:23, Junio C Hamano wrote:
Show 29 quoted lines
> Toon Claes <toon@iotcl.com> writes:
>
>>> - } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) &&
>>> - /*
>>> - * We don't touch pre-existing repeated reverts, because
>>> - * theoretically these can be nested arbitrarily deeply,
>>> - * thus requiring excessive complexity to deal with.
>>> - */
>>> - !starts_with(orig_subject, "Revert \"")) {
>>> - strbuf_addstr(&ctx->message, "Reapply \"");
>>> - strbuf_addstr(&ctx->message, orig_subject);
>>> - strbuf_addstr(&ctx->message, "\n");
>>> + strbuf_addstr(&ctx->message, "\nThis reverts commit ");
>>> } else {
>>> - strbuf_addstr(&ctx->message, "Revert \"");
>>> - strbuf_addstr(&ctx->message, msg.subject);
>>> - strbuf_addstr(&ctx->message, "\"\n");
>>> + sequencer_format_revert_header(&ctx->message, msg.subject, NULL);
>>> }
>>> - strbuf_addstr(&ctx->message, "\nThis reverts commit ");
>>> refer_to_commit(opts, &ctx->message, commit);
>>
>> I still find it somewhat confusing we have some the code that deals with
>> `opts->commit_use_reference` partly in here and partly in
>> sequencer_format_revert_header().
>
> True. Making sure plumbing commands are unaffected by random
> end-user configuration is a good thing, but I am not sure if this
> command is truly a plumbing.With Phillip's sequencer_format_revert_message() approach, replay just passes use_commit_reference=false and gets the full OID path. The split logic goes away, so this concern is resolved regardless of how we classify replay.
Show 7 quoted lines
> >> Part of the confusion comes from sequencer_format_revert_header() being >> called with NULL for the commit OID. >> >> Was is not possible to incorporate Patrick's suggestion[1]? >> >> [1]: https://lore.kernel.org/git/aTZ5RrjnwJ2ZnT7A@pks.im/