{"thread":{"id":"17692","subject":"[PATCH] builtin-branch: improve output when displaying remote branches","startedAt":"2009-02-10T11:01:41Z","lastAt":"2009-02-13T09:40:18Z","messageCount":14,"participants":["Jay Soffian","Junio C Hamano","martin f krafft","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103968","messageId":"1234263701-95463-1-git-send-email-jaysoffian@gmail.com","threadId":"17692","inReplyTo":null,"subject":"[PATCH] builtin-branch: improve output when displaying remote branches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-10T11:01:41Z","receivedAt":"2009-02-10T11:01:41Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"When displaying local and remote branches, prefix the remote branch names with\n\"remotes/\" to make the remote branches clear from the local branches. If\ndisplaying only the remote branches, the prefix is not shown since it would be\nredundant.\n\nWhen displaying a remote branch HEAD (which is a sane symref), show what it\npoints to similar to how \"ls -l\" show symlinks. Also in this case, do not show\nverbose output for the HEAD itself as it is shown immediately below on the\npointed to branch.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nI think this addresses the feedback I got on the original patch,\nhttp://article.gmane.org/gmane.comp.version-control.git/109161\n\nSome sample output:\n\n$ git branch -a\n  master\n  next\n* wip/branch-show-remote-HEAD-2\n  wip/push-docs\n  remotes/origin/HEAD -> master\n  remotes/origin/html\n  remotes/origin/maint\n  remotes/origin/man\n  remotes/origin/master\n  remotes/origin/next\n  remotes/origin/pu\n  remotes/origin/todo\n\n$ git branch -r\n  origin/HEAD -> master\n  origin/html\n  origin/maint\n  origin/man\n  origin/master\n  origin/next\n  origin/pu\n  origin/todo\n\n$ git branch -rv\n  origin/HEAD -> master\n  origin/html           6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9\n  origin/maint          7e1100e gitweb: add $prevent_xss option to prevent XSS by repository content\n  origin/man            67cb1a7 Autogenerated manpages for v1.6.2-rc0-10-gf6b9\n  origin/master         f6b98e4 git-web--browse: Fix check for /bin/start\n  origin/next           417ce12 Merge branch 'master' into next\n  origin/pu             9d798e7 Merge branch 'db/foreign-scm' into pu\n  origin/todo           5ed7079 What's in update\n\nNotice that the verbose output for HEAD is squelched as it would be identical to\nwhat is shown below in \"origin/master\". Of course, if <remote>/HEAD does not\nresolve as a symref pointing to a branch inside <remote> (I don't know why this\nwould happen though...), then it is shown like the other branches.\n\nBTW, I noticed that \"git branch -a --merged\" and \"git branch -av --merged\"\nreturn a different set of branches. I'm not sure why though (but it isn't due to\nthis patch).\n\n builtin-branch.c |   68 ++++++++++++++++++++++++++++++++++++++++-------------\n 1 files changed, 51 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 56a1971..03ad757 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \n struct ref_item {\n \tchar *name;\n-\tunsigned int kind;\n+\tchar *dest;\n+\tunsigned int kind, len;\n \tstruct commit *commit;\n };\n \n@@ -193,13 +194,23 @@ struct ref_list {\n \tint kinds;\n };\n \n+static char *resolve_remote_head_symref(const char *head_name) {\n+\tunsigned char sha1[20];\n+\tint flag;\n+\tconst char *refname;\n+\trefname = resolve_ref(head_name, sha1, 0, &flag);\n+\tif (refname && (flag & REF_ISSYMREF) &&\n+\t    !prefixcmp(refname, \"refs/remotes/\"))\n+\t\treturn xstrdup(refname + strlen(head_name) - 4);\n+\treturn NULL;\n+}\n+\n static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n {\n \tstruct ref_list *ref_list = (struct ref_list*)(cb_data);\n \tstruct ref_item *newitem;\n \tstruct commit *commit;\n \tint kind;\n-\tint len;\n \n \t/* Detect kind */\n \tif (!prefixcmp(refname, \"refs/heads/\")) {\n@@ -239,9 +250,20 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,\n \tnewitem->name = xstrdup(refname);\n \tnewitem->kind = kind;\n \tnewitem->commit = commit;\n-\tlen = strlen(newitem->name);\n-\tif (len > ref_list->maxwidth)\n-\t\tref_list->maxwidth = len;\n+\tnewitem->len = strlen(newitem->name);\n+\tnewitem->dest = (newitem->kind == REF_REMOTE_BRANCH &&\n+\t\t\t newitem->len > 5 &&\n+\t\t\t !strcmp(newitem->name + newitem->len - 5, \"/HEAD\"))\n+\t\t\t? resolve_remote_head_symref(refname - 13) : NULL;\n+\t/* adjust for \" -> \" */\n+\tif (newitem->dest)\n+\t\tnewitem->len += strlen(newitem->dest) + 4;\n+\t/* adjust for \"remotes/\" */\n+\tif (newitem->kind == REF_REMOTE_BRANCH &&\n+\t    ref_list->kinds != REF_REMOTE_BRANCH)\n+\t\tnewitem->len += 8;\n+\tif (newitem->len > ref_list->maxwidth)\n+\t\tref_list->maxwidth = newitem->len;\n \n \treturn 0;\n }\n@@ -250,8 +272,11 @@ static void free_ref_list(struct ref_list *ref_list)\n {\n \tint i;\n \n-\tfor (i = 0; i < ref_list->index; i++)\n+\tfor (i = 0; i < ref_list->index; i++) {\n \t\tfree(ref_list->list[i].name);\n+\t\tif (ref_list->list[i].dest)\n+\t\t\tfree(ref_list->list[i].dest);\n+\t}\n \tfree(ref_list->list);\n }\n \n@@ -292,7 +317,7 @@ static int matches_merge_filter(struct commit *commit)\n }\n \n static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n-\t\t\t   int abbrev, int current)\n+\t\t\t   int abbrev, int current, char *prefix)\n {\n \tchar c;\n \tint color;\n@@ -319,8 +344,13 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \t\tcolor = COLOR_BRANCH_CURRENT;\n \t}\n \n-\tif (verbose) {\n+\tif (item->dest) {\n+\t\tprintf(\"%c %s%s%s%s -> %s\\n\", c, branch_get_color(color),\n+\t\t       prefix, item->name,\n+\t\t       branch_get_color(COLOR_BRANCH_RESET), item->dest);\n+\t} else if (verbose) {\n \t\tstruct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;\n+\t\tstruct strbuf name = STRBUF_INIT;\n \t\tconst char *sub = \" **** invalid ref ****\";\n \n \t\tcommit = item->commit;\n@@ -333,28 +363,29 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \t\tif (item->kind == REF_LOCAL_BRANCH)\n \t\t\tfill_tracking_info(&stat, item->name);\n \n+\t\tstrbuf_addf(&name, \"%s%s\", prefix, item->name);\n \t\tprintf(\"%c %s%-*s%s %s %s%s\\n\", c, branch_get_color(color),\n-\t\t       maxwidth, item->name,\n+\t\t       maxwidth, name.buf,\n \t\t       branch_get_color(COLOR_BRANCH_RESET),\n \t\t       find_unique_abbrev(item->commit->object.sha1, abbrev),\n \t\t       stat.buf, sub);\n \t\tstrbuf_release(&stat);\n \t\tstrbuf_release(&subject);\n+\t\tstrbuf_release(&name);\n \t} else {\n-\t\tprintf(\"%c %s%s%s\\n\", c, branch_get_color(color), item->name,\n-\t\t       branch_get_color(COLOR_BRANCH_RESET));\n+\t\tprintf(\"%c %s%s%s%s\\n\", c, branch_get_color(color), prefix,\n+\t\t       item->name, branch_get_color(COLOR_BRANCH_RESET));\n \t}\n }\n \n static int calc_maxwidth(struct ref_list *refs)\n {\n-\tint i, l, w = 0;\n+\tint i, w = 0;\n \tfor (i = 0; i < refs->index; i++) {\n \t\tif (!matches_merge_filter(refs->list[i].commit))\n \t\t\tcontinue;\n-\t\tl = strlen(refs->list[i].name);\n-\t\tif (l > w)\n-\t\t\tw = l;\n+\t\tif (refs->list[i].len > w)\n+\t\t\tw = refs->list[i].len;\n \t}\n \treturn w;\n }\n@@ -394,7 +425,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str\n \t\titem.commit = head_commit;\n \t\tif (strlen(item.name) > ref_list.maxwidth)\n \t\t\tref_list.maxwidth = strlen(item.name);\n-\t\tprint_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1);\n+\t\tprint_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, \"\");\n \t\tfree(item.name);\n \t}\n \n@@ -402,8 +433,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str\n \t\tint current = !detached &&\n \t\t\t(ref_list.list[i].kind == REF_LOCAL_BRANCH) &&\n \t\t\t!strcmp(ref_list.list[i].name, head);\n+\t\tchar *prefix = (kinds != REF_REMOTE_BRANCH &&\n+\t\t\t\tref_list.list[i].kind == REF_REMOTE_BRANCH)\n+\t\t\t\t? \"remotes/\" : \"\";\n \t\tprint_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose,\n-\t\t\t       abbrev, current);\n+\t\t\t       abbrev, current, prefix);\n \t}\n \n \tfree_ref_list(&ref_list);\n-- \n1.6.2.rc0.12.gbd893\n"},{"id":"104150","messageId":"76718490902102247i49ee91ebo4aaf24a4e5f0f5e8@mail.gmail.com","threadId":"17692","inReplyTo":"1234263701-95463-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] builtin-branch: improve output when displaying remote branches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-11T06:47:25Z","receivedAt":"2009-02-11T06:47:25Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Bump, since this complements gmane 109374.\n\nOn Tue, Feb 10, 2009 at 6:01 AM, Jay Soffian <jaysoffian@gmail.com> wrote:\n> When displaying local and remote branches, prefix the remote branch names with\n> \"remotes/\" to make the remote branches clear from the local branches. If\n> displaying only the remote branches, the prefix is not shown since it would be\n> redundant.\n>\n> When displaying a remote branch HEAD (which is a sane symref), show what it\n> points to similar to how \"ls -l\" show symlinks. Also in this case, do not show\n> verbose output for the HEAD itself as it is shown immediately below on the\n> pointed to branch.\n>\n> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>\n> ---\n> I think this addresses the feedback I got on the original patch,\n> http://article.gmane.org/gmane.comp.version-control.git/109161\n>\n> Some sample output:\n>\n> $ git branch -a\n>  master\n>  next\n> * wip/branch-show-remote-HEAD-2\n>  wip/push-docs\n>  remotes/origin/HEAD -> master\n>  remotes/origin/html\n>  remotes/origin/maint\n>  remotes/origin/man\n>  remotes/origin/master\n>  remotes/origin/next\n>  remotes/origin/pu\n>  remotes/origin/todo\n>\n> $ git branch -r\n>  origin/HEAD -> master\n>  origin/html\n>  origin/maint\n>  origin/man\n>  origin/master\n>  origin/next\n>  origin/pu\n>  origin/todo\n>\n> $ git branch -rv\n>  origin/HEAD -> master\n>  origin/html           6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9\n>  origin/maint          7e1100e gitweb: add $prevent_xss option to prevent XSS by repository content\n>  origin/man            67cb1a7 Autogenerated manpages for v1.6.2-rc0-10-gf6b9\n>  origin/master         f6b98e4 git-web--browse: Fix check for /bin/start\n>  origin/next           417ce12 Merge branch 'master' into next\n>  origin/pu             9d798e7 Merge branch 'db/foreign-scm' into pu\n>  origin/todo           5ed7079 What's in update\n>\n> Notice that the verbose output for HEAD is squelched as it would be identical to\n> what is shown below in \"origin/master\". Of course, if <remote>/HEAD does not\n> resolve as a symref pointing to a branch inside <remote> (I don't know why this\n> would happen though...), then it is shown like the other branches.\n>\n> BTW, I noticed that \"git branch -a --merged\" and \"git branch -av --merged\"\n> return a different set of branches. I'm not sure why though (but it isn't due to\n> this patch).\n>\n>  builtin-branch.c |   68 ++++++++++++++++++++++++++++++++++++++++-------------\n>  1 files changed, 51 insertions(+), 17 deletions(-)\n>\n> diff --git a/builtin-branch.c b/builtin-branch.c\n> index 56a1971..03ad757 100644\n> --- a/builtin-branch.c\n> +++ b/builtin-branch.c\n> @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n>\n>  struct ref_item {\n>        char *name;\n> -       unsigned int kind;\n> +       char *dest;\n> +       unsigned int kind, len;\n>        struct commit *commit;\n>  };\n>\n> @@ -193,13 +194,23 @@ struct ref_list {\n>        int kinds;\n>  };\n>\n> +static char *resolve_remote_head_symref(const char *head_name) {\n> +       unsigned char sha1[20];\n> +       int flag;\n> +       const char *refname;\n> +       refname = resolve_ref(head_name, sha1, 0, &flag);\n> +       if (refname && (flag & REF_ISSYMREF) &&\n> +           !prefixcmp(refname, \"refs/remotes/\"))\n> +               return xstrdup(refname + strlen(head_name) - 4);\n> +       return NULL;\n> +}\n> +\n>  static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n>  {\n>        struct ref_list *ref_list = (struct ref_list*)(cb_data);\n>        struct ref_item *newitem;\n>        struct commit *commit;\n>        int kind;\n> -       int len;\n>\n>        /* Detect kind */\n>        if (!prefixcmp(refname, \"refs/heads/\")) {\n> @@ -239,9 +250,20 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,\n>        newitem->name = xstrdup(refname);\n>        newitem->kind = kind;\n>        newitem->commit = commit;\n> -       len = strlen(newitem->name);\n> -       if (len > ref_list->maxwidth)\n> -               ref_list->maxwidth = len;\n> +       newitem->len = strlen(newitem->name);\n> +       newitem->dest = (newitem->kind == REF_REMOTE_BRANCH &&\n> +                        newitem->len > 5 &&\n> +                        !strcmp(newitem->name + newitem->len - 5, \"/HEAD\"))\n> +                       ? resolve_remote_head_symref(refname - 13) : NULL;\n> +       /* adjust for \" -> \" */\n> +       if (newitem->dest)\n> +               newitem->len += strlen(newitem->dest) + 4;\n> +       /* adjust for \"remotes/\" */\n> +       if (newitem->kind == REF_REMOTE_BRANCH &&\n> +           ref_list->kinds != REF_REMOTE_BRANCH)\n> +               newitem->len += 8;\n> +       if (newitem->len > ref_list->maxwidth)\n> +               ref_list->maxwidth = newitem->len;\n>\n>        return 0;\n>  }\n> @@ -250,8 +272,11 @@ static void free_ref_list(struct ref_list *ref_list)\n>  {\n>        int i;\n>\n> -       for (i = 0; i < ref_list->index; i++)\n> +       for (i = 0; i < ref_list->index; i++) {\n>                free(ref_list->list[i].name);\n> +               if (ref_list->list[i].dest)\n> +                       free(ref_list->list[i].dest);\n> +       }\n>        free(ref_list->list);\n>  }\n>\n> @@ -292,7 +317,7 @@ static int matches_merge_filter(struct commit *commit)\n>  }\n>\n>  static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n> -                          int abbrev, int current)\n> +                          int abbrev, int current, char *prefix)\n>  {\n>        char c;\n>        int color;\n> @@ -319,8 +344,13 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n>                color = COLOR_BRANCH_CURRENT;\n>        }\n>\n> -       if (verbose) {\n> +       if (item->dest) {\n> +               printf(\"%c %s%s%s%s -> %s\\n\", c, branch_get_color(color),\n> +                      prefix, item->name,\n> +                      branch_get_color(COLOR_BRANCH_RESET), item->dest);\n> +       } else if (verbose) {\n>                struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;\n> +               struct strbuf name = STRBUF_INIT;\n>                const char *sub = \" **** invalid ref ****\";\n>\n>                commit = item->commit;\n> @@ -333,28 +363,29 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n>                if (item->kind == REF_LOCAL_BRANCH)\n>                        fill_tracking_info(&stat, item->name);\n>\n> +               strbuf_addf(&name, \"%s%s\", prefix, item->name);\n>                printf(\"%c %s%-*s%s %s %s%s\\n\", c, branch_get_color(color),\n> -                      maxwidth, item->name,\n> +                      maxwidth, name.buf,\n>                       branch_get_color(COLOR_BRANCH_RESET),\n>                       find_unique_abbrev(item->commit->object.sha1, abbrev),\n>                       stat.buf, sub);\n>                strbuf_release(&stat);\n>                strbuf_release(&subject);\n> +               strbuf_release(&name);\n>        } else {\n> -               printf(\"%c %s%s%s\\n\", c, branch_get_color(color), item->name,\n> -                      branch_get_color(COLOR_BRANCH_RESET));\n> +               printf(\"%c %s%s%s%s\\n\", c, branch_get_color(color), prefix,\n> +                      item->name, branch_get_color(COLOR_BRANCH_RESET));\n>        }\n>  }\n>\n>  static int calc_maxwidth(struct ref_list *refs)\n>  {\n> -       int i, l, w = 0;\n> +       int i, w = 0;\n>        for (i = 0; i < refs->index; i++) {\n>                if (!matches_merge_filter(refs->list[i].commit))\n>                        continue;\n> -               l = strlen(refs->list[i].name);\n> -               if (l > w)\n> -                       w = l;\n> +               if (refs->list[i].len > w)\n> +                       w = refs->list[i].len;\n>        }\n>        return w;\n>  }\n> @@ -394,7 +425,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str\n>                item.commit = head_commit;\n>                if (strlen(item.name) > ref_list.maxwidth)\n>                        ref_list.maxwidth = strlen(item.name);\n> -               print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1);\n> +               print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, \"\");\n>                free(item.name);\n>        }\n>\n> @@ -402,8 +433,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str\n>                int current = !detached &&\n>                        (ref_list.list[i].kind == REF_LOCAL_BRANCH) &&\n>                        !strcmp(ref_list.list[i].name, head);\n> +               char *prefix = (kinds != REF_REMOTE_BRANCH &&\n> +                               ref_list.list[i].kind == REF_REMOTE_BRANCH)\n> +                               ? \"remotes/\" : \"\";\n>                print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose,\n> -                              abbrev, current);\n> +                              abbrev, current, prefix);\n>        }\n>\n>        free_ref_list(&ref_list);\n> --\n> 1.6.2.rc0.12.gbd893\n>\n>\n"},{"id":"104338","messageId":"7vskmkqpp4.fsf@gitster.siamese.dyndns.org","threadId":"17692","inReplyTo":"1234263701-95463-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] builtin-branch: improve output when displaying remote branches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-12T03:49:59Z","receivedAt":"2009-02-12T03:49:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> $ git branch -rv\n>   origin/HEAD -> master\n>   origin/html           6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9\n\nDoesn't the misalignment between the above two bother you?\n\n> diff --git a/builtin-branch.c b/builtin-branch.c\n> index 56a1971..03ad757 100644\n> --- a/builtin-branch.c\n> +++ b/builtin-branch.c\n> @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n> +static char *resolve_remote_head_symref(const char *head_name) {\n> +\tunsigned char sha1[20];\n> +\tint flag;\n> +\tconst char *refname;\n> +\trefname = resolve_ref(head_name, sha1, 0, &flag);\n> +\tif (refname && (flag & REF_ISSYMREF) &&\n> +\t    !prefixcmp(refname, \"refs/remotes/\"))\n> +\t\treturn xstrdup(refname + strlen(head_name) - 4);\n\nHere, head_name is like \"refs/remotes/frotz/HEAD\", and you are assuming\nthat resolved refname begins with \"refs/remotes/frotz/\" without checking\nthe \"frotz\" part. It may point at \"refs/remotes/x/y\" in a misconfigured\nrepository and your xstrdup() just ran past the end of the string.\n\nIf the ref you feed to this function turns out not to be a symbolic ref,\nthe caller does do the right thing.  It makes wonder if your caller should\nalways call this, so that you would still work sensibly even if the tracking\nhierarchy has a funny symref refs/remotes/origin/TAIL that is not HEAD.\n\nThe caller is currently this dense code.\n\n> +\tnewitem->len = strlen(newitem->name);\n> +\tnewitem->dest = (newitem->kind == REF_REMOTE_BRANCH &&\n> +\t\t\t newitem->len > 5 &&\n> +\t\t\t !strcmp(newitem->name + newitem->len - 5, \"/HEAD\"))\n> +\t\t\t? resolve_remote_head_symref(refname - 13) : NULL;\n> +\t/* adjust for \" -> \" */\n> +\tif (newitem->dest)\n> +\t\tnewitem->len += strlen(newitem->dest) + 4;\n\nIt can become something like:\n\n\tif (newitem->kind == REF_REMOTE_BRANCH)\n\t\tnewitem->dest = resolve_remote_symref(refname - 13);\n\telse\n\t\tnewitem->dest = NULL;\n        if (newitem->dest)\n        \t...\n\t\nno?\n\n> @@ -250,8 +272,11 @@ static void free_ref_list(struct ref_list *ref_list)\n>  {\n>  \tint i;\n>  \n> -\tfor (i = 0; i < ref_list->index; i++)\n> +\tfor (i = 0; i < ref_list->index; i++) {\n>  \t\tfree(ref_list->list[i].name);\n> +\t\tif (ref_list->list[i].dest)\n> +\t\t\tfree(ref_list->list[i].dest);\n> +\t}\n\nfree(NULL) is Ok; omit the extra check.\n"},{"id":"104340","messageId":"76718490902112030x46bddd84r561705a6b4f9d4b4@mail.gmail.com","threadId":"17692","inReplyTo":"7vskmkqpp4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-branch: improve output when displaying remote branches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-12T04:30:52Z","receivedAt":"2009-02-12T04:30:52Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Feb 11, 2009 at 10:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jay Soffian <jaysoffian@gmail.com> writes:\n>\n>> $ git branch -rv\n>>   origin/HEAD -> master\n>>   origin/html           6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9\n>\n> Doesn't the misalignment between the above two bother you?\n\nThis comment makes me sad. In fact, a previous iteration looked like this:\n\n$ git branch -rv\n origin/HEAD -> master\n origin/html   6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9\n origin/maint  7e1100e gitweb: add $prevent_xss option to prevent XSS\nby repository content\n origin/man    67cb1a7 Autogenerated manpages for v1.6.2-rc0-10-gf6b9\n origin/master f6b98e4 git-web--browse: Fix check for /bin/start\n origin/next   417ce12 Merge branch 'master' into next\n origin/pu     9d798e7 Merge branch 'db/foreign-scm' into pu\n origin/todo   5ed7079 What's in update\n\nIOW, align based on the width of the branch name, completely ignoring\nthe width of \" -> ...\". But I found that ugly. It was actually more\nwork to get it the way it is.\n\n>> diff --git a/builtin-branch.c b/builtin-branch.c\n>> index 56a1971..03ad757 100644\n>> --- a/builtin-branch.c\n>> +++ b/builtin-branch.c\n>> @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n>> +static char *resolve_remote_head_symref(const char *head_name) {\n>> +     unsigned char sha1[20];\n>> +     int flag;\n>> +     const char *refname;\n>> +     refname = resolve_ref(head_name, sha1, 0, &flag);\n>> +     if (refname && (flag & REF_ISSYMREF) &&\n>> +         !prefixcmp(refname, \"refs/remotes/\"))\n>> +             return xstrdup(refname + strlen(head_name) - 4);\n>\n> Here, head_name is like \"refs/remotes/frotz/HEAD\", and you are assuming\n> that resolved refname begins with \"refs/remotes/frotz/\" without checking\n> the \"frotz\" part. It may point at \"refs/remotes/x/y\" in a misconfigured\n> repository and your xstrdup() just ran past the end of the string.\n\nIndeed. Now I'm doubly-sad, that my code sucks so bad. :-(\n\n> If the ref you feed to this function turns out not to be a symbolic ref,\n> the caller does do the right thing.  It makes wonder if your caller should\n> always call this, so that you would still work sensibly even if the tracking\n> hierarchy has a funny symref refs/remotes/origin/TAIL that is not HEAD.\n>\n> The caller is currently this dense code.\n>\n>> +     newitem->len = strlen(newitem->name);\n>> +     newitem->dest = (newitem->kind == REF_REMOTE_BRANCH &&\n>> +                      newitem->len > 5 &&\n>> +                      !strcmp(newitem->name + newitem->len - 5, \"/HEAD\"))\n>> +                     ? resolve_remote_head_symref(refname - 13) : NULL;\n>> +     /* adjust for \" -> \" */\n>> +     if (newitem->dest)\n>> +             newitem->len += strlen(newitem->dest) + 4;\n>\n> It can become something like:\n>\n>        if (newitem->kind == REF_REMOTE_BRANCH)\n>                newitem->dest = resolve_remote_symref(refname - 13);\n>        else\n>                newitem->dest = NULL;\n>        if (newitem->dest)\n>                ...\n>\n> no?\n\nYes indeed. I'll re-roll to to clean this up, but I'm keeping the\nvisual output the same unless you really don't like it.\n\n> free(NULL) is Ok; omit the extra check.\n\nGot it. I think I did something similar in the builtin-remote patch I\nsent you earlier, so I'll make sure to fix that there too when I\nre-roll that one.\n\nThanks for the review.\n\nj.\n"},{"id":"104345","messageId":"7vhc30qki2.fsf@gitster.siamese.dyndns.org","threadId":"17692","inReplyTo":"76718490902112030x46bddd84r561705a6b4f9d4b4@mail.gmail.com","subject":"Re: [PATCH] builtin-branch: improve output when displaying remote branches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-12T05:42:13Z","receivedAt":"2009-02-12T05:42:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> On Wed, Feb 11, 2009 at 10:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Jay Soffian <jaysoffian@gmail.com> writes:\n>>\n>>> $ git branch -rv\n>>>   origin/HEAD -> master\n>>>   origin/html           6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9\n>>\n>> Doesn't the misalignment between the above two bother you?\n>\n> This comment makes me sad. In fact, a previous iteration looked like this:\n>\n> $ git branch -rv\n>  origin/HEAD -> master\n>  origin/html   6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9\n>  origin/maint  7e1100e gitweb: add $prevent_xss option to prevent XSS\n> by repository content\n>  origin/man    67cb1a7 Autogenerated manpages for v1.6.2-rc0-10-gf6b9\n>  origin/master f6b98e4 git-web--browse: Fix check for /bin/start\n>  origin/next   417ce12 Merge branch 'master' into next\n>  origin/pu     9d798e7 Merge branch 'db/foreign-scm' into pu\n>  origin/todo   5ed7079 What's in update\n>\n> IOW, align based on the width of the branch name, completely ignoring\n> the width of \" -> ...\". But I found that ugly. It was actually more\n> work to get it the way it is.\n\nWouldn't something like this easier to read?\n\n>  origin/HEAD   ------> master\n>  origin/html   6116912 Autogenerated HTML docs for v1.6.2-rc0-10-gf6b9\n>  origin/maint  7e1100e gitweb: add $prevent_xss option to prevent XSS\n\nI am not sure about the long arrow.  It may be easier to use \"->\" aligned\nth the right end, but that now falls into bikeshedding, so I'll leve that\nto the list.\n\n>> It can become something like:\n>>\n>>        if (newitem->kind == REF_REMOTE_BRANCH)\n>>                newitem->dest = resolve_remote_symref(refname - 13);\n>>        else\n>>                newitem->dest = NULL;\n>>        if (newitem->dest)\n>>                ...\n>>\n>> no?\n>\n> Yes indeed.\n\nAnother thing you may want to consider is to introduce another variable\n\"name\" that is supposed to be the human readable part (i.e. refname-13\netc.) and leave the refname the full name starting from \"refs/\".  The code\nto add and then subtract to go back and forth made my head spin when I\nread it.\n"},{"id":"104474","messageId":"1234503271-78569-1-git-send-email-jaysoffian@gmail.com","threadId":"17692","inReplyTo":"7vhc30qki2.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2] builtin-branch: improve output when displaying remote branches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T05:34:31Z","receivedAt":"2009-02-13T05:34:31Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"When encountering a symref (typically refs/remotes/<remote>/HEAD),\ndisplay the ref target.\n\nWhen displaying local and remote branches, prefix the remote branch\nnames with \"remotes/\" to make the remote branches clear from the local\nbranches. If displaying only the remote branches, the prefix is not\nshown since it would be redundant.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nChanges from v1:\n\n * I now resolve any symref that I encounter, not just HEAD. As long as it's a\n   valid symref, we show what it points to. If the target points into the same\n   namespace as the source (refs/heads/ or refs/remotes/), then we strip the\n   prefix, otherwise we display the complete target path. So if someone has\n   done something silly like having a refs/remotes/frotz/blarg that points to\n   refs/heads/blam, then we show it as:\n\n   frotz/blarg -> refs/heads/blam\n\n   But in the typical case of refs/remotes/origin/HEAD, it just looks like:\n\n   origin/HEAD -> origin/master\n\n * I decided to leave the \"origin\" (or whatever...) prefix on the target since\n   I think it is clearer.\n\n * I rewrote print_ref_item() to use a strbuf for generating the output -- the\n   code is clearer that way w/all the output permutations now.\n   \nHere's some sample output:\n\n$ git branch \n  foo -> master\n* master\n  rather-long-branch-name\n\n$ git branch -v\n  foo                     -> master\n* master                  51cecb2 initial\n  rather-long-branch-name 51cecb2 initial\n\n$ git branch -v --no-abbrev\n  foo                     -> master\n* master                  51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial\n  rather-long-branch-name 51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial\n\n$ git branch -r\n  frotz/HEAD -> frotz/master\n  frotz/master\n  origin/HEAD -> origin/master\n  origin/WTF -> refs/heads/master\n  origin/master\n\n$ git branch -a\n  foo -> master\n* master\n  rather-long-branch-name\n  remotes/frotz/HEAD -> frotz/master\n  remotes/frotz/master\n  remotes/origin/HEAD -> origin/master\n  remotes/origin/WTF -> refs/heads/master\n  remotes/origin/master\n\n$ git branch -rv\n  frotz/HEAD    -> frotz/master\n  frotz/master  e1d8130 added file2\n  origin/HEAD   -> origin/master\n  origin/WTF    -> refs/heads/master\n  origin/master e1d8130 added file2\n\n$ git branch -av\n  foo                     -> master\n* master                  51cecb2 initial\n  rather-long-branch-name 51cecb2 initial\n  remotes/frotz/HEAD      -> frotz/master\n  remotes/frotz/master    e1d8130 added file2\n  remotes/origin/HEAD     -> origin/master\n  remotes/origin/WTF      -> refs/heads/master\n  remotes/origin/master   e1d8130 added file2\n\nIn the case of verbose output, I did play with making the arrow the same width\nas the commit id (which means accounting for abbrev=<n> and --no-abbrev\nbtw...), but it really didn't look any better IMO.\n\n builtin-branch.c |   80 +++++++++++++++++++++++++++++++++++++++---------------\n 1 files changed, 58 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 56a1971..4154ad2 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \n struct ref_item {\n \tchar *name;\n-\tunsigned int kind;\n+\tchar *dest;\n+\tunsigned int kind, len;\n \tstruct commit *commit;\n };\n \n@@ -193,21 +194,37 @@ struct ref_list {\n \tint kinds;\n };\n \n+static char *resolve_symref(const char *src, const char *prefix)\n+{\n+\tunsigned char sha1[20];\n+\tint flag;\n+\tconst char *dst, *cp;\n+\tdst = resolve_ref(src, sha1, 0, &flag);\n+\tif (!(dst && (flag & REF_ISSYMREF)))\n+\t\treturn NULL;\n+\tif (prefix && !prefixcmp(dst, prefix))\n+\t\treturn xstrdup(skip_prefix(dst, prefix));\n+\telse\n+\t\treturn xstrdup(dst);\n+}\n+\n static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n {\n \tstruct ref_list *ref_list = (struct ref_list*)(cb_data);\n \tstruct ref_item *newitem;\n \tstruct commit *commit;\n \tint kind;\n-\tint len;\n+\tconst char *prefix, *orig_refname = refname;\n \n \t/* Detect kind */\n \tif (!prefixcmp(refname, \"refs/heads/\")) {\n \t\tkind = REF_LOCAL_BRANCH;\n \t\trefname += 11;\n+\t\tprefix = \"refs/heads/\";\n \t} else if (!prefixcmp(refname, \"refs/remotes/\")) {\n \t\tkind = REF_REMOTE_BRANCH;\n \t\trefname += 13;\n+\t\tprefix = \"refs/remotes/\";\n \t} else\n \t\treturn 0;\n \n@@ -239,9 +256,14 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,\n \tnewitem->name = xstrdup(refname);\n \tnewitem->kind = kind;\n \tnewitem->commit = commit;\n-\tlen = strlen(newitem->name);\n-\tif (len > ref_list->maxwidth)\n-\t\tref_list->maxwidth = len;\n+\tnewitem->len = strlen(refname);\n+\tnewitem->dest = resolve_symref(orig_refname, prefix);\n+\t/* adjust for \"remotes/\" */\n+\tif (newitem->kind == REF_REMOTE_BRANCH &&\n+\t    ref_list->kinds != REF_REMOTE_BRANCH)\n+\t\tnewitem->len += 8;\n+\tif (newitem->len > ref_list->maxwidth)\n+\t\tref_list->maxwidth = newitem->len;\n \n \treturn 0;\n }\n@@ -250,8 +272,10 @@ static void free_ref_list(struct ref_list *ref_list)\n {\n \tint i;\n \n-\tfor (i = 0; i < ref_list->index; i++)\n+\tfor (i = 0; i < ref_list->index; i++) {\n \t\tfree(ref_list->list[i].name);\n+\t\tfree(ref_list->list[i].dest);\n+\t}\n \tfree(ref_list->list);\n }\n \n@@ -292,11 +316,12 @@ static int matches_merge_filter(struct commit *commit)\n }\n \n static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n-\t\t\t   int abbrev, int current)\n+\t\t\t   int abbrev, int current, char *prefix)\n {\n \tchar c;\n \tint color;\n \tstruct commit *commit = item->commit;\n+\tstruct strbuf out = STRBUF_INIT, name = STRBUF_INIT;\n \n \tif (!matches_merge_filter(commit))\n \t\treturn;\n@@ -319,7 +344,18 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \t\tcolor = COLOR_BRANCH_CURRENT;\n \t}\n \n-\tif (verbose) {\n+\tstrbuf_addf(&name, \"%s%s\", prefix, item->name);\n+\tif (verbose)\n+\t\tstrbuf_addf(&out, \"%c %s%-*s%s\", c, branch_get_color(color),\n+\t\t\t    maxwidth, name.buf,\n+\t\t\t    branch_get_color(COLOR_BRANCH_RESET));\n+\telse\n+\t\tstrbuf_addf(&out, \"%c %s%s%s\", c, branch_get_color(color),\n+\t\t\t    name.buf, branch_get_color(COLOR_BRANCH_RESET));\n+\n+\tif (item->dest)\n+\t\tstrbuf_addf(&out, \" -> %s\", item->dest);\n+\telse if (verbose) {\n \t\tstruct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;\n \t\tconst char *sub = \" **** invalid ref ****\";\n \n@@ -333,28 +369,25 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \t\tif (item->kind == REF_LOCAL_BRANCH)\n \t\t\tfill_tracking_info(&stat, item->name);\n \n-\t\tprintf(\"%c %s%-*s%s %s %s%s\\n\", c, branch_get_color(color),\n-\t\t       maxwidth, item->name,\n-\t\t       branch_get_color(COLOR_BRANCH_RESET),\n-\t\t       find_unique_abbrev(item->commit->object.sha1, abbrev),\n-\t\t       stat.buf, sub);\n+\t\tstrbuf_addf(&out, \" %s %s%s\",\n+\t\t\tfind_unique_abbrev(item->commit->object.sha1, abbrev),\n+\t\t\tstat.buf, sub);\n \t\tstrbuf_release(&stat);\n \t\tstrbuf_release(&subject);\n-\t} else {\n-\t\tprintf(\"%c %s%s%s\\n\", c, branch_get_color(color), item->name,\n-\t\t       branch_get_color(COLOR_BRANCH_RESET));\n \t}\n+\tprintf(\"%s\\n\", out.buf);\n+\tstrbuf_release(&name);\n+\tstrbuf_release(&out);\n }\n \n static int calc_maxwidth(struct ref_list *refs)\n {\n-\tint i, l, w = 0;\n+\tint i, w = 0;\n \tfor (i = 0; i < refs->index; i++) {\n \t\tif (!matches_merge_filter(refs->list[i].commit))\n \t\t\tcontinue;\n-\t\tl = strlen(refs->list[i].name);\n-\t\tif (l > w)\n-\t\t\tw = l;\n+\t\tif (refs->list[i].len > w)\n+\t\t\tw = refs->list[i].len;\n \t}\n \treturn w;\n }\n@@ -394,7 +427,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str\n \t\titem.commit = head_commit;\n \t\tif (strlen(item.name) > ref_list.maxwidth)\n \t\t\tref_list.maxwidth = strlen(item.name);\n-\t\tprint_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1);\n+\t\tprint_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, \"\");\n \t\tfree(item.name);\n \t}\n \n@@ -402,8 +435,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str\n \t\tint current = !detached &&\n \t\t\t(ref_list.list[i].kind == REF_LOCAL_BRANCH) &&\n \t\t\t!strcmp(ref_list.list[i].name, head);\n+\t\tchar *prefix = (kinds != REF_REMOTE_BRANCH &&\n+\t\t\t\tref_list.list[i].kind == REF_REMOTE_BRANCH)\n+\t\t\t\t? \"remotes/\" : \"\";\n \t\tprint_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose,\n-\t\t\t       abbrev, current);\n+\t\t\t       abbrev, current, prefix);\n \t}\n \n \tfree_ref_list(&ref_list);\n-- \n1.6.2.rc0.209.g7c178\n"},{"id":"104480","messageId":"7v7i3ug7y9.fsf@gitster.siamese.dyndns.org","threadId":"17692","inReplyTo":"1234503271-78569-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v2] builtin-branch: improve output when displaying remote branches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-13T06:35:42Z","receivedAt":"2009-02-13T06:35:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> +\tif (prefix && !prefixcmp(dst, prefix))\n> +\t\treturn xstrdup(skip_prefix(dst, prefix));\n> +\telse\n> +\t\treturn xstrdup(dst);\n> +}\n\nI wonder modern compilers are clever enough to optimze the above to\nsomething more like:\n\n\tpfxlen = prefix ? strlen(prefix) : 0;\n        if (pfxlen && !strncmp(dst, prefix, pfxlen))\n        \treturn xstrdup(dst + pfxlen);\n\telse\n        \treturn xstrdup(dst);\n\ngiven that skip_prefix is an inline function but prefixcmp is not\n(anymore), perhaps not.\n\n>  static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n>  {\n>  \tstruct ref_list *ref_list = (struct ref_list*)(cb_data);\n>  \tstruct ref_item *newitem;\n>  \tstruct commit *commit;\n>  \tint kind;\n> -\tint len;\n> +\tconst char *prefix, *orig_refname = refname;\n>  \n>  \t/* Detect kind */\n>  \tif (!prefixcmp(refname, \"refs/heads/\")) {\n>  \t\tkind = REF_LOCAL_BRANCH;\n>  \t\trefname += 11;\n> +\t\tprefix = \"refs/heads/\";\n>  \t} else if (!prefixcmp(refname, \"refs/remotes/\")) {\n>  \t\tkind = REF_REMOTE_BRANCH;\n>  \t\trefname += 13;\n> +\t\tprefix = \"refs/remotes/\";\n>  \t} else\n>  \t\treturn 0;\n\nOnce you start making each case arm do more things, it might make sense to\nrewrite the above unrolled loop into something like this:\n\n\tstatic struct {\n        \tint kind;\n                const char *prefix;\n                int pfxlen;\n\t} ref_kind[] = {\n        \t{ REF_LOCAL_BRANCH, \"refs/heads/\", 11 },\n        \t{ REF_REMOTE_BRANCH, \"refs/remotes/\", 13 },\n\t};\n\n\tfor (i = 0; i < ARRAY_SIZE(ref_kind); i++) {\n                prefix = ref_kind[i].prefix;\n        \tif (strncmp(refname, prefix, ref_kind[i].pfxlen))\n\t\t\tcontinue;\n\t\tkind = ref_kind[i].kind;\n                refname += ref_kind[i].pfxlen;\n                break;\n\t}\n\tif (ARRAY_SIZE(ref_kind) <= i)\n\t\treturn 0;\n\nThen we can later add new elements more easily, e.g.\n\n                { REF_TOPGIT_BASE, \"refs/top-base/\", 14 },\n;-)\n"},{"id":"104481","messageId":"76718490902122245q6f530bb5sc85a65e065966eda@mail.gmail.com","threadId":"17692","inReplyTo":"7v7i3ug7y9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] builtin-branch: improve output when displaying remote branches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T06:45:33Z","receivedAt":"2009-02-13T06:45:33Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jay Soffian <jaysoffian@gmail.com> writes:\n>\n>> +     if (prefix && !prefixcmp(dst, prefix))\n>> +             return xstrdup(skip_prefix(dst, prefix));\n>> +     else\n>> +             return xstrdup(dst);\n>> +}\n>\n> I wonder modern compilers are clever enough to optimze the above to\n> something more like:\n>\n>        pfxlen = prefix ? strlen(prefix) : 0;\n>        if (pfxlen && !strncmp(dst, prefix, pfxlen))\n>                return xstrdup(dst + pfxlen);\n>        else\n>                return xstrdup(dst);\n>\n> given that skip_prefix is an inline function but prefixcmp is not\n> (anymore), perhaps not.\n>\n>>  static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n>>  {\n>>       struct ref_list *ref_list = (struct ref_list*)(cb_data);\n>>       struct ref_item *newitem;\n>>       struct commit *commit;\n>>       int kind;\n>> -     int len;\n>> +     const char *prefix, *orig_refname = refname;\n>>\n>>       /* Detect kind */\n>>       if (!prefixcmp(refname, \"refs/heads/\")) {\n>>               kind = REF_LOCAL_BRANCH;\n>>               refname += 11;\n>> +             prefix = \"refs/heads/\";\n>>       } else if (!prefixcmp(refname, \"refs/remotes/\")) {\n>>               kind = REF_REMOTE_BRANCH;\n>>               refname += 13;\n>> +             prefix = \"refs/remotes/\";\n>>       } else\n>>               return 0;\n>\n> Once you start making each case arm do more things, it might make sense to\n> rewrite the above unrolled loop into something like this:\n>\n>        static struct {\n>                int kind;\n>                const char *prefix;\n>                int pfxlen;\n>        } ref_kind[] = {\n>                { REF_LOCAL_BRANCH, \"refs/heads/\", 11 },\n>                { REF_REMOTE_BRANCH, \"refs/remotes/\", 13 },\n>        };\n>\n>        for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {\n>                prefix = ref_kind[i].prefix;\n>                if (strncmp(refname, prefix, ref_kind[i].pfxlen))\n>                        continue;\n>                kind = ref_kind[i].kind;\n>                refname += ref_kind[i].pfxlen;\n>                break;\n>        }\n>        if (ARRAY_SIZE(ref_kind) <= i)\n>                return 0;\n>\n> Then we can later add new elements more easily, e.g.\n>\n>                { REF_TOPGIT_BASE, \"refs/top-base/\", 14 },\n> ;-)\n\nThis strikes me as premature optimization. We're just emitting a few\nbranch names here. I'm beginning to lose my motivation to keep working\non this patch. I just wanted to improve the UI slightly. :-(\n\nj.\n"},{"id":"104482","messageId":"20090213064707.GA17681@piper.oerlikon.madduck.net","threadId":"17692","inReplyTo":"7v7i3ug7y9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] builtin-branch: improve output when displaying remote branches","fromName":"martin f krafft","fromEmail":"madduck@madduck.net","sentAt":"2009-02-13T06:47:07Z","receivedAt":"2009-02-13T06:47:07Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"also sprach Junio C Hamano <gitster@pobox.com> [2009.02.13.0735 +0100]:\n> Once you start making each case arm do more things, it might make sense to\n> rewrite the above unrolled loop into something like this:\n[...]\n> \t} ref_kind[] = {\n>         \t{ REF_LOCAL_BRANCH, \"refs/heads/\", 11 },\n>         \t{ REF_REMOTE_BRANCH, \"refs/remotes/\", 13 },\n> \t};\n[...]\n\n> Then we can later add new elements more easily, e.g.\n> \n>                 { REF_TOPGIT_BASE, \"refs/top-base/\", 14 },\n\nAs soon as TopGit is integrated into Git proper, this could make\nsense. However, I don't know when this will happen. In the mean\ntime, hardcoding extensions like you suggest might not scale too\nwell. Wouldn't it make more sense to provide an interface that\nallowed tools to register their own namespaces, and handle those\nappropriately within Git?\n\nMuch of that handling could be taken straight from refs/remotes/*,\nas they are conceptually the same. refs/remotes/* just has\nadditional treatment inside Git, because it's part of the basic\nfeature set. An external feature's namespace wouldn't be, but Git\nalso doesn't need to know anything about those, or treat them\nspecially.\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \n\"doesn't he know who i think i am?\"\n                                                     -- phil collins\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"104487","messageId":"7viqneeqk2.fsf@gitster.siamese.dyndns.org","threadId":"17692","inReplyTo":"20090213064707.GA17681@piper.oerlikon.madduck.net","subject":"Re: [PATCH v2] builtin-branch: improve output when displaying remote branches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-13T07:36:45Z","receivedAt":"2009-02-13T07:36:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"martin f krafft <madduck@madduck.net> writes:\n\n> also sprach Junio C Hamano <gitster@pobox.com> [2009.02.13.0735 +0100]:\n>> Once you start making each case arm do more things, it might make sense to\n>> rewrite the above unrolled loop into something like this:\n> [...]\n>> \t} ref_kind[] = {\n>>         \t{ REF_LOCAL_BRANCH, \"refs/heads/\", 11 },\n>>         \t{ REF_REMOTE_BRANCH, \"refs/remotes/\", 13 },\n>> \t};\n> [...]\n>\n>> Then we can later add new elements more easily, e.g.\n>> \n>>                 { REF_TOPGIT_BASE, \"refs/top-base/\", 14 },\n>\n> As soon as TopGit is integrated into Git proper, this could make\n> sense. However, I don't know when this will happen. In the mean\n> time, hardcoding extensions like you suggest might not scale too\n> well. Wouldn't it make more sense to provide an interface that\n> allowed tools to register their own namespaces, and handle those\n> appropriately within Git?\n\nThe comment applies to the way how ref_kind[] array is initialized, and\nyes it would be nicer to make it extensible once the code that uses the\narray is in palce.\n\nHowever, the codepath that uses the array once it is initialized will not\nhave to change, if you add your interface to register namespaces.\n\nThat is why I suggested the code restructuring first *with* the known two\nhierarchies we know we would want to handle *now*.\n"},{"id":"104489","messageId":"4995232F.1090206@viscovery.net","threadId":"17692","inReplyTo":"1234503271-78569-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v2] builtin-branch: improve output when displaying remote branches","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-13T07:37:19Z","receivedAt":"2009-02-13T07:37:19Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Jay Soffian schrieb:\n> $ git branch -av\n>   foo                     -> master\n> * master                  51cecb2 initial\n>   rather-long-branch-name 51cecb2 initial\n>   remotes/frotz/HEAD      -> frotz/master\n>   remotes/frotz/master    e1d8130 added file2\n>   remotes/origin/HEAD     -> origin/master\n>   remotes/origin/WTF      -> refs/heads/master\n>   remotes/origin/master   e1d8130 added file2\n\nJust in case there's another iteration of this patch, I'd like to see\nexample output like this one (or even more of them) in the commit message\n(perhaps WTF renamed to something else ;).\n\nThanks,\n-- Hannes\n"},{"id":"104491","messageId":"7veiy2eptb.fsf@gitster.siamese.dyndns.org","threadId":"17692","inReplyTo":"76718490902122245q6f530bb5sc85a65e065966eda@mail.gmail.com","subject":"Re: [PATCH v2] builtin-branch: improve output when displaying remote branches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-13T07:52:48Z","receivedAt":"2009-02-13T07:52:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> This strikes me as premature optimization.\n\nIt is *not* an optimization.  It is reduction of code duplication to\nimprove maintainability before copy-and-paste gets out of hand.\n\nThe proper adjective for it is not \"premature\" but \"preemptive\".\n"},{"id":"104493","messageId":"76718490902130006r4f125752wa207056d72630e84@mail.gmail.com","threadId":"17692","inReplyTo":"7veiy2eptb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] builtin-branch: improve output when displaying remote branches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T08:06:25Z","receivedAt":"2009-02-13T08:06:25Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 2:52 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jay Soffian <jaysoffian@gmail.com> writes:\n>\n>> This strikes me as premature optimization.\n>\n> It is *not* an optimization.  It is reduction of code duplication to\n> improve maintainability before copy-and-paste gets out of hand.\n>\n> The proper adjective for it is not \"premature\" but \"preemptive\".\n\nI'll re-roll the patch again. However, if you look at the rest of\nbuiltin-branch.c, you'll see there are more places than that which are\nstructured similarly (e.g. delete_branches). So if someone wanted to\nadd another branch type, that would be the time (IMO), to refactor the\nstatement at the top of append_ref().\n\nj.\n"},{"id":"104503","messageId":"1234518018-94636-1-git-send-email-jaysoffian@gmail.com","threadId":"17692","inReplyTo":"76718490902130006r4f125752wa207056d72630e84@mail.gmail.com","subject":"[PATCH v3] builtin-branch: improve output when displaying remote branches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T09:40:18Z","receivedAt":"2009-02-13T09:40:18Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"When encountering a symref (typically refs/remotes/<remote>/HEAD),\ndisplay the ref target.\n\nWhen displaying local and remote branches, prefix the remote branch\nnames with \"remotes/\" to make the remote branches clear from the local\nbranches. If displaying only the remote branches, the prefix is not\nshown since it would be redundant.\n\nSample output:\n\n$ git branch\n  foo -> master\n* master\n  rather-long-branch-name\n\n$ git branch -v\n  foo                     -> master\n* master                  51cecb2 initial\n  rather-long-branch-name 51cecb2 initial\n\n$ git branch -v --no-abbrev\n  foo                     -> master\n* master                  51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial\n  rather-long-branch-name 51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial\n\n$ git branch -r\n  frotz/HEAD -> frotz/master\n  frotz/master\n  origin/HEAD -> origin/master\n  origin/UNUSUAL -> refs/heads/master\n  origin/master\n\n$ git branch -a\n  foo -> master\n* master\n  rather-long-branch-name\n  remotes/frotz/HEAD -> frotz/master\n  remotes/frotz/master\n  remotes/origin/HEAD -> origin/master\n  remotes/origin/UNUSUAL -> refs/heads/master\n  remotes/origin/master\n\n$ git branch -rv\n  frotz/HEAD     -> frotz/master\n  frotz/master   e1d8130 added file2\n  origin/HEAD    -> origin/master\n  origin/UNUSUAL -> refs/heads/master\n  origin/master  e1d8130 added file2\n\n$ git branch -av\n  foo                     -> master\n* master                  51cecb2 initial\n  rather-long-branch-name 51cecb2 initial\n  remotes/frotz/HEAD      -> frotz/master\n  remotes/frotz/master    e1d8130 added file2\n  remotes/origin/HEAD     -> origin/master\n  remotes/origin/UNUSUAL  -> refs/heads/master\n  remotes/origin/master   e1d8130 added file2\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nImprovements since v2 - http://thread.gmane.org/gmane.comp.version-control.git/109200/focus=109706\n\n * Moved the sample output into the commit message as suggested by Hannes\n\n * Refactored an if statement into a loop as suggested by Junio. This adds more\n   LOC of now, but hopefully it will pay off in the future as git branch learns\n   to emit other branch types than local and remote.\n\n   I personally disagree w/doing this now as there are other places that will\n   need to be touched to handle a third (or more) branch type. I think at the\n   time the next branch type is added, that is when to look for this type of\n   refactoring. Right now it seems like needless churn to me.\n\n   With that off my chest, I will defer to the maintainer. :-)\n\n * Junio delicately pointed out that I was an idiot :-) :-) :-) to call\n   prefixcmp(), just to call skip_prefix() immediately afterward. I think I did\n   better job this time.\n\n builtin-branch.c |  105 ++++++++++++++++++++++++++++++++++++++---------------\n 1 files changed, 75 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 56a1971..7607f6a 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \n struct ref_item {\n \tchar *name;\n-\tunsigned int kind;\n+\tchar *dest;\n+\tunsigned int kind, len;\n \tstruct commit *commit;\n };\n \n@@ -193,22 +194,47 @@ struct ref_list {\n \tint kinds;\n };\n \n+static char *resolve_symref(const char *src, const char *prefix)\n+{\n+\tunsigned char sha1[20];\n+\tint flag;\n+\tconst char *dst, *cp;\n+\n+\tdst = resolve_ref(src, sha1, 0, &flag);\n+\tif (!(dst && (flag & REF_ISSYMREF)))\n+\t\treturn NULL;\n+\tif (prefix && (cp = skip_prefix(dst, prefix)))\n+\t\tdst = cp;\n+\treturn xstrdup(dst);\n+}\n+\n static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n {\n \tstruct ref_list *ref_list = (struct ref_list*)(cb_data);\n \tstruct ref_item *newitem;\n \tstruct commit *commit;\n-\tint kind;\n-\tint len;\n+\tint kind, i;\n+\tconst char *prefix, *orig_refname = refname;\n+\n+\tstatic struct {\n+\t\tint kind;\n+\t\tconst char *prefix;\n+\t\tint pfxlen;\n+\t} ref_kind[] = {\n+\t\t{ REF_LOCAL_BRANCH, \"refs/heads/\", 11 },\n+\t\t{ REF_REMOTE_BRANCH, \"refs/remotes/\", 13 },\n+\t};\n \n \t/* Detect kind */\n-\tif (!prefixcmp(refname, \"refs/heads/\")) {\n-\t\tkind = REF_LOCAL_BRANCH;\n-\t\trefname += 11;\n-\t} else if (!prefixcmp(refname, \"refs/remotes/\")) {\n-\t\tkind = REF_REMOTE_BRANCH;\n-\t\trefname += 13;\n-\t} else\n+\tfor (i = 0; i < ARRAY_SIZE(ref_kind); i++) {\n+\t\tprefix = ref_kind[i].prefix;\n+\t\tif (strncmp(refname, prefix, ref_kind[i].pfxlen))\n+\t\t\tcontinue;\n+\t\tkind = ref_kind[i].kind;\n+\t\trefname += ref_kind[i].pfxlen;\n+\t\tbreak;\n+\t}\n+\tif (ARRAY_SIZE(ref_kind) <= i)\n \t\treturn 0;\n \n \tcommit = lookup_commit_reference_gently(sha1, 1);\n@@ -239,9 +265,14 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,\n \tnewitem->name = xstrdup(refname);\n \tnewitem->kind = kind;\n \tnewitem->commit = commit;\n-\tlen = strlen(newitem->name);\n-\tif (len > ref_list->maxwidth)\n-\t\tref_list->maxwidth = len;\n+\tnewitem->len = strlen(refname);\n+\tnewitem->dest = resolve_symref(orig_refname, prefix);\n+\t/* adjust for \"remotes/\" */\n+\tif (newitem->kind == REF_REMOTE_BRANCH &&\n+\t    ref_list->kinds != REF_REMOTE_BRANCH)\n+\t\tnewitem->len += 8;\n+\tif (newitem->len > ref_list->maxwidth)\n+\t\tref_list->maxwidth = newitem->len;\n \n \treturn 0;\n }\n@@ -250,8 +281,10 @@ static void free_ref_list(struct ref_list *ref_list)\n {\n \tint i;\n \n-\tfor (i = 0; i < ref_list->index; i++)\n+\tfor (i = 0; i < ref_list->index; i++) {\n \t\tfree(ref_list->list[i].name);\n+\t\tfree(ref_list->list[i].dest);\n+\t}\n \tfree(ref_list->list);\n }\n \n@@ -292,11 +325,12 @@ static int matches_merge_filter(struct commit *commit)\n }\n \n static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n-\t\t\t   int abbrev, int current)\n+\t\t\t   int abbrev, int current, char *prefix)\n {\n \tchar c;\n \tint color;\n \tstruct commit *commit = item->commit;\n+\tstruct strbuf out = STRBUF_INIT, name = STRBUF_INIT;\n \n \tif (!matches_merge_filter(commit))\n \t\treturn;\n@@ -319,7 +353,18 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \t\tcolor = COLOR_BRANCH_CURRENT;\n \t}\n \n-\tif (verbose) {\n+\tstrbuf_addf(&name, \"%s%s\", prefix, item->name);\n+\tif (verbose)\n+\t\tstrbuf_addf(&out, \"%c %s%-*s%s\", c, branch_get_color(color),\n+\t\t\t    maxwidth, name.buf,\n+\t\t\t    branch_get_color(COLOR_BRANCH_RESET));\n+\telse\n+\t\tstrbuf_addf(&out, \"%c %s%s%s\", c, branch_get_color(color),\n+\t\t\t    name.buf, branch_get_color(COLOR_BRANCH_RESET));\n+\n+\tif (item->dest)\n+\t\tstrbuf_addf(&out, \" -> %s\", item->dest);\n+\telse if (verbose) {\n \t\tstruct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;\n \t\tconst char *sub = \" **** invalid ref ****\";\n \n@@ -333,28 +378,25 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \t\tif (item->kind == REF_LOCAL_BRANCH)\n \t\t\tfill_tracking_info(&stat, item->name);\n \n-\t\tprintf(\"%c %s%-*s%s %s %s%s\\n\", c, branch_get_color(color),\n-\t\t       maxwidth, item->name,\n-\t\t       branch_get_color(COLOR_BRANCH_RESET),\n-\t\t       find_unique_abbrev(item->commit->object.sha1, abbrev),\n-\t\t       stat.buf, sub);\n+\t\tstrbuf_addf(&out, \" %s %s%s\",\n+\t\t\tfind_unique_abbrev(item->commit->object.sha1, abbrev),\n+\t\t\tstat.buf, sub);\n \t\tstrbuf_release(&stat);\n \t\tstrbuf_release(&subject);\n-\t} else {\n-\t\tprintf(\"%c %s%s%s\\n\", c, branch_get_color(color), item->name,\n-\t\t       branch_get_color(COLOR_BRANCH_RESET));\n \t}\n+\tprintf(\"%s\\n\", out.buf);\n+\tstrbuf_release(&name);\n+\tstrbuf_release(&out);\n }\n \n static int calc_maxwidth(struct ref_list *refs)\n {\n-\tint i, l, w = 0;\n+\tint i, w = 0;\n \tfor (i = 0; i < refs->index; i++) {\n \t\tif (!matches_merge_filter(refs->list[i].commit))\n \t\t\tcontinue;\n-\t\tl = strlen(refs->list[i].name);\n-\t\tif (l > w)\n-\t\t\tw = l;\n+\t\tif (refs->list[i].len > w)\n+\t\t\tw = refs->list[i].len;\n \t}\n \treturn w;\n }\n@@ -394,7 +436,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str\n \t\titem.commit = head_commit;\n \t\tif (strlen(item.name) > ref_list.maxwidth)\n \t\t\tref_list.maxwidth = strlen(item.name);\n-\t\tprint_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1);\n+\t\tprint_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, \"\");\n \t\tfree(item.name);\n \t}\n \n@@ -402,8 +444,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str\n \t\tint current = !detached &&\n \t\t\t(ref_list.list[i].kind == REF_LOCAL_BRANCH) &&\n \t\t\t!strcmp(ref_list.list[i].name, head);\n+\t\tchar *prefix = (kinds != REF_REMOTE_BRANCH &&\n+\t\t\t\tref_list.list[i].kind == REF_REMOTE_BRANCH)\n+\t\t\t\t? \"remotes/\" : \"\";\n \t\tprint_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose,\n-\t\t\t       abbrev, current);\n+\t\t\t       abbrev, current, prefix);\n \t}\n \n \tfree_ref_list(&ref_list);\n-- \n1.6.2.rc0.209.g7c178\n"}]}