Re: [PATCH/WIP 0/9] for-each-ref format improvements

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: May 19, 2013, 11:11 UTC
Message-ID: <CALkWK0=7z91A54jSzc2yZU3g50u8H_f8su1Y+i=D+KxYtqor5g@mail.gmail.com>
In-Reply-To: <1368959235-27777-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote:

> The purpose of this series is to make "for-each-ref --format" powerful
> enough to display what "branch -v" and "branch -vv" do so that we
> could get rid of those display code and use for-each-ref code instead.

Damn, you beat me to it. I just introduced color, and was working on alignment. See $gmane/224692.

Yes, I think this is the direction we should be taking. Poorly thought-out stuff like -v and -vv should be deprecated.

Show 6 quoted lines

> This series introduces:
>
>  - %(current), which either shows "*" if the ref is pointed by HEAD
>    or a space. Junio called it %(headness). I don't like that.
>    I don't like %(current) either but we have to start somewhere.
>    Name suggestion? %(marker)??

How about %(HEAD)?

>  - %(tracking[:upstream]) gives us the exact output that branch -v[v]
>    does. %(upstream) does not include []. We can't change its
>    semantics.

There's already an atom called "upstream", and "upstream:short" works.
 Why not introduce "upstream:diff" for "[ahead x, behind y]" and
"upstream:shortdiff" for "<>" (like in the prompt)?

>  - %(color:...) is pretty much the same as %C family in pretty code.
>    I haven't added code for %(color:foo) == %C(foo) yet. There's a
>    potential ambiguity here: %C(red) == %Cred or %C(red)??

I'd vote for dropping %C<name> altogether and just go with %C(<name>).
 Why do we need %(color:<name>) at all?

Show 5 quoted lines

>  - %(...:aligned) to do left aligning. I'm not entirely sure about
>    this. We might be able to share code with %>, %< and %>< from
>    pretty.c. But we need improvements there too because in
>    for-each-ref case, we could calculate column width but %< would
>    require the user to specify the width.

Yeah, I think we should go with the %> and %< you introduced in pretty.c. Yes, I want to be able to specify width.

>    Do people expect fancy layout with for-each-ref (and branch)? If so
>    we might need to have %(align) or something instead of the simple
>    left alignment case in %(...:aligned)

Why should we deviate from the pretty case?  What is different here?

>  - We may need an equivalent of the space following % in pretty
>    format. If the specifier produces something, then prepend a space,
>    otherwise produce nothing. Do it like %C( tracking) vs
>    %C(tracking)??

Yeah, sounds good.

> You can try this after applying the series, which should give you the
> about close to 'branch -v'. %(tracking) coloring does not work though.

Why doesn't %(tracking) coloring work?

> Nguyễn Thái Ngọc Duy (9):

I'll have a look at this.

Also, I think it'll help to have a --pretty="format:<string>" equivalent to --format="<string>" so that we can introduce pretty names like oneline, short, medium, full. We can eventually deprecate --format for consistency.