Re: [PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Junio

On 18/04/2024 23:54, 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").
> 
> This can of course be (ab)used to make the prefix "[WIP PATCH]" by
> passing "--rfc=WIP".

I think being able to customize the subject prefix in this way is a good 
idea and makes it easy to say [RESEND PATCH program] with --rfc=RESEND 
when format.subjectPrefix is set. We could add a separate option to as 
"--rfc" already exists I think it is reasonable to extend it. (I'm also 
not a good name for such an option would be "--subject-prefix-prefix" 
kind of describes what it does but is far from ideal)

> -		   [--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>"

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

This reworded description is good

>   	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),

This is a change in behavior as it looks like we previously accepted 
"--no-rfc" is that deliberate?

Best Wishes

Phillip