{"thread":{"id":"41952","subject":"[PATCH] rebase: convert revert to squash on autosquash","startedAt":"2016-04-07T15:12:56Z","lastAt":"2016-04-09T17:17:49Z","messageCount":9,"participants":["Michael S. Tsirkin","Johannes Schindelin","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"282838","messageId":"1460041965-31526-1-git-send-email-mst@redhat.com","threadId":"41952","inReplyTo":null,"subject":"[PATCH] rebase: convert revert to squash on autosquash","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2016-04-07T15:12:56Z","receivedAt":"2016-04-07T15:12:56Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Reverts can typically be treated like squash.  Eliminating both the\noriginal commit and the revert would be even nicer, but this seems a bit\nharder to implement.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n git-rebase--interactive.sh | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 6a766ca..6fc1935 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -777,7 +777,7 @@ rearrange_squash () {\n \tdo\n \t\ttest -z \"${format}\" || message=$(git log -n 1 --format=\"%s\" ${sha1})\n \t\tcase \"$message\" in\n-\t\t\"squash! \"*|\"fixup! \"*|\"ack! \"*)\n+\t\t\"squash! \"*|\"fixup! \"*|\"ack! \"*|\"Revert \\\"\"*)\n \t\t\taction=\"${message%%!*}\"\n \t\t\trest=$message\n \t\t\tprefix=\n@@ -789,6 +789,12 @@ rearrange_squash () {\n \t\t\t\t\tprefix=\"$prefix${rest%%!*},\"\n \t\t\t\t\trest=\"${rest#*! }\"\n \t\t\t\t\t;;\n+\t\t\t\t\"Revert \\\"\"*\\\")\n+\t\t\t\t\taction=\"squash\"\n+\t\t\t\t\tprefix=\"Revert,\"\n+\t\t\t\t\trest=\"${rest#Revert \\\"}\"\n+\t\t\t\t\trest=\"${rest%%\\\"}\"\n+\t\t\t\t\t;;\n \t\t\t\t*)\n \t\t\t\t\tbreak\n \t\t\t\t\t;;\n-- \nMST\n"},{"id":"282844","messageId":"alpine.DEB.2.20.1604071720160.2967@virtualbox","threadId":"41952","inReplyTo":"1460041965-31526-1-git-send-email-mst@redhat.com","subject":"Re: [PATCH] rebase: convert revert to squash on autosquash","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-04-07T15:23:09Z","receivedAt":"2016-04-07T15:23:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 7 Apr 2016, Michael S. Tsirkin wrote:\n\n> Reverts can typically be treated like squash.  Eliminating both the\n> original commit and the revert would be even nicer, but this seems a bit\n> harder to implement.\n\nWhoa. This rings a lot of alarm bells, very loudly. It seems you intend to\nintroduce a *major* change in behavior, and all we get to convince us that\nthis is a good change is this puny paragraph (which, by the way, does not\ndo half a good job of explaining to me what you think this patch is\nsupposed to do, let alone of convincing me that what you want is a good\nchange).\n\nSo. What is it again that you want to achieve? Please use plain English,\ne.g. explaining how exactly reverts are typically to be treated like\nsquashes. And please make it convincing, because so far, I am far from\nconvinced.\n\nCiao,\nJohannes\n"},{"id":"282845","messageId":"20160407184026-mutt-send-email-mst@redhat.com","threadId":"41952","inReplyTo":"alpine.DEB.2.20.1604071720160.2967@virtualbox","subject":"Re: [PATCH] rebase: convert revert to squash on autosquash","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2016-04-07T15:51:45Z","receivedAt":"2016-04-07T15:51:45Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Thu, Apr 07, 2016 at 05:23:09PM +0200, Johannes Schindelin wrote:\n> Hi,\n> \n> On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:\n> \n> > Reverts can typically be treated like squash.  Eliminating both the\n> > original commit and the revert would be even nicer, but this seems a bit\n> > harder to implement.\n> \n> Whoa. This rings a lot of alarm bells, very loudly.\n\nWhoa don't be alarmed. It's just a patch :).\nI've been using this patch for more than a year now, so I\nthought I'd post it in case it's useful for others.\n\n\n> It seems you intend to\n> introduce a *major* change in behavior,\n\nDoing this automatically for all users might be a bit too drastic for\nthe upstream git.  So there could be an option, or something - if\nthere's interest I can add that. I thought I'd test the waters before\nI spend time on that.\n\n> and all we get to convince us that\n> this is a good change is this puny paragraph (which, by the way, does not\n> do half a good job of explaining to me what you think this patch is\n> supposed to do, let alone of convincing me that what you want is a good\n> change).\n> \n> So. What is it again that you want to achieve? Please use plain English,\n> e.g. explaining how exactly reverts are typically to be treated like\n> squashes. And please make it convincing, because so far, I am far from\n> convinced.\n> \n> Ciao,\n> Johannes\n\nIt's rather simple.\n\nIf there's a commit later followed by a revert, history can be\nsimplified by squashing them, and if the result is empty, removing both.\n\nThe removal part is not automatic with my patch.  If someone wants to\nimplement it, that would be nice and useful for me.\n\nsquashing/fixing is similar in that they are also useful to keep history\nclean.\n-- \nMST\n"},{"id":"282944","messageId":"alpine.DEB.2.20.1604081309150.2967@virtualbox","threadId":"41952","inReplyTo":"20160407184026-mutt-send-email-mst@redhat.com","subject":"Re: [PATCH] rebase: convert revert to squash on autosquash","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-04-08T11:13:51Z","receivedAt":"2016-04-08T11:13:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nOn Thu, 7 Apr 2016, Michael S. Tsirkin wrote:\n\n> On Thu, Apr 07, 2016 at 05:23:09PM +0200, Johannes Schindelin wrote:\n> > \n> > On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:\n> > \n> > > Reverts can typically be treated like squash.  Eliminating both the\n> > > original commit and the revert would be even nicer, but this seems a bit\n> > > harder to implement.\n> > \n> > Whoa. This rings a lot of alarm bells, very loudly.\n> \n> Whoa don't be alarmed. It's just a patch :).\n\nIt's just a patch. Like every major breakage would be. So: no, there is\nreason to be alarmed if it is likely to disrupt normal usage.\n\n> > It seems you intend to introduce a *major* change in behavior,\n> \n> Doing this automatically for all users might be a bit too drastic for\n> the upstream git.\n\nThat is a pretty safe thing to say, even without the subjunctive.\n\n> If there's a commit later followed by a revert, history can be\n> simplified by squashing them, and if the result is empty, removing both.\n\nTrue. But that is not what the user told Git to do. If the user's\nintention was to squash the reverting patch, she could have easily done\nthis:\n\n\tgit revert -n deadbeef\n\tgit commit --squash deadbeef\n\nwhere \"deadbeef\" is the placeholder for the actual commit to revert.\n\nAnd indeed, I use exactly this song and dance quite frequently, *iff* my\nintention is to drop a patch.\n\nA much better idea than co-opting the \"Revert\" commit message would be to\nintroduce a sibling to --fixup and --squash that you could call --drop.\n\nCiao,\nJohannes\n"},{"id":"282948","messageId":"vpq37qwpbxf.fsf@anie.imag.fr","threadId":"41952","inReplyTo":"20160407184026-mutt-send-email-mst@redhat.com","subject":"Re: [PATCH] rebase: convert revert to squash on autosquash","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-04-08T11:39:08Z","receivedAt":"2016-04-08T11:39:08Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"\"Michael S. Tsirkin\" <mst@redhat.com> writes:\n\n> Doing this automatically for all users might be a bit too drastic for\n> the upstream git.  So there could be an option, or something - if\n> there's interest I can add that. I thought I'd test the waters before\n> I spend time on that.\n\nIf the goal is to test the waters, then adding an RFC tag to your\nsubject helps starting more constructive discussion and avoid alarm\nbells ringing like Johannes' reaction. Having the content of this\nparagraph as a comment below the --- part of the patch also helps.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"282949","messageId":"vpqtwjcnx7v.fsf@anie.imag.fr","threadId":"41952","inReplyTo":"alpine.DEB.2.20.1604081309150.2967@virtualbox","subject":"Re: [PATCH] rebase: convert revert to squash on autosquash","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-04-08T11:42:12Z","receivedAt":"2016-04-08T11:42:12Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> \tgit revert -n deadbeef\n> \tgit commit --squash deadbeef\n>\n> where \"deadbeef\" is the placeholder for the actual commit to revert.\n>\n> And indeed, I use exactly this song and dance quite frequently, *iff* my\n> intention is to drop a patch.\n>\n> A much better idea than co-opting the \"Revert\" commit message would be to\n> introduce a sibling to --fixup and --squash that you could call\n> --drop.\n\nOne could also add --fixup and --squash to \"git revert\", so the above\nwould become\n\n    git revert --squash deadbeef\n\nIn most cases, I find it simpler to just start a rebase -i and drop the\ncommit from rebase's todo-list.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"282950","messageId":"20160408144231-mutt-send-email-mst@redhat.com","threadId":"41952","inReplyTo":"alpine.DEB.2.20.1604081309150.2967@virtualbox","subject":"Re: [PATCH] rebase: convert revert to squash on autosquash","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2016-04-08T11:43:06Z","receivedAt":"2016-04-08T11:43:06Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Fri, Apr 08, 2016 at 01:13:51PM +0200, Johannes Schindelin wrote:\n> Hi Michael,\n> \n> On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:\n> \n> > On Thu, Apr 07, 2016 at 05:23:09PM +0200, Johannes Schindelin wrote:\n> > > \n> > > On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:\n> > > \n> > > > Reverts can typically be treated like squash.  Eliminating both the\n> > > > original commit and the revert would be even nicer, but this seems a bit\n> > > > harder to implement.\n> > > \n> > > Whoa. This rings a lot of alarm bells, very loudly.\n> > \n> > Whoa don't be alarmed. It's just a patch :).\n> \n> It's just a patch. Like every major breakage would be. So: no, there is\n> reason to be alarmed if it is likely to disrupt normal usage.\n> \n> > > It seems you intend to introduce a *major* change in behavior,\n> > \n> > Doing this automatically for all users might be a bit too drastic for\n> > the upstream git.\n> \n> That is a pretty safe thing to say, even without the subjunctive.\n> \n> > If there's a commit later followed by a revert, history can be\n> > simplified by squashing them, and if the result is empty, removing both.\n> \n> True. But that is not what the user told Git to do. If the user's\n> intention was to squash the reverting patch, she could have easily done\n> this:\n> \n> \tgit revert -n deadbeef\n> \tgit commit --squash deadbeef\n> \n> where \"deadbeef\" is the placeholder for the actual commit to revert.\n> \n> And indeed, I use exactly this song and dance quite frequently, *iff* my\n> intention is to drop a patch.\n> \n> A much better idea than co-opting the \"Revert\" commit message would be to\n> introduce a sibling to --fixup and --squash that you could call --drop.\n> \n> Ciao,\n> Johannes\n\nSounds rather cool. Or alternatively\n\ngit revert --squash deadbeef\n"},{"id":"282959","messageId":"20160408170809-mutt-send-email-mst@redhat.com","threadId":"41952","inReplyTo":"vpqtwjcnx7v.fsf@anie.imag.fr","subject":"Re: [PATCH] rebase: convert revert to squash on autosquash","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2016-04-08T14:09:05Z","receivedAt":"2016-04-08T14:09:05Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Fri, Apr 08, 2016 at 01:42:12PM +0200, Matthieu Moy wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > \tgit revert -n deadbeef\n> > \tgit commit --squash deadbeef\n> >\n> > where \"deadbeef\" is the placeholder for the actual commit to revert.\n> >\n> > And indeed, I use exactly this song and dance quite frequently, *iff* my\n> > intention is to drop a patch.\n> >\n> > A much better idea than co-opting the \"Revert\" commit message would be to\n> > introduce a sibling to --fixup and --squash that you could call\n> > --drop.\n> \n> One could also add --fixup and --squash to \"git revert\", so the above\n> would become\n> \n>     git revert --squash deadbeef\n> \n> In most cases, I find it simpler to just start a rebase -i and drop the\n> commit from rebase's todo-list.\n\nAbsolutely, but moving rebase to near the commit to drop also\nmakes it easier to spot where the commit is.\nThoughts?\n\n> -- \n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n"},{"id":"283024","messageId":"20160409201344-mutt-send-email-mst@redhat.com","threadId":"41952","inReplyTo":"alpine.DEB.2.20.1604081309150.2967@virtualbox","subject":"Re: [PATCH] rebase: convert revert to squash on autosquash","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2016-04-09T17:17:49Z","receivedAt":"2016-04-09T17:17:49Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Fri, Apr 08, 2016 at 01:13:51PM +0200, Johannes Schindelin wrote:\n> Hi Michael,\n> \n> On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:\n> \n> > On Thu, Apr 07, 2016 at 05:23:09PM +0200, Johannes Schindelin wrote:\n> > > \n> > > On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:\n> > > \n> > > > Reverts can typically be treated like squash.  Eliminating both the\n> > > > original commit and the revert would be even nicer, but this seems a bit\n> > > > harder to implement.\n> > > \n> > > Whoa. This rings a lot of alarm bells, very loudly.\n> > \n> > Whoa don't be alarmed. It's just a patch :).\n> \n> It's just a patch. Like every major breakage would be. So: no, there is\n> reason to be alarmed if it is likely to disrupt normal usage.\n> \n> > > It seems you intend to introduce a *major* change in behavior,\n> > \n> > Doing this automatically for all users might be a bit too drastic for\n> > the upstream git.\n> \n> That is a pretty safe thing to say, even without the subjunctive.\n> \n> > If there's a commit later followed by a revert, history can be\n> > simplified by squashing them, and if the result is empty, removing both.\n> \n> True. But that is not what the user told Git to do. If the user's\n> intention was to squash the reverting patch, she could have easily done\n> this:\n> \n> \tgit revert -n deadbeef\n> \tgit commit --squash deadbeef\n> \n> where \"deadbeef\" is the placeholder for the actual commit to revert.\n> \n> And indeed, I use exactly this song and dance quite frequently, *iff* my\n> intention is to drop a patch.\n\nWell then you have to decide whether you want to drop it\nwhen you commit.\nIf *I* want do drop the patch when I commit, I just do\ngit rebase.\n\n> A much better idea than co-opting the \"Revert\" commit message would be to\n> introduce a sibling to --fixup and --squash that you could call --drop.\n> \n> Ciao,\n> Johannes\n\nMaybe but it's a different usecase.\n\nWhat this addresses is a case where you first wanted to\navoid rebases, so you reverted.\nBut then you rebased after all.\nNow finding what was reverted automatically is helpful.\n"}]}