Re: [PATCH/RFC 09/10] ref-filter: introduce contents_atom_parser()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 13, 2015, 04:41 UTC
Message-ID: <CAPig+cTVqTc1S8gduLq3qiyuzJNb=LNQiXijEHwqPPzi1YU8UQ@mail.gmail.com>
In-Reply-To: <CAPig+cTbT60cWk+=6EARte68mmQ2xkS5xFgsgh0FbpoueFcBbg@mail.gmail.com>

On Sat, Dec 12, 2015 at 10:10 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 10 quoted lines

> 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).

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

Show 13 quoted lines

>> +       if (!strcmp(buf, "body"))
>> +               atom->u.contents.body = 1;
>> +       else if (!strcmp(buf, "signature"))
>> +               atom->u.contents.signature = 1;
>> +       else if (!strcmp(buf, "subject"))
>> +               atom->u.contents.subject = 1;
>> +       else if (skip_prefix(buf, "lines=", &buf)) {
>> +               atom->u.contents.lines = 1;
>> +               if (strtoul_ui(buf, 10, &atom->u.contents.no_lines))
>> +                       die(_("positive value expected contents:lines=%s"), buf);
>> +       } else
>> +               die(_("improper format entered contents:%s"), buf);
>> +}