{"thread":{"id":"61329","subject":"[PATCH 1/4] format-patch docs: avoid use of parentheses to improve readability","startedAt":"2024-04-17T03:32:51Z","lastAt":"2024-04-19T16:21:46Z","messageCount":51,"participants":["Dragan Simic","Eric Sunshine","Kristoffer Haugsbakk","Patrick Steinhardt","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"493045","messageId":"25b90d065744c01da3f37b6966fd97d699931f6b.1713324598.git.dsimic@manjaro.org","threadId":"61329","inReplyTo":"cover.1713324598.git.dsimic@manjaro.org","subject":"[PATCH 1/4] format-patch docs: avoid use of parentheses to improve readability","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T03:32:41Z","receivedAt":"2024-04-17T03:32:51Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"In general, using the parentheses disrupts the flow and reduces readability,\nso they should be avoided whenever possible. The improved sentence is a clear\nexample, in which the adjustment is obvious and simple.\n\nSigned-off-by: Dragan Simic \n---\n Documentation/git-format-patch.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 728bb3821c17..a5019ab46926 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -239,8 +239,8 @@ the patches (with a value of e.g. \"PATCH my-project\").\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+\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"},{"id":"493046","messageId":"c975f961779b4a7b10c0743b4b8b3ad8c89cb617.1713324598.git.dsimic@manjaro.org","threadId":"61329","inReplyTo":"cover.1713324598.git.dsimic@manjaro.org","subject":"[PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T03:32:42Z","receivedAt":"2024-04-17T03:32:51Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Fix a bug that allows --rfc and -k options to be specified together when\nexecuting \"git format-patch\". This bug was introduced back in the commit\ne0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix sets\"),\nabout eight months ago, but it has remained undetected so far, presumably\nbecause of no associated test coverage.\n\nAdd a new test to the t4014 that covers the mutual exclusivity of the --rfc\nand -k command-line options for \"git format-patch\", for future coverage.\n\nSigned-off-by: Dragan Simic \n---\n builtin/log.c | 5 ++++-\n t/t4014-format-patch.sh | 4 ++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex c0a8bb95e983..e5a238f1cf2c 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -2050,8 +2050,11 @@ 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/* Also mark the subject prefix as modified, for later checks */\n+\tif (rfc) {\n \t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n+\t\tsubject_prefix = 1;\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 e37a1411ee24..e22c4ac34e6e 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order independent' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--rfc and -k cannot be used together' '\n+\ttest_must_fail git format-patch -1 --stdout --rfc -k >patch\n+'\n+\n test_expect_success '--from=ident notices bogus ident' '\n \ttest_must_fail git format-patch -1 --stdout --from=foo >patch\n '\n"},{"id":"493047","messageId":"cover.1713324598.git.dsimic@manjaro.org","threadId":"61329","inReplyTo":null,"subject":"[PATCH 0/4] format-patch: fix an option coexistence bug and add new --resend option","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T03:32:40Z","receivedAt":"2024-04-17T03:32:51Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"This series fixes a bug that allows --rfc and -k options to be specified\ntogether when running \"git format-patch\". This bug was introduced about\neight months ago, but it has remained undetected, presumably because of\nlacking test coverage. While fixing this bug, also add a test that covers\nthis mutual exclusion, for future coverage.\n\nThis series also adds --resend as the new option for \"git format-patch\"\nthat adds \"RESEND\" as a (sub)suffix to the patch subject prefix, which\neventually produces \"[PATCH RESEND]\" as the default patch subject prefix.\nThis subject prefix is commonly used on mailing lists to denote patches\nresent after they had attracted no attention for a while.\n\nDragan Simic (4):\n format-patch docs: avoid use of parentheses to improve readability\n format-patch: fix a bug in option exclusivity and add a test to t4014\n format-patch: new --resend option for adding \"RESEND\" to patch\n subjects\n t4014: add tests to cover --resend option and its exclusivity\n\n Documentation/git-format-patch.txt | 9 +++++--\n builtin/log.c | 16 +++++++++---\n t/t4014-format-patch.sh | 41 ++++++++++++++++++++++++++++++\n 3 files changed, 61 insertions(+), 5 deletions(-)\n\n"},{"id":"493048","messageId":"42865d6c6694b9e6b745c328d717ed244dc25a1a.1713324598.git.dsimic@manjaro.org","threadId":"61329","inReplyTo":"cover.1713324598.git.dsimic@manjaro.org","subject":"[PATCH 4/4] t4014: add tests to cover --resend option and its exclusivity","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T03:32:44Z","receivedAt":"2024-04-17T03:32:52Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Add a few new tests to the t4014 that cover the --resend command-line option\nfor \"git format-patch\", which include the tests for its exclusivity with the\nalready existing -k and --rfc command-line options.\n\nSigned-off-by: Dragan Simic \n---\n t/t4014-format-patch.sh | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex e22c4ac34e6e..bcf7b633e78f 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1401,6 +1401,43 @@ test_expect_success '--rfc and -k cannot be used together' '\n \ttest_must_fail git format-patch -1 --stdout --rfc -k >patch\n '\n \n+test_expect_success '--resend' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [PATCH RESEND 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --resend >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--resend does not overwrite prefix' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [PATCH RFC RESEND 1/1] header with . in it\n+\tEOF\n+\tgit -c format.subjectPrefix=\"PATCH RFC\" \\\n+\t\tformat-patch -n -1 --stdout --resend >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--resend is argument order independent' '\n+\tcat >expect <<-\\EOF &&\n+\tSubject: [PATCH RFC RESEND 1/1] header with . in it\n+\tEOF\n+\tgit format-patch -n -1 --stdout --resend \\\n+\t\t--subject-prefix=\"PATCH RFC\" >patch &&\n+\tgrep \"^Subject:\" patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--resend and -k cannot be used together' '\n+\ttest_must_fail git format-patch -1 --stdout --resend -k >patch\n+'\n+\n+test_expect_success '--rfc and --resend cannot be used together' '\n+\ttest_must_fail git format-patch -1 --stdout --rfc --resend >patch\n+'\n+\n test_expect_success '--from=ident notices bogus ident' '\n \ttest_must_fail git format-patch -1 --stdout --from=foo >patch\n '\n"},{"id":"493049","messageId":"1d9c6ce3df714211889453c245485d46b43edff6.1713324598.git.dsimic@manjaro.org","threadId":"61329","inReplyTo":"cover.1713324598.git.dsimic@manjaro.org","subject":"[PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T03:32:43Z","receivedAt":"2024-04-17T03:32:52Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Add --resend as the new command-line option for \"git format-patch\" that adds\n\"RESEND\" as a (sub)suffix to the patch subject prefix, eventually producing\n\"[PATCH RESEND]\" as the default patch subject prefix.\n\n\"[PATCH RESEND]\" is a patch subject prefix commonly used on mailing lists\nfor patches resent to a mailing list after they had attracted no attention\nfor some time, usually for a couple of weeks. As such, this subject prefix\ndeserves adding --resend as a new shorthand option to \"git format-patch\".\n\nOf course, add the description of the new --resend command-line option to\nthe documentation for \"git format-patch\".\n\nSigned-off-by: Dragan Simic \n---\n Documentation/git-format-patch.txt | 5 +++++\n builtin/log.c | 11 +++++++++--\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex a5019ab46926..8e63b62620ed 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -243,6 +243,11 @@ the patches (with a value of e.g. \"PATCH my-project\").\n \tdefault. RFC means \"Request For Comments\"; use this when sending\n \tan experimental patch for discussion rather than application.\n \n+--resend::\n+\tAppends \"RESEND\" to the subject prefix, producing \"PATCH RESEND\"\n+\tby default. Use this when sending again a patch that had resulted\n+\tin attracting no discussion for a while.\n+\n -v ::\n --reroll-count=::\n \tMark the series as the -th iteration of the topic. The\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e5a238f1cf2c..28f31659bcde 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1908,7 +1908,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+\tint rfc = 0, resend = 0;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n@@ -1933,6 +1933,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\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_BOOL(0, \"resend\", &resend, N_(\"use [PATCH RESEND] 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@@ -2055,6 +2056,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n \t\tsubject_prefix = 1;\n \t}\n+\tif (resend) {\n+\t\tstrbuf_addstr(&sprefix, \" RESEND\");\n+\t\tsubject_prefix = 1;\n+\t}\n \n \tif (reroll_count) {\n \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n@@ -2111,7 +2116,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (numbered && keep_subject)\n \t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"-n\", \"-k\");\n \tif (keep_subject && subject_prefix)\n-\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--subject-prefix/--rfc\", \"-k\");\n+\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--subject-prefix/--rfc/--resend\", \"-k\");\n+\tif (rfc && resend)\n+\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--rfc\", \"--resend\");\n \trev.preserve_subject = keep_subject;\n \n \targc = setup_revisions(argc, argv, &rev, &s_r_opt);\n"},{"id":"493053","messageId":"CAPig+cRkrMDkQKnwaTGY4djwgC6mGqngB-4HfGQm1TNCq4Q4+w@mail.gmail.com","threadId":"61329","inReplyTo":"cover.1713324598.git.dsimic@manjaro.org","subject":"Re: [PATCH 0/4] format-patch: fix an option coexistence bug and add new --resend option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-17T06:02:07Z","receivedAt":"2024-04-17T06:02:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic wrote:\n> This series fixes a bug that allows --rfc and -k options to be specified\n> together when running \"git format-patch\". This bug was introduced about\n> eight months ago, but it has remained undetected, presumably because of\n> lacking test coverage. While fixing this bug, also add a test that covers\n> this mutual exclusion, for future coverage.\n>\n> This series also adds --resend as the new option for \"git format-patch\"\n> that adds \"RESEND\" as a (sub)suffix to the patch subject prefix, which\n> eventually produces \"[PATCH RESEND]\" as the default patch subject prefix.\n> This subject prefix is commonly used on mailing lists to denote patches\n> resent after they had attracted no attention for a while.\n\nI'd recommend splitting this into two series, one which fixes the bug,\nand one which introduces the new feature. Otherwise, the bug fix is\nlikely to be held hostage as reviewers bikeshed over the new feature\nand opine about whether such a feature is even desirable[*]. As a\nresult, the bug fix may take much longer to get applied than if\nsubmitted as a standalone series.\n\n[*] For instance, my knee-jerk reaction is that we don't want to keep\npiling on these special-case flags each time someone wants their new\nfavorite word as a lead-in to \"PATCH\". In addition to --rfc, and\n--resend, the next person might want --rfd or --tbd, etc. More\npalatable would be a general-purpose option which lets you specify the\nprefix which appears in front of \"PATCH\", but even that can be argued\nas unnecessary since we already have --subject-prefix.\n"},{"id":"493054","messageId":"36c4a1e23653c2844a52fd994501287d@manjaro.org","threadId":"61329","inReplyTo":"CAPig+cRkrMDkQKnwaTGY4djwgC6mGqngB-4HfGQm1TNCq4Q4+w@mail.gmail.com","subject":"Re: [PATCH 0/4] format-patch: fix an option coexistence bug and add new --resend option","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T06:07:29Z","receivedAt":"2024-04-17T06:07:31Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Eric,\n\nOn 2024-04-17 08:02, Eric Sunshine wrote:\n> On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic \n> wrote:\n>> This series fixes a bug that allows --rfc and -k options to be \n>> specified\n>> together when running \"git format-patch\". This bug was introduced \n>> about\n>> eight months ago, but it has remained undetected, presumably because \n>> of\n>> lacking test coverage. While fixing this bug, also add a test that \n>> covers\n>> this mutual exclusion, for future coverage.\n>> \n>> This series also adds --resend as the new option for \"git \n>> format-patch\"\n>> that adds \"RESEND\" as a (sub)suffix to the patch subject prefix, which\n>> eventually produces \"[PATCH RESEND]\" as the default patch subject \n>> prefix.\n>> This subject prefix is commonly used on mailing lists to denote \n>> patches\n>> resent after they had attracted no attention for a while.\n> \n> I'd recommend splitting this into two series, one which fixes the bug,\n> and one which introduces the new feature. Otherwise, the bug fix is\n> likely to be held hostage as reviewers bikeshed over the new feature\n> and opine about whether such a feature is even desirable[*]. As a\n> result, the bug fix may take much longer to get applied than if\n> submitted as a standalone series.\n\nThanks for the suggestion! I'll wait a couple of days for more\nfeedback, and I'll then split the series.\n\n> [*] For instance, my knee-jerk reaction is that we don't want to keep\n> piling on these special-case flags each time someone wants their new\n> favorite word as a lead-in to \"PATCH\". In addition to --rfc, and\n> --resend, the next person might want --rfd or --tbd, etc. More\n> palatable would be a general-purpose option which lets you specify the\n> prefix which appears in front of \"PATCH\", but even that can be argued\n> as unnecessary since we already have --subject-prefix.\n\nMakes sense, but in that case accepting the --rfc option, back at the\ntime, was actually some kind of a mistake, if you agree.\n"},{"id":"493055","messageId":"556d4baa-14f9-485a-8db3-0c9a966351a7@app.fastmail.com","threadId":"61329","inReplyTo":"1d9c6ce3df714211889453c245485d46b43edff6.1713324598.git.dsimic@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-04-17T06:14:12Z","receivedAt":"2024-04-17T06:14:44Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Wed, Apr 17, 2024, at 05:32, Dragan Simic wrote:\n> Add --resend as the new command-line option for \"git format-patch\" that adds\n> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually producing\n> \"[PATCH RESEND]\" as the default patch subject prefix.\n\nI think this paragraph is a bit *long*. How about\n\n “ --resend adds \"RESEND\" to the subject prefix (producing \"PATCH\n RESEND\" by default).\n\n(I took this from description of `--rfc`.)\n\nProbably modified to fit in with the other paragraphs.\n\n> Of course, add the description of the new --resend command-line option to\n> the documentation for \"git format-patch\".\n\nThis paragraph can be dropped. ;) Adding documentation along with a new\nfeature doesn’t need to be called out.\n"},{"id":"493056","messageId":"CAPig+cTEp799w2-VEACYThW0COyo0SJLRS_sr-PG=LX++Tompw@mail.gmail.com","threadId":"61329","inReplyTo":"c975f961779b4a7b10c0743b4b8b3ad8c89cb617.1713324598.git.dsimic@manjaro.org","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-17T06:15:45Z","receivedAt":"2024-04-17T06:15:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic wrote:\n> format-patch: fix a bug in option exclusivity and add a test to t4014\n\nReviewers assume that a conscientious patch author will add tests when\nappropriate, so stating that you did so is unnecessary. Thus it's safe\nto omit \"and add a test to t4014\" without negatively impacting\ncomprehension of the subject.\n\n format-patch: ensure --rfc and -k are mutually exclusive\n\n> Fix a bug that allows --rfc and -k options to be specified together when\n> executing \"git format-patch\". This bug was introduced back in the commit\n> e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix sets\"),\n> about eight months ago, but it has remained undetected so far, presumably\n> because of no associated test coverage.\n\nEverything starting at \"...about eight months\" through the end of the\nparagraph could be easily dropped. Reviewers understand implicitly\nthat the bug went undiscovered due to lack of test coverage.\n\n> Add a new test to the t4014 that covers the mutual exclusivity of the --rfc\n> and -k command-line options for \"git format-patch\", for future coverage.\n\nSimilarly, no need for this paragraph. As a conscientious patch\nauthor, reviewers assume that you added the test, so this paragraph\nadds no information. Also, the body of the patch provides this\ninformation clearly without it having to be stated here.\n\n> Signed-off-by: Dragan Simic \n> ---\n> diff --git a/builtin/log.c b/builtin/log.c\n> @@ -2050,8 +2050,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> - if (rfc)\n> + /* Also mark the subject prefix as modified, for later checks */\n> + if (rfc) {\n> strbuf_insertstr(&sprefix, 0, \"RFC \");\n> + subject_prefix = 1;\n> + }\n\nI'm not sure that this new comment (/* Also mark... */) adds any value\nbeyond what the code itself already says. It may actually be confusing\nwith its current placement. Had you placed it immediately above the\n`stubject_prefix = 1` line, it would have been more understandable,\nbut still probably unnecessary since anyone studying this code is\ngoing to have to understand the purpose of `subject_prefix` anyhow.\n\nAt any rate, I doubt that any of these review comments on their own is\nworth a reroll.\n"},{"id":"493060","messageId":"CAPig+cRzCSO_4r02Vc-OpH3Cyz0iTWv+q4oTRg30H5Qvzz=X7g@mail.gmail.com","threadId":"61329","inReplyTo":"36c4a1e23653c2844a52fd994501287d@manjaro.org","subject":"Re: [PATCH 0/4] format-patch: fix an option coexistence bug and add new --resend option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-17T06:23:08Z","receivedAt":"2024-04-17T06:23:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 17, 2024 at 2:07 AM Dragan Simic wrote:\n> On 2024-04-17 08:02, Eric Sunshine wrote:\n> > [*] For instance, my knee-jerk reaction is that we don't want to keep\n> > piling on these special-case flags each time someone wants their new\n> > favorite word as a lead-in to \"PATCH\". In addition to --rfc, and\n> > --resend, the next person might want --rfd or --tbd, etc. More\n> > palatable would be a general-purpose option which lets you specify the\n> > prefix which appears in front of \"PATCH\", but even that can be argued\n> > as unnecessary since we already have --subject-prefix.\n>\n> Makes sense, but in that case accepting the --rfc option, back at the\n> time, was actually some kind of a mistake, if you agree.\n\nPossibly. It does happen that, in retrospect, some changes come to be\nviewed as mistakes. On the other hand, if --rfc existed before\n--subject-prefix was introduced, then --rfc would just be historic\naccretion rather than a mistake. (I didn't check which option came\nfirst.)\n\nAt any rate, we probably want to be careful about piling on more\nspecial-cases without considering general-purpose solutions.\n"},{"id":"493061","messageId":"Zh9r1K2_T5wvVJVC@tanuki","threadId":"61329","inReplyTo":"c975f961779b4a7b10c0743b4b8b3ad8c89cb617.1713324598.git.dsimic@manjaro.org","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-04-17T06:27:32Z","receivedAt":"2024-04-17T06:27:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Apr 17, 2024 at 05:32:42AM +0200, Dragan Simic wrote:\n> Fix a bug that allows --rfc and -k options to be specified together when\n> executing \"git format-patch\". This bug was introduced back in the commit\n> e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix sets\"),\n> about eight months ago, but it has remained undetected so far, presumably\n> because of no associated test coverage.\n> \n> Add a new test to the t4014 that covers the mutual exclusivity of the --rfc\n> and -k command-line options for \"git format-patch\", for future coverage.\n> \n> Signed-off-by: Dragan Simic \n> ---\n> builtin/log.c | 5 ++++-\n> t/t4014-format-patch.sh | 4 ++++\n> 2 files changed, 8 insertions(+), 1 deletion(-)\n> \n> diff --git a/builtin/log.c b/builtin/log.c\n> index c0a8bb95e983..e5a238f1cf2c 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -2050,8 +2050,11 @@ 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/* Also mark the subject prefix as modified, for later checks */\n> +\tif (rfc) {\n> \t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n> +\t\tsubject_prefix = 1;\n> +\t}\n\nAs an alternative fix, can we drop `subject_prefix` and replace it with\n`sprefix.len` instead? It seems to merely be a proxy value for that\nanyway, and if we didn't have that variable then the bug would not have\nbeen possible to begin with.\n\nPatrick\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 e37a1411ee24..e22c4ac34e6e 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order independent' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success '--rfc and -k cannot be used together' '\n> +\ttest_must_fail git format-patch -1 --stdout --rfc -k >patch\n> +'\n> +\n> test_expect_success '--from=ident notices bogus ident' '\n> \ttest_must_fail git format-patch -1 --stdout --from=foo >patch\n> '\n> \n"},{"id":"493062","messageId":"8dd3bf56d595801e0f262329a0000ea4@manjaro.org","threadId":"61329","inReplyTo":"CAPig+cTEp799w2-VEACYThW0COyo0SJLRS_sr-PG=LX++Tompw@mail.gmail.com","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T06:29:53Z","receivedAt":"2024-04-17T06:29:55Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 08:15, Eric Sunshine wrote:\n> On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic \n> wrote:\n>> format-patch: fix a bug in option exclusivity and add a test to t4014\n> \n> Reviewers assume that a conscientious patch author will add tests when\n> appropriate, so stating that you did so is unnecessary. Thus it's safe\n> to omit \"and add a test to t4014\" without negatively impacting\n> comprehension of the subject.\n> \n> format-patch: ensure --rfc and -k are mutually exclusive\n\nMakes sense, but the previous authors obviously weren't diligent\nenough to include such a test, which presumably made the fixed bug\nremain undetected for so long, so I wanted to put some emphasis on\nthe addition of a test.\n\n>> Fix a bug that allows --rfc and -k options to be specified together \n>> when\n>> executing \"git format-patch\". This bug was introduced back in the \n>> commit\n>> e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix \n>> sets\"),\n>> about eight months ago, but it has remained undetected so far, \n>> presumably\n>> because of no associated test coverage.\n> \n> Everything starting at \"...about eight months\" through the end of the\n> paragraph could be easily dropped. Reviewers understand implicitly\n> that the bug went undiscovered due to lack of test coverage.\n\nI have no problems with dropping that part, but IMHO that's nitpicking.\nAlso, dropping it would delete some of the context that people might\nfind useful later.\n\n>> Add a new test to the t4014 that covers the mutual exclusivity of the \n>> --rfc\n>> and -k command-line options for \"git format-patch\", for future \n>> coverage.\n> \n> Similarly, no need for this paragraph. As a conscientious patch\n> author, reviewers assume that you added the test, so this paragraph\n> adds no information. Also, the body of the patch provides this\n> information clearly without it having to be stated here.\n\nWith all the respect, I think that having that paragraph is actually\ngood, because explaining it clearly provides good context for the\nrepository history and people reading it later.\n\n>> Signed-off-by: Dragan Simic \n>> ---\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> @@ -2050,8 +2050,11 @@ int cmd_format_patch(int argc, const char \n>> **argv, const char *prefix)\n>> - if (rfc)\n>> + /* Also mark the subject prefix as modified, for later checks \n>> */\n>> + if (rfc) {\n>> strbuf_insertstr(&sprefix, 0, \"RFC \");\n>> + subject_prefix = 1;\n>> + }\n> \n> I'm not sure that this new comment (/* Also mark... */) adds any value\n> beyond what the code itself already says. It may actually be confusing\n> with its current placement. Had you placed it immediately above the\n> `stubject_prefix = 1` line, it would have been more understandable,\n> but still probably unnecessary since anyone studying this code is\n> going to have to understand the purpose of `subject_prefix` anyhow.\n\nSetting such flags should actually be performed in a callback,\nbut in this case creating a callback isn't warranted, IMHO. Thus,\nthat comment tries to explain why a flag is set out of place.\nI have no objections against removing this comment, if you find\nit doing more harm than good.\n\nI didn't place it immediately above the relevant line because it\nalso applies to the adjacent block for the --resend option, and I\nwanted to reduce the code churn that would result from placing it\nimmediately before the relevant line, and moving it a couple of\nlines above just a couple of patches later.\n\n> At any rate, I doubt that any of these review comments on their own is\n> worth a reroll.\n\nWell, I need to split the series anyway, so the v2 is pretty much\ninevitable.\n"},{"id":"493063","messageId":"e4aa5235-c6ad-45c7-930e-de991cc375f2@app.fastmail.com","threadId":"61329","inReplyTo":"c975f961779b4a7b10c0743b4b8b3ad8c89cb617.1713324598.git.dsimic@manjaro.org","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-04-17T06:33:42Z","receivedAt":"2024-04-17T06:34:18Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"It could be useful to Cc the author of that commit since it’s so\nrecent. Like an FYI.\n\nOn Wed, Apr 17, 2024, at 05:32, Dragan Simic wrote:\n> Fix a bug that allows --rfc and -k options to be specified together when\n> executing \"git format-patch\". This bug was introduced back in the commit\n> e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix sets\"),\n> about eight months ago, but it has remained undetected so far, presumably\n> because of no associated test coverage.\n\nI don’t think speculating on why the bug is still there improves the\ncommit message.\n\nThis paragraph could perhaps be rewritten to\n\n “ Fix a bug from e0d7db7423a (format-patch: --rfc honors what\n --subject-prefix sets, 2023-08-30) that allows --rfc and -k options\n to be specified together when executing \"git format-patch\".\n\nThe extra sentence in the original doesn’t really explain anything more\nabout the commit. Except the “eight months ago”, but here I’ve used the\n“reference” style (not the Linux-style) which contains the date.\n\n> Add a new test to the t4014 that covers the mutual exclusivity of the --rfc\n> and -k command-line options for \"git format-patch\", for future coverage.\n\nI.e. add a regression test. Pretty standard.\n\n>\n> Signed-off-by: Dragan Simic \n> ---\n> builtin/log.c | 5 ++++-\n> t/t4014-format-patch.sh | 4 ++++\n> 2 files changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index c0a8bb95e983..e5a238f1cf2c 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -2050,8 +2050,11 @@ 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/* Also mark the subject prefix as modified, for later checks */\n\nI think the code speaks for itself in this case.\n\n> +\tif (rfc) {\n> \t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n> +\t\tsubject_prefix = 1;\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 e37a1411ee24..e22c4ac34e6e 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order\n> independent' '\n> \ttest_cmp expect actual\n> '\n>\n> +test_expect_success '--rfc and -k cannot be used together' '\n> +\ttest_must_fail git format-patch -1 --stdout --rfc -k >patch\n\nI don’t understand why you redirect to `patch` if you only check the\nexit code. (I don’t expect any stdout since it will fail.)\n\nAlthough it would be nice with a text comparison or grep on the stderr\noutput to make sure that the command died for the expected reason. But I\nhaven’t read the associated code.\n\n> +'\n> +\n> test_expect_success '--from=ident notices bogus ident' '\n> \ttest_must_fail git format-patch -1 --stdout --from=foo >patch\n> '\n"},{"id":"493064","messageId":"CAPig+cRzOHROK0VpkLR9fk7Gr0NRH9VKcH4dGXOuoaO5Ky2c2A@mail.gmail.com","threadId":"61329","inReplyTo":"1d9c6ce3df714211889453c245485d46b43edff6.1713324598.git.dsimic@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-17T06:35:16Z","receivedAt":"2024-04-17T06:35:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic wrote:\n> Add --resend as the new command-line option for \"git format-patch\" that adds\n> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually producing\n> \"[PATCH RESEND]\" as the default patch subject prefix.\n>\n> \"[PATCH RESEND]\" is a patch subject prefix commonly used on mailing lists\n> for patches resent to a mailing list after they had attracted no attention\n> for some time, usually for a couple of weeks. As such, this subject prefix\n> deserves adding --resend as a new shorthand option to \"git format-patch\".\n>\n> Of course, add the description of the new --resend command-line option to\n> the documentation for \"git format-patch\".\n>\n> Signed-off-by: Dragan Simic \n> ---\n> diff --git a/builtin/log.c b/builtin/log.c\n> @@ -2111,7 +2116,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> if (keep_subject && subject_prefix)\n> - die(_(\"options '%s' and '%s' cannot be used together\"), \"--subject-prefix/--rfc\", \"-k\");\n> + die(_(\"options '%s' and '%s' cannot be used together\"), \"--subject-prefix/--rfc/--resend\", \"-k\");\n\nYou probably want to be using die_for_incompatible_opt4() from\nparse-options.h here.\n\n(And you may want a preparatory patch which fixes the preimage to use\ndie_for_incompatible_opt3() for --subject-prefix, --rfc, and -k\nexclusivity, though that may be overkill.)\n"},{"id":"493065","messageId":"4aa0754ee62d78ca9300eb709df561b3@manjaro.org","threadId":"61329","inReplyTo":"556d4baa-14f9-485a-8db3-0c9a966351a7@app.fastmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T06:36:20Z","receivedAt":"2024-04-17T06:36:22Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Kristoffer,\n\nOn 2024-04-17 08:14, Kristoffer Haugsbakk wrote:\n> On Wed, Apr 17, 2024, at 05:32, Dragan Simic wrote:\n>> Add --resend as the new command-line option for \"git format-patch\" \n>> that adds\n>> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually \n>> producing\n>> \"[PATCH RESEND]\" as the default patch subject prefix.\n> \n> I think this paragraph is a bit *long*. How about\n> \n> “ --resend adds \"RESEND\" to the subject prefix (producing \"PATCH\n> RESEND\" by default).\n> \n> (I took this from description of `--rfc`.)\n> \n> Probably modified to fit in with the other paragraphs.\n\nThanks for your feedback!\n\nI also wasn't super happy with that paragraph, because I tried to be\nas technically accurate as possible, but that unfortunately made the\nwording a bit awkward. Though, I'm not really sure that your proposed\ndescription is actually better, because the parenthesis should in\ngeneral be avoided, because they disrupt the flow. It also doesn't\nuse imperative mood.\n\nOf course, I'll see to improve that paragraph.\n\n>> Of course, add the description of the new --resend command-line option \n>> to\n>> the documentation for \"git format-patch\".\n> \n> This paragraph can be dropped. ;) Adding documentation along with a new\n> feature doesn’t need to be called out.\n\nMakes sense.\n"},{"id":"493066","messageId":"CAPig+cSGZr4zE=Dp7Z58CN0kmkpXdc8SOopXmB9=ry4gwNkq=w@mail.gmail.com","threadId":"61329","inReplyTo":"e4aa5235-c6ad-45c7-930e-de991cc375f2@app.fastmail.com","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-17T06:40:42Z","receivedAt":"2024-04-17T06:40:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 17, 2024 at 2:34 AM Kristoffer Haugsbakk\n wrote:\n> On Wed, Apr 17, 2024, at 05:32, Dragan Simic wrote:\n> > Fix a bug that allows --rfc and -k options to be specified together when\n> > executing \"git format-patch\". This bug was introduced back in the commit\n> > e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix sets\"),\n> > about eight months ago, but it has remained undetected so far, presumably\n> > because of no associated test coverage.\n>\n> I don’t think speculating on why the bug is still there improves the\n> commit message.\n>\n> This paragraph could perhaps be rewritten to\n>\n> “ Fix a bug from e0d7db7423a (format-patch: --rfc honors what\n> --subject-prefix sets, 2023-08-30) that allows --rfc and -k options\n> to be specified together when executing \"git format-patch\".\n>\n> The extra sentence in the original doesn’t really explain anything more\n> about the commit. Except the “eight months ago”, but here I’ve used the\n> “reference” style (not the Linux-style) which contains the date.\n> > @@ -2050,8 +2050,11 @@ int cmd_format_patch(int argc, const char\n> > - if (rfc)\n> > + /* Also mark the subject prefix as modified, for later checks */\n>\n> I think the code speaks for itself in this case.\n\nApparently we're thinking along the same lines since we both said\nessentially the same things in our reviews.\n\n> > +test_expect_success '--rfc and -k cannot be used together' '\n> > + test_must_fail git format-patch -1 --stdout --rfc -k >patch\n>\n> I don’t understand why you redirect to `patch` if you only check the\n> exit code. (I don’t expect any stdout since it will fail.)\n\nI had the same question but left it unwritten since I noticed that\nthis new test is modelled after the test immediately following it in\nthe script, and the existing test also redirects to \"patch\"\nunnecessarily. So, if it's done this way for consistency with existing\ntests, I don't mind letting it slide.\n"},{"id":"493067","messageId":"b7568429acad91ff2d9a1574111441a3@manjaro.org","threadId":"61329","inReplyTo":"CAPig+cRzCSO_4r02Vc-OpH3Cyz0iTWv+q4oTRg30H5Qvzz=X7g@mail.gmail.com","subject":"Re: [PATCH 0/4] format-patch: fix an option coexistence bug and add new --resend option","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T06:43:03Z","receivedAt":"2024-04-17T06:43:05Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 08:23, Eric Sunshine wrote:\n> On Wed, Apr 17, 2024 at 2:07 AM Dragan Simic \n> wrote:\n>> On 2024-04-17 08:02, Eric Sunshine wrote:\n>> > [*] For instance, my knee-jerk reaction is that we don't want to keep\n>> > piling on these special-case flags each time someone wants their new\n>> > favorite word as a lead-in to \"PATCH\". In addition to --rfc, and\n>> > --resend, the next person might want --rfd or --tbd, etc. More\n>> > palatable would be a general-purpose option which lets you specify the\n>> > prefix which appears in front of \"PATCH\", but even that can be argued\n>> > as unnecessary since we already have --subject-prefix.\n>> \n>> Makes sense, but in that case accepting the --rfc option, back at the\n>> time, was actually some kind of a mistake, if you agree.\n> \n> Possibly. It does happen that, in retrospect, some changes come to be\n> viewed as mistakes. On the other hand, if --rfc existed before\n> --subject-prefix was introduced, then --rfc would just be historic\n> accretion rather than a mistake. (I didn't check which option came\n> first.)\n> \n> At any rate, we probably want to be careful about piling on more\n> special-cases without considering general-purpose solutions.\n\nYes, but the usability should also be taken into consideration.\nIOW, perhaps typing just --rfc or --resend is rather quick and\nusable, instead of having to use a general-purpose solution and\ntype much more, or instead of having to create an alias.\n"},{"id":"493068","messageId":"f054eb17-2eea-40f5-b201-92432aa0ad9c@app.fastmail.com","threadId":"61329","inReplyTo":"4aa0754ee62d78ca9300eb709df561b3@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-04-17T06:43:00Z","receivedAt":"2024-04-17T06:43:22Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Wed, Apr 17, 2024, at 08:36, Dragan Simic wrote:\n> It also doesn't use imperative mood.\n\nThe sentence describes what the option does (usage). It doesn’t explain\nwhat the commit message does. In context:\n\n Teach format-patch about --resend\n\n --resend adds \"RESEND\" to the subject prefix (producing \"PATCH\n RESEND\" by default).\n\n-- \nKristoffer Haugsbakk\n\n\n"},{"id":"493069","messageId":"CAPig+cRBjosyadQHO03fcCz7YBc=T04ytHkpt9UU87tLaiSOgw@mail.gmail.com","threadId":"61329","inReplyTo":"42865d6c6694b9e6b745c328d717ed244dc25a1a.1713324598.git.dsimic@manjaro.org","subject":"Re: [PATCH 4/4] t4014: add tests to cover --resend option and its exclusivity","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-17T06:48:12Z","receivedAt":"2024-04-17T06:48:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic wrote:\n> Add a few new tests to the t4014 that cover the --resend command-line option\n> for \"git format-patch\", which include the tests for its exclusivity with the\n> already existing -k and --rfc command-line options.\n\nI'd recommend squashing this patch into [3/4] which introduces the\n--resend option since it's easier to review the tests when the code\nwhich is being tested is still fresh in one's mind. (For the same\nreason, reviewers like to see documentation added in the same patch\nwhich changes the code since it's easier to verify that the\ndocumentation matches the implementation while it's fresh in the\nmind.)\n\n> Signed-off-by: Dragan Simic \n> ---\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> @@ -1401,6 +1401,43 @@ test_expect_success '--rfc and -k cannot be used together' '\n> +test_expect_success '--resend' '\n> + cat >expect <<-\\EOF &&\n> + Subject: [PATCH RESEND 1/1] header with . in it\n> + EOF\n\nIn all of the new tests, since it's just a single line body, it could\njust as easily be created with `echo`:\n\n echo \"Subject: [PATCH RESEND 1/1] header with . in it\" >expect &&\n\nOn the other hand, if you're following precedent in this script, then\nusing a here-doc may be just fine.\n\nAt any rate it's somewhat subjective and not worth a reroll.\n\n> + git format-patch -n -1 --stdout --resend >patch &&\n> + grep \"^Subject:\" patch >actual &&\n> + test_cmp expect actual\n> +'\n"},{"id":"493070","messageId":"78a7b4ab2fe94a45d71c2114a503629c@manjaro.org","threadId":"61329","inReplyTo":"e4aa5235-c6ad-45c7-930e-de991cc375f2@app.fastmail.com","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T06:54:57Z","receivedAt":"2024-04-17T06:54:59Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 08:33, Kristoffer Haugsbakk wrote:\n> It could be useful to Cc the author of that commit since it’s so\n> recent. Like an FYI.\n\nGood point. Will do that in the v2.\n\n> On Wed, Apr 17, 2024, at 05:32, Dragan Simic wrote:\n>> Fix a bug that allows --rfc and -k options to be specified together \n>> when\n>> executing \"git format-patch\". This bug was introduced back in the \n>> commit\n>> e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix \n>> sets\"),\n>> about eight months ago, but it has remained undetected so far, \n>> presumably\n>> because of no associated test coverage.\n> \n> I don’t think speculating on why the bug is still there improves the\n> commit message.\n\nPerhaps you're right, but perhaps I'm also right with that speculation. \n:)\n\n> This paragraph could perhaps be rewritten to\n> \n> “ Fix a bug from e0d7db7423a (format-patch: --rfc honors what\n> --subject-prefix sets, 2023-08-30) that allows --rfc and -k options\n> to be specified together when executing \"git format-patch\".\n> \n> The extra sentence in the original doesn’t really explain anything more\n> about the commit. Except the “eight months ago”, but here I’ve used the\n> “reference” style (not the Linux-style) which contains the date.\n\nI'm fine with that. Though, I just tried to explain it all in prose,\nwhich may actually be helpful to the people going through the repository\nhistory later.\n\n>> Add a new test to the t4014 that covers the mutual exclusivity of the \n>> --rfc\n>> and -k command-line options for \"git format-patch\", for future \n>> coverage.\n> \n> I.e. add a regression test. Pretty standard.\n\nYes, pretty standard, but again, it obviously wasn't that standard\nto the other authors, who missed to include such a test.\n\n>> Signed-off-by: Dragan Simic \n>> ---\n>> builtin/log.c | 5 ++++-\n>> t/t4014-format-patch.sh | 4 ++++\n>> 2 files changed, 8 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index c0a8bb95e983..e5a238f1cf2c 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -2050,8 +2050,11 @@ 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/* Also mark the subject prefix as modified, for later checks */\n> \n> I think the code speaks for itself in this case.\n\nAlright, two votes so far, so this comments gets deleted in the v2. :)\nI'm perfectly fine with that.\n\n>> +\tif (rfc) {\n>> \t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n>> +\t\tsubject_prefix = 1;\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 e37a1411ee24..e22c4ac34e6e 100755\n>> --- a/t/t4014-format-patch.sh\n>> +++ b/t/t4014-format-patch.sh\n>> @@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order\n>> independent' '\n>> \ttest_cmp expect actual\n>> '\n>> \n>> +test_expect_success '--rfc and -k cannot be used together' '\n>> +\ttest_must_fail git format-patch -1 --stdout --rfc -k >patch\n> \n> I don’t understand why you redirect to `patch` if you only check the\n> exit code. (I don’t expect any stdout since it will fail.)\n\nYou're right, but who knows what might actually happen in the\nfuture, i.e. while someone in the future makes some changes to\nthe code and runs this test? It's better to stay on the safe\nside and prevent some output from appearing somewhere.\n\n> Although it would be nice with a text comparison or grep on the stderr\n> output to make sure that the command died for the expected reason. But \n> I\n> haven’t read the associated code.\n\nYes, it would be nice, and the same thoughts actually already\ncrossed my mind while working on this patch, but there are already\nmore similar tests that don't validate such stderr outputs. Thus,\nperhaps it would be better to improve such tests, including this one,\nin a separate follow-up series.\n\n>> +'\n>> +\n>> test_expect_success '--from=ident notices bogus ident' '\n>> \ttest_must_fail git format-patch -1 --stdout --from=foo >patch\n>> '\n"},{"id":"493071","messageId":"47ac35d45693aa5b9fa061e85da6e176@manjaro.org","threadId":"61329","inReplyTo":"Zh9r1K2_T5wvVJVC@tanuki","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T06:56:22Z","receivedAt":"2024-04-17T06:56:24Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Patrick,\n\nOn 2024-04-17 08:27, Patrick Steinhardt wrote:\n> On Wed, Apr 17, 2024 at 05:32:42AM +0200, Dragan Simic wrote:\n>> Fix a bug that allows --rfc and -k options to be specified together \n>> when\n>> executing \"git format-patch\". This bug was introduced back in the \n>> commit\n>> e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix \n>> sets\"),\n>> about eight months ago, but it has remained undetected so far, \n>> presumably\n>> because of no associated test coverage.\n>> \n>> Add a new test to the t4014 that covers the mutual exclusivity of the \n>> --rfc\n>> and -k command-line options for \"git format-patch\", for future \n>> coverage.\n>> \n>> Signed-off-by: Dragan Simic \n>> ---\n>> builtin/log.c | 5 ++++-\n>> t/t4014-format-patch.sh | 4 ++++\n>> 2 files changed, 8 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index c0a8bb95e983..e5a238f1cf2c 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -2050,8 +2050,11 @@ 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/* Also mark the subject prefix as modified, for later checks */\n>> +\tif (rfc) {\n>> \t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n>> +\t\tsubject_prefix = 1;\n>> +\t}\n> \n> As an alternative fix, can we drop `subject_prefix` and replace it with\n> `sprefix.len` instead? It seems to merely be a proxy value for that\n> anyway, and if we didn't have that variable then the bug would not have\n> been possible to begin with.\n\nThanks for your feedback!\n\nI'll think about it, and I'll come back a bit later with an update.\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 e37a1411ee24..e22c4ac34e6e 100755\n>> --- a/t/t4014-format-patch.sh\n>> +++ b/t/t4014-format-patch.sh\n>> @@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order \n>> independent' '\n>> \ttest_cmp expect actual\n>> '\n>> \n>> +test_expect_success '--rfc and -k cannot be used together' '\n>> +\ttest_must_fail git format-patch -1 --stdout --rfc -k >patch\n>> +'\n>> +\n>> test_expect_success '--from=ident notices bogus ident' '\n>> \ttest_must_fail git format-patch -1 --stdout --from=foo >patch\n>> '\n>> \n"},{"id":"493072","messageId":"a0b93341380c2157f6b87e19129abb49@manjaro.org","threadId":"61329","inReplyTo":"CAPig+cRzOHROK0VpkLR9fk7Gr0NRH9VKcH4dGXOuoaO5Ky2c2A@mail.gmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T07:05:14Z","receivedAt":"2024-04-17T07:05:16Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 08:35, Eric Sunshine wrote:\n> On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic \n> wrote:\n>> Add --resend as the new command-line option for \"git format-patch\" \n>> that adds\n>> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually \n>> producing\n>> \"[PATCH RESEND]\" as the default patch subject prefix.\n>> \n>> \"[PATCH RESEND]\" is a patch subject prefix commonly used on mailing \n>> lists\n>> for patches resent to a mailing list after they had attracted no \n>> attention\n>> for some time, usually for a couple of weeks. As such, this subject \n>> prefix\n>> deserves adding --resend as a new shorthand option to \"git \n>> format-patch\".\n>> \n>> Of course, add the description of the new --resend command-line option \n>> to\n>> the documentation for \"git format-patch\".\n>> \n>> Signed-off-by: Dragan Simic \n>> ---\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> @@ -2111,7 +2116,9 @@ int cmd_format_patch(int argc, const char \n>> **argv, const char *prefix)\n>> if (keep_subject && subject_prefix)\n>> - die(_(\"options '%s' and '%s' cannot be used \n>> together\"), \"--subject-prefix/--rfc\", \"-k\");\n>> + die(_(\"options '%s' and '%s' cannot be used \n>> together\"), \"--subject-prefix/--rfc/--resend\", \"-k\");\n> \n> You probably want to be using die_for_incompatible_opt4() from\n> parse-options.h here.\n\nThanks for the suggestion. Frankly, I haven't researched the\navailable options, assuming that the current code uses the right\noption. Of course, I'll have a detailed look into it.\n\n> (And you may want a preparatory patch which fixes the preimage to use\n> die_for_incompatible_opt3() for --subject-prefix, --rfc, and -k\n> exclusivity, though that may be overkill.)\n\nI'm not really sure what to do. Maybe the other reviewers would\nprefer an orthogonal approach instead? Maybe that would be better\nfor bisecting later, if need arises for that?\n"},{"id":"493073","messageId":"9a6a9cb1d9dd07bbbbc47616c510779a@manjaro.org","threadId":"61329","inReplyTo":"CAPig+cSGZr4zE=Dp7Z58CN0kmkpXdc8SOopXmB9=ry4gwNkq=w@mail.gmail.com","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T07:11:00Z","receivedAt":"2024-04-17T07:11:02Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 08:40, Eric Sunshine wrote:\n> On Wed, Apr 17, 2024 at 2:34 AM Kristoffer Haugsbakk\n> wrote:\n>> On Wed, Apr 17, 2024, at 05:32, Dragan Simic wrote:\n>> > Fix a bug that allows --rfc and -k options to be specified together when\n>> > executing \"git format-patch\". This bug was introduced back in the commit\n>> > e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix sets\"),\n>> > about eight months ago, but it has remained undetected so far, presumably\n>> > because of no associated test coverage.\n>> \n>> I don’t think speculating on why the bug is still there improves the\n>> commit message.\n>> \n>> This paragraph could perhaps be rewritten to\n>> \n>> “ Fix a bug from e0d7db7423a (format-patch: --rfc honors what\n>> --subject-prefix sets, 2023-08-30) that allows --rfc and -k \n>> options\n>> to be specified together when executing \"git format-patch\".\n>> \n>> The extra sentence in the original doesn’t really explain anything \n>> more\n>> about the commit. Except the “eight months ago”, but here I’ve used \n>> the\n>> “reference” style (not the Linux-style) which contains the date.\n>> > @@ -2050,8 +2050,11 @@ int cmd_format_patch(int argc, const char\n>> > - if (rfc)\n>> > + /* Also mark the subject prefix as modified, for later checks */\n>> \n>> I think the code speaks for itself in this case.\n> \n> Apparently we're thinking along the same lines since we both said\n> essentially the same things in our reviews.\n\nTwo votes, so the comments goes away. :)\n\n>> > +test_expect_success '--rfc and -k cannot be used together' '\n>> > + test_must_fail git format-patch -1 --stdout --rfc -k >patch\n>> \n>> I don’t understand why you redirect to `patch` if you only check the\n>> exit code. (I don’t expect any stdout since it will fail.)\n> \n> I had the same question but left it unwritten since I noticed that\n> this new test is modelled after the test immediately following it in\n> the script, and the existing test also redirects to \"patch\"\n> unnecessarily. So, if it's done this way for consistency with existing\n> tests, I don't mind letting it slide.\n\nYes, I also wasn't super happy with this new test, as I already noted\nin one of my replies, but improving this and the other similar tests\nis most probably something best left for a follow-up series.\n"},{"id":"493074","messageId":"60927cebd6128ee490c826d010e52da2@manjaro.org","threadId":"61329","inReplyTo":"f054eb17-2eea-40f5-b201-92432aa0ad9c@app.fastmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T07:16:46Z","receivedAt":"2024-04-17T07:16:48Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 08:43, Kristoffer Haugsbakk wrote:\n> On Wed, Apr 17, 2024, at 08:36, Dragan Simic wrote:\n>> It also doesn't use imperative mood.\n> \n> The sentence describes what the option does (usage). It doesn’t explain\n> what the commit message does. In context:\n> \n> Teach format-patch about --resend\n> \n> --resend adds \"RESEND\" to the subject prefix (producing \"PATCH\n> RESEND\" by default).\n\nFrankly, I don't like the \"teach abc xyz\" wording very much. It isn't\nsome intelligent being to be taught something new. :) That's just my\npersonal preference, of course.\n\nFurthermore, starting a sentence with \"--resend\" isn't very good. \nPlease\nnote that you're still using parenthesis, for which I already explained\nwhy they should be avoided.\n"},{"id":"493075","messageId":"CAPig+cRPUQW5ux7oKwDO5Nu46fRHrs6LrUoxnFvX9D9oNjqteg@mail.gmail.com","threadId":"61329","inReplyTo":"a0b93341380c2157f6b87e19129abb49@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-17T07:17:29Z","receivedAt":"2024-04-17T07:17:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 17, 2024 at 3:05 AM Dragan Simic wrote:\n> On 2024-04-17 08:35, Eric Sunshine wrote:\n> > On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic \n> > wrote:\n> >> - die(_(\"options '%s' and '%s' cannot be used\n> >> together\"), \"--subject-prefix/--rfc\", \"-k\");\n> >> + die(_(\"options '%s' and '%s' cannot be used\n> >> together\"), \"--subject-prefix/--rfc/--resend\", \"-k\");\n> >\n> > You probably want to be using die_for_incompatible_opt4() from\n> > parse-options.h here.\n>\n> Thanks for the suggestion. Frankly, I haven't researched the\n> available options, assuming that the current code uses the right\n> option. Of course, I'll have a detailed look into it.\n>\n> > (And you may want a preparatory patch which fixes the preimage to use\n> > die_for_incompatible_opt3() for --subject-prefix, --rfc, and -k\n> > exclusivity, though that may be overkill.)\n>\n> I'm not really sure what to do. Maybe the other reviewers would\n> prefer an orthogonal approach instead? Maybe that would be better\n> for bisecting later, if need arises for that?\n\nThe comment about using die_for_incompatible_opt4() in this patch is\nthe meaningful one.\n\nYou are very welcome to ignore the parenthesized comment about a\npreparatory patch. There is probably very little value in such a patch\nto fix the preimage to use die_for_incompatible_opt3(), only to then\napply this patch which updates it to use die_for_incompatible_opt4().\nThat would just be busy-work for you and for reviewers. I mentioned it\nonly because I noticed that the preimage was doing it wrong (not using\ndie_for_incompatible_opt3()), which presumably misled you into\ncontinuing that mistake.\n"},{"id":"493076","messageId":"82846020aedfdc5eadf5bc8349575ec1@manjaro.org","threadId":"61329","inReplyTo":"CAPig+cRBjosyadQHO03fcCz7YBc=T04ytHkpt9UU87tLaiSOgw@mail.gmail.com","subject":"Re: [PATCH 4/4] t4014: add tests to cover --resend option and its exclusivity","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T07:20:57Z","receivedAt":"2024-04-17T07:20:59Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 08:48, Eric Sunshine wrote:\n> On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic \n> wrote:\n>> Add a few new tests to the t4014 that cover the --resend command-line \n>> option\n>> for \"git format-patch\", which include the tests for its exclusivity \n>> with the\n>> already existing -k and --rfc command-line options.\n> \n> I'd recommend squashing this patch into [3/4] which introduces the\n> --resend option since it's easier to review the tests when the code\n> which is being tested is still fresh in one's mind. (For the same\n> reason, reviewers like to see documentation added in the same patch\n> which changes the code since it's easier to verify that the\n> documentation matches the implementation while it's fresh in the\n> mind.)\n\nI'm fine with that. Squashing these two patches together might also\nbe good for bisecting later, if need arises.\n\n>> Signed-off-by: Dragan Simic \n>> ---\n>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n>> @@ -1401,6 +1401,43 @@ test_expect_success '--rfc and -k cannot be \n>> used together' '\n>> +test_expect_success '--resend' '\n>> + cat >expect <<-\\EOF &&\n>> + Subject: [PATCH RESEND 1/1] header with . in it\n>> + EOF\n> \n> In all of the new tests, since it's just a single line body, it could\n> just as easily be created with `echo`:\n> \n> echo \"Subject: [PATCH RESEND 1/1] header with . in it\" >expect &&\n> \n> On the other hand, if you're following precedent in this script, then\n> using a here-doc may be just fine.\n\nI agree that using \"echo ...\" would be nicer. Though, I just wanted\nto follow the already existing tests for consistency, which may actually\noutweigh a nicer approach.\n\n> At any rate it's somewhat subjective and not worth a reroll.\n> \n>> + git format-patch -n -1 --stdout --resend >patch &&\n>> + grep \"^Subject:\" patch >actual &&\n>> + test_cmp expect actual\n>> +'\n"},{"id":"493077","messageId":"a797d1443c01cca634040206a42bc5f2@manjaro.org","threadId":"61329","inReplyTo":"CAPig+cRPUQW5ux7oKwDO5Nu46fRHrs6LrUoxnFvX9D9oNjqteg@mail.gmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T07:25:59Z","receivedAt":"2024-04-17T07:26:01Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 09:17, Eric Sunshine wrote:\n> On Wed, Apr 17, 2024 at 3:05 AM Dragan Simic \n> wrote:\n>> On 2024-04-17 08:35, Eric Sunshine wrote:\n>> > On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic \n>> > wrote:\n>> >> - die(_(\"options '%s' and '%s' cannot be used\n>> >> together\"), \"--subject-prefix/--rfc\", \"-k\");\n>> >> + die(_(\"options '%s' and '%s' cannot be used\n>> >> together\"), \"--subject-prefix/--rfc/--resend\", \"-k\");\n>> >\n>> > You probably want to be using die_for_incompatible_opt4() from\n>> > parse-options.h here.\n>> \n>> Thanks for the suggestion. Frankly, I haven't researched the\n>> available options, assuming that the current code uses the right\n>> option. Of course, I'll have a detailed look into it.\n>> \n>> > (And you may want a preparatory patch which fixes the preimage to use\n>> > die_for_incompatible_opt3() for --subject-prefix, --rfc, and -k\n>> > exclusivity, though that may be overkill.)\n>> \n>> I'm not really sure what to do. Maybe the other reviewers would\n>> prefer an orthogonal approach instead? Maybe that would be better\n>> for bisecting later, if need arises for that?\n> \n> The comment about using die_for_incompatible_opt4() in this patch is\n> the meaningful one.\n> \n> You are very welcome to ignore the parenthesized comment about a\n> preparatory patch. There is probably very little value in such a patch\n> to fix the preimage to use die_for_incompatible_opt3(), only to then\n> apply this patch which updates it to use die_for_incompatible_opt4().\n> That would just be busy-work for you and for reviewers. I mentioned it\n> only because I noticed that the preimage was doing it wrong (not using\n> die_for_incompatible_opt3()), which presumably misled you into\n> continuing that mistake.\n\nAh, makes sense, thanks for the clarification! :)\n"},{"id":"493083","messageId":"154b085c-3e92-4eb6-b6a6-97aa02f8f07d@gmail.com","threadId":"61329","inReplyTo":"1d9c6ce3df714211889453c245485d46b43edff6.1713324598.git.dsimic@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-04-17T10:02:28Z","receivedAt":"2024-04-17T10:02:31Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dragan\n\nOn 17/04/2024 04:32, Dragan Simic wrote:\n> Add --resend as the new command-line option for \"git format-patch\" that adds\n> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually producing\n> \"[PATCH RESEND]\" as the default patch subject prefix.\n> \n> \"[PATCH RESEND]\" is a patch subject prefix commonly used on mailing lists\n> for patches resent to a mailing list after they had attracted no attention\n> for some time, usually for a couple of weeks. As such, this subject prefix\n> deserves adding --resend as a new shorthand option to \"git format-patch\".\n\nPlaying devil's advocate for a minute, is this really common enough to \njustify a new option when the user can use \"--subject-prefix='PATCH \nRESEND'\" instead?\n\nBest Wishes\n\nPhillip\n\n> Of course, add the description of the new --resend command-line option to\n> the documentation for \"git format-patch\".\n> \n> Signed-off-by: Dragan Simic \n> ---\n> Documentation/git-format-patch.txt | 5 +++++\n> builtin/log.c | 11 +++++++++--\n> 2 files changed, 14 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index a5019ab46926..8e63b62620ed 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -243,6 +243,11 @@ the patches (with a value of e.g. \"PATCH my-project\").\n> \tdefault. RFC means \"Request For Comments\"; use this when sending\n> \tan experimental patch for discussion rather than application.\n> \n> +--resend::\n> +\tAppends \"RESEND\" to the subject prefix, producing \"PATCH RESEND\"\n> +\tby default. Use this when sending again a patch that had resulted\n> +\tin attracting no discussion for a while.\n> +\n> -v ::\n> --reroll-count=::\n> \tMark the series as the -th iteration of the topic. The\n> diff --git a/builtin/log.c b/builtin/log.c\n> index e5a238f1cf2c..28f31659bcde 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1908,7 +1908,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> +\tint rfc = 0, resend = 0;\n> \n> \tconst struct option builtin_format_patch_options[] = {\n> \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n> @@ -1933,6 +1933,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\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_BOOL(0, \"resend\", &resend, N_(\"use [PATCH RESEND] 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> @@ -2055,6 +2056,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n> \t\tsubject_prefix = 1;\n> \t}\n> +\tif (resend) {\n> +\t\tstrbuf_addstr(&sprefix, \" RESEND\");\n> +\t\tsubject_prefix = 1;\n> +\t}\n> \n> \tif (reroll_count) {\n> \t\tstrbuf_addf(&sprefix, \" v%s\", reroll_count);\n> @@ -2111,7 +2116,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \tif (numbered && keep_subject)\n> \t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"-n\", \"-k\");\n> \tif (keep_subject && subject_prefix)\n> -\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--subject-prefix/--rfc\", \"-k\");\n> +\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--subject-prefix/--rfc/--resend\", \"-k\");\n> +\tif (rfc && resend)\n> +\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--rfc\", \"--resend\");\n> \trev.preserve_subject = keep_subject;\n> \n> \targc = setup_revisions(argc, argv, &rev, &s_r_opt);\n> \n"},{"id":"493084","messageId":"1f31004bd8445e1e4717817638d5509a@manjaro.org","threadId":"61329","inReplyTo":"154b085c-3e92-4eb6-b6a6-97aa02f8f07d@gmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T10:52:48Z","receivedAt":"2024-04-17T10:52:51Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Phillip,\n\nOn 2024-04-17 12:02, Phillip Wood wrote:\n> On 17/04/2024 04:32, Dragan Simic wrote:\n>> Add --resend as the new command-line option for \"git format-patch\" \n>> that adds\n>> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually \n>> producing\n>> \"[PATCH RESEND]\" as the default patch subject prefix.\n>> \n>> \"[PATCH RESEND]\" is a patch subject prefix commonly used on mailing \n>> lists\n>> for patches resent to a mailing list after they had attracted no \n>> attention\n>> for some time, usually for a couple of weeks. As such, this subject \n>> prefix\n>> deserves adding --resend as a new shorthand option to \"git \n>> format-patch\".\n> \n> Playing devil's advocate for a minute, is this really common enough to\n> justify a new option when the user can use \"--subject-prefix='PATCH\n> RESEND'\" instead?\n\nBased on my experience, \"[PATCH RESEND]\" is roughly as commonly\nused as \"[PATCH RFC]\". In other words, it obviously isn't used\nas much as the good, old plain \"[PATCH]\", but it is used.\n\nWe should also take the overall usability into account, if you\nagree. Just like with \"--rfc\", typing \"--resend\" is much easier\nand quicker than typing \"--subject-prefix='PATCH RESEND'\", which\nis a lot. Defining an alias can help, of course, but that isn't\nalways a convenient solution.\n"},{"id":"493085","messageId":"d60e5ddd-643d-41f2-849d-6ab660df734c@app.fastmail.com","threadId":"61329","inReplyTo":"1f31004bd8445e1e4717817638d5509a@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-04-17T11:31:19Z","receivedAt":"2024-04-17T11:31:41Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Wed, Apr 17, 2024, at 12:52, Dragan Simic wrote:\n> Hello Phillip,\n>\n> On 2024-04-17 12:02, Phillip Wood wrote:\n>> On 17/04/2024 04:32, Dragan Simic wrote:\n>>> Add --resend as the new command-line option for \"git format-patch\"\n>>> that adds\n>>> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually\n>>> producing\n>>> \"[PATCH RESEND]\" as the default patch subject prefix.\n>>>\n>>> \"[PATCH RESEND]\" is a patch subject prefix commonly used on mailing\n>>> lists\n>>> for patches resent to a mailing list after they had attracted no\n>>> attention\n>>> for some time, usually for a couple of weeks. As such, this subject\n>>> prefix\n>>> deserves adding --resend as a new shorthand option to \"git\n>>> format-patch\".\n>>\n>> Playing devil's advocate for a minute, is this really common enough to\n>> justify a new option when the user can use \"--subject-prefix='PATCH\n>> RESEND'\" instead?\n>\n> Based on my experience, \"[PATCH RESEND]\" is roughly as commonly\n> used as \"[PATCH RFC]\". In other words, it obviously isn't used\n> as much as the good, old plain \"[PATCH]\", but it is used.\n\nThe format-patch generated string is `RFC PATCH`.\n\nThe number of emails with `PATCH RESEND` for this list:[1]\n\n```\n$ git log --oneline --fixed-strings --grep='[PATCH RESEND' | wc -l\n28\n```\n\nFor RFC:\n\n```\n$ git log --oneline --fixed-strings --grep='[RFC PATCH' | wc -l\n1181\n```\n\n† 1: According to http://lore.kernel.org/git/1\n\n-- \nKristoffer Haugsbakk\n\n"},{"id":"493086","messageId":"e3caab896300a2da9fcde2e0b2efe3d2@manjaro.org","threadId":"61329","inReplyTo":"d60e5ddd-643d-41f2-849d-6ab660df734c@app.fastmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T11:34:52Z","receivedAt":"2024-04-17T11:34:54Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 13:31, Kristoffer Haugsbakk wrote:\n> On Wed, Apr 17, 2024, at 12:52, Dragan Simic wrote:\n>> On 2024-04-17 12:02, Phillip Wood wrote:\n>>> On 17/04/2024 04:32, Dragan Simic wrote:\n>>>> Add --resend as the new command-line option for \"git format-patch\"\n>>>> that adds\n>>>> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually\n>>>> producing\n>>>> \"[PATCH RESEND]\" as the default patch subject prefix.\n>>>> \n>>>> \"[PATCH RESEND]\" is a patch subject prefix commonly used on mailing\n>>>> lists\n>>>> for patches resent to a mailing list after they had attracted no\n>>>> attention\n>>>> for some time, usually for a couple of weeks. As such, this subject\n>>>> prefix\n>>>> deserves adding --resend as a new shorthand option to \"git\n>>>> format-patch\".\n>>> \n>>> Playing devil's advocate for a minute, is this really common enough \n>>> to\n>>> justify a new option when the user can use \"--subject-prefix='PATCH\n>>> RESEND'\" instead?\n>> \n>> Based on my experience, \"[PATCH RESEND]\" is roughly as commonly\n>> used as \"[PATCH RFC]\". In other words, it obviously isn't used\n>> as much as the good, old plain \"[PATCH]\", but it is used.\n> \n> The format-patch generated string is `RFC PATCH`.\n\nTrue. It's just that I more often see \"PATCH RFC\", for some reason.\nPlease note that I'm also taking other mailing lists into account.\n\n> The number of emails with `PATCH RESEND` for this list:[1]\n> \n> $ git log --oneline --fixed-strings --grep='[PATCH RESEND' | wc -l\n> 28\n> \n> For RFC:\n> \n> $ git log --oneline --fixed-strings --grep='[RFC PATCH' | wc -l\n> 1181\n> \n> † 1: According to http://lore.kernel.org/git/1\n\nI wonder what does it say for \"RESEND\" only?\n"},{"id":"493087","messageId":"675e2dec-a80e-4b5d-84ab-75ec5604a1be@app.fastmail.com","threadId":"61329","inReplyTo":"9a6a9cb1d9dd07bbbbc47616c510779a@manjaro.org","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-04-17T11:38:08Z","receivedAt":"2024-04-17T11:38:29Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Wed, Apr 17, 2024, at 09:11, Dragan Simic wrote:\n>> I had the same question but left it unwritten since I noticed that\n>> this new test is modelled after the test immediately following it in\n>> the script, and the existing test also redirects to \"patch\"\n>> unnecessarily. So, if it's done this way for consistency with existing\n>> tests, I don't mind letting it slide.\n>\n> Yes, I also wasn't super happy with this new test, as I already noted\n> in one of my replies, but improving this and the other similar tests\n> is most probably something best left for a follow-up series.\n\nI don’t see the point in writing the test in mimic-neighbors way only to\nimprove it shortly after.\n\nIf the test can be written in a better way then the other tests can be\nimproved later. Or now. I think I’ve seen other discussions were a less\ngood pattern wasn’t accepted in new tests even though they were used in\nexisting ones. The reviewer then pointed out that the other tests should\nbe updated later.\n\nThat’s just my opinion and recollection.\n\n-- \nKristoffer Haugsbakk\n\n"},{"id":"493088","messageId":"eafb3b1a-59ad-4b51-b5fb-061b82e06c81@app.fastmail.com","threadId":"61329","inReplyTo":"e3caab896300a2da9fcde2e0b2efe3d2@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-04-17T11:40:45Z","receivedAt":"2024-04-17T11:41:07Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Wed, Apr 17, 2024, at 13:34, Dragan Simic wrote:\n>> $ git log --oneline --fixed-strings --grep='[RFC PATCH' | wc -l\n>> 1181\n>>\n>> † 1: According to http://lore.kernel.org/git/1\n>\n> I wonder what does it say for \"RESEND\" only?\n\n```\n$ git log --oneline --fixed-strings --grep='[RESEND' | wc -l\n27\n$ git log --oneline --fixed-strings --grep='RESEND' | wc -l\n57\n```\n"},{"id":"493089","messageId":"b4d2b3faaf2914b7083327d5a4be3905@manjaro.org","threadId":"61329","inReplyTo":"e3caab896300a2da9fcde2e0b2efe3d2@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T11:43:59Z","receivedAt":"2024-04-17T11:44:01Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 13:34, Dragan Simic wrote:\n> On 2024-04-17 13:31, Kristoffer Haugsbakk wrote:\n>> On Wed, Apr 17, 2024, at 12:52, Dragan Simic wrote:\n>>> On 2024-04-17 12:02, Phillip Wood wrote:\n>>>> On 17/04/2024 04:32, Dragan Simic wrote:\n>>>>> Add --resend as the new command-line option for \"git format-patch\"\n>>>>> that adds\n>>>>> \"RESEND\" as a (sub)suffix to the patch subject prefix, eventually\n>>>>> producing\n>>>>> \"[PATCH RESEND]\" as the default patch subject prefix.\n>>>>> \n>>>>> \"[PATCH RESEND]\" is a patch subject prefix commonly used on mailing\n>>>>> lists\n>>>>> for patches resent to a mailing list after they had attracted no\n>>>>> attention\n>>>>> for some time, usually for a couple of weeks. As such, this \n>>>>> subject\n>>>>> prefix\n>>>>> deserves adding --resend as a new shorthand option to \"git\n>>>>> format-patch\".\n>>>> \n>>>> Playing devil's advocate for a minute, is this really common enough \n>>>> to\n>>>> justify a new option when the user can use \"--subject-prefix='PATCH\n>>>> RESEND'\" instead?\n>>> \n>>> Based on my experience, \"[PATCH RESEND]\" is roughly as commonly\n>>> used as \"[PATCH RFC]\". In other words, it obviously isn't used\n>>> as much as the good, old plain \"[PATCH]\", but it is used.\n>> \n>> The format-patch generated string is `RFC PATCH`.\n> \n> True. It's just that I more often see \"PATCH RFC\", for some reason.\n> Please note that I'm also taking other mailing lists into account.\n> \n>> The number of emails with `PATCH RESEND` for this list:[1]\n>> \n>> $ git log --oneline --fixed-strings --grep='[PATCH RESEND' | wc -l\n>> 28\n>> \n>> For RFC:\n>> \n>> $ git log --oneline --fixed-strings --grep='[RFC PATCH' | wc -l\n>> 1181\n>> \n>> † 1: According to http://lore.kernel.org/git/1\n> \n> I wonder what does it say for \"RESEND\" only?\n\nHere are some numbers pulled from https://lore.kernel.org/linux-kernel/:\n\n- \"RFC\": ~400,000\n- \"PATCH RFC\": ~50,000\n- \"RFC PATCH\": ~200,000\n- \"RESEND\": ~200,000\n- \"PATCH RESEND\": ~30,000\n- \"RESEND PATCH\": ~30,000\n\nThough, I'm not sure how accurate those numbers are. Even a cursory\nlook at the produced search results shows inaccuracy of the search\nmatches. There's probably some \"fuzzy logic\" at play there.\n"},{"id":"493090","messageId":"0ade0ce2348ab24617e19bc60e648b64@manjaro.org","threadId":"61329","inReplyTo":"675e2dec-a80e-4b5d-84ab-75ec5604a1be@app.fastmail.com","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T11:48:22Z","receivedAt":"2024-04-17T11:48:24Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 13:38, Kristoffer Haugsbakk wrote:\n> On Wed, Apr 17, 2024, at 09:11, Dragan Simic wrote:\n>>> I had the same question but left it unwritten since I noticed that\n>>> this new test is modelled after the test immediately following it in\n>>> the script, and the existing test also redirects to \"patch\"\n>>> unnecessarily. So, if it's done this way for consistency with \n>>> existing\n>>> tests, I don't mind letting it slide.\n>> \n>> Yes, I also wasn't super happy with this new test, as I already noted\n>> in one of my replies, but improving this and the other similar tests\n>> is most probably something best left for a follow-up series.\n> \n> I don’t see the point in writing the test in mimic-neighbors way only \n> to\n> improve it shortly after.\n\nWell, the logic is quite simple: let me get this patch accepted,\nand we'll deal with the improvements later. Though, don't get me\nwrong, I'd always prefer to see things done the right way, but the\ntime, just like the other resources, is limited.\n\n> If the test can be written in a better way then the other tests can be\n> improved later. Or now. I think I’ve seen other discussions were a less\n> good pattern wasn’t accepted in new tests even though they were used in\n> existing ones. The reviewer then pointed out that the other tests \n> should\n> be updated later.\n> \n> That’s just my opinion and recollection.\n\nI see, but this makes me wonder how often the other tests actually\nget improved later?\n"},{"id":"493091","messageId":"d2a4e3b3a5abbf1fe1e8a164b410edd8@manjaro.org","threadId":"61329","inReplyTo":"e4aa5235-c6ad-45c7-930e-de991cc375f2@app.fastmail.com","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T12:00:29Z","receivedAt":"2024-04-17T12:00:31Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 08:33, Kristoffer Haugsbakk wrote:\n> On Wed, Apr 17, 2024, at 05:32, Dragan Simic wrote:\n>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n>> index e37a1411ee24..e22c4ac34e6e 100755\n>> --- a/t/t4014-format-patch.sh\n>> +++ b/t/t4014-format-patch.sh\n>> @@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order\n>> independent' '\n>> \ttest_cmp expect actual\n>> '\n>> \n>> +test_expect_success '--rfc and -k cannot be used together' '\n>> +\ttest_must_fail git format-patch -1 --stdout --rfc -k >patch\n> \n> I don’t understand why you redirect to `patch` if you only check the\n> exit code. (I don’t expect any stdout since it will fail.)\n> \n> Although it would be nice with a text comparison or grep on the stderr\n> output to make sure that the command died for the expected reason. But \n> I\n> haven’t read the associated code.\n\nActually, if you agree, we should check both the stdout\nand stderr -- the former for emptiness, and the latter for\nthe expected error message.\n\n>> +'\n>> +\n>> test_expect_success '--from=ident notices bogus ident' '\n>> \ttest_must_fail git format-patch -1 --stdout --from=foo >patch\n>> '\n"},{"id":"493094","messageId":"xmqq7cgwau1v.fsf@gitster.g","threadId":"61329","inReplyTo":"154b085c-3e92-4eb6-b6a6-97aa02f8f07d@gmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-17T15:27:24Z","receivedAt":"2024-04-17T15:27:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Playing devil's advocate for a minute, is this really common enough to\n> justify a new option when the user can use \"--subject-prefix='PATCH\n> RESEND'\" instead?\n\nThe same applies to \"--rfc\", but the justification goes like this.\n\n * When you are working on a single subsystem in a larger project,\n your patches would want to carry the subsystem name. You'd use\n \"--subject-prefix='PATCH frotz'\" (and more likely it comes from\n format.subjectPrefix in a working repository dedicated to work on\n the frotz subsystem) for that.\n\n * In the context of working on that subsystem, sometimes you would\n need to mark your patch as a RFC patch, i.e., \"[RFC PATCH frotz]\",\n and that is done per-invocation basis (i.e., you are not always\n constantly sending an RFC) with \"--rfc\".\n\nHaving orthogonal two mechanisms whose results are concatenated\ntogether is handy than having to specify the whole thing.\n\nI somehow thought that during the review of the \"--rfc\" option a few\nideas were brought up to deal with adornments other than but similar\nto RFC. I still think the approach to make \"--rfc\" take an optional\nvalue, e.g., \"--rfc=WIP\" from the repository working in \"frotz\"\nsubsystem would produce \"[WIP PATCH frotz v2 2/4]\" a reasonable one.\n\ncf. https://lore.kernel.org/git/xmqqbkepep9k.fsf@gitster.g/\n\nThanks.\n\n"},{"id":"493106","messageId":"c2cb9268c29ae4a5cac34383b7443763@manjaro.org","threadId":"61329","inReplyTo":"xmqq7cgwau1v.fsf@gitster.g","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T17:34:09Z","receivedAt":"2024-04-17T17:34:11Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Junio,\n\nOn 2024-04-17 17:27, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> Playing devil's advocate for a minute, is this really common enough to\n>> justify a new option when the user can use \"--subject-prefix='PATCH\n>> RESEND'\" instead?\n> \n> The same applies to \"--rfc\", but the justification goes like this.\n> \n> * When you are working on a single subsystem in a larger project,\n> your patches would want to carry the subsystem name. You'd use\n> \"--subject-prefix='PATCH frotz'\" (and more likely it comes from\n> format.subjectPrefix in a working repository dedicated to work on\n> the frotz subsystem) for that.\n> \n> * In the context of working on that subsystem, sometimes you would\n> need to mark your patch as a RFC patch, i.e., \"[RFC PATCH frotz]\",\n> and that is done per-invocation basis (i.e., you are not always\n> constantly sending an RFC) with \"--rfc\".\n> \n> Having orthogonal two mechanisms whose results are concatenated\n> together is handy than having to specify the whole thing.\n> \n> I somehow thought that during the review of the \"--rfc\" option a few\n> ideas were brought up to deal with adornments other than but similar\n> to RFC. I still think the approach to make \"--rfc\" take an optional\n> value, e.g., \"--rfc=WIP\" from the repository working in \"frotz\"\n> subsystem would produce \"[WIP PATCH frotz v2 2/4]\" a reasonable one.\n\nWith all due respect, \"--rfc=WIP\" looks like a kludge, simply\nbecause \"--rfc\" should, IIUC, be some kind of a fixed shorthand.\nPerhaps a new option should be added for that purpose, but I'm\nnot really sure how it could be called.\n\n> cf. https://lore.kernel.org/git/xmqqbkepep9k.fsf@gitster.g/\n> \n> Thanks.\n"},{"id":"493110","messageId":"xmqqle5b66sr.fsf@gitster.g","threadId":"61329","inReplyTo":"c2cb9268c29ae4a5cac34383b7443763@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-17T21:03:16Z","receivedAt":"2024-04-17T21:03:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dragan Simic writes:\n\n> With all due respect, \"--rfc=WIP\" looks like a kludge, simply\n> because \"--rfc\" should, IIUC, be some kind of a fixed shorthand.\n\nI wouldn't use \"should\" there. In any case, we are not going to add\nunbounded number of --wip, --resend, etc., on top of what we have\nalready. Introducing --something={WIP,RESEND,RFC,HACK,...} and\ndeprecating --rfc is not something I would object to, though.\n\nThanks.\n"},{"id":"493111","messageId":"19d5f3d4c99fc1da24c80ac2a9ee8bf8@manjaro.org","threadId":"61329","inReplyTo":"xmqqle5b66sr.fsf@gitster.g","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T21:09:03Z","receivedAt":"2024-04-17T21:09:05Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 23:03, Junio C Hamano wrote:\n> Dragan Simic writes:\n> \n>> With all due respect, \"--rfc=WIP\" looks like a kludge, simply\n>> because \"--rfc\" should, IIUC, be some kind of a fixed shorthand.\n> \n> I wouldn't use \"should\" there. In any case, we are not going to add\n> unbounded number of --wip, --resend, etc., on top of what we have\n> already. Introducing --something={WIP,RESEND,RFC,HACK,...} and\n> deprecating --rfc is not something I would object to, though.\n\nGood to know, thanks. I'll drop the patches that add \"--resend\"\nas a new command-line option, and I'll think a bit about the solution\nyou described as acceptable.\n"},{"id":"493115","messageId":"84dcb80be916f85cbb6a4b99aea0d76b@manjaro.org","threadId":"61329","inReplyTo":"19d5f3d4c99fc1da24c80ac2a9ee8bf8@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-18T03:12:55Z","receivedAt":"2024-04-18T03:12:57Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-04-17 23:09, Dragan Simic wrote:\n> On 2024-04-17 23:03, Junio C Hamano wrote:\n>> Dragan Simic writes:\n>> \n>>> With all due respect, \"--rfc=WIP\" looks like a kludge, simply\n>>> because \"--rfc\" should, IIUC, be some kind of a fixed shorthand.\n>> \n>> I wouldn't use \"should\" there. In any case, we are not going to add\n>> unbounded number of --wip, --resend, etc., on top of what we have\n>> already. Introducing --something={WIP,RESEND,RFC,HACK,...} and\n>> deprecating --rfc is not something I would object to, though.\n> \n> Good to know, thanks. I'll drop the patches that add \"--resend\"\n> as a new command-line option, and I'll think a bit about the solution\n> you described as acceptable.\n\nHow about introducing \"--label=\" as the new option, where\n\"\" could also contain '$' as the last character, which would\nget stripped while indicating that the label is to treated as a\n(sub)suffix, instead of as a (sub)prefix.\n\nFor example, \"--label=RFC\" would be equal to the current \"--rfc\"\n(which would also become deprecated), producing \"[RFC PATCH]\",\n\"--label=WIP\" would produce \"[WIP PATCH]\", and \"--label=RESEND$\"\nwould produce \"[PATCH RESEND]\".\n\nSpecifying '$' before a space character in a command line doesn't\ntrigger parameter substitution or variable expansion by the shell,\nwhich means that using '$' as a \"suffix anchor\", as proposed above,\nwould require no escaping or use of single quotation marks, making\nit more convenient to use.\n\nPlease, let me know your thoughts.\n"},{"id":"493118","messageId":"fa2532fdfd57944c7dd915b0ecc9ce15@manjaro.org","threadId":"61329","inReplyTo":"47ac35d45693aa5b9fa061e85da6e176@manjaro.org","subject":"Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-18T09:12:26Z","receivedAt":"2024-04-18T09:12:28Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Patrick,\n\nOn 2024-04-17 08:56, Dragan Simic wrote:\n> On 2024-04-17 08:27, Patrick Steinhardt wrote:\n>> On Wed, Apr 17, 2024 at 05:32:42AM +0200, Dragan Simic wrote:\n>>> Fix a bug that allows --rfc and -k options to be specified together \n>>> when\n>>> executing \"git format-patch\". This bug was introduced back in the \n>>> commit\n>>> e0d7db7423a9 (\"format-patch: --rfc honors what --subject-prefix \n>>> sets\"),\n>>> about eight months ago, but it has remained undetected so far, \n>>> presumably\n>>> because of no associated test coverage.\n>>> \n>>> Add a new test to the t4014 that covers the mutual exclusivity of the \n>>> --rfc\n>>> and -k command-line options for \"git format-patch\", for future \n>>> coverage.\n>>> \n>>> Signed-off-by: Dragan Simic \n>>> ---\n>>> builtin/log.c | 5 ++++-\n>>> t/t4014-format-patch.sh | 4 ++++\n>>> 2 files changed, 8 insertions(+), 1 deletion(-)\n>>> \n>>> diff --git a/builtin/log.c b/builtin/log.c\n>>> index c0a8bb95e983..e5a238f1cf2c 100644\n>>> --- a/builtin/log.c\n>>> +++ b/builtin/log.c\n>>> @@ -2050,8 +2050,11 @@ 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/* Also mark the subject prefix as modified, for later checks */\n>>> +\tif (rfc) {\n>>> \t\tstrbuf_insertstr(&sprefix, 0, \"RFC \");\n>>> +\t\tsubject_prefix = 1;\n>>> +\t}\n>> \n>> As an alternative fix, can we drop `subject_prefix` and replace it \n>> with\n>> `sprefix.len` instead? It seems to merely be a proxy value for that\n>> anyway, and if we didn't have that variable then the bug would not \n>> have\n>> been possible to begin with.\n> \n> Thanks for your feedback!\n> \n> I'll think about it, and I'll come back a bit later with an update.\n\nUnfortunately, we can't use sprefix.len instead, because it can\nstill be zero even if the --subject-prefix option was present, more\nspecifically if we receive --subject-prefix=\"\" on the command line.\n\nThe checks that use subject_prefix need to check if --subject-prefix\nwas specified at all as an option, instead of checking if the actual\nsubject prefix is of non-zero length.\n\nAs you already noted, if sprefix.len was used instead of the separate\nsubject_prefix variable, the '--rfc -k' bug wouldn't be possible, but\nthe new '--subject-prefix=\"\" -k' bug would be possible instead.\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 e37a1411ee24..e22c4ac34e6e 100755\n>>> --- a/t/t4014-format-patch.sh\n>>> +++ b/t/t4014-format-patch.sh\n>>> @@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order \n>>> independent' '\n>>> \ttest_cmp expect actual\n>>> '\n>>> \n>>> +test_expect_success '--rfc and -k cannot be used together' '\n>>> +\ttest_must_fail git format-patch -1 --stdout --rfc -k >patch\n>>> +'\n>>> +\n>>> test_expect_success '--from=ident notices bogus ident' '\n>>> \ttest_must_fail git format-patch -1 --stdout --from=foo >patch\n>>> '\n"},{"id":"493155","messageId":"cb2d20938cc9b11e621103575b1bb379@manjaro.org","threadId":"61329","inReplyTo":"a0b93341380c2157f6b87e19129abb49@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-18T20:04:57Z","receivedAt":"2024-04-18T20:04:59Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Eric,\n\nOn 2024-04-17 09:05, Dragan Simic wrote:\n> On 2024-04-17 08:35, Eric Sunshine wrote:\n>> On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic \n>> wrote:\n>>> diff --git a/builtin/log.c b/builtin/log.c\n>>> @@ -2111,7 +2116,9 @@ int cmd_format_patch(int argc, const char \n>>> **argv, const char *prefix)\n>>> if (keep_subject && subject_prefix)\n>>> - die(_(\"options '%s' and '%s' cannot be used \n>>> together\"), \"--subject-prefix/--rfc\", \"-k\");\n>>> + die(_(\"options '%s' and '%s' cannot be used \n>>> together\"), \"--subject-prefix/--rfc/--resend\", \"-k\");\n>> \n>> You probably want to be using die_for_incompatible_opt4() from\n>> parse-options.h here.\n> \n> Thanks for the suggestion. Frankly, I haven't researched the\n> available options, assuming that the current code uses the right\n> option. Of course, I'll have a detailed look into it.\n\nUnfortunately, die_for_incompatible_opt3() cannot be used because\nit also prevents the --subject-prefix and --rfc options from being\nused together, which is expected to be possible.\n\n>> (And you may want a preparatory patch which fixes the preimage to use\n>> die_for_incompatible_opt3() for --subject-prefix, --rfc, and -k\n>> exclusivity, though that may be overkill.)\n> \n> I'm not really sure what to do. Maybe the other reviewers would\n> prefer an orthogonal approach instead? Maybe that would be better\n> for bisecting later, if need arises for that?\n"},{"id":"493172","messageId":"xmqq5xwepafi.fsf@gitster.g","threadId":"61329","inReplyTo":"84dcb80be916f85cbb6a4b99aea0d76b@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-18T22:34:25Z","receivedAt":"2024-04-18T22:34:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dragan Simic writes:\n\n>>>> With all due respect, \"--rfc=WIP\" looks like a kludge, simply\n>>>> because \"--rfc\" should, IIUC, be some kind of a fixed shorthand.\n>>> I wouldn't use \"should\" there.\n\n> How about introducing \"--label=\" as the new option,...\n\nI still think --rfc=WIP is a lot more natural and easier to\nunderstand, and it is just the matter of how you introduce it.\nI'll show you how in a separate patch later.\n\nThe problem I see with an overly generic word like \"label\" is that\nit would mislead readers to say \"--label=important\" and expect it to\nappear on an extra e-mail header, not as a part of \"Subject:\".\n\nBut we can do this to get the ball rolling, without bikeshedding\nwhat option name to use. Until we find a good name, users can\nuse --rfc=WIP and when we do find a good name, it can be added\nas a synonym, possibly deprecating --rfc, and if we never agree\non a good name, that is fine as well.\n"},{"id":"493174","messageId":"e6afba5f27887fb35e2e236135ac06b8@manjaro.org","threadId":"61329","inReplyTo":"xmqq5xwepafi.fsf@gitster.g","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-19T00:08:15Z","receivedAt":"2024-04-19T00:08:18Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Junio,\n\nOn 2024-04-19 00:34, Junio C Hamano wrote:\n> Dragan Simic writes:\n> \n>>>>> With all due respect, \"--rfc=WIP\" looks like a kludge, simply\n>>>>> because \"--rfc\" should, IIUC, be some kind of a fixed shorthand.\n>>>> I wouldn't use \"should\" there.\n> \n>> How about introducing \"--label=\" as the new option,...\n> \n> I still think --rfc=WIP is a lot more natural and easier to\n> understand, and it is just the matter of how you introduce it.\n> I'll show you how in a separate patch later.\n> \n> The problem I see with an overly generic word like \"label\" is that\n> it would mislead readers to say \"--label=important\" and expect it to\n> appear on an extra e-mail header, not as a part of \"Subject:\".\n\n\"Label\" is a little generic, I'll give you that.\n\n> But we can do this to get the ball rolling, without bikeshedding\n> what option name to use. Until we find a good name, users can\n> use --rfc=WIP and when we do find a good name, it can be added\n> as a synonym, possibly deprecating --rfc, and if we never agree\n> on a good name, that is fine as well.\n\nIf you insist, let's do it that way! :)\n"},{"id":"493175","messageId":"CAPig+cT9A9N=zGZDXuB+c17L8hZ-h5zvZgD5W-8VYqiM9QaBew@mail.gmail.com","threadId":"61329","inReplyTo":"xmqq5xwepafi.fsf@gitster.g","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-19T00:15:11Z","receivedAt":"2024-04-19T00:15:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 18, 2024 at 6:34 PM Junio C Hamano wrote:\n> Dragan Simic writes:\n> > How about introducing \"--label=\" as the new option,...\n>\n> I still think --rfc=WIP is a lot more natural and easier to\n> understand, and it is just the matter of how you introduce it.\n> I'll show you how in a separate patch later.\n>\n> The problem I see with an overly generic word like \"label\" is that\n> it would mislead readers to say \"--label=important\" and expect it to\n> appear on an extra e-mail header, not as a part of \"Subject:\".\n>\n> But we can do this to get the ball rolling, without bikeshedding\n> what option name to use. Until we find a good name, users can\n> use --rfc=WIP and when we do find a good name, it can be added\n> as a synonym, possibly deprecating --rfc, and if we never agree\n> on a good name, that is fine as well.\n\nI remain skeptical that adding such an option is necessary, even\nthough I made a similar suggestion earlier in this discussion as an\nalternative to `--resend`. I'm especially skeptical since the existing\n`--subject-prefix` covers this use-case already (i.e.\n`--subject-prefix=\"RESEND PATCH\"`). It's dead simple to use and\ndoesn't require any magical incantations with corresponding complex\nimplementation such as the proposed `--label=RESEND$` which renders as\n\"[PATCH RESEND]\" instead of \"[RESEND PATCH]\"; `--subject-prefix`\nalready handles this without any need for magic.\n\nI do understand and am sympathetic to the desire to reduce the typing\nload (hence, the original `--resend` proposal), but I have difficulty\nbelieving that `git format-patch` is so commonly used throughout the\nday that the time saved by typing `--resend` over\n`--subject-prefix=\"RESEND PATCH\"` warrants the extra implementation,\ndocumentation, and testing baggage. Likewise, I don't see the value in\n`--label=WIP` (or `--rfc=WIP` or whatever) over the existing more\ngeneral `--subject-prefix`.\n\nIf reducing the typing load is the primary concern, then a very simple\nmiddle-ground would be to give `--subject-prefix` a short alias (i.e.\n`-S`). It's true that `-S \"RESEND PATCH\"` doesn't reduce the typing\nload as much as `--resend` does over `--subject-prefix=\"RESEND\nPATCH\"`, but it seems a reasonable alternative which doesn't\nsignificantly increase implementation, documentation, and testing\ncosts.\n"},{"id":"493177","messageId":"a24045ae382f91fed6a499d93690e31f@manjaro.org","threadId":"61329","inReplyTo":"CAPig+cT9A9N=zGZDXuB+c17L8hZ-h5zvZgD5W-8VYqiM9QaBew@mail.gmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-19T00:45:23Z","receivedAt":"2024-04-19T00:45:27Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Eric,\n\nOn 2024-04-19 02:15, Eric Sunshine wrote:\n> On Thu, Apr 18, 2024 at 6:34 PM Junio C Hamano \n> wrote:\n>> Dragan Simic writes:\n>> > How about introducing \"--label=\" as the new option,...\n>> \n>> I still think --rfc=WIP is a lot more natural and easier to\n>> understand, and it is just the matter of how you introduce it.\n>> I'll show you how in a separate patch later.\n>> \n>> The problem I see with an overly generic word like \"label\" is that\n>> it would mislead readers to say \"--label=important\" and expect it to\n>> appear on an extra e-mail header, not as a part of \"Subject:\".\n>> \n>> But we can do this to get the ball rolling, without bikeshedding\n>> what option name to use. Until we find a good name, users can\n>> use --rfc=WIP and when we do find a good name, it can be added\n>> as a synonym, possibly deprecating --rfc, and if we never agree\n>> on a good name, that is fine as well.\n> \n> I remain skeptical that adding such an option is necessary, even\n> though I made a similar suggestion earlier in this discussion as an\n> alternative to `--resend`. I'm especially skeptical since the existing\n> `--subject-prefix` covers this use-case already (i.e.\n> `--subject-prefix=\"RESEND PATCH\"`). It's dead simple to use and\n> doesn't require any magical incantations with corresponding complex\n> implementation such as the proposed `--label=RESEND$` which renders as\n> \"[PATCH RESEND]\" instead of \"[RESEND PATCH]\"; `--subject-prefix`\n> already handles this without any need for magic.\n> \n> I do understand and am sympathetic to the desire to reduce the typing\n> load (hence, the original `--resend` proposal), but I have difficulty\n> believing that `git format-patch` is so commonly used throughout the\n> day that the time saved by typing `--resend` over\n> `--subject-prefix=\"RESEND PATCH\"` warrants the extra implementation,\n> documentation, and testing baggage. Likewise, I don't see the value in\n> `--label=WIP` (or `--rfc=WIP` or whatever) over the existing more\n> general `--subject-prefix`.\n\nAn additional reason, IMHO, for having \"--rfc\", \"--rfc=\"\nor \"--resend\" is to reuse what's already configured through the\n\"format.subjectPrefix\" configuration option. In the sense of not\nredefining what's already configured in ~/.gitconfig (in this case,\n\"PATCH\" or \"PATCH lib\", for example), by specifying an additional\ncommand-line option.\n\nIf some user configures different values for \"format.subjectPrefix\"\nin different local repositories, such as when working on different\nsubsystems, it becomes rather easy to get lost in all those prefixes,\nif the user needs to remember and type them entirely while using\n\"--subject-prefix=\" to add more \"labels\" to a prefix.\n\nI hope it makes sense the way I wrote it above.\n\n> If reducing the typing load is the primary concern, then a very simple\n> middle-ground would be to give `--subject-prefix` a short alias (i.e.\n> `-S`). It's true that `-S \"RESEND PATCH\"` doesn't reduce the typing\n> load as much as `--resend` does over `--subject-prefix=\"RESEND\n> PATCH\"`, but it seems a reasonable alternative which doesn't\n> significantly increase implementation, documentation, and testing\n> costs.\n\nI'd support the addition of a short alias for the already existing\n\"--subject-prefix\" option.\n"},{"id":"493179","messageId":"xmqqedb2nlpf.fsf@gitster.g","threadId":"61329","inReplyTo":"CAPig+cT9A9N=zGZDXuB+c17L8hZ-h5zvZgD5W-8VYqiM9QaBew@mail.gmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-19T02:13:48Z","receivedAt":"2024-04-19T02:13:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> I do understand and am sympathetic to the desire to reduce the typing\n> load (hence, the original `--resend` proposal), but I have difficulty\n> believing that `git format-patch` is so commonly used throughout the\n> day that the time saved by typing `--resend` over\n> `--subject-prefix=\"RESEND PATCH\"` warrants the extra implementation,\n> documentation, and testing baggage. Likewise, I don't see the value in\n> `--label=WIP` (or `--rfc=WIP` or whatever) over the existing more\n> general `--subject-prefix`.\n\nI am not interested in adding unbounded number of --wip and the like\nat all, but the value you seem to be missing of the separate \"--rfc\"\nis that there are folks who configure something other than \"PATCH\"\nto \"format.subjectPrefix\". They do not want to keep typing\n--subject-prefix=\"PATCH net-next\" on the command line, so they use\nthe configuration variable, which is \"set it once and forget\". The\nstress is on the fact that they can forget about it.\n\nIf they are told to say --subject-prefix=\"RFC PATCH net-next\" when\nthey want to send an RFC patch as one-shot basis, they would not be\nhappy. That is where the value of a command line \"--rfc\" for a\nparticular invocation is---they don't have to remember or care that\ntheir normal subject prefix is \"PATCH net-next\", which is required\nif you forced them to use --subject-prefix.\n"},{"id":"493181","messageId":"CAPig+cQHyDbnZGXJ0qjfn6JcOv3V=_RgZGMNnxkd4OcVfaE-sA@mail.gmail.com","threadId":"61329","inReplyTo":"a24045ae382f91fed6a499d93690e31f@manjaro.org","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-19T03:05:24Z","receivedAt":"2024-04-19T03:05:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 18, 2024 at 8:45 PM Dragan Simic wrote:\n> On 2024-04-19 02:15, Eric Sunshine wrote:\n> > I do understand and am sympathetic to the desire to reduce the typing\n> > load (hence, the original `--resend` proposal), but I have difficulty\n> > believing that `git format-patch` is so commonly used throughout the\n> > day that the time saved by typing `--resend` over\n> > `--subject-prefix=\"RESEND PATCH\"` warrants the extra implementation,\n> > documentation, and testing baggage. Likewise, I don't see the value in\n> > `--label=WIP` (or `--rfc=WIP` or whatever) over the existing more\n> > general `--subject-prefix`.\n>\n> An additional reason, IMHO, for having \"--rfc\", \"--rfc=\"\n> or \"--resend\" is to reuse what's already configured through the\n> \"format.subjectPrefix\" configuration option. In the sense of not\n> redefining what's already configured in ~/.gitconfig (in this case,\n> \"PATCH\" or \"PATCH lib\", for example), by specifying an additional\n> command-line option.\n>\n> If some user configures different values for \"format.subjectPrefix\"\n> in different local repositories, such as when working on different\n> subsystems, it becomes rather easy to get lost in all those prefixes,\n> if the user needs to remember and type them entirely while using\n> \"--subject-prefix=\" to add more \"labels\" to a prefix.\n>\n> I hope it makes sense the way I wrote it above.\n\nYes, that makes sense. I wasn't aware of that behavior, as I have\nnever had a need to set that configuration.\n"},{"id":"493182","messageId":"CAPig+cRpxvYAJpHahsWxRP=ekr9wwWoxK9_c0vRehDiuzgP72g@mail.gmail.com","threadId":"61329","inReplyTo":"xmqqedb2nlpf.fsf@gitster.g","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-04-19T03:07:12Z","receivedAt":"2024-04-19T03:07:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 18, 2024 at 10:13 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > I do understand and am sympathetic to the desire to reduce the typing\n> > load (hence, the original `--resend` proposal), but I have difficulty\n> > believing that `git format-patch` is so commonly used throughout the\n> > day that the time saved by typing `--resend` over\n> > `--subject-prefix=\"RESEND PATCH\"` warrants the extra implementation,\n> > documentation, and testing baggage. Likewise, I don't see the value in\n> > `--label=WIP` (or `--rfc=WIP` or whatever) over the existing more\n> > general `--subject-prefix`.\n>\n> I am not interested in adding unbounded number of --wip and the like\n> at all, but the value you seem to be missing of the separate \"--rfc\"\n> is that there are folks who configure something other than \"PATCH\"\n> to \"format.subjectPrefix\". They do not want to keep typing\n> --subject-prefix=\"PATCH net-next\" on the command line, so they use\n> the configuration variable, which is \"set it once and forget\". The\n> stress is on the fact that they can forget about it.\n\nIndeed. I was unaware of that behavior.\n"},{"id":"493223","messageId":"xmqqsezhl3vr.fsf@gitster.g","threadId":"61329","inReplyTo":"CAPig+cRpxvYAJpHahsWxRP=ekr9wwWoxK9_c0vRehDiuzgP72g@mail.gmail.com","subject":"Re: [PATCH 3/4] format-patch: new --resend option for adding \"RESEND\" to patch subjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-19T16:21:44Z","receivedAt":"2024-04-19T16:21:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> ..., but the value you seem to be missing of the separate \"--rfc\"\n>> is that there are folks who configure something other than \"PATCH\"\n>> to \"format.subjectPrefix\". They do not want to keep typing\n>> --subject-prefix=\"PATCH net-next\" on the command line, so they use\n>> the configuration variable, which is \"set it once and forget\". The\n>> stress is on the fact that they can forget about it.\n>\n> Indeed. I was unaware of that behavior.\n\nThe very original --rfc did overwrote --subject-prefix and did not\nhave a good reason to exist. But with the relatively recent update,\nit gained its usefulness.\n\nThanks.\n"}]}