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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 4, 2010, 19:37 UTC
Message-ID: <7vmxpovq0r.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1288783029-11226-1-git-send-email-philip@foolip.org>

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.