Thread (28 messages) flat view 28 messages, 3 authors, 2016-06-15

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

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:07:39

On Tue, Jan 5, 2016 at 3:02 AM, Karthik Nayak [off-list ref] wrote:
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help