threads / patch / 25626

patchDocumentation: fix misnested -l and --contains in git-tag synopsis

Subject: [PATCH] Documentation: fix misnested -l and --contains in git-tag synopsis

## tl;dr

4 messages between Nov 3, 2010 and Nov 5, 2010. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Philip Jägenstedt· Nov 3, 2010, 11:17 UTC · lore

This has been broken since the --contains option was added in commit 32c35cfb1e9c8523b9d60e5095f1c49ebaef0279

Signed-off-by: Philip Jägenstedt <philip@foolip.org>
---
 Documentation/git-tag.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to Documentation/git-tag.txt +1 −1
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 31c78a8..8ad89d7 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [<pattern>]
+'git tag' [-n[<num>]] -l [<pattern>] [--contains <commit>]
 'git tag' -v <tagname>...
 
 DESCRIPTION
-- 
1.7.1
Junio C Hamano· Nov 4, 2010, 19:37 UTC · re: Philip Jägenstedt · lore

Re: [PATCH] Documentation: fix misnested -l and --contains in git-tag synopsis

Philip Jägenstedt <philip@foolip.org> writes:
Show 19 quoted lines
> This has been broken since the --contains option was added in
> commit 32c35cfb1e9c8523b9d60e5095f1c49ebaef0279
>
> Signed-off-by: Philip Jägenstedt <philip@foolip.org>
> ---
>  Documentation/git-tag.txt |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 31c78a8..8ad89d7 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>  'git tag' [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>]
>  	<tagname> [<commit> | <object>]
>  'git tag' -d <tagname>...
> -'git tag' [-n[<num>]] -l [--contains <commit>] [<pattern>]
> +'git tag' [-n[<num>]] -l [<pattern>] [--contains <commit>]
>  'git tag' -v <tagname>...

I do not think there is any "mis" nor "nesting" here. The -l option can be used to tell the command to operate in "list tags" mode, and in that mode it can use a pattern to limit the output (with or without --contains <commit>). All of these are supported:

    git tag -l --contains v1.7.0 v\*
    git tag -l v\*
    git tag --contains v1.7.0 -l v\*
    git tag -l v\* --contains v1.7.0

IOW, <pattern> is _not_ an optional argument to the -l option. In fact, I think we should support more than one patterns, even though currently this seems to silently ignore k\*:

    git tag --contains v1.7.0 -l v\* k\*

As I tend to consider that "limit by pattern" is like pathspecs (in this case, it is _not_ a pathspec, though), having the pattern at the end looks the most natural, but it may be just me. And that is why I think we ought to accept and apply more than one patterns here.

I would further say that "git tag -l <pattern> --contains <commit>" should be an error, even though the command currently seems to take it; perhaps parameter reordering is in effect? I didn't look closely enough.

Philip Jägenstedt· Nov 5, 2010, 09:28 UTC · re: Junio C Hamano · lore

Re: [PATCH] Documentation: fix misnested -l and --contains in git-tag synopsis

On Thu, Nov 4, 2010 at 20:37, Junio C Hamano <gitster@pobox.com> wrote:
Show 47 quoted lines
> Philip Jägenstedt <philip@foolip.org> writes:
>
>> This has been broken since the --contains option was added in
>> commit 32c35cfb1e9c8523b9d60e5095f1c49ebaef0279
>>
>> Signed-off-by: Philip Jägenstedt <philip@foolip.org>
>> ---
>>  Documentation/git-tag.txt |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
>> index 31c78a8..8ad89d7 100644
>> --- a/Documentation/git-tag.txt
>> +++ b/Documentation/git-tag.txt
>> @@ -12,7 +12,7 @@ SYNOPSIS
>>  'git tag' [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>]
>>       <tagname> [<commit> | <object>]
>>  'git tag' -d <tagname>...
>> -'git tag' [-n[<num>]] -l [--contains <commit>] [<pattern>]
>> +'git tag' [-n[<num>]] -l [<pattern>] [--contains <commit>]
>>  'git tag' -v <tagname>...
>
> I do not think there is any "mis" nor "nesting" here.  The -l option can
> be used to tell the command to operate in "list tags" mode, and in that
> mode it can use a pattern to limit the output (with or without --contains
> <commit>).  All of these are supported:
>
>    git tag -l --contains v1.7.0 v\*
>    git tag -l v\*
>    git tag --contains v1.7.0 -l v\*
>    git tag -l v\* --contains v1.7.0
>
> IOW, <pattern> is _not_ an optional argument to the -l option.  In fact, I
> think we should support more than one patterns, even though currently this
> seems to silently ignore k\*:
>
>    git tag --contains v1.7.0 -l v\* k\*
>
> As I tend to consider that "limit by pattern" is like pathspecs (in this
> case, it is _not_ a pathspec, though), having the pattern at the end looks
> the most natural, but it may be just me.  And that is why I think we ought
> to accept and apply more than one patterns here.
>
> I would further say that "git tag -l <pattern> --contains <commit>" should
> be an error, even though the command currently seems to take it; perhaps
> parameter reordering is in effect?  I didn't look closely enough.
>

From the documentation it's not clear that -l and --contains are related at all, they look (and behave?) like orthogonal options. If they do in fact interact in some non-obvious way or if -l <pattern> --contains <commit> means something different from -l --contains <commit> <pattern>, it would be great if that were made more clear. (I wouldn't have expected the later to work at all, thus my assumption that there was a typo in the documentation.)

-- 
Philip Jägenstedt
Junio C Hamano· Nov 5, 2010, 16:51 UTC · re: Philip Jägenstedt · lore

Re: [PATCH] Documentation: fix misnested -l and --contains in git-tag synopsis

Philip Jägenstedt <philip@foolip.org> writes:
> ... they do in fact interact in some non-obvious way or if -l <pattern>
> --contains <commit> means something different from -l --contains
> <commit> <pattern>, it would be great if that were made more clear.

There is no difference; at least there shouldn't be any, and if there are, you found a bug in the implementation.

Let's try to see if I can explain it better.
"git tag" command operates in four modes:
 - "Creating" is the default mode.
 - "Verifying" is triggered with the -v option.
 - "Deleting" is triggered with the -d option.
 - "Listing" is triggered with the -l option.

In "Listing" mode, you can limit which ones are shown by adding <pattern> at the end [*1*]; "--contains <commit>" can also be used to limit. You can change how tags are shown by "-n <lines>" option.

Absense of -v, -d, or -l options does not automatically mean "Creating" mode; when it is clear from other options that "Listing" is requested, we do so without "-l". When you have "--contains <commit>", for example, it is obvious that you want a listing. The same if you have "-n1".

[Footnote]

*1* I think allowing <pattern> anywhere on the command line but at the end is actually a bug.

Even though it can be argued that it does not _hurt_ not to refuse such a command line (i.e. the command would do something useful), it appears to me that it did hurt in this particular case by giving a false impression that somehow "-l" takes an optional <pattern> argument by allowing other options after "-l <pattern>" is given.

Arguably, it is the same kind of "usability measure" as our dwimming into listing mode without "-l" when we can infer the intent of the user from other options, but I think we sometimes overdo usability measures to the point that they negatively affect the learning curve of users.

I also think allowing only one <pattern> is a bug. The command should filter the tags by union of <pattern>s, just like how pathspec works.

← back to recent threads