From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Pimlott Subject: Re: [PATCH] rebase -i: fixup fixup! fixup! Date: Wed, 26 Jun 2013 17:20:32 -0700 Message-ID: <1372291877-sup-8201@pimlott.net> References: <20130611180530.GA18488@oinkpad.pimlott.net> <87obbc8otc.fsf@hexa.v.cablecom.net> <1371237209-sup-639@pimlott.net> <1371278908-sup-1930@pimlott.net> <7vk3lvlmat.fsf@alter.siamese.dyndns.org> <87ip1e2tzx.fsf@hexa.v.cablecom.net> <7v7ghtjwbb.fsf@alter.siamese.dyndns.org> <8738shi2ht.fsf@linux-k42r.v.cablecom.net> <7vwqpshkxj.fsf@alter.siamese.dyndns.org> <1372190294-sup-1398@pimlott.net> <7v61x127bw.fsf@alter.siamese.dyndns.org> <1372198415-sup-2114@pimlott.net> <1372283778-sup-5704@pimlott.net> <7vd2r8v3fa.fsf@alter.siamese.dyndns.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Thomas Rast , git To: Junio C Hamano X-From: git-owner@vger.kernel.org Thu Jun 27 02:20:47 2013 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Urzx7-00041F-GV for gcvg-git-2@plane.gmane.org; Thu, 27 Jun 2013 02:20:46 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752222Ab3F0AUl (ORCPT ); Wed, 26 Jun 2013 20:20:41 -0400 Received: from pimlott.net ([72.249.23.100]:47979 "EHLO fugue.pimlott.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751663Ab3F0AUk (ORCPT ); Wed, 26 Jun 2013 20:20:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=pimlott.net; s=default; h=Content-Transfer-Encoding:Message-Id:Date:References:In-reply-to:To:From:Subject:Cc:Content-Type; bh=BGe+MS45nMDyTMLmtaLcw1/s0kwqji2G0yqjRozEV8Y=; b=oKdgvQJ1zR1gzwKnfKz3204tnlZsPbCMlBkkmvw8IrXVJhNHJXL6syI/hEK3f1xUIT9DZA0SEIW3zfNW3XDgOGu3dcDpEFLtHhVhyI4lW1Fo87ROzRrGJKZhBpCdS2BMH1xLd/V145lFdC1Enbk0co8xkZAcvFmFLaf4+Pcc01M=; Received: from andrew by fugue.pimlott.net with local (Exim 4.72) (envelope-from ) id 1Urzwu-0001uD-GU; Wed, 26 Jun 2013 17:20:32 -0700 In-reply-to: <7vd2r8v3fa.fsf@alter.siamese.dyndns.org> User-Agent: Sup/git Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: Excerpts from Junio C Hamano's message of Wed Jun 26 16:48:57 -0700 2013: > Andrew Pimlott writes: > > In order to test this, I wrote a helper function to dump the rebase -i > > todo list. Would you like this introduced in its own patch, or > > combined? See below. > > Depends on how involved the addition of the tests that actually use > the helper, but in general it would be a good idea to add it in the > first patch that actually uses it. Unused code added in a separate > patch will not point at that patch when bisecting, if that unused > code was broken from the beginning (not that I see anything > immediately broken in the code the following adds). Ok, here is the complete commit, incorporating all feedback. Andrew ---8<--- Subject: [PATCH 1/3] rebase -i: handle fixup! fixup! in --autosquash In rebase -i --autosquash, ignore all "fixup! " or "squash! " after the first. This supports the case when a git commit --fixup/--squash referred to an earlier fixup/squash instead of the original commit (whether intentionally, as when the user expressly meant to note that the commit fixes an earlier fixup; or inadvertently, as when the user meant to refer to the original commit with :/msg; or out of laziness, as when the user could remember how to refer to the fixup but not the original). In the todo list, the full commit message is preserved, in case it provides useful cues to the user. A test helper set_cat_todo_editor is introduced to check this. Helped-by: Thomas Rast Helped-by: Junio C Hamano Signed-off-by: Andrew Pimlott --- Documentation/git-rebase.txt | 4 ++- git-rebase--interactive.sh | 25 ++++++++++++++---- t/lib-rebase.sh | 14 +++++++++++ t/t3415-rebase-autosquash.sh | 57 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 6 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index c84854a..6b2e1c8 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -389,7 +389,9 @@ squash/fixup series. the same ..., automatically modify the todo list of rebase -i so that the commit marked for squashing comes right after the commit to be modified, and change the action of the moved - commit from `pick` to `squash` (or `fixup`). + commit from `pick` to `squash` (or `fixup`). Ignores subsequent + "fixup! " or "squash! " after the first, in case you referred to an + earlier fixup/squash with `git commit --fixup/--squash`. + This option is only valid when the '--interactive' option is used. + diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index f953d8d..169e876 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -689,8 +689,22 @@ rearrange_squash () { case "$message" in "squash! "*|"fixup! "*) action="${message%%!*}" - rest="${message#*! }" - echo "$sha1 $action $rest" + rest=$message + prefix= + # skip all squash! or fixup! (but save for later) + while : + do + case "$rest" in + "squash! "*|"fixup! "*) + prefix="$prefix${rest%%!*}," + rest="${rest#*! }" + ;; + *) + break + ;; + esac + done + echo "$sha1 $action $prefix $rest" # if it's a single word, try to resolve to a full sha1 and # emit a second copy. This allows us to match on both message # and on sha1 prefix @@ -699,7 +713,7 @@ rearrange_squash () { if test -n "$fullsha"; then # prefix the action to uniquely identify this line as # intended for full sha1 match - echo "$sha1 +$action $fullsha" + echo "$sha1 +$action $prefix $fullsha" fi fi esac @@ -714,7 +728,7 @@ rearrange_squash () { esac printf '%s\n' "$pick $sha1 $message" used="$used$sha1 " - while read -r squash action msg_content + while read -r squash action msg_prefix msg_content do case " $used" in *" $squash "*) continue ;; @@ -730,7 +744,8 @@ rearrange_squash () { case "$message" in "$msg_content"*) emit=1;; esac ;; esac if test $emit = 1; then - printf '%s\n' "$action $squash $action! $msg_content" + real_prefix=$(echo "$msg_prefix" | sed "s/,/! /g") + printf '%s\n' "$action $squash ${real_prefix}$msg_content" used="$used$squash " fi done <"$1.sq" diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 4b74ae4..be5a4c7 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -66,6 +66,20 @@ EOF chmod a+x fake-editor.sh } +# After set_cat_todo_editor, rebase -i will write the todo list (ignoring +# blank lines and comments) to stdout, and exit failure (so you should run +# it with test_must_fail). This can be used to verify the expected user +# experience, for todo list changes that do not affect the outcome of +# rebase; or as an extra check in addition to checking the outcome. + +set_cat_todo_editor () { + write_script fake-editor.sh <<\EOF +grep "^[^#]" "$1" +exit 1 +EOF + test_set_editor "$(pwd)/fake-editor.sh" +} + # checks that the revisions in "$2" represent a linear range with the # subjects in "$1" test_linear_range () { diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index a1e86c4..7c989f9 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh @@ -4,6 +4,8 @@ test_description='auto squash' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-rebase.sh + test_expect_success setup ' echo 0 >file0 && git add . && @@ -193,4 +195,59 @@ test_expect_success 'use commit --squash' ' test_auto_commit_flags squash 2 ' +test_auto_fixup_fixup () { + git reset --hard base && + echo 1 >file1 && + git add -u && + test_tick && + git commit -m "$1! first" && + echo 2 >file1 && + git add -u && + test_tick && + git commit -m "$1! $2! first" && + git tag "final-$1-$2" && + test_tick && + ( + set_cat_todo_editor && + test_must_fail git rebase --autosquash -i HEAD^^^^ >actual && + cat >expected <actual && + test_line_count = 3 actual + git diff --exit-code "final-$1-$2" && + test 2 = "$(git cat-file blob HEAD^:file1)" && + if test "$1" = "fixup" + then + test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) + elif test "$1" = "squash" + then + test 3 = $(git cat-file commit HEAD^ | grep first | wc -l) + else + false + fi +} + +test_expect_success 'fixup! fixup!' ' + test_auto_fixup_fixup fixup fixup +' + +test_expect_success 'fixup! squash!' ' + test_auto_fixup_fixup fixup squash +' + +test_expect_success 'squash! squash!' ' + test_auto_fixup_fixup squash squash +' + +test_expect_success 'squash! fixup!' ' + test_auto_fixup_fixup squash fixup +' + test_done -- 1.7.10.4