On Tue, Feb 07, 2012 at 02:12:02PM -0500, Jeff King wrote:
Show 12 quoted lines
>On Tue, Feb 07, 2012 at 11:02:28AM -0800, Tom Grennan wrote:
>
>> >Would using sha1_array save us from having to create our own data
>> >structure? As a bonus, it can do O(lg n) lookups, though I seriously
>> >doubt anyone will provide a large number of "--points-at".
>>
>> Thanks, but I now realize that I also need to save the pointed at
>> refname to detect lightweight tags that have matching sha's but
>> different names.
>
>I'm not sure I understand. Wouldn't you match lightweight tags by the
>sha1 they point at? Something like:
I think the following would show the pointed at tag too.
$ git tag my-v1.7.9 v1.7.9
$ ./git-tag -l --points-at v1.7.9
my-v1.7.9
v1.7.9
vs.
$ ./git-tag -l --points-at v1.7.9
my-v1.7.9
I found that I had to filter matching refnames.
Show 60 quoted lines
> static int tag_points_at(struct sha1_array *sa,
> const unsigned char *sha1)
> {
> struct object *obj;
>
> /* Lightweight tag of an interesting sha1? */
> if (sha1_array_lookup(sa, sha1) >= 0)
> return 1;
>
> /* Otherwise, maybe a tag object pointing to an interesting sha1 */
> obj = parse_object(sha1);
> if (!obj)
> return 0; /* or probably we should even just die() */
> if (obj->type != OBJ_TAG)
> return 0;
> if (sha1_array_lookup(sa, ((struct tag *)obj)->tagged->sha1) < 0)
> return 0;
> return 1;
> }
>
>> >Also, should you check "unset"? When we have options that build a list,
>> >usually doing "--no-foo" will clear the list. E.g., this:
>> >
>> > git tag --points-at=foo --points-at=bar --no-points-at --points-at=baz
>> >
>> >should look only for "baz".
>>
>> Ahh, so I just need to:
>> if (unset) {
>> if (*opt_value)
>> free_points_at(*opt_value);
>> *opt_value = NULL;
>> return 0;
>> }
>
>Yes, exactly.
>
>> >> + {
>> >> + 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,
>> >> + },
>> >
>> >I think you can drop the LASTARG_DEFAULT here, as it is no longer
>> >optional, no?
>>
>> You mean flags = 0 instead of PARSE_OPT_LASTARG_DEFAULT, right?
>
>Right. Though without flags, you can probably just use the OPT_CALLBACK
>wrapper, like:
>
> OPT_CALLBACK(0, "points-at", &points_at, "object",
> "print only annotated|signed tags of the object",
> parse_opt_points_at)
>
>Note that if you are going to handle lightweight tags, that description
>should probably be updated.
>
>-PeffTom Grennan <tmgrennan@gmail.com>