| Message ID | 20240421185915.1031590-3-gitster@pobox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | format-patch --rfc=WIP | expand |
Hello Junio, On 2024-04-21 20:59, Junio C Hamano wrote: > 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: There are even more issues, such as the grammar-related ones. Let me explain, please, as accurately as I can do that as a non-native English speaker who spent many years studying English grammar. Writing "RFC PATCH" puts "RFC" into the role of an adjective, which is fine. The same obviously applies to "WIP PATCH". On the other hand, writing "RESEND PATCH" puts "RESEND" into its only possible role, which is transitive verb, and results in "RESEND PATCH" that serves as a very simple sentence in imperative mood. I'm not sure that, strictly technically speaking, having simple sentences as the prefixes is the desired outcome. Technically, we should use "RE-SENT PATCH" to be on the correct side from the grammar perspective, with "RE-SENT" serving as an adjective. Before you ask, no, we can't use "RESENT" there, because its meaning is completely different. However, nobody uses "RE-SENT PATCH", or at least I haven't seen it used yet. When it comes to "PATCH RESEND", "RESEND" remains in its transitive verb role, but "PATCH", as a noun, becomes a modifier of the verb. Thus, the resulting meaning of "PATCH RESEND" becomes something like "resend an item that's a patch", but not written in form of a simple (or less simple) sentence. Strictly technically, a noun should only modify another noun, but bending English grammar a bit this way is much better than actually having a simple sentence as a prefix. With all this in mind, I think that allowing the "--rfc=-<string>" form is the way to go. Computer lingo often bends English grammar to a certain degree, to achieve compactness, but bending and dumbing it down more that it's actually necessary isn't something that we should embrace. I hope all this makes sense. > (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.
Hi Dragan On 21/04/2024 20:37, Dragan Simic wrote: > Hello Junio, > > On 2024-04-21 20:59, Junio C Hamano wrote: >> 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: > > There are even more issues, such as the grammar-related ones. I think it is best to view the subject prefix as a list of space separated labels or keywords rather than part of a grammatically correct sentence. Best Wishes Phillip Let > me explain, please, as accurately as I can do that as a non-native > English speaker who spent many years studying English grammar. > > Writing "RFC PATCH" puts "RFC" into the role of an adjective, which > is fine. The same obviously applies to "WIP PATCH". On the other > hand, writing "RESEND PATCH" puts "RESEND" into its only possible > role, which is transitive verb, and results in "RESEND PATCH" that > serves as a very simple sentence in imperative mood. I'm not sure > that, strictly technically speaking, having simple sentences as the > prefixes is the desired outcome. > > Technically, we should use "RE-SENT PATCH" to be on the correct side > from the grammar perspective, with "RE-SENT" serving as an adjective. > Before you ask, no, we can't use "RESENT" there, because its meaning > is completely different. However, nobody uses "RE-SENT PATCH", or > at least I haven't seen it used yet. > > When it comes to "PATCH RESEND", "RESEND" remains in its transitive > verb role, but "PATCH", as a noun, becomes a modifier of the verb. > Thus, the resulting meaning of "PATCH RESEND" becomes something like > "resend an item that's a patch", but not written in form of a simple > (or less simple) sentence. Strictly technically, a noun should only > modify another noun, but bending English grammar a bit this way is > much better than actually having a simple sentence as a prefix. > > With all this in mind, I think that allowing the "--rfc=-<string>" > form is the way to go. Computer lingo often bends English grammar > to a certain degree, to achieve compactness, but bending and dumbing > it down more that it's actually necessary isn't something that we > should embrace. > > I hope all this makes sense. > >> (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.
Hello Phillip, On 2024-04-24 12:17, Phillip Wood wrote: > On 21/04/2024 20:37, Dragan Simic wrote: >> On 2024-04-21 20:59, Junio C Hamano wrote: >>> 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: >> >> There are even more issues, such as the grammar-related ones. > > I think it is best to view the subject prefix as a list of space > separated labels or keywords rather than part of a grammatically > correct sentence. With all due respect, I strongly disagree. Viewing it that way and letting "[RESEND PATCH]" be accepted as correct (and even enforced a bit) is exactly what I below referred to as embracing the bending of English grammar beyond what's actually necessary. Please, let me remind you that I spent more than a couple of years on English Wikipedia, writing new and expanding already existing computing-related articles, during which time I participated in more than a few grammar-related discussions. All that makes me more "sensitive" to breaking the English grammar rules when that actually isn't necessary or beneficial. > Let >> me explain, please, as accurately as I can do that as a non-native >> English speaker who spent many years studying English grammar. >> >> Writing "RFC PATCH" puts "RFC" into the role of an adjective, which >> is fine. The same obviously applies to "WIP PATCH". On the other >> hand, writing "RESEND PATCH" puts "RESEND" into its only possible >> role, which is transitive verb, and results in "RESEND PATCH" that >> serves as a very simple sentence in imperative mood. I'm not sure >> that, strictly technically speaking, having simple sentences as the >> prefixes is the desired outcome. >> >> Technically, we should use "RE-SENT PATCH" to be on the correct side >> from the grammar perspective, with "RE-SENT" serving as an adjective. >> Before you ask, no, we can't use "RESENT" there, because its meaning >> is completely different. However, nobody uses "RE-SENT PATCH", or >> at least I haven't seen it used yet. >> >> When it comes to "PATCH RESEND", "RESEND" remains in its transitive >> verb role, but "PATCH", as a noun, becomes a modifier of the verb. >> Thus, the resulting meaning of "PATCH RESEND" becomes something like >> "resend an item that's a patch", but not written in form of a simple >> (or less simple) sentence. Strictly technically, a noun should only >> modify another noun, but bending English grammar a bit this way is >> much better than actually having a simple sentence as a prefix. >> >> With all this in mind, I think that allowing the "--rfc=-<string>" >> form is the way to go. Computer lingo often bends English grammar >> to a certain degree, to achieve compactness, but bending and dumbing >> it down more that it's actually necessary isn't something that we >> should embrace. >> >> I hope all this makes sense. >> >>> (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.
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
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(-)