patchformat-patch: allow --rfc to optionally take a value, like --rfc=WIP

> 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").
> 
> This can of course be (ab)used to make the prefix "[WIP PATCH]" by
> passing "--rfc=WIP".

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  Documentation/git-format-patch.txt | 15 ++++++++++-----
>  builtin/log.c                      | 22 ++++++++++++++++++----
>  t/t4014-format-patch.sh            |  9 +++++++++
>  3 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/git-format-patch.txt
> b/Documentation/git-format-patch.txt
> index 728bb3821c..8d634f5b1b 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -20,7 +20,7 @@ SYNOPSIS
>  		   [--in-reply-to=<message-id>] [--suffix=.<sfx>]
>  		   [--ignore-if-in-upstream] [--always]
>  		   [--cover-from-description=<mode>]
> -		   [--rfc] [--subject-prefix=<subject-prefix>]
> +		   [--rfc[=<rfc>]] [--subject-prefix=<subject-prefix>]
>  		   [(--reroll-count|-v) <n>]
>  		   [--to=<email>] [--cc=<email>]
>  		   [--[no-]cover-letter] [--quiet]
> @@ -238,10 +238,15 @@ the patches (with a value of e.g. "PATCH 
> my-project").
>  	value of the `format.filenameMaxLength` configuration
>  	variable, or 64 if unconfigured.
> 
> ---rfc::
> -	Prepends "RFC" to the subject prefix (producing "RFC PATCH" by
> -	default). RFC means "Request For Comments"; use this when sending
> -	an experimental patch for discussion rather than application.
> +--rfc[=<rfc>]::
> +	Prepends the string _<rfc>_ (defaults to "RFC") to
> +	the subject prefix.  As the subject prefix defaults to
> +	"PATCH", you'll get "RFC PATCH" by default.
> ++
> +RFC means "Request For Comments"; use this when sending
> +an experimental patch for discussion rather than application.
> +"--rfc=WIP" may also be a useful way to indicate that a patch
> +is not complete yet.
> 
>  -v <n>::
>  --reroll-count=<n>::
> diff --git a/builtin/log.c b/builtin/log.c
> index c0a8bb95e9..2d6e0f3688 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1494,6 +1494,18 @@ static int subject_prefix_callback(const struct
> option *opt, const char *arg,
>  	return 0;
>  }
> 
> +static int rfc_callback(const struct option *opt, const char *arg,
> +			int unset)
> +{
> +	struct strbuf *rfc;
> +
> +	BUG_ON_OPT_NEG(unset);
> +	rfc = opt->value;
> +	strbuf_reset(rfc);
> +	strbuf_addstr(rfc, arg ? arg : "RFC");
> +	return 0;
> +}

> +
>  static int numbered_cmdline_opt = 0;
> 
>  static int numbered_callback(const struct option *opt, const char 
> *arg,
> @@ -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;
>  	int creation_factor = -1;
> -	int rfc = 0;
> 
>  	const struct option builtin_format_patch_options[] = {
>  		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
> @@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char
> **argv, const char *prefix)
>  			    N_("mark the series as Nth re-roll")),
>  		OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max,
>  			    N_("max length of output filename")),
> -		OPT_BOOL(0, "rfc", &rfc, N_("use [RFC PATCH] instead of [PATCH]")),
> +		OPT_CALLBACK_F(0, "rfc", &rfc, N_("extra"),
> +			       N_("add <extra> (default 'RFC') before 'PATCH'"),
> +			       PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),
>  		OPT_STRING(0, "cover-from-description", &cover_from_description_arg,
>  			    N_("cover-from-description-mode"),
>  			    N_("generate parts of a cover letter based on a branch's 
> description")),
> @@ -2050,8 +2064,8 @@ int cmd_format_patch(int argc, const char
> **argv, const char *prefix)
>  	if (cover_from_description_arg)
>  		cover_from_description_mode =
> parse_cover_from_description(cover_from_description_arg);
> 
> -	if (rfc)
> -		strbuf_insertstr(&sprefix, 0, "RFC ");
> +	if (rfc.len)
> +		strbuf_insertf(&sprefix, 0, "%s ", rfc.buf);
> 
>  	if (reroll_count) {
>  		strbuf_addf(&sprefix, " v%s", reroll_count);
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index e37a1411ee..905858da35 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1377,6 +1377,15 @@ test_expect_success '--rfc' '
>  	test_cmp expect actual
>  '
> 
> +test_expect_success '--rfc=WIP' '
> +	cat >expect <<-\EOF &&
> +	Subject: [WIP PATCH 1/1] header with . in it
> +	EOF
> +	git format-patch -n -1 --stdout --rfc=WIP >patch &&
> +	grep "^Subject:" patch >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success '--rfc does not overwrite prefix' '
>  	cat >expect <<-\EOF &&
>  	Subject: [RFC PATCH foobar 1/1] header with . in it

> 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").
> 
> This can of course be (ab)used to make the prefix "[WIP PATCH]" by
> passing "--rfc=WIP".

> ---rfc::
> -	Prepends "RFC" to the subject prefix (producing "RFC PATCH" by
> -	default). RFC means "Request For Comments"; use this when sending
> -	an experimental patch for discussion rather than application.
> +--rfc[=<rfc>]::
> +	Prepends the string _<rfc>_ (defaults to "RFC") to
> +	the subject prefix.  As the subject prefix defaults to
> +	"PATCH", you'll get "RFC PATCH" by default.
> ++
> +RFC means "Request For Comments"; use this when sending
> +an experimental patch for discussion rather than application.
> +"--rfc=WIP" may also be a useful way to indicate that a patch
> +is not complete yet.

>   	const struct option builtin_format_patch_options[] = {
>   		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
> @@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>   			    N_("mark the series as Nth re-roll")),
>   		OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max,
>   			    N_("max length of output filename")),
> -		OPT_BOOL(0, "rfc", &rfc, N_("use [RFC PATCH] instead of [PATCH]")),
> +		OPT_CALLBACK_F(0, "rfc", &rfc, N_("extra"),
> +			       N_("add <extra> (default 'RFC') before 'PATCH'"),
> +			       PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),

>> -		   [--rfc] [--subject-prefix=<subject-prefix>]
>> +		   [--rfc[=<rfc>]] [--subject-prefix=<subject-prefix>]
>
> Nit: in the documentation we use "<rfc>" for the placeholder but in
> the code we use "<extra>"

>> @@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>>   			    N_("mark the series as Nth re-roll")),
>>   		OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max,
>>   			    N_("max length of output filename")),
>> -		OPT_BOOL(0, "rfc", &rfc, N_("use [RFC PATCH] instead of [PATCH]")),
>> +		OPT_CALLBACK_F(0, "rfc", &rfc, N_("extra"),
>> +			       N_("add <extra> (default 'RFC') before 'PATCH'"),
>> +			       PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),
>
> This is a change in behavior as it looks like we previously accepted
> "--no-rfc" is that deliberate?

> Phillip Wood <phillip.wood123@gmail.com> writes:
>>> @@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char 
>>> **argv, const char *prefix)
>>>   			    N_("mark the series as Nth re-roll")),
>>>   		OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max,
>>>   			    N_("max length of output filename")),
>>> -		OPT_BOOL(0, "rfc", &rfc, N_("use [RFC PATCH] instead of 
>>> [PATCH]")),
>>> +		OPT_CALLBACK_F(0, "rfc", &rfc, N_("extra"),
>>> +			       N_("add <extra> (default 'RFC') before 'PATCH'"),
>>> +			       PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),
>> 
>> This is a change in behavior as it looks like we previously accepted
>> "--no-rfc" is that deliberate?
> 
> I just matched the subject-prefix without thinking.  Will fix.
> 
> Here is what I plan to squash in, but we are about to enter the
> pre-release stabilization period, so the progress on this new
> feature will have to slow down.

> ---rfc::
> -	Prepends "RFC" to the subject prefix (producing "RFC PATCH" by
> -	default). RFC means "Request For Comments"; use this when sending
> -	an experimental patch for discussion rather than application.
> +--rfc[=<rfc>]::
> +	Prepends the string _<rfc>_ (defaults to "RFC") to
> +	the subject prefix.  As the subject prefix defaults to
> +	"PATCH", you'll get "RFC PATCH" by default.
> ++
> +RFC means "Request For Comments"; use this when sending
> +an experimental patch for discussion rather than application.
> +"--rfc=WIP" may also be a useful way to indicate that a patch
> +is not complete yet.

> On Thu, Apr 18, 2024 at 03:54:25PM -0700, Junio C Hamano wrote:
>
>> ---rfc::
>> -	Prepends "RFC" to the subject prefix (producing "RFC PATCH" by
>> -	default). RFC means "Request For Comments"; use this when sending
>> -	an experimental patch for discussion rather than application.
>> +--rfc[=<rfc>]::
>> +	Prepends the string _<rfc>_ (defaults to "RFC") to
>> +	the subject prefix.  As the subject prefix defaults to
>> +	"PATCH", you'll get "RFC PATCH" by default.
>> ++
>> +RFC means "Request For Comments"; use this when sending
>> +an experimental patch for discussion rather than application.
>> +"--rfc=WIP" may also be a useful way to indicate that a patch
>> +is not complete yet.
>
> It's probably worth spelling out WIP here, too, like:
>
>   ...is not complete yet ("WIP" stands for "Work In Progress").
>
> These are necessarily going to be project-specific phrases, and readers
> of the manpage may not be familiar with our conventions.

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

> @@ -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;

>> +	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().

> In the previous step, the "--rfc" option of "format-patch" learned
> to take an optional string value to prepend to the subject prefix,
> so that --rfc=WIP can give "[WIP PATCH]".  This commit shows that
> the mechanism can be extended easily to allow "--rfc=-(WIP)" [*1*]
> to signal that the extra string is to be appended instead of getting
> prepended, resulting in "[PATCH (WIP)]".
> 
> Having worked on the patch, I am personally not 100% on board with
> this part of the feature myself, and that is why this update is a
> separate step from the main "--rfc takes an optional string value"
> step.
> 
> If a way to prepend an arbitrary adornment is added to the system,
> and people can now say "--rfc=RESEND" to produce "[RESEND PATCH]",
> those who used to use "[PATCH RESEND]" by tweaking the subject
> prefix in other ways [*2*] would do one of three things:

>  (1) keep using their existing ways and keep sending their message
>      with the "[RESEND PATCH]" prefix.
> 
>  (2) rejoice in the new automation, switch to use "--rfc=RESEND",
>      and start sending their messages with "[RESEND PATCH]" prefix
>      instead of "[PATCH RESEND]" prefix.
> 
>  (3) complain and demand a way to append instead of prepend so that
>      they can use an automation to produce "[PATCH RESEND]" prefix.
> 
> I do not believe in adding slightly different ways that allow users
> to be "original" when such differences do not make the world better
> in meaningful ways [*3*], and I expect there are many more folks who
> share that sentiment and go to route (2) than those who go to route
> (3).  If my expectation is true, it means that this patch goes in a
> wrong direction, and I would be happy to drop it.
> 
> 
> [Footnote]
> 
>  *1* The syntax takes inspiration from Perl's three-arg open syntax
>      that uses pipes "open fh, '|-', 'cmd'", where the dash signals
>      "the other stuff comes here".
> 
>  *2* ... because "--rfc" in released versions does not even take any
>      string value to prepend, let alone append, to begin with.
> 
>  *3* 
> https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/
>      gathered some stats to observe that "[RFC PATCH]" is more
>      common than "[PATCH RFC]" by a wide margin, while trying to see
>      how common "[RESEND PATCH]" (or "[PATCH RESED]") were used (the
>      answer: much less common).  But it wouldn't have needed to
>      count "[PATCH RFC]" and "[RFC PATCH]" separately if using a
>      prefix and not a suffix (or vice versa) were established more
>      firmly as the standard practice.
> 
>      It is a fine example that useless diversity making needless
>      work.

> Hello Junio,
> 
> On 2024-04-21 20:59, Junio C Hamano wrote:
>> In the previous step, the "--rfc" option of "format-patch" learned
>> to take an optional string value to prepend to the subject prefix,
>> so that --rfc=WIP can give "[WIP PATCH]".  This commit shows that
>> the mechanism can be extended easily to allow "--rfc=-(WIP)" [*1*]
>> to signal that the extra string is to be appended instead of getting
>> prepended, resulting in "[PATCH (WIP)]".
>>
>> Having worked on the patch, I am personally not 100% on board with
>> this part of the feature myself, and that is why this update is a
>> separate step from the main "--rfc takes an optional string value"
>> step.
>>
>> If a way to prepend an arbitrary adornment is added to the system,
>> and people can now say "--rfc=RESEND" to produce "[RESEND PATCH]",
>> those who used to use "[PATCH RESEND]" by tweaking the subject
>> prefix in other ways [*2*] would do one of three things:
> 
> There are even more issues, such as the grammar-related ones. 

> me explain, please, as accurately as I can do that as a non-native
> English speaker who spent many years studying English grammar.
> 
> Writing "RFC PATCH" puts "RFC" into the role of an adjective, which
> is fine.  The same obviously applies to "WIP PATCH".  On the other
> hand, writing "RESEND PATCH" puts "RESEND" into its only possible
> role, which is transitive verb, and results in "RESEND PATCH" that
> serves as a very simple sentence in imperative mood.  I'm not sure
> that, strictly technically speaking, having simple sentences as the
> prefixes is the desired outcome.
> 
> Technically, we should use "RE-SENT PATCH" to be on the correct side
> from the grammar perspective, with "RE-SENT" serving as an adjective.
> Before you ask, no, we can't use "RESENT" there, because its meaning
> is completely different.  However, nobody uses "RE-SENT PATCH", or
> at least I haven't seen it used yet.
> 
> When it comes to "PATCH RESEND", "RESEND" remains in its transitive
> verb role, but "PATCH", as a noun, becomes a modifier of the verb.
> Thus, the resulting meaning of "PATCH RESEND" becomes something like
> "resend an item that's a patch", but not written in form of a simple
> (or less simple) sentence.  Strictly technically, a noun should only
> modify another noun, but bending English grammar a bit this way is
> much better than actually having a simple sentence as a prefix.
> 
> With all this in mind, I think that allowing the "--rfc=-<string>"
> form is the way to go.  Computer lingo often bends English grammar
> to a certain degree, to achieve compactness, but bending and dumbing
> it down more that it's actually necessary isn't something that we
> should embrace.
> 
> I hope all this makes sense.
> 
>>  (1) keep using their existing ways and keep sending their message
>>      with the "[RESEND PATCH]" prefix.
>>
>>  (2) rejoice in the new automation, switch to use "--rfc=RESEND",
>>      and start sending their messages with "[RESEND PATCH]" prefix
>>      instead of "[PATCH RESEND]" prefix.
>>
>>  (3) complain and demand a way to append instead of prepend so that
>>      they can use an automation to produce "[PATCH RESEND]" prefix.
>>
>> I do not believe in adding slightly different ways that allow users
>> to be "original" when such differences do not make the world better
>> in meaningful ways [*3*], and I expect there are many more folks who
>> share that sentiment and go to route (2) than those who go to route
>> (3).  If my expectation is true, it means that this patch goes in a
>> wrong direction, and I would be happy to drop it.
>>
>>
>> [Footnote]
>>
>>  *1* The syntax takes inspiration from Perl's three-arg open syntax
>>      that uses pipes "open fh, '|-', 'cmd'", where the dash signals
>>      "the other stuff comes here".
>>
>>  *2* ... because "--rfc" in released versions does not even take any
>>      string value to prepend, let alone append, to begin with.
>>
>>  *3* 
>> https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/
>>      gathered some stats to observe that "[RFC PATCH]" is more
>>      common than "[PATCH RFC]" by a wide margin, while trying to see
>>      how common "[RESEND PATCH]" (or "[PATCH RESED]") were used (the
>>      answer: much less common).  But it wouldn't have needed to
>>      count "[PATCH RFC]" and "[RFC PATCH]" separately if using a
>>      prefix and not a suffix (or vice versa) were established more
>>      firmly as the standard practice.
>>
>>      It is a fine example that useless diversity making needless
>>      work.

> On 21/04/2024 20:37, Dragan Simic wrote:
>> On 2024-04-21 20:59, Junio C Hamano wrote:
>>> In the previous step, the "--rfc" option of "format-patch" learned
>>> to take an optional string value to prepend to the subject prefix,
>>> so that --rfc=WIP can give "[WIP PATCH]".  This commit shows that
>>> the mechanism can be extended easily to allow "--rfc=-(WIP)" [*1*]
>>> to signal that the extra string is to be appended instead of getting
>>> prepended, resulting in "[PATCH (WIP)]".
>>> 
>>> Having worked on the patch, I am personally not 100% on board with
>>> this part of the feature myself, and that is why this update is a
>>> separate step from the main "--rfc takes an optional string value"
>>> step.
>>> 
>>> If a way to prepend an arbitrary adornment is added to the system,
>>> and people can now say "--rfc=RESEND" to produce "[RESEND PATCH]",
>>> those who used to use "[PATCH RESEND]" by tweaking the subject
>>> prefix in other ways [*2*] would do one of three things:
>> 
>> There are even more issues, such as the grammar-related ones.
> 
> I think it is best to view the subject prefix as a list of space
> separated labels or keywords rather than part of a grammatically
> correct sentence.

>  Let
>> me explain, please, as accurately as I can do that as a non-native
>> English speaker who spent many years studying English grammar.
>> 
>> Writing "RFC PATCH" puts "RFC" into the role of an adjective, which
>> is fine.  The same obviously applies to "WIP PATCH".  On the other
>> hand, writing "RESEND PATCH" puts "RESEND" into its only possible
>> role, which is transitive verb, and results in "RESEND PATCH" that
>> serves as a very simple sentence in imperative mood.  I'm not sure
>> that, strictly technically speaking, having simple sentences as the
>> prefixes is the desired outcome.
>> 
>> Technically, we should use "RE-SENT PATCH" to be on the correct side
>> from the grammar perspective, with "RE-SENT" serving as an adjective.
>> Before you ask, no, we can't use "RESENT" there, because its meaning
>> is completely different.  However, nobody uses "RE-SENT PATCH", or
>> at least I haven't seen it used yet.
>> 
>> When it comes to "PATCH RESEND", "RESEND" remains in its transitive
>> verb role, but "PATCH", as a noun, becomes a modifier of the verb.
>> Thus, the resulting meaning of "PATCH RESEND" becomes something like
>> "resend an item that's a patch", but not written in form of a simple
>> (or less simple) sentence.  Strictly technically, a noun should only
>> modify another noun, but bending English grammar a bit this way is
>> much better than actually having a simple sentence as a prefix.
>> 
>> With all this in mind, I think that allowing the "--rfc=-<string>"
>> form is the way to go.  Computer lingo often bends English grammar
>> to a certain degree, to achieve compactness, but bending and dumbing
>> it down more that it's actually necessary isn't something that we
>> should embrace.
>> 
>> I hope all this makes sense.
>> 
>>>  (1) keep using their existing ways and keep sending their message
>>>      with the "[RESEND PATCH]" prefix.
>>> 
>>>  (2) rejoice in the new automation, switch to use "--rfc=RESEND",
>>>      and start sending their messages with "[RESEND PATCH]" prefix
>>>      instead of "[PATCH RESEND]" prefix.
>>> 
>>>  (3) complain and demand a way to append instead of prepend so that
>>>      they can use an automation to produce "[PATCH RESEND]" prefix.
>>> 
>>> I do not believe in adding slightly different ways that allow users
>>> to be "original" when such differences do not make the world better
>>> in meaningful ways [*3*], and I expect there are many more folks who
>>> share that sentiment and go to route (2) than those who go to route
>>> (3).  If my expectation is true, it means that this patch goes in a
>>> wrong direction, and I would be happy to drop it.
>>> 
>>> 
>>> [Footnote]
>>> 
>>>  *1* The syntax takes inspiration from Perl's three-arg open syntax
>>>      that uses pipes "open fh, '|-', 'cmd'", where the dash signals
>>>      "the other stuff comes here".
>>> 
>>>  *2* ... because "--rfc" in released versions does not even take any
>>>      string value to prepend, let alone append, to begin with.
>>> 
>>>  *3* 
>>> https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/
>>>      gathered some stats to observe that "[RFC PATCH]" is more
>>>      common than "[PATCH RFC]" by a wide margin, while trying to see
>>>      how common "[RESEND PATCH]" (or "[PATCH RESED]") were used (the
>>>      answer: much less common).  But it wouldn't have needed to
>>>      count "[PATCH RFC]" and "[RFC PATCH]" separately if using a
>>>      prefix and not a suffix (or vice versa) were established more
>>>      firmly as the standard practice.
>>> 
>>>      It is a fine example that useless diversity making needless
>>>      work.

> 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.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>   Documentation/git-format-patch.txt | 15 ++++++++++-----
>   builtin/log.c                      | 23 +++++++++++++++++++----
>   t/t4014-format-patch.sh            | 21 +++++++++++++++++++--
>   3 files changed, 48 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 728bb3821c..e553810b1e 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -20,7 +20,7 @@ SYNOPSIS
>   		   [--in-reply-to=<message-id>] [--suffix=.<sfx>]
>   		   [--ignore-if-in-upstream] [--always]
>   		   [--cover-from-description=<mode>]
> -		   [--rfc] [--subject-prefix=<subject-prefix>]
> +		   [--rfc[=<rfc>]] [--subject-prefix=<subject-prefix>]
>   		   [(--reroll-count|-v) <n>]
>   		   [--to=<email>] [--cc=<email>]
>   		   [--[no-]cover-letter] [--quiet]
> @@ -238,10 +238,15 @@ the patches (with a value of e.g. "PATCH my-project").
>   	value of the `format.filenameMaxLength` configuration
>   	variable, or 64 if unconfigured.
>   
> ---rfc::
> -	Prepends "RFC" to the subject prefix (producing "RFC PATCH" by
> -	default). RFC means "Request For Comments"; use this when sending
> -	an experimental patch for discussion rather than application.
> +--rfc[=<rfc>]::
> +	Prepends the string _<rfc>_ (defaults to "RFC") to
> +	the subject prefix.  As the subject prefix defaults to
> +	"PATCH", you'll get "RFC PATCH" by default.
> ++
> +RFC means "Request For Comments"; use this when sending
> +an experimental patch for discussion rather than application.
> +"--rfc=WIP" may also be a useful way to indicate that a patch
> +is not complete yet ("WIP" stands for "Work In Progress").
>   
>   -v <n>::
>   --reroll-count=<n>::
> diff --git a/builtin/log.c b/builtin/log.c
> index c0a8bb95e9..97ca885b33 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1494,6 +1494,19 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,
>   	return 0;
>   }
>   
> +static int rfc_callback(const struct option *opt, const char *arg,
> +			int unset)
> +{
> +	const char **rfc = opt->value;
> +
> +	*rfc = opt->value;
> +	if (unset)
> +		*rfc = NULL;
> +	else
> +		*rfc = arg ? arg : "RFC";
> +	return 0;
> +}
> +
>   static int numbered_cmdline_opt = 0;
>   
>   static int numbered_callback(const struct option *opt, const char *arg,
> @@ -1907,8 +1920,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;
> +	const char *rfc = NULL;
>   	int creation_factor = -1;
> -	int rfc = 0;
>   
>   	const struct option builtin_format_patch_options[] = {
>   		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
> @@ -1932,7 +1945,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>   			    N_("mark the series as Nth re-roll")),
>   		OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max,
>   			    N_("max length of output filename")),
> -		OPT_BOOL(0, "rfc", &rfc, N_("use [RFC PATCH] instead of [PATCH]")),
> +		OPT_CALLBACK_F(0, "rfc", &rfc, N_("rfc"),
> +			       N_("add <rfc> (default 'RFC') before 'PATCH'"),
> +			       PARSE_OPT_OPTARG, rfc_callback),
>   		OPT_STRING(0, "cover-from-description", &cover_from_description_arg,
>   			    N_("cover-from-description-mode"),
>   			    N_("generate parts of a cover letter based on a branch's description")),
> @@ -2050,8 +2065,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>   	if (cover_from_description_arg)
>   		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
>   
> -	if (rfc)
> -		strbuf_insertstr(&sprefix, 0, "RFC ");
> +	if (rfc && rfc[0])
> +		strbuf_insertf(&sprefix, 0, "%s ", rfc);
>   
>   	if (reroll_count) {
>   		strbuf_addf(&sprefix, " v%s", reroll_count);
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index e37a1411ee..645c4189f9 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1368,13 +1368,30 @@ test_expect_success 'empty subject prefix does not have extra space' '
>   	test_cmp expect actual
>   '
>   
> -test_expect_success '--rfc' '
> +test_expect_success '--rfc and --no-rfc' '
>   	cat >expect <<-\EOF &&
>   	Subject: [RFC PATCH 1/1] header with . in it
>   	EOF
>   	git format-patch -n -1 --stdout --rfc >patch &&
>   	grep "^Subject:" patch >actual &&
> -	test_cmp expect actual
> +	test_cmp expect actual &&
> +	git format-patch -n -1 --stdout --rfc --no-rfc >patch &&
> +	sed -e "s/RFC //" expect >expect-raw &&
> +	grep "^Subject:" patch >actual &&
> +	test_cmp expect-raw actual
> +'
> +
> +test_expect_success '--rfc=WIP and --rfc=' '
> +	cat >expect <<-\EOF &&
> +	Subject: [WIP PATCH 1/1] header with . in it
> +	EOF
> +	git format-patch -n -1 --stdout --rfc=WIP >patch &&
> +	grep "^Subject:" patch >actual &&
> +	test_cmp expect actual &&
> +	git format-patch -n -1 --stdout --rfc --rfc= >patch &&
> +	sed -e "s/WIP //" expect >expect-raw &&
> +	grep "^Subject:" patch >actual &&
> +	test_cmp expect-raw actual
>   '
>   
>   test_expect_success '--rfc does not overwrite prefix' '

> In the previous step, the "--rfc" option of "format-patch" learned
> to take an optional string value to prepend to the subject prefix,
> so that --rfc=WIP can give "[WIP PATCH]".
> 
> There may be cases in which the extra string wants to come after the
> subject prefix.  Extend the mechanism to allow "--rfc=-(WIP)" [*] to
> signal that the extra string is to be appended instead of getting
> prepended, resulting in "[PATCH (WIP)]".
> 
> In the documentation, discourage (ab)using "--rfc=-RFC" to say
> "[PATCH RFC]" just to be different, when "[RFC PATCH]" is the norm.
> 
> [Footnote]
> 
>   * The syntax takes inspiration from Perl's open syntax that opens
>     pipes "open fh, '|-', 'cmd'", where the dash signals "the other
>     stuff comes here".

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>   Documentation/git-format-patch.txt | 6 ++++++
>   builtin/log.c                      | 8 ++++++--
>   t/t4014-format-patch.sh            | 9 +++++++++
>   3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index e553810b1e..369af2c4a7 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -247,6 +247,12 @@ RFC means "Request For Comments"; use this when sending
>   an experimental patch for discussion rather than application.
>   "--rfc=WIP" may also be a useful way to indicate that a patch
>   is not complete yet ("WIP" stands for "Work In Progress").
> ++
> +If the convention of the receiving community for a particular extra
> +string is to have it _after_ the subject prefix, the string _<rfc>_
> +can be prefixed with a dash ("`-`") to signal that the the rest of
> +the _<rfc>_ string should be appended to the subject prefix instead,
> +e.g., `--rfc='-(WIP)'` results in "PATCH (WIP)".
>   
>   -v <n>::
>   --reroll-count=<n>::
> diff --git a/builtin/log.c b/builtin/log.c
> index 97ca885b33..4750e480e6 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -2065,8 +2065,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>   	if (cover_from_description_arg)
>   		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
>   
> -	if (rfc && rfc[0])
> -		strbuf_insertf(&sprefix, 0, "%s ", rfc);
> +	if (rfc && rfc[0]) {
> +		if (rfc[0] == '-')
> +			strbuf_addf(&sprefix, " %s", rfc + 1);
> +		else
> +			strbuf_insertf(&sprefix, 0, "%s ", rfc);
> +	}
>   
>   	if (reroll_count) {
>   		strbuf_addf(&sprefix, " v%s", reroll_count);
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 645c4189f9..fcbde15b16 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1394,6 +1394,15 @@ test_expect_success '--rfc=WIP and --rfc=' '
>   	test_cmp expect-raw actual
>   '
>   
> +test_expect_success '--rfc=-(WIP) appends' '
> +	cat >expect <<-\EOF &&
> +	Subject: [PATCH (WIP) 1/1] header with . in it
> +	EOF
> +	git format-patch -n -1 --stdout --rfc="-(WIP)" >patch &&
> +	grep "^Subject:" patch >actual &&
> +	test_cmp expect actual
> +'
> +
>   test_expect_success '--rfc does not overwrite prefix' '
>   	cat >expect <<-\EOF &&
>   	Subject: [RFC PATCH foobar 1/1] header with . in it

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> I'm not convinced this is a good idea as I'm not sure how adding "RFC"
>> at the end of the subject prefix makes the world better than just
>> having at the start of the prefix and I find using "-" to do that
>> quite confusing.
> 
> I am not convinced it is a good idea, either.  "PATCH (WIP)" was the
> best example I could come up with.  I am also a fan of "a list of
> space separated labels or keywords" you mentioned, but *if* a
> project convention somewhere is to have them before "PATCH", then it
> is not entirely unreasonable to wish to have a way to prepend these
> labels.
> 
> But I am fine to drop it for the sake of simplicity.  It would help
> discourage users from trying to be "original" in a way that does not
> make a material difference.  If a project comes with a concrete need
> to prepend, the patch is always resurrectable from the list archive.

> On 23/04/2024 18:52, Junio C Hamano wrote:
>> In the previous step, the "--rfc" option of "format-patch" learned
>> to take an optional string value to prepend to the subject prefix,
>> so that --rfc=WIP can give "[WIP PATCH]".
>> 
>> There may be cases in which the extra string wants to come after the
>> subject prefix.  Extend the mechanism to allow "--rfc=-(WIP)" [*] to
>> signal that the extra string is to be appended instead of getting
>> prepended, resulting in "[PATCH (WIP)]".
>> 
>> In the documentation, discourage (ab)using "--rfc=-RFC" to say
>> "[PATCH RFC]" just to be different, when "[RFC PATCH]" is the norm.
>> 
>> [Footnote]
>> 
>>   * The syntax takes inspiration from Perl's open syntax that opens
>>     pipes "open fh, '|-', 'cmd'", where the dash signals "the other
>>     stuff comes here".
> 
> I'm not convinced this is a good idea as I'm not sure how adding "RFC"
> at the end of the subject prefix makes the world better than just
> having at the start of the prefix and I find using "-" to do that
> quite confusing.

>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>   Documentation/git-format-patch.txt | 6 ++++++
>>   builtin/log.c                      | 8 ++++++--
>>   t/t4014-format-patch.sh            | 9 +++++++++
>>   3 files changed, 21 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/git-format-patch.txt 
>> b/Documentation/git-format-patch.txt
>> index e553810b1e..369af2c4a7 100644
>> --- a/Documentation/git-format-patch.txt
>> +++ b/Documentation/git-format-patch.txt
>> @@ -247,6 +247,12 @@ RFC means "Request For Comments"; use this when 
>> sending
>>   an experimental patch for discussion rather than application.
>>   "--rfc=WIP" may also be a useful way to indicate that a patch
>>   is not complete yet ("WIP" stands for "Work In Progress").
>> ++
>> +If the convention of the receiving community for a particular extra
>> +string is to have it _after_ the subject prefix, the string _<rfc>_
>> +can be prefixed with a dash ("`-`") to signal that the the rest of
>> +the _<rfc>_ string should be appended to the subject prefix instead,
>> +e.g., `--rfc='-(WIP)'` results in "PATCH (WIP)".
>>     -v <n>::
>>   --reroll-count=<n>::
>> diff --git a/builtin/log.c b/builtin/log.c
>> index 97ca885b33..4750e480e6 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -2065,8 +2065,12 @@ int cmd_format_patch(int argc, const char 
>> **argv, const char *prefix)
>>   	if (cover_from_description_arg)
>>   		cover_from_description_mode = 
>> parse_cover_from_description(cover_from_description_arg);
>>   -	if (rfc && rfc[0])
>> -		strbuf_insertf(&sprefix, 0, "%s ", rfc);
>> +	if (rfc && rfc[0]) {
>> +		if (rfc[0] == '-')
>> +			strbuf_addf(&sprefix, " %s", rfc + 1);
>> +		else
>> +			strbuf_insertf(&sprefix, 0, "%s ", rfc);
>> +	}
>>     	if (reroll_count) {
>>   		strbuf_addf(&sprefix, " v%s", reroll_count);
>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>> index 645c4189f9..fcbde15b16 100755
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -1394,6 +1394,15 @@ test_expect_success '--rfc=WIP and --rfc=' '
>>   	test_cmp expect-raw actual
>>   '
>>   +test_expect_success '--rfc=-(WIP) appends' '
>> +	cat >expect <<-\EOF &&
>> +	Subject: [PATCH (WIP) 1/1] header with . in it
>> +	EOF
>> +	git format-patch -n -1 --stdout --rfc="-(WIP)" >patch &&
>> +	grep "^Subject:" patch >actual &&
>> +	test_cmp expect actual
>> +'
>> +
>>   test_expect_success '--rfc does not overwrite prefix' '
>>   	cat >expect <<-\EOF &&
>>   	Subject: [RFC PATCH foobar 1/1] header with . in it