On Wed, Dec 16, 2015 at 10:30 AM, Karthik Nayak <[email protected]> wrote:
> Introduce contents_atom_parser() which will parse the '%(contents)'
> atom and store information into the 'used_atom' structure based on the
> modifiers used along with the atom.
>
> Helped-by: Ramsay Jones <[email protected]>
> Signed-off-by: Karthik Nayak <[email protected]>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -39,6 +39,10 @@ static struct used_atom {
> struct align align;
> enum { RR_SHORTEN, RR_TRACK, RR_TRACKSHORT, RR_NORMAL }
> remote_ref;
> + struct {
> + enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG,
> C_SUB } option;
> + unsigned int no_lines;
'no_lines' sounds like "no lines!". How about 'nlines' instead?
> + } contents;
> } u;
> } *used_atom;
> static int used_atom_cnt, need_tagged, need_symref;
> @@ -90,6 +94,36 @@ static void remote_ref_atom_parser(struct used_atom *atom)
> +static void contents_atom_parser(struct used_atom *atom)
> +{
> + const char * buf;
> +
> + if (match_atom_name(atom->str, "contents", &buf))
> + atom->u.contents.option = C_BARE;
> + else if (match_atom_name(atom->str, "subject", &buf)) {
The original code used strcmp() and matched only "subject", however
the new code will incorrectly match both "subject" and
"subject:whatever". Therefore, you should be using strcmp() here
rather than match_atom_name().
Ditto for "body".
> + atom->u.contents.option = C_SUB;
> + return;
> + } else if (match_atom_name(atom->str, "body", &buf)) {
> + atom->u.contents.option = C_BODY_DEP;
> + return;
> + }
> + if (!buf)
> + return;
It's not easy to see that this 'if (!buf)' check relates to the
"contents" check at the very top of the if/else if/ chain since there
are entirely unrelated checks in between. Reorganizing it can improve
clarity:
if (!strcmp("subject")) {
...
return;
} else if (!strcmp("body")) {
...
return;
} else if (!match_atom_name(...,"contents", &buf))
die("BUG: expected 'contents' or 'contents:'");
if (!buf) {
atom->u.contents.option = C_BARE;
return;
}
> + if (!strcmp(buf, "body"))
> + atom->u.contents.option = C_BODY;
> + else if (!strcmp(buf, "signature"))
> + atom->u.contents.option = C_SIG;
> + else if (!strcmp(buf, "subject"))
> + atom->u.contents.option = C_SUB;
> + else if (skip_prefix(buf, "lines=", &buf)) {
> + atom->u.contents.option = C_LINES;
> + if (strtoul_ui(buf, 10, &atom->u.contents.no_lines))
> + die(_("positive value expected contents:lines=%s"),
> buf);
> + } else
> + die(_("unrecognized %%(contents) argument: %s"), buf);
> +}
> @@ -777,28 +801,23 @@ static void grab_sub_body_contents(struct atom_value
> *val, int deref, struct obj
> &bodypos, &bodylen, &nonsiglen,
> &sigpos, &siglen);
>
> - if (!strcmp(name, "subject"))
> + if (atom->u.contents.option == C_SUB)
> v->s = copy_subject(subpos, sublen);
> - else if (!strcmp(name, "contents:subject"))
> - v->s = copy_subject(subpos, sublen);
> - else if (!strcmp(name, "body"))
> + else if (atom->u.contents.option == C_BODY_DEP)
> v->s = xmemdupz(bodypos, bodylen);
> - else if (!strcmp(name, "contents:body"))
> + else if (atom->u.contents.option == C_BODY)
> v->s = xmemdupz(bodypos, nonsiglen);
> - else if (!strcmp(name, "contents:signature"))
> + else if (atom->u.contents.option == C_SIG)
> v->s = xmemdupz(sigpos, siglen);
> - else if (!strcmp(name, "contents"))
> - v->s = xstrdup(subpos);
> - else if (skip_prefix(name, "contents:lines=", &valp)) {
> + else if (atom->u.contents.option == C_LINES) {
> struct strbuf s = STRBUF_INIT;
> const char *contents_end = bodylen + bodypos - siglen;
>
> - if (strtoul_ui(valp, 10, &v->u.contents.lines))
> - die(_("positive value expected
> contents:lines=%s"), valp);
> /* Size is the length of the message after removing
> the signature */
> - append_lines(&s, subpos, contents_end - subpos,
> v->u.contents.lines);
> + append_lines(&s, subpos, contents_end - subpos,
> atom->u.contents.no_lines);
> v->s = strbuf_detach(&s, NULL);
> - }
> + } else if (atom->u.contents.option == C_BARE)
> + v->s = xstrdup(subpos);
> }
> }
>
> --
> 2.6.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
