Re: [PATCH 1/3] t/checkout-last: checkout - doesn't work after rebase -i

2013-06-13 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 These four are all valid ways to spell the rebase -i master step.

 and I think it is sensible to expect

  (1) they all behave the same way; or

Yes.  My reasoning is very simple: a rebase is a rebase; it should not
write checkout:  messages to the reflog.  Therefore, the @{-N}
will ignore it; for the purposes of checkout -, the rebase event is
inconsequential.
--
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 1/3] t/checkout-last: checkout - doesn't work after rebase -i

2013-06-10 Thread Ramkumar Ramachandra
The following command

  $ git checkout -

does not work as expected after a 'git rebase -i'.

Add a failing test documenting this bug.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 t/t2012-checkout-last.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index b44de9d..5729487 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -116,4 +116,12 @@ test_expect_success 'master...' '
test z$(git rev-parse --verify HEAD) = z$(git rev-parse --verify 
master^)
 '
 
+test_expect_failure 'checkout - works after an interactive rebase' '
+   git checkout master 
+   git checkout other 
+   git rebase -i master 
+   git checkout - 
+   test z$(git symbolic-ref HEAD) = zrefs/heads/master
+'
+
 test_done
-- 
1.8.3.254.g60f9e5b

--
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 1/3] t/checkout-last: checkout - doesn't work after rebase -i

2013-06-10 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 The following command

   $ git checkout -

 does not work as expected after a 'git rebase -i'.

 Add a failing test documenting this bug.

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  t/t2012-checkout-last.sh | 8 
  1 file changed, 8 insertions(+)

 diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
 index b44de9d..5729487 100755
 --- a/t/t2012-checkout-last.sh
 +++ b/t/t2012-checkout-last.sh
 @@ -116,4 +116,12 @@ test_expect_success 'master...' '
   test z$(git rev-parse --verify HEAD) = z$(git rev-parse --verify 
 master^)
  '
  
 +test_expect_failure 'checkout - works after an interactive rebase' '
 + git checkout master 
 + git checkout other 
 + git rebase -i master 
 + git checkout - 
 + test z$(git symbolic-ref HEAD) = zrefs/heads/master
 +'

Hmph, you were on other and then ran rebase -i to rebase it.
When you are done, you are on other.  You want to go back to the
one before you checked out the branch you started your rebase -i
on, which is master.

OK, the expectation makes sense to me.

These four are all valid ways to spell the rebase -i master step.

git rebase -i master
git rebase -i --onto master master
git rebase -i master other
git rebase -i --onto master master other

and I think it is sensible to expect

 (1) they all behave the same way; or

 (2) the first two behave the same, but the latter two explicitly
 checks out 'other' by giving the last argument.  If you are not
 on 'other' when you run the rebase -i, checkout - should
 come back to the branch before 'other', i.e. the branch you
 started your rebase -i on.

In other words, (2) would mean:

git checkout master 
git checkout -b naster 
git rebase -i master other 
# ends up on other
test_string_equal $(git symbolic-ref HEAD) refs/heads/other 
git checkout - 
# we were on naster before we rebased 'other'
test_string_equal $(git symbolic-ref HEAD) refs/heads/naster

It is a bit unclear what the following should expect.

git checkout master 
git checkout other 
git rebase -i master other 
# ends up on other
test_string_equal $(git symbolic-ref HEAD) refs/heads/other 
git checkout - 
# we are on ??? before we rebased other
test_string_equal $(git symbolic-ref HEAD) refs/heads/???

One could argue that the first checkout other and then rebase done
by rebase becomes a no-op and the user should be aware of that
because rebase is started while on that other branch, in which case
we should come back to 'master'.  From consistency point of view,
one could instead argue that we were on 'other' before we rebased
it, in which case it may not be unreasonable for checkout - to
come back to 'other'.  I tend to prefer the former (i.e. go back to
'master') over the latter but not by a large margin.

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