[PATCH] t7500: fix flipped actual/expect
Signed-off-by: Andrew Pimlott and...@pimlott.net --- t/t7500-commit.sh |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh index 436b7b6..e166ac8 100755 --- a/t/t7500-commit.sh +++ b/t/t7500-commit.sh @@ -13,8 +13,8 @@ commit_msg_is () { expect=commit_msg_is.expect actual=commit_msg_is.actual - printf %s $(git log --pretty=format:%s%b -1) $expect - printf %s $1 $actual + printf %s $(git log --pretty=format:%s%b -1) $actual + printf %s $1 $expect test_i18ncmp $expect $actual } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] lib-rebase: document exec_ in FAKE_LINES
Signed-off-by: Andrew Pimlott and...@pimlott.net --- t/lib-rebase.sh |2 ++ 1 file changed, 2 insertions(+) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index cfd3409..7f119e2 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -17,6 +17,8 @@ # (squash, fixup, edit, or reword) and the SHA1 taken # from the specified line. # +# exec_cmd_with_args -- add an exec cmd with args line. +# # # -- Add a comment line. # #-- Add a blank line. -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lib-rebase: use write_script
Excerpts from Junio C Hamano's message of Thu Jun 27 13:50:45 -0700 2013: Andrew Pimlott and...@pimlott.net writes: I should update the function I introduced first. I will re-submit the rebase -i --autosquash patch and wait for acceptance before trying to fix other things. Thanks. Applies on top of rebase -i patch already accepted. Mostly whitespace changes. Thanks for your other help. Andrew ---8--- Subject: [PATCH] lib-rebase: style: use write_script, -\EOF Signed-off-by: Andrew Pimlott and...@pimlott.net --- t/lib-rebase.sh | 74 +++ 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 7f119e2..8ff87fb 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -24,48 +24,46 @@ #-- Add a blank line. set_fake_editor () { - echo #!$SHELL_PATH fake-editor.sh - cat fake-editor.sh \EOF -case $1 in -*/COMMIT_EDITMSG) - test -z $EXPECT_HEADER_COUNT || - test $EXPECT_HEADER_COUNT = $(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' $1) || + write_script fake-editor.sh -\EOF + case $1 in + */COMMIT_EDITMSG) + test -z $EXPECT_HEADER_COUNT || + test $EXPECT_HEADER_COUNT = $(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' $1) || + exit + test -z $FAKE_COMMIT_MESSAGE || echo $FAKE_COMMIT_MESSAGE $1 + test -z $FAKE_COMMIT_AMEND || echo $FAKE_COMMIT_AMEND $1 exit - test -z $FAKE_COMMIT_MESSAGE || echo $FAKE_COMMIT_MESSAGE $1 - test -z $FAKE_COMMIT_AMEND || echo $FAKE_COMMIT_AMEND $1 - exit - ;; -esac -test -z $EXPECT_COUNT || - test $EXPECT_COUNT = $(sed -e '/^#/d' -e '/^$/d' $1 | wc -l) || - exit -test -z $FAKE_LINES exit -grep -v '^#' $1 $1.tmp -rm -f $1 -echo 'rebase -i script before editing:' -cat $1.tmp -action=pick -for line in $FAKE_LINES; do - case $line in - squash|fixup|edit|reword) - action=$line;; - exec*) - echo $line | sed 's/_/ /g' $1;; - #) - echo '# comment' $1;; - ) - echo $1;; - *) - sed -n ${line}s/^pick/$action/p $1.tmp $1 - action=pick;; + ;; esac -done -echo 'rebase -i script after editing:' -cat $1 -EOF + test -z $EXPECT_COUNT || + test $EXPECT_COUNT = $(sed -e '/^#/d' -e '/^$/d' $1 | wc -l) || + exit + test -z $FAKE_LINES exit + grep -v '^#' $1 $1.tmp + rm -f $1 + echo 'rebase -i script before editing:' + cat $1.tmp + action=pick + for line in $FAKE_LINES; do + case $line in + squash|fixup|edit|reword) + action=$line;; + exec*) + echo $line | sed 's/_/ /g' $1;; + #) + echo '# comment' $1;; + ) + echo $1;; + *) + sed -n ${line}s/^pick/$action/p $1.tmp $1 + action=pick;; + esac + done + echo 'rebase -i script after editing:' + cat $1 + EOF test_set_editor $(pwd)/fake-editor.sh - chmod a+x fake-editor.sh } # After set_cat_todo_editor, rebase -i will write the todo list (ignoring -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t7500: fix flipped actual/expect
Excerpts from Junio C Hamano's message of Mon Jul 01 09:52:05 -0700 2013: Wow. How could all of us missed this for a long time? :-) I don't know, but little is more frustrating than a misleading diagnostic. BTW, I didn't expect git-send-email to send those two messages in a thread. I'll keep them separate next time. Andrew -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Junio C Hamano's message of Thu Jun 27 13:52:33 -0700 2013: Two issues here, which I'll locally amend (no need to resend): Great! Thank you for your help and patience. cat expected -EOF pick ... ... EOF test_cmp expected actual Is that two issues? Andrew -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] lib-rebase: use write_script
Signed-off-by: Andrew Pimlott and...@pimlott.net --- t/lib-rebase.sh |4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 0b41155..7b42199 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -24,8 +24,7 @@ #-- Add a blank line. set_fake_editor () { - echo #!$SHELL_PATH fake-editor.sh - cat fake-editor.sh \EOF + write_script fake-editor.sh \EOF case $1 in */COMMIT_EDITMSG) test -z $EXPECT_HEADER_COUNT || @@ -65,7 +64,6 @@ cat $1 EOF test_set_editor $(pwd)/fake-editor.sh - chmod a+x fake-editor.sh } # After set_cat_todo_editor, rebase -i will write the todo list (ignoring -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lib-rebase: use write_script
Excerpts from Junio C Hamano's message of Thu Jun 27 11:37:31 -0700 2013: Thanks, but it should probably be write_script fake-editor.sh -\EOF case $1 in ... EOF test_set_editor ... if the aim is to modernize this part. Yes, the goal is to make that file consistently use the current practice. (My syntax highlighting doesn't like it, but...) I should update the function I introduced first. I will re-submit the rebase -i --autosquash patch and wait for acceptance before trying to fix other things. Andrew -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Andrew Pimlott's message of Wed Jun 26 17:20:32 -0700 2013: Excerpts from Junio C Hamano's message of Wed Jun 26 16:48:57 -0700 2013: Andrew Pimlott and...@pimlott.net 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. Updated for recommended here-doc style. Andrew ---8--- Subject: [PATCH] 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 tr...@inf.ethz.ch Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Andrew Pimlott and...@pimlott.net --- 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
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Andrew Pimlott's message of Tue Jun 25 16:03:52 -0700 2013: Thomas's patch didn't do this: fixup! or squash! after the first is simply discarded, so you see: pick d78c915 original fixup 0c6388e fixup! original fixup d15b556 fixup! original fixup 1e39bcd fixup! original But it will be a simple change to keep all the fixup!s and squash!s. I will do this (and try to make up for the carelessness of my previous patch). 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. Andrew ---8--- Subject: [PATCH] lib-rebase: set_cat_todo_editor Add a helper for testing rebase -i todo lists. This can be used to verify the expected user experience, even for todo list changes that do not affect the outcome of rebase. Signed-off-by: Andrew Pimlott and...@pimlott.net --- t/lib-rebase.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 4b74ae4..d118dd6 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -66,6 +66,19 @@ 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. + +set_cat_todo_editor () { + echo #!$SHELL_PATH fake-editor.sh + cat fake-editor.sh \EOF +grep ^[^#] $1 +exit 1 +EOF + chmod a+x fake-editor.sh + 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 () { -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Junio C Hamano's message of Wed Jun 26 16:48:57 -0700 2013: Andrew Pimlott and...@pimlott.net 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 tr...@inf.ethz.ch Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Andrew Pimlott and...@pimlott.net --- 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
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Junio C Hamano's message of Mon Jun 17 07:27:20 -0700 2013: Thomas Rast tr...@inf.ethz.ch writes: I'm not sure it's worth arguing about whether the fixup! fixup! is a symptom of some underlying problem, and changing rebase is only tapering over the symptom; or whether it's actually a useful distinction. If they are about the same complexity, then my instict tells me that it is a better design not to strip on the writing side. Thank you for the discussion. Sorry I have joined recently. I agree that it is better to preserve information as long as feasible. If we are going to strip it, it may as well be later. That is Thomas's rearrange_squash patch, which I will send again. The next question is, do we go all the way and respect the nested fixup!s in rearrange_squash? I understand the case for it, though it's hardly compelling to me in practice. :-) That would be more complicated than Thomas's patch. But I'm happy to try it if someone gives me a nudge. If not, at least the information is preserved in case someone wants to do this later. Regarding patches, I tried to follow the SubmittingPatches guidelines, but I was confused about how to include a commit in an existing thread. I think I was mislead by git-format-patch(1), When a patch is part of an ongoing discussion..., which says to remove most header fields. So if I don't want to break the discussion, should I append the unedited format-patch output to my message after scissors, or should I send it as a whole new message with --in-reply-to? Or something else? I'll try the first. Andrew ---8--- From 99023bff23f18a341441d6b7c447d9630a11b489 Mon Sep 17 00:00:00 2001 From: Andrew Pimlott and...@pimlott.net Date: Fri, 14 Jun 2013 10:33:16 -0700 Subject: [PATCH 1/4] rebase -i: handle fixup! fixup! in --autosquash In rebase -i --autosquash, ignore all fixup! or squash! after the first. Handy in case a git commit --fixup/--squash referred to an earlier fixup/squash instead of the original commit, for example with :/msg. Signed-off-by: Andrew Pimlott and...@pimlott.net --- Documentation/git-rebase.txt |4 +++- git-rebase--interactive.sh | 13 ++- t/t3415-rebase-autosquash.sh | 49 ++ 3 files changed, 64 insertions(+), 2 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..54ed4c3 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -689,7 +689,18 @@ rearrange_squash () { case $message in squash! *|fixup! *) action=${message%%!*} - rest=${message#*! } + rest=$message + # ignore any squash! or fixup! after the first + while : ; do + case $rest in + squash! *|fixup! *) + rest=${rest#*! } + ;; + *) + break + ;; + esac + done echo $sha1 $action $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 diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index a1e86c4..1a3f40a 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh @@ -193,4 +193,53 @@ 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 + git rebase --autosquash -i HEAD + git log --oneline actual + test_pause + if [ $1 = fixup ]; then + test_line_count = 3 actual + elif [ $1 = squash
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Junio C Hamano's message of Tue Jun 25 14:45:07 -0700 2013: After all, autosquash will give the user an opportunity to eyeball the result of automatic rearrangement. If the user did this: git commit -m original git commit --fixup original ;# obviously fixing the first one git commit --fixup '!fixup original' ;# explicitly fixing the second git commit --fixup original ;# may want to fix the first one and then git rebase --autosquash gave him this: pick d78c915 original fixup 0c6388e original fixup d15b556 !fixup original fixup 1e39bcd original I assume you mean: pick d78c915 original fixup 0c6388e fixup! original fixup d15b556 fixup! fixup! original fixup 1e39bcd !fixup! original The current master code tries to keep the original commit message intact. I assume you would preserve that behavior, so you would want to see fixup! fixup! it may not be what the user originally intended, but I think it is OK. As long as !fixup original message is kept in the buffer, the user can notice and rearrange, e.g. Thomas's patch didn't do this: fixup! or squash! after the first is simply discarded, so you see: pick d78c915 original fixup 0c6388e fixup! original fixup d15b556 fixup! original fixup 1e39bcd !fixup! original But it will be a simple change to keep all the fixup!s and squash!s. I will do this (and try to make up for the carelessness of my previous patch). Andrew -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Junio C Hamano's message of Tue Jun 25 14:33:18 -0700 2013: Andrew Pimlott and...@pimlott.net writes: Just reponding for the procedual part for now. So if I don't want to break the discussion, should I append the unedited format-patch output to my message after scissors, or should I send it as a whole new message with --in-reply-to? Or something else? I'll try the first. Which is fine, and you are almost there, but you do not want (1) From 99023b... that is not part of the message (it is a delimiter between multiple patches when/in case a file contains more than one); (2) From: Andrew... that is the same as the e-mail header in the message I am responding to; (3) Date: ... which is older than the e-mail header in the message I am responding to---the latter is the date people actually saw this patch on the mailing list, so it is preferrable to use it than the timestamp in your repository. So in this case, I'd expect to see, after the -- 8 -- line, only Subject: line, a blank and the log message. Thank you. It was not clear to me even after several doc readings what git-mailinfo would look for where. I think I assumed that the idea was to transmit the original commit perfectly, and I stubbornly failed to give up that assumption even when it clearly didn't fit. Everything makes more sense with the understanding that the receiver will pull together non-patch metadata in the way that makes sense from his point of view (and that a different commit will come back via fetch). I will take a whack at clarifying the docs if I have time. Andrew -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Andrew Pimlott's message of Fri Jun 14 12:31:57 -0700 2013: It happened to work and I added a test. But then it occurred to me that it might have been better to fix commit --fixup/--squash to strip the fixup! or squash! from the referenced commit in the first place. Anyhow, below is my patch for --autosquash, but unles someone has an objection to doing it in commit, I'll work on that. Here is a patch for commit.c that does this. Andrew When building the commit message for --fixup/--squash, ignore a leading fixup! or squash! on the referenced commit. Handy in case the user referred to an earlier squash/fixup commit instead of the original commit, for example with :/msg. Signed-off-by: Andrew Pimlott and...@pimlott.net --- builtin/commit.c | 18 ++ t/t7500-commit.sh | 36 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 1621dfc..370df88 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -601,8 +601,13 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (!c) die(_(could not lookup commit %s), squash_message); ctx.output_encoding = get_commit_output_encoding(); - format_commit_message(c, squash! %s\n\n, sb, - ctx); + format_commit_message(c, %s\n\n, sb, ctx); + if (!prefixcmp(sb.buf, fixup! )) { + strbuf_remove(sb, 0, strlen(fixup! )); + } else if (!prefixcmp(sb.buf, squash! )) { + strbuf_remove(sb, 0, strlen(squash! )); + } + strbuf_insert(sb, 0, squash! , strlen(squash! )); } } @@ -634,8 +639,13 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (!commit) die(_(could not lookup commit %s), fixup_message); ctx.output_encoding = get_commit_output_encoding(); - format_commit_message(commit, fixup! %s\n\n, - sb, ctx); + format_commit_message(commit, %s\n\n, sb, ctx); + if (!prefixcmp(sb.buf, fixup! )) { + strbuf_remove(sb, 0, strlen(fixup! )); + } else if (!prefixcmp(sb.buf, squash! )) { + strbuf_remove(sb, 0, strlen(squash! )); + } + strbuf_insert(sb, 0, fixup! , strlen(fixup! )); hook_arg1 = message; } else if (!stat(git_path(MERGE_MSG), statbuf)) { if (strbuf_read_file(sb, git_path(MERGE_MSG), 0) 0) diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh index 436b7b6..ecdf74a 100755 --- a/t/t7500-commit.sh +++ b/t/t7500-commit.sh @@ -320,4 +320,40 @@ test_expect_success 'invalid message options when using --fixup' ' test_must_fail git commit --fixup HEAD~1 -F log ' +test_expect_success 'commit --fixup of existing fixup' ' + commit_for_rebase_autosquash_setup + git commit --fixup HEAD~1 + echo fourth content line foo + git add foo + git commit --fixup HEAD + commit_msg_is fixup! target message subject line +' + +test_expect_success 'commit --fixup of existing squash' ' + commit_for_rebase_autosquash_setup + git commit --squash HEAD~1 + echo fourth content line foo + git add foo + git commit --fixup HEAD + commit_msg_is fixup! target message subject line +' + +test_expect_success 'commit --squash of existing squash' ' + commit_for_rebase_autosquash_setup + git commit --squash HEAD~1 + echo fourth content line foo + git add foo + git commit --squash HEAD + commit_msg_is squash! target message subject linecommit message +' + +test_expect_success 'commit --squash of existing fixup' ' + commit_for_rebase_autosquash_setup + git commit --fixup HEAD~1 + echo fourth content line foo + git add foo + git commit --squash HEAD + commit_msg_is squash! target message subject linecommit message +' + test_done -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Thomas Rast's message of Tue Jun 11 11:50:07 -0700 2013: Andrew Pimlott and...@pimlott.net writes: git commit -m 'fix nasty bug' ... git commit --fixup :/nasty ... git commit --fixup :/nasty The second :/nasty resolves to the previous fixup, not the initial commit. I could have made the regular expression more precise, but this would be a hassle. Would a change to support fixup! fixup! be considered? Sure, why not. You could start with something like the patch below (untested). If that happens to work, just add a test and a good commit message. It happened to work and I added a test. But then it occurred to me that it might have been better to fix commit --fixup/--squash to strip the fixup! or squash! from the referenced commit in the first place. Anyhow, below is my patch for --autosquash, but unles someone has an objection to doing it in commit, I'll work on that. Andrew Ignore subsequent fixup! or squash! after the first. Handy in case a git commit --fixup/--squash referred to a previous fixup/squash instead of the original commit, for example with :/msg. Signed-off-by: Andrew Pimlott and...@pimlott.net --- Documentation/git-rebase.txt |4 +++- git-rebase--interactive.sh | 13 ++- t/t3415-rebase-autosquash.sh | 49 ++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index c84854a..725cf27 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 a + previous 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..54ed4c3 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -689,7 +689,18 @@ rearrange_squash () { case $message in squash! *|fixup! *) action=${message%%!*} - rest=${message#*! } + rest=$message + # ignore any squash! or fixup! after the first + while : ; do + case $rest in + squash! *|fixup! *) + rest=${rest#*! } + ;; + *) + break + ;; + esac + done echo $sha1 $action $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 diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index a1e86c4..1a3f40a 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh @@ -193,4 +193,53 @@ 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 + git rebase --autosquash -i HEAD + git log --oneline actual + test_pause + if [ $1 = fixup ]; then + test_line_count = 3 actual + elif [ $1 = squash ]; then + test_line_count = 4 actual + else + false + fi + git diff --exit-code final-$1-$2 + test 2 = $(git cat-file blob HEAD^:file1) + if [ $1 = fixup ]; then + test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) + elif [ $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 -- To unsubscribe from this list: send the line unsubscribe git in the body
rebase --autosquash does not handle fixup! of fixup!
git rebase -i --autosquash does not handle a fixup! of a fixup!, such as the history: aaa fix nasty bug ... bbb fixup! fix nasty bug ... ccc fixup! fixup! fix nasty bug --autosquash produces: pick aaa fix nasty bug fixup bbb fixup! fix nasty bug ... pick ccc fixup! fixup! fix nasty bug This defeats the workflow I was hoping to use: git commit -m 'fix nasty bug' ... git commit --fixup :/nasty ... git commit --fixup :/nasty The second :/nasty resolves to the previous fixup, not the initial commit. I could have made the regular expression more precise, but this would be a hassle. Would a change to support fixup! fixup! be considered? Andrew (Please Cc: me on replies.) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html