{"thread":{"id":"61335","subject":"[PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","startedAt":"2024-04-18T22:54:28Z","lastAt":"2024-04-24T16:34:15Z","messageCount":24,"participants":["Junio C Hamano","Dragan Simic","Phillip Wood","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"493173","messageId":"xmqqzftqnuxq.fsf@gitster.g","threadId":"61335","inReplyTo":null,"subject":"[PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-18T22:54:25Z","receivedAt":"2024-04-18T22:54:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the \"--rfc\" option, we can tweak the \"[PATCH]\" (or whatever\nstring specified with the \"--subject-prefix\" option, instead of\n\"PATCH\") that we prefix the title of the commit with into \"[RFC\nPATCH]\", but some projects may want \"[rfc PATCH]\". Adding a new\noption, e.g., \"--rfc-lowercase\", to support such need every time\nsomebody wants to use different strings would lead to insanity of\naccumulating unbounded number of such options.\n\nAllow an optional value specified for the option, so that users can\nuse \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n\"--rfc=RFC\").\n\nThis can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\npassing \"--rfc=WIP\".\n\nSigned-off-by: Junio C Hamano \n---\n\n Documentation/git-format-patch.txt | 15 ++++++++++-----\n builtin/log.c | 22 ++++++++++++++++++----\n t/t4014-format-patch.sh | 9 +++++++++\n 3 files changed, 37 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 728bb3821c..8d634f5b1b 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,7 +20,7 @@ SYNOPSIS\n \t\t [--in-reply-to=] [--suffix=.]\n \t\t [--ignore-if-in-upstream] [--always]\n \t\t [--cover-from-description=]\n-\t\t [--rfc] [--subject-prefix=]\n+\t\t [--rfc[=]] [--subject-prefix=]\n \t\t [(--reroll-count|-v) ]\n \t\t [--to=] [--cc=]\n \t\t [--[no-]cover-letter] [--quiet]\n@@ -238,10 +238,15 @@ the patches (with a value of e.g. \"PATCH my-project\").\n \tvalue of the `format.filenameMaxLength` configuration\n \tvariable, or 64 if unconfigured.\n \n---rfc::\n-\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n-\tdefault). RFC means \"Request For Comments\"; use this when sending\n-\tan experimental patch for discussion rather than application.\n+--rfc[=]::\n+\tPrepends the string __ (defaults to \"RFC\") to\n+\tthe subject prefix. As the subject prefix defaults to\n+\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n++\n+RFC means \"Request For Comments\"; use this when sending\n+an experimental patch for discussion rather than application.\n+\"--rfc=WIP\" may also be a useful way to indicate that a patch\n+is not complete yet.\n \n -v ::\n --reroll-count=::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex c0a8bb95e9..2d6e0f3688 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1494,6 +1494,18 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+static int rfc_callback(const struct option *opt, const char *arg,\n+\t\t\tint unset)\n+{\n+\tstruct strbuf *rfc;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\trfc = opt->value;\n+\tstrbuf_reset(rfc);\n+\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n+\treturn 0;\n+}\n+\n static int numbered_cmdline_opt = 0;\n \n static int numbered_callback(const struct option *opt, const char *arg,\n@@ -1907,8 +1919,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tstruct strbuf sprefix = STRBUF_INIT;\n+\tstruct strbuf rfc = STRBUF_INIT;\n \tint creation_factor = -1;\n-\tint rfc = 0;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n@@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"mark the series as Nth re-roll\")),\n \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n \t\t\t N_(\"max length of output filename\")),\n-\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n+\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"extra\"),\n+\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n+\t\t\t PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n@@ -2050,8 +2064,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n-\tif (rfc)\n-\t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n+\tif (rfc.len)\n+\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc.buf);\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex e37a1411ee..905858da35 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1377,6 +1377,15 @@ test_expect_success '--rfc' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--rfc=WIP' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [WIP PATCH 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --rfc=WIP >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--rfc does not overwrite prefix' '\n \tcat >expect <<-\\EOF &&\n \tSubject: [RFC PATCH foobar 1/1] header with . in it\n-- \n2.44.0-651-g21306a098c\n\n"},{"id":"493176","messageId":"47453c9701315d791c580bfdd679a6f1@manjaro.org","threadId":"61335","inReplyTo":"xmqqzftqnuxq.fsf@gitster.g","subject":"Re: [PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-19T00:29:16Z","receivedAt":"2024-04-19T00:29:18Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Junio,\n\nPlease see my comments below.\n\nOn 2024-04-19 00:54, Junio C Hamano wrote:\n> With the \"--rfc\" option, we can tweak the \"[PATCH]\" (or whatever\n> string specified with the \"--subject-prefix\" option, instead of\n> \"PATCH\") that we prefix the title of the commit with into \"[RFC\n> PATCH]\", but some projects may want \"[rfc PATCH]\". Adding a new\n> option, e.g., \"--rfc-lowercase\", to support such need every time\n> somebody wants to use different strings would lead to insanity of\n> accumulating unbounded number of such options.\n> \n> Allow an optional value specified for the option, so that users can\n> use \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n> \"--rfc=RFC\").\n> \n> This can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\n> passing \"--rfc=WIP\".\n\nAren't we also going to allow patch subject prefixes such as\n\"[PATCH RESEND]\" to be produced this way? Please see my earlier\nresponse [1] for a possible solution.\n\n[1] \nhttps://lore.kernel.org/git/84dcb80be916f85cbb6a4b99aea0d76b@manjaro.org/\n\n> Signed-off-by: Junio C Hamano \n> ---\n> \n> Documentation/git-format-patch.txt | 15 ++++++++++-----\n> builtin/log.c | 22 ++++++++++++++++++----\n> t/t4014-format-patch.sh | 9 +++++++++\n> 3 files changed, 37 insertions(+), 9 deletions(-)\n> \n> diff --git a/Documentation/git-format-patch.txt\n> b/Documentation/git-format-patch.txt\n> index 728bb3821c..8d634f5b1b 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -20,7 +20,7 @@ SYNOPSIS\n> \t\t [--in-reply-to=] [--suffix=.]\n> \t\t [--ignore-if-in-upstream] [--always]\n> \t\t [--cover-from-description=]\n> -\t\t [--rfc] [--subject-prefix=]\n> +\t\t [--rfc[=]] [--subject-prefix=]\n> \t\t [(--reroll-count|-v) ]\n> \t\t [--to=] [--cc=]\n> \t\t [--[no-]cover-letter] [--quiet]\n> @@ -238,10 +238,15 @@ the patches (with a value of e.g. \"PATCH \n> my-project\").\n> \tvalue of the `format.filenameMaxLength` configuration\n> \tvariable, or 64 if unconfigured.\n> \n> ---rfc::\n> -\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n> -\tdefault). RFC means \"Request For Comments\"; use this when sending\n> -\tan experimental patch for discussion rather than application.\n> +--rfc[=]::\n> +\tPrepends the string __ (defaults to \"RFC\") to\n> +\tthe subject prefix. As the subject prefix defaults to\n> +\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n> ++\n> +RFC means \"Request For Comments\"; use this when sending\n> +an experimental patch for discussion rather than application.\n> +\"--rfc=WIP\" may also be a useful way to indicate that a patch\n> +is not complete yet.\n> \n> -v ::\n> --reroll-count=::\n> diff --git a/builtin/log.c b/builtin/log.c\n> index c0a8bb95e9..2d6e0f3688 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1494,6 +1494,18 @@ static int subject_prefix_callback(const struct\n> option *opt, const char *arg,\n> \treturn 0;\n> }\n> \n> +static int rfc_callback(const struct option *opt, const char *arg,\n> +\t\t\tint unset)\n> +{\n> +\tstruct strbuf *rfc;\n> +\n> +\tBUG_ON_OPT_NEG(unset);\n> +\trfc = opt->value;\n> +\tstrbuf_reset(rfc);\n> +\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n> +\treturn 0;\n> +}\n\n\"subject_prefix = 1;\" should also be in this function, to prevent\n'--rfc=\"\" -k' from being a valid combination of parameters.\n\nChecking \"rfc.len\" later in the code checks the length of the string\npassed for '--rfc=\"\"', which can be zero, instead of checking\nfor the existence of \"--rfc\" as an option on the command-line, so the\n\"subject_prefix\" flag also needs to be set.\n\n> +\n> static int numbered_cmdline_opt = 0;\n> \n> static int numbered_callback(const struct option *opt, const char \n> *arg,\n> @@ -1907,8 +1919,8 @@ int cmd_format_patch(int argc, const char\n> **argv, const char *prefix)\n> \tstruct strbuf rdiff2 = STRBUF_INIT;\n> \tstruct strbuf rdiff_title = STRBUF_INIT;\n> \tstruct strbuf sprefix = STRBUF_INIT;\n> +\tstruct strbuf rfc = STRBUF_INIT;\n> \tint creation_factor = -1;\n> -\tint rfc = 0;\n> \n> \tconst struct option builtin_format_patch_options[] = {\n> \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n> @@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char\n> **argv, const char *prefix)\n> \t\t\t N_(\"mark the series as Nth re-roll\")),\n> \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n> \t\t\t N_(\"max length of output filename\")),\n> -\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n> +\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"extra\"),\n> +\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n> +\t\t\t PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),\n> \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n> \t\t\t N_(\"cover-from-description-mode\"),\n> \t\t\t N_(\"generate parts of a cover letter based on a branch's \n> description\")),\n> @@ -2050,8 +2064,8 @@ int cmd_format_patch(int argc, const char\n> **argv, const char *prefix)\n> \tif (cover_from_description_arg)\n> \t\tcover_from_description_mode =\n> parse_cover_from_description(cover_from_description_arg);\n> \n> -\tif (rfc)\n> -\t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n> +\tif (rfc.len)\n> +\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc.buf);\n> \n> \tif (reroll_count) {\n> \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index e37a1411ee..905858da35 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1377,6 +1377,15 @@ test_expect_success '--rfc' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success '--rfc=WIP' '\n> +\tcat >expect <<-\\EOF &&\n> +\tSubject: [WIP PATCH 1/1] header with . in it\n> +\tEOF\n> +\tgit format-patch -n -1 --stdout --rfc=WIP >patch &&\n> +\tgrep \"^Subject:\" patch >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_expect_success '--rfc does not overwrite prefix' '\n> \tcat >expect <<-\\EOF &&\n> \tSubject: [RFC PATCH foobar 1/1] header with . in it\n"},{"id":"493218","messageId":"9f846a3d-5244-43ec-b392-156665be0929@gmail.com","threadId":"61335","inReplyTo":"xmqqzftqnuxq.fsf@gitster.g","subject":"Re: [PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-04-19T14:09:54Z","receivedAt":"2024-04-19T14:09:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 18/04/2024 23:54, Junio C Hamano wrote:\n> With the \"--rfc\" option, we can tweak the \"[PATCH]\" (or whatever\n> string specified with the \"--subject-prefix\" option, instead of\n> \"PATCH\") that we prefix the title of the commit with into \"[RFC\n> PATCH]\", but some projects may want \"[rfc PATCH]\". Adding a new\n> option, e.g., \"--rfc-lowercase\", to support such need every time\n> somebody wants to use different strings would lead to insanity of\n> accumulating unbounded number of such options.\n> \n> Allow an optional value specified for the option, so that users can\n> use \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n> \"--rfc=RFC\").\n> \n> This can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\n> passing \"--rfc=WIP\".\n\nI think being able to customize the subject prefix in this way is a good \nidea and makes it easy to say [RESEND PATCH program] with --rfc=RESEND \nwhen format.subjectPrefix is set. We could add a separate option to as \n\"--rfc\" already exists I think it is reasonable to extend it. (I'm also \nnot a good name for such an option would be \"--subject-prefix-prefix\" \nkind of describes what it does but is far from ideal)\n\n> -\t\t [--rfc] [--subject-prefix=]\n> +\t\t [--rfc[=]] [--subject-prefix=]\n\nNit: in the documentation we use \"\" for the placeholder but in the \ncode we use \"\"\n\n> ---rfc::\n> -\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n> -\tdefault). RFC means \"Request For Comments\"; use this when sending\n> -\tan experimental patch for discussion rather than application.\n> +--rfc[=]::\n> +\tPrepends the string __ (defaults to \"RFC\") to\n> +\tthe subject prefix. As the subject prefix defaults to\n> +\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n> ++\n> +RFC means \"Request For Comments\"; use this when sending\n> +an experimental patch for discussion rather than application.\n> +\"--rfc=WIP\" may also be a useful way to indicate that a patch\n> +is not complete yet.\n\nThis reworded description is good\n\n> \tconst struct option builtin_format_patch_options[] = {\n> \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n> @@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"mark the series as Nth re-roll\")),\n> \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n> \t\t\t N_(\"max length of output filename\")),\n> -\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n> +\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"extra\"),\n> +\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n> +\t\t\t PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),\n\nThis is a change in behavior as it looks like we previously accepted \n\"--no-rfc\" is that deliberate?\n\nBest Wishes\n\nPhillip\n"},{"id":"493224","messageId":"xmqq1q71l1xz.fsf@gitster.g","threadId":"61335","inReplyTo":"9f846a3d-5244-43ec-b392-156665be0929@gmail.com","subject":"Re: [PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-19T17:03:36Z","receivedAt":"2024-04-19T17:03:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>> -\t\t [--rfc] [--subject-prefix=]\n>> +\t\t [--rfc[=]] [--subject-prefix=]\n>\n> Nit: in the documentation we use \"\" for the placeholder but in\n> the code we use \"\"\n\nYou're right. Will fix.\n\n>> @@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>> \t\t\t N_(\"mark the series as Nth re-roll\")),\n>> \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n>> \t\t\t N_(\"max length of output filename\")),\n>> -\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n>> +\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"extra\"),\n>> +\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n>> +\t\t\t PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),\n>\n> This is a change in behavior as it looks like we previously accepted\n> \"--no-rfc\" is that deliberate?\n\nI just matched the subject-prefix without thinking. Will fix.\n\nHere is what I plan to squash in, but we are about to enter the\npre-release stabilization period, so the progress on this new\nfeature will have to slow down.\n\nThanks.\n\n builtin/log.c | 10 +++++-----\n t/t4014-format-patch.sh | 16 ++++++++++++----\n 2 files changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git i/builtin/log.c w/builtin/log.c\nindex 2d6e0f3688..0e9c84e51d 100644\n--- i/builtin/log.c\n+++ w/builtin/log.c\n@@ -1499,10 +1499,10 @@ static int rfc_callback(const struct option *opt, const char *arg,\n {\n \tstruct strbuf *rfc;\n \n-\tBUG_ON_OPT_NEG(unset);\n \trfc = opt->value;\n \tstrbuf_reset(rfc);\n-\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n+\tif (!unset)\n+\t\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n \treturn 0;\n }\n \n@@ -1944,9 +1944,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"mark the series as Nth re-roll\")),\n \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n \t\t\t N_(\"max length of output filename\")),\n-\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"extra\"),\n-\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n-\t\t\t PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),\n+\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"rfc\"),\n+\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n+\t\t\t PARSE_OPT_OPTARG, rfc_callback),\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\ndiff --git i/t/t4014-format-patch.sh w/t/t4014-format-patch.sh\nindex 905858da35..645c4189f9 100755\n--- i/t/t4014-format-patch.sh\n+++ w/t/t4014-format-patch.sh\n@@ -1368,22 +1368,30 @@ test_expect_success 'empty subject prefix does not have extra space' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--rfc' '\n+test_expect_success '--rfc and --no-rfc' '\n \tcat >expect <<-\\EOF &&\n \tSubject: [RFC PATCH 1/1] header with . in it\n \tEOF\n \tgit format-patch -n -1 --stdout --rfc >patch &&\n \tgrep \"^Subject:\" patch >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\tgit format-patch -n -1 --stdout --rfc --no-rfc >patch &&\n+\tsed -e \"s/RFC //\" expect >expect-raw &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect-raw actual\n '\n \n-test_expect_success '--rfc=WIP' '\n+test_expect_success '--rfc=WIP and --rfc=' '\n \tcat >expect <<-\\EOF &&\n \tSubject: [WIP PATCH 1/1] header with . in it\n \tEOF\n \tgit format-patch -n -1 --stdout --rfc=WIP >patch &&\n \tgrep \"^Subject:\" patch >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\tgit format-patch -n -1 --stdout --rfc --rfc= >patch &&\n+\tsed -e \"s/WIP //\" expect >expect-raw &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect-raw actual\n '\n \n test_expect_success '--rfc does not overwrite prefix' '\n"},{"id":"493229","messageId":"20240419180012.GA523357@coredump.intra.peff.net","threadId":"61335","inReplyTo":"xmqqzftqnuxq.fsf@gitster.g","subject":"Re: [PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-04-19T18:00:12Z","receivedAt":"2024-04-19T18:00:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 18, 2024 at 03:54:25PM -0700, Junio C Hamano wrote:\n\n> ---rfc::\n> -\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n> -\tdefault). RFC means \"Request For Comments\"; use this when sending\n> -\tan experimental patch for discussion rather than application.\n> +--rfc[=]::\n> +\tPrepends the string __ (defaults to \"RFC\") to\n> +\tthe subject prefix. As the subject prefix defaults to\n> +\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n> ++\n> +RFC means \"Request For Comments\"; use this when sending\n> +an experimental patch for discussion rather than application.\n> +\"--rfc=WIP\" may also be a useful way to indicate that a patch\n> +is not complete yet.\n\nIt's probably worth spelling out WIP here, too, like:\n\n ...is not complete yet (\"WIP\" stands for \"Work In Progress\").\n\nThese are necessarily going to be project-specific phrases, and readers\nof the manpage may not be familiar with our conventions.\n\n-Peff\n"},{"id":"493231","messageId":"xmqq1q71jjuc.fsf@gitster.g","threadId":"61335","inReplyTo":"20240419180012.GA523357@coredump.intra.peff.net","subject":"Re: [PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-19T18:19:55Z","receivedAt":"2024-04-19T18:20:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Apr 18, 2024 at 03:54:25PM -0700, Junio C Hamano wrote:\n>\n>> ---rfc::\n>> -\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n>> -\tdefault). RFC means \"Request For Comments\"; use this when sending\n>> -\tan experimental patch for discussion rather than application.\n>> +--rfc[=]::\n>> +\tPrepends the string __ (defaults to \"RFC\") to\n>> +\tthe subject prefix. As the subject prefix defaults to\n>> +\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n>> ++\n>> +RFC means \"Request For Comments\"; use this when sending\n>> +an experimental patch for discussion rather than application.\n>> +\"--rfc=WIP\" may also be a useful way to indicate that a patch\n>> +is not complete yet.\n>\n> It's probably worth spelling out WIP here, too, like:\n>\n> ...is not complete yet (\"WIP\" stands for \"Work In Progress\").\n>\n> These are necessarily going to be project-specific phrases, and readers\n> of the manpage may not be familiar with our conventions.\n\nGood point. Will squash in.\n\n"},{"id":"493242","messageId":"xmqqy1993tc1.fsf@gitster.g","threadId":"61335","inReplyTo":"xmqqzftqnuxq.fsf@gitster.g","subject":"[PATCH v2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-19T22:01:34Z","receivedAt":"2024-04-19T22:01:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the \"--rfc\" option, we can tweak the \"[PATCH]\" (or whatever\nstring specified with the \"--subject-prefix\" option, instead of\n\"PATCH\") that we prefix the title of the commit with into \"[RFC\nPATCH]\", but some projects may want \"[rfc PATCH]\". Adding a new\noption, e.g., \"--rfc-lowercase\", to support such need every time\nsomebody wants to use different strings would lead to insanity of\naccumulating unbounded number of such options.\n\nAllow an optional value specified for the option, so that users can\nuse \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n\"--rfc=RFC\") if they wanted to.\n\nThis can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\npassing \"--rfc=WIP\". Passing an empty string, i.e., \"--rfc=\", is\nthe same as \"--no-rfc\" to override an option given earlier on the\nsame command line.\n\nSigned-off-by: Junio C Hamano \n---\nRange-diff against v1:\n1: 4077ed52e8 ! 1: bffe0055d0 format-patch: allow --rfc to optionally take a value, like --rfc=WIP\n @@ Commit message\n \n Allow an optional value specified for the option, so that users can\n use \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n - \"--rfc=RFC\").\n + \"--rfc=RFC\") if they wanted to.\n \n This can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\n - passing \"--rfc=WIP\".\n + passing \"--rfc=WIP\". Passing an empty string, i.e., \"--rfc=\", is\n + the same as \"--no-rfc\" to override an option given earlier on the\n + same command line.\n \n Signed-off-by: Junio C Hamano \n \n @@ Documentation/git-format-patch.txt: the patches (with a value of e.g. \"PATCH my-\n +RFC means \"Request For Comments\"; use this when sending\n +an experimental patch for discussion rather than application.\n +\"--rfc=WIP\" may also be a useful way to indicate that a patch\n -+is not complete yet.\n ++is not complete yet (\"WIP\" stands for \"Work In Progress\").\n \n -v ::\n --reroll-count=::\n @@ builtin/log.c: static int subject_prefix_callback(const struct option *opt, cons\n +{\n +\tstruct strbuf *rfc;\n +\n -+\tBUG_ON_OPT_NEG(unset);\n +\trfc = opt->value;\n +\tstrbuf_reset(rfc);\n -+\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n ++\tif (!unset)\n ++\t\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n +\treturn 0;\n +}\n +\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n \t\t\t N_(\"max length of output filename\")),\n -\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n -+\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"extra\"),\n -+\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n -+\t\t\t PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),\n ++\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"rfc\"),\n ++\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n ++\t\t\t PARSE_OPT_OPTARG, rfc_callback),\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n \n ## t/t4014-format-patch.sh ##\n -@@ t/t4014-format-patch.sh: test_expect_success '--rfc' '\n +@@ t/t4014-format-patch.sh: test_expect_success 'empty subject prefix does not have extra space' '\n \ttest_cmp expect actual\n '\n \n -+test_expect_success '--rfc=WIP' '\n +-test_expect_success '--rfc' '\n ++test_expect_success '--rfc and --no-rfc' '\n + \tcat >expect <<-\\EOF &&\n + \tSubject: [RFC PATCH 1/1] header with . in it\n + \tEOF\n + \tgit format-patch -n -1 --stdout --rfc >patch &&\n + \tgrep \"^Subject:\" patch >actual &&\n +-\ttest_cmp expect actual\n ++\ttest_cmp expect actual &&\n ++\tgit format-patch -n -1 --stdout --rfc --no-rfc >patch &&\n ++\tsed -e \"s/RFC //\" expect >expect-raw &&\n ++\tgrep \"^Subject:\" patch >actual &&\n ++\ttest_cmp expect-raw actual\n ++'\n ++\n ++test_expect_success '--rfc=WIP and --rfc=' '\n +\tcat >expect <<-\\EOF &&\n +\tSubject: [WIP PATCH 1/1] header with . in it\n +\tEOF\n +\tgit format-patch -n -1 --stdout --rfc=WIP >patch &&\n +\tgrep \"^Subject:\" patch >actual &&\n -+\ttest_cmp expect actual\n -+'\n -+\n ++\ttest_cmp expect actual &&\n ++\tgit format-patch -n -1 --stdout --rfc --rfc= >patch &&\n ++\tsed -e \"s/WIP //\" expect >expect-raw &&\n ++\tgrep \"^Subject:\" patch >actual &&\n ++\ttest_cmp expect-raw actual\n + '\n + \n test_expect_success '--rfc does not overwrite prefix' '\n - \tcat >expect <<-\\EOF &&\n - \tSubject: [RFC PATCH foobar 1/1] header with . in it\n\n Documentation/git-format-patch.txt | 15 ++++++++++-----\n builtin/log.c | 22 ++++++++++++++++++----\n t/t4014-format-patch.sh | 21 +++++++++++++++++++--\n 3 files changed, 47 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 728bb3821c..e553810b1e 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,7 +20,7 @@ SYNOPSIS\n \t\t [--in-reply-to=] [--suffix=.]\n \t\t [--ignore-if-in-upstream] [--always]\n \t\t [--cover-from-description=]\n-\t\t [--rfc] [--subject-prefix=]\n+\t\t [--rfc[=]] [--subject-prefix=]\n \t\t [(--reroll-count|-v) ]\n \t\t [--to=] [--cc=]\n \t\t [--[no-]cover-letter] [--quiet]\n@@ -238,10 +238,15 @@ the patches (with a value of e.g. \"PATCH my-project\").\n \tvalue of the `format.filenameMaxLength` configuration\n \tvariable, or 64 if unconfigured.\n \n---rfc::\n-\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n-\tdefault). RFC means \"Request For Comments\"; use this when sending\n-\tan experimental patch for discussion rather than application.\n+--rfc[=]::\n+\tPrepends the string __ (defaults to \"RFC\") to\n+\tthe subject prefix. As the subject prefix defaults to\n+\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n++\n+RFC means \"Request For Comments\"; use this when sending\n+an experimental patch for discussion rather than application.\n+\"--rfc=WIP\" may also be a useful way to indicate that a patch\n+is not complete yet (\"WIP\" stands for \"Work In Progress\").\n \n -v ::\n --reroll-count=::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex c0a8bb95e9..0e9c84e51d 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1494,6 +1494,18 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+static int rfc_callback(const struct option *opt, const char *arg,\n+\t\t\tint unset)\n+{\n+\tstruct strbuf *rfc;\n+\n+\trfc = opt->value;\n+\tstrbuf_reset(rfc);\n+\tif (!unset)\n+\t\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n+\treturn 0;\n+}\n+\n static int numbered_cmdline_opt = 0;\n \n static int numbered_callback(const struct option *opt, const char *arg,\n@@ -1907,8 +1919,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tstruct strbuf sprefix = STRBUF_INIT;\n+\tstruct strbuf rfc = STRBUF_INIT;\n \tint creation_factor = -1;\n-\tint rfc = 0;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n@@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"mark the series as Nth re-roll\")),\n \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n \t\t\t N_(\"max length of output filename\")),\n-\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n+\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"rfc\"),\n+\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n+\t\t\t PARSE_OPT_OPTARG, rfc_callback),\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n@@ -2050,8 +2064,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n-\tif (rfc)\n-\t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n+\tif (rfc.len)\n+\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc.buf);\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex e37a1411ee..645c4189f9 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1368,13 +1368,30 @@ test_expect_success 'empty subject prefix does not have extra space' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--rfc' '\n+test_expect_success '--rfc and --no-rfc' '\n \tcat >expect <<-\\EOF &&\n \tSubject: [RFC PATCH 1/1] header with . in it\n \tEOF\n \tgit format-patch -n -1 --stdout --rfc >patch &&\n \tgrep \"^Subject:\" patch >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\tgit format-patch -n -1 --stdout --rfc --no-rfc >patch &&\n+\tsed -e \"s/RFC //\" expect >expect-raw &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect-raw actual\n+'\n+\n+test_expect_success '--rfc=WIP and --rfc=' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [WIP PATCH 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --rfc=WIP >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual &&\n+\tgit format-patch -n -1 --stdout --rfc --rfc= >patch &&\n+\tsed -e \"s/WIP //\" expect >expect-raw &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect-raw actual\n '\n \n test_expect_success '--rfc does not overwrite prefix' '\n-- \n2.45.0-rc0\n\n"},{"id":"493269","messageId":"7a60e90310024a06ba105892b54965a8@manjaro.org","threadId":"61335","inReplyTo":"xmqq1q71l1xz.fsf@gitster.g","subject":"Re: [PATCH] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-21T14:18:25Z","receivedAt":"2024-04-21T14:18:27Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Junio,\n\nOn 2024-04-19 19:03, Junio C Hamano wrote:\n> Phillip Wood writes:\n>>> @@ -1932,7 +1944,9 @@ int cmd_format_patch(int argc, const char \n>>> **argv, const char *prefix)\n>>> \t\t\t N_(\"mark the series as Nth re-roll\")),\n>>> \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n>>> \t\t\t N_(\"max length of output filename\")),\n>>> -\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of \n>>> [PATCH]\")),\n>>> +\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"extra\"),\n>>> +\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n>>> +\t\t\t PARSE_OPT_NONEG|PARSE_OPT_OPTARG, rfc_callback),\n>> \n>> This is a change in behavior as it looks like we previously accepted\n>> \"--no-rfc\" is that deliberate?\n> \n> I just matched the subject-prefix without thinking. Will fix.\n> \n> Here is what I plan to squash in, but we are about to enter the\n> pre-release stabilization period, so the progress on this new\n> feature will have to slow down.\n\nLet me remind you about the need to also support \"[PATCH RESEND]\",\nfor example, in this new feature. Please see my earlier response [1]\nfor a possible solution.\n\nEven some instructions for submitting patches, in some projects,\nspecify \"[PATCH RESEND]\" as the expected prefix, not \"[RESEND PATCH]\".\nThus, suffixes for the prefix should be supported.\n\n[1] \nhttps://lore.kernel.org/git/84dcb80be916f85cbb6a4b99aea0d76b@manjaro.org/\n"},{"id":"493270","messageId":"440ee2b7-2ed8-4c8b-96e8-623ef45efe42@gmail.com","threadId":"61335","inReplyTo":"xmqqy1993tc1.fsf@gitster.g","subject":"Re: [PATCH v2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-04-21T15:41:23Z","receivedAt":"2024-04-21T15:41:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 19/04/2024 23:01, Junio C Hamano wrote:\n> With the \"--rfc\" option, we can tweak the \"[PATCH]\" (or whatever\n> string specified with the \"--subject-prefix\" option, instead of\n> \"PATCH\") that we prefix the title of the commit with into \"[RFC\n> PATCH]\", but some projects may want \"[rfc PATCH]\". Adding a new\n> option, e.g., \"--rfc-lowercase\", to support such need every time\n> somebody wants to use different strings would lead to insanity of\n> accumulating unbounded number of such options.\n> \n> Allow an optional value specified for the option, so that users can\n> use \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n> \"--rfc=RFC\") if they wanted to.\n> \n> This can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\n> passing \"--rfc=WIP\". Passing an empty string, i.e., \"--rfc=\", is\n> the same as \"--no-rfc\" to override an option given earlier on the\n> same command line.\n\nThe changes in this version all look good to me, I've left one comment \nbelow.\n\n> @@ -1907,8 +1919,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \tstruct strbuf rdiff2 = STRBUF_INIT;\n> \tstruct strbuf rdiff_title = STRBUF_INIT;\n> \tstruct strbuf sprefix = STRBUF_INIT;\n> +\tstruct strbuf rfc = STRBUF_INIT;\n\nLooking at this again, do we really need an strbuf here? I think we \ncould we use \"const char *\" instead as we always store a static string \nin it which would avoid a memory leak from not calling strbuf_release().\n\nBest Wishes\n\nPhillip\n"},{"id":"493280","messageId":"xmqqle561r1f.fsf@gitster.g","threadId":"61335","inReplyTo":"440ee2b7-2ed8-4c8b-96e8-623ef45efe42@gmail.com","subject":"Re: [PATCH v2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-21T18:58:36Z","receivedAt":"2024-04-21T18:58:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>> +\tstruct strbuf rfc = STRBUF_INIT;\n>\n> Looking at this again, do we really need an strbuf here? I think we\n> could we use \"const char *\" instead as we always store a static string\n> in it which would avoid a memory leak from not calling\n> strbuf_release().\n\nThanks for spotting the leak ;-)\n\nAnd indeed, unlike sprefix, we do not need it to be an editable\nstrbuf.\n\nShould be sufficient to squash in something like this.\n\n builtin/log.c | 13 ++++++-------\n 1 file changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git c/builtin/log.c w/builtin/log.c\nindex 0e9c84e51d..5c1c6f9b15 100644\n--- c/builtin/log.c\n+++ w/builtin/log.c\n@@ -1497,12 +1497,11 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,\n static int rfc_callback(const struct option *opt, const char *arg,\n \t\t\tint unset)\n {\n-\tstruct strbuf *rfc;\n+\tconst char **rfc = opt->value;\n \n-\trfc = opt->value;\n-\tstrbuf_reset(rfc);\n+\t*rfc = opt->value;\n \tif (!unset)\n-\t\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n+\t\t*rfc = arg ? arg : \"RFC\";\n \treturn 0;\n }\n \n@@ -1919,7 +1918,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tstruct strbuf sprefix = STRBUF_INIT;\n-\tstruct strbuf rfc = STRBUF_INIT;\n+\tconst char *rfc = NULL;\n \tint creation_factor = -1;\n \n \tconst struct option builtin_format_patch_options[] = {\n@@ -2064,8 +2063,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n-\tif (rfc.len)\n-\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc.buf);\n+\tif (rfc)\n+\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n"},{"id":"493281","messageId":"20240421185915.1031590-1-gitster@pobox.com","threadId":"61335","inReplyTo":"xmqqy1993tc1.fsf@gitster.g","subject":"[PATCH v3 0/2] format-patch --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-21T18:59:13Z","receivedAt":"2024-04-21T18:59:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"So here is the hopefully final round. I think the first one is\ngood, but the second one is of dubious and possibly negative value\n(for the reasons, see its proposed log message), so I am undecided.\n\nJunio C Hamano (2):\n format-patch: allow --rfc to optionally take a value, like --rfc=WIP\n format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]\n\n Documentation/git-format-patch.txt | 19 ++++++++++++++-----\n builtin/log.c | 25 +++++++++++++++++++++----\n t/t4014-format-patch.sh | 28 +++++++++++++++++++++++++++-\n 3 files changed, 62 insertions(+), 10 deletions(-)\n\nRange-diff against v2:\n1: bffe0055d0 ! 1: a874b85cb5 format-patch: allow --rfc to optionally take a value, like --rfc=WIP\n @@ builtin/log.c: static int subject_prefix_callback(const struct option *opt, cons\n +static int rfc_callback(const struct option *opt, const char *arg,\n +\t\t\tint unset)\n +{\n -+\tstruct strbuf *rfc;\n ++\tconst char **rfc = opt->value;\n +\n -+\trfc = opt->value;\n -+\tstrbuf_reset(rfc);\n ++\t*rfc = opt->value;\n +\tif (!unset)\n -+\t\tstrbuf_addstr(rfc, arg ? arg : \"RFC\");\n ++\t\t*rfc = arg ? arg : \"RFC\";\n +\treturn 0;\n +}\n +\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tstruct strbuf sprefix = STRBUF_INIT;\n -+\tstruct strbuf rfc = STRBUF_INIT;\n ++\tconst char *rfc = NULL;\n \tint creation_factor = -1;\n -\tint rfc = 0;\n \n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)\n - \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n --\tif (rfc)\n + \tif (rfc)\n -\t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n -+\tif (rfc.len)\n -+\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc.buf);\n ++\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n-: ---------- > 2: 6b82e903b6 format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]\n-- \n2.45.0-rc0\n\n"},{"id":"493282","messageId":"20240421185915.1031590-3-gitster@pobox.com","threadId":"61335","inReplyTo":"20240421185915.1031590-1-gitster@pobox.com","subject":"[PATCH v3 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-21T18:59:15Z","receivedAt":"2024-04-21T18:59:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In the previous step, the \"--rfc\" option of \"format-patch\" learned\nto take an optional string value to prepend to the subject prefix,\nso that --rfc=WIP can give \"[WIP PATCH]\". This commit shows that\nthe mechanism can be extended easily to allow \"--rfc=-(WIP)\" [*1*]\nto signal that the extra string is to be appended instead of getting\nprepended, resulting in \"[PATCH (WIP)]\".\n\nHaving worked on the patch, I am personally not 100% on board with\nthis part of the feature myself, and that is why this update is a\nseparate step from the main \"--rfc takes an optional string value\"\nstep.\n\nIf a way to prepend an arbitrary adornment is added to the system,\nand people can now say \"--rfc=RESEND\" to produce \"[RESEND PATCH]\",\nthose who used to use \"[PATCH RESEND]\" by tweaking the subject\nprefix in other ways [*2*] would do one of three things:\n\n (1) keep using their existing ways and keep sending their message\n with the \"[RESEND PATCH]\" prefix.\n\n (2) rejoice in the new automation, switch to use \"--rfc=RESEND\",\n and start sending their messages with \"[RESEND PATCH]\" prefix\n instead of \"[PATCH RESEND]\" prefix.\n\n (3) complain and demand a way to append instead of prepend so that\n they can use an automation to produce \"[PATCH RESEND]\" prefix.\n\nI do not believe in adding slightly different ways that allow users\nto be \"original\" when such differences do not make the world better\nin meaningful ways [*3*], and I expect there are many more folks who\nshare that sentiment and go to route (2) than those who go to route\n(3). If my expectation is true, it means that this patch goes in a\nwrong direction, and I would be happy to drop it.\n\n\n[Footnote]\n\n *1* The syntax takes inspiration from Perl's three-arg open syntax\n that uses pipes \"open fh, '|-', 'cmd'\", where the dash signals\n \"the other stuff comes here\".\n\n *2* ... because \"--rfc\" in released versions does not even take any\n string value to prepend, let alone append, to begin with.\n\n *3* https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/\n gathered some stats to observe that \"[RFC PATCH]\" is more\n common than \"[PATCH RFC]\" by a wide margin, while trying to see\n how common \"[RESEND PATCH]\" (or \"[PATCH RESED]\") were used (the\n answer: much less common). But it wouldn't have needed to\n count \"[PATCH RFC]\" and \"[RFC PATCH]\" separately if using a\n prefix and not a suffix (or vice versa) were established more\n firmly as the standard practice.\n\n It is a fine example that useless diversity making needless\n work.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-format-patch.txt | 4 ++++\n builtin/log.c | 8 ++++++--\n t/t4014-format-patch.sh | 9 +++++++++\n 3 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex e553810b1e..dbccb210cd 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -247,6 +247,10 @@ RFC means \"Request For Comments\"; use this when sending\n an experimental patch for discussion rather than application.\n \"--rfc=WIP\" may also be a useful way to indicate that a patch\n is not complete yet (\"WIP\" stands for \"Work In Progress\").\n++\n+If the string __ begins with a dash (\"`-`\"), the rest of the\n+__ string is appended to the subject prefix instead, e.g.,\n+`--rfc='-(WIP)'` results in \"PATCH (WIP)\".\n \n -v ::\n --reroll-count=::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 5c1c6f9b15..dd11953260 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -2063,8 +2063,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n-\tif (rfc)\n-\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n+\tif (rfc) {\n+\t\tif (rfc[0] == '-')\n+\t\t\tstrbuf_addf(&sprefix, \" %s\", rfc + 1);\n+\t\telse\n+\t\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n+\t}\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 645c4189f9..fcbde15b16 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1394,6 +1394,15 @@ test_expect_success '--rfc=WIP and --rfc=' '\n \ttest_cmp expect-raw actual\n '\n \n+test_expect_success '--rfc=-(WIP) appends' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [PATCH (WIP) 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --rfc=\"-(WIP)\" >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--rfc does not overwrite prefix' '\n \tcat >expect <<-\\EOF &&\n \tSubject: [RFC PATCH foobar 1/1] header with . in it\n-- \n2.45.0-rc0\n\n"},{"id":"493283","messageId":"20240421185915.1031590-2-gitster@pobox.com","threadId":"61335","inReplyTo":"20240421185915.1031590-1-gitster@pobox.com","subject":"[PATCH v3 1/2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-21T18:59:14Z","receivedAt":"2024-04-21T18:59:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the \"--rfc\" option, we can tweak the \"[PATCH]\" (or whatever\nstring specified with the \"--subject-prefix\" option, instead of\n\"PATCH\") that we prefix the title of the commit with into \"[RFC\nPATCH]\", but some projects may want \"[rfc PATCH]\". Adding a new\noption, e.g., \"--rfc-lowercase\", to support such need every time\nsomebody wants to use different strings would lead to insanity of\naccumulating unbounded number of such options.\n\nAllow an optional value specified for the option, so that users can\nuse \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n\"--rfc=RFC\") if they wanted to.\n\nThis can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\npassing \"--rfc=WIP\". Passing an empty string, i.e., \"--rfc=\", is\nthe same as \"--no-rfc\" to override an option given earlier on the\nsame command line.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-format-patch.txt | 15 ++++++++++-----\n builtin/log.c | 19 ++++++++++++++++---\n t/t4014-format-patch.sh | 21 +++++++++++++++++++--\n 3 files changed, 45 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 728bb3821c..e553810b1e 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,7 +20,7 @@ SYNOPSIS\n \t\t [--in-reply-to=] [--suffix=.]\n \t\t [--ignore-if-in-upstream] [--always]\n \t\t [--cover-from-description=]\n-\t\t [--rfc] [--subject-prefix=]\n+\t\t [--rfc[=]] [--subject-prefix=]\n \t\t [(--reroll-count|-v) ]\n \t\t [--to=] [--cc=]\n \t\t [--[no-]cover-letter] [--quiet]\n@@ -238,10 +238,15 @@ the patches (with a value of e.g. \"PATCH my-project\").\n \tvalue of the `format.filenameMaxLength` configuration\n \tvariable, or 64 if unconfigured.\n \n---rfc::\n-\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n-\tdefault). RFC means \"Request For Comments\"; use this when sending\n-\tan experimental patch for discussion rather than application.\n+--rfc[=]::\n+\tPrepends the string __ (defaults to \"RFC\") to\n+\tthe subject prefix. As the subject prefix defaults to\n+\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n++\n+RFC means \"Request For Comments\"; use this when sending\n+an experimental patch for discussion rather than application.\n+\"--rfc=WIP\" may also be a useful way to indicate that a patch\n+is not complete yet (\"WIP\" stands for \"Work In Progress\").\n \n -v ::\n --reroll-count=::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex c0a8bb95e9..5c1c6f9b15 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1494,6 +1494,17 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+static int rfc_callback(const struct option *opt, const char *arg,\n+\t\t\tint unset)\n+{\n+\tconst char **rfc = opt->value;\n+\n+\t*rfc = opt->value;\n+\tif (!unset)\n+\t\t*rfc = arg ? arg : \"RFC\";\n+\treturn 0;\n+}\n+\n static int numbered_cmdline_opt = 0;\n \n static int numbered_callback(const struct option *opt, const char *arg,\n@@ -1907,8 +1918,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tstruct strbuf sprefix = STRBUF_INIT;\n+\tconst char *rfc = NULL;\n \tint creation_factor = -1;\n-\tint rfc = 0;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n@@ -1932,7 +1943,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"mark the series as Nth re-roll\")),\n \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n \t\t\t N_(\"max length of output filename\")),\n-\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n+\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"rfc\"),\n+\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n+\t\t\t PARSE_OPT_OPTARG, rfc_callback),\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n@@ -2051,7 +2064,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n \tif (rfc)\n-\t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n+\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex e37a1411ee..645c4189f9 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1368,13 +1368,30 @@ test_expect_success 'empty subject prefix does not have extra space' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--rfc' '\n+test_expect_success '--rfc and --no-rfc' '\n \tcat >expect <<-\\EOF &&\n \tSubject: [RFC PATCH 1/1] header with . in it\n \tEOF\n \tgit format-patch -n -1 --stdout --rfc >patch &&\n \tgrep \"^Subject:\" patch >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\tgit format-patch -n -1 --stdout --rfc --no-rfc >patch &&\n+\tsed -e \"s/RFC //\" expect >expect-raw &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect-raw actual\n+'\n+\n+test_expect_success '--rfc=WIP and --rfc=' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [WIP PATCH 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --rfc=WIP >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual &&\n+\tgit format-patch -n -1 --stdout --rfc --rfc= >patch &&\n+\tsed -e \"s/WIP //\" expect >expect-raw &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect-raw actual\n '\n \n test_expect_success '--rfc does not overwrite prefix' '\n-- \n2.45.0-rc0\n\n"},{"id":"493292","messageId":"f9aae9692493e4b722ce9f38de73c810@manjaro.org","threadId":"61335","inReplyTo":"20240421185915.1031590-3-gitster@pobox.com","subject":"Re: [PATCH v3 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-21T19:37:32Z","receivedAt":"2024-04-21T19:37:34Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Junio,\n\nOn 2024-04-21 20:59, Junio C Hamano wrote:\n> In the previous step, the \"--rfc\" option of \"format-patch\" learned\n> to take an optional string value to prepend to the subject prefix,\n> so that --rfc=WIP can give \"[WIP PATCH]\". This commit shows that\n> the mechanism can be extended easily to allow \"--rfc=-(WIP)\" [*1*]\n> to signal that the extra string is to be appended instead of getting\n> prepended, resulting in \"[PATCH (WIP)]\".\n> \n> Having worked on the patch, I am personally not 100% on board with\n> this part of the feature myself, and that is why this update is a\n> separate step from the main \"--rfc takes an optional string value\"\n> step.\n> \n> If a way to prepend an arbitrary adornment is added to the system,\n> and people can now say \"--rfc=RESEND\" to produce \"[RESEND PATCH]\",\n> those who used to use \"[PATCH RESEND]\" by tweaking the subject\n> prefix in other ways [*2*] would do one of three things:\n\nThere are even more issues, such as the grammar-related ones. Let\nme explain, please, as accurately as I can do that as a non-native\nEnglish speaker who spent many years studying English grammar.\n\nWriting \"RFC PATCH\" puts \"RFC\" into the role of an adjective, which\nis fine. The same obviously applies to \"WIP PATCH\". On the other\nhand, writing \"RESEND PATCH\" puts \"RESEND\" into its only possible\nrole, which is transitive verb, and results in \"RESEND PATCH\" that\nserves as a very simple sentence in imperative mood. I'm not sure\nthat, strictly technically speaking, having simple sentences as the\nprefixes is the desired outcome.\n\nTechnically, we should use \"RE-SENT PATCH\" to be on the correct side\nfrom the grammar perspective, with \"RE-SENT\" serving as an adjective.\nBefore you ask, no, we can't use \"RESENT\" there, because its meaning\nis completely different. However, nobody uses \"RE-SENT PATCH\", or\nat least I haven't seen it used yet.\n\nWhen it comes to \"PATCH RESEND\", \"RESEND\" remains in its transitive\nverb role, but \"PATCH\", as a noun, becomes a modifier of the verb.\nThus, the resulting meaning of \"PATCH RESEND\" becomes something like\n\"resend an item that's a patch\", but not written in form of a simple\n(or less simple) sentence. Strictly technically, a noun should only\nmodify another noun, but bending English grammar a bit this way is\nmuch better than actually having a simple sentence as a prefix.\n\nWith all this in mind, I think that allowing the \"--rfc=-\"\nform is the way to go. Computer lingo often bends English grammar\nto a certain degree, to achieve compactness, but bending and dumbing\nit down more that it's actually necessary isn't something that we\nshould embrace.\n\nI hope all this makes sense.\n\n> (1) keep using their existing ways and keep sending their message\n> with the \"[RESEND PATCH]\" prefix.\n> \n> (2) rejoice in the new automation, switch to use \"--rfc=RESEND\",\n> and start sending their messages with \"[RESEND PATCH]\" prefix\n> instead of \"[PATCH RESEND]\" prefix.\n> \n> (3) complain and demand a way to append instead of prepend so that\n> they can use an automation to produce \"[PATCH RESEND]\" prefix.\n> \n> I do not believe in adding slightly different ways that allow users\n> to be \"original\" when such differences do not make the world better\n> in meaningful ways [*3*], and I expect there are many more folks who\n> share that sentiment and go to route (2) than those who go to route\n> (3). If my expectation is true, it means that this patch goes in a\n> wrong direction, and I would be happy to drop it.\n> \n> \n> [Footnote]\n> \n> *1* The syntax takes inspiration from Perl's three-arg open syntax\n> that uses pipes \"open fh, '|-', 'cmd'\", where the dash signals\n> \"the other stuff comes here\".\n> \n> *2* ... because \"--rfc\" in released versions does not even take any\n> string value to prepend, let alone append, to begin with.\n> \n> *3* \n> https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/\n> gathered some stats to observe that \"[RFC PATCH]\" is more\n> common than \"[PATCH RFC]\" by a wide margin, while trying to see\n> how common \"[RESEND PATCH]\" (or \"[PATCH RESED]\") were used (the\n> answer: much less common). But it wouldn't have needed to\n> count \"[PATCH RFC]\" and \"[RFC PATCH]\" separately if using a\n> prefix and not a suffix (or vice versa) were established more\n> firmly as the standard practice.\n> \n> It is a fine example that useless diversity making needless\n> work.\n"},{"id":"493361","messageId":"20240423175234.170434-1-gitster@pobox.com","threadId":"61335","inReplyTo":"20240421185915.1031590-1-gitster@pobox.com","subject":"[PATCH v4 0/2] format-patch --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-23T17:52:32Z","receivedAt":"2024-04-23T17:52:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In the previous iteration, I botched the implementation of the\n\"--no-rfc\" case and somehow the test was \"lucky\" enough to leave\nuninitialized pieces of memory filled with '\\0's and did not catch\nit.\n\nThe proposed log message for the second one has been reworded, and\nthe documentation (obliquely) cautions against abusing the suffix\nform just to be different.\n\nJunio C Hamano (2):\n format-patch: allow --rfc to optionally take a value, like --rfc=WIP\n format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]\n\n Documentation/git-format-patch.txt | 21 ++++++++++++++++-----\n builtin/log.c | 27 +++++++++++++++++++++++----\n t/t4014-format-patch.sh | 28 +++++++++++++++++++++++++++-\n 3 files changed, 66 insertions(+), 10 deletions(-)\n\nRange-diff against v3:\n1: 7e022c85f7 ! 1: 30798e1211 format-patch: allow --rfc to optionally take a value, like --rfc=WIP\n @@ builtin/log.c: static int subject_prefix_callback(const struct option *opt, cons\n +\tconst char **rfc = opt->value;\n +\n +\t*rfc = opt->value;\n -+\tif (!unset)\n ++\tif (unset)\n ++\t\t*rfc = NULL;\n ++\telse\n +\t\t*rfc = arg ? arg : \"RFC\";\n +\treturn 0;\n +}\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)\n + \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n - \tif (rfc)\n +-\tif (rfc)\n -\t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n ++\tif (rfc && rfc[0])\n +\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n \n \tif (reroll_count) {\n2: 474041bdf8 ! 2: fdbe198cd7 format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]\n @@ Commit message\n \n In the previous step, the \"--rfc\" option of \"format-patch\" learned\n to take an optional string value to prepend to the subject prefix,\n - so that --rfc=WIP can give \"[WIP PATCH]\". This commit shows that\n - the mechanism can be extended easily to allow \"--rfc=-(WIP)\" [*1*]\n - to signal that the extra string is to be appended instead of getting\n - prepended, resulting in \"[PATCH (WIP)]\".\n -\n - Having worked on the patch, I am personally not 100% on board with\n - this part of the feature myself, and that is why this update is a\n - separate step from the main \"--rfc takes an optional string value\"\n - step.\n -\n - If a way to prepend an arbitrary adornment is added to the system,\n - and people can now say \"--rfc=RESEND\" to produce \"[RESEND PATCH]\",\n - those who used to use \"[PATCH RESEND]\" by tweaking the subject\n - prefix in other ways [*2*] would do one of three things:\n -\n - (1) keep using their existing ways and keep sending their message\n - with the \"[RESEND PATCH]\" prefix.\n -\n - (2) rejoice in the new automation, switch to use \"--rfc=RESEND\",\n - and start sending their messages with \"[RESEND PATCH]\" prefix\n - instead of \"[PATCH RESEND]\" prefix.\n + so that --rfc=WIP can give \"[WIP PATCH]\".\n \n - (3) complain and demand a way to append instead of prepend so that\n - they can use an automation to produce \"[PATCH RESEND]\" prefix.\n + There may be cases in which the extra string wants to come after the\n + subject prefix. Extend the mechanism to allow \"--rfc=-(WIP)\" [*] to\n + signal that the extra string is to be appended instead of getting\n + prepended, resulting in \"[PATCH (WIP)]\".\n \n - I do not believe in adding slightly different ways that allow users\n - to be \"original\" when such differences do not make the world better\n - in meaningful ways [*3*], and I expect there are many more folks who\n - share that sentiment and go to route (2) than those who go to route\n - (3). If my expectation is true, it means that this patch goes in a\n - wrong direction, and I would be happy to drop it.\n + In the documentation, discourage (ab)using \"--rfc=-RFC\" to say\n + \"[PATCH RFC]\" just to be different, when \"[RFC PATCH]\" is the norm.\n \n [Footnote]\n \n - *1* The syntax takes inspiration from Perl's three-arg open syntax\n - that uses pipes \"open fh, '|-', 'cmd'\", where the dash signals\n - \"the other stuff comes here\".\n -\n - *2* ... because \"--rfc\" in released versions does not even take any\n - string value to prepend, let alone append, to begin with.\n -\n - *3* https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/\n - gathered some stats to observe that \"[RFC PATCH]\" is more\n - common than \"[PATCH RFC]\" by a wide margin, while trying to see\n - how common \"[RESEND PATCH]\" (or \"[PATCH RESED]\") were used (the\n - answer: much less common). But it wouldn't have needed to\n - count \"[PATCH RFC]\" and \"[RFC PATCH]\" separately if using a\n - prefix and not a suffix (or vice versa) were established more\n - firmly as the standard practice.\n -\n - It is a fine example that useless diversity making needless\n - work.\n + * The syntax takes inspiration from Perl's open syntax that opens\n + pipes \"open fh, '|-', 'cmd'\", where the dash signals \"the other\n + stuff comes here\".\n \n Signed-off-by: Junio C Hamano \n \n @@ Documentation/git-format-patch.txt: RFC means \"Request For Comments\"; use this w\n \"--rfc=WIP\" may also be a useful way to indicate that a patch\n is not complete yet (\"WIP\" stands for \"Work In Progress\").\n ++\n -+If the string __ begins with a dash (\"`-`\"), the rest of the\n -+__ string is appended to the subject prefix instead, e.g.,\n -+`--rfc='-(WIP)'` results in \"PATCH (WIP)\".\n ++If the convention of the receiving community for a particular extra\n ++string is to have it _after_ the subject prefix, the string __\n ++can be prefixed with a dash (\"`-`\") to signal that the the rest of\n ++the __ string should be appended to the subject prefix instead,\n ++e.g., `--rfc='-(WIP)'` results in \"PATCH (WIP)\".\n \n -v ::\n --reroll-count=::\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n --\tif (rfc)\n +-\tif (rfc && rfc[0])\n -\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n -+\tif (rfc) {\n ++\tif (rfc && rfc[0]) {\n +\t\tif (rfc[0] == '-')\n +\t\t\tstrbuf_addf(&sprefix, \" %s\", rfc + 1);\n +\t\telse\n-- \n2.45.0-rc0-3-g00e10ef10e\n\n"},{"id":"493362","messageId":"20240423175234.170434-2-gitster@pobox.com","threadId":"61335","inReplyTo":"20240423175234.170434-1-gitster@pobox.com","subject":"[PATCH v4 1/2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-23T17:52:33Z","receivedAt":"2024-04-23T17:52:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the \"--rfc\" option, we can tweak the \"[PATCH]\" (or whatever\nstring specified with the \"--subject-prefix\" option, instead of\n\"PATCH\") that we prefix the title of the commit with into \"[RFC\nPATCH]\", but some projects may want \"[rfc PATCH]\". Adding a new\noption, e.g., \"--rfc-lowercase\", to support such need every time\nsomebody wants to use different strings would lead to insanity of\naccumulating unbounded number of such options.\n\nAllow an optional value specified for the option, so that users can\nuse \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n\"--rfc=RFC\") if they wanted to.\n\nThis can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\npassing \"--rfc=WIP\". Passing an empty string, i.e., \"--rfc=\", is\nthe same as \"--no-rfc\" to override an option given earlier on the\nsame command line.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-format-patch.txt | 15 ++++++++++-----\n builtin/log.c | 23 +++++++++++++++++++----\n t/t4014-format-patch.sh | 21 +++++++++++++++++++--\n 3 files changed, 48 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 728bb3821c..e553810b1e 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,7 +20,7 @@ SYNOPSIS\n \t\t [--in-reply-to=] [--suffix=.]\n \t\t [--ignore-if-in-upstream] [--always]\n \t\t [--cover-from-description=]\n-\t\t [--rfc] [--subject-prefix=]\n+\t\t [--rfc[=]] [--subject-prefix=]\n \t\t [(--reroll-count|-v) ]\n \t\t [--to=] [--cc=]\n \t\t [--[no-]cover-letter] [--quiet]\n@@ -238,10 +238,15 @@ the patches (with a value of e.g. \"PATCH my-project\").\n \tvalue of the `format.filenameMaxLength` configuration\n \tvariable, or 64 if unconfigured.\n \n---rfc::\n-\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n-\tdefault). RFC means \"Request For Comments\"; use this when sending\n-\tan experimental patch for discussion rather than application.\n+--rfc[=]::\n+\tPrepends the string __ (defaults to \"RFC\") to\n+\tthe subject prefix. As the subject prefix defaults to\n+\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n++\n+RFC means \"Request For Comments\"; use this when sending\n+an experimental patch for discussion rather than application.\n+\"--rfc=WIP\" may also be a useful way to indicate that a patch\n+is not complete yet (\"WIP\" stands for \"Work In Progress\").\n \n -v ::\n --reroll-count=::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex c0a8bb95e9..97ca885b33 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1494,6 +1494,19 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+static int rfc_callback(const struct option *opt, const char *arg,\n+\t\t\tint unset)\n+{\n+\tconst char **rfc = opt->value;\n+\n+\t*rfc = opt->value;\n+\tif (unset)\n+\t\t*rfc = NULL;\n+\telse\n+\t\t*rfc = arg ? arg : \"RFC\";\n+\treturn 0;\n+}\n+\n static int numbered_cmdline_opt = 0;\n \n static int numbered_callback(const struct option *opt, const char *arg,\n@@ -1907,8 +1920,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tstruct strbuf sprefix = STRBUF_INIT;\n+\tconst char *rfc = NULL;\n \tint creation_factor = -1;\n-\tint rfc = 0;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n@@ -1932,7 +1945,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"mark the series as Nth re-roll\")),\n \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n \t\t\t N_(\"max length of output filename\")),\n-\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n+\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"rfc\"),\n+\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n+\t\t\t PARSE_OPT_OPTARG, rfc_callback),\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n@@ -2050,8 +2065,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n-\tif (rfc)\n-\t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n+\tif (rfc && rfc[0])\n+\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex e37a1411ee..645c4189f9 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1368,13 +1368,30 @@ test_expect_success 'empty subject prefix does not have extra space' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--rfc' '\n+test_expect_success '--rfc and --no-rfc' '\n \tcat >expect <<-\\EOF &&\n \tSubject: [RFC PATCH 1/1] header with . in it\n \tEOF\n \tgit format-patch -n -1 --stdout --rfc >patch &&\n \tgrep \"^Subject:\" patch >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\tgit format-patch -n -1 --stdout --rfc --no-rfc >patch &&\n+\tsed -e \"s/RFC //\" expect >expect-raw &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect-raw actual\n+'\n+\n+test_expect_success '--rfc=WIP and --rfc=' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [WIP PATCH 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --rfc=WIP >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual &&\n+\tgit format-patch -n -1 --stdout --rfc --rfc= >patch &&\n+\tsed -e \"s/WIP //\" expect >expect-raw &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect-raw actual\n '\n \n test_expect_success '--rfc does not overwrite prefix' '\n-- \n2.45.0-rc0-3-g00e10ef10e\n\n"},{"id":"493363","messageId":"20240423175234.170434-3-gitster@pobox.com","threadId":"61335","inReplyTo":"20240423175234.170434-1-gitster@pobox.com","subject":"[PATCH v4 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-23T17:52:34Z","receivedAt":"2024-04-23T17:52:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In the previous step, the \"--rfc\" option of \"format-patch\" learned\nto take an optional string value to prepend to the subject prefix,\nso that --rfc=WIP can give \"[WIP PATCH]\".\n\nThere may be cases in which the extra string wants to come after the\nsubject prefix. Extend the mechanism to allow \"--rfc=-(WIP)\" [*] to\nsignal that the extra string is to be appended instead of getting\nprepended, resulting in \"[PATCH (WIP)]\".\n\nIn the documentation, discourage (ab)using \"--rfc=-RFC\" to say\n\"[PATCH RFC]\" just to be different, when \"[RFC PATCH]\" is the norm.\n\n[Footnote]\n\n * The syntax takes inspiration from Perl's open syntax that opens\n pipes \"open fh, '|-', 'cmd'\", where the dash signals \"the other\n stuff comes here\".\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-format-patch.txt | 6 ++++++\n builtin/log.c | 8 ++++++--\n t/t4014-format-patch.sh | 9 +++++++++\n 3 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex e553810b1e..369af2c4a7 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -247,6 +247,12 @@ RFC means \"Request For Comments\"; use this when sending\n an experimental patch for discussion rather than application.\n \"--rfc=WIP\" may also be a useful way to indicate that a patch\n is not complete yet (\"WIP\" stands for \"Work In Progress\").\n++\n+If the convention of the receiving community for a particular extra\n+string is to have it _after_ the subject prefix, the string __\n+can be prefixed with a dash (\"`-`\") to signal that the the rest of\n+the __ string should be appended to the subject prefix instead,\n+e.g., `--rfc='-(WIP)'` results in \"PATCH (WIP)\".\n \n -v ::\n --reroll-count=::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 97ca885b33..4750e480e6 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -2065,8 +2065,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n-\tif (rfc && rfc[0])\n-\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n+\tif (rfc && rfc[0]) {\n+\t\tif (rfc[0] == '-')\n+\t\t\tstrbuf_addf(&sprefix, \" %s\", rfc + 1);\n+\t\telse\n+\t\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n+\t}\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 645c4189f9..fcbde15b16 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1394,6 +1394,15 @@ test_expect_success '--rfc=WIP and --rfc=' '\n \ttest_cmp expect-raw actual\n '\n \n+test_expect_success '--rfc=-(WIP) appends' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [PATCH (WIP) 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --rfc=\"-(WIP)\" >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--rfc does not overwrite prefix' '\n \tcat >expect <<-\\EOF &&\n \tSubject: [RFC PATCH foobar 1/1] header with . in it\n-- \n2.45.0-rc0-3-g00e10ef10e\n\n"},{"id":"493397","messageId":"aa628eba-1545-47ac-9773-8aff47ca4adf@gmail.com","threadId":"61335","inReplyTo":"20240423175234.170434-3-gitster@pobox.com","subject":"Re: [PATCH v4 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-04-24T10:16:08Z","receivedAt":"2024-04-24T10:16:12Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 23/04/2024 18:52, Junio C Hamano wrote:\n> In the previous step, the \"--rfc\" option of \"format-patch\" learned\n> to take an optional string value to prepend to the subject prefix,\n> so that --rfc=WIP can give \"[WIP PATCH]\".\n> \n> There may be cases in which the extra string wants to come after the\n> subject prefix. Extend the mechanism to allow \"--rfc=-(WIP)\" [*] to\n> signal that the extra string is to be appended instead of getting\n> prepended, resulting in \"[PATCH (WIP)]\".\n> \n> In the documentation, discourage (ab)using \"--rfc=-RFC\" to say\n> \"[PATCH RFC]\" just to be different, when \"[RFC PATCH]\" is the norm.\n> \n> [Footnote]\n> \n> * The syntax takes inspiration from Perl's open syntax that opens\n> pipes \"open fh, '|-', 'cmd'\", where the dash signals \"the other\n> stuff comes here\".\n\nI'm not convinced this is a good idea as I'm not sure how adding \"RFC\" \nat the end of the subject prefix makes the world better than just having \nat the start of the prefix and I find using \"-\" to do that quite confusing.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/git-format-patch.txt | 6 ++++++\n> builtin/log.c | 8 ++++++--\n> t/t4014-format-patch.sh | 9 +++++++++\n> 3 files changed, 21 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index e553810b1e..369af2c4a7 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -247,6 +247,12 @@ RFC means \"Request For Comments\"; use this when sending\n> an experimental patch for discussion rather than application.\n> \"--rfc=WIP\" may also be a useful way to indicate that a patch\n> is not complete yet (\"WIP\" stands for \"Work In Progress\").\n> ++\n> +If the convention of the receiving community for a particular extra\n> +string is to have it _after_ the subject prefix, the string __\n> +can be prefixed with a dash (\"`-`\") to signal that the the rest of\n> +the __ string should be appended to the subject prefix instead,\n> +e.g., `--rfc='-(WIP)'` results in \"PATCH (WIP)\".\n> \n> -v ::\n> --reroll-count=::\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 97ca885b33..4750e480e6 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -2065,8 +2065,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \tif (cover_from_description_arg)\n> \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n> \n> -\tif (rfc && rfc[0])\n> -\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n> +\tif (rfc && rfc[0]) {\n> +\t\tif (rfc[0] == '-')\n> +\t\t\tstrbuf_addf(&sprefix, \" %s\", rfc + 1);\n> +\t\telse\n> +\t\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n> +\t}\n> \n> \tif (reroll_count) {\n> \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 645c4189f9..fcbde15b16 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1394,6 +1394,15 @@ test_expect_success '--rfc=WIP and --rfc=' '\n> \ttest_cmp expect-raw actual\n> '\n> \n> +test_expect_success '--rfc=-(WIP) appends' '\n> +\tcat >expect <<-\\EOF &&\n> +\tSubject: [PATCH (WIP) 1/1] header with . in it\n> +\tEOF\n> +\tgit format-patch -n -1 --stdout --rfc=\"-(WIP)\" >patch &&\n> +\tgrep \"^Subject:\" patch >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_expect_success '--rfc does not overwrite prefix' '\n> \tcat >expect <<-\\EOF &&\n> \tSubject: [RFC PATCH foobar 1/1] header with . in it\n"},{"id":"493398","messageId":"0664749c-6a85-445f-a794-535024e9fcef@gmail.com","threadId":"61335","inReplyTo":"20240423175234.170434-2-gitster@pobox.com","subject":"Re: [PATCH v4 1/2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-04-24T10:16:13Z","receivedAt":"2024-04-24T10:16:17Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nThis version looks good to me\n\nBest Wishes\n\nPhillip\n\nOn 23/04/2024 18:52, Junio C Hamano wrote:\n> With the \"--rfc\" option, we can tweak the \"[PATCH]\" (or whatever\n> string specified with the \"--subject-prefix\" option, instead of\n> \"PATCH\") that we prefix the title of the commit with into \"[RFC\n> PATCH]\", but some projects may want \"[rfc PATCH]\". Adding a new\n> option, e.g., \"--rfc-lowercase\", to support such need every time\n> somebody wants to use different strings would lead to insanity of\n> accumulating unbounded number of such options.\n> \n> Allow an optional value specified for the option, so that users can\n> use \"--rfc=rfc\" (think of \"--rfc\" without value as a short-hand for\n> \"--rfc=RFC\") if they wanted to.\n> \n> This can of course be (ab)used to make the prefix \"[WIP PATCH]\" by\n> passing \"--rfc=WIP\". Passing an empty string, i.e., \"--rfc=\", is\n> the same as \"--no-rfc\" to override an option given earlier on the\n> same command line.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/git-format-patch.txt | 15 ++++++++++-----\n> builtin/log.c | 23 +++++++++++++++++++----\n> t/t4014-format-patch.sh | 21 +++++++++++++++++++--\n> 3 files changed, 48 insertions(+), 11 deletions(-)\n> \n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index 728bb3821c..e553810b1e 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -20,7 +20,7 @@ SYNOPSIS\n> \t\t [--in-reply-to=] [--suffix=.]\n> \t\t [--ignore-if-in-upstream] [--always]\n> \t\t [--cover-from-description=]\n> -\t\t [--rfc] [--subject-prefix=]\n> +\t\t [--rfc[=]] [--subject-prefix=]\n> \t\t [(--reroll-count|-v) ]\n> \t\t [--to=] [--cc=]\n> \t\t [--[no-]cover-letter] [--quiet]\n> @@ -238,10 +238,15 @@ the patches (with a value of e.g. \"PATCH my-project\").\n> \tvalue of the `format.filenameMaxLength` configuration\n> \tvariable, or 64 if unconfigured.\n> \n> ---rfc::\n> -\tPrepends \"RFC\" to the subject prefix (producing \"RFC PATCH\" by\n> -\tdefault). RFC means \"Request For Comments\"; use this when sending\n> -\tan experimental patch for discussion rather than application.\n> +--rfc[=]::\n> +\tPrepends the string __ (defaults to \"RFC\") to\n> +\tthe subject prefix. As the subject prefix defaults to\n> +\t\"PATCH\", you'll get \"RFC PATCH\" by default.\n> ++\n> +RFC means \"Request For Comments\"; use this when sending\n> +an experimental patch for discussion rather than application.\n> +\"--rfc=WIP\" may also be a useful way to indicate that a patch\n> +is not complete yet (\"WIP\" stands for \"Work In Progress\").\n> \n> -v ::\n> --reroll-count=::\n> diff --git a/builtin/log.c b/builtin/log.c\n> index c0a8bb95e9..97ca885b33 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1494,6 +1494,19 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,\n> \treturn 0;\n> }\n> \n> +static int rfc_callback(const struct option *opt, const char *arg,\n> +\t\t\tint unset)\n> +{\n> +\tconst char **rfc = opt->value;\n> +\n> +\t*rfc = opt->value;\n> +\tif (unset)\n> +\t\t*rfc = NULL;\n> +\telse\n> +\t\t*rfc = arg ? arg : \"RFC\";\n> +\treturn 0;\n> +}\n> +\n> static int numbered_cmdline_opt = 0;\n> \n> static int numbered_callback(const struct option *opt, const char *arg,\n> @@ -1907,8 +1920,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \tstruct strbuf rdiff2 = STRBUF_INIT;\n> \tstruct strbuf rdiff_title = STRBUF_INIT;\n> \tstruct strbuf sprefix = STRBUF_INIT;\n> +\tconst char *rfc = NULL;\n> \tint creation_factor = -1;\n> -\tint rfc = 0;\n> \n> \tconst struct option builtin_format_patch_options[] = {\n> \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n> @@ -1932,7 +1945,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"mark the series as Nth re-roll\")),\n> \t\tOPT_INTEGER(0, \"filename-max-length\", &fmt_patch_name_max,\n> \t\t\t N_(\"max length of output filename\")),\n> -\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\n> +\t\tOPT_CALLBACK_F(0, \"rfc\", &rfc, N_(\"rfc\"),\n> +\t\t\t N_(\"add (default 'RFC') before 'PATCH'\"),\n> +\t\t\t PARSE_OPT_OPTARG, rfc_callback),\n> \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n> \t\t\t N_(\"cover-from-description-mode\"),\n> \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n> @@ -2050,8 +2065,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \tif (cover_from_description_arg)\n> \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n> \n> -\tif (rfc)\n> -\t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n> +\tif (rfc && rfc[0])\n> +\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n> \n> \tif (reroll_count) {\n> \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index e37a1411ee..645c4189f9 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1368,13 +1368,30 @@ test_expect_success 'empty subject prefix does not have extra space' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_success '--rfc' '\n> +test_expect_success '--rfc and --no-rfc' '\n> \tcat >expect <<-\\EOF &&\n> \tSubject: [RFC PATCH 1/1] header with . in it\n> \tEOF\n> \tgit format-patch -n -1 --stdout --rfc >patch &&\n> \tgrep \"^Subject:\" patch >actual &&\n> -\ttest_cmp expect actual\n> +\ttest_cmp expect actual &&\n> +\tgit format-patch -n -1 --stdout --rfc --no-rfc >patch &&\n> +\tsed -e \"s/RFC //\" expect >expect-raw &&\n> +\tgrep \"^Subject:\" patch >actual &&\n> +\ttest_cmp expect-raw actual\n> +'\n> +\n> +test_expect_success '--rfc=WIP and --rfc=' '\n> +\tcat >expect <<-\\EOF &&\n> +\tSubject: [WIP PATCH 1/1] header with . in it\n> +\tEOF\n> +\tgit format-patch -n -1 --stdout --rfc=WIP >patch &&\n> +\tgrep \"^Subject:\" patch >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit format-patch -n -1 --stdout --rfc --rfc= >patch &&\n> +\tsed -e \"s/WIP //\" expect >expect-raw &&\n> +\tgrep \"^Subject:\" patch >actual &&\n> +\ttest_cmp expect-raw actual\n> '\n> \n> test_expect_success '--rfc does not overwrite prefix' '\n"},{"id":"493399","messageId":"e4206b6d-13bb-4332-93b3-09721c1b4152@gmail.com","threadId":"61335","inReplyTo":"f9aae9692493e4b722ce9f38de73c810@manjaro.org","subject":"Re: [PATCH v3 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-04-24T10:17:55Z","receivedAt":"2024-04-24T10:18:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dragan\n\nOn 21/04/2024 20:37, Dragan Simic wrote:\n> Hello Junio,\n> \n> On 2024-04-21 20:59, Junio C Hamano wrote:\n>> In the previous step, the \"--rfc\" option of \"format-patch\" learned\n>> to take an optional string value to prepend to the subject prefix,\n>> so that --rfc=WIP can give \"[WIP PATCH]\".  This commit shows that\n>> the mechanism can be extended easily to allow \"--rfc=-(WIP)\" [*1*]\n>> to signal that the extra string is to be appended instead of getting\n>> prepended, resulting in \"[PATCH (WIP)]\".\n>>\n>> Having worked on the patch, I am personally not 100% on board with\n>> this part of the feature myself, and that is why this update is a\n>> separate step from the main \"--rfc takes an optional string value\"\n>> step.\n>>\n>> If a way to prepend an arbitrary adornment is added to the system,\n>> and people can now say \"--rfc=RESEND\" to produce \"[RESEND PATCH]\",\n>> those who used to use \"[PATCH RESEND]\" by tweaking the subject\n>> prefix in other ways [*2*] would do one of three things:\n> \n> There are even more issues, such as the grammar-related ones. \n\nI think it is best to view the subject prefix as a list of space \nseparated labels or keywords rather than part of a grammatically correct \nsentence.\n\nBest Wishes\n\nPhillip\n\n Let\n> me explain, please, as accurately as I can do that as a non-native\n> English speaker who spent many years studying English grammar.\n> \n> Writing \"RFC PATCH\" puts \"RFC\" into the role of an adjective, which\n> is fine.  The same obviously applies to \"WIP PATCH\".  On the other\n> hand, writing \"RESEND PATCH\" puts \"RESEND\" into its only possible\n> role, which is transitive verb, and results in \"RESEND PATCH\" that\n> serves as a very simple sentence in imperative mood.  I'm not sure\n> that, strictly technically speaking, having simple sentences as the\n> prefixes is the desired outcome.\n> \n> Technically, we should use \"RE-SENT PATCH\" to be on the correct side\n> from the grammar perspective, with \"RE-SENT\" serving as an adjective.\n> Before you ask, no, we can't use \"RESENT\" there, because its meaning\n> is completely different.  However, nobody uses \"RE-SENT PATCH\", or\n> at least I haven't seen it used yet.\n> \n> When it comes to \"PATCH RESEND\", \"RESEND\" remains in its transitive\n> verb role, but \"PATCH\", as a noun, becomes a modifier of the verb.\n> Thus, the resulting meaning of \"PATCH RESEND\" becomes something like\n> \"resend an item that's a patch\", but not written in form of a simple\n> (or less simple) sentence.  Strictly technically, a noun should only\n> modify another noun, but bending English grammar a bit this way is\n> much better than actually having a simple sentence as a prefix.\n> \n> With all this in mind, I think that allowing the \"--rfc=-\"\n> form is the way to go.  Computer lingo often bends English grammar\n> to a certain degree, to achieve compactness, but bending and dumbing\n> it down more that it's actually necessary isn't something that we\n> should embrace.\n> \n> I hope all this makes sense.\n> \n>>  (1) keep using their existing ways and keep sending their message\n>>      with the \"[RESEND PATCH]\" prefix.\n>>\n>>  (2) rejoice in the new automation, switch to use \"--rfc=RESEND\",\n>>      and start sending their messages with \"[RESEND PATCH]\" prefix\n>>      instead of \"[PATCH RESEND]\" prefix.\n>>\n>>  (3) complain and demand a way to append instead of prepend so that\n>>      they can use an automation to produce \"[PATCH RESEND]\" prefix.\n>>\n>> I do not believe in adding slightly different ways that allow users\n>> to be \"original\" when such differences do not make the world better\n>> in meaningful ways [*3*], and I expect there are many more folks who\n>> share that sentiment and go to route (2) than those who go to route\n>> (3).  If my expectation is true, it means that this patch goes in a\n>> wrong direction, and I would be happy to drop it.\n>>\n>>\n>> [Footnote]\n>>\n>>  *1* The syntax takes inspiration from Perl's three-arg open syntax\n>>      that uses pipes \"open fh, '|-', 'cmd'\", where the dash signals\n>>      \"the other stuff comes here\".\n>>\n>>  *2* ... because \"--rfc\" in released versions does not even take any\n>>      string value to prepend, let alone append, to begin with.\n>>\n>>  *3* \n>> https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/\n>>      gathered some stats to observe that \"[RFC PATCH]\" is more\n>>      common than \"[PATCH RFC]\" by a wide margin, while trying to see\n>>      how common \"[RESEND PATCH]\" (or \"[PATCH RESED]\") were used (the\n>>      answer: much less common).  But it wouldn't have needed to\n>>      count \"[PATCH RFC]\" and \"[RFC PATCH]\" separately if using a\n>>      prefix and not a suffix (or vice versa) were established more\n>>      firmly as the standard practice.\n>>\n>>      It is a fine example that useless diversity making needless\n>>      work.\n"},{"id":"493407","messageId":"xmqqplue2361.fsf@gitster.g","threadId":"61335","inReplyTo":"aa628eba-1545-47ac-9773-8aff47ca4adf@gmail.com","subject":"Re: [PATCH v4 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-24T15:25:42Z","receivedAt":"2024-04-24T15:25:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> I'm not convinced this is a good idea as I'm not sure how adding \"RFC\"\n> at the end of the subject prefix makes the world better than just\n> having at the start of the prefix and I find using \"-\" to do that\n> quite confusing.\n\nI am not convinced it is a good idea, either. \"PATCH (WIP)\" was the\nbest example I could come up with. I am also a fan of \"a list of\nspace separated labels or keywords\" you mentioned, but *if* a\nproject convention somewhere is to have them before \"PATCH\", then it\nis not entirely unreasonable to wish to have a way to prepend these\nlabels.\n\nBut I am fine to drop it for the sake of simplicity. It would help\ndiscourage users from trying to be \"original\" in a way that does not\nmake a material difference. If a project comes with a concrete need\nto prepend, the patch is always resurrectable from the list archive.\n\nAs to the syntax, I think \"-\" is a fairly good way to indicate\nwhether it goes to the front or back. When told to \"Combine '-RFC'\nand 'PATCH'\", I expect that most people would give 'PATCH-RFC' and\nnot '-RFC PATCH'.\n\nThanks.\n"},{"id":"493409","messageId":"115acd1529d9529ef5bb095c074ad83d@manjaro.org","threadId":"61335","inReplyTo":"e4206b6d-13bb-4332-93b3-09721c1b4152@gmail.com","subject":"Re: [PATCH v3 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-24T15:52:39Z","receivedAt":"2024-04-24T15:52:41Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Phillip,\n\nOn 2024-04-24 12:17, Phillip Wood wrote:\n> On 21/04/2024 20:37, Dragan Simic wrote:\n>> On 2024-04-21 20:59, Junio C Hamano wrote:\n>>> In the previous step, the \"--rfc\" option of \"format-patch\" learned\n>>> to take an optional string value to prepend to the subject prefix,\n>>> so that --rfc=WIP can give \"[WIP PATCH]\".  This commit shows that\n>>> the mechanism can be extended easily to allow \"--rfc=-(WIP)\" [*1*]\n>>> to signal that the extra string is to be appended instead of getting\n>>> prepended, resulting in \"[PATCH (WIP)]\".\n>>> \n>>> Having worked on the patch, I am personally not 100% on board with\n>>> this part of the feature myself, and that is why this update is a\n>>> separate step from the main \"--rfc takes an optional string value\"\n>>> step.\n>>> \n>>> If a way to prepend an arbitrary adornment is added to the system,\n>>> and people can now say \"--rfc=RESEND\" to produce \"[RESEND PATCH]\",\n>>> those who used to use \"[PATCH RESEND]\" by tweaking the subject\n>>> prefix in other ways [*2*] would do one of three things:\n>> \n>> There are even more issues, such as the grammar-related ones.\n> \n> I think it is best to view the subject prefix as a list of space\n> separated labels or keywords rather than part of a grammatically\n> correct sentence.\n\nWith all due respect, I strongly disagree. Viewing it that way and\nletting \"[RESEND PATCH]\" be accepted as correct (and even enforced\na bit) is exactly what I below referred to as embracing the bending\nof English grammar beyond what's actually necessary.\n\nPlease, let me remind you that I spent more than a couple of years\non English Wikipedia, writing new and expanding already existing\ncomputing-related articles, during which time I participated in more\nthan a few grammar-related discussions. All that makes me more\n\"sensitive\" to breaking the English grammar rules when that actually\nisn't necessary or beneficial.\n\n> Let\n>> me explain, please, as accurately as I can do that as a non-native\n>> English speaker who spent many years studying English grammar.\n>> \n>> Writing \"RFC PATCH\" puts \"RFC\" into the role of an adjective, which\n>> is fine.  The same obviously applies to \"WIP PATCH\".  On the other\n>> hand, writing \"RESEND PATCH\" puts \"RESEND\" into its only possible\n>> role, which is transitive verb, and results in \"RESEND PATCH\" that\n>> serves as a very simple sentence in imperative mood.  I'm not sure\n>> that, strictly technically speaking, having simple sentences as the\n>> prefixes is the desired outcome.\n>> \n>> Technically, we should use \"RE-SENT PATCH\" to be on the correct side\n>> from the grammar perspective, with \"RE-SENT\" serving as an adjective.\n>> Before you ask, no, we can't use \"RESENT\" there, because its meaning\n>> is completely different.  However, nobody uses \"RE-SENT PATCH\", or\n>> at least I haven't seen it used yet.\n>> \n>> When it comes to \"PATCH RESEND\", \"RESEND\" remains in its transitive\n>> verb role, but \"PATCH\", as a noun, becomes a modifier of the verb.\n>> Thus, the resulting meaning of \"PATCH RESEND\" becomes something like\n>> \"resend an item that's a patch\", but not written in form of a simple\n>> (or less simple) sentence.  Strictly technically, a noun should only\n>> modify another noun, but bending English grammar a bit this way is\n>> much better than actually having a simple sentence as a prefix.\n>> \n>> With all this in mind, I think that allowing the \"--rfc=-\"\n>> form is the way to go.  Computer lingo often bends English grammar\n>> to a certain degree, to achieve compactness, but bending and dumbing\n>> it down more that it's actually necessary isn't something that we\n>> should embrace.\n>> \n>> I hope all this makes sense.\n>> \n>>>  (1) keep using their existing ways and keep sending their message\n>>>      with the \"[RESEND PATCH]\" prefix.\n>>> \n>>>  (2) rejoice in the new automation, switch to use \"--rfc=RESEND\",\n>>>      and start sending their messages with \"[RESEND PATCH]\" prefix\n>>>      instead of \"[PATCH RESEND]\" prefix.\n>>> \n>>>  (3) complain and demand a way to append instead of prepend so that\n>>>      they can use an automation to produce \"[PATCH RESEND]\" prefix.\n>>> \n>>> I do not believe in adding slightly different ways that allow users\n>>> to be \"original\" when such differences do not make the world better\n>>> in meaningful ways [*3*], and I expect there are many more folks who\n>>> share that sentiment and go to route (2) than those who go to route\n>>> (3).  If my expectation is true, it means that this patch goes in a\n>>> wrong direction, and I would be happy to drop it.\n>>> \n>>> \n>>> [Footnote]\n>>> \n>>>  *1* The syntax takes inspiration from Perl's three-arg open syntax\n>>>      that uses pipes \"open fh, '|-', 'cmd'\", where the dash signals\n>>>      \"the other stuff comes here\".\n>>> \n>>>  *2* ... because \"--rfc\" in released versions does not even take any\n>>>      string value to prepend, let alone append, to begin with.\n>>> \n>>>  *3* \n>>> https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/\n>>>      gathered some stats to observe that \"[RFC PATCH]\" is more\n>>>      common than \"[PATCH RFC]\" by a wide margin, while trying to see\n>>>      how common \"[RESEND PATCH]\" (or \"[PATCH RESED]\") were used (the\n>>>      answer: much less common).  But it wouldn't have needed to\n>>>      count \"[PATCH RFC]\" and \"[RFC PATCH]\" separately if using a\n>>>      prefix and not a suffix (or vice versa) were established more\n>>>      firmly as the standard practice.\n>>> \n>>>      It is a fine example that useless diversity making needless\n>>>      work.\n"},{"id":"493410","messageId":"ff5003c369c5247ad1a0b3a51d1581db@manjaro.org","threadId":"61335","inReplyTo":"aa628eba-1545-47ac-9773-8aff47ca4adf@gmail.com","subject":"Re: [PATCH v4 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-24T15:58:06Z","receivedAt":"2024-04-24T15:58:09Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Phillip,\n\nOn 2024-04-24 12:16, Phillip Wood wrote:\n> On 23/04/2024 18:52, Junio C Hamano wrote:\n>> In the previous step, the \"--rfc\" option of \"format-patch\" learned\n>> to take an optional string value to prepend to the subject prefix,\n>> so that --rfc=WIP can give \"[WIP PATCH]\".\n>> \n>> There may be cases in which the extra string wants to come after the\n>> subject prefix. Extend the mechanism to allow \"--rfc=-(WIP)\" [*] to\n>> signal that the extra string is to be appended instead of getting\n>> prepended, resulting in \"[PATCH (WIP)]\".\n>> \n>> In the documentation, discourage (ab)using \"--rfc=-RFC\" to say\n>> \"[PATCH RFC]\" just to be different, when \"[RFC PATCH]\" is the norm.\n>> \n>> [Footnote]\n>> \n>> * The syntax takes inspiration from Perl's open syntax that opens\n>> pipes \"open fh, '|-', 'cmd'\", where the dash signals \"the other\n>> stuff comes here\".\n> \n> I'm not convinced this is a good idea as I'm not sure how adding \"RFC\"\n> at the end of the subject prefix makes the world better than just\n> having at the start of the prefix and I find using \"-\" to do that\n> quite confusing.\n\nPlease, read my earlier responses [1][2] to see why does this\nfeature actually make the world a bit better. To sum it up, just\nas there's bit rot, there's also English grammar rot, which we\nshouldn't embrace or promote.\n\n[1] \nhttps://lore.kernel.org/git/f9aae9692493e4b722ce9f38de73c810@manjaro.org/\n[2] \nhttps://lore.kernel.org/git/115acd1529d9529ef5bb095c074ad83d@manjaro.org/\n\n>> Signed-off-by: Junio C Hamano \n>> ---\n>> Documentation/git-format-patch.txt | 6 ++++++\n>> builtin/log.c | 8 ++++++--\n>> t/t4014-format-patch.sh | 9 +++++++++\n>> 3 files changed, 21 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/Documentation/git-format-patch.txt \n>> b/Documentation/git-format-patch.txt\n>> index e553810b1e..369af2c4a7 100644\n>> --- a/Documentation/git-format-patch.txt\n>> +++ b/Documentation/git-format-patch.txt\n>> @@ -247,6 +247,12 @@ RFC means \"Request For Comments\"; use this when \n>> sending\n>> an experimental patch for discussion rather than application.\n>> \"--rfc=WIP\" may also be a useful way to indicate that a patch\n>> is not complete yet (\"WIP\" stands for \"Work In Progress\").\n>> ++\n>> +If the convention of the receiving community for a particular extra\n>> +string is to have it _after_ the subject prefix, the string __\n>> +can be prefixed with a dash (\"`-`\") to signal that the the rest of\n>> +the __ string should be appended to the subject prefix instead,\n>> +e.g., `--rfc='-(WIP)'` results in \"PATCH (WIP)\".\n>> -v ::\n>> --reroll-count=::\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 97ca885b33..4750e480e6 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -2065,8 +2065,12 @@ int cmd_format_patch(int argc, const char \n>> **argv, const char *prefix)\n>> \tif (cover_from_description_arg)\n>> \t\tcover_from_description_mode = \n>> parse_cover_from_description(cover_from_description_arg);\n>> -\tif (rfc && rfc[0])\n>> -\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n>> +\tif (rfc && rfc[0]) {\n>> +\t\tif (rfc[0] == '-')\n>> +\t\t\tstrbuf_addf(&sprefix, \" %s\", rfc + 1);\n>> +\t\telse\n>> +\t\t\tstrbuf_insertf(&sprefix, 0, \"%s \", rfc);\n>> +\t}\n>> \tif (reroll_count) {\n>> \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n>> index 645c4189f9..fcbde15b16 100755\n>> --- a/t/t4014-format-patch.sh\n>> +++ b/t/t4014-format-patch.sh\n>> @@ -1394,6 +1394,15 @@ test_expect_success '--rfc=WIP and --rfc=' '\n>> \ttest_cmp expect-raw actual\n>> '\n>> +test_expect_success '--rfc=-(WIP) appends' '\n>> +\tcat >expect <<-\\EOF &&\n>> +\tSubject: [PATCH (WIP) 1/1] header with . in it\n>> +\tEOF\n>> +\tgit format-patch -n -1 --stdout --rfc=\"-(WIP)\" >patch &&\n>> +\tgrep \"^Subject:\" patch >actual &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>> test_expect_success '--rfc does not overwrite prefix' '\n>> \tcat >expect <<-\\EOF &&\n>> \tSubject: [RFC PATCH foobar 1/1] header with . in it\n"},{"id":"493419","messageId":"6f379c7c2fcca2194dcb28032216b4b1@manjaro.org","threadId":"61335","inReplyTo":"xmqqplue2361.fsf@gitster.g","subject":"Re: [PATCH v4 2/2] format-patch: \"--rfc=-(WIP)\" appends to produce [PATCH (WIP)]","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-24T16:34:13Z","receivedAt":"2024-04-24T16:34:15Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Junio,\n\nOn 2024-04-24 17:25, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> I'm not convinced this is a good idea as I'm not sure how adding \"RFC\"\n>> at the end of the subject prefix makes the world better than just\n>> having at the start of the prefix and I find using \"-\" to do that\n>> quite confusing.\n> \n> I am not convinced it is a good idea, either. \"PATCH (WIP)\" was the\n> best example I could come up with. I am also a fan of \"a list of\n> space separated labels or keywords\" you mentioned, but *if* a\n> project convention somewhere is to have them before \"PATCH\", then it\n> is not entirely unreasonable to wish to have a way to prepend these\n> labels.\n> \n> But I am fine to drop it for the sake of simplicity. It would help\n> discourage users from trying to be \"original\" in a way that does not\n> make a material difference. If a project comes with a concrete need\n> to prepend, the patch is always resurrectable from the list archive.\n\nYes, it would help with discouraging the users from becoming\n\"inventive\", but would also promote the rot of English grammar,\nas I already tried to explain. [1][2]\n\nI'm always for simplicity, unless it actually results in some\npossibly negative effects.\n\n[1] \nhttps://lore.kernel.org/git/f9aae9692493e4b722ce9f38de73c810@manjaro.org/\n[2] \nhttps://lore.kernel.org/git/115acd1529d9529ef5bb095c074ad83d@manjaro.org/\n\n> As to the syntax, I think \"-\" is a fairly good way to indicate\n> whether it goes to the front or back. When told to \"Combine '-RFC'\n> and 'PATCH'\", I expect that most people would give 'PATCH-RFC' and\n> not '-RFC PATCH'.\n\nI find the syntax just fine.\n"}]}