[PATCH] rebase: convert revert to squash on autosquash

Reverts can typically be treated like squash.  Eliminating both the
original commit and the revert would be even nicer, but this seems a bit
harder to implement.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 git-rebase--interactive.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6a766ca..6fc1935 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh

@@ -777,7 +777,7 @@ rearrange_squash () {
 	do
 		test -z "${format}" || message=$(git log -n 1 --format="%s" ${sha1})
 		case "$message" in
-		"squash! "*|"fixup! "*|"ack! "*)
+		"squash! "*|"fixup! "*|"ack! "*|"Revert \""*)
 			action="${message%%!*}"
 			rest=$message
 			prefix=

@@ -789,6 +789,12 @@ rearrange_squash () {
 					prefix="$prefix${rest%%!*},"
 					rest="${rest#*! }"
 					;;
+				"Revert \""*\")
+					action="squash"
+					prefix="Revert,"
+					rest="${rest#Revert \"}"
+					rest="${rest%%\"}"
+					;;
 				*)
 					break
 					;;

-- 
MST

Hi,

On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:

Reverts can typically be treated like squash.  Eliminating both the
original commit and the revert would be even nicer, but this seems a bit
harder to implement.

Whoa. This rings a lot of alarm bells, very loudly. It seems you intend to
introduce a *major* change in behavior, and all we get to convince us that
this is a good change is this puny paragraph (which, by the way, does not
do half a good job of explaining to me what you think this patch is
supposed to do, let alone of convincing me that what you want is a good
change).

So. What is it again that you want to achieve? Please use plain English,
e.g. explaining how exactly reverts are typically to be treated like
squashes. And please make it convincing, because so far, I am far from
convinced.

Ciao,
Johannes

On Thu, Apr 07, 2016 at 05:23:09PM +0200, Johannes Schindelin wrote:

Hi,

On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:

quoted

Reverts can typically be treated like squash.  Eliminating both the
original commit and the revert would be even nicer, but this seems a bit
harder to implement.

Whoa. This rings a lot of alarm bells, very loudly.

Whoa don't be alarmed. It's just a patch :).
I've been using this patch for more than a year now, so I
thought I'd post it in case it's useful for others.

It seems you intend to
introduce a *major* change in behavior,

Doing this automatically for all users might be a bit too drastic for
the upstream git.  So there could be an option, or something - if
there's interest I can add that. I thought I'd test the waters before
I spend time on that.

and all we get to convince us that
this is a good change is this puny paragraph (which, by the way, does not
do half a good job of explaining to me what you think this patch is
supposed to do, let alone of convincing me that what you want is a good
change).

So. What is it again that you want to achieve? Please use plain English,
e.g. explaining how exactly reverts are typically to be treated like
squashes. And please make it convincing, because so far, I am far from
convinced.

Ciao,
Johannes

It's rather simple.

If there's a commit later followed by a revert, history can be
simplified by squashing them, and if the result is empty, removing both.

The removal part is not automatic with my patch.  If someone wants to
implement it, that would be nice and useful for me.

squashing/fixing is similar in that they are also useful to keep history
clean.
-- 
MST

Hi Michael,

On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:

On Thu, Apr 07, 2016 at 05:23:09PM +0200, Johannes Schindelin wrote:

quoted

On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:

quoted

Reverts can typically be treated like squash.  Eliminating both the
original commit and the revert would be even nicer, but this seems a bit
harder to implement.

Whoa. This rings a lot of alarm bells, very loudly.

Whoa don't be alarmed. It's just a patch :).

It's just a patch. Like every major breakage would be. So: no, there is
reason to be alarmed if it is likely to disrupt normal usage.

quoted

It seems you intend to introduce a *major* change in behavior,

Doing this automatically for all users might be a bit too drastic for
the upstream git.

That is a pretty safe thing to say, even without the subjunctive.

If there's a commit later followed by a revert, history can be
simplified by squashing them, and if the result is empty, removing both.

True. But that is not what the user told Git to do. If the user's
intention was to squash the reverting patch, she could have easily done
this:

	git revert -n deadbeef
	git commit --squash deadbeef

where "deadbeef" is the placeholder for the actual commit to revert.

And indeed, I use exactly this song and dance quite frequently, *iff* my
intention is to drop a patch.

A much better idea than co-opting the "Revert" commit message would be to
introduce a sibling to --fixup and --squash that you could call --drop.

Ciao,
Johannes

On Fri, Apr 08, 2016 at 01:13:51PM +0200, Johannes Schindelin wrote:

Hi Michael,

On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:

quoted

On Thu, Apr 07, 2016 at 05:23:09PM +0200, Johannes Schindelin wrote:

quoted

On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:

quoted

Reverts can typically be treated like squash.  Eliminating both the
original commit and the revert would be even nicer, but this seems a bit
harder to implement.

Whoa. This rings a lot of alarm bells, very loudly.

Whoa don't be alarmed. It's just a patch :).

It's just a patch. Like every major breakage would be. So: no, there is
reason to be alarmed if it is likely to disrupt normal usage.

quoted

quoted

It seems you intend to introduce a *major* change in behavior,

Doing this automatically for all users might be a bit too drastic for
the upstream git.

That is a pretty safe thing to say, even without the subjunctive.

quoted

If there's a commit later followed by a revert, history can be
simplified by squashing them, and if the result is empty, removing both.

True. But that is not what the user told Git to do. If the user's
intention was to squash the reverting patch, she could have easily done
this:

	git revert -n deadbeef
	git commit --squash deadbeef

where "deadbeef" is the placeholder for the actual commit to revert.

And indeed, I use exactly this song and dance quite frequently, *iff* my
intention is to drop a patch.

A much better idea than co-opting the "Revert" commit message would be to
introduce a sibling to --fixup and --squash that you could call --drop.

Ciao,
Johannes

Sounds rather cool. Or alternatively

git revert --squash deadbeef

On Fri, Apr 08, 2016 at 01:13:51PM +0200, Johannes Schindelin wrote:

Hi Michael,

On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:

quoted

On Thu, Apr 07, 2016 at 05:23:09PM +0200, Johannes Schindelin wrote:

quoted

On Thu, 7 Apr 2016, Michael S. Tsirkin wrote:

quoted

Reverts can typically be treated like squash.  Eliminating both the
original commit and the revert would be even nicer, but this seems a bit
harder to implement.

Whoa. This rings a lot of alarm bells, very loudly.

Whoa don't be alarmed. It's just a patch :).

It's just a patch. Like every major breakage would be. So: no, there is
reason to be alarmed if it is likely to disrupt normal usage.

quoted

quoted

It seems you intend to introduce a *major* change in behavior,

Doing this automatically for all users might be a bit too drastic for
the upstream git.

That is a pretty safe thing to say, even without the subjunctive.

quoted

If there's a commit later followed by a revert, history can be
simplified by squashing them, and if the result is empty, removing both.

True. But that is not what the user told Git to do. If the user's
intention was to squash the reverting patch, she could have easily done
this:

	git revert -n deadbeef
	git commit --squash deadbeef

where "deadbeef" is the placeholder for the actual commit to revert.

And indeed, I use exactly this song and dance quite frequently, *iff* my
intention is to drop a patch.

Well then you have to decide whether you want to drop it
when you commit.
If *I* want do drop the patch when I commit, I just do
git rebase.

A much better idea than co-opting the "Revert" commit message would be to
introduce a sibling to --fixup and --squash that you could call --drop.

Ciao,
Johannes

Maybe but it's a different usecase.

What this addresses is a case where you first wanted to
avoid rebases, so you reverted.
But then you rebased after all.
Now finding what was reverted automatically is helpful.