Re: [PATCH 1/9] ref-filter: implement %(if), %(then), and %(else) atoms

From: Karthik Nayak <karthik.188@gmail.com>
Date: Oct 3, 2015, 07:05 UTC
Message-ID: <CAOLa=ZQ31+gEktXh=QJJL+_McRMHhQynHAg__DpfuUsdc-5uWg@mail.gmail.com>
In-Reply-To: <xmqqpp0x3spo.fsf@gitster.mtv.corp.google.com>

On Sat, Oct 3, 2015 at 2:15 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +static int is_empty(const char * s){
>> +     while (*s != '\0') {
>> +             if (!isspace(*s))
>> +                     return 0;
>> +             s++;
>> +     }
>> +     return 1;
>> +}
>
> My knee-jerk reaction was "why is space so special?", but if a
> caller really cared, it can do "%(if:not_equal=)%(something)%(then)"
> to unignore spaces in %(something), so it is not a huge deal.  It
> may be that ignoring spaces when checking if something is empty is
> so common that this default is useful---I cannot tell offhand.
>
>

My reason for doing this was the "HEAD" atom which prints "*" or " ", hence I thought strings with only spaces in general shouldn't be accepted.

Show 12 quoted lines

>> +if::
>> +     Used as %(if)..%(then)..(%end) or %(if)..%(then)..%(else)..%(end).
>> +     If there is an atom with value or string literal after the
>> +     %(if) then everything after the %(then) is printed, else if
>> +     the %(else) atom is used, then everything after %(else) is
>> +     printed.
>
> I notice that "we ignore space when evaluating the string before
> %(then)" is not mentioned here.  That fact, and an example or two
> that illustrates the situation where this "ignore spaces" behaviour
> is useful, would be a good thing to document here.
>

Oh Yes! Will do that :)

-- 
Regards,
Karthik Nayak