Subject: Re: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer

From: Junio C Hamano <[email-masked]>
Date: Sun, 02 Aug 2015 10:58:19 -0700

Karthik Nayak <[email-masked]> writes:

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

My knee-jerk reaction to the former was "Eeww, the users is forced
to keep verbosely typing unnecessarily '%(', ')%(then)' forever,
only because the implementor was too lazy to do the job properly in
parse_atom()".  I do not think the latter a good idea at all.

But I think the former is worth considering, as it will later allow
us to extend it to various forms, e.g.

    %(if:notempty)%(atom)%(then)...%(end)
    %(if)%(atom)%(then)...%(elif)%(atom1)%(atom2)%(then)...%(end)

Two points being

 (1) the default "string is not empty" does not have to be the only
     test criteria, by leaving the door to add %(if:<condition>)
     later;

 (2) what is tested does not have to be a single atom (that is why
     I do not think the one without %(then) good at all) but can be
     a string that can later be interpreted.

And syntactically, something like this even may want to be
introduced later:

    %(if:expr)master == %(refname:short)%(then)...%(end)