Re: [PATCH] Add option -L to git-tag.
- From
- Matthijs Melchior <mmelchior@xs4all.nl>
- Date
- Jun 3, 2007, 00:04 UTC
- Message-ID
- <f3t0hn$siq$1@sea.gmane.org>
- In-Reply-To
- <7vvee6qkr4.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
Show 40 quoted lines
> Matthijs Melchior <mmelchior@xs4all.nl> writes:
>
>> Junio C Hamano wrote:
>> ...
>>> I dunno; I've never been very good at the user interfaces.
>>>
>> Yes, since I have this command and have seen the tag annotations in the
>> git repository, I think we need this extra parameter.
>> I propose to give the number of lines you want to see, with 0 gives all.
>> So it will be --pretty=<max-number-lines> to limit the output and be
>> able to find interesting stuff before looking at the complete message.
>
> Please do not use the same word used elsewhere ('pretty') and
> make it mean something different (they are 'short', 'oneline', etc.
> in other places).
>
> Regardless of how we might do a single-liner output, I suspect that
> Instead of showing them like this (your patch):
>
> $ git tag -L v2.6.1* | head -n 6
> v2.6.11
> This is the 2.6.11 tree object.
> v2.6.11-tree
> This is the 2.6.11 tree object.
> v2.6.12
> This is the final 2.6.12 release
>
> showing them in this way might be more pleasant:
>
> $ git tag -L v2.6.1* | head -n 3
> v2.6.11 This is the 2.6.11 tree object.
> v2.6.11-tree This is the 2.6.11 tree object.
> v2.6.12 This is the final 2.6.12 release
>
> This matches the way "git branch -v" without other arguments,
> which I think is the moral equivalent for branches to your "git
> tag -L", shows a bit more information than the usual (we could
> even say "git tag -l -v" but -v is already taken -- we could
> still do "git tag --list --verbose" and leave the short '-v' to
> mean 'verify' but I dunno).Yes, this is a good idea. I have dropped the -L option, and in stead introduced the -n option. The -n option specifies how many lines of annotation you want to see. Not using -n gives the old -l behavior, just -n gives the tag and first annotation line together, and -n 9999 gives the full annotation.
I think this is better than having two different list options.
Show 15 quoted lines
> > This is a slightly related tangent, but I've been wanting to > extend the "the first line is special 'one-line summary', > separated by a blank line from the rest of the more descriptive > message" convention used in the commit log message formatter. > When somebody asks for --pretty=oneline, instead of showing the > "first line", we would give the first paragraph, with LFs > replaced with SPs to make it a single line. This would not > affect commit log messages that follow the above convention. > > If your tags have a few lines to describe what the commits are > about, it might make it easier to get the overview by applying > the same "first paragraph squashed down to a single line" logic, > grab the first paragraph, present it as a one-liner" in the > format shown above.
I will leave this for another time...
Show 12 quoted lines
> >>>> - 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). >>> >> This accident can be repaired by documenting it.... :) > > That would cast the implementation in stone, avoidance of which > was the point of my comment. >
Yes, I understand. It would be good if the manual page for rev-parse said something about the order in which the arguments are processed or generated. Currently this is not mentioned, so I was not surprised to find the generated names were sorted.
Show 55 quoted lines
>>> 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. >>> >> It will always exit 0, either from sed or git-cat. >> >> ... >> >> (maybe the exit code is not worth the added complexity...). > > That's 40% satisfactory answer. > > I do not speak for others, but when I comment on a patch, saying > "This might be better done this other way", or "This change > might be bad", I do not necessarily expect/want you to agree > with me on all counts. I would very much be happier to get a > counter argument back -- that's how we both learn things and > make progress. > > Unlike Linus, I am not always right ;-) > > But more seriously, I sometimes deliberately make suggestions > that I know are not optimal, because I want to involve other > people (not necessarily the author of the patch, but others on > the list) in the process of making improvements. > > The "40%" satisfactory part comes from that you correctly > answered that your version now always exits zero while the > original diagnosed the "no such tag whatsoever" situation with > non-zero exit, with a slight hint that you think it might be > better not to differenciate the "no match" case. > > What I would prefer to see is to make that "slight hint" more > explicit. As you say, "is not worth the added complexity" is a > possible justification, but in this particular case, I think we > could (and probably should) even argue that the current exit > code is not so useful. It might go like this... > > Although "git tag -l <pattern>" currently signals non-match > with its exit code, "git tag -l do-i-have-this-tag" is not > the right way to ask that question to begin with, because > the tagname parameter is always taken as a pattern. For > that, "git show-ref --verify refs/tags/do-i-have-this-tag" > is much better. I do not think the current exit status from > "git-tag -l <pattern>" is useful, and we should change it to > exit with 0 unless we see other errors (e.g. "not a git > repository"), regardless of the addition of -L option. > > and you would have got the remaining 60% ;-). > > I care about documenting the change in behaviour and justifying > why we changed the behavikour in the commit log. >
Take 3 of the patch is forthcoming. - A new -n option is introduced, specifying the number of annotation lines to print. The default value is 0. - The <pattern> is now a shell pattern, and not a list if grep parameters. It is called a pattern and not re, after all. - The -l <pattern> may be repeated with a different <pattern> - The exit code for -l is now always 0.
Thanks.
Regards, Matthijs Melchior.