Re: [PATCH] Add git-findtags

Martin Langhoff <martin@catalyst.net.nz> writes:

> +my @tagfiles   = `find $git_dir/refs/tags -follow -type f`; # haystack

Please do this with File::Find(), unless you have a compelling
reason not to.  $git_dir could contain shell metacharacters
and/or whitespace.

> +    # grab the first 2 lines (the whole tag could be large)
> +    my $tagobj = `git-cat-file tag $tagid | head -n2 `;

Careful; $tagfile could be a lightweight tag, and you would want
to consider it a hit if ($tagid eq $target).

BTW, does `` in Perl behave well upon SIGPIPE (hehe, we now have
Merlyn on the list so I can ask these silly Perl questions ;-))?

> +    if ($tagobj =~  m/^type commit$/m) { # only deal with commits
> +
> +	if ($tagobj =~ m/^object $target$/m) { # match on the commit
> +	    print basename($tagfile) . "\n";

Please show "tags/bl/ah" when ".git/refs/tags/bl/ah" points at
the target, not just "ah".  Alternatively, "bl/ah" may also be
acceptable, since this command is about tags.

> +	} elsif ( $opt_t &&                      # follow the commit
> +		 $tagobj =~ m/^object (\S+)$/m) { # and try to match trees

I am not sure how useful '-t' would be in practice.  If it is, I
wonder if it would also be useful to look for a subtree match..

> +sub quickread {
> +    my $file = shift;
> +    local $/; undef $/; # slurp mode

Nit -- "local $/;" without "undef $/;" would do just fine.

BTW, wouldn't it be easier for this particular script, and more
useful in general, if something like what 'git-rev-parse' does
for commit objects when given "REV^0" is supported for tags?

I am uncertain the about syntax/notation, but just like "^0" is
a postfix operator for "peel the onion repeatedly until you get
a commit, or barf if you ended up with a non-commit", maybe if
we had "peel the onion repeatedly until you get a non-tag"
operator, let's call it '%', then you could say something like
this:

        # This is pseudo-code -- it does not handle tagname with
        # embedded $IFS letter very well.

        # list local refs
	git-rev-parse --symbolic --all |

        # limit only to tags
        sed -ne '/^refs\/tags\/p' |
        while read tagname
        do
        	if test $(git-rev-parse --verify "${tagname}%") = "$target"
		then
                	echo "$tagname"
		fi
	done

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html