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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 21, 2017, 18:45 UTC
Message-ID: <xmqqo9wupixz.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170321125901.10652-11-avarab@gmail.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.

Show 6 quoted lines

> 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.

Show 6 quoted lines

> 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.

Show 10 quoted lines

> --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?

Show 19 quoted lines

> 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

Show 5 quoted lines

> @@ -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.

Show 11 quoted lines

> +	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 &&