[PATCHv3 0/2] Add a "fixup" command to "rebase --interactive"
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Dec 7, 2009, 09:20 UTC
- Message-ID
- <cover.1260177312.git.mhagger@alum.mit.edu>
- In-Reply-To
- <7vskbn9s1k.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
Show 21 quoted lines
> 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 :-).
Show 32 quoted lines
>> + 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(-)