On Thu, Mar 23, 2017 at 6:29 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> 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? Sounds good, but a topic for another series I think. I'd rather stop adding new things to this already big series, and it's not introducing any new confusion with -n, just making the now confusing "tag -l -n 100" be equivalently written as "tag -n 100". >> @@ -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? Nope, we already have detailed tests for "tag -n 100" and the like and what that output should actually return earlier in the patch, this bit is just removing it from the test for failing invocations. >> 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.