From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 10 Nov 2016 23:26:22 GMT
Subject: Re: [PATCH v7 03/17] ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
Message-ID: <xmqqoa1m5335.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CA+P7+xoavgQ=NK8GGGXf90ESuPs+mFwHzEWbsnjWjZ4V7TWpjw@mail.gmail.com>

Jacob Keller <jacob.keller@gmail.com> writes:

>> @@ -49,6 +51,10 @@ static struct used_atom {
>>                         enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option;
>>                         unsigned int nlines;
>>                 } contents;
>> +               struct {
>> +                       const char *if_equals,
>> +                               *not_equals;
>
>
> Same here, why do we need both strings here stored separately? Could
> we instead store which state to check and store the string once? I'm
> not sure that really buys us any storage.

I am not sure if storage is an issue, but I tend to agree that it
would be semantically cleaner if this was done as a pair of <what
operation uses this string constant?, the string constant>, and the
former would be enum { COMPARE_EQUAL, COMPARE_UNEQUAL}.

You could later enhance the comparison operator more easily with
such an arrangement (e.g. if-equals-case-insensitively).