From: Karthik Nayak <karthik.188@gmail.com>
Date: Sun, 13 Dec 2015 19:36:09 GMT
Subject: Re: [PATCH/RFC 09/10] ref-filter: introduce contents_atom_parser()
Message-ID: <CAOLa=ZQ4wpeerP2x5dKy51hDosqWQ7wgasP-8hbbOU_ydm3N8A@mail.gmail.com>
In-Reply-To: <CAPig+cTVqTc1S8gduLq3qiyuzJNb=LNQiXijEHwqPPzi1YU8UQ@mail.gmail.com>

On Sun, Dec 13, 2015 at 10:11 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Dec 12, 2015 at 10:10 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Wed, Nov 11, 2015 at 2:44 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>>> +void contents_atom_parser(struct used_atom *atom)
>>> +{
>>> +       const char * buf;
>>> +
>>> +       if (match_atom_name(atom->str, "contents", &buf))
>>> +               atom->u.contents.all = 1;
>>> +
>>> +       if (!buf)
>>> +               return;
>
> Also, isn't this logic somewhat bogus? u.contents.all is set to 1 for
> both bare %(contents) and decorated %(contents:whatever). Then, below,
> you also set one of .body, .signature, .subject, or .lines if a
> decoration is specified. So, now you have both .all and one of the
> other attributes set to 1, which is rather nonsensical (if I
> understand correctly).
>

The problem is its not mutually exclusive here, the 'all' was supposed to
act as a way of checking if its a contents atom, since populate_value() doesn't
really check that again.

So if any of the others were selected we would implement %(contents:<value>)
else the bare %(contents:<value>) would be selected.

> If you change this to an enum as suggested in my previous email, then
> the problem goes away.
>

Yup. Will do. Thanks

-- 
Regards,
Karthik Nayak