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

[Michael Haggerty <mhagger@alum.mit.edu>]

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(-)