# [PATCH] sha1_name: reorganize get_sha1_basic() 6 messages from 2013-05-02 to 2013-05-02. Participants: Felipe Contreras, Ramkumar Ramachandra, Duy Nguyen. Thread: https://gitlist.dev/t/33698 ## Felipe Contreras, 2013-05-02 00:49 Subject: [PATCH] sha1_name: reorganize get_sha1_basic() Message-ID: <1367455791-30091-1-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367455791-30091-1-git-send-email-felipe.contreras%40gmail.com ``` 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 --- 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 ``` ## Felipe Contreras, 2013-05-02 00:53 Subject: Re: [PATCH] sha1_name: reorganize get_sha1_basic() Message-ID: URL: https://gitlist.dev/e/CAMP44s2_riedK8gsHELRgQXd%2B7XWaTC6ErhPqX3xqWQoc5pJKQ%40mail.gmail.com In-Reply-To: <1367455791-30091-1-git-send-email-felipe.contreras@gmail.com> ``` On Wed, May 1, 2013 at 7:49 PM, Felipe Contreras 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 ``` ## Felipe Contreras, 2013-05-02 01:00 Subject: Re: [PATCH] sha1_name: reorganize get_sha1_basic() Message-ID: URL: https://gitlist.dev/e/CAMP44s0_9aXu0eZ0w_dkkoR4No3ZzKVsdzAb_E1wud-ZpOW6dg%40mail.gmail.com In-Reply-To: <1367455791-30091-1-git-send-email-felipe.contreras@gmail.com> ``` On Wed, May 1, 2013 at 7:49 PM, Felipe Contreras 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 > --- > 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 ``` ## Ramkumar Ramachandra, 2013-05-02 09:40 Subject: Re: [PATCH] sha1_name: reorganize get_sha1_basic() Message-ID: URL: https://gitlist.dev/e/CALkWK0%3DqknGMuHN-zJzLXDNRLFdaBssccnkXbXbM8SZ%2BORKiZA%40mail.gmail.com In-Reply-To: <1367455791-30091-1-git-send-email-felipe.contreras@gmail.com> ``` 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. ``` ## Duy Nguyen, 2013-05-02 10:19 Subject: Re: [PATCH] sha1_name: reorganize get_sha1_basic() Message-ID: URL: https://gitlist.dev/e/CACsJy8CWGJZByedxhzuU8OkPPBc0XmfZqE_e_ehmt55t-tHmsQ%40mail.gmail.com In-Reply-To: <1367455791-30091-1-git-send-email-felipe.contreras@gmail.com> ``` On Thu, May 2, 2013 at 7:49 AM, Felipe Contreras 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 ``` ## Felipe Contreras, 2013-05-02 17:52 Subject: Re: [PATCH] sha1_name: reorganize get_sha1_basic() Message-ID: URL: https://gitlist.dev/e/CAMP44s2HPRpbot2YhTmRNRmGj_1-%2BBdnjW9kTVXwq3Ni0YiObA%40mail.gmail.com In-Reply-To: ``` On Thu, May 2, 2013 at 5:19 AM, Duy Nguyen wrote: > On Thu, May 2, 2013 at 7:49 AM, Felipe Contreras > 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 ```