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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 12, 2009, 05:42 UTC
Message-ID: <7vhc30qki2.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <76718490902112030x46bddd84r561705a6b4f9d4b4@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

Show 25 quoted lines

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

Show 12 quoted lines

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