Re: [RFC/PATCH] tag: add --points-at list option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 5, 2012, 23:31 UTC
- Message-ID
- <7vvcnkeu2i.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1328480887-27463-1-git-send-email-tmgrennan@gmail.com>
Tom Grennan <tmgrennan@gmail.com> writes:
Show 14 quoted lines
> @@ -105,16 +107,28 @@ static int show_reference(const char *refname, const unsigned char *sha1,
> return 0;
> }
>
> + buf = read_sha1_file(sha1, &type, &size);
> + if (!buf || !size)
> + return 0;
> +
> + if (filter->points_at) {
> + unsigned char tagged_sha1[20];
> + if (memcmp("object ", buf, 7) \
> + || buf[47] != '\n' \
> + || get_sha1_hex(buf + 7, tagged_sha1) \
> + || memcmp(filter->points_at, tagged_sha1, 20)) {Do we need these backslashes at the end of these lines?
Show 8 quoted lines
> @@ -143,16 +157,20 @@ static int show_reference(const char *refname, const unsigned char *sha1,
> }
>
> static int list_tags(const char **patterns, int lines,
> - struct commit_list *with_commit)
> + struct commit_list *with_commit,
> + unsigned char *points_at)
> {It strikes me somewhat odd that you can give a list of commits to filter when using "--contains" (e.g. "--contains v1.7.9 --contains 1.7.8.4"), but you can only ask for a single object with "--points-at" from the UI point of view.
Show 18 quoted lines
> @@ -375,12 +393,28 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
> return check_refname_format(sb->buf, 0);
> }
>
> +int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
> +{
> + unsigned char *sha1;
> +
> + if (!arg)
> + return -1;
> + sha1 = xmalloc(20);
> + if (get_sha1(arg, sha1)) {
> + free(sha1);
> + return error("malformed object name %s", arg);
> + }
> + *(unsigned char **)opt->value = sha1;
> + return 0;
> +}We are ignoring earlier --points-at argument without telling the user that we do not support more than one.
Would it become too much unnecessary addition of new code if you supported multiple --points-at on the command line for the sake of consistency?
Show 10 quoted lines
> @@ -417,6 +451,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)"HEAD",
> + },I wonder if defaulting to HEAD even makes sense for --points-at. When you are chasing a bug and checked out an old version that originally had problem, "git tag --contains" that defaults to HEAD does have a value. It tells us what releases are potentially contaminated with the buggy commit.
But does a similar use case support points-at that defaults to HEAD?
Other than that, thanks for a pleasant read.