Re: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
On Wed, Jul 29, 2015 at 11:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 26 quoted lines
> 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.
Show 39 quoted lines
>
>>> - 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