{"thread":{"id":"60172","subject":"[PATCH v4] format-patch: --rfc honors what --subject-prefix sets","startedAt":"2023-08-30T18:29:52Z","lastAt":"2023-09-01T16:48:53Z","messageCount":7,"participants":["Drew DeVault","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"481188","messageId":"20230830064646.30904-1-sir@cmpwn.com","threadId":"60172","inReplyTo":null,"subject":"[PATCH v4] format-patch: --rfc honors what --subject-prefix sets","fromName":"Drew DeVault","fromEmail":"sir@cmpwn.com","sentAt":"2023-08-30T06:43:33Z","receivedAt":"2023-08-30T18:29:52Z","isPatch":true,"sender":{"key":"sir@cmpwn.com","avatar":"https://avatars.githubusercontent.com/u/1310872?v=4"},"body":"Rather than replacing the configured subject prefix (either through the\ngit config or command line) entirely with \"RFC PATCH\", this change\nprepends RFC to whatever subject prefix was already in use.\n\nThis is useful, for example, when a user is working on a repository that\nhas a subject prefix considered to disambiguate patches:\n\n\tgit config format.subjectPrefix 'PATCH my-project'\n\nPrior to this change, formatting patches with --rfc would lose the\n'my-project' information.\n\nSigned-off-by: Drew DeVault <sir@cmpwn.com>\n---\nv4 refactors --subject-prefix to interact directly with the sbuffer same\nas anything else that manipulates the prefix. Thanks for the suggestion,\nJunio, this is much better.\n\nMinor correction to the documentation is also included, and a second\ntest just for good measure which demonstrates that the order of\narguments no longer important.\n\n Documentation/git-format-patch.txt | 18 +++++++++++------\n builtin/log.c                      | 31 +++++++++++++++---------------\n t/t4014-format-patch.sh            | 22 ++++++++++++++++++++-\n 3 files changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 373b46fc0d..b96e142a8d 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -217,9 +217,15 @@ populated with placeholder text.\n \n --subject-prefix=<subject prefix>::\n \tInstead of the standard '[PATCH]' prefix in the subject\n-\tline, instead use '[<subject prefix>]'. This\n-\tallows for useful naming of a patch series, and can be\n-\tcombined with the `--numbered` option.\n+\tline, instead use '[<subject prefix>]'. This can be used\n+\tto name a patch series, and can be combined with the\n+\t`--numbered` option.\n++\n+The configuration variable `format.subjectPrefix` may also be used\n+to to configure a subject prefix to apply to a given repository for\n+all patches. This is often useful on mailing lists which receive\n+patches for several repositories and can be used to disambiguate\n+the patches (with a value of e.g. \"PATCH my-project\").\n \n --filename-max-length=<n>::\n \tInstead of the standard 64 bytes, chomp the generated output\n@@ -229,9 +235,9 @@ populated with placeholder text.\n \tvariable, or 64 if unconfigured.\n \n --rfc::\n-\tAlias for `--subject-prefix=\"RFC PATCH\"`. RFC means \"Request For\n-\tComments\"; use this when sending an experimental patch for\n-\tdiscussion rather than application.\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 \n -v <n>::\n --reroll-count=<n>::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex db3a88bfe9..29c86dc798 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1468,19 +1468,16 @@ static int subject_prefix = 0;\n static int subject_prefix_callback(const struct option *opt, const char *arg,\n \t\t\t    int unset)\n {\n+\tstruct strbuf *sprefix;\n+\n \tBUG_ON_OPT_NEG(unset);\n+\tsprefix = (struct strbuf *)opt->value;\n \tsubject_prefix = 1;\n-\t((struct rev_info *)opt->value)->subject_prefix = arg;\n+\tstrbuf_reset(sprefix);\n+\tstrbuf_addstr(sprefix, arg);\n \treturn 0;\n }\n \n-static int rfc_callback(const struct option *opt, const char *arg, int unset)\n-{\n-\tBUG_ON_OPT_NEG(unset);\n-\tBUG_ON_OPT_ARG(arg);\n-\treturn subject_prefix_callback(opt, \"RFC PATCH\", unset);\n-}\n-\n static int numbered_cmdline_opt = 0;\n \n static int numbered_callback(const struct option *opt, const char *arg,\n@@ -1907,6 +1904,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tstruct strbuf sprefix = 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@@ -1930,13 +1928,11 @@ 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\", &rev, NULL,\n-\t\t\t    N_(\"use [RFC PATCH] instead of [PATCH]\"),\n-\t\t\t    PARSE_OPT_NOARG | PARSE_OPT_NONEG, rfc_callback),\n+\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\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-\t\tOPT_CALLBACK_F(0, \"subject-prefix\", &rev, N_(\"prefix\"),\n+\t\tOPT_CALLBACK_F(0, \"subject-prefix\", &sprefix, N_(\"prefix\"),\n \t\t\t    N_(\"use [<prefix>] instead of [PATCH]\"),\n \t\t\t    PARSE_OPT_NONEG, subject_prefix_callback),\n \t\tOPT_CALLBACK_F('o', \"output-directory\", &output_directory,\n@@ -2016,11 +2012,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.max_parents = 1;\n \trev.diffopt.flags.recursive = 1;\n \trev.diffopt.no_free = 1;\n-\trev.subject_prefix = fmt_patch_subject_prefix;\n \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n \ts_r_opt.def = \"HEAD\";\n \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n \n+\tstrbuf_addstr(&sprefix, fmt_patch_subject_prefix);\n \tif (format_no_prefix)\n \t\tdiff_set_noprefix(&rev.diffopt);\n \n@@ -2048,13 +2044,16 @@ 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+\n \tif (reroll_count) {\n-\t\tstrbuf_addf(&sprefix, \"%s v%s\",\n-\t\t\t    rev.subject_prefix, reroll_count);\n+\t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n \t\trev.reroll_count = reroll_count;\n-\t\trev.subject_prefix = sprefix.buf;\n \t}\n \n+\trev.subject_prefix = sprefix.buf;\n+\n \tfor (i = 0; i < extra_hdr.nr; i++) {\n \t\tstrbuf_addstr(&buf, extra_hdr.items[i].string);\n \t\tstrbuf_addch(&buf, '\\n');\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 3cf2b7a7fb..9fa1f3bc7a 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1373,7 +1373,27 @@ test_expect_success '--rfc' '\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+\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+\tEOF\n+\tgit -c format.subjectPrefix=\"PATCH foobar\" \\\n+\t\tformat-patch -n -1 --stdout --rfc >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--rfc is argument order independent' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [RFC PATCH foobar 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --rfc \\\n+\t\t--subject-prefix=\"PATCH foobar\" >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.42.0\n\n"},{"id":"481195","messageId":"xmqqsf808h4g.fsf@gitster.g","threadId":"60172","inReplyTo":"20230830064646.30904-1-sir@cmpwn.com","subject":"Re: [PATCH v4] format-patch: --rfc honors what --subject-prefix sets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-30T19:28:15Z","receivedAt":"2023-08-30T20:46:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Drew DeVault <sir@cmpwn.com> writes:\n\n> Minor correction to the documentation is also included, and a second\n> test just for good measure which demonstrates that the order of\n> arguments no longer important.\n\nPerfect.\n\n>  Documentation/git-format-patch.txt | 18 +++++++++++------\n>  builtin/log.c                      | 31 +++++++++++++++---------------\n>  t/t4014-format-patch.sh            | 22 ++++++++++++++++++++-\n>  3 files changed, 48 insertions(+), 23 deletions(-)\n>\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index 373b46fc0d..b96e142a8d 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -217,9 +217,15 @@ populated with placeholder text.\n>  \n>  --subject-prefix=<subject prefix>::\n>  \tInstead of the standard '[PATCH]' prefix in the subject\n> -\tline, instead use '[<subject prefix>]'. This\n> -\tallows for useful naming of a patch series, and can be\n> -\tcombined with the `--numbered` option.\n> +\tline, instead use '[<subject prefix>]'. This can be used\n> +\tto name a patch series, and can be combined with the\n> +\t`--numbered` option.\n> ++\n> +The configuration variable `format.subjectPrefix` may also be used\n> +to to configure a subject prefix to apply to a given repository for\n> +all patches. This is often useful on mailing lists which receive\n> +patches for several repositories and can be used to disambiguate\n> +the patches (with a value of e.g. \"PATCH my-project\").\n\nNice.\n\nI'll locally fix \"to to\" -> \"to\" while queuing; no need to reroll\nonly to fix this.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index db3a88bfe9..29c86dc798 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1468,19 +1468,16 @@ static int subject_prefix = 0;\n>  static int subject_prefix_callback(const struct option *opt, const char *arg,\n>  \t\t\t    int unset)\n>  {\n> +\tstruct strbuf *sprefix;\n> +\n>  \tBUG_ON_OPT_NEG(unset);\n> +\tsprefix = (struct strbuf *)opt->value;\n>  \tsubject_prefix = 1;\n> -\t((struct rev_info *)opt->value)->subject_prefix = arg;\n> +\tstrbuf_reset(sprefix);\n> +\tstrbuf_addstr(sprefix, arg);\n>  \treturn 0;\n>  }\n\nOK.\n\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 3cf2b7a7fb..9fa1f3bc7a 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1373,7 +1373,27 @@ test_expect_success '--rfc' '\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> +\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> +\tEOF\n> +\tgit -c format.subjectPrefix=\"PATCH foobar\" \\\n> +\t\tformat-patch -n -1 --stdout --rfc >patch &&\n> +\tgrep \"^Subject:\" patch >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success '--rfc is argument order independent' '\n> +\tcat >expect <<-\\EOF &&\n> +\tSubject: [RFC PATCH foobar 1/1] header with . in it\n> +\tEOF\n> +\tgit format-patch -n -1 --stdout --rfc \\\n> +\t\t--subject-prefix=\"PATCH foobar\" >patch &&\n> +\tgrep \"^Subject:\" patch >actual &&\n>  \ttest_cmp expect actual\n>  '\n\nNice.\n\nWill queue.  Let's wait to see if others find something fishy for a\nday or two and then merge it down to 'next'.\n\nThanks.\n"},{"id":"481268","messageId":"20230831212950.GA949706@coredump.intra.peff.net","threadId":"60172","inReplyTo":"xmqqsf808h4g.fsf@gitster.g","subject":"Re: [PATCH v4] format-patch: --rfc honors what --subject-prefix sets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-08-31T21:29:50Z","receivedAt":"2023-08-31T21:29:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 30, 2023 at 12:28:15PM -0700, Junio C Hamano wrote:\n\n> Will queue.  Let's wait to see if others find something fishy for a\n> day or two and then merge it down to 'next'.\n\nIt looks good to me, and I'm much happier with where the refactoring\nended up compared to the earlier versions. I did have two nits, but I'm\ncontent if neither is addressed.\n\nOne is that the commit message doesn't really describe the refactoring\nof --subject-prefix. I'm OK with that rationale being in the list\narchive, though.\n\n> >  static int subject_prefix_callback(const struct option *opt, const char *arg,\n> >  \t\t\t    int unset)\n> >  {\n> > +\tstruct strbuf *sprefix;\n> > +\n> >  \tBUG_ON_OPT_NEG(unset);\n> > +\tsprefix = (struct strbuf *)opt->value;\n> >  \tsubject_prefix = 1;\n> > -\t((struct rev_info *)opt->value)->subject_prefix = arg;\n> > +\tstrbuf_reset(sprefix);\n> > +\tstrbuf_addstr(sprefix, arg);\n> >  \treturn 0;\n> >  }\n> \n> OK.\n\nThe cast is unnecessary here, since opt->value is a void pointer which\nallows implicit casts. Just:\n\n  struct strbuf *sprefix = opt->value;\n\nis IMHO a little more readable. But as we're just passing it along to\nstrbuf functions anyway, it would also work to do:\n\n  strbuf_reset(opt->value);\n  strbuf_addstr(opt->value, arg);\n\nI think we're deep into questions of style / preference here, so I'm OK\nwith any of them. It's probably only that I've recently been refactoring\nso many parseopt callbacks with the same pattern that I have opinions at\nall. ;)\n\n-Peff\n"},{"id":"481271","messageId":"xmqqv8cuswah.fsf@gitster.g","threadId":"60172","inReplyTo":"20230831212950.GA949706@coredump.intra.peff.net","subject":"Re: [PATCH v4] format-patch: --rfc honors what --subject-prefix sets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-31T22:04:54Z","receivedAt":"2023-08-31T22:05:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Aug 30, 2023 at 12:28:15PM -0700, Junio C Hamano wrote:\n>\n>> Will queue.  Let's wait to see if others find something fishy for a\n>> day or two and then merge it down to 'next'.\n>\n> It looks good to me, and I'm much happier with where the refactoring\n> ended up compared to the earlier versions. I did have two nits, but I'm\n> content if neither is addressed.\n>\n> One is that the commit message doesn't really describe the refactoring\n> of --subject-prefix. I'm OK with that rationale being in the list\n> archive, though.\n\nYeah, I agree that the reason for the change deserves to be\nrecorded.\n\n>> >  static int subject_prefix_callback(const struct option *opt, const char *arg,\n>> >  \t\t\t    int unset)\n>> >  {\n>> > +\tstruct strbuf *sprefix;\n>> > +\n>> >  \tBUG_ON_OPT_NEG(unset);\n>> > +\tsprefix = (struct strbuf *)opt->value;\n>> >  \tsubject_prefix = 1;\n>> > -\t((struct rev_info *)opt->value)->subject_prefix = arg;\n>> > +\tstrbuf_reset(sprefix);\n>> > +\tstrbuf_addstr(sprefix, arg);\n>> >  \treturn 0;\n>> >  }\n>> \n>> OK.\n>\n> The cast is unnecessary here, since opt->value is a void pointer which\n> allows implicit casts. Just:\n>\n>   struct strbuf *sprefix = opt->value;\n>\n> is IMHO a little more readable. But as we're just passing it along to\n> strbuf functions anyway, it would also work to do:\n>\n>   strbuf_reset(opt->value);\n>   strbuf_addstr(opt->value, arg);\n>\n> I think we're deep into questions of style / preference here, so I'm OK\n> with any of them. It's probably only that I've recently been refactoring\n> so many parseopt callbacks with the same pattern that I have opinions at\n> all. ;)\n\nSure.  FWIW, I like giving a meaningful name to a thing that is used\nmore than once, so my preference is the original minus excess cast.\n\nThanks for a review.  Here is a tentative rewrite\n\n--- >8 ---\nFrom e0d7db7423a91673c001aaa5e580c815ce2f7f92 Mon Sep 17 00:00:00 2001\nFrom: Drew DeVault <sir@cmpwn.com>\nDate: Wed, 30 Aug 2023 08:43:33 +0200\nSubject: [PATCH v5] format-patch: --rfc honors what --subject-prefix sets\n\nRather than replacing the configured subject prefix (either through the\ngit config or command line) entirely with \"RFC PATCH\", this change\nprepends RFC to whatever subject prefix was already in use.\n\nThis is useful, for example, when a user is working on a repository that\nhas a subject prefix considered to disambiguate patches:\n\n\tgit config format.subjectPrefix 'PATCH my-project'\n\nPrior to this change, formatting patches with --rfc would lose the\n'my-project' information.\n\nThe data flow for the subject-prefix was that rev.subject_prefix\nwere to be kept the authoritative version of the subject prefix even\nwhile parsing command line options, and sprefix variable was used as\na temporary area to futz with it.  Now, the parsing code has been\nrefactored to build the subject prefix into the sprefix variable and\nassigns its value at the end to rev.subject_prefix, which makes the\nflow easier to grasp.\n\nSigned-off-by: Drew DeVault <sir@cmpwn.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\nRange-diff:\n1:  fcd7aa53d2 ! 1:  e0d7db7423 format-patch: --rfc honors what --subject-prefix sets\n    @@ Commit message\n         Prior to this change, formatting patches with --rfc would lose the\n         'my-project' information.\n     \n    +    The data flow for the subject-prefix was that rev.subject_prefix\n    +    were to be kept the authoritative version of the subject prefix even\n    +    while parsing command line options, and sprefix variable was used as\n    +    a temporary area to futz with it.  Now, the parsing code has been\n    +    refactored to build the subject prefix into the sprefix variable and\n    +    assigns its value at the end to rev.subject_prefix, which makes the\n    +    flow easier to grasp.\n    +\n         Signed-off-by: Drew DeVault <sir@cmpwn.com>\n         Signed-off-by: Junio C Hamano <gitster@pobox.com>\n     \n    @@ builtin/log.c: static int subject_prefix = 0;\n     +\tstruct strbuf *sprefix;\n     +\n      \tBUG_ON_OPT_NEG(unset);\n    -+\tsprefix = (struct strbuf *)opt->value;\n    ++\tsprefix = opt->value;\n      \tsubject_prefix = 1;\n     -\t((struct rev_info *)opt->value)->subject_prefix = arg;\n     +\tstrbuf_reset(sprefix);\n\n Documentation/git-format-patch.txt | 18 +++++++++++------\n builtin/log.c                      | 31 +++++++++++++++---------------\n t/t4014-format-patch.sh            | 22 ++++++++++++++++++++-\n 3 files changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 373b46fc0d..62345ed764 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -217,9 +217,15 @@ populated with placeholder text.\n \n --subject-prefix=<subject prefix>::\n \tInstead of the standard '[PATCH]' prefix in the subject\n-\tline, instead use '[<subject prefix>]'. This\n-\tallows for useful naming of a patch series, and can be\n-\tcombined with the `--numbered` option.\n+\tline, instead use '[<subject prefix>]'. This can be used\n+\tto name a patch series, and can be combined with the\n+\t`--numbered` option.\n++\n+The configuration variable `format.subjectPrefix` may also be used\n+to configure a subject prefix to apply to a given repository for\n+all patches. This is often useful on mailing lists which receive\n+patches for several repositories and can be used to disambiguate\n+the patches (with a value of e.g. \"PATCH my-project\").\n \n --filename-max-length=<n>::\n \tInstead of the standard 64 bytes, chomp the generated output\n@@ -229,9 +235,9 @@ populated with placeholder text.\n \tvariable, or 64 if unconfigured.\n \n --rfc::\n-\tAlias for `--subject-prefix=\"RFC PATCH\"`. RFC means \"Request For\n-\tComments\"; use this when sending an experimental patch for\n-\tdiscussion rather than application.\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 \n -v <n>::\n --reroll-count=<n>::\ndiff --git a/builtin/log.c b/builtin/log.c\nindex db3a88bfe9..75762b497d 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1468,19 +1468,16 @@ static int subject_prefix = 0;\n static int subject_prefix_callback(const struct option *opt, const char *arg,\n \t\t\t    int unset)\n {\n+\tstruct strbuf *sprefix;\n+\n \tBUG_ON_OPT_NEG(unset);\n+\tsprefix = opt->value;\n \tsubject_prefix = 1;\n-\t((struct rev_info *)opt->value)->subject_prefix = arg;\n+\tstrbuf_reset(sprefix);\n+\tstrbuf_addstr(sprefix, arg);\n \treturn 0;\n }\n \n-static int rfc_callback(const struct option *opt, const char *arg, int unset)\n-{\n-\tBUG_ON_OPT_NEG(unset);\n-\tBUG_ON_OPT_ARG(arg);\n-\treturn subject_prefix_callback(opt, \"RFC PATCH\", unset);\n-}\n-\n static int numbered_cmdline_opt = 0;\n \n static int numbered_callback(const struct option *opt, const char *arg,\n@@ -1907,6 +1904,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tstruct strbuf sprefix = 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@@ -1930,13 +1928,11 @@ 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\", &rev, NULL,\n-\t\t\t    N_(\"use [RFC PATCH] instead of [PATCH]\"),\n-\t\t\t    PARSE_OPT_NOARG | PARSE_OPT_NONEG, rfc_callback),\n+\t\tOPT_BOOL(0, \"rfc\", &rfc, N_(\"use [RFC PATCH] instead of [PATCH]\")),\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-\t\tOPT_CALLBACK_F(0, \"subject-prefix\", &rev, N_(\"prefix\"),\n+\t\tOPT_CALLBACK_F(0, \"subject-prefix\", &sprefix, N_(\"prefix\"),\n \t\t\t    N_(\"use [<prefix>] instead of [PATCH]\"),\n \t\t\t    PARSE_OPT_NONEG, subject_prefix_callback),\n \t\tOPT_CALLBACK_F('o', \"output-directory\", &output_directory,\n@@ -2016,11 +2012,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.max_parents = 1;\n \trev.diffopt.flags.recursive = 1;\n \trev.diffopt.no_free = 1;\n-\trev.subject_prefix = fmt_patch_subject_prefix;\n \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n \ts_r_opt.def = \"HEAD\";\n \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n \n+\tstrbuf_addstr(&sprefix, fmt_patch_subject_prefix);\n \tif (format_no_prefix)\n \t\tdiff_set_noprefix(&rev.diffopt);\n \n@@ -2048,13 +2044,16 @@ 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+\n \tif (reroll_count) {\n-\t\tstrbuf_addf(&sprefix, \"%s v%s\",\n-\t\t\t    rev.subject_prefix, reroll_count);\n+\t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n \t\trev.reroll_count = reroll_count;\n-\t\trev.subject_prefix = sprefix.buf;\n \t}\n \n+\trev.subject_prefix = sprefix.buf;\n+\n \tfor (i = 0; i < extra_hdr.nr; i++) {\n \t\tstrbuf_addstr(&buf, extra_hdr.items[i].string);\n \t\tstrbuf_addch(&buf, '\\n');\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 3cf2b7a7fb..9fa1f3bc7a 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1373,7 +1373,27 @@ test_expect_success '--rfc' '\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+\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+\tEOF\n+\tgit -c format.subjectPrefix=\"PATCH foobar\" \\\n+\t\tformat-patch -n -1 --stdout --rfc >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--rfc is argument order independent' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [RFC PATCH foobar 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --rfc \\\n+\t\t--subject-prefix=\"PATCH foobar\" >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.42.0-100-g3525f1dbc1\n\n"},{"id":"481274","messageId":"20230831223201.GB952036@coredump.intra.peff.net","threadId":"60172","inReplyTo":"xmqqv8cuswah.fsf@gitster.g","subject":"Re: [PATCH v4] format-patch: --rfc honors what --subject-prefix sets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-08-31T22:32:01Z","receivedAt":"2023-08-31T22:32:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 31, 2023 at 03:04:54PM -0700, Junio C Hamano wrote:\n\n> Sure.  FWIW, I like giving a meaningful name to a thing that is used\n> more than once, so my preference is the original minus excess cast.\n> \n> Thanks for a review.  Here is a tentative rewrite\n\nThanks, looks good to me.\n\n-Peff\n"},{"id":"481280","messageId":"CV7EK073OLB2.3Q4Y31O55ZY9P@taiga","threadId":"60172","inReplyTo":"xmqqv8cuswah.fsf@gitster.g","subject":"Re: [PATCH v4] format-patch: --rfc honors what --subject-prefix sets","fromName":"Drew DeVault","fromEmail":"sir@cmpwn.com","sentAt":"2023-09-01T07:29:00Z","receivedAt":"2023-09-01T07:29:15Z","isPatch":true,"sender":{"key":"sir@cmpwn.com","avatar":"https://avatars.githubusercontent.com/u/1310872?v=4"},"body":"+1 to proposed changes\n"},{"id":"481300","messageId":"xmqq1qfhrg9f.fsf@gitster.g","threadId":"60172","inReplyTo":"CV7EK073OLB2.3Q4Y31O55ZY9P@taiga","subject":"Re: [PATCH v4] format-patch: --rfc honors what --subject-prefix sets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-01T16:48:44Z","receivedAt":"2023-09-01T16:48:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Drew DeVault\" <sir@cmpwn.com> writes:\n\n> +1 to proposed changes\n\nThanks, let's merge that version to 'next' then.\n"}]}