From: Karthik Nayak <karthik.188@gmail.com>
Date: Sun, 02 Aug 2015 12:59:08 GMT
Subject: Re: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer
Message-ID: <CAOLa=ZREZ2M_txEQR9jPET5_+MXx0D6cfW0X7SS4-TP9MShVrA@mail.gmail.com>
In-Reply-To: <CAPig+cTyK=xDbtsK+q0eL=wmZ87xWue8SMJm=DQOzX22R1AOtQ@mail.gmail.com>

On Sat, Aug 1, 2015 at 2:33 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Aug 1, 2015 at 2:48 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> On Thu, Jul 30, 2015 at 2:57 AM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Good point! I just was wondering if we need another atom just to print a star.
>> But your points certainly are valid. I'll implement this. Thanks :)
>>
>>>> This would remove the need of making the printing of the "*" to be
>>>> needed by ref-filter. As this is only needed in branch.c
>>>>
>>>> If going on what you're saying we could have a "%(starifcurrent)" atom or
>>>> something, but I don't see a general use for this.
>>>
>>> To have a really customizeable format, you would want to allow e.g.
>>>
>>>   git branch --format '%(starifcurrent) %(objectname) %(refname)'
>>>
>>> if the user wants to get the sha1 before the refname, and still have the
>>> star in front. It's a bit frustrating to have a hardcoded format that
>>> the user can't reproduce with the --format option, since it means you
>>> can't easily make a small variation on it.
>>
>> Agreed. will have a "starifcurrent" atom :)
>
> Please don't. It's better to avoid these highly specialized solutions
> and instead seek general purpose ones. For instance, utilizing the
> %(if:) atom suggested elsewhere, you might add an %(iscurrentbranch)
> which would allow a format like this:
>
>     %(if:iscurrentbranch)*%(endif)
>
> Even more generic would be an %(ifeq:x:y) conditional and a
> %(currentbranch) atom:
>
>     %(ifeq:refname:currentbranch)*%(endif)
>
> Those are just a couple ideas. Other variations are possible and
> likely preferable to the specialized %(starifcurrent).

This makes sense, thanks. But implementing something like
"%(if:<atom>)" seems to not be as easy as I thought it would be.

First we need to parse that inner atom, but the used_atom_cnt is based
on how many atoms there are initially, which doesn't count this inner atom.

Although we could have a way around that, we'd need to again call populate_value
from itself to get that inner atom's value. This causes more problems.
Either ways
I'm looking at ways around this.
A simple solution would be to do :

%(if)%(atom)%(then).....%(endif)

or just

%(if)%(atom).....%(endif)

-- 
Regards,
Karthik Nayak