* [PATCH] sha1_name: reorganize get_sha1_basic()
@ 2013-05-02 0:49 Felipe Contreras
2013-05-02 0:53 ` Felipe Contreras
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Felipe Contreras @ 2013-05-02 0:49 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ramkumar Ramachandra, Johannes Schindelin,
Thomas Rast, Felipe Contreras
Through the years the functionality to handle @{-N} and @{u} has moved
around the code, and as a result, code that once made sense, doesn't any
more.
There is no need to call this function recursively with the branch of
@{-N} substituted because dwim_{ref,log} already replaces it.
However, there's one corner-case where @{-N} resolves to a detached
HEAD, in which case we wouldn't get any ref back.
So we parse the nth-prior manually, and deal with it depending on
weather it's a SHA-1, or a ref.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
sha1_name.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 3820f28..110afbb 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -431,6 +431,7 @@ static inline int upstream_mark(const char *string, int len)
}
static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);
+static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);
static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
{
@@ -438,6 +439,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
char *real_ref = NULL;
int refs_found = 0;
int at, reflog_len;
+ struct strbuf buf = STRBUF_INIT;
if (len == 40 && !get_sha1_hex(str, sha1))
return 0;
@@ -447,6 +449,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
if (len && str[len-1] == '}') {
for (at = len-2; at >= 0; at--) {
if (str[at] == '@' && str[at+1] == '{') {
+ if (at == 0 && str[2] == '-')
+ /* @{-N} syntax */
+ continue;
if (!upstream_mark(str + at, len - at)) {
reflog_len = (len-1) - (at+2);
len = at;
@@ -460,20 +465,19 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
if (len && ambiguous_path(str, len))
return -1;
- if (!len && reflog_len) {
- struct strbuf buf = STRBUF_INIT;
- int ret;
- /* try the @{-N} syntax for n-th checkout */
- ret = interpret_branch_name(str+at, &buf);
- if (ret > 0) {
- /* substitute this branch name and restart */
- return get_sha1_1(buf.buf, buf.len, sha1, 0);
- } else if (ret == 0) {
- return -1;
- }
+ if (interpret_nth_prior_checkout(str, &buf) > 0) {
+ int detached;
+
+ detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));
+ strbuf_release(&buf);
+ if (detached)
+ return 0;
+ }
+
+ if (!len && reflog_len)
/* allow "@{...}" to mean the current branch reflog */
refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
- } else if (reflog_len)
+ else if (reflog_len)
refs_found = dwim_log(str, len, sha1, &real_ref);
else
refs_found = dwim_ref(str, len, sha1, &real_ref);
--
1.8.3.rc0.399.gc96a135
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] sha1_name: reorganize get_sha1_basic()
2013-05-02 0:49 [PATCH] sha1_name: reorganize get_sha1_basic() Felipe Contreras
@ 2013-05-02 0:53 ` Felipe Contreras
2013-05-02 1:00 ` Felipe Contreras
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2013-05-02 0:53 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ramkumar Ramachandra, Johannes Schindelin,
Thomas Rast, Felipe Contreras
On Wed, May 1, 2013 at 7:49 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Through the years the functionality to handle @{-N} and @{u} has moved
> around the code, and as a result, code that once made sense, doesn't any
> more.
>
> There is no need to call this function recursively with the branch of
> @{-N} substituted because dwim_{ref,log} already replaces it.
>
> However, there's one corner-case where @{-N} resolves to a detached
> HEAD, in which case we wouldn't get any ref back.
>
> So we parse the nth-prior manually, and deal with it depending on
> weather it's a SHA-1, or a ref.
Forgot to mention; inspired by a patch from Ramkumar Ramachandra.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sha1_name: reorganize get_sha1_basic()
2013-05-02 0:49 [PATCH] sha1_name: reorganize get_sha1_basic() Felipe Contreras
2013-05-02 0:53 ` Felipe Contreras
@ 2013-05-02 1:00 ` Felipe Contreras
2013-05-02 9:40 ` Ramkumar Ramachandra
2013-05-02 10:19 ` Duy Nguyen
3 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2013-05-02 1:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ramkumar Ramachandra, Johannes Schindelin,
Thomas Rast, Felipe Contreras
On Wed, May 1, 2013 at 7:49 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Through the years the functionality to handle @{-N} and @{u} has moved
> around the code, and as a result, code that once made sense, doesn't any
> more.
>
> There is no need to call this function recursively with the branch of
> @{-N} substituted because dwim_{ref,log} already replaces it.
>
> However, there's one corner-case where @{-N} resolves to a detached
> HEAD, in which case we wouldn't get any ref back.
>
> So we parse the nth-prior manually, and deal with it depending on
> weather it's a SHA-1, or a ref.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> sha1_name.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 3820f28..110afbb 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -431,6 +431,7 @@ static inline int upstream_mark(const char *string, int len)
> }
>
> static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);
> +static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);
>
> static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
> {
> @@ -438,6 +439,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
> char *real_ref = NULL;
> int refs_found = 0;
> int at, reflog_len;
> + struct strbuf buf = STRBUF_INIT;
>
> if (len == 40 && !get_sha1_hex(str, sha1))
> return 0;
> @@ -447,6 +449,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
> if (len && str[len-1] == '}') {
> for (at = len-2; at >= 0; at--) {
> if (str[at] == '@' && str[at+1] == '{') {
> + if (at == 0 && str[2] == '-')
> + /* @{-N} syntax */
> + continue;
> if (!upstream_mark(str + at, len - at)) {
> reflog_len = (len-1) - (at+2);
> len = at;
> @@ -460,20 +465,19 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
> if (len && ambiguous_path(str, len))
> return -1;
>
> - if (!len && reflog_len) {
> - struct strbuf buf = STRBUF_INIT;
> - int ret;
> - /* try the @{-N} syntax for n-th checkout */
> - ret = interpret_branch_name(str+at, &buf);
> - if (ret > 0) {
> - /* substitute this branch name and restart */
> - return get_sha1_1(buf.buf, buf.len, sha1, 0);
> - } else if (ret == 0) {
> - return -1;
> - }
> + if (interpret_nth_prior_checkout(str, &buf) > 0) {
> + int detached;
> +
> + detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));
> + strbuf_release(&buf);
> + if (detached)
> + return 0;
> + }
> +
> + if (!len && reflog_len)
> /* allow "@{...}" to mean the current branch reflog */
> refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
> - } else if (reflog_len)
> + else if (reflog_len)
> refs_found = dwim_log(str, len, sha1, &real_ref);
> else
> refs_found = dwim_ref(str, len, sha1, &real_ref);
> --
This might make more sense:
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -438,8 +438,7 @@ static int get_sha1_basic(const char *str, int
len, unsigned char *sha1)
static const char *warn_msg = "refname '%.*s' is ambiguous.";
char *real_ref = NULL;
int refs_found = 0;
- int at, reflog_len;
- struct strbuf buf = STRBUF_INIT;
+ int at, reflog_len, nth_prior = 0;
if (len == 40 && !get_sha1_hex(str, sha1))
return 0;
@@ -449,9 +448,10 @@ static int get_sha1_basic(const char *str, int
len, unsigned char *sha1)
if (len && str[len-1] == '}') {
for (at = len-2; at >= 0; at--) {
if (str[at] == '@' && str[at+1] == '{') {
- if (at == 0 && str[2] == '-')
- /* @{-N} syntax */
+ if (at == 0 && str[2] == '-') {
+ nth_prior = 1;
continue;
+ }
if (!upstream_mark(str + at, len - at)) {
reflog_len = (len-1) - (at+2);
len = at;
@@ -465,9 +465,12 @@ static int get_sha1_basic(const char *str, int
len, unsigned char *sha1)
if (len && ambiguous_path(str, len))
return -1;
- if (interpret_nth_prior_checkout(str, &buf) > 0) {
+ if (nth_prior) {
+ struct strbuf buf = STRBUF_INIT;
int detached;
+ interpret_nth_prior_checkout(str, &buf);
+
detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));
strbuf_release(&buf);
if (detached)
--
Felipe Contreras
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sha1_name: reorganize get_sha1_basic()
2013-05-02 0:49 [PATCH] sha1_name: reorganize get_sha1_basic() Felipe Contreras
2013-05-02 0:53 ` Felipe Contreras
2013-05-02 1:00 ` Felipe Contreras
@ 2013-05-02 9:40 ` Ramkumar Ramachandra
2013-05-02 10:19 ` Duy Nguyen
3 siblings, 0 replies; 6+ messages in thread
From: Ramkumar Ramachandra @ 2013-05-02 9:40 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Johannes Schindelin, Thomas Rast
Felipe Contreras wrote:
> @@ -447,6 +449,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
> if (len && str[len-1] == '}') {
> for (at = len-2; at >= 0; at--) {
> if (str[at] == '@' && str[at+1] == '{') {
> + if (at == 0 && str[2] == '-')
> + /* @{-N} syntax */
> + continue;
Nicely done. Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sha1_name: reorganize get_sha1_basic()
2013-05-02 0:49 [PATCH] sha1_name: reorganize get_sha1_basic() Felipe Contreras
` (2 preceding siblings ...)
2013-05-02 9:40 ` Ramkumar Ramachandra
@ 2013-05-02 10:19 ` Duy Nguyen
2013-05-02 17:52 ` Felipe Contreras
3 siblings, 1 reply; 6+ messages in thread
From: Duy Nguyen @ 2013-05-02 10:19 UTC (permalink / raw)
To: Felipe Contreras
Cc: Git Mailing List, Junio C Hamano, Ramkumar Ramachandra,
Johannes Schindelin, Thomas Rast
On Thu, May 2, 2013 at 7:49 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Through the years the functionality to handle @{-N} and @{u} has moved
> around the code, and as a result, code that once made sense, doesn't any
> more.
>
> There is no need to call this function recursively with the branch of
> @{-N} substituted because dwim_{ref,log} already replaces it.
>
> However, there's one corner-case where @{-N} resolves to a detached
> HEAD, in which case we wouldn't get any ref back.
>
> So we parse the nth-prior manually, and deal with it depending on
> weather it's a SHA-1, or a ref.
If you are brave, reorganize the whole extended sha-1 parsing code. I
think the right-to-left parsing is confusing, not to mention it has to
treat ":path" differently. But may be that's too big work for little
gain.
--
Duy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sha1_name: reorganize get_sha1_basic()
2013-05-02 10:19 ` Duy Nguyen
@ 2013-05-02 17:52 ` Felipe Contreras
0 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2013-05-02 17:52 UTC (permalink / raw)
To: Duy Nguyen
Cc: Git Mailing List, Junio C Hamano, Ramkumar Ramachandra,
Johannes Schindelin, Thomas Rast
On Thu, May 2, 2013 at 5:19 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Thu, May 2, 2013 at 7:49 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Through the years the functionality to handle @{-N} and @{u} has moved
>> around the code, and as a result, code that once made sense, doesn't any
>> more.
>>
>> There is no need to call this function recursively with the branch of
>> @{-N} substituted because dwim_{ref,log} already replaces it.
>>
>> However, there's one corner-case where @{-N} resolves to a detached
>> HEAD, in which case we wouldn't get any ref back.
>>
>> So we parse the nth-prior manually, and deal with it depending on
>> weather it's a SHA-1, or a ref.
>
> If you are brave, reorganize the whole extended sha-1 parsing code. I
> think the right-to-left parsing is confusing, not to mention it has to
> treat ":path" differently. But may be that's too big work for little
> gain.
Yeah, it took me a while to figure it was the other way around to what
I thought was natural. Maybe after the dust with '@' is settled.
FTR. This doesn't work @{u}@{u}.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-02 17:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-02 0:49 [PATCH] sha1_name: reorganize get_sha1_basic() Felipe Contreras
2013-05-02 0:53 ` Felipe Contreras
2013-05-02 1:00 ` Felipe Contreras
2013-05-02 9:40 ` Ramkumar Ramachandra
2013-05-02 10:19 ` Duy Nguyen
2013-05-02 17:52 ` Felipe Contreras
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.