On Tue, Feb 07, 2012 at 12:35:19AM -0800, Junio C Hamano wrote:
>Tom Grennan <tmgrennan@xxxxxxxxx> writes:
>
>> +struct points_at {
>> + struct points_at *next;
>> + unsigned char *sha1;
>> +};
>
>struct points_at {
> struct points_at *next;
> unsigned char sha1[20];
>};
>
>would save you from having to allocate and free always in pairs, no?
Yep
>> +static void free_points_at (struct points_at *points_at)
>
>Please lose the SP before (.
Oops
>> + if (type != OBJ_TAG
>> + || (tag = lookup_tag(sha1), !tag)
>> + || parse_tag_buffer(tag, buf, size) < 0) {
>
>Even though I personally prefer to cascade a long expression like this, so
>that you see a parse tree when you tilt your head 90-degrees to the left,
>I think the prevalent style in Git codebase is
>
> if (A-long-long-expression ||
> B-long-long-expression ||
> C-long-long-expression) {
>
>Also we try to avoid assignment in the conditional.
I like to compact multiple conditions to a common exit but also appreciate
the fear and loathing of comma's.
While rearranging this I finally understand how to include lightweight tags.
struct points_at *pa;
const unsigned char *tagged_sha1 = (const unsigned char *)"";
/* First look for lightweight tags - those with matching sha's
* but different names */
for (pa = points_at; pa; pa = pa->next)
if (!hashcmp(pa->sha1, sha1) && strcmp(pa->refname, refname))
return pa;
buf = read_sha1_file(sha1, &type, &size);
if (buf) {
if (type == OBJ_TAG) {
tag = lookup_tag(sha1);
if (parse_tag_buffer(tag, buf, size) >= 0)
tagged_sha1 = tag->tagged->sha1;
}
free(buf);
}
while (points_at && hashcmp(points_at->sha1, tagged_sha1))
points_at = points_at->next;
return points_at;
For example,
$ ./git-tag tomg-lw-v1.7.9 v1.7.9
$ ./git-tag -a tomg-lw-v1.7.9 v1.7.9
$ ./git-tag -s tomg-lw-v1.7.9 v1.7.9
$ ./git-tag -s tomg-README HEAD:README
$ ./git-tag -l --points-at v1.7.9 --points-at HEAD:README
tomg-README
tomg-annotate-v1.7.9
tomg-lw-v1.7.9
tomg-signed-v1.7.9
$ ./git-tag -l --points-at v1.7.9 --points-at HEAD:README \*v1.7.9
tomg-annotate-v1.7.9
tomg-lw-v1.7.9
tomg-signed-v1.7.9
>> @@ -432,6 +500,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>> PARSE_OPT_LASTARG_DEFAULT,
>> parse_opt_with_commit, (intptr_t)"HEAD",
>> },
>> + {
>> + OPTION_CALLBACK, 0, "points-at", &points_at, "object",
>> + "print only annotated|signed tags of the object",
>> + PARSE_OPT_LASTARG_DEFAULT,
>> + parse_opt_points_at, (intptr_t)NULL,
>> + },
>
>If you are going to reject NULL anyway, do you still need to mark this as
>lastarg-default?
>
>Looking for example in parse-options.h, I found this:
>
> #define OPT_STRING_LIST(s, l, v, a, h) \
> { OPTION_CALLBACK, (s), (l), (v), (a), \
> (h), 0, &parse_opt_string_list }
>
>which is used by "git clone" to mark its -c option.
>
>Running "git clone -c" gives me
>
> error: switch 'c' requires a value
>
>without any extra code in the caller of parse_options().
Cool
>Other than that, looks cleanly done.
>
>Thanks. I'll take another look after I wake up in the morning.
Thanks, I'll send v3 later today.
--
TomG
--
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