Re: [PATCH v3 00/15] ref-filter: use parsing functions

On Thu, Jan 7, 2016 at 2:44 AM, Eric Sunshine [off-list ref] wrote:

On Tue, Jan 5, 2016 at 3:02 AM, Karthik Nayak [off-list ref] wrote:

quoted

Eric suggested that I make match_atom_name() not return a value [0]. I
haven't done that as we use match_atom_name() in [14/15] for matching
'subject' and 'body' in contents_atom_parser() and although Eric
suggested I use strcmp() instead, this would not work as we need to
check for derefernced 'subject' and 'body' atoms.
[0]: http://article.gmane.org/gmane.comp.version-control.git/282701

I don't understand the difficulty. It should be easy to manually skip
the 'deref' for this one particular case:

    const char *name = atom->name;
    if (*name == '*')
        name++;

Which would allow this unnecessarily complicated code from patch 14/15:

    if (match_atom_name(atom->name, "subject", &buf) && !buf) {
        ...
        return;
    } else if (match_atom_name(atom->name, "body", &buf) && !buf) {
        ...
        return;
    } if (!match_atom_name(atom->name, "contents", &buf))
        die("BUG: parsing non-'contents'");

to be simplified to the more easily understood form suggested during
review[1] of v2:

    if (!strcmp(name, "subject")) {
        ...
        return;
    } else if (!strcmp(name, "body")) {
        ...
        return;
    } else if (!match_atom_name(name,"contents", &buf))
        die("BUG: expected 'contents' or 'contents:'");

You could also just use (!strcmp("body") || !strcmp("*body")) rather
than skipping "*" manually, but the repetition makes that a bit
noisier and uglier.

[1]: http://article.gmane.org/gmane.comp.version-control.git/282645

Definitely not a difficulty per se. Just that it seems like something
match_atom_name()
seems to be fit for. As the function name suggests that we're matching
the atom name
and the check for '!buf' indicates that no options are to be included
for that particular atom.

Also after Junio's suggestion[1], I think It looks better now[2]. But
either ways, I'm not
strongly against what you're saying, so my opinion on this matter is
quite flexible.

[1]: http://article.gmane.org/gmane.comp.version-control.git/283404
[2]: http://article.gmane.org/gmane.comp.version-control.git/283449

-- 
Regards,
Karthik Nayak