Re: [PATCH 15/21] Make tag names (i.e. the tag object's "tag" line) optional

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 10, 2007, 09:07 UTC
Message-ID: <Pine.LNX.4.64.0706101004440.4059@racer.site>
In-Reply-To: <200706090219.12011.johan@herland.net>

Hi,

On Sat, 9 Jun 2007, Johan Herland wrote:

Show 9 quoted lines

> The tag line is now optional. If not given in the tag object data, it
> defaults to the empty string ("") in the parsed tag object.
> 
> The patch also adds a change to git-show; when asked to display a tag
> object with no name (missing "tag" header), we will show the tag's sha1
> instead of an empty string.
> 
> Finally the patch includes some tweaks to the selftests to make them 
> work with optional tag names.

If you don't actually _test_ missing tag names, you might just as well leave the tests alone.

> -					t->tag,
> +					*(t->tag) ? t->tag : name,

This is misleading. What you wanted to say is t->tag[0] == '\0', or *(t->tag) == '\0'.

As you wrote it, you have to think a couple of times why it is okay to dereference t->tag, to check if you say t->tag.

Besides, it breaks if you _do_ have an empty tag. In that case, I _want_ to see that it is actually empty, and _not_ the SHA1 substituted for it.

Ciao, Dscho