Re: [PATCH/RFC 10/10] ref-filter: introduce objectname_atom_parser()
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:07:26
On Wed, Nov 25, 2015 at 8:44 AM, Karthik Nayak [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Introduce objectname_atom_parser() which will parse the '%(objectname)' atom and store information into the 'used_atom' structure based on the modifiers used along with the atom. Signed-off-by: Karthik Nayak <redacted> ---diff --git a/ref-filter.c b/ref-filter.c@@ -50,6 +50,10 @@ static struct used_atom { lines : 1, no_lines; } contents; + struct { + unsigned int shorten : 1, + full : 1; + } objectname;
Same comment as in my patch 8 and 9 reviews: If 'shorten' and 'full' are mutually exclusive, then an enum would be clearer. In fact, if there are only these two states (full and short), then this could be a simple boolean named 'shorten'.
quoted hunk ↗ jump to hunk
} u; } *used_atom; static int used_atom_cnt, need_tagged, need_symref;@@ -123,6 +127,21 @@ void contents_atom_parser(struct used_atom *atom) +void objectname_atom_parser(struct used_atom *atom) +{ + const char * buf; + + if (match_atom_name(atom->str, "objectname", &buf)) + atom->u.objectname.full = 1;
Same comment about bogus logic as in patch 9 review: u.objectname.full and u.objectname.shorten are both set to 1 for %(objectname:short).
+ + if (!buf) + return;
Same comment about misplaced blank line: Put the blank line after the conditional rather than before or drop it altogether.
+ if (!strcmp(buf, "short"))
+ atom->u.objectname.shorten = 1;
+ else
+ die(_("improper format entered objectname:%s"), buf);Maybe just "unrecognized objectname:%s" or something?
quoted hunk ↗ jump to hunk
+} +@@ -463,15 +482,16 @@ static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned lo } static int grab_objectname(const char *name, const unsigned char *sha1, - struct atom_value *v) + struct atom_value *v, struct used_atom *atom) { - if (!strcmp(name, "objectname")) { - v->s = xstrdup(sha1_to_hex(sha1)); - return 1; - } - if (!strcmp(name, "objectname:short")) { - v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV)); - return 1; + if (starts_with(name, "objectname")) { + if (atom->u.objectname.shorten) { + v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV)); + return 1; + } else if (atom->u.objectname.full) { + v->s = xstrdup(sha1_to_hex(sha1)); + return 1; + } } return 0; }@@ -495,7 +515,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct object v->s = xstrfmt("%lu", sz); } else if (deref) - grab_objectname(name, obj->sha1, v); + grab_objectname(name, obj->sha1, v, &used_atom[i]); } }@@ -1004,7 +1024,7 @@ static void populate_value(struct ref_array_item *ref) v->s = xstrdup(buf + 1); } continue; - } else if (!deref && grab_objectname(name, ref->objectname, v)) { + } else if (!deref && grab_objectname(name, ref->objectname, v, atom)) { continue; } else if (!strcmp(name, "HEAD")) { const char *head; --2.6.2