[PATCHv2 0/2] Add a "fixup" command to "rebase --interactive"

[PATCHv2 0/2] Add a "fixup" command to "rebase --interactive"

[Michael Haggerty]

Thanks, everybody, for all of the feedback.  This is the redone patch
series, which I think addresses all of your suggestions.

I would still like to implement "don't open the editor if there are
only fixups", but if it's OK I'll work on that in a separate patch
series when I have time.

Michael J Gruber wrote:
> My first reaction to the subject was "Huh? What repository?". So I
> suggest something like
> 
> t3404: Better document the original repository layout
> 
> as a more descriptive subject.

Good idea.  Done.

Johannes Schindelin wrote:
> Alternatively, one could use the test_commit function, I guess. [...]

Yes, that's much easier.  Changed.  This makes the old first and
second patches reduce to a single one.

Johannes Schindelin wrote:
> On Fri, 4 Dec 2009, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
>>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>>> index 0bd3bf7..539413d 100755
>>> --- a/git-rebase--interactive.sh
>>> +++ b/git-rebase--interactive.sh
>>> @@ -302,7 +302,7 @@ nth_string () {
>>>  
>>>  make_squash_message () {
>>>  	if test -f "$SQUASH_MSG"; then
>>> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
>>> +		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
>>>  			< "$SQUASH_MSG" | sed -ne '$p')+1))
>> This sed replacement worries me.  I don't have a time to check myself
>> today but do we use \(this\|or\|that\) alternates with our sed script
>> already elsewhere in the codebase (test scripts do not count)?
>>
>> Otherwise this may suddenly be breaking a platform that has an
>> implementation of sed that may be substandard but so far has been
>> sufficient to work with git.
> 
> IIRC "|" was not correctly handled by BSD sed (used e.g. in MacOSX).
> 
> So maybe it would be best to just look for "commit message"?  I agree with
> Michael that the regex should not be too loose.

Thanks for pointing this out.  I replaced the problematic part of the
regexp with "[snrt][tdh]", which isn't quite so selective but should
be portable.  (This is the same fix that Junio suggested.)

Björn Gustavsson wrote:
> On Fri, Dec 4, 2009 at 3:36 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> Nitpick: the recommended style is to leave out the full stop
> at the end of the first line of the commit message.

Nit picked.

Junio C Hamano wrote:
> IIRC, the end result of the bikeshedding for the name of the command was
> won by Dscho's "fixup":
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/127923/focus=121820

That's fine with me (the abbreviation is even the same :-) ).
Changed.

Michael

Michael Haggerty (2):
  t3404: Use test_commit to set up test repository
  Add a command "fixup" to rebase --interactive

 Documentation/git-rebase.txt  |   13 +++--
 git-rebase--interactive.sh    |   51 +++++++++++++++++----
 t/lib-rebase.sh               |    7 ++-
 t/t3404-rebase-interactive.sh |   96 +++++++++++++++++++++-------------------
 4 files changed, 103 insertions(+), 64 deletions(-)

[PATCHv2 1/2] t3404: Use test_commit to set up test repository

[Michael Haggerty]

Also adjust "expected" text to reflect the file contents generated by
test_commit, which are slightly different than those generated by the
old code.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 t/t3404-rebase-interactive.sh |   66 ++++++++++++----------------------------
 1 files changed, 20 insertions(+), 46 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3a37793..778daf4 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -16,53 +16,26 @@ set_fake_editor
 
 # set up two branches like this:
 #
-# A - B - C - D - E
+# A - B - C - D - E     (master)
 #   \
-#     F - G - H
+#     F - G - H         (branch1)
 #       \
-#         I
+#         I             (branch2)
 #
-# where B, D and G touch the same file.
+# where A, B, D and G touch the same file.
 
 test_expect_success 'setup' '
-	: > file1 &&
-	git add file1 &&
-	test_tick &&
-	git commit -m A &&
-	git tag A &&
-	echo 1 > file1 &&
-	test_tick &&
-	git commit -m B file1 &&
-	: > file2 &&
-	git add file2 &&
-	test_tick &&
-	git commit -m C &&
-	echo 2 > file1 &&
-	test_tick &&
-	git commit -m D file1 &&
-	: > file3 &&
-	git add file3 &&
-	test_tick &&
-	git commit -m E &&
+	test_commit A file1 &&
+	test_commit B file1 &&
+	test_commit C file2 &&
+	test_commit D file1 &&
+	test_commit E file3 &&
 	git checkout -b branch1 A &&
-	: > file4 &&
-	git add file4 &&
-	test_tick &&
-	git commit -m F &&
-	git tag F &&
-	echo 3 > file1 &&
-	test_tick &&
-	git commit -m G file1 &&
-	: > file5 &&
-	git add file5 &&
-	test_tick &&
-	git commit -m H &&
+	test_commit F file4 &&
+	test_commit G file1 &&
+	test_commit H file5 &&
 	git checkout -b branch2 F &&
-	: > file6 &&
-	git add file6 &&
-	test_tick &&
-	git commit -m I &&
-	git tag I
+	test_commit I file6
 '
 
 test_expect_success 'no changes are a nop' '
@@ -111,19 +84,20 @@ test_expect_success 'exchange two commits' '
 
 cat > expect << EOF
 diff --git a/file1 b/file1
-index e69de29..00750ed 100644
+index f70f10e..fd79235 100644
 --- a/file1
 +++ b/file1
-@@ -0,0 +1 @@
-+3
+@@ -1 +1 @@
+-A
++G
 EOF
 
 cat > expect2 << EOF
 <<<<<<< HEAD
-2
+D
 =======
-3
->>>>>>> b7ca976... G
+G
+>>>>>>> 91201e5... G
 EOF
 
 test_expect_success 'stop on conflicting pick' '
-- 
1.6.6.rc1.34.gd7b83

[PATCHv2 2/2] Add a command "fixup" to rebase --interactive

[Michael Haggerty]

The command is like "squash", except that it discards the commit message
of the corresponding commit.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 Documentation/git-rebase.txt  |   13 ++++++----
 git-rebase--interactive.sh    |   51 +++++++++++++++++++++++++++++++++--------
 t/lib-rebase.sh               |    7 +++--
 t/t3404-rebase-interactive.sh |   30 ++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index ca5e1e8..9b648ec 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -382,9 +382,12 @@ If you just want to edit the commit message for a commit, replace the
 command "pick" with the command "reword".
 
 If you want to fold two or more commits into one, replace the command
-"pick" with "squash" for the second and subsequent commit.  If the
-commits had different authors, it will attribute the squashed commit to
-the author of the first commit.
+"pick" for the second and subsequent commits with "squash" or "fixup".
+If the commits had different authors, the folded commit will be
+attributed to the author of the first commit.  The suggested commit
+message for the folded commit is the concatenation of the commit
+messages of the first commit and of those with the "squash" command,
+but omits the commit messages of commits with the "fixup" command.
 
 'git-rebase' will stop when "pick" has been replaced with "edit" or
 when a command fails due to merge errors. When you are done editing
@@ -512,8 +515,8 @@ Easy case: The changes are literally the same.::
 Hard case: The changes are not the same.::
 
 	This happens if the 'subsystem' rebase had conflicts, or used
-	`\--interactive` to omit, edit, or squash commits; or if the
-	upstream used one of `commit \--amend`, `reset`, or
+	`\--interactive` to omit, edit, squash, or fixup commits; or
+	if the upstream used one of `commit \--amend`, `reset`, or
 	`filter-branch`.
 
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0bd3bf7..a7de5ea 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -302,7 +302,13 @@ nth_string () {
 
 make_squash_message () {
 	if test -f "$SQUASH_MSG"; then
-		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
+		# We want to be careful about matching only the commit
+		# message comment lines generated by this function.
+		# But supposedly some sed versions don't handle "\|"
+		# correctly, so instead of "\(st\|nd\|rd\|th\)", use
+		# the less accurate "[snrt][tdh]" to match the
+		# nth_string endings.
+		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)[snrt][tdh] commit message.*:/\1/p" \
 			< "$SQUASH_MSG" | sed -ne '$p')+1))
 		echo "# This is a combination of $COUNT commits."
 		sed -e 1d -e '2,/^./{
@@ -315,10 +321,26 @@ make_squash_message () {
 		echo
 		git cat-file commit HEAD | sed -e '1,/^$/d'
 	fi
-	echo
-	echo "# This is the $(nth_string $COUNT) commit message:"
-	echo
-	git cat-file commit $1 | sed -e '1,/^$/d'
+	case $1 in
+	squash)
+		echo
+		echo "# This is the $(nth_string $COUNT) commit message:"
+		echo
+		git cat-file commit $2 | sed -e '1,/^$/d'
+		;;
+	fixup)
+		echo
+		echo "# The $(nth_string $COUNT) commit message will be skipped:"
+		echo
+		# Comment the lines of the commit message out using
+		# "#" rather than "# " (a) to make them more distinct
+		# from the explanatory comments added by this function
+		# and (b) to make it less likely that the sed regexp
+		# above will be confused by a commented-out commit
+		# message.
+		git cat-file commit $2 | sed -e '1,/^$/d' -e 's/^/#/'
+		;;
+	esac
 }
 
 peek_next_command () {
@@ -367,20 +389,28 @@ do_next () {
 		warn
 		exit 0
 		;;
-	squash|s)
-		comment_for_reflog squash
+	squash|s|fixup|f)
+		case "$command" in
+		squash|s)
+			squash_style=squash
+			;;
+		fixup|f)
+			squash_style=fixup
+			;;
+		esac
+		comment_for_reflog $squash_style
 
 		test -f "$DONE" && has_action "$DONE" ||
-			die "Cannot 'squash' without a previous commit"
+			die "Cannot '$squash_style' without a previous commit"
 
 		mark_action_done
-		make_squash_message $sha1 > "$MSG"
+		make_squash_message $squash_style $sha1 > "$MSG"
 		failed=f
 		author_script=$(get_author_ident_from_commit HEAD)
 		output git reset --soft HEAD^
 		pick_one -n $sha1 || failed=t
 		case "$(peek_next_command)" in
-		squash|s)
+		squash|s|fixup|f)
 			USE_OUTPUT=output
 			MSG_OPT=-F
 			EDIT_OR_FILE="$MSG"
@@ -768,6 +798,7 @@ first and then run 'git rebase --continue' again."
 #  r, reword = use commit, but edit the commit message
 #  e, edit = use commit, but stop for amending
 #  s, squash = use commit, but meld into previous commit
+#  f, fixup = like "squash", but discard this commit's log message
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62f452c..f4dda02 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -9,8 +9,9 @@
 #
 #	"[<lineno1>] [<lineno2>]..."
 #
-#   If a line number is prefixed with "squash", "edit", or "reword", the
-#   respective line's command will be replaced with the specified one.
+#   If a line number is prefixed with "squash", "fixup", "edit", or
+#   "reword", the respective line's command will be replaced with the
+#   specified one.
 
 set_fake_editor () {
 	echo "#!$SHELL_PATH" >fake-editor.sh
@@ -32,7 +33,7 @@ cat "$1".tmp
 action=pick
 for line in $FAKE_LINES; do
 	case $line in
-	squash|edit|reword)
+	squash|fixup|edit|reword)
 		action="$line";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 778daf4..ea26115 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -235,6 +235,36 @@ test_expect_success 'multi-squash only fires up editor once' '
 	test 1 = $(git show | grep ONCE | wc -l)
 '
 
+test_expect_success 'multi-fixup only fires up editor once' '
+	git checkout -b multi-fixup E &&
+	base=$(git rev-parse HEAD~4) &&
+	FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 fixup 2 fixup 3 fixup 4" \
+		git rebase -i $base &&
+	test $base = $(git rev-parse HEAD^) &&
+	test 1 = $(git show | grep ONCE | wc -l) &&
+	git checkout to-be-rebased &&
+	git branch -D multi-fixup
+'
+
+cat > expect-squash-fixup << EOF
+B
+
+D
+
+ONCE
+EOF
+
+test_expect_success 'squash and fixup generate correct log messages' '
+	git checkout -b squash-fixup E &&
+	base=$(git rev-parse HEAD~4) &&
+	FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 fixup 2 squash 3 fixup 4" \
+		git rebase -i $base &&
+	git cat-file commit HEAD | sed -e 1,/^\$/d > actual-squash-fixup &&
+	test_cmp expect-squash-fixup actual-squash-fixup &&
+	git checkout to-be-rebased &&
+	git branch -D squash-fixup
+'
+
 test_expect_success 'squash works as expected' '
 	for n in one two three four
 	do
-- 
1.6.6.rc1.34.gd7b83

Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0bd3bf7..a7de5ea 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -302,7 +302,13 @@ nth_string () {
>  
>  make_squash_message () {
>  	if test -f "$SQUASH_MSG"; then
> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
> +		# We want to be careful about matching only the commit
> +		# message comment lines generated by this function.

> +		# But supposedly some sed versions don't handle "\|"
> +		# correctly, so instead of "\(st\|nd\|rd\|th\)", use
> +		# the less accurate "[snrt][tdh]" to match the
> +		# nth_string endings.

I'd drop this comment; blaming POSIX-compliant sed without GNU extension
is simply wrong.

> +		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)[snrt][tdh] commit message.*:/\1/p" \
>  			< "$SQUASH_MSG" | sed -ne '$p')+1))
>  		echo "# This is a combination of $COUNT commits."
>  		sed -e 1d -e '2,/^./{
> @@ -315,10 +321,26 @@ make_squash_message () {
>  		echo
>  		git cat-file commit HEAD | sed -e '1,/^$/d'
>  	fi
> -	echo
> -	echo "# This is the $(nth_string $COUNT) commit message:"
> -	echo
> -	git cat-file commit $1 | sed -e '1,/^$/d'
> +	case $1 in
> +	squash)
> +		echo
> +		echo "# This is the $(nth_string $COUNT) commit message:"
> +		echo
> +		git cat-file commit $2 | sed -e '1,/^$/d'
> +		;;
> +	fixup)
> +		echo
> +		echo "# The $(nth_string $COUNT) commit message will be skipped:"
> +		echo
> +		# Comment the lines of the commit message out using
> +		# "#" rather than "# " (a) to make them more distinct
> +		# from the explanatory comments added by this function
> +		# and (b) to make it less likely that the sed regexp
> +		# above will be confused by a commented-out commit
> +		# message.

Use "#  " as prefix and you won't have to worry about a line in the log
message that begins with " Th", no?

In any case, I agree that a comment like this is necessary to warn anybody
who will be touching the code that the COUNT=$((...))  needs to avoid
matching what is produced here, but I find the above 6-line comment a bit
too excessive.

[PATCHv3 0/2] Add a "fixup" command to "rebase --interactive"

[Michael Haggerty]

Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index 0bd3bf7..a7de5ea 100755
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -302,7 +302,13 @@ nth_string () {
>>  
>>  make_squash_message () {
>>  	if test -f "$SQUASH_MSG"; then
>> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
>> +		# We want to be careful about matching only the commit
>> +		# message comment lines generated by this function.
> 
>> +		# But supposedly some sed versions don't handle "\|"
>> +		# correctly, so instead of "\(st\|nd\|rd\|th\)", use
>> +		# the less accurate "[snrt][tdh]" to match the
>> +		# nth_string endings.
> 
> I'd drop this comment; blaming POSIX-compliant sed without GNU extension
> is simply wrong.

Fair enough.  I hope you don't mind my leaving a line explaining the
cryptic "[snrt][tdh]" to save Dscho a couple of seconds next time :-).

>> +		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)[snrt][tdh] commit message.*:/\1/p" \
>>  			< "$SQUASH_MSG" | sed -ne '$p')+1))
>>  		echo "# This is a combination of $COUNT commits."
>>  		sed -e 1d -e '2,/^./{
>> @@ -315,10 +321,26 @@ make_squash_message () {
>>  		echo
>>  		git cat-file commit HEAD | sed -e '1,/^$/d'
>>  	fi
>> -	echo
>> -	echo "# This is the $(nth_string $COUNT) commit message:"
>> -	echo
>> -	git cat-file commit $1 | sed -e '1,/^$/d'
>> +	case $1 in
>> +	squash)
>> +		echo
>> +		echo "# This is the $(nth_string $COUNT) commit message:"
>> +		echo
>> +		git cat-file commit $2 | sed -e '1,/^$/d'
>> +		;;
>> +	fixup)
>> +		echo
>> +		echo "# The $(nth_string $COUNT) commit message will be skipped:"
>> +		echo
>> +		# Comment the lines of the commit message out using
>> +		# "#" rather than "# " (a) to make them more distinct
>> +		# from the explanatory comments added by this function
>> +		# and (b) to make it less likely that the sed regexp
>> +		# above will be confused by a commented-out commit
>> +		# message.
> 
> Use "#  " as prefix and you won't have to worry about a line in the log
> message that begins with " Th", no?

The scenario seems pretty unlikely to me.  The line would not only
have to start with " Th" but also match the rest of the regexp.  And
as far as I can see, the only ill effect of a mismatch would be to
throw off COUNT, which itself is only used in the instruction
comments.

By the way, if you are really worried about accidental matches to the
regexp, this is not the end of the story.  It could be that a squashed
commit's log message has lines that match the regexp; e.g.,

    commit -m '# This is my 380th commit message today:'

The commented-out line survives this first commit and would confuse an
interactive squash (with or without my patch).

Amusingly, if you want to indicate at commit time that a commit
message should be omitted at rebase time, you can add a comment
character intentionally:

    commit -m '# this comment will be stripped out at the next squash'

This peculiarity also has nothing to do with the new "fixup" feature.

> In any case, I agree that a comment like this is necessary to warn anybody
> who will be touching the code that the COUNT=$((...))  needs to avoid
> matching what is produced here, but I find the above 6-line comment a bit
> too excessive.

I have substituted a terser alternative.  Feel free to edit the
comment or delete it altogether.

Michael


Michael Haggerty (2):
  t3404: Use test_commit to set up test repository
  Add a command "fixup" to rebase --interactive

 Documentation/git-rebase.txt  |   13 +++--
 git-rebase--interactive.sh    |   45 +++++++++++++++----
 t/lib-rebase.sh               |    7 ++-
 t/t3404-rebase-interactive.sh |   96 +++++++++++++++++++++-------------------
 4 files changed, 97 insertions(+), 64 deletions(-)

[PATCHv3 1/2] t3404: Use test_commit to set up test repository

[Michael Haggerty]

Also adjust "expected" text to reflect the file contents generated by
test_commit, which are slightly different than those generated by the
old code.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 t/t3404-rebase-interactive.sh |   66 ++++++++++++----------------------------
 1 files changed, 20 insertions(+), 46 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3a37793..778daf4 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -16,53 +16,26 @@ set_fake_editor
 
 # set up two branches like this:
 #
-# A - B - C - D - E
+# A - B - C - D - E     (master)
 #   \
-#     F - G - H
+#     F - G - H         (branch1)
 #       \
-#         I
+#         I             (branch2)
 #
-# where B, D and G touch the same file.
+# where A, B, D and G touch the same file.
 
 test_expect_success 'setup' '
-	: > file1 &&
-	git add file1 &&
-	test_tick &&
-	git commit -m A &&
-	git tag A &&
-	echo 1 > file1 &&
-	test_tick &&
-	git commit -m B file1 &&
-	: > file2 &&
-	git add file2 &&
-	test_tick &&
-	git commit -m C &&
-	echo 2 > file1 &&
-	test_tick &&
-	git commit -m D file1 &&
-	: > file3 &&
-	git add file3 &&
-	test_tick &&
-	git commit -m E &&
+	test_commit A file1 &&
+	test_commit B file1 &&
+	test_commit C file2 &&
+	test_commit D file1 &&
+	test_commit E file3 &&
 	git checkout -b branch1 A &&
-	: > file4 &&
-	git add file4 &&
-	test_tick &&
-	git commit -m F &&
-	git tag F &&
-	echo 3 > file1 &&
-	test_tick &&
-	git commit -m G file1 &&
-	: > file5 &&
-	git add file5 &&
-	test_tick &&
-	git commit -m H &&
+	test_commit F file4 &&
+	test_commit G file1 &&
+	test_commit H file5 &&
 	git checkout -b branch2 F &&
-	: > file6 &&
-	git add file6 &&
-	test_tick &&
-	git commit -m I &&
-	git tag I
+	test_commit I file6
 '
 
 test_expect_success 'no changes are a nop' '
@@ -111,19 +84,20 @@ test_expect_success 'exchange two commits' '
 
 cat > expect << EOF
 diff --git a/file1 b/file1
-index e69de29..00750ed 100644
+index f70f10e..fd79235 100644
 --- a/file1
 +++ b/file1
-@@ -0,0 +1 @@
-+3
+@@ -1 +1 @@
+-A
++G
 EOF
 
 cat > expect2 << EOF
 <<<<<<< HEAD
-2
+D
 =======
-3
->>>>>>> b7ca976... G
+G
+>>>>>>> 91201e5... G
 EOF
 
 test_expect_success 'stop on conflicting pick' '
-- 
1.6.5.5

[PATCHv3 2/2] Add a command "fixup" to rebase --interactive

[Michael Haggerty]

The command is like "squash", except that it discards the commit message
of the corresponding commit.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 Documentation/git-rebase.txt  |   13 +++++++----
 git-rebase--interactive.sh    |   45 +++++++++++++++++++++++++++++++---------
 t/lib-rebase.sh               |    7 +++--
 t/t3404-rebase-interactive.sh |   30 +++++++++++++++++++++++++++
 4 files changed, 77 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index ca5e1e8..9b648ec 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -382,9 +382,12 @@ If you just want to edit the commit message for a commit, replace the
 command "pick" with the command "reword".
 
 If you want to fold two or more commits into one, replace the command
-"pick" with "squash" for the second and subsequent commit.  If the
-commits had different authors, it will attribute the squashed commit to
-the author of the first commit.
+"pick" for the second and subsequent commits with "squash" or "fixup".
+If the commits had different authors, the folded commit will be
+attributed to the author of the first commit.  The suggested commit
+message for the folded commit is the concatenation of the commit
+messages of the first commit and of those with the "squash" command,
+but omits the commit messages of commits with the "fixup" command.
 
 'git-rebase' will stop when "pick" has been replaced with "edit" or
 when a command fails due to merge errors. When you are done editing
@@ -512,8 +515,8 @@ Easy case: The changes are literally the same.::
 Hard case: The changes are not the same.::
 
 	This happens if the 'subsystem' rebase had conflicts, or used
-	`\--interactive` to omit, edit, or squash commits; or if the
-	upstream used one of `commit \--amend`, `reset`, or
+	`\--interactive` to omit, edit, squash, or fixup commits; or
+	if the upstream used one of `commit \--amend`, `reset`, or
 	`filter-branch`.
 
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0bd3bf7..cd2dce1 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -302,7 +302,10 @@ nth_string () {
 
 make_squash_message () {
 	if test -f "$SQUASH_MSG"; then
-		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
+		# We want to be careful about matching only the commit
+		# message comment lines generated by this function.
+		# "[snrt][tdh]" matches the nth_string endings.
+		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)[snrt][tdh] commit message.*:/\1/p" \
 			< "$SQUASH_MSG" | sed -ne '$p')+1))
 		echo "# This is a combination of $COUNT commits."
 		sed -e 1d -e '2,/^./{
@@ -315,10 +318,23 @@ make_squash_message () {
 		echo
 		git cat-file commit HEAD | sed -e '1,/^$/d'
 	fi
-	echo
-	echo "# This is the $(nth_string $COUNT) commit message:"
-	echo
-	git cat-file commit $1 | sed -e '1,/^$/d'
+	case $1 in
+	squash)
+		echo
+		echo "# This is the $(nth_string $COUNT) commit message:"
+		echo
+		git cat-file commit $2 | sed -e '1,/^$/d'
+		;;
+	fixup)
+		echo
+		echo "# The $(nth_string $COUNT) commit message will be skipped:"
+		echo
+		# Comment the lines of the commit message out using
+		# "#" rather than "# " to make them less likely to
+		# confuse the sed regexp above.
+		git cat-file commit $2 | sed -e '1,/^$/d' -e 's/^/#/'
+		;;
+	esac
 }
 
 peek_next_command () {
@@ -367,20 +383,28 @@ do_next () {
 		warn
 		exit 0
 		;;
-	squash|s)
-		comment_for_reflog squash
+	squash|s|fixup|f)
+		case "$command" in
+		squash|s)
+			squash_style=squash
+			;;
+		fixup|f)
+			squash_style=fixup
+			;;
+		esac
+		comment_for_reflog $squash_style
 
 		test -f "$DONE" && has_action "$DONE" ||
-			die "Cannot 'squash' without a previous commit"
+			die "Cannot '$squash_style' without a previous commit"
 
 		mark_action_done
-		make_squash_message $sha1 > "$MSG"
+		make_squash_message $squash_style $sha1 > "$MSG"
 		failed=f
 		author_script=$(get_author_ident_from_commit HEAD)
 		output git reset --soft HEAD^
 		pick_one -n $sha1 || failed=t
 		case "$(peek_next_command)" in
-		squash|s)
+		squash|s|fixup|f)
 			USE_OUTPUT=output
 			MSG_OPT=-F
 			EDIT_OR_FILE="$MSG"
@@ -768,6 +792,7 @@ first and then run 'git rebase --continue' again."
 #  r, reword = use commit, but edit the commit message
 #  e, edit = use commit, but stop for amending
 #  s, squash = use commit, but meld into previous commit
+#  f, fixup = like "squash", but discard this commit's log message
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62f452c..f4dda02 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -9,8 +9,9 @@
 #
 #	"[<lineno1>] [<lineno2>]..."
 #
-#   If a line number is prefixed with "squash", "edit", or "reword", the
-#   respective line's command will be replaced with the specified one.
+#   If a line number is prefixed with "squash", "fixup", "edit", or
+#   "reword", the respective line's command will be replaced with the
+#   specified one.
 
 set_fake_editor () {
 	echo "#!$SHELL_PATH" >fake-editor.sh
@@ -32,7 +33,7 @@ cat "$1".tmp
 action=pick
 for line in $FAKE_LINES; do
 	case $line in
-	squash|edit|reword)
+	squash|fixup|edit|reword)
 		action="$line";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 778daf4..ea26115 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -235,6 +235,36 @@ test_expect_success 'multi-squash only fires up editor once' '
 	test 1 = $(git show | grep ONCE | wc -l)
 '
 
+test_expect_success 'multi-fixup only fires up editor once' '
+	git checkout -b multi-fixup E &&
+	base=$(git rev-parse HEAD~4) &&
+	FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 fixup 2 fixup 3 fixup 4" \
+		git rebase -i $base &&
+	test $base = $(git rev-parse HEAD^) &&
+	test 1 = $(git show | grep ONCE | wc -l) &&
+	git checkout to-be-rebased &&
+	git branch -D multi-fixup
+'
+
+cat > expect-squash-fixup << EOF
+B
+
+D
+
+ONCE
+EOF
+
+test_expect_success 'squash and fixup generate correct log messages' '
+	git checkout -b squash-fixup E &&
+	base=$(git rev-parse HEAD~4) &&
+	FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 fixup 2 squash 3 fixup 4" \
+		git rebase -i $base &&
+	git cat-file commit HEAD | sed -e 1,/^\$/d > actual-squash-fixup &&
+	test_cmp expect-squash-fixup actual-squash-fixup &&
+	git checkout to-be-rebased &&
+	git branch -D squash-fixup
+'
+
 test_expect_success 'squash works as expected' '
 	for n in one two three four
 	do
-- 
1.6.5.5

Re: [PATCHv3 0/2] Add a "fixup" command to "rebase --interactive"

[Johannes Schindelin]

Hi,

On Mon, 7 Dec 2009, Michael Haggerty wrote:

> Junio C Hamano wrote:
> > Michael Haggerty <mhagger@alum.mit.edu> writes:
> > 
> >> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> >> index 0bd3bf7..a7de5ea 100755
> >> --- a/git-rebase--interactive.sh
> >> +++ b/git-rebase--interactive.sh
> >> @@ -302,7 +302,13 @@ nth_string () {
> >>  
> >>  make_squash_message () {
> >>  	if test -f "$SQUASH_MSG"; then
> >> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
> >> +		# We want to be careful about matching only the commit
> >> +		# message comment lines generated by this function.
> > 
> >> +		# But supposedly some sed versions don't handle "\|"
> >> +		# correctly, so instead of "\(st\|nd\|rd\|th\)", use
> >> +		# the less accurate "[snrt][tdh]" to match the
> >> +		# nth_string endings.
> > 
> > I'd drop this comment; blaming POSIX-compliant sed without GNU extension
> > is simply wrong.
> 
> Fair enough.  I hope you don't mind my leaving a line explaining the
> cryptic "[snrt][tdh]" to save Dscho a couple of seconds next time :-).

Thanks, very much appreciated here.

My ACK for the patches is still valid.

Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive

[Sverre Rabbelier]

Heya,

On Mon, Dec 7, 2009 at 05:22, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> The command is like "squash", except that it discards the commit message
> of the corresponding commit.

No no, wait, wasn't "fixup" supposed to let you just edit the commit
message of the commit you're fixing up? :(

-- 
Cheers,

Sverre Rabbelier

Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive

[Michael Haggerty]

Sverre Rabbelier wrote:
> On Mon, Dec 7, 2009 at 05:22, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> The command is like "squash", except that it discards the commit message
>> of the corresponding commit.
> 
> No no, wait, wasn't "fixup" supposed to let you just edit the commit
> message of the commit you're fixing up? :(

That command is called "reword".

Michael

Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive

[Matthieu Moy]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Mon, Dec 7, 2009 at 05:22, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> The command is like "squash", except that it discards the commit message
>> of the corresponding commit.
>
> No no, wait, wasn't "fixup" supposed to let you just edit the commit
> message of the commit you're fixing up? :(

This is "reword", already in Git (6741aa6c399dec3d8f0b2, Wed Oct 7
08:13:23 2009).

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive

[Sverre Rabbelier]

Heya,

On Mon, Dec 7, 2009 at 12:57, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> This is "reword", already in Git (6741aa6c399dec3d8f0b2, Wed Oct 7
> 08:13:23 2009).

Oh, wow, I've been missing out then :D. Thanks both!

-- 
Cheers,

Sverre Rabbelier

Re: [PATCHv3 1/2] t3404: Use test_commit to set up test repository

[Junio C Hamano]

Thanks; will re-queue [2/2] as [1/2] seems to be unchanged.