From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 05 Dec 2025 11:33:23 GMT
Subject: Re: [PATCH v2 1/2] sequencer: extract revert message formatting into shared function
Message-ID: <aTLDA11AKs0jlxFJ@pks.im>
In-Reply-To: <20251202201611.22137-2-siddharthasthana31@gmail.com>

On Wed, Dec 03, 2025 at 01:46:10AM +0530, Siddharth Asthana wrote:
> diff --git a/sequencer.c b/sequencer.c
> index 5476d39ba9..9f621aef4b 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2365,22 +2365,10 @@ static int do_pick_commit(struct repository *r,
>  		if (opts->commit_use_reference) {
>  			strbuf_commented_addf(&ctx->message, comment_line_str,
>  				"*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
> -		} 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);
>  		}
> -		strbuf_addstr(&ctx->message, "\nThis reverts commit ");
>  		refer_to_commit(opts, &ctx->message, commit);
>  
>  		if (commit->parents && commit->parents->next) {

Is there any reason why we don't also handle `refer_to_commit()` in that
new function?

Patrick