From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sun, 21 Apr 2024 15:41:23 GMT
Subject: Re: [PATCH v2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP
Message-ID: <440ee2b7-2ed8-4c8b-96e8-623ef45efe42@gmail.com>
In-Reply-To: <xmqqy1993tc1.fsf@gitster.g>

Hi Junio

On 19/04/2024 23:01, Junio C Hamano wrote:
> With the "--rfc" option, we can tweak the "[PATCH]" (or whatever
> string specified with the "--subject-prefix" option, instead of
> "PATCH") that we prefix the title of the commit with into "[RFC
> PATCH]", but some projects may want "[rfc PATCH]".  Adding a new
> option, e.g., "--rfc-lowercase", to support such need every time
> somebody wants to use different strings would lead to insanity of
> accumulating unbounded number of such options.
> 
> Allow an optional value specified for the option, so that users can
> use "--rfc=rfc" (think of "--rfc" without value as a short-hand for
> "--rfc=RFC") if they wanted to.
> 
> This can of course be (ab)used to make the prefix "[WIP PATCH]" by
> passing "--rfc=WIP".  Passing an empty string, i.e., "--rfc=", is
> the same as "--no-rfc" to override an option given earlier on the
> same command line.

The changes in this version all look good to me, I've left one comment 
below.

> @@ -1907,8 +1919,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>   	struct strbuf rdiff2 = STRBUF_INIT;
>   	struct strbuf rdiff_title = STRBUF_INIT;
>   	struct strbuf sprefix = STRBUF_INIT;
> +	struct strbuf rfc = STRBUF_INIT;

Looking at this again, do we really need an strbuf here? I think we 
could we use "const char *" instead as we always store a static string 
in it which would avoid a memory leak from not calling strbuf_release().

Best Wishes

Phillip