{"thread":{"id":"59666","subject":"[PATCH v2] sequencer: beautify subject of reverts of reverts","startedAt":"2023-04-28T08:35:38Z","lastAt":"2023-09-11T21:38:42Z","messageCount":53,"participants":["Oswald Buddenhagen","Junio C Hamano","Phillip Wood","Linus Arver","rsbecker@nexbridge.com","Eric Sunshine","Taylor Blau","Kristoffer Haugsbakk"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"476256","messageId":"20230428083528.1699221-1-oswald.buddenhagen@gmx.de","threadId":"59666","inReplyTo":null,"subject":"[PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-04-28T08:35:28Z","receivedAt":"2023-04-28T08:35:38Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\na more humane `Reapply \"foo\"`.\n\nThe alternative `Revert^2 \"foo\"`, etc. was considered, but it was deemed\nover-engineered and \"too nerdy\". Instead, people should get creative\nwith the subjects when they recurse reverts that deeply. The proposed\nchange encourages that by example and explicit recommendation.\n\nSigned-off-by: Oswald Buddenhagen \nCc: Junio C Hamano \nCc: Kristoffer Haugsbakk \n---\nv2:\n- add discussion to commit message\n- add paragraph to docu\n- add test\n- use skip_prefix() instead of starts_with()\n- catch pre-existing double reverts\n---\n Documentation/git-revert.txt | 6 ++++++\n sequencer.c | 14 ++++++++++++++\n t/t3515-revert-subjects.sh | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 52 insertions(+)\n create mode 100755 t/t3515-revert-subjects.sh\n\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex d2e10d3dce..e8fa513607 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -31,6 +31,12 @@ both will discard uncommitted changes in your working directory.\n See \"Reset, restore and revert\" in linkgit:git[1] for the differences\n between the three commands.\n \n+The command generates the subject 'Revert \"\"' for the resulting\n+commit, assuming the original commit's subject is '<title>'. Reverting\n+such a reversion commit in turn yields the subject 'Reapply \"<title>\"'.\n+These can of course be modified in the editor when the reason for\n+reverting is described.\n+\n OPTIONS\n -------\n <commit>...::\ndiff --git a/sequencer.c b/sequencer.c\nindex 3be23d7ca2..61e466470e 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2227,13 +2227,27 @@ static int do_pick_commit(struct repository *r,\n \t */\n \n \tif (command == TODO_REVERT) {\n+\t\tconst char *orig_subject;\n+\n \t\tbase = commit;\n \t\tbase_label = msg.label;\n \t\tnext = parent;\n \t\tnext_label = msg.parent_label;\n \t\tif (opts->commit_use_reference) {\n \t\t\tstrbuf_addstr(&msgbuf,\n \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n+\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject)) {\n+\t\t\tif (skip_prefix(orig_subject, \"Revert \\\"\", &orig_subject)) {\n+\t\t\t\t/*\n+\t\t\t\t * This prevents the generation of somewhat unintuitive (even if\n+\t\t\t\t * not incorrect) 'Reapply \"Revert \"' titles from legacy double\n+\t\t\t\t * reverts. Fixing up deeper recursions is left to the user.\n+\t\t\t\t */\n+\t\t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"Reapply \\\"\");\n+\t\t\t} else {\n+\t\t\t\tstrbuf_addstr(&msgbuf, \"Reapply \\\"\");\n+\t\t\t}\n+\t\t\tstrbuf_addstr(&msgbuf, orig_subject);\n \t\t} else {\n \t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n \t\t\tstrbuf_addstr(&msgbuf, msg.subject);\ndiff --git a/t/t3515-revert-subjects.sh b/t/t3515-revert-subjects.sh\nnew file mode 100755\nindex 0000000000..ea4319fd15\n--- /dev/null\n+++ b/t/t3515-revert-subjects.sh\n@@ -0,0 +1,32 @@\n+#!/bin/sh\n+\n+test_description='git revert produces the expected subject'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'fresh reverts' '\n+ test_commit --no-tag A file1 &&\n+ test_commit --no-tag B file1 &&\n+ git revert --no-edit HEAD &&\n+ echo \"Revert \\\"B\\\"\" > expect &&\n+ git log -1 --pretty=%s > actual &&\n+ test_cmp expect actual &&\n+ git revert --no-edit HEAD &&\n+ echo \"Reapply \\\"B\\\"\" > expect &&\n+ git log -1 --pretty=%s > actual &&\n+ test_cmp expect actual &&\n+ git revert --no-edit HEAD &&\n+ echo \"Revert \\\"Reapply \\\"B\\\"\\\"\" > expect &&\n+ git log -1 --pretty=%s > actual &&\n+ test_cmp expect actual\n+'\n+\n+test_expect_success 'legacy double revert' '\n+ test_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n+ git revert --no-edit HEAD &&\n+ echo \"Revert \\\"Reapply \\\"B\\\"\\\"\" > expect &&\n+ git log -1 --pretty=%s > actual &&\n+ test_cmp expect actual\n+'\n+\n+test_done\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"476282","messageId":"xmqqcz3netxr.fsf@gitster.g","threadId":"59666","inReplyTo":"20230428083528.1699221-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-04-28T18:35:28Z","receivedAt":"2023-04-28T18:36:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\n> a more humane `Reapply \"foo\"`.\n>\n> The alternative `Revert^2 \"foo\"`, etc. was considered, but it was deemed\n> over-engineered and \"too nerdy\". Instead, people should get creative\n> with the subjects when they recurse reverts that deeply. The proposed\n> change encourages that by example and explicit recommendation.\n>\n> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n> ---\n\n> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> index d2e10d3dce..e8fa513607 100644\n> --- a/Documentation/git-revert.txt\n> +++ b/Documentation/git-revert.txt\n> @@ -31,6 +31,12 @@ both will discard uncommitted changes in your working directory.\n> See \"Reset, restore and revert\" in linkgit:git[1] for the differences\n> between the three commands.\n> \n> +The command generates the subject 'Revert \"<title>\"' for the resulting\n> +commit, assuming the original commit's subject is '<title>'. Reverting\n> +such a reversion commit in turn yields the subject 'Reapply \"<title>\"'.\n\nClearly written.\n\n> +These can of course be modified in the editor when the reason for\n> +reverting is described.\n\nNot just the title but the entire message can be edited and that is\nby design. Having to modify what this new mechanism does when\nexisting users do not like the new behaviour will annoy them, and\nthis sentence will not be a good enough excuse to ask them\nforgiveness for breaking their established practice, either.\n\nSo, I am not sure if there is a point to have this sentence here.\n\n> diff --git a/sequencer.c b/sequencer.c\n> index 3be23d7ca2..61e466470e 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2227,13 +2227,27 @@ static int do_pick_commit(struct repository *r,\n> \t */\n> \n> \tif (command == TODO_REVERT) {\n> +\t\tconst char *orig_subject;\n> +\n> \t\tbase = commit;\n> \t\tbase_label = msg.label;\n> \t\tnext = parent;\n> \t\tnext_label = msg.parent_label;\n> \t\tif (opts->commit_use_reference) {\n> \t\t\tstrbuf_addstr(&msgbuf,\n> \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n> +\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject)) {\n> +\t\t\tif (skip_prefix(orig_subject, \"Revert \\\"\", &orig_subject)) {\n> +\t\t\t\t/*\n> +\t\t\t\t * This prevents the generation of somewhat unintuitive (even if\n> +\t\t\t\t * not incorrect) 'Reapply \"Revert \"' titles from legacy double\n> +\t\t\t\t * reverts. Fixing up deeper recursions is left to the user.\n> +\t\t\t\t */\n\nGood comment but in an overwide paragraph.\n\n> +\t\t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"Reapply \\\"\");\n> +\t\t\t} else {\n> +\t\t\t\tstrbuf_addstr(&msgbuf, \"Reapply \\\"\");\n> +\t\t\t}\n> +\t\t\tstrbuf_addstr(&msgbuf, orig_subject);\n> \t\t} else {\n> \t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n> \t\t\tstrbuf_addstr(&msgbuf, msg.subject);\n\n\n> diff --git a/t/t3515-revert-subjects.sh b/t/t3515-revert-subjects.sh\n> new file mode 100755\n> index 0000000000..ea4319fd15\n> --- /dev/null\n> +++ b/t/t3515-revert-subjects.sh\n\nIt is a bit unexpectd that we need an entire new file to test this.\nIt is doubly bad that the title of the file is only about the\nsubject of revert commits and does not allow other things to be\nadded later. Are we planning to have a lot more creativity in how\nautomatically generated subject of revert commits would read?\n\nIf there isn't a good enough test coverage for the \"git revert\"\ncommand already, then having a new file to test \"git revert\" would\nbe an excellent idea, adding one here is a very welcome addition,\nand it is perfectly fine to start such a new test with only these\nnew tests that protects the new \"Revert Revert to Reapply\" feature.\n\nBut if there is a test file already for \"git revert\" that covers\nother behaviour of the command, \"create two new commits, i.e. revert\nand revert of revert, and then try reverting them and see what their\nsubject says\" ought to be a simple addition or two to such an\nexisting test file. Doesn't t3501 seem a better home for them? The\nlast handful of tests there are about how the auto-generated log is\nphrased, and would form a good group with this new feature, wouldn't\nit?\n\n> @@ -0,0 +1,32 @@\n> +#!/bin/sh\n> +\n> +test_description='git revert produces the expected subject'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'fresh reverts' '\n> + test_commit --no-tag A file1 &&\n> + test_commit --no-tag B file1 &&\n> + git revert --no-edit HEAD &&\n> + echo \"Revert \\\"B\\\"\" > expect &&\n\nStyle. See Documentation/CodingGuidelines and look for \"For shell\nscripts specifically\".\n\n> + git log -1 --pretty=%s > actual &&\n> + test_cmp expect actual &&\n> + git revert --no-edit HEAD &&\n> + echo \"Reapply \\\"B\\\"\" > expect &&\n> + git log -1 --pretty=%s > actual &&\n> + test_cmp expect actual &&\n> + git revert --no-edit HEAD &&\n> + echo \"Revert \\\"Reapply \\\"B\\\"\\\"\" > expect &&\n> + git log -1 --pretty=%s > actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'legacy double revert' '\n> + test_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n> + git revert --no-edit HEAD &&\n> + echo \"Revert \\\"Reapply \\\"B\\\"\\\"\" > expect &&\n> + git log -1 --pretty=%s > actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_done\n\nThanks.\n"},{"id":"476290","messageId":"ZEwafQmat347la3/@ugly","threadId":"59666","inReplyTo":"xmqqcz3netxr.fsf@gitster.g","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-04-28T19:11:57Z","receivedAt":"2023-04-28T19:12:04Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Apr 28, 2023 at 11:35:28AM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>> +The command generates the subject 'Revert \"<title>\"' for the resulting\n>> +commit, assuming the original commit's subject is '<title>'. Reverting\n>> +such a reversion commit in turn yields the subject 'Reapply \"<title>\"'.\n>\n>Clearly written.\n>\n>> +These can of course be modified in the editor when the reason for\n>> +reverting is described.\n>\n>Not just the title but the entire message can be edited and that is\n>by design. Having to modify what this new mechanism does when\n>existing users do not like the new behaviour will annoy them, and\n>this sentence will not be a good enough excuse to ask them\n>forgiveness for breaking their established practice, either.\n>\n>So, I am not sure if there is a point to have this sentence here.\n>\nwell, it's the one sentence i copied verbatim from your proposal. :-D\n\nbut i don't get the argument anyway. i think the docu is pretty \npointless except to emphasize that the generated subject is a default \nthat should be edited when circumstances recommend it. in fact, i \nwouldn't mind writing just that, with a notice that the default attempts \nto be somewhat natural for repeated reverts.\n\n>> \t\t\tstrbuf_addstr(&msgbuf,\n>> \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n>> +\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject)) {\n>> +\t\t\tif (skip_prefix(orig_subject, \"Revert \\\"\", &orig_subject)) {\n>> +\t\t\t\t/*\n>> +\t\t\t\t * This prevents the generation of somewhat unintuitive (even if\n>> +\t\t\t\t * not incorrect) 'Reapply \"Revert \"' titles from legacy double\n>> +\t\t\t\t * reverts. Fixing up deeper recursions is left to the user.\n>> +\t\t\t\t */\n>\n>Good comment but in an overwide paragraph.\n>\nthere are several lines in the lower 90-ies in that file, one of them \nseen in the patch context. would 88 be fine?\n(too narrow flowed text looks silly, imo.)\n\n>Doesn't t3501 seem a better home for them?\n>\nlooking closer at it, i guess it kind of does. the file's contents have \nclearly grown to fulfill the filename's broad promise, but nobody \nbothered to adjust the test description and make the setup title more \nspecific. any takers?\n\n-- ossi\n"},{"id":"476351","messageId":"xmqq4jow6lyh.fsf@gitster.g","threadId":"59666","inReplyTo":"ZEwafQmat347la3/@ugly","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-01T16:44:06Z","receivedAt":"2023-05-01T16:44:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> but i don't get the argument anyway. i think the docu is pretty\n> pointless except to emphasize that the generated subject is a default\n> that should be edited when circumstances recommend it. in fact, i\n> wouldn't mind writing just that, with a notice that the default\n> attempts to be somewhat natural for repeated reverts.\n\nI think it is very well known that the user gets the default message\nto be edited in the editor. I would understand if the instruction\nis \"do not edit this line, because ...\", but otherwise it is pretty\nup to the user to do whatever they like to the log message, no?\n\n"},{"id":"476372","messageId":"ZFAOpRpoPWjn8s1B@ugly","threadId":"59666","inReplyTo":"xmqq4jow6lyh.fsf@gitster.g","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-05-01T19:10:29Z","receivedAt":"2023-05-01T19:10:37Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Mon, May 01, 2023 at 09:44:06AM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> but i don't get the argument anyway. i think the docu is pretty\n>> pointless except to emphasize that the generated subject is a default\n>> that should be edited when circumstances recommend it. in fact, i\n>> wouldn't mind writing just that, with a notice that the default\n>> attempts to be somewhat natural for repeated reverts.\n>\n>I think it is very well known that the user gets the default message\n>to be edited in the editor. I would understand if the instruction\n>is \"do not edit this line, because ...\", but otherwise it is pretty\n>up to the user to do whatever they like to the log message, no?\n>\nwell, yeah. but not everybody seems to get that editing the title \nspecifically is actually an acceptable thing to do - see my earlier \nmessages in the other sub-thread. so i'd go with something like:\n\n Git attempts to make the subject of reverts of reverts somewhat \n natural, but will inevitably fail at greater depths. Editing these \n subjects is recommended.\n\nalso, this should be probably a note near the bottom, rather than being \nso close to the top. in fact, it's probably a good idea to add a \nDISCUSSION section with some basic guidelines, like git-commit has.\n\n-- ossi\n\n"},{"id":"476373","messageId":"xmqqmt2n50jb.fsf@gitster.g","threadId":"59666","inReplyTo":"ZFAOpRpoPWjn8s1B@ugly","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-01T19:12:08Z","receivedAt":"2023-05-01T19:12:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> being so close to the top. in fact, it's probably a good idea to add a\n> DISCUSSION section with some basic guidelines, like git-commit has.\n\n;-) Sounds quite sensible.\n\nThanks.\n"},{"id":"476629","messageId":"xmqqa5yielmy.fsf@gitster.g","threadId":"59666","inReplyTo":"ZEwafQmat347la3/@ugly","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-05T17:25:09Z","receivedAt":"2023-05-05T17:25:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n>>Doesn't t3501 seem a better home for them?\n>>\n> looking closer at it, i guess it kind of does. the file's contents\n> have clearly grown to fulfill the filename's broad promise, but nobody\n> bothered to adjust the test description and make the setup title more\n> specific. any takers?\n\nJust dropping \"with renames\" from the test description would be\nfine, no? Existing tests in the early part of the script cover\nnot just renames but unknown command line option, operating on a\ndirty working tree, etc. that are not specific to any renames.\n\nOne more thing I forgot was that your test scripts were indented by\n4 spaces; please use tabs for indent to match existing ones when you\nadd tests to an existing script.\n\nThanks.\n"},{"id":"477431","messageId":"3f5e4116-54e6-9753-f925-ed4a9f6e3518@gmail.com","threadId":"59666","inReplyTo":"20230428083528.1699221-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-05-17T09:05:51Z","receivedAt":"2023-05-17T09:06:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Oswald\n\nOn 28/04/2023 09:35, Oswald Buddenhagen wrote:\n> Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\n> a more humane `Reapply \"foo\"`.\n> \n> The alternative `Revert^2 \"foo\"`, etc. was considered, but it was deemed\n> over-engineered and \"too nerdy\". Instead, people should get creative\n> with the subjects when they recurse reverts that deeply. The proposed\n> change encourages that by example and explicit recommendation.\n> \n> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n> Cc: Junio C Hamano <gitster@pobox.com>\n> Cc: Kristoffer Haugsbakk <code@khaugsbakk.name>\n> ---\n> v2:\n> - add discussion to commit message\n> - add paragraph to docu\n> - add test\n> - use skip_prefix() instead of starts_with()\n> - catch pre-existing double reverts\n> ---\n> Documentation/git-revert.txt | 6 ++++++\n> sequencer.c | 14 ++++++++++++++\n> t/t3515-revert-subjects.sh | 32 ++++++++++++++++++++++++++++++++\n> 3 files changed, 52 insertions(+)\n> create mode 100755 t/t3515-revert-subjects.sh\n> \n> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> index d2e10d3dce..e8fa513607 100644\n> --- a/Documentation/git-revert.txt\n> +++ b/Documentation/git-revert.txt\n> @@ -31,6 +31,12 @@ both will discard uncommitted changes in your working directory.\n> See \"Reset, restore and revert\" in linkgit:git[1] for the differences\n> between the three commands.\n> \n> +The command generates the subject 'Revert \"<title>\"' for the resulting\n> +commit, assuming the original commit's subject is '<title>'. Reverting\n> +such a reversion commit in turn yields the subject 'Reapply \"<title>\"'.\n> +These can of course be modified in the editor when the reason for\n> +reverting is described.\n> +\n> OPTIONS\n> -------\n> <commit>...::\n> diff --git a/sequencer.c b/sequencer.c\n> index 3be23d7ca2..61e466470e 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2227,13 +2227,27 @@ static int do_pick_commit(struct repository *r,\n> \t */\n> \n> \tif (command == TODO_REVERT) {\n> +\t\tconst char *orig_subject;\n> +\n> \t\tbase = commit;\n> \t\tbase_label = msg.label;\n> \t\tnext = parent;\n> \t\tnext_label = msg.parent_label;\n> \t\tif (opts->commit_use_reference) {\n> \t\t\tstrbuf_addstr(&msgbuf,\n> \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n> +\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject)) {\n> +\t\t\tif (skip_prefix(orig_subject, \"Revert \\\"\", &orig_subject)) {\n\nI think it is probably worth adding\n\n\tif (starts_with(orig_subject, \"Revert \\\"\"))\n\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n\telse\n\nhere to make sure that we don't end up with a subject starting \"Revert \n\\\"Reapply \\\"Revert ...\".\n\nBest Wishes\n\nPhillip\n\n> +\t\t\t\t/*\n> +\t\t\t\t * This prevents the generation of somewhat unintuitive (even if\n> +\t\t\t\t * not incorrect) 'Reapply \"Revert \"' titles from legacy double\n> +\t\t\t\t * reverts. Fixing up deeper recursions is left to the user.\n> +\t\t\t\t */\n> +\t\t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"Reapply \\\"\");\n> +\t\t\t} else {\n> +\t\t\t\tstrbuf_addstr(&msgbuf, \"Reapply \\\"\");\n> +\t\t\t}\n> +\t\t\tstrbuf_addstr(&msgbuf, orig_subject);\n> \t\t} else {\n> \t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n> \t\t\tstrbuf_addstr(&msgbuf, msg.subject);\n> diff --git a/t/t3515-revert-subjects.sh b/t/t3515-revert-subjects.sh\n> new file mode 100755\n> index 0000000000..ea4319fd15\n> --- /dev/null\n> +++ b/t/t3515-revert-subjects.sh\n> @@ -0,0 +1,32 @@\n> +#!/bin/sh\n> +\n> +test_description='git revert produces the expected subject'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'fresh reverts' '\n> + test_commit --no-tag A file1 &&\n> + test_commit --no-tag B file1 &&\n> + git revert --no-edit HEAD &&\n> + echo \"Revert \\\"B\\\"\" > expect &&\n> + git log -1 --pretty=%s > actual &&\n> + test_cmp expect actual &&\n> + git revert --no-edit HEAD &&\n> + echo \"Reapply \\\"B\\\"\" > expect &&\n> + git log -1 --pretty=%s > actual &&\n> + test_cmp expect actual &&\n> + git revert --no-edit HEAD &&\n> + echo \"Revert \\\"Reapply \\\"B\\\"\\\"\" > expect &&\n> + git log -1 --pretty=%s > actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'legacy double revert' '\n> + test_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n> + git revert --no-edit HEAD &&\n> + echo \"Revert \\\"Reapply \\\"B\\\"\\\"\" > expect &&\n> + git log -1 --pretty=%s > actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_done\n"},{"id":"477434","messageId":"ZGSlqAPwaLhgWm6v@ugly","threadId":"59666","inReplyTo":"3f5e4116-54e6-9753-f925-ed4a9f6e3518@gmail.com","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-05-17T10:00:08Z","receivedAt":"2023-05-17T10:00:22Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Wed, May 17, 2023 at 10:05:51AM +0100, Phillip Wood wrote:\n>On 28/04/2023 09:35, Oswald Buddenhagen wrote:\n>> +\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject)) {\n>> +\t\t\tif (skip_prefix(orig_subject, \"Revert \\\"\", &orig_subject)) {\n>\n>I think it is probably worth adding\n>\n>\tif (starts_with(orig_subject, \"Revert \\\"\"))\n>\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n>\telse\n>\n>here to make sure that we don't end up with a subject starting \"Revert \n>\\\"Reapply \\\"Revert ...\".\n>\ni can't follow you.\n\nhow is the concern not covered by the subsequent comment?\n\n>> +\t\t\t\t/*\n>> +\t\t\t\t * This prevents the generation of somewhat unintuitive (even if\n>> +\t\t\t\t * not incorrect) 'Reapply \"Revert \"' titles from legacy double\n>> +\t\t\t\t * reverts. Fixing up deeper recursions is left to the user.\n>> +\t\t\t\t */\n\nregards,\nossi\n"},{"id":"477435","messageId":"2d416834-ef3e-01a2-6be0-9e88bc0de25e@gmail.com","threadId":"59666","inReplyTo":"ZGSlqAPwaLhgWm6v@ugly","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-05-17T11:20:03Z","receivedAt":"2023-05-17T11:20:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/05/2023 11:00, Oswald Buddenhagen wrote:\n> On Wed, May 17, 2023 at 10:05:51AM +0100, Phillip Wood wrote:\n>> On 28/04/2023 09:35, Oswald Buddenhagen wrote:\n>>> +        } else if (skip_prefix(msg.subject, \"Revert \\\"\", \n>>> &orig_subject)) {\n>>> +            if (skip_prefix(orig_subject, \"Revert \\\"\", \n>>> &orig_subject)) {\n>>\n>> I think it is probably worth adding\n>>\n>>     if (starts_with(orig_subject, \"Revert \\\"\"))\n>>         strbuf_addstr(&msgbuf, \"Revert \\\"\");\n>>     else\n>>\n>> here to make sure that we don't end up with a subject starting \"Revert \n>> \\\"Reapply \\\"Revert ...\".\n>>\n> i can't follow you.\n> \n> how is the concern not covered by the subsequent comment?\n\nThat comment says that reverting a commit with a subject line\n\n\tRevert \"Revert some subject\"\n\nwill result in the new commit having a subject\n\n\tRevert \"Reapply some subject\"\n\nI'm saying that reverting a commit with a subject line\n\n\tRevert \"Revert \"Revert some subject\"\"\n\nshould result in the new commit having the subject\n\n\tRevert \"Revert \"Revert \"Revert some subject\"\"\"\n\n(i.e. at that point we stop trying to be clever) rather than\n\n\tRevert \"Reapply \"Revert some subject\"\"\n\nwhich I think is what this patch produces.\n\nBest Wishes\n\nPhillip\n\n>>> +                /*\n>>> +                 * This prevents the generation of somewhat \n>>> unintuitive (even if\n>>> +                 * not incorrect) 'Reapply \"Revert \"' titles from \n>>> legacy double\n>>> +                 * reverts. Fixing up deeper recursions is left to \n>>> the user.\n>>> +                 */\n> \n> regards,\n> ossi\n\n"},{"id":"477462","messageId":"ZGUIqBU0+Vr5LSBF@ugly","threadId":"59666","inReplyTo":"2d416834-ef3e-01a2-6be0-9e88bc0de25e@gmail.com","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-05-17T17:02:32Z","receivedAt":"2023-05-17T17:02:39Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Wed, May 17, 2023 at 12:20:03PM +0100, Phillip Wood wrote:\n>I'm saying that reverting a commit with a subject line\n>\n>\tRevert \"Revert \"Revert some subject\"\"\n>\n>should result in the new commit having the subject\n>\n>\tRevert \"Revert \"Revert \"Revert some subject\"\"\"\n>\n>(i.e. at that point we stop trying to be clever) rather than\n>\n>\tRevert \"Reapply \"Revert some subject\"\"\n>\nright.\nhow about filing that under GIGO and extending the comment?\ni mean, when you actually run into this situation, you should be \nre-thinking your life choices rather than stressing about git producing \na somewhat suboptimal commit message template ...\n\nregards,\nossi\n"},{"id":"477509","messageId":"10523968-0f02-f483-69c4-24e62e839f70@gmail.com","threadId":"59666","inReplyTo":"ZGUIqBU0+Vr5LSBF@ugly","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-05-18T09:58:26Z","receivedAt":"2023-05-18T09:58:34Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/05/2023 18:02, Oswald Buddenhagen wrote:\n> On Wed, May 17, 2023 at 12:20:03PM +0100, Phillip Wood wrote:\n>> I'm saying that reverting a commit with a subject line\n>>\n>>     Revert \"Revert \"Revert some subject\"\"\n>>\n>> should result in the new commit having the subject\n>>\n>>     Revert \"Revert \"Revert \"Revert some subject\"\"\"\n>>\n>> (i.e. at that point we stop trying to be clever) rather than\n>>\n>>     Revert \"Reapply \"Revert some subject\"\"\n>>\n> right.\n> how about filing that under GIGO and extending the comment?\n> i mean, when you actually run into this situation, you should be \n> re-thinking your life choices rather than stressing about git producing \n> a somewhat suboptimal commit message template ...\n\nGiven that it is simple to handle this case and Junio is expecting a \nre-roll of this topic[1] then I think it would be worth just adding \nthose three lines.\n\nBest Wishes\n\nPhillip\n\n[1] https://lore.kernel.org/git/xmqqa5y3ssss.fsf@gitster.g/\n\n> regards,\n> ossi\n\n"},{"id":"477530","messageId":"xmqqmt21txid.fsf@gitster.g","threadId":"59666","inReplyTo":"10523968-0f02-f483-69c4-24e62e839f70@gmail.com","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-18T16:28:10Z","receivedAt":"2023-05-18T16:28:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n>>> (i.e. at that point we stop trying to be clever) rather than\n>>>\n>>>     Revert \"Reapply \"Revert some subject\"\"\n>>>\n>> right.\n>>\n>> how about filing that under GIGO and extending the comment?\n>> i mean, when you actually run into this situation, you should be\n>> re-thinking your life choices rather than stressing about git\n>> producing a somewhat suboptimal commit message template ...\n>\n> Given that it is simple to handle this case and Junio is expecting a\n> re-roll of this topic[1] then I think it would be worth just adding\n> those three lines.\n\nPhillip, your first message with these three lines were a bit dense\nto understand (in other words, to me, it did not immediately \"click\"\nhow these lines contribute to avoiding the revert-reapply-revert\nsequence that is awkward).\n\nBut after reading the exchange bewteen you two [*], your suggestion\nmakes quite a lot of sense to me. It is better for a code to behave\nin a dumb but explainable way, than to attempting and failing to act\ntoo clever for its own worth.\n\nOswald, I do not think GIGO is really an excuse in this case, when\nthe only value of the topic is to make the behaviour less awkward by\ncreating something better than a repeated revert-revert sequence,\nrevert-reapply-revert is worse, as it is markedly harder to guess\nwhat it really means for a reversion of revert-revert-revert than\n\"revert\" repeated four times. If anything, it is the cleverness of\n\"lets call revert of revert a reapply\" code without Phillip's\nsuggestion that creates a garbage output, no?\n\nThanks.\n\n\n[Footnote]\n\n * Oswald, please refrain from using Mail-Followup-To; I wanted to\n deliver the contents of this message specifically to you and\n Phillip, but Phillip's MUA followed your Mail-Followup-To and\n lost your address, which made me look it up and add it myself.\n\n Please refer to these first before saying \"I use it because...\"\n if you are going to respond:\n\n https://lore.kernel.org/git/7v4pndfjym.fsf@assigned-by-dhcp.cox.net/\n https://lore.kernel.org/git/7vei7zjr3y.fsf@alter.siamese.dyndns.org/\n"},{"id":"479959","messageId":"owly7cqkfvyu.fsf@fine.c.googlers.com","threadId":"59666","inReplyTo":"xmqqmt21txid.fsf@gitster.g","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2023-07-28T05:26:01Z","receivedAt":"2023-07-28T05:26:07Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> It is better for a code to behave\n> in a dumb but explainable way, than to attempting and failing to act\n> too clever for its own worth.\n\nI completely agree.\n\n> Oswald, I do not think GIGO is really an excuse in this case, when\n> the only value of the topic is to make the behaviour less awkward by\n> creating something better than a repeated revert-revert sequence,\n> revert-reapply-revert is worse, as it is markedly harder to guess\n> what it really means for a reversion of revert-revert-revert than\n> \"revert\" repeated four times. \n\nHow about introducing a suffix (+ or -) after the word \"Revert\" to\nindicate the application/inclusion (+) or removal (-) of a commit? Example:\n\n - \"foo: bar baz quux\"\n - Revert \"foo: bar baz quux\"\n - Revert(+) Revert(-) \"foo: bar baz quux\"\n - Revert(-) Revert(+) Revert(-) \"foo: bar baz quux\"\n - Revert(+) Revert(-) Revert(+) Revert(-) \"foo: bar baz quux\"\n\nI think the above increases readability. I chose to keep the same style\nas the status quo for the first revert, because the \"(-)\" suffix alone\nwithout a neighboring \"(+)\", as in\n\n Revert(-) \"foo: bar baz quux\"\n\nmight confuse users. This style would also do away with the multiple\nquoting levels that make the current multi-revert subject lines look\nmessy at the end. Example:\n\n Revert \"Revert \"Revert \"Revert some subject\"\"\"\n ^\n This part is starting to\n become noisy.\n\n(Sorry for jumping into this thread so late, but the mention of this\ntopic on the recent \"What's cooking\" message [1] (that this topic would\nbe discarded) got me interested.)\n\n[1] https://lore.kernel.org/git/xmqqpm4d9g54.fsf@gitster.g/T/#mf4edccc7bbc6365a03eaf106121694a27559d275\n"},{"id":"479963","messageId":"ZMOOQTMk2wFwtSfa@ugly","threadId":"59666","inReplyTo":"owly7cqkfvyu.fsf@fine.c.googlers.com","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-07-28T09:45:37Z","receivedAt":"2023-07-28T09:45:46Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Thu, Jul 27, 2023 at 10:26:01PM -0700, Linus Arver wrote:\n>How about introducing a suffix (+ or -) after the word \"Revert\" to\n>indicate the application/inclusion (+) or removal (-) of a commit?\n>\ni think that falls squarely into the \"too nerdy\" category, like the \nRevert^n proposal does.\n\n>(Sorry for jumping into this thread so late, but the mention of this\n>topic on the recent \"What's cooking\" message [1] (that this topic would\n>be discarded) got me interested.)\n>\ni actually have finally updated my patches, but then found a problem in \nmy script i'm using for submitting them, and want to fix that first for \ndogfooding purposes. shouldn't be long.\n\nregards\n"},{"id":"479967","messageId":"xmqqpm4c5ax9.fsf@gitster.g","threadId":"59666","inReplyTo":"ZMOOQTMk2wFwtSfa@ugly","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-28T15:10:42Z","receivedAt":"2023-07-28T15:10:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> On Thu, Jul 27, 2023 at 10:26:01PM -0700, Linus Arver wrote:\n>>How about introducing a suffix (+ or -) after the word \"Revert\" to\n>>indicate the application/inclusion (+) or removal (-) of a commit?\n>>\n> i think that falls squarely into the \"too nerdy\" category, like the\n> Revert^n proposal does.\n\nTrue, but instead of dismissing it (or ^n) as \"too nerdy\", let's\ncompare it with what we are trying to achieve and see why we feel it\nis not desirable. I think we are trying to find a good balance\nbetween aesthetics and usefulness. The former should take lower\nprecedence, as it would be more subjective between the two.\n\nThe usefulness of the message comes from its information content.\nWhat do we want to read out of these messages? I think we want\na title that immediately lets us know three things:\n\n (1) What the original patch was about. \n (2) What the final state is.\n (3) How involved was the road to get to the final state has been.\n\nAs to (1), we are not proposing to lose what comes \"Revert\", so this\ninformation is not lost under any proposal we have seen so far in\nthe discussion.\n\nAs to (2), with the current \"Revert\" -> \"Revert Revert\" -> \"Revert\nRevert Revert\" -> ..., you have to count, which is cumbersome and\ndoes not give you an immediate access to that information. With\n\"Revert^n\", you'd see if n is even or odd to determine, which is\nmuch better than the status quo, but it takes practice to interpret.\nWith \"Revert\" -> \"Reapply\" -> \"Revert Reapply\" -> \"Reapply Reapply\"\n-> ..., the first word would give you the final state immediately.\n\nWe want to know (3), because between a change whose revert was\nreverted and a change that hasn't been involved in any revert, there\nmay be no difference in the end result, the former is likely to be\ntrickier and merits more careful inspection than the latter. With\n\"Revert^n\", we read how large the number n is to find the\ninformation. With the current \"the Revert repeated number of times\"\nor your \"a pair of frontmost Reverts become one Reapply\", the length\nof the Revert/Reapply prefix conveys this information, but this is\nassociated with the cost of pushing the original title further to\nthe right and hard to read/find. Note that, while the number of\ntimes revert-reapply sequence took place is a useful piece of\ninformation, the exact number may not be all that important.\n\nAnd from the above discussion, I wonder if the following would be a\ngood place to stop:\n\n - The first revert is as before: Revert \"original title\"\n - A revert of a revert becomes: Reapply \"original title\"\n - A revert of a reapply becomes: Revert Reapply \"original title\"\n - A revert of \"Revert Reapply\" becomes: Reapply Reapply \"original title\"\n - A revert of \"Reapply Reapply\" becomes: Revert Reapply \"original title\"\n\nIn other words, we accept the fact that wedo not need exact number\nof times reversions were done, and use that to simplify the output\nto make sure we will not spend more than two words in the front of\nthe title. That would help to keep the original title visible,\nwhile still allowing us to distinguish the ones that was reverted up\nto four times (and \"Revert Reapply\" and \"Reapply Reapply\" only tell\nus \"final state is to (discard|accept) the original but it took us\n_many_ times\", without saying exactly how many).\n"},{"id":"479969","messageId":"ZMPgn1QQltyE7koe@ugly","threadId":"59666","inReplyTo":"xmqqpm4c5ax9.fsf@gitster.g","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-07-28T15:37:03Z","receivedAt":"2023-07-28T15:37:11Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Jul 28, 2023 at 08:10:42AM -0700, Junio C Hamano wrote:\n>And from the above discussion, I wonder if the following would be a\n>good place to stop:\n>\n> - The first revert is as before: Revert \"original title\"\n> - A revert of a revert becomes: Reapply \"original title\"\n> - A revert of a reapply becomes: Revert Reapply \"original title\"\n> - A revert of \"Revert Reapply\" becomes: Reapply Reapply \"original title\"\n> - A revert of \"Reapply Reapply\" becomes: Revert Reapply \"original title\"\n>\n>In other words, we accept the fact that we do not need exact number\n>of times reversions were done, and use that to simplify the output\n>to make sure we will not spend more than two words in the front of\n>the title. That would help to keep the original title visible,\n>while still allowing us to distinguish the ones that was reverted up\n>to four times (and \"Revert Reapply\" and \"Reapply Reapply\" only tell\n>us \"final state is to (discard|accept) the original but it took us\n>_many_ times\", without saying exactly how many).\n>\ni would not bother automating it, because it falls into the \"you should \nget creative when that happens\" category (which is codified in the \nmanual by my reworked patches).\n\nalso, the \"no more than two words\" is sort of arbitrary - one can make a \npretty convincing argument for just one word as well.\n\nfinally, just dropping that info would typically result in multiple \n(non-trivial) commits with the same summary, which i don't really like. \nleaving the uglier long variant (and the user hopefully amending it) \navoids it.\n\ni think i'll steal some of the text i didn't quote for the commit \nmessage, though. ^^\n\nregards\n"},{"id":"479971","messageId":"xmqqwmyk3slm.fsf@gitster.g","threadId":"59666","inReplyTo":"ZMPgn1QQltyE7koe@ugly","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-28T16:31:49Z","receivedAt":"2023-07-28T16:31:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> also, the \"no more than two words\" is sort of arbitrary - one can make\n> a pretty convincing argument for just one word as well.\n\nI doubt it. If you squash \"revert revert revert\" into \"revert\", it\nmeans \"revert\" no longer means \"singly reverted\", so you destroy the\ngoal (3) completely. Using two at least lets you differentiate\n\"ended up rejecting after reverted multiple times\" and \"reverted\njust once\".\n\n> finally, just dropping that info would typically result in multiple\n> (non-trivial) commits with the same summary, which i don't really\n> like. leaving the uglier long variant (and the user hopefully\n> amending it) avoids it.\n\nActually, I am fine with your \n\n> ... it falls into the \"you\n> should get creative when that happens\" category (which is codified in\n> the manual by my reworked patches).\n\nand leave this whole discussion behind it.\n\nIf we were doing something, we should make sure what we are doing is\nreasonable, and moving away from evaluation criteria like \"beautify\"\nand \"too nerdy\" and steping back to see what we are trying to\nachieve was an attempt to refocus the discussion. From that point\nof view, allowing arbitrary number of \"Reapply\" repeated, optionally\nprefixed by a single \"Revert\", does not sound like it is much better\ncompared to the current one---is it worth this much time to discuss,\nonly to halve the length of long runs of \"Revert\"?\n"},{"id":"479972","messageId":"ZMPxKVsMvISQpXx4@ugly","threadId":"59666","inReplyTo":"xmqqwmyk3slm.fsf@gitster.g","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-07-28T16:47:37Z","receivedAt":"2023-07-28T16:50:14Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Jul 28, 2023 at 09:31:49AM -0700, Junio C Hamano wrote:\n>From that point\n>of view, allowing arbitrary number of \"Reapply\" repeated, optionally\n>prefixed by a single \"Revert\", does not sound like it is much better\n>compared to the current one---is it worth this much time to discuss,\n>only to halve the length of long runs of \"Revert\"?\n>\nyes, for two reasons:\n- the single \"reapply\" case is actually common; it's usually done after \n a previously missed pre-requisite was applied.\n- the fact that it's \"beautified\" _at all_ sends a signal (see previous \n mails). it doesn't have to be particularly sophisticated for that.\n\nregards\n"},{"id":"479973","messageId":"owly3518ey4x.fsf@fine.c.googlers.com","threadId":"59666","inReplyTo":"xmqqpm4c5ax9.fsf@gitster.g","subject":"Re: [PATCH v2] sequencer: beautify subject of reverts of reverts","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2023-07-28T17:36:46Z","receivedAt":"2023-07-28T17:37:21Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The usefulness of the message comes from its information content.\n> What do we want to read out of these messages? I think we want\n> a title that immediately lets us know three things:\n>\n> (1) What the original patch was about. \n> (2) What the final state is.\n> (3) How involved was the road to get to the final state has been.\n>\n> As to (1), we are not proposing to lose what comes \"Revert\", so this\n> information is not lost under any proposal we have seen so far in\n> the discussion.\n\nAgreed.\n\n> As to (2), with the current \"Revert\" -> \"Revert Revert\" -> \"Revert\n> Revert Revert\" -> ..., you have to count, which is cumbersome and\n> does not give you an immediate access to that information. With\n> \"Revert^n\", you'd see if n is even or odd to determine, which is\n> much better than the status quo\n\nI actually think \"Revert^n\" is much worse than what we have, mainly\nbecause the \"^n\" syntax is already used in other subcommands (e.g.,\nrebase). It may well be that we already have overloaded syntax\nelsewhere, but we should avoid overloading where possible.\n\n> , but it takes practice to interpret.\n> With \"Revert\" -> \"Reapply\" -> \"Revert Reapply\" -> \"Reapply Reapply\"\n> -> ..., the first word would give you the final state immediately.\n\nI agree. But to take the idea further, maybe \"Reapply Reapply\" should be\nshortened to just\n\n Reapply* \"original title\"\n\nand likewise any ultimate _removal_ of the commit (no matter the exact\npairwise count of the word \"Revert\") should be shortened to\n\n Revert* \"original title\"\n\n? The trailing asterisk in each case would indicate that this\nreapplication was not the first reapplication. We could then put\nin the commit message body text something like\n\n--8<---------------cut here--------------------------------->8---\n *NOTE: This is not the first time we've reverted XXX.\n--8<---------------cut here----------------^---------^------>8---\n | |\n | Commit SHA.\n |\n Could be \"reapplied\" if\n \"Reapply*\" is the title\n prefix.\n\nto help stress point (3) that you described in your list above.\n\n> We want to know (3), because between a change whose revert was\n> reverted and a change that hasn't been involved in any revert, there\n> may be no difference in the end result, the former is likely to be\n> trickier and merits more careful inspection than the latter.\n\nAgreed.\n\n> With\n> \"Revert^n\", we read how large the number n is to find the\n> information. With the current \"the Revert repeated number of times\"\n> or your \"a pair of frontmost Reverts become one Reapply\", the length\n> of the Revert/Reapply prefix conveys this information, but this is\n> associated with the cost of pushing the original title further to\n> the right and hard to read/find.\n\nI agree that trying to keep the title short is very important. I didn't\nthink too deeply about this point previously, but I think it is equally\nimportant as the other points you've enumerated above.\n\n> Note that, while the number of\n> times revert-reapply sequence took place is a useful piece of\n> information, the exact number may not be all that important.\n\nAgreed. For this reason I would prefer just using a single asterisk (as\nin my example) after the first occurrence of the revert/reapplication.\n\n> And from the above discussion, I wonder if the following would be a\n> good place to stop:\n>\n> - The first revert is as before: Revert \"original title\"\n> - A revert of a revert becomes: Reapply \"original title\"\n> - A revert of a reapply becomes: Revert Reapply \"original title\"\n> - A revert of \"Revert Reapply\" becomes: Reapply Reapply \"original title\"\n> - A revert of \"Reapply Reapply\" becomes: Revert Reapply \"original title\"\n>\n> In other words, we accept the fact that wedo not need exact number\n> of times reversions were done, and use that to simplify the output\n> to make sure we will not spend more than two words in the front of\n> the title. That would help to keep the original title visible,\n> while still allowing us to distinguish the ones that was reverted up\n> to four times (and \"Revert Reapply\" and \"Reapply Reapply\" only tell\n> us \"final state is to (discard|accept) the original but it took us\n> _many_ times\", without saying exactly how many).\n\nI very much like your idea of just \"stopping the recursion\" early on.\nThe only difference is that I would prefer to drop the second \"Reapply\"\nword in your examples and replace them with an asterisk (this makes them\nthe same as in my example).\n\nI think having this second word \"Reapply\" would make the title\npotentially hurt readability, especially because the first word is\nreally the only one users should be looking at anyway to figure out the\nfinal state. Another reason for dropping it is because a well-written\ncommit message title should have a single verb near the beginning, but\nyour style would break this convention.\n\nIf you don't like the \"asterisk\" idea, another form might be\n\n - A revert of a reapply becomes: Revert reapplication of \"original title\"\n - A revert of the above becomes: Reapply revert of \"original title\"\n - A revert of the above becomes: Revert reapplication of \"original title\"\n - A revert of the above becomes: Reapply revert of \"original title\"\n\nand so forth.\n"},{"id":"480372","messageId":"20230809171531.2564807-2-oswald.buddenhagen@gmx.de","threadId":"59666","inReplyTo":"20230809171531.2564807-1-oswald.buddenhagen@gmx.de","subject":"[PATCH v3 2/2] doc: revert: add discussion","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-09T17:15:31Z","receivedAt":"2023-08-09T17:15:45Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"The section is inspired by git-commit.txt.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\n\nCc: Junio C Hamano <gitster@pobox.com>\n\n---\n\nwhile thinking about what to write, i came up with an idea for another\nimprovement: with (implicit) --edit, the template message would end up\nbeing:\n\n This reverts commit <sha1>,\n because <PUT REASON HERE>.\n---\n Documentation/git-revert.txt | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex d2e10d3dce..2b52dc89a8 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -142,6 +142,16 @@ EXAMPLES\n \tchanges. The revert only modifies the working tree and the\n \tindex.\n \n+DISCUSSION\n+----------\n+\n+While git creates a basic commit message automatically, you really\n+should not leave it at that. In particular, it is _strongly_\n+recommended to explain why the original commit is being reverted.\n+Repeatedly reverting reversions yields increasingly unwieldy\n+commit subjects; latest when you arrive at 'Reapply \"Reapply\n+\"<original subject>\"\"' you should get creative.\n+\n CONFIGURATION\n -------------\n \n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480376","messageId":"20230809171531.2564807-1-oswald.buddenhagen@gmx.de","threadId":"59666","inReplyTo":"20230428083528.1699221-1-oswald.buddenhagen@gmx.de","subject":"[PATCH v3 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-09T17:15:30Z","receivedAt":"2023-08-09T17:15:47Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\na more humane `Reapply \"foo\"`.\n\nThis is done for two reasons:\n- To cover the actually common case of just a double revert.\n- To encourage people to rewrite summaries of recursive reverts by\n setting an example (a subsequent commit will also do this explicitly\n in the documentation).\n\nTo achieve these goals, the mechanism does not need to be particularly\nsophisticated. Therefore, more complicated alternatives which would\n\"compress more efficiently\" have not been implemented.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\nv3:\n- capitulate at first sight of a pre-existing recursive reversion, as\n handling the edge cases is a bottomless pit\n- reworked commit message again\n- moved test into existing file\n- generalized docu change and factored it out\n\nv2:\n- add discussion to commit message\n- add paragraph to docu\n- add test\n- use skip_prefix() instead of starts_with()\n- catch pre-existing double reverts\n\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Kristoffer Haugsbakk <code@khaugsbakk.name>\nCc: Phillip Wood <phillip.wood123@gmail.com>\n---\n sequencer.c | 11 +++++++++++\n t/t3501-revert-cherry-pick.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 36 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex cc9821ece2..12ec158922 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2249,13 +2249,24 @@ static int do_pick_commit(struct repository *r,\n \t */\n \n \tif (command == TODO_REVERT) {\n+\t\tconst char *orig_subject;\n+\n \t\tbase = commit;\n \t\tbase_label = msg.label;\n \t\tnext = parent;\n \t\tnext_label = msg.parent_label;\n \t\tif (opts->commit_use_reference) {\n \t\t\tstrbuf_addstr(&msgbuf,\n \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n+\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject) &&\n+\t\t\t /*\n+\t\t\t * We don't touch pre-existing repeated reverts, because\n+\t\t\t * theoretically these can be nested arbitrarily deeply,\n+\t\t\t * thus requiring excessive complexity to deal with.\n+\t\t\t */\n+\t\t\t !starts_with(orig_subject, \"Revert \\\"\")) {\n+\t\t\tstrbuf_addstr(&msgbuf, \"Reapply \\\"\");\n+\t\t\tstrbuf_addstr(&msgbuf, orig_subject);\n \t\t} else {\n \t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n \t\t\tstrbuf_addstr(&msgbuf, msg.subject);\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex e2ef619323..7011e3a421 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -176,6 +176,31 @@ test_expect_success 'advice from failed revert' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'title of fresh reverts' '\n+\ttest_commit --no-tag A file1 &&\n+\ttest_commit --no-tag B file1 &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Revert \\\"B\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Reapply \\\"B\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Revert \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'title of legacy double revert' '\n+\ttest_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'identification of reverted commit (default)' '\n \ttest_commit to-ident &&\n \ttest_when_finished \"git reset --hard to-ident\" &&\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480506","messageId":"owly8raih8ho.fsf@fine.c.googlers.com","threadId":"59666","inReplyTo":"20230809171531.2564807-2-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2023-08-10T21:50:59Z","receivedAt":"2023-08-10T21:51:11Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> while thinking about what to write, i came up with an idea for another\n> improvement: with (implicit) --edit, the template message would end up\n> being:\n>\n> This reverts commit <sha1>,\n> because <PUT REASON HERE>.\n\nThis sounds great to me.\n\nNit: the \"doc: revert: add discussion\" subject line should probably be more\nlike \"revert doc: suggest adding the 'why' behind reverts\".\n\n> ---\n> Documentation/git-revert.txt | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n>\n> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> index d2e10d3dce..2b52dc89a8 100644\n> --- a/Documentation/git-revert.txt\n> +++ b/Documentation/git-revert.txt\n> @@ -142,6 +142,16 @@ EXAMPLES\n> \tchanges. The revert only modifies the working tree and the\n> \tindex.\n>\n> +DISCUSSION\n> +----------\n> +\n> +While git creates a basic commit message automatically, you really\n> +should not leave it at that. In particular, it is _strongly_\n> +recommended to explain why the original commit is being reverted.\n> +Repeatedly reverting reversions yields increasingly unwieldy\n> +commit subjects; latest when you arrive at 'Reapply \"Reapply\n> +\"<original subject>\"\"' you should get creative.\n\nThe word \"latest\" here sounds odd. Ditto for \"get creative\". How about\nthe following rewording?\n\n While git creates a basic commit message automatically, it is\n _strongly_ recommended to explain why the original commit is being\n reverted. In addition, repeatedly reverting the same commit will\n result in increasingly unwieldy subject lines, for example 'Reapply\n \"Reapply \"<original subject>\"\"'. Please consider rewording such\n subject lines to reflect the reason why the original commit is being\n reapplied again.\n"},{"id":"480507","messageId":"owly5y5mh81i.fsf@fine.c.googlers.com","threadId":"59666","inReplyTo":"owly8raih8ho.fsf@fine.c.googlers.com","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2023-08-10T22:00:41Z","receivedAt":"2023-08-10T22:00:45Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"Linus Arver <linusa@google.com> writes:\n\n> How about\n> the following rewording?\n>\n> While git creates a basic commit message automatically, it is\n> _strongly_ recommended to explain why the original commit is being\n> reverted. In addition, repeatedly reverting the same commit will\n\nHmph, \"repeatedly reverting the same commit\" sounds wrong because\nstrictly speaking there is only 1 \"same commit\" (the original commit).\nPerhaps\n\n In addition, repeatedly reverting the same progression of reverts will\n\nor even\n\n In addition, repeatedly reverting the same revert chain will\n\nis better here?\n"},{"id":"480532","messageId":"ZNYuUh27ByphTH04@ugly","threadId":"59666","inReplyTo":"owly5y5mh81i.fsf@fine.c.googlers.com","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-11T12:49:22Z","receivedAt":"2023-08-11T12:49:26Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Thu, Aug 10, 2023 at 02:50:59PM -0700, Linus Arver wrote:\n>Nit: the \"doc: revert: add discussion\" subject line should probably be more\n>like \"revert doc: suggest adding the 'why' behind reverts\".\n>\nthis is counter to the prevalent \"big endian\" prefix style, and is in \nthis case really easy to misread.\ni also intentionally kept the subject generic, because the content \ncovers two matters (the reasoning and the subjects, which is also the \nreason why this is a separate patch to start with).\n\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>> +DISCUSSION\n>> +----------\n>> +\n>> +While git creates a basic commit message automatically, you really\n>> +should not leave it at that. In particular, it is _strongly_\n>> +recommended to explain why the original commit is being reverted.\n>> +Repeatedly reverting reversions yields increasingly unwieldy\n>> +commit subjects; latest when you arrive at 'Reapply \"Reapply\n>> +\"<original subject>\"\"' you should get creative.\n>\n>The word \"latest\" here sounds odd. Ditto for \"get creative\".\n>\nyeah, i suppose. i wasn't sure how formal i should make it - things \naren't consistent to start with.\n\n> How about the following rewording?\n>\n> While git creates a basic commit message automatically, it is\n> _strongly_ recommended to explain why the original commit is being\n> reverted. In addition, repeatedly reverting the same commit will\n> result in increasingly unwieldy subject lines,\n\n>for example 'Reapply \"Reapply \"<original subject>\"\"'.\n>\nyou turned it from a suggested threshold into an example. at this point \nit appears superfluous to me.\n\n>Please consider rewording such\n> subject lines to reflect the reason why the original commit is being\n> reapplied again.\n>\nthe reasoning most likely wouldn't fit into the subject.\nalso, the original request to explain the reasoning applies \ntransitively, so i don't think it's really necessary to point it out \nexplicitly.\n\nOn Thu, Aug 10, 2023 at 03:00:41PM -0700, Linus Arver wrote:\n>Hmph, \"repeatedly reverting the same commit\" sounds wrong because\n>strictly speaking there is only 1 \"same commit\" (the original commit).\n>Perhaps\n>\n> In addition, repeatedly reverting the same progression of reverts will\n>\n>or even\n>\n> In addition, repeatedly reverting the same revert chain will\n>\n>is better here?\n>\nwe used \"recursive reverts\" elsewhere. but i'm not sure whether that's \nsufficiently intuitive and formally correct.\n\nanyway, what's wrong with my original proposal?\n\nso in summary, how about:\n\n While git creates a basic commit message automatically, it is\n _strongly_ recommended to explain why the original commit is being\n reverted. In addition, repeatedly reverting reversions will\n result in increasingly unwieldy subject lines. Please consider \n rewording these into something shorter and more unique.\n\nregards\n"},{"id":"480541","messageId":"dba3f15a-3575-e4f9-2291-c5a342cfed43@gmail.com","threadId":"59666","inReplyTo":"20230809171531.2564807-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v3 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-08-11T15:05:03Z","receivedAt":"2023-08-11T15:05:11Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Oswald\n\nOn 09/08/2023 18:15, Oswald Buddenhagen wrote:\n> Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\n> a more humane `Reapply \"foo\"`.\n> \n> This is done for two reasons:\n> - To cover the actually common case of just a double revert.\n> - To encourage people to rewrite summaries of recursive reverts by\n> setting an example (a subsequent commit will also do this explicitly\n> in the documentation).\n> \n> To achieve these goals, the mechanism does not need to be particularly\n> sophisticated. Therefore, more complicated alternatives which would\n> \"compress more efficiently\" have not been implemented.\n\nThis all looks good to me, it seems quite sensible just to bail out if \nwe see an existing recursive reversion. I'm not suggesting you change \nthese tests but for future reference we now have a test_commit_message() \nfunction which was merged a few days ago to simplify tests like these.\n\nThanks for working on it\n\nPhillip\n\n> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n> \n> ---\n> v3:\n> - capitulate at first sight of a pre-existing recursive reversion, as\n> handling the edge cases is a bottomless pit\n> - reworked commit message again\n> - moved test into existing file\n> - generalized docu change and factored it out\n> \n> v2:\n> - add discussion to commit message\n> - add paragraph to docu\n> - add test\n> - use skip_prefix() instead of starts_with()\n> - catch pre-existing double reverts\n> \n> Cc: Junio C Hamano <gitster@pobox.com>\n> Cc: Kristoffer Haugsbakk <code@khaugsbakk.name>\n> Cc: Phillip Wood <phillip.wood123@gmail.com>\n> ---\n> sequencer.c | 11 +++++++++++\n> t/t3501-revert-cherry-pick.sh | 25 +++++++++++++++++++++++++\n> 2 files changed, 36 insertions(+)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index cc9821ece2..12ec158922 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2249,13 +2249,24 @@ static int do_pick_commit(struct repository *r,\n> \t */\n> \n> \tif (command == TODO_REVERT) {\n> +\t\tconst char *orig_subject;\n> +\n> \t\tbase = commit;\n> \t\tbase_label = msg.label;\n> \t\tnext = parent;\n> \t\tnext_label = msg.parent_label;\n> \t\tif (opts->commit_use_reference) {\n> \t\t\tstrbuf_addstr(&msgbuf,\n> \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n> +\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject) &&\n> +\t\t\t /*\n> +\t\t\t * We don't touch pre-existing repeated reverts, because\n> +\t\t\t * theoretically these can be nested arbitrarily deeply,\n> +\t\t\t * thus requiring excessive complexity to deal with.\n> +\t\t\t */\n> +\t\t\t !starts_with(orig_subject, \"Revert \\\"\")) {\n> +\t\t\tstrbuf_addstr(&msgbuf, \"Reapply \\\"\");\n> +\t\t\tstrbuf_addstr(&msgbuf, orig_subject);\n> \t\t} else {\n> \t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n> \t\t\tstrbuf_addstr(&msgbuf, msg.subject);\n> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n> index e2ef619323..7011e3a421 100755\n> --- a/t/t3501-revert-cherry-pick.sh\n> +++ b/t/t3501-revert-cherry-pick.sh\n> @@ -176,6 +176,31 @@ test_expect_success 'advice from failed revert' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success 'title of fresh reverts' '\n> +\ttest_commit --no-tag A file1 &&\n> +\ttest_commit --no-tag B file1 &&\n> +\tgit revert --no-edit HEAD &&\n> +\techo \"Revert \\\"B\\\"\" >expect &&\n> +\tgit log -1 --pretty=%s >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit revert --no-edit HEAD &&\n> +\techo \"Reapply \\\"B\\\"\" >expect &&\n> +\tgit log -1 --pretty=%s >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit revert --no-edit HEAD &&\n> +\techo \"Revert \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n> +\tgit log -1 --pretty=%s >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'title of legacy double revert' '\n> +\ttest_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n> +\tgit revert --no-edit HEAD &&\n> +\techo \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\" >expect &&\n> +\tgit log -1 --pretty=%s >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_expect_success 'identification of reverted commit (default)' '\n> \ttest_commit to-ident &&\n> \ttest_when_finished \"git reset --hard to-ident\" &&\n"},{"id":"480542","messageId":"b9f8c965-731d-84eb-f60e-fbed418f9ca2@gmail.com","threadId":"59666","inReplyTo":"owly8raih8ho.fsf@fine.c.googlers.com","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-08-11T15:08:14Z","receivedAt":"2023-08-11T15:08:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 10/08/2023 22:50, Linus Arver wrote:\n> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>> +DISCUSSION\n>> +----------\n>> +\n>> +While git creates a basic commit message automatically, you really\n>> +should not leave it at that. In particular, it is _strongly_\n>> +recommended to explain why the original commit is being reverted.\n>> +Repeatedly reverting reversions yields increasingly unwieldy\n>> +commit subjects; latest when you arrive at 'Reapply \"Reapply\n>> +\"<original subject>\"\"' you should get creative.\n> \n> The word \"latest\" here sounds odd. Ditto for \"get creative\". How about\n> the following rewording?\n> \n> While git creates a basic commit message automatically, it is\n> _strongly_ recommended to explain why the original commit is being\n> reverted. In addition, repeatedly reverting the same commit will\n> result in increasingly unwieldy subject lines, for example 'Reapply\n> \"Reapply \"<original subject>\"\"'. Please consider rewording such\n> subject lines to reflect the reason why the original commit is being\n> reapplied again.\n\nThat's a good suggestion, I think having the example will help readers \nunderstand the issue being described.\n\nBest Wishes\n\nPhillip\n"},{"id":"480543","messageId":"07028529-cbe1-55d0-4ab0-9f3ec03a4fd1@gmail.com","threadId":"59666","inReplyTo":"owly5y5mh81i.fsf@fine.c.googlers.com","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-08-11T15:10:55Z","receivedAt":"2023-08-11T15:11:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 10/08/2023 23:00, Linus Arver wrote:\n> Linus Arver <linusa@google.com> writes:\n> \n>> How about\n>> the following rewording?\n>>\n>> While git creates a basic commit message automatically, it is\n>> _strongly_ recommended to explain why the original commit is being\n>> reverted. In addition, repeatedly reverting the same commit will\n> \n> Hmph, \"repeatedly reverting the same commit\" sounds wrong because\n> strictly speaking there is only 1 \"same commit\" (the original commit).\n\nWhile it isn't strictly accurate I think that wording is easy enough to \nunderstand. I think it is hard to find a more accurate wording that \nisn't too verbose or cumbersome.\n\nBest Wishes\n\nPhillip\n\n\n> Perhaps\n> \n> In addition, repeatedly reverting the same progression of reverts will\n> \n> or even\n> \n> In addition, repeatedly reverting the same revert chain will\n> \n> is better here?\n"},{"id":"480548","messageId":"xmqq1qg9v7k9.fsf@gitster.g","threadId":"59666","inReplyTo":"dba3f15a-3575-e4f9-2291-c5a342cfed43@gmail.com","subject":"Re: [PATCH v3 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-11T16:59:34Z","receivedAt":"2023-08-11T16:59:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Hi Oswald\n>\n> On 09/08/2023 18:15, Oswald Buddenhagen wrote:\n>> Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\n>> a more humane `Reapply \"foo\"`.\n>> This is done for two reasons:\n>> - To cover the actually common case of just a double revert.\n>> - To encourage people to rewrite summaries of recursive reverts by\n>> setting an example (a subsequent commit will also do this explicitly\n>> in the documentation).\n>> To achieve these goals, the mechanism does not need to be\n>> particularly\n>> sophisticated. Therefore, more complicated alternatives which would\n>> \"compress more efficiently\" have not been implemented.\n>\n> This all looks good to me, it seems quite sensible just to bail out if\n> we see an existing recursive reversion.\n\nYes, explicitly refraining from becoming overly cute is a good\ndesign decision.\n\n>> diff --git a/sequencer.c b/sequencer.c\n>> index cc9821ece2..12ec158922 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -2249,13 +2249,24 @@ static int do_pick_commit(struct repository *r,\n>> \t */\n>> \tif (command == TODO_REVERT) {\n>> +\t\tconst char *orig_subject;\n>> +\n>> \t\tbase = commit;\n>> \t\tbase_label = msg.label;\n>> \t\tnext = parent;\n>> \t\tnext_label = msg.parent_label;\n>> \t\tif (opts->commit_use_reference) {\n>> \t\t\tstrbuf_addstr(&msgbuf,\n>> \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n>> +\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject) &&\n>> +\t\t\t /*\n>> +\t\t\t * We don't touch pre-existing repeated reverts, because\n>> +\t\t\t * theoretically these can be nested arbitrarily deeply,\n>> +\t\t\t * thus requiring excessive complexity to deal with.\n>> +\t\t\t */\n>> +\t\t\t !starts_with(orig_subject, \"Revert \\\"\")) {\n>> +\t\t\tstrbuf_addstr(&msgbuf, \"Reapply \\\"\");\n>> +\t\t\tstrbuf_addstr(&msgbuf, orig_subject);\n\nBeing simple-and-stupid to deal only with the most common case, and\ndocumenting that it is deliberate that we do not deal with more\ncomplex cases in the in-code comment and in the log message, are\nvery good in this case.\n\n>> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n>> index e2ef619323..7011e3a421 100755\n>> --- a/t/t3501-revert-cherry-pick.sh\n>> +++ b/t/t3501-revert-cherry-pick.sh\n>> @@ -176,6 +176,31 @@ test_expect_success 'advice from failed revert' '\n>> \ttest_cmp expected actual\n>> '\n>> +test_expect_success 'title of fresh reverts' '\n>> +\ttest_commit --no-tag A file1 &&\n>> +\ttest_commit --no-tag B file1 &&\n>> +\tgit revert --no-edit HEAD &&\n>> +\techo \"Revert \\\"B\\\"\" >expect &&\n>> +\tgit log -1 --pretty=%s >actual &&\n>> +\ttest_cmp expect actual &&\n>> +\tgit revert --no-edit HEAD &&\n>> +\techo \"Reapply \\\"B\\\"\" >expect &&\n>> +\tgit log -1 --pretty=%s >actual &&\n>> +\ttest_cmp expect actual &&\n>> +\tgit revert --no-edit HEAD &&\n>> +\techo \"Revert \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n>> +\tgit log -1 --pretty=%s >actual &&\n>> +\ttest_cmp expect actual\n>> +'\n\nPresumably the next time this gets reverted we will see a doubled\nreapply? Isn't that something we care about documenting as a part\nof this test? i.e. another four-line block after the above?\n\n\tgit revert --no-edit HEAD &&\n\techo \"Reapply \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n\tgit log -1 --pretty=%s >actual &&\n\ttest_cmp expect actual\n\n>> +test_expect_success 'title of legacy double revert' '\n>> +\ttest_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n>> +\tgit revert --no-edit HEAD &&\n>> +\techo \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\" >expect &&\n>> +\tgit log -1 --pretty=%s >actual &&\n>> +\ttest_cmp expect actual\n>> +'\n\nGood.\n\n>> test_expect_success 'identification of reverted commit (default)' '\n>> \ttest_commit to-ident &&\n>> \ttest_when_finished \"git reset --hard to-ident\" &&\n"},{"id":"480549","messageId":"xmqqsf8ptsqf.fsf@gitster.g","threadId":"59666","inReplyTo":"owly8raih8ho.fsf@fine.c.googlers.com","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-11T17:05:12Z","receivedAt":"2023-08-11T17:05:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Arver <linusa@google.com> writes:\n\n> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> while thinking about what to write, i came up with an idea for another\n>> improvement: with (implicit) --edit, the template message would end up\n>> being:\n>>\n>> This reverts commit <sha1>,\n>> because <PUT REASON HERE>.\n>\n> This sounds great to me.\n\nOh, absolutely. I rarely do a revert myself (other than reverting a\npremature merge out of 'next'), but giving a better instruction in\nthe commit log editor buffer as template is a very good idea.\n\n> Nit: the \"doc: revert: add discussion\" subject line should probably be more\n> like \"revert doc: suggest adding the 'why' behind reverts\".\n\nGood suggestion.\n\n> The word \"latest\" here sounds odd. Ditto for \"get creative\". How about\n> the following rewording?\n>\n> While git creates a basic commit message automatically, it is\n> _strongly_ recommended to explain why the original commit is being\n> reverted. In addition, repeatedly reverting the same commit will\n> result in increasingly unwieldy subject lines, for example 'Reapply\n> \"Reapply \"<original subject>\"\"'. Please consider rewording such\n> subject lines to reflect the reason why the original commit is being\n> reapplied again.\n\nSounds better, but let me read the remaining discussion first ;-)\n\nThanks.\n"},{"id":"480557","messageId":"xmqqmsyxtshx.fsf@gitster.g","threadId":"59666","inReplyTo":"b9f8c965-731d-84eb-f60e-fbed418f9ca2@gmail.com","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-11T17:10:18Z","receivedAt":"2023-08-11T17:11:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> On 10/08/2023 22:50, Linus Arver wrote:\n>> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>>> +DISCUSSION\n>>> +----------\n>>> +\n>>> +While git creates a basic commit message automatically, you really\n>>> +should not leave it at that. In particular, it is _strongly_\n>>> +recommended to explain why the original commit is being reverted.\n>>> +Repeatedly reverting reversions yields increasingly unwieldy\n>>> +commit subjects; latest when you arrive at 'Reapply \"Reapply\n>>> +\"<original subject>\"\"' you should get creative.\n>> The word \"latest\" here sounds odd. Ditto for \"get creative\". How\n>> about\n>> the following rewording?\n>> While git creates a basic commit message automatically, it is\n>> _strongly_ recommended to explain why the original commit is being\n>> reverted. In addition, repeatedly reverting the same commit will\n>> result in increasingly unwieldy subject lines, for example 'Reapply\n>> \"Reapply \"<original subject>\"\"'. Please consider rewording such\n>> subject lines to reflect the reason why the original commit is being\n>> reapplied again.\n>\n> That's a good suggestion, I think having the example will help readers\n> understand the issue being described.\n\nSounds very good.\n\n"},{"id":"480558","messageId":"ZNZsIfwj2t2wYWEG@ugly","threadId":"59666","inReplyTo":"xmqq1qg9v7k9.fsf@gitster.g","subject":"Re: [PATCH v3 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-11T17:13:05Z","receivedAt":"2023-08-11T17:13:11Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Aug 11, 2023 at 09:59:34AM -0700, Junio C Hamano wrote:\n>Presumably the next time this gets reverted we will see a doubled\n>reapply?\n>\nyes\n\n>Isn't that something we care about documenting as a part\n>of this test? i.e. another four-line block after the above?\n>\nthe third case documents that it's the same as the first case, i.e., \n\"nothing special\". so at this point we have full coverage in all \nregards. going beyond that would be redundant, and we'd again get into \nthe \"uh, where do we stop?\" situation.\n\nregards\n"},{"id":"480562","messageId":"xmqq5y5ltqwd.fsf_-_@gitster.g","threadId":"59666","inReplyTo":"xmqqsf8ptsqf.fsf@gitster.g","subject":"Re* [PATCH v3 2/2] doc: revert: add discussion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-11T17:44:50Z","receivedAt":"2023-08-11T17:44:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Linus Arver <linusa@google.com> writes:\n>\n>> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>>\n>>> while thinking about what to write, i came up with an idea for another\n>>> improvement: with (implicit) --edit, the template message would end up\n>>> being:\n>>>\n>>> This reverts commit <sha1>,\n>>> because <PUT REASON HERE>.\n>>\n>> This sounds great to me.\n>\n> Oh, absolutely. I rarely do a revert myself (other than reverting a\n> premature merge out of 'next'), but giving a better instruction in\n> the commit log editor buffer as template is a very good idea.\n\nIt might be just the matter of doing something like the attached\npatch on top of Oswald's, reusing the existing code to instruct the\nuser to describe the reversion.\n\n------- >8 ------------- >8 ------------- >8 ------------- >8 -------\nSubject: [PATCH 3/2] revert: force explaining overly complex revert chain\n\nOnce we revert reverts of revert and reach \"Reapply \"Reapply \"...\"\",\nit becomes too unweirdly to read a reversion of such a comit.\n\nWe instruct the user to explain why the reversion is done in their\nown words when using the revert.reference mode, and the instruction\napplies equally for such an overly complex revert chain. The\nrationale for such a sequence of events should be recorded to help\nfuture developers.\n\nBuilding on top of the recent Oswald's work to turn \"revert revert\"\ninto \"reapply\", let's turn the reference mode automatically on in\nsuch a case.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * I left the reference to the second parent to honor the command\n line option and configuration variable even when this new\n mechanism kicks in and this is very much deliberate. As a commit\n that is a revert (or reapply) should be single parent (because a\n revert of a merge is a single parent commit), the choice does not\n make any difference in practice.\n\n sequencer.c | 16 +++++++++++-----\n t/t3501-revert-cherry-pick.sh | 11 ++++++++++-\n 2 files changed, 21 insertions(+), 6 deletions(-)\n\ndiff --git c/sequencer.c w/sequencer.c\nindex 7dc13fdcca..43bb558518 100644\n--- c/sequencer.c\n+++ w/sequencer.c\n@@ -2130,10 +2130,10 @@ static int should_edit(struct replay_opts *opts) {\n \treturn opts->edit;\n }\n \n-static void refer_to_commit(struct replay_opts *opts,\n+static void refer_to_commit(int use_reference,\n \t\t\t struct strbuf *msgbuf, struct commit *commit)\n {\n-\tif (opts->commit_use_reference) {\n+\tif (use_reference) {\n \t\tstruct pretty_print_context ctx = {\n \t\t\t.abbrev = DEFAULT_ABBREV,\n \t\t\t.date_mode.type = DATE_SHORT,\n@@ -2250,12 +2250,18 @@ static int do_pick_commit(struct repository *r,\n \n \tif (command == TODO_REVERT) {\n \t\tconst char *orig_subject;\n+\t\tint use_reference = opts->commit_use_reference;\n \n \t\tbase = commit;\n \t\tbase_label = msg.label;\n \t\tnext = parent;\n \t\tnext_label = msg.parent_label;\n-\t\tif (opts->commit_use_reference) {\n+\n+\t\tif (starts_with(msg.subject, \"Reapply \\\"Reapply \\\"\"))\n+\t\t\t/* fifth time is too many - force reference format*/\n+\t\t\tuse_reference = 1;\n+\n+\t\tif (use_reference) {\n \t\t\tstrbuf_addstr(&msgbuf,\n \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n \t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject) &&\n@@ -2273,11 +2279,11 @@ static int do_pick_commit(struct repository *r,\n \t\t\tstrbuf_addstr(&msgbuf, \"\\\"\");\n \t\t}\n \t\tstrbuf_addstr(&msgbuf, \"\\n\\nThis reverts commit \");\n-\t\trefer_to_commit(opts, &msgbuf, commit);\n+\t\trefer_to_commit(use_reference, &msgbuf, commit);\n \n \t\tif (commit->parents && commit->parents->next) {\n \t\t\tstrbuf_addstr(&msgbuf, \", reversing\\nchanges made to \");\n-\t\t\trefer_to_commit(opts, &msgbuf, parent);\n+\t\t\trefer_to_commit(opts->commit_use_reference, &msgbuf, parent);\n \t\t}\n \t\tstrbuf_addstr(&msgbuf, \".\\n\");\n \t} else {\ndiff --git c/t/t3501-revert-cherry-pick.sh w/t/t3501-revert-cherry-pick.sh\nindex 7011e3a421..7a8715d3f4 100755\n--- c/t/t3501-revert-cherry-pick.sh\n+++ w/t/t3501-revert-cherry-pick.sh\n@@ -190,7 +190,16 @@ test_expect_success 'title of fresh reverts' '\n \tgit revert --no-edit HEAD &&\n \techo \"Revert \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n \tgit log -1 --pretty=%s >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Reapply \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Give the stronger instruction for unusually complex case\n+\tgit revert --no-edit HEAD &&\n+\tgit log -1 --pretty=%s >actual &&\n+\tgrep -F -e \"# *** SAY WHY WE ARE REVERTING\" actual\n '\n \n test_expect_success 'title of legacy double revert' '\n"},{"id":"480563","messageId":"xmqq1qg9tqhq.fsf@gitster.g","threadId":"59666","inReplyTo":"xmqq5y5ltqwd.fsf_-_@gitster.g","subject":"Re: Re* [PATCH v3 2/2] doc: revert: add discussion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-11T17:53:37Z","receivedAt":"2023-08-11T17:53:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> +\n> +\t\tif (starts_with(msg.subject, \"Reapply \\\"Reapply \\\"\"))\n> +\t\t\t/* fifth time is too many - force reference format*/\n> +\t\t\tuse_reference = 1;\n\nCome to think of it, as the documentation patch in the series cited\ndouble reapply as too unwieldy, we probably should stop before\nproducing such commit. We can update \"Reapply \\\"Reapply\" above to\n\"Revert \\\"Reapply\" and then \"fifth\" -> \"fourth\". The test update\nbelow must also be adjusted, if we want to take that route.\n\n"},{"id":"480564","messageId":"033a01d9cc7d$2290e600$67b2b200$@nexbridge.com","threadId":"59666","inReplyTo":"xmqq1qg9tqhq.fsf@gitster.g","subject":"RE: Re* [PATCH v3 2/2] doc: revert: add discussion","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-08-11T17:56:14Z","receivedAt":"2023-08-11T17:56:29Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Friday, August 11, 2023 1:54 PM, Junio C Hamano wrote:\n>Junio C Hamano <gitster@pobox.com> writes:\n>\n>> +\n>> +\t\tif (starts_with(msg.subject, \"Reapply \\\"Reapply \\\"\"))\n>> +\t\t\t/* fifth time is too many - force reference format*/\n>> +\t\t\tuse_reference = 1;\n>\n>Come to think of it, as the documentation patch in the series cited double\nreapply as\n>too unwieldy, we probably should stop before producing such commit. We can\n>update \"Reapply \\\"Reapply\" above to \"Revert \\\"Reapply\" and then \"fifth\" ->\n\"fourth\".\n>The test update below must also be adjusted, if we want to take that route.\n\nMay I suggest a potential quick solution. Perhaps we could leave this up to\nusers by putting in an --amend or --reword option to cause a prompt for a\ncomment for the reverted commit instead of trying to come up with a\none-size-fits-all solution.\n\n"},{"id":"480570","messageId":"CAPig+cS7XVZNOrV4POFqOmYDvALF6_SAxs0PhvgZWe66M3TR2Q@mail.gmail.com","threadId":"59666","inReplyTo":"xmqq5y5ltqwd.fsf_-_@gitster.g","subject":"Re: Re* [PATCH v3 2/2] doc: revert: add discussion","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-08-11T18:16:09Z","receivedAt":"2023-08-11T18:16:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 11, 2023 at 2:12 PM Junio C Hamano <gitster@pobox.com> wrote:\n> Subject: [PATCH 3/2] revert: force explaining overly complex revert chain\n>\n> Once we revert reverts of revert and reach \"Reapply \"Reapply \"...\"\",\n> it becomes too unweirdly to read a reversion of such a comit.\n\ns/unweirdly/unwieldy/\ns/comit/commit/\n\n> We instruct the user to explain why the reversion is done in their\n> own words when using the revert.reference mode, and the instruction\n> applies equally for such an overly complex revert chain. The\n> rationale for such a sequence of events should be recorded to help\n> future developers.\n>\n> Building on top of the recent Oswald's work to turn \"revert revert\"\n> into \"reapply\", let's turn the reference mode automatically on in\n> such a case.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"},{"id":"480571","messageId":"ZNZ7GhVkLuwYOPij@ugly","threadId":"59666","inReplyTo":"xmqq5y5ltqwd.fsf_-_@gitster.g","subject":"Re: Re* [PATCH v3 2/2] doc: revert: add discussion","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-11T18:16:58Z","receivedAt":"2023-08-11T18:17:08Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Aug 11, 2023 at 10:44:50AM -0700, Junio C Hamano wrote:\n>Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Linus Arver <linusa@google.com> writes:\n>>\n>>> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>>>\n>>>> while thinking about what to write, i came up with an idea for another\n>>>> improvement: with (implicit) --edit, the template message would end up\n>>>> being:\n>>>>\n>>>> This reverts commit <sha1>,\n>>>> because <PUT REASON HERE>.\n>>>\n>>> This sounds great to me.\n>>\n>> Oh, absolutely. I rarely do a revert myself (other than reverting a\n>> premature merge out of 'next'), but giving a better instruction in\n>> the commit log editor buffer as template is a very good idea.\n>\n>It might be just the matter of doing something like the attached\n>patch on top of Oswald's, reusing the existing code to instruct the\n>user to describe the reversion.\n>\nhmm, this seems to be going down a too narrow road - my idea was to make \nthis fully orthogonal to reverting reverts in particular (note that i \nattached it to the generic \"discussion\" patch rather than the \"reverts \nof reverts\" one).\ni didn't think about the integration with existing options yet.\n\nregards\n"},{"id":"480578","messageId":"xmqq1qg9s6uj.fsf@gitster.g","threadId":"59666","inReplyTo":"ZNZ7GhVkLuwYOPij@ugly","subject":"Re: Re* [PATCH v3 2/2] doc: revert: add discussion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-11T19:43:16Z","receivedAt":"2023-08-11T19:43:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> make this fully orthogonal to reverting reverts in particular (note\n> that i attached it to the generic \"discussion\" patch rather than the\n> \"reverts of reverts\" one).\n\nI viewed that as a patch to add \"discussion on how to explain\nrevert\" (not necessarily \"revert of revert\", but how \"revert\" in\ngeneral should be explained).\n\n> i didn't think about the integration with existing options yet.\n\nNow I did ;-). \n\nThe discussion of what the right way to present and justify a revert\nwas done before, and I think revert.reference and the \"--reference\"\noption came out of it. It aims the same \"do not just describe the\nfact what was reverted, but you should explain why\" spirit. While\nwhat I showed in my illustration patch was not meant as \"integration\nwith existing options\", it is inevitable that reuse of existing code\nthat was written earlier for improving the workflow in the same\nspirit may get involved.\n\nPerhaps we should also tweak the commit log template to give a\ngentle knudge to the user to use revert.reference and then enhance\nthe help text we give.\n\nInstead of\n\n Revert \"doc: revert: add discussion\"\n\n This reverts commit 7139d1298993b0148ad429cd7cb4824223b7f420.\n\nyou may see something along the lines of ...\n\n # Consider retitling to reflect WHY you are reverting.\n # You may also want to set revert.reference configuration to\n # help encuraging a better title for revert commits.\n Revert \"doc: revert: add discussion\"\n\n This reverts commit 7139d1298993b0148ad429cd7cb4824223b7f420\n because <REASON OF REVERSION HERE>\n\nI ran out of time budget for today to think about a topic that is\nnot releant to the current release, so I'd stop here.\n\nTHanks.\n"},{"id":"480605","messageId":"owly350pfal6.fsf@fine.c.googlers.com","threadId":"59666","inReplyTo":"ZNYuUh27ByphTH04@ugly","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2023-08-11T23:00:53Z","receivedAt":"2023-08-11T23:03:30Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> On Thu, Aug 10, 2023 at 02:50:59PM -0700, Linus Arver wrote:\n>>Nit: the \"doc: revert: add discussion\" subject line should probably be more\n>>like \"revert doc: suggest adding the 'why' behind reverts\".\n>>\n> this is counter to the prevalent \"big endian\" prefix style, and is in \n> this case really easy to misread.\n> i also intentionally kept the subject generic, because the content \n> covers two matters (the reasoning and the subjects, which is also the \n> reason why this is a separate patch to start with).\n\nI think the phrase \"add discussion\" in \"doc: revert: add discussion\"\ndoesn't add much value, because your patch's diff is very easy to read\n(in that it adds a new DISCUSSION section). I just wanted to replace it\nwith something more useful that gives more information than just repeat\n(somewhat redundantly) what is obvious by looking at the patch.\n\nI also learned recently that there should just be one colon \":\" in the\nsubject, which is why I suggested \"revert doc\" as the prefix instead of\n\"doc: revert: ...\".\n\n>>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>>> +DISCUSSION\n>>> +----------\n>>> +\n>>> +While git creates a basic commit message automatically, you really\n>>> +should not leave it at that. In particular, it is _strongly_\n>>> +recommended to explain why the original commit is being reverted.\n>>> +Repeatedly reverting reversions yields increasingly unwieldy\n>>> +commit subjects; latest when you arrive at 'Reapply \"Reapply\n>>> +\"<original subject>\"\"' you should get creative.\n>>\n>>The word \"latest\" here sounds odd. Ditto for \"get creative\".\n>>\n> yeah, i suppose. i wasn't sure how formal i should make it - things \n> aren't consistent to start with.\n\nFor our discussion I will define \"formal\" style as the writing style\nused in traditional reference texts, for example dictionaries and\nencyclopedias.\n\nFor manpages, I think we should stick to formal style as much as\npossible. The main concern I have is for readers of our manpages where\nEnglish may not be their first language. Although I understood what you\nmeant by the phrase \"get creative\", others may not understand so\nreadily. If there are places in our manpages where we do not use formal\nstyle, I think we should fix them.\n\nFor other types of documentation like tutorials, I think the style\ndoesn't have to be as formal, (in fact it should be as informal as\npossible) because a tutorial should be enjoyable to read from beginning\nto end. This is unlike a manpage where most of the time the user reads\nspecific (sub)sections to get the exact, precise information they need\n(just like looking up a word in a dictionary).\n\n>> How about the following rewording?\n>>\n>> While git creates a basic commit message automatically, it is\n>> _strongly_ recommended to explain why the original commit is being\n>> reverted. In addition, repeatedly reverting the same commit will\n>> result in increasingly unwieldy subject lines,\n>\n>>for example 'Reapply \"Reapply \"<original subject>\"\"'.\n>>\n> you turned it from a suggested threshold into an example. at this point \n> it appears superfluous to me.\n>\n>>Please consider rewording such\n>> subject lines to reflect the reason why the original commit is being\n>> reapplied again.\n>>\n> the reasoning most likely wouldn't fit into the subject.\n\nHence the language \"to _reflect_ the reason\", because the \"reason\"\nshould belong in the commit message body text.\n\n> also, the original request to explain the reasoning applies \n> transitively, so i don't think it's really necessary to point it out \n> explicitly.\n\nIt may be that a user will think only giving the revert reason in the\nbody text is enough, while leaving the subject line as is. I wanted to\nbreak this line of thinking by providing additional instructions.\n\n> On Thu, Aug 10, 2023 at 03:00:41PM -0700, Linus Arver wrote:\n>>Hmph, \"repeatedly reverting the same commit\" sounds wrong because\n>>strictly speaking there is only 1 \"same commit\" (the original commit).\n>>Perhaps\n>>\n>> In addition, repeatedly reverting the same progression of reverts will\n>>\n>>or even\n>>\n>> In addition, repeatedly reverting the same revert chain will\n>>\n>>is better here?\n>>\n> we used \"recursive reverts\" elsewhere. but i'm not sure whether that's \n> sufficiently intuitive and formally correct.\n\nAgreed. We might have to just define the correct phrasing ourselves in\n\"man gitglossary\". Surprisingly I see that the term \"revert\" (which can\nbe both a verb _and_ a noun) is not defined there.\n\n> [...]\n>\n> so in summary, how about:\n>\n> While git creates a basic commit message automatically, it is\n> _strongly_ recommended to explain why the original commit is being\n> reverted. In addition, repeatedly reverting reversions will\n> result in increasingly unwieldy subject lines. Please consider \n> rewording these into something shorter and more unique.\n\nThis is definitely better. But others in this thread have already\ncommented that my version looks good (after seeing your version also,\npresumably).\n"},{"id":"480628","messageId":"ZNclyKWYw4j0C7wM@ugly","threadId":"59666","inReplyTo":"07028529-cbe1-55d0-4ab0-9f3ec03a4fd1@gmail.com","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-12T06:25:12Z","receivedAt":"2023-08-12T06:25:21Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Aug 11, 2023 at 04:10:55PM +0100, Phillip Wood wrote:\n>On 10/08/2023 23:00, Linus Arver wrote:\n>> Linus Arver <linusa@google.com> writes:\n>> \n>>> How about\n>>> the following rewording?\n>>>\n>>> While git creates a basic commit message automatically, it is\n>>> _strongly_ recommended to explain why the original commit is being\n>>> reverted. In addition, repeatedly reverting the same commit will\n>> \n>> Hmph, \"repeatedly reverting the same commit\" sounds wrong because\n>> strictly speaking there is only 1 \"same commit\" (the original commit).\n>\n>While it isn't strictly accurate I think that wording is easy enough to \n>understand.\n>\nyes, but why would that be _better_ than saying \"repeatedly reverting \nreversions\" like i did?\n\nregards\n\n"},{"id":"480629","messageId":"ZNcyhUL89WVXOv3F@ugly","threadId":"59666","inReplyTo":"owly350pfal6.fsf@fine.c.googlers.com","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-12T07:19:33Z","receivedAt":"2023-08-12T07:19:37Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Aug 11, 2023 at 04:00:53PM -0700, Linus Arver wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> On Thu, Aug 10, 2023 at 02:50:59PM -0700, Linus Arver wrote:\n>>>Nit: the \"doc: revert: add discussion\" subject line should probably be more\n>>>like \"revert doc: suggest adding the 'why' behind reverts\".\n>>>\n>> this is counter to the prevalent \"big endian\" prefix style, and is in \n>> this case really easy to misread.\n>\n>I also learned recently that there should just be one colon \":\" in the\n>subject, which is why I suggested \"revert doc\" as the prefix instead of\n>\"doc: revert: ...\".\n>\nin what context was this preference expressed?\nbecause here, it's rather counter-productive: most commands are verbs \nfor obvious reasons, so using that style sets the reader up for \nmisparsing the subject on first try. this could be avoided by quoting \nthe command, but that looks noisy in the subject.\nso rather, i'd follow another precedent, 'git-revert.txt: ', which is \nunambiguous.\n\n>> i also intentionally kept the subject generic, because the content \n>> covers two matters (the reasoning and the subjects, which is also the \n>> reason why this is a separate patch to start with).\n>\n>I think the phrase \"add discussion\" in \"doc: revert: add discussion\"\n>doesn't add much value, because your patch's diff is very easy to read\n>(in that it adds a new DISCUSSION section). I just wanted to replace it\n>with something more useful that gives more information than\n\n>just repeat\n>(somewhat redundantly) what is obvious by looking at the patch.\n>\nbut ... that's exactly what a subject is supposed to do!\n\n>>>Please consider rewording such\n>>> subject lines to reflect the reason why the original commit is being\n>>> reapplied again.\n>>>\n>> the reasoning most likely wouldn't fit into the subject.\n>\n>Hence the language \"to _reflect_ the reason\", because the \"reason\"\n>should belong in the commit message body text.\n>\ni don't think that's how most people would actually read this.\nand i still don't see how that instruction could be meaningfully \nfollowed.\n\n>> also, the original request to explain the reasoning applies \n>> transitively, so i don't think it's really necessary to point it out \n>> explicitly.\n>\n>It may be that a user will think only giving the revert reason in the\n>body text is enough, while leaving the subject line as is. I wanted to\n>break this line of thinking by providing additional instructions.\n>\nyes, that's the whole intention of this patch. but i don't see how \nmaking it more convoluted than my proposal helps in any way.\n\n>This is definitely better. But others in this thread have already\n>commented that my version looks good (after seeing your version also,\n>presumably).\n>\nwell, i'm also an \"others\" when it comes to your proposal, and i find it \nconfusing.\n\nregards\n"},{"id":"480639","messageId":"xmqqy1iemw75.fsf@gitster.g","threadId":"59666","inReplyTo":"ZNclyKWYw4j0C7wM@ugly","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-13T22:09:02Z","receivedAt":"2023-08-13T22:09:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> On Fri, Aug 11, 2023 at 04:10:55PM +0100, Phillip Wood wrote:\n>>On 10/08/2023 23:00, Linus Arver wrote:\n>>> Hmph, \"repeatedly reverting the same commit\" sounds wrong because\n>>> strictly speaking there is only 1 \"same commit\" (the original commit).\n>>\n>> While it isn't strictly accurate I think that wording is easy enough\n>> to understand.\n>>\n> yes, but why would that be _better_ than saying \"repeatedly reverting\n> reversions\" like i did?\n\nTo me at least, \"repeatedly reverting reversions\" sounds more like a\nriddle, compared to \"repeatedly reverting the same commit\", whose\nintent sounds fairly obvious. An explicit mention of \"commit\", which\nis a more familiar noun to folks than \"reversion\", does contribute to\nit, I suspect.\n\nThat would be how I explain why one is _better_ over the other, but\nof course these things are subjective, so I'd rather see us not\nasking such questions too often: which is more familiar, \"commit\" vs\n\"reversion\", especially to new folks who are starting to use \"git\"\nand reading the manual page for \"git revert\"?\n\n"},{"id":"480646","messageId":"ZNo2oPaAsSISBalq@ugly","threadId":"59666","inReplyTo":"xmqqy1iemw75.fsf@gitster.g","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-14T14:13:52Z","receivedAt":"2023-08-14T14:16:32Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Sun, Aug 13, 2023 at 03:09:02PM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> On Fri, Aug 11, 2023 at 04:10:55PM +0100, Phillip Wood wrote:\n>>>On 10/08/2023 23:00, Linus Arver wrote:\n>>>> Hmph, \"repeatedly reverting the same commit\" sounds wrong because\n>>>> strictly speaking there is only 1 \"same commit\" (the original commit).\n>>>\n>>> While it isn't strictly accurate I think that wording is easy enough\n>>> to understand.\n>>>\n>> yes, but why would that be _better_ than saying \"repeatedly reverting\n>> reversions\" like i did?\n>\n>To me at least, \"repeatedly reverting reversions\" sounds more like a\n>riddle, compared to \"repeatedly reverting the same commit\", whose\n>intent sounds fairly obvious.\n>\na more natural way for git users to say it would be \"reverting reverts\", \nwhich i think everyone in the target audience would understand, but it \nseems linguistically questionable to me. native speakers may want to \nopine ...\n\n>An explicit mention of \"commit\", which\n>is a more familiar noun to folks than \"reversion\", does contribute to\n>it, I suspect.\n>\nyes, but \"commit\" may be misunderstood, as linus pointed out in his \nreply to himself. phillip dismissed the concern, but i don't think \nambiguity is a good idea in the authoritative documentation.\n\nunfortunately, linus' proposed alternatives seem even more like \n\"riddles\" to me than what i am proposing.\n\nregards\n"},{"id":"480846","messageId":"20230821170720.577850-2-oswald.buddenhagen@gmx.de","threadId":"59666","inReplyTo":"20230821170720.577850-1-oswald.buddenhagen@gmx.de","subject":"[PATCH v4 2/2] git-revert.txt: add discussion","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-21T17:07:20Z","receivedAt":"2023-08-21T17:07:29Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"The section is inspired by git-commit.txt.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\nv4:\n- adjusted summary prefix & payload wording\n\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Linus Arver <linusa@google.com>\nCc: Phillip Wood <phillip.wood123@gmail.com>\nCc: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n Documentation/git-revert.txt | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex d2e10d3dce..cbe0208834 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -142,6 +142,16 @@ EXAMPLES\n \tchanges. The revert only modifies the working tree and the\n \tindex.\n \n+DISCUSSION\n+----------\n+\n+While git creates a basic commit message automatically, it is\n+_strongly_ recommended to explain why the original commit is being\n+reverted.\n+In addition, repeatedly reverting reverts will result in increasingly\n+unwieldy subject lines, for example 'Reapply \"Reapply \"<original subject>\"\"'.\n+Please consider rewording these to be shorter and more unique.\n+\n CONFIGURATION\n -------------\n \n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480847","messageId":"20230821170720.577850-1-oswald.buddenhagen@gmx.de","threadId":"59666","inReplyTo":"20230809171531.2564807-1-oswald.buddenhagen@gmx.de","subject":"[PATCH v4 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-21T17:07:19Z","receivedAt":"2023-08-21T17:07:34Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\na more humane `Reapply \"foo\"`.\n\nThis is done for two reasons:\n- To cover the actually common case of just a double revert.\n- To encourage people to rewrite summaries of recursive reverts by\n setting an example (a subsequent commit will also do this explicitly\n in the documentation).\n\nTo achieve these goals, the mechanism does not need to be particularly\nsophisticated. Therefore, more complicated alternatives which would\n\"compress more efficiently\" have not been implemented.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\nv3:\n- capitulate at first sight of a pre-existing recursive reversion, as\n handling the edge cases is a bottomless pit\n- reworked commit message again\n- moved test into existing file\n- generalized docu change and factored it out\n\nv2:\n- add discussion to commit message\n- add paragraph to docu\n- add test\n- use skip_prefix() instead of starts_with()\n- catch pre-existing double reverts\n\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Kristoffer Haugsbakk <code@khaugsbakk.name>\nCc: Phillip Wood <phillip.wood123@gmail.com>\n---\n sequencer.c | 11 +++++++++++\n t/t3501-revert-cherry-pick.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 36 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex cc9821ece2..12ec158922 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2249,13 +2249,24 @@ static int do_pick_commit(struct repository *r,\n \t */\n \n \tif (command == TODO_REVERT) {\n+\t\tconst char *orig_subject;\n+\n \t\tbase = commit;\n \t\tbase_label = msg.label;\n \t\tnext = parent;\n \t\tnext_label = msg.parent_label;\n \t\tif (opts->commit_use_reference) {\n \t\t\tstrbuf_addstr(&msgbuf,\n \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n+\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject) &&\n+\t\t\t /*\n+\t\t\t * We don't touch pre-existing repeated reverts, because\n+\t\t\t * theoretically these can be nested arbitrarily deeply,\n+\t\t\t * thus requiring excessive complexity to deal with.\n+\t\t\t */\n+\t\t\t !starts_with(orig_subject, \"Revert \\\"\")) {\n+\t\t\tstrbuf_addstr(&msgbuf, \"Reapply \\\"\");\n+\t\t\tstrbuf_addstr(&msgbuf, orig_subject);\n \t\t} else {\n \t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n \t\t\tstrbuf_addstr(&msgbuf, msg.subject);\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex e2ef619323..7011e3a421 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -176,6 +176,31 @@ test_expect_success 'advice from failed revert' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'title of fresh reverts' '\n+\ttest_commit --no-tag A file1 &&\n+\ttest_commit --no-tag B file1 &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Revert \\\"B\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Reapply \\\"B\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Revert \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'title of legacy double revert' '\n+\ttest_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n+\tgit revert --no-edit HEAD &&\n+\techo \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'identification of reverted commit (default)' '\n \ttest_commit to-ident &&\n \ttest_when_finished \"git reset --hard to-ident\" &&\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480853","messageId":"xmqqjztop7pf.fsf@gitster.g","threadId":"59666","inReplyTo":"20230821170720.577850-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-21T18:32:28Z","receivedAt":"2023-08-21T18:32:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\n> a more humane `Reapply \"foo\"`.\n\nLooking good. Will requeue. Thanks.\n"},{"id":"480971","messageId":"ZOZnNDd2pMX6M2Au@nand.local","threadId":"59666","inReplyTo":"20230821170720.577850-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-08-23T20:08:20Z","receivedAt":"2023-08-23T20:09:21Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Aug 21, 2023 at 07:07:19PM +0200, Oswald Buddenhagen wrote:\n> To achieve these goals, the mechanism does not need to be particularly\n> sophisticated. Therefore, more complicated alternatives which would\n> \"compress more efficiently\" have not been implemented.\n\nThis version is looking good. The main functionality is well-reasoned\nand straightforwardly implemented. One minor suggestion that you could\nconsider squashing in is some test clean-up like so:\n\n--- 8< ---\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 7011e3a421..4dee71d6d5 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -176,29 +176,27 @@ test_expect_success 'advice from failed revert' '\n \ttest_cmp expected actual\n '\n\n+test_expect_commit_msg () {\n+\techo \"$@\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual\n+}\n+\n test_expect_success 'title of fresh reverts' '\n \ttest_commit --no-tag A file1 &&\n \ttest_commit --no-tag B file1 &&\n \tgit revert --no-edit HEAD &&\n-\techo \"Revert \\\"B\\\"\" >expect &&\n-\tgit log -1 --pretty=%s >actual &&\n-\ttest_cmp expect actual &&\n+\ttest_expect_commit_msg \"Revert \\\"B\\\"\" &&\n \tgit revert --no-edit HEAD &&\n-\techo \"Reapply \\\"B\\\"\" >expect &&\n-\tgit log -1 --pretty=%s >actual &&\n-\ttest_cmp expect actual &&\n+\ttest_expect_commit_msg \"Reapply \\\"B\\\"\" &&\n \tgit revert --no-edit HEAD &&\n-\techo \"Revert \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n-\tgit log -1 --pretty=%s >actual &&\n-\ttest_cmp expect actual\n+\ttest_expect_commit_msg \"Revert \\\"Reapply \\\"B\\\"\\\"\"\n '\n\n test_expect_success 'title of legacy double revert' '\n \ttest_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n \tgit revert --no-edit HEAD &&\n-\techo \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\" >expect &&\n-\tgit log -1 --pretty=%s >actual &&\n-\ttest_cmp expect actual\n+\ttest_expect_commit_msg \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\"\n '\n\n test_expect_success 'identification of reverted commit (default)' '\n--- >8 ---\n\nTo my eyes, it makes checking the subject of our revert commit against\nan expected value more readable by factoring out the echo, git log,\ntest_cmp pattern.\n\nThanks,\nTaylor\n"},{"id":"480975","messageId":"xmqqsf89e8wz.fsf@gitster.g","threadId":"59666","inReplyTo":"ZOZnNDd2pMX6M2Au@nand.local","subject":"Re: [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-23T21:38:36Z","receivedAt":"2023-08-23T21:41:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> This version is looking good. The main functionality is well-reasoned\n> and straightforwardly implemented. One minor suggestion that you could\n> consider squashing in is some test clean-up like so:\n>\n> --- 8< ---\n> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n> index 7011e3a421..4dee71d6d5 100755\n> --- a/t/t3501-revert-cherry-pick.sh\n> +++ b/t/t3501-revert-cherry-pick.sh\n> @@ -176,29 +176,27 @@ test_expect_success 'advice from failed revert' '\n> \ttest_cmp expected actual\n> '\n>\n> +test_expect_commit_msg () {\n> +\techo \"$@\" >expect &&\n> +\tgit log -1 --pretty=%s >actual &&\n> +\ttest_cmp expect actual\n> +}\n> +\n> test_expect_success 'title of fresh reverts' '\n> \ttest_commit --no-tag A file1 &&\n> \ttest_commit --no-tag B file1 &&\n> \tgit revert --no-edit HEAD &&\n> -\techo \"Revert \\\"B\\\"\" >expect &&\n> -\tgit log -1 --pretty=%s >actual &&\n> -\ttest_cmp expect actual &&\n> +\ttest_expect_commit_msg \"Revert \\\"B\\\"\" &&\n> \tgit revert --no-edit HEAD &&\n> -\techo \"Reapply \\\"B\\\"\" >expect &&\n> -\tgit log -1 --pretty=%s >actual &&\n> -\ttest_cmp expect actual &&\n> +\ttest_expect_commit_msg \"Reapply \\\"B\\\"\" &&\n> \tgit revert --no-edit HEAD &&\n> -\techo \"Revert \\\"Reapply \\\"B\\\"\\\"\" >expect &&\n> -\tgit log -1 --pretty=%s >actual &&\n> -\ttest_cmp expect actual\n> +\ttest_expect_commit_msg \"Revert \\\"Reapply \\\"B\\\"\\\"\"\n> '\n>\n> test_expect_success 'title of legacy double revert' '\n> \ttest_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n> \tgit revert --no-edit HEAD &&\n> -\techo \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\" >expect &&\n> -\tgit log -1 --pretty=%s >actual &&\n> -\ttest_cmp expect actual\n> +\ttest_expect_commit_msg \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\"\n> '\n>\n> test_expect_success 'identification of reverted commit (default)' '\n> --- >8 ---\n>\n> To my eyes, it makes checking the subject of our revert commit against\n> an expected value more readable by factoring out the echo, git log,\n> test_cmp pattern.\n\nYeah it does make the test more concise and what is expected stand\nout more clearly. Good suggestion.\n\n\n\n"},{"id":"480988","messageId":"ZOb1ViHIaqX8PcHV@ugly","threadId":"59666","inReplyTo":"xmqqsf89e8wz.fsf@gitster.g","subject":"Re: [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-24T06:14:46Z","receivedAt":"2023-08-24T06:16:04Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Wed, Aug 23, 2023 at 02:38:36PM -0700, Junio C Hamano wrote:\n>Taylor Blau <me@ttaylorr.com> writes:\n>\n>> This version is looking good. The main functionality is well-reasoned\n>> and straightforwardly implemented. One minor suggestion that you could\n>> consider squashing in is some test clean-up like so:\n>>\n>\n>Yeah it does make the test more concise and what is expected stand\n>out more clearly. Good suggestion.\n>\nagreed. do you want to squash it on your end, or should i reroll?\n\nregards\n"},{"id":"481322","messageId":"20230902072035.652549-1-oswald.buddenhagen@gmx.de","threadId":"59666","inReplyTo":"xmqqsf89e8wz.fsf@gitster.g","subject":"[PATCH v5] sequencer: beautify subject of reverts of reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-09-02T07:20:35Z","receivedAt":"2023-09-02T07:20:43Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\na more humane `Reapply \"foo\"`.\n\nThis is done for two reasons:\n- To cover the actually common case of just a double revert.\n- To encourage people to rewrite summaries of recursive reverts by\n setting an example (a subsequent commit will also do this explicitly\n in the documentation).\n\nTo achieve these goals, the mechanism does not need to be particularly\nsophisticated. Therefore, more complicated alternatives which would\n\"compress more efficiently\" have not been implemented.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n\n---\nv4:\n- factor out verification of subject as per taylor's patch, with minor\n modifications.\n fwiw, it might make sense to put this into test-lib-functions.sh right\n after test_commit_message(), then named test_commit_subject(). not\n sure it would be worth it, given an equally generic implementation\n would be kinda over-engineered, and the discoverability is kinda poor.\n\nv3:\n- capitulate at first sight of a pre-existing recursive reversion, as\n handling the edge cases is a bottomless pit\n- reworked commit message again\n- moved test into existing file\n- generalized docu change and factored it out\n\nv2:\n- add discussion to commit message\n- add paragraph to docu\n- add test\n- use skip_prefix() instead of starts_with()\n- catch pre-existing double reverts\n\nCc: Junio C Hamano <gitster@pobox.com>\nCc: Kristoffer Haugsbakk <code@khaugsbakk.name>\nCc: Phillip Wood <phillip.wood123@gmail.com>\n---\n sequencer.c | 11 +++++++++++\n t/t3501-revert-cherry-pick.sh | 23 +++++++++++++++++++++++\n 2 files changed, 34 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex cc9821ece2..12ec158922 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2249,13 +2249,24 @@ static int do_pick_commit(struct repository *r,\n \t */\n \n \tif (command == TODO_REVERT) {\n+\t\tconst char *orig_subject;\n+\n \t\tbase = commit;\n \t\tbase_label = msg.label;\n \t\tnext = parent;\n \t\tnext_label = msg.parent_label;\n \t\tif (opts->commit_use_reference) {\n \t\t\tstrbuf_addstr(&msgbuf,\n \t\t\t\t\"# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***\");\n+\t\t} else if (skip_prefix(msg.subject, \"Revert \\\"\", &orig_subject) &&\n+\t\t\t /*\n+\t\t\t * We don't touch pre-existing repeated reverts, because\n+\t\t\t * theoretically these can be nested arbitrarily deeply,\n+\t\t\t * thus requiring excessive complexity to deal with.\n+\t\t\t */\n+\t\t\t !starts_with(orig_subject, \"Revert \\\"\")) {\n+\t\t\tstrbuf_addstr(&msgbuf, \"Reapply \\\"\");\n+\t\t\tstrbuf_addstr(&msgbuf, orig_subject);\n \t\t} else {\n \t\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n \t\t\tstrbuf_addstr(&msgbuf, msg.subject);\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex e2ef619323..4158590322 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -176,6 +176,29 @@ test_expect_success 'advice from failed revert' '\n \ttest_cmp expected actual\n '\n \n+test_expect_subject () {\n+\techo \"$1\" >expect &&\n+\tgit log -1 --pretty=%s >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success 'titles of fresh reverts' '\n+\ttest_commit --no-tag A file1 &&\n+\ttest_commit --no-tag B file1 &&\n+\tgit revert --no-edit HEAD &&\n+\ttest_expect_subject \"Revert \\\"B\\\"\" &&\n+\tgit revert --no-edit HEAD &&\n+\ttest_expect_subject \"Reapply \\\"B\\\"\" &&\n+\tgit revert --no-edit HEAD &&\n+\ttest_expect_subject \"Revert \\\"Reapply \\\"B\\\"\\\"\"\n+'\n+\n+test_expect_success 'title of legacy double revert' '\n+\ttest_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n+\tgit revert --no-edit HEAD &&\n+\ttest_expect_subject \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\"\n+'\n+\n test_expect_success 'identification of reverted commit (default)' '\n \ttest_commit to-ident &&\n \ttest_when_finished \"git reset --hard to-ident\" &&\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"481340","messageId":"xmqqsf7wkyd2.fsf@gitster.g","threadId":"59666","inReplyTo":"20230902072035.652549-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v5] sequencer: beautify subject of reverts of reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-02T22:24:09Z","receivedAt":"2023-09-02T22:24:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> ---\n> v4:\n> - factor out verification of subject as per taylor's patch, with minor\n> modifications.\n\nThe change seems to make the test quite straight-forward to read.\n\nLet's mark the topic for 'next'.\n\nThanks.\n\n> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n> index e2ef619323..4158590322 100755\n> --- a/t/t3501-revert-cherry-pick.sh\n> +++ b/t/t3501-revert-cherry-pick.sh\n> @@ -176,6 +176,29 @@ test_expect_success 'advice from failed revert' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_subject () {\n> +\techo \"$1\" >expect &&\n> +\tgit log -1 --pretty=%s >actual &&\n> +\ttest_cmp expect actual\n> +}\n> +\n> +test_expect_success 'titles of fresh reverts' '\n> +\ttest_commit --no-tag A file1 &&\n> +\ttest_commit --no-tag B file1 &&\n> +\tgit revert --no-edit HEAD &&\n> +\ttest_expect_subject \"Revert \\\"B\\\"\" &&\n> +\tgit revert --no-edit HEAD &&\n> +\ttest_expect_subject \"Reapply \\\"B\\\"\" &&\n> +\tgit revert --no-edit HEAD &&\n> +\ttest_expect_subject \"Revert \\\"Reapply \\\"B\\\"\\\"\"\n> +'\n> +\n> +test_expect_success 'title of legacy double revert' '\n> +\ttest_commit --no-tag \"Revert \\\"Revert \\\"B\\\"\\\"\" file1 &&\n> +\tgit revert --no-edit HEAD &&\n> +\ttest_expect_subject \"Revert \\\"Revert \\\"Revert \\\"B\\\"\\\"\\\"\"\n> +'\n> +\n> test_expect_success 'identification of reverted commit (default)' '\n> \ttest_commit to-ident &&\n> \ttest_when_finished \"git reset --hard to-ident\" &&\n"},{"id":"481525","messageId":"owlytts5llje.fsf@fine.c.googlers.com","threadId":"59666","inReplyTo":"ZNcyhUL89WVXOv3F@ugly","subject":"Re: [PATCH v3 2/2] doc: revert: add discussion","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2023-09-07T21:29:25Z","receivedAt":"2023-09-07T21:29:31Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"First, I apologize for the long delay in my response. I only work on Git\n20% of the time, and that 20% can become 0% due to factors outside my\ncontrol.\n\nOswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> On Fri, Aug 11, 2023 at 04:00:53PM -0700, Linus Arver wrote:\n>>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>>\n>>> On Thu, Aug 10, 2023 at 02:50:59PM -0700, Linus Arver wrote:\n>>>>Nit: the \"doc: revert: add discussion\" subject line should probably be more\n>>>>like \"revert doc: suggest adding the 'why' behind reverts\".\n>>>>\n>>> this is counter to the prevalent \"big endian\" prefix style, and is in \n>>> this case really easy to misread.\n>>\n>>I also learned recently that there should just be one colon \":\" in the\n>>subject, which is why I suggested \"revert doc\" as the prefix instead of\n>>\"doc: revert: ...\".\n>>\n> in what context was this preference expressed?\n\nIIRC, it was from a conversation off-list with the folks at Google's\nGit-core team.\n\n> because here, it's rather counter-productive: most commands are verbs \n> for obvious reasons, so using that style sets the reader up for \n> misparsing the subject on first try.\n\nI think the convention for commit titles is\n\n <prefix>: <action>\n\nso the phrase \"revert doc: add discussion\", where the <prefix> is\n\"revert doc\" does not parse any worse than \"doc: revert: add\ndiscussion\". That is, the <prefix> is never confused with the <action>\n(they are separated by the colon).\n\n> this could be avoided by quoting \n> the command, but that looks noisy in the subject.\n> so rather, i'd follow another precedent, 'git-revert.txt: ', which is \n> unambiguous.\n\nSGTM.\n\n>>> i also intentionally kept the subject generic, because the content \n>>> covers two matters (the reasoning and the subjects, which is also the \n>>> reason why this is a separate patch to start with).\n>>\n>>I think the phrase \"add discussion\" in \"doc: revert: add discussion\"\n>>doesn't add much value, because your patch's diff is very easy to read\n>>(in that it adds a new DISCUSSION section). I just wanted to replace it\n>>with something more useful that gives more information than\n>\n>>just repeat\n>>(somewhat redundantly) what is obvious by looking at the patch.\n>>\n> but ... that's exactly what a subject is supposed to do!\n\nI think the rule of thumb is to explain the goodness of what a commit\nbrings, rather than focus on what is literally happening. This is\nbecause the former is more valuable. So instead of\n\n \"git-revert.txt: add discussion\"\n\nyou could say\n\n \"git-revert.txt: advise against default commit message\"\n\nand now you don't have to look at the patch to see (roughly) what kind\nof discussion was added.\n\n>>>>Please consider rewording such\n>>>> subject lines to reflect the reason why the original commit is being\n>>>> reapplied again.\n>>>>\n>>> the reasoning most likely wouldn't fit into the subject.\n>>\n>>Hence the language \"to _reflect_ the reason\", because the \"reason\"\n>>should belong in the commit message body text.\n>>\n> i don't think that's how most people would actually read this.\n> and i still don't see how that instruction could be meaningfully \n> followed.\n\nOK, you may be right.\n\n>>> also, the original request to explain the reasoning applies \n>>> transitively, so i don't think it's really necessary to point it out \n>>> explicitly.\n>>\n>>It may be that a user will think only giving the revert reason in the\n>>body text is enough, while leaving the subject line as is. I wanted to\n>>break this line of thinking by providing additional instructions.\n>>\n> yes, that's the whole intention of this patch. but i don't see how \n> making it more convoluted than my proposal helps in any way.\n\nWell, even if a review makes something more convoluted, it may generate\ndiscussion and drive consensus on the better way(s) of doing something.\nI see value in that course of events.\n\nOf course you are free to reject review comments that you truly believe\nare inferior to the approach you've already taken.\n\nBut overall, when I see a reviewer's comment on this mailing list, I\nassume they are trying to make my patch better. Similarly when I\nreviewed your patch my intent was to provide actionable feedback to try\nto make it better. I'm sorry if I did not come across that way.\n\n>>This is definitely better. But others in this thread have already\n>>commented that my version looks good (after seeing your version also,\n>>presumably).\n>>\n> well, i'm also an \"others\" when it comes to your proposal, and i find it \n> confusing.\n\nI think you did the right thing by responding to my comments, and\npointing to things you found confusing.\n"},{"id":"481683","messageId":"568b853a-cf71-4262-86be-7b65cf066d93@app.fastmail.com","threadId":"59666","inReplyTo":"20230902072035.652549-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v5] sequencer: beautify subject of reverts of reverts","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-11T20:12:39Z","receivedAt":"2023-09-11T21:38:42Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Sat, Sep 2, 2023, at 09:20, Oswald Buddenhagen wrote:\n> Instead of generating a silly-looking `Revert \"Revert \"foo\"\"`, make it\n> a more humane `Reapply \"foo\"`.\n\nCongrats on a nice series. It's very “lean and mean”—focused, not\nexcessive.\n\nAnd I think I will remember the phrase “too nerdy” for a while. ;)\n\nMaybe we will get this message template the next time we revert a\nmerge.[1]\n\n> If you merge the updated side branch (with D at its tip), none of the\n> changes made in A or B will be in the result, because they were reverted\n> by W. That is what Alan saw.\n>\n> [...]\n>\n> In such a situation, you would want to first revert the previous revert,\n> which would make the history look like this: ...\n\n🔗 1: https://github.com/git/git/blob/master/Documentation/howto/revert-a-faulty-merge.txt\n\nCheers\n\n-- \nKristoffer\n"}]}