Re: [PATCH v7 03/17] ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)

[Jacob Keller <jacob.keller@gmail.com>]

On Thu, Nov 10, 2016 at 3:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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).

The main advantage I see is that it ensures in the API that there is
no way for it to be both "equal" and "not equal" at the same time,
where as two separate pointers while not actually done in practice
could both be set somehow, leading to potential questions.

Thanks,
Jake