Re: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom

[Karthik Nayak <karthik.188@gmail.com>]

On Wed, Jul 29, 2015 at 11:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>> A handful of "huh?" on the design.
>>>
>>>  - The atom says "if *exists*" and explanation says "has a value".
>>>    How are they related?  Does an atom whose value is an empty
>>>    string has a value?  Or is "ifexists" meant to be used only to
>>>    ignore meaningless atom, e.g. %(*objectname) applied to a ref that
>>>    refers to an object that is not an annotated tag?
>>
>> It's meant to ignore meaningless atom. atom's whose values are empty
>> strings are ignored.
>
> That is a self-contradicting answer.
>
> If you ask for "%(*objectname)" on a commit, that request truly is
> meaningless, as a commit is not an annotated tag that points at another
> object whose objectname is being asked for.
>
> But if a commit has an empty log message (you should be able to
> create such an object with commit-tree), then "%(subject)" would be
> an empty string.  The fact that the commit happens to have an empty
> string as its message is far from meaningless.
>
> Either you ignore an empty string, or you ignore meaningless one.
> Which does "ifexists" mean?

I meant ignore atom values which are empty, sorry for the confusion.

>
>>>  - That %s looks ugly.  Are there cases where a user may want to say
>>>    %(ifexists:[%i]) or something other than 's' after that per-cent?
>>
>> Couldn't think of a better replacer, any suggestions would be welcome :)
>
> See below.
>
>> Its given as example, is that misleading?
>
> Othewise I wouldn't be asking.
>
>>>  - What, if anything, is allowed to come between %(ifexists...) and
>>>    the next atom like %(refname)?  For example, are these valid
>>>    constructs?
>>>
>>>     . %(ifexists...)%(padright:20)%(refname)
>>
>> Doesn't work ...
>> ...
>>>  - This syntax does not seem to allow switching on an attribute to
>>>    show or not to show another, e.g. "if %(*objectname) makes sense,
>>>    then show '%(padright:20)%(refname:short) %(*subject)' for it".
>>
>> Yes this doesn't do that,
>
> One way to do all of the above is to make it
>
>     %(ifexists:atom:expansionString)
>
> That is, for example:
>
>  "%(ifexists:*objectname:tag %(color:blue)%(refname:short)%(color:reset))"
>
> would give you a string "tag v1.0" with "v1.0" painted in blue for
> refs/tags/v1.0 but nothing for refs/heads/master.
>
> Obviously expansionString part needs some escaping mechanism to
> allow it to include an unmatched ")".

I liked your other idea of if and endif better :)

-- 
Regards,
Karthik Nayak