Re: [PATCH v2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Apr 21, 2024, 15:41 UTC
- 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:
Show 16 quoted lines
> 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.
Show 5 quoted lines
> @@ -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