[PATCH] builtin-branch: improve output when displaying remote branches

[PATCH] builtin-branch: improve output when displaying remote branches

[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

Re: [PATCH] builtin-branch: improve output when displaying remote branches

[Jay Soffian]

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
>
>

Re: [PATCH] builtin-branch: improve output when displaying remote branches

[Junio C Hamano]

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.

Re: [PATCH] builtin-branch: improve output when displaying remote branches

[Jay Soffian]

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.

Re: [PATCH] builtin-branch: improve output when displaying remote branches

[Junio C Hamano]

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.

[PATCH v2] builtin-branch: improve output when displaying remote branches

[Jay Soffian]

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

Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

[Junio C Hamano]

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 },
;-)

Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

[Jay Soffian]

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.

Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

[martin f krafft]

[-- 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 --]

Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

[Junio C Hamano]

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*.

Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

[Johannes Sixt]

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

Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

[Junio C Hamano]

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".

Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

[Jay Soffian]

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.

[PATCH v3] builtin-branch: improve output when displaying remote branches

[Jay Soffian]

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