Re: [PATCH] rebase -i: auto-squash commits

5 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] rebase -i: auto-squash commits

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:57

Nanako Shiraishi [off-list ref] writes:

      pick 1/3 Clean up ...
      pick 2/3 Lay the groundwork
      pick 3/3 Implement
      pick 4/3 squash to "clean up"
  
  that I'll change to 
  
      pick 1/3 Clean up ...
      squash 4/3 squash to "clean up"
      pick 2/3 Lay the groundwork
      pick 3/3 Implement
  
  and then I'll need to edit the commit message for the first two combined.

How about this patch?  It does not let you say 'squash to "clean up"'
but other people who are more skillfull than me can enhance such details.

I have to admit that I wished to see something like this for more than
once.  It would have been nicer if the patch went one step further and did
"squash the patch, but use the log message from the commit that is
squashed into, without even asking for a consolidated message", but I
think it is a reasonable start.

But as Dscho already objected to, this is a new feature that is
potentially dangerous --- there is a risk of matching a commit that was
not intended for squashing, albeit small.  We may want an explicit option
to enable it.  On the other hand, you may be able to argue that use of
"interactive" rebase is already a sign that the user is likely to want
such a convenience, though.

quoted hunk

 git-rebase--interactive.sh   |   31 +++++++++++++++++++++++++++++++
 t/t3414-rebase-autosquash.sh |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 0 deletions(-)
 create mode 100755 t/t3414-rebase-autosquash.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f96d887..0832164 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh

@@ -482,6 +482,35 @@ get_saved_options () {
 	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
 }
 
+# Rearrange the todo list that has both "pick sha1 msg" and
+# "pick sha1 squash to msg" in it, so that the latter comes
+# immediately after the former, and change "pick" to "squash".
+rearrange_squash () {
+	sed -n -e 's/^pick \([0-9a-f]*\) squash to /\1 /p' "$1" >"$1.sq"
+	test -s "$1.sq" || return
+
+	used=
+	while read pick sha1 message
+	do
+		case " $used" in
+		*" $sha1 "*) continue ;;
+		esac
+		echo "$pick $sha1 $message"
+		while read squash msg
+		do
+			case "$message" in
+			"$msg"*)

I guess we could even loosen this "must match the leading substring
exactly" restriction if we can expose Dscho's Levenstein to Porcelain
writers.

+				echo "squash $squash to $msg"
+				used="$used$squash "
+				break
+				;;

Do you really want to break here?  What happens if I have more than one
fixup patches to the same commit?

quoted hunk

+			esac
+		done <"$1.sq"
+	done <"$1" >"$1.rearranged"
+
+	cat "$1.rearranged" >"$1"
+}
+
 while test $# != 0
 do
 	case "$1" in

@@ -746,6 +776,7 @@ first and then run 'git rebase --continue' again."
 		fi
 
 		test -s "$TODO" || echo noop >> "$TODO"
+		rearrange_squash "$TODO"
 		cat >> "$TODO" << EOF
 
 # Rebase $SHORTREVISIONS onto $SHORTONTO

diff --git a/t/t3414-rebase-autosquash.sh b/t/t3414-rebase-autosquash.sh
new file mode 100755
index 0000000..ddb0daf
--- /dev/null
+++ b/t/t3414-rebase-autosquash.sh

@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='auto squash'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo 0 > file0
+	git add .
+	test_tick
+	git commit -m "initial commit"
+	echo 0 > file1
+	echo 2 > file2
+	git add .
+	test_tick
+	git commit -m "first commit"
+	echo 3 > file3
+	git add .
+	test_tick
+	git commit -m "second commit"
+'

These tests want to be stringed together with && to catch possible
breakages during the setup.  The same for the real test below.

+test_expect_success 'auto squash' '
+	echo 1 > file1
+	git add -u
+	test_tick
+	git commit -m "squash to first"
+	git tag final
+	test_tick
+	git rebase -i HEAD^^^
+	git log --oneline >actual
+	test 3 = $(wc -l <actual) &&

Not just count, but you would want to make sure that the rewritten "first
commit" now has the desired tree ("1" instead of "0" in file1, if I am
reading the test correctly).

+	git diff --exit-code final
+'
+
+test_done

[PATCH v2] rebase -i --autosquash: auto-squash commits

From: Nanako Shiraishi <hidden>
Date: 2016-06-15 22:46:58

Teach a new option, --autosquash, to the interactive rebase.
When the commit log message begins with "!fixup ...", and there
is a commit whose title begins with the same ..., automatically
modify the todo list of rebase -i so that the commit marked for
squashing come right after the commit to be modified, and change
the action of the moved commit from pick to squash.

This will help the use case outlined in

From: Junio C Hamano [off-list ref]
Date: Wed, 17 Jun 2009 09:33:19 -0700
Subject: Re: git rebase --interactive squash/squish/fold/rollup
Message-ID: [off-list ref]

and further explained in

From: Junio C Hamano [off-list ref]
Date: Thu, 18 Jun 2009 00:54:47 -0700
Subject: Re: [PATCH] rebase -i: auto-squash commits
Message-ID: [off-list ref]

Signed-off-by: Nanako Shiraishi <redacted>
---

Changes from my yesterday's patch are as follows.

* The feature is disabled by default; the user needs to explicitly ask for it with --autosquash option.
* Squashing more than one commits to the same commit should work.
* The commit message must begin with a more magic string "!fixup" instead of "squash to".
* Commands in the test script are joined with &&.
* The test examines the content of the file to verify that the commit was correctly squashed.
* Add documentation.

Documentation/git-rebase.txt | 9 +++++++++
git-rebase--interactive.sh | 35 +++++++++++++++++++++++++++++++++++
t/t3414-rebase-autosquash.sh | 37 +++++++++++++++++++++++++++++++++++++
3 files changed, 81 insertions(+), 0 deletions(-)
create mode 100755 t/t3414-rebase-autosquash.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 26f3b7b..0c2f99e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt

@@ -293,6 +293,15 @@ OPTIONS
 	root commits will be rewritten to have <newbase> as parent
 	instead.
 
+--autosquash::
+	When the commit log message begins with "!fixup ...", and there
+	is a commit whose title begins with the same ..., automatically
+	modify the todo list of rebase -i so that the commit marked for
+	squashing come right after the commit to be modified, and change
+	the action of the moved commit from pick to squash.
++
+This option is only valid when '--interactive' option is used.
+
 include::merge-strategies.txt[]
 
 NOTES

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f96d887..6e223d5 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh

@@ -28,6 +28,7 @@ abort              abort rebasing process and restore original branch
 skip               skip current patch and continue rebasing process
 no-verify          override pre-rebase hook from stopping the operation
 root               rebase all reachable commmits up to the root(s)
+autosquash         automatically squash commits that begin with !fixup
 "
 
 . git-sh-setup

@@ -46,6 +47,7 @@ ONTO=
 VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
 REBASE_ROOT=
+AUTOSQUASH=
 
 GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
 mark the corrected paths with 'git add <paths>', and

@@ -482,6 +484,35 @@ get_saved_options () {
 	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
 }
 
+# Rearrange the todo list that has both "pick sha1 msg" and
+# "pick sha1 !fixup msg" appears in it so that the latter
+# comes immediately after the former, and change "pick" to
+# "squash".
+rearrange_squash () {
+	sed -n -e 's/^pick \([0-9a-f]*\) !fixup /\1 /p' "$1" >"$1.sq"
+	test -s "$1.sq" || return
+
+	used=
+	while read pick sha1 message
+	do
+		case " $used" in
+		*" $sha1 "*) continue ;;
+		esac
+		echo "$pick $sha1 $message"
+		while read squash msg
+		do
+			case "$message" in
+			"$msg"*)
+				echo "squash $squash !fixup $msg"
+				used="$used$squash "
+				;;
+			esac
+		done <"$1.sq"
+	done <"$1" >"$1.rearranged"
+
+	cat "$1.rearranged" >"$1"
+}
+
 while test $# != 0
 do
 	case "$1" in

@@ -587,6 +618,9 @@ first and then run 'git rebase --continue' again."
 	--root)
 		REBASE_ROOT=t
 		;;
+	--autosquash)
+		AUTOSQUASH=t
+		;;
 	--onto)
 		shift
 		ONTO=$(git rev-parse --verify "$1") ||

@@ -746,6 +780,7 @@ first and then run 'git rebase --continue' again."
 		fi
 
 		test -s "$TODO" || echo noop >> "$TODO"
+		test -n "$AUTOSQUASH" && rearrange_squash "$TODO"
 		cat >> "$TODO" << EOF
 
 # Rebase $SHORTREVISIONS onto $SHORTONTO

diff --git a/t/t3414-rebase-autosquash.sh b/t/t3414-rebase-autosquash.sh
new file mode 100755
index 0000000..161cab4
--- /dev/null
+++ b/t/t3414-rebase-autosquash.sh

@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='auto squash'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo 0 > file0 &&
+	git add . &&
+	test_tick &&
+	git commit -m "initial commit" &&
+	echo 0 > file1 &&
+	echo 2 > file2 &&
+	git add . &&
+	test_tick &&
+	git commit -m "first commit" &&
+	echo 3 > file3 &&
+	git add . &&
+	test_tick &&
+	git commit -m "second commit"
+'
+
+test_expect_success 'auto squash' '
+	echo 1 > file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "!fixup first"
+	git tag final &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final &&
+	test 1 = "$(git cat-file blob HEAD^:file1)"
+'
+
+test_done

-- 
1.6.2.GIT

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH v2] rebase -i --autosquash: auto-squash commits

From: Alex Riesen <hidden>
Date: 2016-06-15 22:46:58

2009/6/18 Nanako Shiraishi [off-list ref]:

Teach a new option, --autosquash, to the interactive rebase.
When the commit log message begins with "!fixup ...", and there

Can I suggest to rename it into "--autofixup"? Or even "--auto=!fixup"?
Just so that people have one thing less to remember.

Re: [PATCH v2] rebase -i --autosquash: auto-squash commits

From: Wincent Colaiuta <hidden>
Date: 2016-06-15 22:46:58

El 18/6/2009, a las 23:55, Nanako Shiraishi escribió:

Teach a new option, --autosquash, to the interactive rebase.
When the commit log message begins with "!fixup ...", and there
is a commit whose title begins with the same ..., automatically
modify the todo list of rebase -i so that the commit marked for
squashing come right after the commit to be modified, and change
the action of the moved commit from pick to squash.

This will help the use case outlined in

   From: Junio C Hamano [off-list ref]
   Date: Wed, 17 Jun 2009 09:33:19 -0700
   Subject: Re: git rebase --interactive squash/squish/fold/rollup
   Message-ID: [off-list ref]

Definitely a fairly common workflow for me. Faced with a sequence like  
this:

	[1/3] Cleanup
	[2/3] Lay groundwork
	[3/3] Implement feature
	[4/4] Doh! more cleanup that should have gone in [1/3]

I usually just let 4/4 stand as a separate commit with a message like:

	More cleanup of XYZ

	Ideally this should have been included in commit abcd1234,
	but wasn't noticed until too late.

Seeing as I'm not perfect, I don't necessarily spend time manipulating  
the history to make it appear that I really am perfect.

Even so, if asked to imagine an ideal workflow for this scenario, I  
don't really want a new switch for "git rebase -i", but rather the  
ability to do "git commit --amend" on a non-head commit. (I know this  
has come up on the list back in February under the subject "FEATURE  
suggestion git commit --amend <ref>".)

Basically, if you do the following:

	edit
	git add foo
	git commit -m "Cleanup"
	edit
	git add foo
	git commit -m "Lay groundwork"
	edit
	git add foo
	git commit -m "Implement feature"
	# doh! found stuff that should have gone in in step one!
	edit
	git add foo
	git commit --amend HEAD~3

My intention would be for git to actually:

	1. Create a temporary throw-away commit (without updating the HEAD)

	2. Do the equivalent of using "git rebase -i" to squash that  
temporary commit into the HEAD~3 commit, providing you with the  
opportunity to edit the adjust the commit message if necessary.

	3. In the event of failure to replay the other commits on top, you  
would want the process to dump you back where you started (same HEAD  
as before, with same changes staged in the index) and an error message  
informing you that the changes didn't apply cleanly and that you  
should use "git rebase -i" instead to walk through the process manually.

At least for me that would be the ideal interface to this kind of  
feature. I can't really see myself using these magic commit messages  
and the --autosquash switch.

However, the "FEATURE suggestion git commit --amend <ref>" thread  
caused a lot of objections to be raised. Things like:

	- what if <ref> is a merge?

	- what if there are merges between <ref> and the current HEAD?

	- what if the amendment breaks reapplication of later commits?

	- what if <ref> is not an ancestor of the current HEAD?

	- what if <ref> is part of more than one branch? (and would the user  
be confused if it was only rewritten on one branch?)

Basically as I see it, the kind of workflow being discussed here  
should only be for the simple case of amending really simple histories  
(basic topic branches) and should bail loudly if pretty much any of  
the above conditions are true.

Cheers,
Wincent

Re: [PATCH v2] rebase -i --autosquash: auto-squash commits

From: Nanako Shiraishi <hidden>
Date: 2016-06-15 22:46:58

Quoting Wincent Colaiuta [off-list ref]:

El 18/6/2009, a las 23:55, Nanako Shiraishi escribió:
...

quoted

This will help the use case outlined in

   From: Junio C Hamano [off-list ref]
   Date: Wed, 17 Jun 2009 09:33:19 -0700
   Subject: Re: git rebase --interactive squash/squish/fold/rollup
   Message-ID: [off-list ref]

Definitely a fairly common workflow for me. Faced with a sequence like
this:

	[1/3] Cleanup
	[2/3] Lay groundwork
	[3/3] Implement feature
	[4/4] Doh! more cleanup that should have gone in [1/3]

I usually just let 4/4 stand as a separate commit with a message like:

	More cleanup of XYZ

	Ideally this should have been included in commit abcd1234,
	but wasn't noticed until too late.

Seeing as I'm not perfect, I don't necessarily spend time manipulating
the history to make it appear that I really am perfect.

I don't think it is about pretending to be perfect.
If you are preparing a patch series to be reviewed, it is a minimum required courtesy to the reviewers to remove such earlier mistakes before submitting.
It is called "making your series presentable."

Even so, if asked to imagine an ideal workflow for this scenario, I
don't really want a new switch for "git rebase -i", but rather the
ability to do "git commit --amend" on a non-head commit. (I know this
has come up on the list back in February under the subject "FEATURE
suggestion git commit --amend <ref>".)

I think you didn't read the explanation by Junio (the second message I quoted) why that is only one of the options, and isn't a satisfying solution for him. He explicitly said that he doesn't want his momentum disrupted by having to go back before he finishes the series, while admitting that the way you suggest may fit other people's workflow better.

As to the extra option, I don't like it, either (my original patch didn't have it). I added it only because Johannes Schindelin objected to the patch that the feature can trigger unexpectedly.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help