From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 25 Feb 2026 21:53:18 GMT
Subject: Re: [PATCH v3 1/2] sequencer: extract revert message formatting into shared function
Message-ID: <xmqqcy1s8p81.fsf@gitster.g>
In-Reply-To: <87wm07e4ck.fsf@iotcl.com>

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.

> 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/