[PATCH] t7500: fix flipped actual/expect

2013-07-01 Thread Andrew Pimlott

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

2013-07-01 Thread Andrew Pimlott

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

2013-07-01 Thread Andrew Pimlott
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

2013-07-01 Thread Andrew Pimlott
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!

2013-06-28 Thread Andrew Pimlott
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

2013-06-27 Thread Andrew Pimlott

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

2013-06-27 Thread Andrew Pimlott
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!

2013-06-27 Thread Andrew Pimlott
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!

2013-06-26 Thread Andrew Pimlott
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!

2013-06-26 Thread Andrew Pimlott
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!

2013-06-25 Thread Andrew Pimlott
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!

2013-06-25 Thread Andrew Pimlott
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!

2013-06-25 Thread Andrew Pimlott
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!

2013-06-15 Thread Andrew Pimlott
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!

2013-06-14 Thread Andrew Pimlott
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!

2013-06-11 Thread Andrew Pimlott
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