Re: [PATCH v2 10/16] tag: change misleading --list <pattern> documentation

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> And after Jeff's change, one that took multiple patterns:
>
>     git tag --list 'v*rc*' --list '*2.8*'

I do not think this is a correct depiction of the evolution, and is
a confusing description.  It should say

    git tag --list 'v*rc*' '*2.8*'

instead.

When the logic was still in scripted Porcelain, <pattern> _was_ an
optional argument to the "-l" option (see b867c7c2 ("git-tag: -l to
list tags (usability).", 2006-02-17) you quoted earlier).  But way
before Jeff's change, when the command was reimplemented in C and
started using parse_options(), <pattern> stopped being an argument
to the "-l" option.  What Jeff's change did was that the original
code that only took

    git tag -l 'v*rc*'

to also take

    git tag -l 'v*rc*' '*2.8*'

i.e. multiple patterns.  Yes, thanks to parse_options, you could
have -l twice on the command line, but "multiple patterns" does not
have anything to do with having to (or allowing to) use '-l'
multiple times.

> But since it's actually a toggle all of these work as well, and
> produce identical output to the last example above:
>
>     git tag --list 'v*rc*' '*2.8*'
>     git tag --list 'v*rc*' '*2.8*' --list --list --list
>     git tag --list 'v*rc*' '*2.8*' --list -l --list -l --list

So citing Jeff's change is irrelevant to explain these.  I doubt you
even need to show these variations.

> Now the documentation is more in tune with how the "branch" command
> describes its --list option since commit cddd127b9a ("branch:
> introduce --list option", 2011-08-28).
>
> Change the test suite to assert that these invocations work for the
> cases that weren't already being tested for.

These two paragraphs are relevant.

> --l <pattern>::
> ---list <pattern>::
> -	List tags with names that match the given pattern (or all if no
> -	pattern is given).  Running "git tag" without arguments also
> -	lists all tags. The pattern is a shell wildcard (i.e., matched
> -	using fnmatch(3)).  Multiple patterns may be given; if any of
> -	them matches, the tag is shown.
> +-l::
> +--list::
> +	Activate the list mode. `git tag <pattern>` would try to create a

Dont say <pattern> on this line.  It is `git tag <name>`.

> +	tag, use `git tag --list <pattern>` to list matching branches, (or

Perhaps <pattern>... instead to signal that you can give multiple
patterns?

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 958c77ab86..1de7185be0 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -118,6 +118,18 @@ test_expect_success 'listing all tags if one exists should succeed' '
>  	git tag
>  '
>  
> +cat >expect <<EOF
> +mytag
> +EOF
> +test_expect_success 'Multiple -l or --list options are equivalent to one -l option' '
> +	git tag -l -l >actual &&
> +	test_cmp expect actual &&
> +	git tag --list --list >actual &&
> +	test_cmp expect actual &&
> +	git tag --list -l --list >actual &&
> +	test_cmp expect actual
> +'

The "-l/-d/-v" options follow the last-one-wins rule, no?  Perhaps
also show how this one works in this test (while retitling it)?

	git tag -d -v -l

> @@ -336,6 +348,15 @@ test_expect_success 'tag -l can accept multiple patterns' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'tag -l can accept multiple patterns interleaved with -l or --list options' '

Please drop "interleaved", as we are not encouraging GNUism.  We
just tolerate it but do not guarantee to keep them working.

> +	git tag -l "v1*" "v0*" >actual &&
> +	test_cmp expect actual &&
> +	git tag -l "v1*" --list "v0*" >actual &&
> +	test_cmp expect actual &&
> +	git tag -l "v1*" "v0*" -l --list >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'listing tags in column' '
>  	COLUMNS=40 git tag -l --column=row >actual &&
>  	cat >expected <<\EOF &&