Re: [PATCH] t3400 (rebase): add failing test for a peculiar rev spec
Ramkumar Ramachandra 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
Re: [PATCH] t3400 (rebase): add failing test for a peculiar rev spec
Junio C Hamano 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 cut&paste 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
Ramkumar Ramachandra 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 ':/' 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
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
Ramkumar Ramachandra 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