Re: [PATCH] t3400 (rebase): add failing test for a peculiar rev spec

2013-04-19 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 peel_committish () {
 case $1 in
 :/*)
 peeltmp=$(git rev-parse --verify $1) 
 git rev-parse --verify $peeltmp^0
 ;;
 *)
 git rev-parse --verify $1^0
 ;;
 esac
 }

Thanks.  Yeah, that's the obvious hack solution: special handling
for refspecs of the kind :/text.  Will write a patch for it soon
(currently struggling to complete rebase.autostash).  In the meantime,
please queue this patch.

I'm more interested in knowing what you think of my first point: is
:/text fundamentally broken, as it can't be combined with other
operators like the other rev specs can?  If so, how do you think we
should fix it?
--
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] t3400 (rebase): add failing test for a peculiar rev spec

2013-04-19 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 I'm more interested in knowing what you think of my first point: is
 :/text fundamentally broken, as it can't be combined with other
 operators like the other rev specs can?  If so, how do you think we
 should fix it?

The question :/string wants to ask is fundamentally ambiguous once
you start wanting to give arbitrary string to it.

Think what this asks:

git show :/Fix regression introduced by commit v1.5.1-rc1~113^2

Is it looking for a commit that fixes v1.5.1-rc1 and then locate the
side branch that waas merged by the 113th-generation parent of that
fix?  Or is it looking for the commit that fixes v1.5.1-rc1~113^2?

When we introduced a postfix $rev^{/string} operator at 32574b68c57f
(get_sha1: support $commit^{/regex} syntax, 2010-12-13) to dig from
a given rev, we designed it so that we can disambiguate between the
two: HEAD^{/Fix v1.5.1~113^2} vs HEAD^{/Fix v1.5.1}~113^2.

But the tradeoff made with 28a4d9404438 (object name: introduce
':/oneline prefix' notation, 2007-02-24), which is much older than
the $rev^{/string} syntax, is to help casual interactive use by not
requiring sometimes-hard-to-type line noise characters.

Even back then, I did not want to paint us into a corner we cannot
escape from, and made sure the syntax has an escape hatch built-in.

cf.

  http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477

If you cared deeply enough, you could activate it, to take something
like:

git show :/!(s=Fix regression by v1.5.1-rc1~113^2)~20

to say The 20th-generation parent of the commit whose subject says
it fixed regression at v1.5.1-rc1~113^2.

That would make this

:/!(a=Ramkumar)(s=move diff.wordRegex)~4

a way to find the fourth-generation parent of 2ead7a674d7f (t5516
(fetch-push): update test description, 2013-04-02) as a natural
extension, I guess.

--
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] t3400 (rebase): add failing test for a peculiar rev spec

2013-04-19 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 That would make this

   :/!(a=Ramkumar)(s=move diff.wordRegex)~4

 a way to find the fourth-generation parent of ...

... 0b830ac52137 (Documentation: move diff.wordRegex from config.txt
to diff-config.txt, 2012-11-13)

I had a log output on another terminal and cutpaste a wrong one in
the original message.
--
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] t3400 (rebase): add failing test for a peculiar rev spec

2013-04-19 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 peel_committish () {
 case $1 in
 :/*)
 peeltmp=$(git rev-parse --verify $1) 
 git rev-parse --verify $peeltmp^0
 ;;
 *)
 git rev-parse --verify $1^0
 ;;
 esac
 }

 Thanks.  Yeah, that's the obvious hack solution: special handling
 for refspecs of the kind :/text.

By the way, that is not a hack, but is merely working within the
given constraints.  Welcome to the real life.

Unlike rebase that has the luxury of possibly update the
underlying plumbing, third-party tools have to work within the
constraints of existing versions of Git.  It is better to learn that
you sometimes need to do so, and how to do so when you need to.
--
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] t3400 (rebase): add failing test for a peculiar rev spec

2013-04-18 Thread Ramkumar Ramachandra
'git rebase' does not recognize revisions specified as :/text.  This
is because the attempts to rev-parse ${REV}^0, which fails in this
case.  Add a test to document this failure.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Okay, so I'm not sure what the right fix for this is.

 - The :/text syntax seems to be broken in the first place, as it
   can't be combined with ^ or @.  I'd like to be able to say
   {:/bardery}^1, or even {:/foomery}^{/communist mule}.  Why
   shouldn't I be allowed to do this?

 - The failure occurs in git-rebase.sh:403.  Is it using the ^0 only
   to make sure that the revision specified is a commit?  Surely,
   there'a a better way to do this?

 Can someone point me in the right direction?

 t/t3400-rebase.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index f6cc102..42ca1e0 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,14 @@ test_expect_success 'rebase fast-forward to master' '
test_i18ngrep Fast-forwarded HEAD to my-topic-branch out
 '
 
+test_expect_failure 'rebase against revision specified as :/text' '
+   git checkout my-topic-branch^ 
+   sha1=$(git rev-parse :/Add B) 
+   git rebase $sha1 
+   git checkout my-topic-branch^ 
+   git rebase :/Add B
+'
+
 test_expect_success 'the rebase operation should not have destroyed author 
information' '
! (git log | grep Author: | grep )
 '
-- 
1.8.2.1.423.g4fb5c0a.dirty

--
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] t3400 (rebase): add failing test for a peculiar rev spec

2013-04-18 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 'git rebase' does not recognize revisions specified as :/text.  This
 is because the attempts to rev-parse ${REV}^0, which fails in this
 case.  Add a test to document this failure.

  - The failure occurs in git-rebase.sh:403.  Is it using the ^0 only
to make sure that the revision specified is a commit?  Surely,
there'a a better way to do this?

  Can someone point me in the right direction?

How about ${REV}^0 into

nREV=$(git rev-parse ${REV})^0

and use it where you need an object name that needs to be parsed by
get_sha1(), e.g.

git checkout -q $nREV^0

I would suggest a helper function in git-sh-setup, something like:

peel_committish () {
case $1 in
:/*)
peeltmp=$(git rev-parse --verify $1) 
git rev-parse --verify $peeltmp^0
;;
*)
git rev-parse --verify $1^0
;;
esac
}

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