* [PATCH] builtin-branch: improve output when displaying remote branches @ 2009-02-10 11:01 Jay Soffian 2009-02-11 6:47 ` Jay Soffian 2009-02-12 3:49 ` Junio C Hamano 0 siblings, 2 replies; 14+ messages in thread From: Jay Soffian @ 2009-02-10 11:01 UTC (permalink / raw) To: git; +Cc: Jay Soffian When displaying local and remote branches, prefix the remote branch names with "remotes/" to make the remote branches clear from the local branches. If displaying only the remote branches, the prefix is not shown since it would be redundant. When displaying a remote branch HEAD (which is a sane symref), show what it points to similar to how "ls -l" show symlinks. Also in this case, do not show verbose output for the HEAD itself as it is shown immediately below on the pointed to branch. Signed-off-by: Jay Soffian <jaysoffian@gmail.com> --- I think this addresses the feedback I got on the original patch, http://article.gmane.org/gmane.comp.version-control.git/109161 Some sample output: $ git branch -a master next * wip/branch-show-remote-HEAD-2 wip/push-docs remotes/origin/HEAD -> master remotes/origin/html remotes/origin/maint remotes/origin/man remotes/origin/master remotes/origin/next remotes/origin/pu remotes/origin/todo $ git branch -r origin/HEAD -> master origin/html origin/maint origin/man origin/master origin/next origin/pu origin/todo $ git branch -rv origin/HEAD -> master origin/html 6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9 origin/maint 7e1100e gitweb: add $prevent_xss option to prevent XSS by repository content origin/man 67cb1a7 Autogenerated manpages for v1.6.2-rc0-10-gf6b9 origin/master f6b98e4 git-web--browse: Fix check for /bin/start origin/next 417ce12 Merge branch 'master' into next origin/pu 9d798e7 Merge branch 'db/foreign-scm' into pu origin/todo 5ed7079 What's in update Notice that the verbose output for HEAD is squelched as it would be identical to what is shown below in "origin/master". Of course, if <remote>/HEAD does not resolve as a symref pointing to a branch inside <remote> (I don't know why this would happen though...), then it is shown like the other branches. BTW, I noticed that "git branch -a --merged" and "git branch -av --merged" return a different set of branches. I'm not sure why though (but it isn't due to this patch). builtin-branch.c | 68 ++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 51 insertions(+), 17 deletions(-) diff --git a/builtin-branch.c b/builtin-branch.c index 56a1971..03ad757 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) struct ref_item { char *name; - unsigned int kind; + char *dest; + unsigned int kind, len; struct commit *commit; }; @@ -193,13 +194,23 @@ struct ref_list { int kinds; }; +static char *resolve_remote_head_symref(const char *head_name) { + unsigned char sha1[20]; + int flag; + const char *refname; + refname = resolve_ref(head_name, sha1, 0, &flag); + if (refname && (flag & REF_ISSYMREF) && + !prefixcmp(refname, "refs/remotes/")) + return xstrdup(refname + strlen(head_name) - 4); + return NULL; +} + static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) { struct ref_list *ref_list = (struct ref_list*)(cb_data); struct ref_item *newitem; struct commit *commit; int kind; - int len; /* Detect kind */ if (!prefixcmp(refname, "refs/heads/")) { @@ -239,9 +250,20 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, newitem->name = xstrdup(refname); newitem->kind = kind; newitem->commit = commit; - len = strlen(newitem->name); - if (len > ref_list->maxwidth) - ref_list->maxwidth = len; + newitem->len = strlen(newitem->name); + newitem->dest = (newitem->kind == REF_REMOTE_BRANCH && + newitem->len > 5 && + !strcmp(newitem->name + newitem->len - 5, "/HEAD")) + ? resolve_remote_head_symref(refname - 13) : NULL; + /* adjust for " -> " */ + if (newitem->dest) + newitem->len += strlen(newitem->dest) + 4; + /* adjust for "remotes/" */ + if (newitem->kind == REF_REMOTE_BRANCH && + ref_list->kinds != REF_REMOTE_BRANCH) + newitem->len += 8; + if (newitem->len > ref_list->maxwidth) + ref_list->maxwidth = newitem->len; return 0; } @@ -250,8 +272,11 @@ static void free_ref_list(struct ref_list *ref_list) { int i; - for (i = 0; i < ref_list->index; i++) + for (i = 0; i < ref_list->index; i++) { free(ref_list->list[i].name); + if (ref_list->list[i].dest) + free(ref_list->list[i].dest); + } free(ref_list->list); } @@ -292,7 +317,7 @@ static int matches_merge_filter(struct commit *commit) } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, - int abbrev, int current) + int abbrev, int current, char *prefix) { char c; int color; @@ -319,8 +344,13 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = COLOR_BRANCH_CURRENT; } - if (verbose) { + if (item->dest) { + printf("%c %s%s%s%s -> %s\n", c, branch_get_color(color), + prefix, item->name, + branch_get_color(COLOR_BRANCH_RESET), item->dest); + } else if (verbose) { struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT; + struct strbuf name = STRBUF_INIT; const char *sub = " **** invalid ref ****"; commit = item->commit; @@ -333,28 +363,29 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, if (item->kind == REF_LOCAL_BRANCH) fill_tracking_info(&stat, item->name); + strbuf_addf(&name, "%s%s", prefix, item->name); printf("%c %s%-*s%s %s %s%s\n", c, branch_get_color(color), - maxwidth, item->name, + maxwidth, name.buf, branch_get_color(COLOR_BRANCH_RESET), find_unique_abbrev(item->commit->object.sha1, abbrev), stat.buf, sub); strbuf_release(&stat); strbuf_release(&subject); + strbuf_release(&name); } else { - printf("%c %s%s%s\n", c, branch_get_color(color), item->name, - branch_get_color(COLOR_BRANCH_RESET)); + printf("%c %s%s%s%s\n", c, branch_get_color(color), prefix, + item->name, branch_get_color(COLOR_BRANCH_RESET)); } } static int calc_maxwidth(struct ref_list *refs) { - int i, l, w = 0; + int i, w = 0; for (i = 0; i < refs->index; i++) { if (!matches_merge_filter(refs->list[i].commit)) continue; - l = strlen(refs->list[i].name); - if (l > w) - w = l; + if (refs->list[i].len > w) + w = refs->list[i].len; } return w; } @@ -394,7 +425,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str item.commit = head_commit; if (strlen(item.name) > ref_list.maxwidth) ref_list.maxwidth = strlen(item.name); - print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1); + print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, ""); free(item.name); } @@ -402,8 +433,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str int current = !detached && (ref_list.list[i].kind == REF_LOCAL_BRANCH) && !strcmp(ref_list.list[i].name, head); + char *prefix = (kinds != REF_REMOTE_BRANCH && + ref_list.list[i].kind == REF_REMOTE_BRANCH) + ? "remotes/" : ""; print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose, - abbrev, current); + abbrev, current, prefix); } free_ref_list(&ref_list); -- 1.6.2.rc0.12.gbd893 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] builtin-branch: improve output when displaying remote branches 2009-02-10 11:01 [PATCH] builtin-branch: improve output when displaying remote branches Jay Soffian @ 2009-02-11 6:47 ` Jay Soffian 2009-02-12 3:49 ` Junio C Hamano 1 sibling, 0 replies; 14+ messages in thread From: Jay Soffian @ 2009-02-11 6:47 UTC (permalink / raw) To: git; +Cc: Junio C Hamano Bump, since this complements gmane 109374. On Tue, Feb 10, 2009 at 6:01 AM, Jay Soffian <jaysoffian@gmail.com> wrote: > When displaying local and remote branches, prefix the remote branch names with > "remotes/" to make the remote branches clear from the local branches. If > displaying only the remote branches, the prefix is not shown since it would be > redundant. > > When displaying a remote branch HEAD (which is a sane symref), show what it > points to similar to how "ls -l" show symlinks. Also in this case, do not show > verbose output for the HEAD itself as it is shown immediately below on the > pointed to branch. > > Signed-off-by: Jay Soffian <jaysoffian@gmail.com> > --- > I think this addresses the feedback I got on the original patch, > http://article.gmane.org/gmane.comp.version-control.git/109161 > > Some sample output: > > $ git branch -a > master > next > * wip/branch-show-remote-HEAD-2 > wip/push-docs > remotes/origin/HEAD -> master > remotes/origin/html > remotes/origin/maint > remotes/origin/man > remotes/origin/master > remotes/origin/next > remotes/origin/pu > remotes/origin/todo > > $ git branch -r > origin/HEAD -> master > origin/html > origin/maint > origin/man > origin/master > origin/next > origin/pu > origin/todo > > $ git branch -rv > origin/HEAD -> master > origin/html 6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9 > origin/maint 7e1100e gitweb: add $prevent_xss option to prevent XSS by repository content > origin/man 67cb1a7 Autogenerated manpages for v1.6.2-rc0-10-gf6b9 > origin/master f6b98e4 git-web--browse: Fix check for /bin/start > origin/next 417ce12 Merge branch 'master' into next > origin/pu 9d798e7 Merge branch 'db/foreign-scm' into pu > origin/todo 5ed7079 What's in update > > Notice that the verbose output for HEAD is squelched as it would be identical to > what is shown below in "origin/master". Of course, if <remote>/HEAD does not > resolve as a symref pointing to a branch inside <remote> (I don't know why this > would happen though...), then it is shown like the other branches. > > BTW, I noticed that "git branch -a --merged" and "git branch -av --merged" > return a different set of branches. I'm not sure why though (but it isn't due to > this patch). > > builtin-branch.c | 68 ++++++++++++++++++++++++++++++++++++++++------------- > 1 files changed, 51 insertions(+), 17 deletions(-) > > diff --git a/builtin-branch.c b/builtin-branch.c > index 56a1971..03ad757 100644 > --- a/builtin-branch.c > +++ b/builtin-branch.c > @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) > > struct ref_item { > char *name; > - unsigned int kind; > + char *dest; > + unsigned int kind, len; > struct commit *commit; > }; > > @@ -193,13 +194,23 @@ struct ref_list { > int kinds; > }; > > +static char *resolve_remote_head_symref(const char *head_name) { > + unsigned char sha1[20]; > + int flag; > + const char *refname; > + refname = resolve_ref(head_name, sha1, 0, &flag); > + if (refname && (flag & REF_ISSYMREF) && > + !prefixcmp(refname, "refs/remotes/")) > + return xstrdup(refname + strlen(head_name) - 4); > + return NULL; > +} > + > static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) > { > struct ref_list *ref_list = (struct ref_list*)(cb_data); > struct ref_item *newitem; > struct commit *commit; > int kind; > - int len; > > /* Detect kind */ > if (!prefixcmp(refname, "refs/heads/")) { > @@ -239,9 +250,20 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, > newitem->name = xstrdup(refname); > newitem->kind = kind; > newitem->commit = commit; > - len = strlen(newitem->name); > - if (len > ref_list->maxwidth) > - ref_list->maxwidth = len; > + newitem->len = strlen(newitem->name); > + newitem->dest = (newitem->kind == REF_REMOTE_BRANCH && > + newitem->len > 5 && > + !strcmp(newitem->name + newitem->len - 5, "/HEAD")) > + ? resolve_remote_head_symref(refname - 13) : NULL; > + /* adjust for " -> " */ > + if (newitem->dest) > + newitem->len += strlen(newitem->dest) + 4; > + /* adjust for "remotes/" */ > + if (newitem->kind == REF_REMOTE_BRANCH && > + ref_list->kinds != REF_REMOTE_BRANCH) > + newitem->len += 8; > + if (newitem->len > ref_list->maxwidth) > + ref_list->maxwidth = newitem->len; > > return 0; > } > @@ -250,8 +272,11 @@ static void free_ref_list(struct ref_list *ref_list) > { > int i; > > - for (i = 0; i < ref_list->index; i++) > + for (i = 0; i < ref_list->index; i++) { > free(ref_list->list[i].name); > + if (ref_list->list[i].dest) > + free(ref_list->list[i].dest); > + } > free(ref_list->list); > } > > @@ -292,7 +317,7 @@ static int matches_merge_filter(struct commit *commit) > } > > static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, > - int abbrev, int current) > + int abbrev, int current, char *prefix) > { > char c; > int color; > @@ -319,8 +344,13 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, > color = COLOR_BRANCH_CURRENT; > } > > - if (verbose) { > + if (item->dest) { > + printf("%c %s%s%s%s -> %s\n", c, branch_get_color(color), > + prefix, item->name, > + branch_get_color(COLOR_BRANCH_RESET), item->dest); > + } else if (verbose) { > struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT; > + struct strbuf name = STRBUF_INIT; > const char *sub = " **** invalid ref ****"; > > commit = item->commit; > @@ -333,28 +363,29 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, > if (item->kind == REF_LOCAL_BRANCH) > fill_tracking_info(&stat, item->name); > > + strbuf_addf(&name, "%s%s", prefix, item->name); > printf("%c %s%-*s%s %s %s%s\n", c, branch_get_color(color), > - maxwidth, item->name, > + maxwidth, name.buf, > branch_get_color(COLOR_BRANCH_RESET), > find_unique_abbrev(item->commit->object.sha1, abbrev), > stat.buf, sub); > strbuf_release(&stat); > strbuf_release(&subject); > + strbuf_release(&name); > } else { > - printf("%c %s%s%s\n", c, branch_get_color(color), item->name, > - branch_get_color(COLOR_BRANCH_RESET)); > + printf("%c %s%s%s%s\n", c, branch_get_color(color), prefix, > + item->name, branch_get_color(COLOR_BRANCH_RESET)); > } > } > > static int calc_maxwidth(struct ref_list *refs) > { > - int i, l, w = 0; > + int i, w = 0; > for (i = 0; i < refs->index; i++) { > if (!matches_merge_filter(refs->list[i].commit)) > continue; > - l = strlen(refs->list[i].name); > - if (l > w) > - w = l; > + if (refs->list[i].len > w) > + w = refs->list[i].len; > } > return w; > } > @@ -394,7 +425,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str > item.commit = head_commit; > if (strlen(item.name) > ref_list.maxwidth) > ref_list.maxwidth = strlen(item.name); > - print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1); > + print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, ""); > free(item.name); > } > > @@ -402,8 +433,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str > int current = !detached && > (ref_list.list[i].kind == REF_LOCAL_BRANCH) && > !strcmp(ref_list.list[i].name, head); > + char *prefix = (kinds != REF_REMOTE_BRANCH && > + ref_list.list[i].kind == REF_REMOTE_BRANCH) > + ? "remotes/" : ""; > print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose, > - abbrev, current); > + abbrev, current, prefix); > } > > free_ref_list(&ref_list); > -- > 1.6.2.rc0.12.gbd893 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] builtin-branch: improve output when displaying remote branches 2009-02-10 11:01 [PATCH] builtin-branch: improve output when displaying remote branches Jay Soffian 2009-02-11 6:47 ` Jay Soffian @ 2009-02-12 3:49 ` Junio C Hamano 2009-02-12 4:30 ` Jay Soffian 1 sibling, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2009-02-12 3:49 UTC (permalink / raw) To: Jay Soffian; +Cc: git Jay Soffian <jaysoffian@gmail.com> writes: > $ git branch -rv > origin/HEAD -> master > origin/html 6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9 Doesn't the misalignment between the above two bother you? > diff --git a/builtin-branch.c b/builtin-branch.c > index 56a1971..03ad757 100644 > --- a/builtin-branch.c > +++ b/builtin-branch.c > @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) > +static char *resolve_remote_head_symref(const char *head_name) { > + unsigned char sha1[20]; > + int flag; > + const char *refname; > + refname = resolve_ref(head_name, sha1, 0, &flag); > + if (refname && (flag & REF_ISSYMREF) && > + !prefixcmp(refname, "refs/remotes/")) > + return xstrdup(refname + strlen(head_name) - 4); Here, head_name is like "refs/remotes/frotz/HEAD", and you are assuming that resolved refname begins with "refs/remotes/frotz/" without checking the "frotz" part. It may point at "refs/remotes/x/y" in a misconfigured repository and your xstrdup() just ran past the end of the string. If the ref you feed to this function turns out not to be a symbolic ref, the caller does do the right thing. It makes wonder if your caller should always call this, so that you would still work sensibly even if the tracking hierarchy has a funny symref refs/remotes/origin/TAIL that is not HEAD. The caller is currently this dense code. > + newitem->len = strlen(newitem->name); > + newitem->dest = (newitem->kind == REF_REMOTE_BRANCH && > + newitem->len > 5 && > + !strcmp(newitem->name + newitem->len - 5, "/HEAD")) > + ? resolve_remote_head_symref(refname - 13) : NULL; > + /* adjust for " -> " */ > + if (newitem->dest) > + newitem->len += strlen(newitem->dest) + 4; It can become something like: if (newitem->kind == REF_REMOTE_BRANCH) newitem->dest = resolve_remote_symref(refname - 13); else newitem->dest = NULL; if (newitem->dest) ... no? > @@ -250,8 +272,11 @@ static void free_ref_list(struct ref_list *ref_list) > { > int i; > > - for (i = 0; i < ref_list->index; i++) > + for (i = 0; i < ref_list->index; i++) { > free(ref_list->list[i].name); > + if (ref_list->list[i].dest) > + free(ref_list->list[i].dest); > + } free(NULL) is Ok; omit the extra check. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] builtin-branch: improve output when displaying remote branches 2009-02-12 3:49 ` Junio C Hamano @ 2009-02-12 4:30 ` Jay Soffian 2009-02-12 5:42 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Jay Soffian @ 2009-02-12 4:30 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Wed, Feb 11, 2009 at 10:49 PM, Junio C Hamano <gitster@pobox.com> wrote: > Jay Soffian <jaysoffian@gmail.com> writes: > >> $ git branch -rv >> origin/HEAD -> master >> origin/html 6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9 > > Doesn't the misalignment between the above two bother you? This comment makes me sad. In fact, a previous iteration looked like this: $ git branch -rv origin/HEAD -> master origin/html 6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9 origin/maint 7e1100e gitweb: add $prevent_xss option to prevent XSS by repository content origin/man 67cb1a7 Autogenerated manpages for v1.6.2-rc0-10-gf6b9 origin/master f6b98e4 git-web--browse: Fix check for /bin/start origin/next 417ce12 Merge branch 'master' into next origin/pu 9d798e7 Merge branch 'db/foreign-scm' into pu origin/todo 5ed7079 What's in update IOW, align based on the width of the branch name, completely ignoring the width of " -> ...". But I found that ugly. It was actually more work to get it the way it is. >> diff --git a/builtin-branch.c b/builtin-branch.c >> index 56a1971..03ad757 100644 >> --- a/builtin-branch.c >> +++ b/builtin-branch.c >> @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) >> +static char *resolve_remote_head_symref(const char *head_name) { >> + unsigned char sha1[20]; >> + int flag; >> + const char *refname; >> + refname = resolve_ref(head_name, sha1, 0, &flag); >> + if (refname && (flag & REF_ISSYMREF) && >> + !prefixcmp(refname, "refs/remotes/")) >> + return xstrdup(refname + strlen(head_name) - 4); > > Here, head_name is like "refs/remotes/frotz/HEAD", and you are assuming > that resolved refname begins with "refs/remotes/frotz/" without checking > the "frotz" part. It may point at "refs/remotes/x/y" in a misconfigured > repository and your xstrdup() just ran past the end of the string. Indeed. Now I'm doubly-sad, that my code sucks so bad. :-( > If the ref you feed to this function turns out not to be a symbolic ref, > the caller does do the right thing. It makes wonder if your caller should > always call this, so that you would still work sensibly even if the tracking > hierarchy has a funny symref refs/remotes/origin/TAIL that is not HEAD. > > The caller is currently this dense code. > >> + newitem->len = strlen(newitem->name); >> + newitem->dest = (newitem->kind == REF_REMOTE_BRANCH && >> + newitem->len > 5 && >> + !strcmp(newitem->name + newitem->len - 5, "/HEAD")) >> + ? resolve_remote_head_symref(refname - 13) : NULL; >> + /* adjust for " -> " */ >> + if (newitem->dest) >> + newitem->len += strlen(newitem->dest) + 4; > > It can become something like: > > if (newitem->kind == REF_REMOTE_BRANCH) > newitem->dest = resolve_remote_symref(refname - 13); > else > newitem->dest = NULL; > if (newitem->dest) > ... > > no? Yes indeed. I'll re-roll to to clean this up, but I'm keeping the visual output the same unless you really don't like it. > free(NULL) is Ok; omit the extra check. Got it. I think I did something similar in the builtin-remote patch I sent you earlier, so I'll make sure to fix that there too when I re-roll that one. Thanks for the review. j. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] builtin-branch: improve output when displaying remote branches 2009-02-12 4:30 ` Jay Soffian @ 2009-02-12 5:42 ` Junio C Hamano 2009-02-13 5:34 ` [PATCH v2] " Jay Soffian 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2009-02-12 5:42 UTC (permalink / raw) To: Jay Soffian; +Cc: git Jay Soffian <jaysoffian@gmail.com> writes: > On Wed, Feb 11, 2009 at 10:49 PM, Junio C Hamano <gitster@pobox.com> wrote: >> Jay Soffian <jaysoffian@gmail.com> writes: >> >>> $ git branch -rv >>> origin/HEAD -> master >>> origin/html 6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9 >> >> Doesn't the misalignment between the above two bother you? > > This comment makes me sad. In fact, a previous iteration looked like this: > > $ git branch -rv > origin/HEAD -> master > origin/html 6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9 > origin/maint 7e1100e gitweb: add $prevent_xss option to prevent XSS > by repository content > origin/man 67cb1a7 Autogenerated manpages for v1.6.2-rc0-10-gf6b9 > origin/master f6b98e4 git-web--browse: Fix check for /bin/start > origin/next 417ce12 Merge branch 'master' into next > origin/pu 9d798e7 Merge branch 'db/foreign-scm' into pu > origin/todo 5ed7079 What's in update > > IOW, align based on the width of the branch name, completely ignoring > the width of " -> ...". But I found that ugly. It was actually more > work to get it the way it is. Wouldn't something like this easier to read? > origin/HEAD ------> master > origin/html 6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9 > origin/maint 7e1100e gitweb: add $prevent_xss option to prevent XSS I am not sure about the long arrow. It may be easier to use "->" aligned th the right end, but that now falls into bikeshedding, so I'll leve that to the list. >> It can become something like: >> >> if (newitem->kind == REF_REMOTE_BRANCH) >> newitem->dest = resolve_remote_symref(refname - 13); >> else >> newitem->dest = NULL; >> if (newitem->dest) >> ... >> >> no? > > Yes indeed. Another thing you may want to consider is to introduce another variable "name" that is supposed to be the human readable part (i.e. refname-13 etc.) and leave the refname the full name starting from "refs/". The code to add and then subtract to go back and forth made my head spin when I read it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] builtin-branch: improve output when displaying remote branches 2009-02-12 5:42 ` Junio C Hamano @ 2009-02-13 5:34 ` Jay Soffian 2009-02-13 6:35 ` Junio C Hamano 2009-02-13 7:37 ` Johannes Sixt 0 siblings, 2 replies; 14+ messages in thread From: Jay Soffian @ 2009-02-13 5:34 UTC (permalink / raw) To: git; +Cc: Jay Soffian, gitster When encountering a symref (typically refs/remotes/<remote>/HEAD), display the ref target. When displaying local and remote branches, prefix the remote branch names with "remotes/" to make the remote branches clear from the local branches. If displaying only the remote branches, the prefix is not shown since it would be redundant. Signed-off-by: Jay Soffian <jaysoffian@gmail.com> --- Changes from v1: * I now resolve any symref that I encounter, not just HEAD. As long as it's a valid symref, we show what it points to. If the target points into the same namespace as the source (refs/heads/ or refs/remotes/), then we strip the prefix, otherwise we display the complete target path. So if someone has done something silly like having a refs/remotes/frotz/blarg that points to refs/heads/blam, then we show it as: frotz/blarg -> refs/heads/blam But in the typical case of refs/remotes/origin/HEAD, it just looks like: origin/HEAD -> origin/master * I decided to leave the "origin" (or whatever...) prefix on the target since I think it is clearer. * I rewrote print_ref_item() to use a strbuf for generating the output -- the code is clearer that way w/all the output permutations now. Here's some sample output: $ git branch foo -> master * master rather-long-branch-name $ git branch -v foo -> master * master 51cecb2 initial rather-long-branch-name 51cecb2 initial $ git branch -v --no-abbrev foo -> master * master 51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial rather-long-branch-name 51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial $ git branch -r frotz/HEAD -> frotz/master frotz/master origin/HEAD -> origin/master origin/WTF -> refs/heads/master origin/master $ git branch -a foo -> master * master rather-long-branch-name remotes/frotz/HEAD -> frotz/master remotes/frotz/master remotes/origin/HEAD -> origin/master remotes/origin/WTF -> refs/heads/master remotes/origin/master $ git branch -rv frotz/HEAD -> frotz/master frotz/master e1d8130 added file2 origin/HEAD -> origin/master origin/WTF -> refs/heads/master origin/master e1d8130 added file2 $ git branch -av foo -> master * master 51cecb2 initial rather-long-branch-name 51cecb2 initial remotes/frotz/HEAD -> frotz/master remotes/frotz/master e1d8130 added file2 remotes/origin/HEAD -> origin/master remotes/origin/WTF -> refs/heads/master remotes/origin/master e1d8130 added file2 In the case of verbose output, I did play with making the arrow the same width as the commit id (which means accounting for abbrev=<n> and --no-abbrev btw...), but it really didn't look any better IMO. builtin-branch.c | 80 +++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 58 insertions(+), 22 deletions(-) diff --git a/builtin-branch.c b/builtin-branch.c index 56a1971..4154ad2 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) struct ref_item { char *name; - unsigned int kind; + char *dest; + unsigned int kind, len; struct commit *commit; }; @@ -193,21 +194,37 @@ struct ref_list { int kinds; }; +static char *resolve_symref(const char *src, const char *prefix) +{ + unsigned char sha1[20]; + int flag; + const char *dst, *cp; + dst = resolve_ref(src, sha1, 0, &flag); + if (!(dst && (flag & REF_ISSYMREF))) + return NULL; + if (prefix && !prefixcmp(dst, prefix)) + return xstrdup(skip_prefix(dst, prefix)); + else + return xstrdup(dst); +} + static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) { struct ref_list *ref_list = (struct ref_list*)(cb_data); struct ref_item *newitem; struct commit *commit; int kind; - int len; + const char *prefix, *orig_refname = refname; /* Detect kind */ if (!prefixcmp(refname, "refs/heads/")) { kind = REF_LOCAL_BRANCH; refname += 11; + prefix = "refs/heads/"; } else if (!prefixcmp(refname, "refs/remotes/")) { kind = REF_REMOTE_BRANCH; refname += 13; + prefix = "refs/remotes/"; } else return 0; @@ -239,9 +256,14 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, newitem->name = xstrdup(refname); newitem->kind = kind; newitem->commit = commit; - len = strlen(newitem->name); - if (len > ref_list->maxwidth) - ref_list->maxwidth = len; + newitem->len = strlen(refname); + newitem->dest = resolve_symref(orig_refname, prefix); + /* adjust for "remotes/" */ + if (newitem->kind == REF_REMOTE_BRANCH && + ref_list->kinds != REF_REMOTE_BRANCH) + newitem->len += 8; + if (newitem->len > ref_list->maxwidth) + ref_list->maxwidth = newitem->len; return 0; } @@ -250,8 +272,10 @@ static void free_ref_list(struct ref_list *ref_list) { int i; - for (i = 0; i < ref_list->index; i++) + for (i = 0; i < ref_list->index; i++) { free(ref_list->list[i].name); + free(ref_list->list[i].dest); + } free(ref_list->list); } @@ -292,11 +316,12 @@ static int matches_merge_filter(struct commit *commit) } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, - int abbrev, int current) + int abbrev, int current, char *prefix) { char c; int color; struct commit *commit = item->commit; + struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; if (!matches_merge_filter(commit)) return; @@ -319,7 +344,18 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = COLOR_BRANCH_CURRENT; } - if (verbose) { + strbuf_addf(&name, "%s%s", prefix, item->name); + if (verbose) + strbuf_addf(&out, "%c %s%-*s%s", c, branch_get_color(color), + maxwidth, name.buf, + branch_get_color(COLOR_BRANCH_RESET)); + else + strbuf_addf(&out, "%c %s%s%s", c, branch_get_color(color), + name.buf, branch_get_color(COLOR_BRANCH_RESET)); + + if (item->dest) + strbuf_addf(&out, " -> %s", item->dest); + else if (verbose) { struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT; const char *sub = " **** invalid ref ****"; @@ -333,28 +369,25 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, if (item->kind == REF_LOCAL_BRANCH) fill_tracking_info(&stat, item->name); - printf("%c %s%-*s%s %s %s%s\n", c, branch_get_color(color), - maxwidth, item->name, - branch_get_color(COLOR_BRANCH_RESET), - find_unique_abbrev(item->commit->object.sha1, abbrev), - stat.buf, sub); + strbuf_addf(&out, " %s %s%s", + find_unique_abbrev(item->commit->object.sha1, abbrev), + stat.buf, sub); strbuf_release(&stat); strbuf_release(&subject); - } else { - printf("%c %s%s%s\n", c, branch_get_color(color), item->name, - branch_get_color(COLOR_BRANCH_RESET)); } + printf("%s\n", out.buf); + strbuf_release(&name); + strbuf_release(&out); } static int calc_maxwidth(struct ref_list *refs) { - int i, l, w = 0; + int i, w = 0; for (i = 0; i < refs->index; i++) { if (!matches_merge_filter(refs->list[i].commit)) continue; - l = strlen(refs->list[i].name); - if (l > w) - w = l; + if (refs->list[i].len > w) + w = refs->list[i].len; } return w; } @@ -394,7 +427,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str item.commit = head_commit; if (strlen(item.name) > ref_list.maxwidth) ref_list.maxwidth = strlen(item.name); - print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1); + print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, ""); free(item.name); } @@ -402,8 +435,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str int current = !detached && (ref_list.list[i].kind == REF_LOCAL_BRANCH) && !strcmp(ref_list.list[i].name, head); + char *prefix = (kinds != REF_REMOTE_BRANCH && + ref_list.list[i].kind == REF_REMOTE_BRANCH) + ? "remotes/" : ""; print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose, - abbrev, current); + abbrev, current, prefix); } free_ref_list(&ref_list); -- 1.6.2.rc0.209.g7c178 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] builtin-branch: improve output when displaying remote branches 2009-02-13 5:34 ` [PATCH v2] " Jay Soffian @ 2009-02-13 6:35 ` Junio C Hamano 2009-02-13 6:45 ` Jay Soffian 2009-02-13 6:47 ` [PATCH v2] " martin f krafft 2009-02-13 7:37 ` Johannes Sixt 1 sibling, 2 replies; 14+ messages in thread From: Junio C Hamano @ 2009-02-13 6:35 UTC (permalink / raw) To: Jay Soffian; +Cc: git, gitster Jay Soffian <jaysoffian@gmail.com> writes: > + if (prefix && !prefixcmp(dst, prefix)) > + return xstrdup(skip_prefix(dst, prefix)); > + else > + return xstrdup(dst); > +} I wonder modern compilers are clever enough to optimze the above to something more like: pfxlen = prefix ? strlen(prefix) : 0; if (pfxlen && !strncmp(dst, prefix, pfxlen)) return xstrdup(dst + pfxlen); else return xstrdup(dst); given that skip_prefix is an inline function but prefixcmp is not (anymore), perhaps not. > static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) > { > struct ref_list *ref_list = (struct ref_list*)(cb_data); > struct ref_item *newitem; > struct commit *commit; > int kind; > - int len; > + const char *prefix, *orig_refname = refname; > > /* Detect kind */ > if (!prefixcmp(refname, "refs/heads/")) { > kind = REF_LOCAL_BRANCH; > refname += 11; > + prefix = "refs/heads/"; > } else if (!prefixcmp(refname, "refs/remotes/")) { > kind = REF_REMOTE_BRANCH; > refname += 13; > + prefix = "refs/remotes/"; > } else > return 0; Once you start making each case arm do more things, it might make sense to rewrite the above unrolled loop into something like this: static struct { int kind; const char *prefix; int pfxlen; } ref_kind[] = { { REF_LOCAL_BRANCH, "refs/heads/", 11 }, { REF_REMOTE_BRANCH, "refs/remotes/", 13 }, }; for (i = 0; i < ARRAY_SIZE(ref_kind); i++) { prefix = ref_kind[i].prefix; if (strncmp(refname, prefix, ref_kind[i].pfxlen)) continue; kind = ref_kind[i].kind; refname += ref_kind[i].pfxlen; break; } if (ARRAY_SIZE(ref_kind) <= i) return 0; Then we can later add new elements more easily, e.g. { REF_TOPGIT_BASE, "refs/top-base/", 14 }, ;-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] builtin-branch: improve output when displaying remote branches 2009-02-13 6:35 ` Junio C Hamano @ 2009-02-13 6:45 ` Jay Soffian 2009-02-13 7:52 ` Junio C Hamano 2009-02-13 6:47 ` [PATCH v2] " martin f krafft 1 sibling, 1 reply; 14+ messages in thread From: Jay Soffian @ 2009-02-13 6:45 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Feb 13, 2009 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote: > Jay Soffian <jaysoffian@gmail.com> writes: > >> + if (prefix && !prefixcmp(dst, prefix)) >> + return xstrdup(skip_prefix(dst, prefix)); >> + else >> + return xstrdup(dst); >> +} > > I wonder modern compilers are clever enough to optimze the above to > something more like: > > pfxlen = prefix ? strlen(prefix) : 0; > if (pfxlen && !strncmp(dst, prefix, pfxlen)) > return xstrdup(dst + pfxlen); > else > return xstrdup(dst); > > given that skip_prefix is an inline function but prefixcmp is not > (anymore), perhaps not. > >> static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) >> { >> struct ref_list *ref_list = (struct ref_list*)(cb_data); >> struct ref_item *newitem; >> struct commit *commit; >> int kind; >> - int len; >> + const char *prefix, *orig_refname = refname; >> >> /* Detect kind */ >> if (!prefixcmp(refname, "refs/heads/")) { >> kind = REF_LOCAL_BRANCH; >> refname += 11; >> + prefix = "refs/heads/"; >> } else if (!prefixcmp(refname, "refs/remotes/")) { >> kind = REF_REMOTE_BRANCH; >> refname += 13; >> + prefix = "refs/remotes/"; >> } else >> return 0; > > Once you start making each case arm do more things, it might make sense to > rewrite the above unrolled loop into something like this: > > static struct { > int kind; > const char *prefix; > int pfxlen; > } ref_kind[] = { > { REF_LOCAL_BRANCH, "refs/heads/", 11 }, > { REF_REMOTE_BRANCH, "refs/remotes/", 13 }, > }; > > for (i = 0; i < ARRAY_SIZE(ref_kind); i++) { > prefix = ref_kind[i].prefix; > if (strncmp(refname, prefix, ref_kind[i].pfxlen)) > continue; > kind = ref_kind[i].kind; > refname += ref_kind[i].pfxlen; > break; > } > if (ARRAY_SIZE(ref_kind) <= i) > return 0; > > Then we can later add new elements more easily, e.g. > > { REF_TOPGIT_BASE, "refs/top-base/", 14 }, > ;-) This strikes me as premature optimization. We're just emitting a few branch names here. I'm beginning to lose my motivation to keep working on this patch. I just wanted to improve the UI slightly. :-( j. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] builtin-branch: improve output when displaying remote branches 2009-02-13 6:45 ` Jay Soffian @ 2009-02-13 7:52 ` Junio C Hamano 2009-02-13 8:06 ` Jay Soffian 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2009-02-13 7:52 UTC (permalink / raw) To: Jay Soffian; +Cc: git Jay Soffian <jaysoffian@gmail.com> writes: > This strikes me as premature optimization. It is *not* an optimization. It is reduction of code duplication to improve maintainability before copy-and-paste gets out of hand. The proper adjective for it is not "premature" but "preemptive". ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] builtin-branch: improve output when displaying remote branches 2009-02-13 7:52 ` Junio C Hamano @ 2009-02-13 8:06 ` Jay Soffian 2009-02-13 9:40 ` [PATCH v3] " Jay Soffian 0 siblings, 1 reply; 14+ messages in thread From: Jay Soffian @ 2009-02-13 8:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Feb 13, 2009 at 2:52 AM, Junio C Hamano <gitster@pobox.com> wrote: > Jay Soffian <jaysoffian@gmail.com> writes: > >> This strikes me as premature optimization. > > It is *not* an optimization. It is reduction of code duplication to > improve maintainability before copy-and-paste gets out of hand. > > The proper adjective for it is not "premature" but "preemptive". I'll re-roll the patch again. However, if you look at the rest of builtin-branch.c, you'll see there are more places than that which are structured similarly (e.g. delete_branches). So if someone wanted to add another branch type, that would be the time (IMO), to refactor the statement at the top of append_ref(). j. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3] builtin-branch: improve output when displaying remote branches 2009-02-13 8:06 ` Jay Soffian @ 2009-02-13 9:40 ` Jay Soffian 0 siblings, 0 replies; 14+ messages in thread From: Jay Soffian @ 2009-02-13 9:40 UTC (permalink / raw) To: git; +Cc: Jay Soffian, gitster, j.sixt, madduck When encountering a symref (typically refs/remotes/<remote>/HEAD), display the ref target. When displaying local and remote branches, prefix the remote branch names with "remotes/" to make the remote branches clear from the local branches. If displaying only the remote branches, the prefix is not shown since it would be redundant. Sample output: $ git branch foo -> master * master rather-long-branch-name $ git branch -v foo -> master * master 51cecb2 initial rather-long-branch-name 51cecb2 initial $ git branch -v --no-abbrev foo -> master * master 51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial rather-long-branch-name 51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial $ git branch -r frotz/HEAD -> frotz/master frotz/master origin/HEAD -> origin/master origin/UNUSUAL -> refs/heads/master origin/master $ git branch -a foo -> master * master rather-long-branch-name remotes/frotz/HEAD -> frotz/master remotes/frotz/master remotes/origin/HEAD -> origin/master remotes/origin/UNUSUAL -> refs/heads/master remotes/origin/master $ git branch -rv frotz/HEAD -> frotz/master frotz/master e1d8130 added file2 origin/HEAD -> origin/master origin/UNUSUAL -> refs/heads/master origin/master e1d8130 added file2 $ git branch -av foo -> master * master 51cecb2 initial rather-long-branch-name 51cecb2 initial remotes/frotz/HEAD -> frotz/master remotes/frotz/master e1d8130 added file2 remotes/origin/HEAD -> origin/master remotes/origin/UNUSUAL -> refs/heads/master remotes/origin/master e1d8130 added file2 Signed-off-by: Jay Soffian <jaysoffian@gmail.com> --- Improvements since v2 - http://thread.gmane.org/gmane.comp.version-control.git/109200/focus=109706 * Moved the sample output into the commit message as suggested by Hannes * Refactored an if statement into a loop as suggested by Junio. This adds more LOC of now, but hopefully it will pay off in the future as git branch learns to emit other branch types than local and remote. I personally disagree w/doing this now as there are other places that will need to be touched to handle a third (or more) branch type. I think at the time the next branch type is added, that is when to look for this type of refactoring. Right now it seems like needless churn to me. With that off my chest, I will defer to the maintainer. :-) * Junio delicately pointed out that I was an idiot :-) :-) :-) to call prefixcmp(), just to call skip_prefix() immediately afterward. I think I did better job this time. builtin-branch.c | 105 ++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 75 insertions(+), 30 deletions(-) diff --git a/builtin-branch.c b/builtin-branch.c index 56a1971..7607f6a 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) struct ref_item { char *name; - unsigned int kind; + char *dest; + unsigned int kind, len; struct commit *commit; }; @@ -193,22 +194,47 @@ struct ref_list { int kinds; }; +static char *resolve_symref(const char *src, const char *prefix) +{ + unsigned char sha1[20]; + int flag; + const char *dst, *cp; + + dst = resolve_ref(src, sha1, 0, &flag); + if (!(dst && (flag & REF_ISSYMREF))) + return NULL; + if (prefix && (cp = skip_prefix(dst, prefix))) + dst = cp; + return xstrdup(dst); +} + static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) { struct ref_list *ref_list = (struct ref_list*)(cb_data); struct ref_item *newitem; struct commit *commit; - int kind; - int len; + int kind, i; + const char *prefix, *orig_refname = refname; + + static struct { + int kind; + const char *prefix; + int pfxlen; + } ref_kind[] = { + { REF_LOCAL_BRANCH, "refs/heads/", 11 }, + { REF_REMOTE_BRANCH, "refs/remotes/", 13 }, + }; /* Detect kind */ - if (!prefixcmp(refname, "refs/heads/")) { - kind = REF_LOCAL_BRANCH; - refname += 11; - } else if (!prefixcmp(refname, "refs/remotes/")) { - kind = REF_REMOTE_BRANCH; - refname += 13; - } else + for (i = 0; i < ARRAY_SIZE(ref_kind); i++) { + prefix = ref_kind[i].prefix; + if (strncmp(refname, prefix, ref_kind[i].pfxlen)) + continue; + kind = ref_kind[i].kind; + refname += ref_kind[i].pfxlen; + break; + } + if (ARRAY_SIZE(ref_kind) <= i) return 0; commit = lookup_commit_reference_gently(sha1, 1); @@ -239,9 +265,14 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, newitem->name = xstrdup(refname); newitem->kind = kind; newitem->commit = commit; - len = strlen(newitem->name); - if (len > ref_list->maxwidth) - ref_list->maxwidth = len; + newitem->len = strlen(refname); + newitem->dest = resolve_symref(orig_refname, prefix); + /* adjust for "remotes/" */ + if (newitem->kind == REF_REMOTE_BRANCH && + ref_list->kinds != REF_REMOTE_BRANCH) + newitem->len += 8; + if (newitem->len > ref_list->maxwidth) + ref_list->maxwidth = newitem->len; return 0; } @@ -250,8 +281,10 @@ static void free_ref_list(struct ref_list *ref_list) { int i; - for (i = 0; i < ref_list->index; i++) + for (i = 0; i < ref_list->index; i++) { free(ref_list->list[i].name); + free(ref_list->list[i].dest); + } free(ref_list->list); } @@ -292,11 +325,12 @@ static int matches_merge_filter(struct commit *commit) } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, - int abbrev, int current) + int abbrev, int current, char *prefix) { char c; int color; struct commit *commit = item->commit; + struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; if (!matches_merge_filter(commit)) return; @@ -319,7 +353,18 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = COLOR_BRANCH_CURRENT; } - if (verbose) { + strbuf_addf(&name, "%s%s", prefix, item->name); + if (verbose) + strbuf_addf(&out, "%c %s%-*s%s", c, branch_get_color(color), + maxwidth, name.buf, + branch_get_color(COLOR_BRANCH_RESET)); + else + strbuf_addf(&out, "%c %s%s%s", c, branch_get_color(color), + name.buf, branch_get_color(COLOR_BRANCH_RESET)); + + if (item->dest) + strbuf_addf(&out, " -> %s", item->dest); + else if (verbose) { struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT; const char *sub = " **** invalid ref ****"; @@ -333,28 +378,25 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, if (item->kind == REF_LOCAL_BRANCH) fill_tracking_info(&stat, item->name); - printf("%c %s%-*s%s %s %s%s\n", c, branch_get_color(color), - maxwidth, item->name, - branch_get_color(COLOR_BRANCH_RESET), - find_unique_abbrev(item->commit->object.sha1, abbrev), - stat.buf, sub); + strbuf_addf(&out, " %s %s%s", + find_unique_abbrev(item->commit->object.sha1, abbrev), + stat.buf, sub); strbuf_release(&stat); strbuf_release(&subject); - } else { - printf("%c %s%s%s\n", c, branch_get_color(color), item->name, - branch_get_color(COLOR_BRANCH_RESET)); } + printf("%s\n", out.buf); + strbuf_release(&name); + strbuf_release(&out); } static int calc_maxwidth(struct ref_list *refs) { - int i, l, w = 0; + int i, w = 0; for (i = 0; i < refs->index; i++) { if (!matches_merge_filter(refs->list[i].commit)) continue; - l = strlen(refs->list[i].name); - if (l > w) - w = l; + if (refs->list[i].len > w) + w = refs->list[i].len; } return w; } @@ -394,7 +436,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str item.commit = head_commit; if (strlen(item.name) > ref_list.maxwidth) ref_list.maxwidth = strlen(item.name); - print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1); + print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, ""); free(item.name); } @@ -402,8 +444,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str int current = !detached && (ref_list.list[i].kind == REF_LOCAL_BRANCH) && !strcmp(ref_list.list[i].name, head); + char *prefix = (kinds != REF_REMOTE_BRANCH && + ref_list.list[i].kind == REF_REMOTE_BRANCH) + ? "remotes/" : ""; print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose, - abbrev, current); + abbrev, current, prefix); } free_ref_list(&ref_list); -- 1.6.2.rc0.209.g7c178 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] builtin-branch: improve output when displaying remote branches 2009-02-13 6:35 ` Junio C Hamano 2009-02-13 6:45 ` Jay Soffian @ 2009-02-13 6:47 ` martin f krafft 2009-02-13 7:36 ` Junio C Hamano 1 sibling, 1 reply; 14+ messages in thread From: martin f krafft @ 2009-02-13 6:47 UTC (permalink / raw) To: Junio C Hamano, Jay Soffian, git [-- Attachment #1: Type: text/plain, Size: 1393 bytes --] also sprach Junio C Hamano <gitster@pobox.com> [2009.02.13.0735 +0100]: > Once you start making each case arm do more things, it might make sense to > rewrite the above unrolled loop into something like this: [...] > } ref_kind[] = { > { REF_LOCAL_BRANCH, "refs/heads/", 11 }, > { REF_REMOTE_BRANCH, "refs/remotes/", 13 }, > }; [...] > Then we can later add new elements more easily, e.g. > > { REF_TOPGIT_BASE, "refs/top-base/", 14 }, As soon as TopGit is integrated into Git proper, this could make sense. However, I don't know when this will happen. In the mean time, hardcoding extensions like you suggest might not scale too well. Wouldn't it make more sense to provide an interface that allowed tools to register their own namespaces, and handle those appropriately within Git? Much of that handling could be taken straight from refs/remotes/*, as they are conceptually the same. refs/remotes/* just has additional treatment inside Git, because it's part of the basic feature set. An external feature's namespace wouldn't be, but Git also doesn't need to know anything about those, or treat them specially. -- martin | http://madduck.net/ | http://two.sentenc.es/ "doesn't he know who i think i am?" -- phil collins spamtraps: madduck.bogus@madduck.net [-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] builtin-branch: improve output when displaying remote branches 2009-02-13 6:47 ` [PATCH v2] " martin f krafft @ 2009-02-13 7:36 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2009-02-13 7:36 UTC (permalink / raw) To: martin f krafft; +Cc: Jay Soffian, git martin f krafft <madduck@madduck.net> writes: > also sprach Junio C Hamano <gitster@pobox.com> [2009.02.13.0735 +0100]: >> Once you start making each case arm do more things, it might make sense to >> rewrite the above unrolled loop into something like this: > [...] >> } ref_kind[] = { >> { REF_LOCAL_BRANCH, "refs/heads/", 11 }, >> { REF_REMOTE_BRANCH, "refs/remotes/", 13 }, >> }; > [...] > >> Then we can later add new elements more easily, e.g. >> >> { REF_TOPGIT_BASE, "refs/top-base/", 14 }, > > As soon as TopGit is integrated into Git proper, this could make > sense. However, I don't know when this will happen. In the mean > time, hardcoding extensions like you suggest might not scale too > well. Wouldn't it make more sense to provide an interface that > allowed tools to register their own namespaces, and handle those > appropriately within Git? The comment applies to the way how ref_kind[] array is initialized, and yes it would be nicer to make it extensible once the code that uses the array is in palce. However, the codepath that uses the array once it is initialized will not have to change, if you add your interface to register namespaces. That is why I suggested the code restructuring first *with* the known two hierarchies we know we would want to handle *now*. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] builtin-branch: improve output when displaying remote branches 2009-02-13 5:34 ` [PATCH v2] " Jay Soffian 2009-02-13 6:35 ` Junio C Hamano @ 2009-02-13 7:37 ` Johannes Sixt 1 sibling, 0 replies; 14+ messages in thread From: Johannes Sixt @ 2009-02-13 7:37 UTC (permalink / raw) To: Jay Soffian; +Cc: git, gitster Jay Soffian schrieb: > $ git branch -av > foo -> master > * master 51cecb2 initial > rather-long-branch-name 51cecb2 initial > remotes/frotz/HEAD -> frotz/master > remotes/frotz/master e1d8130 added file2 > remotes/origin/HEAD -> origin/master > remotes/origin/WTF -> refs/heads/master > remotes/origin/master e1d8130 added file2 Just in case there's another iteration of this patch, I'd like to see example output like this one (or even more of them) in the commit message (perhaps WTF renamed to something else ;). Thanks, -- Hannes ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2009-02-13 9:41 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-02-10 11:01 [PATCH] builtin-branch: improve output when displaying remote branches Jay Soffian 2009-02-11 6:47 ` Jay Soffian 2009-02-12 3:49 ` Junio C Hamano 2009-02-12 4:30 ` Jay Soffian 2009-02-12 5:42 ` Junio C Hamano 2009-02-13 5:34 ` [PATCH v2] " Jay Soffian 2009-02-13 6:35 ` Junio C Hamano 2009-02-13 6:45 ` Jay Soffian 2009-02-13 7:52 ` Junio C Hamano 2009-02-13 8:06 ` Jay Soffian 2009-02-13 9:40 ` [PATCH v3] " Jay Soffian 2009-02-13 6:47 ` [PATCH v2] " martin f krafft 2009-02-13 7:36 ` Junio C Hamano 2009-02-13 7:37 ` Johannes Sixt
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.