[PATCH v3 2/2] format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)]

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 21, 2024, 18:59 UTC
Message-ID: <20240421185915.1031590-3-gitster@pobox.com>
In-Reply-To: <20240421185915.1031590-1-gitster@pobox.com>

In the previous step, the "--rfc" option of "format-patch" learned to take an optional string value to prepend to the subject prefix, so that --rfc=WIP can give "[WIP PATCH]". This commit shows that the mechanism can be extended easily to allow "--rfc=-(WIP)" [*1*] to signal that the extra string is to be appended instead of getting prepended, resulting in "[PATCH (WIP)]".

Having worked on the patch, I am personally not 100% on board with this part of the feature myself, and that is why this update is a separate step from the main "--rfc takes an optional string value" step.

If a way to prepend an arbitrary adornment is added to the system, and people can now say "--rfc=RESEND" to produce "[RESEND PATCH]", those who used to use "[PATCH RESEND]" by tweaking the subject prefix in other ways [*2*] would do one of three things:

 (1) keep using their existing ways and keep sending their message
     with the "[RESEND PATCH]" prefix.

 (2) rejoice in the new automation, switch to use "--rfc=RESEND",
     and start sending their messages with "[RESEND PATCH]" prefix
     instead of "[PATCH RESEND]" prefix.

 (3) complain and demand a way to append instead of prepend so that
     they can use an automation to produce "[PATCH RESEND]" prefix.

I do not believe in adding slightly different ways that allow users to be "original" when such differences do not make the world better in meaningful ways [*3*], and I expect there are many more folks who share that sentiment and go to route (2) than those who go to route (3). If my expectation is true, it means that this patch goes in a wrong direction, and I would be happy to drop it.

[Footnote]

 *1* The syntax takes inspiration from Perl's three-arg open syntax
     that uses pipes "open fh, '|-', 'cmd'", where the dash signals
     "the other stuff comes here".

 *2* ... because "--rfc" in released versions does not even take any
     string value to prepend, let alone append, to begin with.

 *3* https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/
     gathered some stats to observe that "[RFC PATCH]" is more
     common than "[PATCH RFC]" by a wide margin, while trying to see
     how common "[RESEND PATCH]" (or "[PATCH RESED]") were used (the
     answer: much less common).  But it wouldn't have needed to
     count "[PATCH RFC]" and "[RFC PATCH]" separately if using a
     prefix and not a suffix (or vice versa) were established more
     firmly as the standard practice.

     It is a fine example that useless diversity making needless
     work.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-format-patch.txt | 4 ++++
 builtin/log.c                      | 8 ++++++--
 t/t4014-format-patch.sh            | 9 +++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index e553810b1e..dbccb210cd 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -247,6 +247,10 @@ RFC means "Request For Comments"; use this when sending
 an experimental patch for discussion rather than application.
 "--rfc=WIP" may also be a useful way to indicate that a patch
 is not complete yet ("WIP" stands for "Work In Progress").
++
+If the string _<rfc>_ begins with a dash ("`-`"), the rest of the
+_<rfc>_ string is appended to the subject prefix instead, e.g.,
+`--rfc='-(WIP)'` results in "PATCH (WIP)".
 
 -v <n>::
 --reroll-count=<n>::
diff --git a/builtin/log.c b/builtin/log.c
index 5c1c6f9b15..dd11953260 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2063,8 +2063,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (cover_from_description_arg)
 		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
 
-	if (rfc)
-		strbuf_insertf(&sprefix, 0, "%s ", rfc);
+	if (rfc) {
+		if (rfc[0] == '-')
+			strbuf_addf(&sprefix, " %s", rfc + 1);
+		else
+			strbuf_insertf(&sprefix, 0, "%s ", rfc);
+	}
 
 	if (reroll_count) {
 		strbuf_addf(&sprefix, " v%s", reroll_count);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 645c4189f9..fcbde15b16 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1394,6 +1394,15 @@ test_expect_success '--rfc=WIP and --rfc=' '
 	test_cmp expect-raw actual
 '
 
+test_expect_success '--rfc=-(WIP) appends' '
+	cat >expect <<-\EOF &&
+	Subject: [PATCH (WIP) 1/1] header with . in it
+	EOF
+	git format-patch -n -1 --stdout --rfc="-(WIP)" >patch &&
+	grep "^Subject:" patch >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '--rfc does not overwrite prefix' '
 	cat >expect <<-\EOF &&
 	Subject: [RFC PATCH foobar 1/1] header with . in it

-- 
2.45.0-rc0