# [RFC/PATCH 1/2] for-each-ref: add new format 'refbasename' 8 messages from 2008-08-29 to 2008-08-31. Participants: SZEDER Gábor, Shawn O. Pearce, Bert Wesarg, Junio C Hamano. Thread: https://gitlist.dev/t/15274 ## SZEDER Gábor, 2008-08-29 10:37 Subject: [RFC/PATCH 1/2] for-each-ref: add new format 'refbasename' Message-ID: <0329f1ab2642e1dab701b5fc6517bfb1b4ea2d46.1220004755.git.szeder@ira.uka.de> URL: https://gitlist.dev/e/0329f1ab2642e1dab701b5fc6517bfb1b4ea2d46.1220004755.git.szeder%40ira.uka.de ``` fot-each-ref's refname format outputs each ref in its full format, e.g. 'refs/heads/foo' and 'refs/tags/bar'. However, there are tools that need only the last part of the refname, e.g. only 'foo' and 'bar'. Such a tool is git's bash completion script, which spends considerable amount of time removing the unneeded parts from for-each-ref's output. Therefore, we introduce a new for-each-ref format called 'refbasename', which strips everything before and including the second '/' in the ref's name from the output. Signed-off-by: SZEDER Gábor --- I assumed that refs always look like 'refs/{heads,tags,whatever}/foo', hence this patch breaks if a ref might look like 'refs/foo' or just 'foo'. But can I really rely on that? builtin-for-each-ref.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c index 21e92bb..1993ff4 100644 --- a/builtin-for-each-ref.c +++ b/builtin-for-each-ref.c @@ -66,6 +66,7 @@ static struct { { "subject" }, { "body" }, { "contents" }, + { "refbasename" }, }; /* @@ -577,6 +578,10 @@ static void populate_value(struct refinfo *ref) char *s = xmalloc(len + 4); sprintf(s, "%s^{}", ref->refname); v->s = s; + } else if (!strcmp(name, "refbasename")) { + char * p = strchr(ref->refname, '/'); + p = strchr(p+1, '/'); + v->s = p+1; } } -- 1.6.0.1.133.g10dd.dirty ``` ## SZEDER Gábor, 2008-08-29 10:37 Subject: [RFC/PATCH 2/2] bash: use for-each-ref format 'refbasename' Message-ID: <31df5ea75841b142240e04ac82153c39d5e11abb.1220004755.git.szeder@ira.uka.de> URL: https://gitlist.dev/e/31df5ea75841b142240e04ac82153c39d5e11abb.1220004755.git.szeder%40ira.uka.de In-Reply-To: <0329f1ab2642e1dab701b5fc6517bfb1b4ea2d46.1220004755.git.szeder@ira.uka.de> ``` Using this format simplifies the code for completing refs and (in some cases) improves performance significantly. For repositories like the current git.git (with more than 200 refs) there are no real performance difference, but for a repository with 2000 refs the total time needed to complete the refs is around 40% less (from around 480ms to around 295ms). Signed-off-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 26 ++++++-------------------- 1 files changed, 6 insertions(+), 20 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 4f64f8a..8269c42 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -154,11 +154,8 @@ __git_heads () { local cmd i is_hash=y dir="$(__gitdir "$1")" if [ -d "$dir" ]; then - for i in $(git --git-dir="$dir" \ - for-each-ref --format='%(refname)' \ - refs/heads ); do - echo "${i#refs/heads/}" - done + git --git-dir="$dir" for-each-ref --format='%(refbasename)' \ + refs/heads return fi for i in $(git ls-remote "$1" 2>/dev/null); do @@ -175,11 +172,8 @@ __git_tags () { local cmd i is_hash=y dir="$(__gitdir "$1")" if [ -d "$dir" ]; then - for i in $(git --git-dir="$dir" \ - for-each-ref --format='%(refname)' \ - refs/tags ); do - echo "${i#refs/tags/}" - done + git --git-dir="$dir" for-each-ref --format='%(refbasename)' \ + refs/tags return fi for i in $(git ls-remote "$1" 2>/dev/null); do @@ -197,16 +191,8 @@ __git_refs () local cmd i is_hash=y dir="$(__gitdir "$1")" if [ -d "$dir" ]; then if [ -e "$dir/HEAD" ]; then echo HEAD; fi - for i in $(git --git-dir="$dir" \ - for-each-ref --format='%(refname)' \ - refs/tags refs/heads refs/remotes); do - case "$i" in - refs/tags/*) echo "${i#refs/tags/}" ;; - refs/heads/*) echo "${i#refs/heads/}" ;; - refs/remotes/*) echo "${i#refs/remotes/}" ;; - *) echo "$i" ;; - esac - done + git --git-dir="$dir" for-each-ref --format='%(refbasename)' \ + refs/tags refs/heads refs/remotes return fi for i in $(git ls-remote "$dir" 2>/dev/null); do -- 1.6.0.1.133.g10dd.dirty ``` ## Shawn O. Pearce, 2008-08-29 14:34 Subject: Re: [RFC/PATCH 1/2] for-each-ref: add new format 'refbasename' Message-ID: <20080829143448.GC7403@spearce.org> URL: https://gitlist.dev/e/20080829143448.GC7403%40spearce.org In-Reply-To: <0329f1ab2642e1dab701b5fc6517bfb1b4ea2d46.1220004755.git.szeder@ira.uka.de> ``` SZEDER GGGbor wrote: > Therefore, we introduce a new for-each-ref format called 'refbasename', > which strips everything before and including the second '/' in the ref's > name from the output. > > I assumed that refs always look like 'refs/{heads,tags,whatever}/foo', > hence this patch breaks if a ref might look like 'refs/foo' or just > 'foo'. But can I really rely on that? > > diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c > index 21e92bb..1993ff4 100644 > --- a/builtin-for-each-ref.c > +++ b/builtin-for-each-ref.c > @@ -577,6 +578,10 @@ static void populate_value(struct refinfo *ref) > char *s = xmalloc(len + 4); > sprintf(s, "%s^{}", ref->refname); > v->s = s; > + } else if (!strcmp(name, "refbasename")) { > + char * p = strchr(ref->refname, '/'); > + p = strchr(p+1, '/'); > + v->s = p+1; Please be careful here and check for !p. A refname may be missing one or two '/' in which case you will cause the process to segfault. I don't think its a good idea to assume you'll always have to '/' in the name. "refs/foo" can be created by git-update-ref. Or if we ever started to report on HEAD this output tag would crash. Also, as a style nit, its "p + 1" not "p+1". Test cases? On the other hand, I like where this is going. Given that it has such a nice effect on the bash completion for larger repos I'd like to see it in Git. -- Shawn. ``` ## SZEDER Gábor, 2008-08-29 16:45 Subject: [RFC/PATCH 1/2 v2] for-each-ref: add new format 'refbasename' Message-ID: <20080829164508.GF8000@neumann> URL: https://gitlist.dev/e/20080829164508.GF8000%40neumann In-Reply-To: <20080829143448.GC7403@spearce.org> ``` for-each-ref's refname format outputs each ref in its full format, e.g. 'refs/heads/foo' and 'refs/tags/bar'. However, there are tools that need only the last part of the refname, e.g. only 'foo' and 'bar'. Such a tool is git's bash completion script, which spends considerable amount of time removing the unneeded parts from for-each-ref's output. Therefore, we introduce a new for-each-ref format called 'refbasename', which strips the leading parts of the refname: * If the refname doesn't contain any '/', then it is printed as is. * If the refname contains one '/', then the string following the '/' is printed (e.g. 'refs/foo' becomes 'foo'). * If the refname contains two (or more) '/', then the string following the second '/' is printed (e.g. 'refs/heads/foo' becomes 'foo'). Signed-off-by: SZEDER Gábor --- On Fri, Aug 29, 2008 at 07:34:48AM -0700, Shawn O. Pearce wrote: > SZEDER GGGbor wrote: > > + } else if (!strcmp(name, "refbasename")) { > > + char * p = strchr(ref->refname, '/'); > > + p = strchr(p+1, '/'); > > + v->s = p+1; > > Please be careful here and check for !p. A refname may be missing > one or two '/' in which case you will cause the process to segfault. > > I don't think its a good idea to assume you'll always have to '/' > in the name. "refs/foo" can be created by git-update-ref. Or if > we ever started to report on HEAD this output tag would crash. Ah, I feared as much. This raises the question what should the refbasename format print in those cases. The first case (no '/' at all) is trivial, but the other two are a bit problematic: * The refname 'foo/bar' seems to be valid, or at least the command sequence 'mkdir .git/foo; cp .git/refs/heads/master .git/foo/bar; git checkout foo/bar' works as expected. OTOH, 'git for-each-ref --format="%(refname)"' doesn't print anything. * The refname 'refs/head/foo/bar' should become 'foo/bar': it's in sync with how 'git branch' lists it, and this behaviour is needed by the bash completion changes. However, unfortunately it contradicts to the format's name, because basename removes all directory components. Maybe someone has a better idea on how to name this format... builtin-for-each-ref.c | 12 ++++++++++++ t/t6300-for-each-ref.sh | 2 ++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c index 21e92bb..23d064b 100644 --- a/builtin-for-each-ref.c +++ b/builtin-for-each-ref.c @@ -66,6 +66,7 @@ static struct { { "subject" }, { "body" }, { "contents" }, + { "refbasename" }, }; /* @@ -577,6 +578,17 @@ static void populate_value(struct refinfo *ref) char *s = xmalloc(len + 4); sprintf(s, "%s^{}", ref->refname); v->s = s; + } else if (!strcmp(name, "refbasename")) { + char * slash1 = strchr(ref->refname, '/'); + if (!slash1) + v->s = ref->refname; + else { + char * slash2 = strchr(slash1 + 1, '/'); + if (!slash2) + v->s = slash1 + 1; + else + v->s = slash2 + 1; + } } } diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 8ced593..b317a01 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -66,6 +66,7 @@ test_atom head subject 'Initial' test_atom head body '' test_atom head contents 'Initial ' +test_atom head refbasename 'master' test_atom tag refname refs/tags/testtag test_atom tag objecttype tag @@ -95,6 +96,7 @@ test_atom tag subject 'Tagging at 1151939927' test_atom tag body '' test_atom tag contents 'Tagging at 1151939927 ' +test_atom tag refbasename 'testtag' test_expect_success 'Check invalid atoms names are errors' ' test_must_fail git-for-each-ref --format="%(INVALID)" refs/heads -- 1.6.0.1.149.g903b0 ``` ## Bert Wesarg, 2008-08-29 18:21 Subject: Re: [RFC/PATCH 1/2] for-each-ref: add new format 'refbasename' Message-ID: <36ca99e90808291121j7b202917r2308719970668be3@mail.gmail.com> URL: https://gitlist.dev/e/36ca99e90808291121j7b202917r2308719970668be3%40mail.gmail.com In-Reply-To: <0329f1ab2642e1dab701b5fc6517bfb1b4ea2d46.1220004755.git.szeder@ira.uka.de> ``` On Fri, Aug 29, 2008 at 12:37, SZEDER Gábor wrote: > fot-each-ref's refname format outputs each ref in its full format, e.g. > 'refs/heads/foo' and 'refs/tags/bar'. However, there are tools that > need only the last part of the refname, e.g. only 'foo' and 'bar'. Such > a tool is git's bash completion script, which spends considerable amount > of time removing the unneeded parts from for-each-ref's output. > > Therefore, we introduce a new for-each-ref format called 'refbasename', > which strips everything before and including the second '/' in the ref's > name from the output. Why not strip the pattern (given on the command line) from the refname? I.e. with a new format 'refshort' a 'git for-each-ref --format=%(refshort) refs/heads/' would strip off the refs/heads/ from each ref. This should also work for multiple patterns (refs/heads/ refs/tags/) and for patterns which needs a fnmatch call (which has the FNM_PATHNAME option). The bash completion script calls for-each-ref always with patterns. Bert ``` ## Bert Wesarg, 2008-08-29 21:41 Subject: [PATCH] for-each-ref: new 'refshort' format Message-ID: <1220046116-23703-1-git-send-email-bert.wesarg@googlemail.com> URL: https://gitlist.dev/e/1220046116-23703-1-git-send-email-bert.wesarg%40googlemail.com In-Reply-To: <36ca99e90808291121j7b202917r2308719970668be3@mail.gmail.com> ``` Here is a first sketch for this idea. This strips from the refname the common prefix with the matched pattern. Signed-off-by: Bert Wesarg --- I think we should use a ':short' modifier to 'reflog' and '*reflog'. builtin-for-each-ref.c | 39 +++++++++++++++++++++++++++++++++++++-- 1 files changed, 37 insertions(+), 2 deletions(-) diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c index 21e92bb..b80d753 100644 --- a/builtin-for-each-ref.c +++ b/builtin-for-each-ref.c @@ -31,6 +31,7 @@ struct ref_sort { struct refinfo { char *refname; + const char *pattern; /* the pattern which matched this ref */ unsigned char objectname[20]; struct atom_value *value; }; @@ -66,6 +67,7 @@ static struct { { "subject" }, { "body" }, { "contents" }, + { "refshort" }, }; /* @@ -546,6 +548,36 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v } /* + * Use the matched pattern from ref to shorten the refname + */ +static char *get_short_ref(struct refinfo *ref) +{ + int rlen, plen, len = 0; + + if (!ref->pattern) + return ref->refname; + + rlen = strlen(ref->refname); + plen = strlen(ref->pattern); + + if ((plen <= rlen) && + !strncmp(ref->refname, ref->pattern, plen) && + (ref->refname[plen] == '\0' || + ref->refname[plen] == '/' || + ref->pattern[plen - 1] == '/')) { + len = plen + (ref->refname[plen] == '/'); + } else { + len = strcspn(ref->pattern, "*?["); + while (len >= 0 && ref->pattern[len] != '/') + --len; + len++; + } + + return ref->refname + len; +} + + +/* * Parse the object referred by ref, and grab needed value. */ static void populate_value(struct refinfo *ref) @@ -577,6 +609,8 @@ static void populate_value(struct refinfo *ref) char *s = xmalloc(len + 4); sprintf(s, "%s^{}", ref->refname); v->s = s; + } else if (!strcmp(name, "refshort")) { + v->s = get_short_ref(ref); } } @@ -641,9 +675,9 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f struct grab_ref_cbdata *cb = cb_data; struct refinfo *ref; int cnt; + const char **pattern = cb->grab_pattern; - if (*cb->grab_pattern) { - const char **pattern; + if (*pattern) { int namelen = strlen(refname); for (pattern = cb->grab_pattern; *pattern; pattern++) { const char *p = *pattern; @@ -668,6 +702,7 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f */ ref = xcalloc(1, sizeof(*ref)); ref->refname = xstrdup(refname); + ref->pattern = *pattern; hashcpy(ref->objectname, sha1); cnt = cb->grab_cnt; -- 1.6.0 ``` ## Junio C Hamano, 2008-08-31 04:31 Subject: Re: [PATCH] for-each-ref: new 'refshort' format Message-ID: <7vmyiterfd.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmyiterfd.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1220046116-23703-1-git-send-email-bert.wesarg@googlemail.com> ``` Bert Wesarg writes: > Here is a first sketch for this idea. > > This strips from the refname the common prefix with the matched pattern. Looks quite straightforward and to the point. ``` ## Bert Wesarg, 2008-08-31 07:11 Subject: Re: [PATCH] for-each-ref: new 'refshort' format Message-ID: <36ca99e90808310011s3af5929fw5bb67be2bff4fab@mail.gmail.com> URL: https://gitlist.dev/e/36ca99e90808310011s3af5929fw5bb67be2bff4fab%40mail.gmail.com In-Reply-To: <7vmyiterfd.fsf@gitster.siamese.dyndns.org> ``` On Sun, Aug 31, 2008 at 06:31, Junio C Hamano wrote: > Bert Wesarg writes: > >> Here is a first sketch for this idea. >> >> This strips from the refname the common prefix with the matched pattern. > > Looks quite straightforward and to the point. > What do you think about the ':short' modifier for 'refname' or '*refname' instead 'refshort'? i.e. %(refname:short) Bert ```