From: Siddharth Asthana Date: Fri, 06 Mar 2026 04:55:06 GMT Subject: Re: [PATCH v3 1/2] sequencer: extract revert message formatting into shared function Message-ID: <272a8ce4-08a7-490d-901a-ca8b5e72eb41@gmail.com> In-Reply-To: On 26/02/26 03:23, Junio C Hamano wrote: > Toon Claes 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. > >> 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/