Re: [PATCH] Add option -L to git-tag.

> Matthijs Melchior <mmelchior@xs4all.nl> writes:
>
>   
>>   This will list the selected tags and include annotations, if any.
>>
>> Signed-off-by: Matthijs Melchior <mmelchior@xs4all.nl>
>> ---
>>
>> This patch has been created to allow me to easily see the annotations with tags.
>> I have not found any other way to do this...
>>     
>
> Hmmmm.  This feature ought to belong to git-show, but that
> command already has its own interpretation of how a tag should
> be shown.
>
> Do we care about the "one line summary"?  Perhaps...
>
> 	$ git tag --pretty=short -L v2.6.11-tree
> 	v2.6.11-tree
>             This is the 2.6.11 tree object.
>
> 	$ git tag -L v2.6.11-tree
> 	v2.6.11-tree
>             This is the 2.6.11 tree object.
>
> 	    NOTE! There's no commit for this, since it happened...
>
> The answer to this question would really depend on why we would
> want to have this feature.  "git-tag" started as a way to
> "create", and then it gained -l to "list" and -v to "verify".  I
> think your -L is meant to be an extension to "list", but I
> suspect that while you are listing many things you would keep
> the annotation short-and-sweet, and you would want to view the
> full description when your interest is focused at a single one.
> Maybe we would want a separate "print" action for the full
> contents and use the short one for "list but more verbosely than
> usual"?
>
> I dunno; I've never been very good at the user interfaces.
>   

>   
>>  - Sorting the tag names resulting from git-rev-parse is not nessecary since
>>    the list of tags is already deliverd in sorted order.
>>     
>
> This I am a bit reluctant about, as that sorting done by
> rev-parse is purely by accident (i.e. it is an implementation
> detail).
>   

>   
>>  - Using git-cat-file -t on every tag is expensive, but there is no alternative
>>     
>
> This is Ok, as we have somebody working on doing git-tag as a
> built-in, and once that happens, we do not have to pay the
> performance penalty.  So I would think at this point we should
> concentrate on discussing the usefulness of this new feature and
> correctness of your implementation, as that would set the course
> for the future.  On the other hand, "cat-file -t" performance
> issues will not stay with us forever.
>
>   
>> @@ -26,13 +26,23 @@ do
>>      -f)
>>  	force=1
>>  	;;
>> -    -l)
>> -	case "$#" in
>> -	1)
>> -		set x . ;;
>> -	esac
>> +    -l|-L)
>> +	TAGSONLY=true
>> +	[ "$1" = -L ] && TAGSONLY=false
>> +	[ "$#" = 1 ] && set x .
>>  	shift
>> +	git rev-parse --symbolic --tags | grep "$@" |
>> +	    while read TAG
>> +	    do
>> +		echo "$TAG"
>> +		$TAGSONLY && continue
>> +		OBJTYPE=$(git cat-file -t "$TAG")
>> +		case $OBJTYPE in
>> +		    tag)    git cat-file $OBJTYPE "$TAG" |
>> +				sed '1,/^$/d;/^-----BEGIN PGP SIGNATURE-----$/Q;s/^/    /'
>> +			    ;;
>> +		esac
>>     
>
> Micronit.  If you already know it is a tag, you do not have to
> say "cat-file $OBJTYPE".
>   

> Style.  Please indent the case arm to the same level as, not
> deeper than, case/esac.
> This is the same as C's switch() { case ...: } indentation rule.
>
> Please do not feed multiple expressions concatenated with
> semicolon to sed, as it is one of the often observed portability
> issues (not all the world is GNU yet).  Write it like this
> instead:
>
>         case "$OBJTYPE" in
>         tag)
>                 git cat-file tag "$TAG" |
>                 sed -e '1,/^$/d' \
>                     -e '/^-----BEGIN PGP SIGNATURE-----$/Q' \
>                     -e s/^/    /'
>                 ;;
>         esac
>
>   
>> +	    done
>>  	exit $?
>>     

> What does this command exit with now?  It used to be that
>
> 	$ git tag -l no-such-tag-at-all ; echo $?
>
> said "1", I think, because grep did not match.
>