On Sun, Dec 13, 2015 at 10:19 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Wed, Nov 25, 2015 at 8:44 AM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>> 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 <Karthik.188@xxxxxxxxx>
>> ---
>> 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'.
>
>> } 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).
>
I guess I responded to the same issue, will work on it.
>> +
>> + if (!buf)
>> + return;
>
> Same comment about misplaced blank line: Put the blank line after the
> conditional rather than before or drop it altogether.
>
Will change.
>> + if (!strcmp(buf, "short"))
>> + atom->u.objectname.shorten = 1;
>> + else
>> + die(_("improper format entered objectname:%s"), buf);
>
> Maybe just "unrecognized objectname:%s" or something?
>
die(_("unrecognized %%(objectname) argument: %s"), buf);
to keep things consistent
>> +}
>> +
>> @@ -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
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html