Re: [PATCH v3 12/16] tag: implicitly supply --list given another list-like option

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 23, 2017, 17:29 UTC
Message-ID: <xmqqlgrvewak.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170323130529.11361-13-avarab@gmail.com>

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

Show 17 quoted lines

> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 2640c6955c..491dba065c 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -82,10 +82,11 @@ OPTIONS
>  
>  -n<num>::
>  	<num> specifies how many lines from the annotation, if any,
> -	are printed when using -l.
> -	The default is not to print any annotation lines.
> -	If no number is given to `-n`, only the first line is printed.
> -	If the tag is not annotated, the commit message is displayed instead.
> +	are printed when using -l. Implies `--list`.
> ++
> +The default is not to print any annotation lines.
> +If no number is given to `-n`, only the first line is printed.
> +If the tag is not annotated, the commit message is displayed instead.

I thought we would want to have -n thing separate as we are much less sure than the others, but let's forget about that and commit to default to -l with -n.

Given the confusion I had during the review of the previous round, I would think this should clarify what it means "If no number is given to -n" a bit more strongly. Namely, the behaviour we see in this test:

> +test_expect_success 'The -n 100 invocation means -n --list 100, not -n100' '

can be a common mistake and needs to be warned about.

I'd drop "The default is not to print any annotation lines". It is not just unnecessary (we make a specific mention about the default when we act as if an option were given even if the user doesn't explicitly give it, and not triggering a special feature when it is not asked for _is_ the default everywhere else), but is confusing, because it is unclear if it is talking about the default case of not giving any -n<num> option, or the default case of not giving <num> but still the -n option.

Perhaps something along the lines of ...

-n<num>::
	Show the message of the annotated tag when using the
	`--list` mode.  A number <num> can be directly attached to
	the `-n` option (e.g. `-n100`; not `-n 100` as separate
	arguments) to specify how many lines of the message to show.
	Without <num>, only the first line of the message is shown.
	For an unannotated tag that points at a commit, the commit
	message is used instead.

... may be less confusing?

Show 5 quoted lines

> @@ -1496,7 +1517,6 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
>  	test_must_fail git tag -l -v &&
>  	test_must_fail git tag -l -d &&
>  	test_must_fail git tag -l -v -d &&
> -	test_must_fail git tag -n 100 &&

Hmph, wouldn't we want to instead replace it with

	... something to set expectation ... >expect &&
	git tag -n 100 >actual &&
	test_cmp expect actual &&

here?

>  	test_must_fail git tag -n 100 -v &&
>  	test_must_fail git tag -l -m msg &&
>  	test_must_fail git tag -l -F some file &&

Thanks.