From: Jeff King <peff@peff.net>
To: Tom Grennan <tmgrennan@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org, jasampler@gmail.com
Subject: Re: [PATCHv2] tag: add --points-at list option
Date: Tue, 7 Feb 2012 19:25:54 -0500 [thread overview]
Message-ID: <20120208002554.GA6035@sigill.intra.peff.net> (raw)
In-Reply-To: <20120207220806.GD6264@tgrennan-laptop>
On Tue, Feb 07, 2012 at 02:08:06PM -0800, Tom Grennan wrote:
> v1 and v2 wouldn't list lightweight tags of the points-at objects.
> Both versions behave like this:
> $ git tag my-lw-v1.7.9 v1.7.9
> $ git tag my-a-v1.7.9 v1.7.9
> $ git tag my-s-v1.7.9 v1.7.9
> $ git tag -l --points-at v1.7.9
> my-a-v1.7.9
> my-s-v1.7.9
I assume the 2nd and 3rd line should be:
$ git tag -a my-a-v1.7.9 v1.7.9
$ git tag -s my-s-v1.7.9 v1.7.9
> static struct points_at *match_points_at(struct points_at *points_at,
> const char *refname,
> const unsigned char *sha1)
> {
> struct object *obj;
> struct points_at *pa;
> const unsigned char *tagged_sha1;
>
> /* 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;
OK, I see what you are trying to accomplish here. But I really don't
like it. Two complaints:
1. Why is the name of the tag relevant? That is, if you are interested
in lightweight tags, and you have two tag refs, "refs/tags/a" and
"refs/tags/b", both pointing to the same tag object, then in what
situation is it useful to show "a" but not "b"?
It seems to me you would either want lightweight tags or not. And I
thought not, because the point of this was to reveal signatures or
annotations about a tag. Your my-lw-v1.7.9 says neither. Why do we
want to show it?
Also, it's not symmetric. What if I say "git tag
--points-at=my-lw-v1.7.9"? Then I would get your signed and
annotated tags (even though they're _not_ saying anything about
ny-lw-v1.7.9), and I would get v1.7.9 (even though it's not saying
anything about it either; in fact, it's the opposite!).
2. I thought --points-at was about providing an object name. But it's
not. It's about providing a particular string. So with this code,
"git tag --points-at=v1.7.9" and "git tag --points-at=$(git
rev-parse v1.7.9)" are two different things. Which seems odd and
un-git-like to me.
Your documentation says "Only list annotated or signed tags of the
given object", which implies to me that --points-at is an arbitrary
object specifier, not a specific tagname.
It seems like your rationale is just avoiding a mention of v1.7.9
because, hey, it was obviously on the command line and the user isn't
interested in it. But I don't think that's true. The user asked for
every tag pointing to v1.7.9's object, and v1.7.9 is such a tag. It is
no more or less true for v1.7.9 than it is for my-lw-v1.7.9.
-Peff
next prev parent reply other threads:[~2012-02-08 0:26 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-05 22:28 [RFC/PATCH] tag: add --points-at list option Tom Grennan
2012-02-05 23:31 ` Junio C Hamano
2012-02-06 5:48 ` Tom Grennan
2012-02-06 6:25 ` Junio C Hamano
2012-02-06 6:45 ` Tom Grennan
2012-02-06 0:04 ` Jeff King
2012-02-06 6:32 ` Tom Grennan
2012-02-06 7:04 ` Jeff King
2012-02-06 7:13 ` Jeff King
2012-02-06 7:45 ` Jeff King
2012-02-06 8:11 ` Jeff King
2012-02-06 8:13 ` [PATCH 1/3] tag: fix output of "tag -n" when errors occur Jeff King
2012-02-06 8:13 ` [PATCH 2/3] tag: die when listing missing or corrupt objects Jeff King
2012-02-06 8:32 ` Junio C Hamano
2012-02-06 8:34 ` Jeff King
2012-02-06 8:36 ` Junio C Hamano
2012-02-06 8:38 ` Jeff King
2012-02-06 18:04 ` Junio C Hamano
2012-02-06 18:13 ` Junio C Hamano
2012-02-06 20:12 ` Jeff King
2012-02-06 23:34 ` Junio C Hamano
2012-02-08 21:01 ` Jeff King
2012-02-09 4:33 ` Junio C Hamano
2012-02-06 8:14 ` [PATCH 3/3] tag: don't show non-tag contents with "-n" Jeff King
2012-02-07 7:01 ` [PATCHv2] tag: add --points-at list option Tom Grennan
2012-02-07 7:01 ` Tom Grennan
2012-02-07 8:35 ` Junio C Hamano
2012-02-07 18:05 ` Tom Grennan
2012-02-07 16:05 ` Jeff King
2012-02-07 19:02 ` Tom Grennan
2012-02-07 19:12 ` Jeff King
2012-02-07 19:22 ` Tom Grennan
2012-02-07 19:36 ` Jeff King
2012-02-07 20:20 ` Junio C Hamano
2012-02-07 21:30 ` Jeff King
2012-02-07 22:08 ` Tom Grennan
2012-02-08 0:25 ` Jeff King [this message]
2012-02-08 1:45 ` Tom Grennan
2012-02-08 15:31 ` Jeff King
2012-02-08 6:21 ` [PATCHv3] " Tom Grennan
2012-02-08 6:21 ` Tom Grennan
2012-02-08 15:44 ` Jeff King
2012-02-08 18:43 ` Tom Grennan
2012-02-08 18:57 ` Jeff King
2012-02-08 20:12 ` [PATCHv4] " Tom Grennan
2012-02-08 20:12 ` Tom Grennan
2012-02-08 20:58 ` Jeff King
2012-02-08 22:15 ` Tom Grennan
2012-02-08 23:03 ` [PATCH-master] " Tom Grennan
2012-02-08 23:03 ` Tom Grennan
2012-02-09 1:44 ` Jeff King
2012-02-09 4:29 ` Junio C Hamano
2012-02-08 18:58 ` [PATCHv3] " Tom Grennan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120208002554.GA6035@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jasampler@gmail.com \
--cc=tmgrennan@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).