Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"

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

Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:50

Matthieu Moy [off-list ref] writes:
Michael J Gruber [off-list ref] writes:
quoted
quoted
If the idea of a "fix" command is acceptable, then I would like to
implement a further convenience: if a group of commits to be folded
together includes *only* "fix" commits, then the first log message
should be used without even opening an editor.  But I would like to
get a reaction to the "fix" command in general before doing so.
I'd say that would make a useful command ("fix") even more useful, being
just the right counterpart to "reword" for trivial commit message fixes.
+1 for fix, and +1 for the "don't even launch the editor" too.
I like it, too.  Also I vaguely recall that there was a series that died
that would have allowed you to give hints to help this behaviour at the
time you make "fix-up" commits; we may want to resurrect it on top of this
feature.

Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:47:50

Hi,

On Fri, 4 Dec 2009, Junio C Hamano wrote:
Matthieu Moy [off-list ref] writes:
quoted
Michael J Gruber [off-list ref] writes:
quoted
quoted
If the idea of a "fix" command is acceptable, then I would like to
implement a further convenience: if a group of commits to be folded
together includes *only* "fix" commits, then the first log message
should be used without even opening an editor.  But I would like to
get a reaction to the "fix" command in general before doing so.
I'd say that would make a useful command ("fix") even more useful, being
just the right counterpart to "reword" for trivial commit message fixes.
+1 for fix, and +1 for the "don't even launch the editor" too.
I like it, too.  Also I vaguely recall that there was a series that died
that would have allowed you to give hints to help this behaviour at the
time you make "fix-up" commits; we may want to resurrect it on top of this
feature.
I'll just repeat this exactly one more time: it is not always possible to 
know whether you make a fix-up commit, and it is not always possible to be 
sure that you want to amend the next time you do a rebase.

So: Commit time is definitely a bad time to decide on the action in some 
future rebase event.

Ciao,
Dscho

Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"

From: Nanako Shiraishi <hidden>
Date: 2016-06-15 22:47:50

Quoting Johannes Schindelin [off-list ref]
Hi,

On Fri, 4 Dec 2009, Junio C Hamano wrote:
quoted
Matthieu Moy [off-list ref] writes:
quoted
Michael J Gruber [off-list ref] writes:
quoted
quoted
If the idea of a "fix" command is acceptable, then I would like to
implement a further convenience: if a group of commits to be folded
together includes *only* "fix" commits, then the first log message
should be used without even opening an editor.  But I would like to
get a reaction to the "fix" command in general before doing so.
I'd say that would make a useful command ("fix") even more useful, being
just the right counterpart to "reword" for trivial commit message fixes.
+1 for fix, and +1 for the "don't even launch the editor" too.
I like it, too.  Also I vaguely recall that there was a series that died
that would have allowed you to give hints to help this behaviour at the
time you make "fix-up" commits; we may want to resurrect it on top of this
feature.
I'll just repeat this exactly one more time: it is not always possible to 
know whether you make a fix-up commit, and it is not always possible to be 
sure that you want to amend the next time you do a rebase.

So: Commit time is definitely a bad time to decide on the action in some 
future rebase event.
I think Junio is referring to this thread:

  http://thread.gmane.org/gmane.comp.version-control.git/127923/focus=121874

The old patch added a convention to mark a fix-up commit 
with a special string "!fixup" and refer to which commit 
in the series it is fixing.  It added --autosquash option
to rebase--interactive that tells it to move such a commit 
to an appropriate place in the series and change its 'pick' 
to 'squash'. I think with Michael's patches, it can change 
'pick' to 'fix' instead.

I too think Michael's "fix" is a good feature, and in the 
workflow by Shawn, he knows he is fixing up an earlier 
commit, and he knows he doesn't want to add anything to 
the message by the fix-up commit when he makes that commit 
(how else would he have messages like "a", "s", or "foo").

I don't think your objection should block *others* (like
Shawn and Junio) who can decide when they make commits 
from using the feature from my old patch to make it even 
easier to clean up their topics. If *you* can't decide if 
you want to amend or not when you make a fix-up commit, you 
can leave your fix-up commits unmarked, run interactive 
rebase without the --autosquash option, and use Michael's 
'fix' manually. People who can sometimes but not always 
decide when they make commits can do the same when they 
can't.

Isn't it what Junio suggested by his "on top of this feature", 
and wouldn't that make everybody happy?

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

Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:50

Nanako Shiraishi [off-list ref] writes:
I think Junio is referring to this thread:

  http://thread.gmane.org/gmane.comp.version-control.git/127923/focus=121874
Yes, that was the one I had in mind, and it is a shame that we somehow
ended up not taking it, perhaps in an improved form, back then.  Even
today alone, I missed the "mark for later fixing/squashing and then rebase
at the end" feature twice at dayjob.
I too think Michael's "fix" is a good feature, and in the 
workflow by Shawn, he knows he is fixing up an earlier 
commit, and he knows he doesn't want to add anything to 
the message by the fix-up commit when he makes that commit 
(how else would he have messages like "a", "s", or "foo").
There is a slight distinction between "Shawn's fix-up commits have garbage
message and he does not want any part of them in the final commit message"
and "Shawn is happy with the message of the original commit whose tree
these fix-up commits are meant to correct."  He may still not be entirely
happy with the original message.  Wanting to edit the commit log message,
and not wanting to use the messages from follow-up commits, are two
different things.

I would agree that it is a good idea for "rebase -i" with only "fix" and
not "squash" to skip the editing of the final message.  You manually move,
or tell your "rebase --autosquash" option to automatically move, the
follow-up commits next to the ones they are meant to correct while editing
the rebase-i insn, and you change their "pick" to "fix" (or "fixup" as
Dscho and others suggested in the earlier round you quoted) only when you
know you want to keep the message of the original commit.  Otherwise you
can change them to "squash" not to "fix", and you can edit the final log
message that way.

If Michael rolls his second round with your "--autosquash", or you do so
yourself on top of his patch, I think it _might_ be safer to mark the ones
automatically moved as "squash", and not as "fix", and have the users
explicitly change the "squash" they want to "fix" themselves.
Alternatively, you can also use two magic tokens (i.e. instead of one
"fixup!", allow people to use "squash!" and "fixup!")  and change the
action chosen for the moved commits to "squash" and "fixup" respectively.
I don't think your objection should block *others* (like
Shawn and Junio) who can decide when they make commits 
from using the feature from my old patch to make it even 
easier to clean up their topics. If *you* can't decide if 
you want to amend or not when you make a fix-up commit, you 
can leave your fix-up commits unmarked, run interactive 
rebase without the --autosquash option, and use Michael's 
'fix' manually. People who can sometimes but not always 
decide when they make commits can do the same when they 
can't.

Isn't it what Junio suggested by his "on top of this feature", 
and wouldn't that make everybody happy?
The answer to the first question is "yes"---I did't understand why Dscho
objected to a feature he can choose not to use if he doesn't want to, and
I still don't (unless I was misreading your earlier patch back then and it
somehow forced all rebase-i users to decide upfront at the commit time,
but I highly doubt it).

I don't know about the second one but I am guessing it would also be
"yes".

Re: [PATCH 0/3] Add a "fix" command to "rebase --interactive"

From: Nanako Shiraishi <hidden>
Date: 2016-06-15 22:47:51

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.

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

 Junio C Hamano [off-list ref] writes:

 > If Michael rolls his second round with your "--autosquash", or you do so
 > yourself on top of his patch, I think it _might_ be safer to mark the ones
 > automatically moved as "squash", and not as "fix", and have the users
 > explicitly change the "squash" they want to "fix" themselves.
 > Alternatively, you can also use two magic tokens (i.e. instead of one
 > "fixup!", allow people to use "squash!" and "fixup!")  and change the
 > action chosen for the moved commits to "squash" and "fixup" respectively.

 Here is a rebased and updated version of my patch from June 
 2009. It should apply cleanly on top of Michael's patch.

 Documentation/git-rebase.txt |   10 +++++++
 git-rebase--interactive.sh   |   43 +++++++++++++++++++++++++++++++
 t/t3415-rebase-autosquash.sh |   58 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+), 0 deletions(-)
 create mode 100755 t/t3415-rebase-autosquash.sh
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9b648ec..87cb62d 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -308,6 +308,16 @@ which makes little sense.
 	root commits will be rewritten to have <newbase> as parent
 	instead.
 
+--autosquash::
+	When the commit log message begins with "!squash ..." (or
+	"!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 quashing come right after the 
+	commit to be modified, and change the action of the moved 
+	commit from `pick` to `squash` (or `fixup`).
++
+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 30de96e..b014231 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         move commits that begin with !squash/!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
@@ -519,6 +521,43 @@ 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/!squash msg" appears in it so that the latter
+# comes immediately after the former, and change "pick" to
+# "fixup"/"squash".
+rearrange_squash () {
+	sed -n -e 's/^pick \([0-9a-f]*\) !\(squash\) /\1 \2 /p' \
+		-e 's/^pick \([0-9a-f]*\) !\(fixup\) /\1 \2 /p' \
+		"$1" >"$1.sq"
+	test -s "$1.sq" || return
+
+	sed -e '/^pick [0-9a-f]* !squash /d' \
+		-e '/^pick [0-9a-f]* !fixup /d' \
+		"$1" |
+	(
+		used=
+		while read pick sha1 message
+		do
+			echo "$pick $sha1 $message"
+			while read squash action msg
+			do
+				case " $used" in
+				*" $squash "*)
+					continue ;;
+				esac
+				case "$message" in
+				"$msg"*)
+					echo "$action $squash !$action $msg"
+					used="$used$squash "
+					;;
+				esac
+			done <"$1.sq"
+		done >"$1.rearranged"
+	)
+	cat "$1.rearranged" >"$1"
+	rm -f "$1.sq"
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -624,6 +663,9 @@ first and then run 'git rebase --continue' again."
 	--root)
 		REBASE_ROOT=t
 		;;
+	--autosquash)
+		AUTOSQUASH=t
+		;;
 	--onto)
 		shift
 		ONTO=$(git rev-parse --verify "$1") ||
@@ -783,6 +825,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/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
new file mode 100755
index 0000000..5ea2073
--- /dev/null
+++ b/t/t3415-rebase-autosquash.sh
@@ -0,0 +1,58 @@
+#!/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" &&
+	git tag base
+'
+
+test_expect_success 'auto fixup' '
+	git reset --hard base &&
+	echo 1 > file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "!fixup first"
+
+	git tag final-fixup &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final-fixup &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)
+'
+
+test_expect_success 'auto squash' '
+	git reset --hard base &&
+	echo 1 > file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "!squash first"
+
+	git tag final-squash &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final-squash &&
+	test 1 = "$(git cat-file blob HEAD^:file1)" &&
+	test 2 = $(git cat-file commit HEAD^ | grep first | wc -l)
+'
+
+test_done
-- 
1.6.6.rc0.60.g4926




-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help