git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 23, 2017, 20:32 UTC
Message-ID
<CACBZZX6HnUZSV49c2uRqW3u-+Tc-5LtaaEG=Yype1155qR0ifw@mail.gmail.com>
In-Reply-To
<xmqqlgrvewak.fsf@gitster.mtv.corp.google.com>
On Thu, Mar 23, 2017 at 6:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 54 quoted lines
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> 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".

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

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.

Show 5 quoted lines
>>       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.
Previous: Junio C HamanoNext: Junio C Hamano
Message 15 of 25 in “Various changes to the "tag" command & related”
  1. 00/16 Various changes to the "tag" command & relatedÆvar Arnfjörð Bjarmason, Mar 23, 2017
  2. 01/16 tag doc: move the description of --[no-]merged earlierÆvar Arnfjörð Bjarmason, Mar 23, 2017
  3. 02/16 tag doc: split up the --[no-]merged documentationÆvar Arnfjörð Bjarmason, Mar 23, 2017
  4. 03/16 tag doc: reword --[no-]merged to talk about commits, not tipsÆvar Arnfjörð Bjarmason, Mar 23, 2017
  5. 04/16 ref-filter: make combining --merged & --no-merged an errorÆvar Arnfjörð Bjarmason, Mar 23, 2017
  6. 05/16 ref-filter: add test for --contains on a non-commitÆvar Arnfjörð Bjarmason, Mar 23, 2017
  7. 06/16 tag: remove a TODO item from the test suiteÆvar Arnfjörð Bjarmason, Mar 23, 2017
  8. 08/16 for-each-ref: partly change <object> to <commit> in helpÆvar Arnfjörð Bjarmason, Mar 23, 2017
  9. 07/16 tag tests: fix a typo in a test descriptionÆvar Arnfjörð Bjarmason, Mar 23, 2017
  10. 11/16 tag: change misleading --list <pattern> documentationÆvar Arnfjörð Bjarmason, Mar 23, 2017
  11. Junio C HamanoMar 23, 2017
  12. 13/16 tag: change --point-at to default to HEADÆvar Arnfjörð Bjarmason, Mar 23, 2017
  13. 12/16 tag: implicitly supply --list given another list-like optionÆvar Arnfjörð Bjarmason, Mar 23, 2017
  14. Junio C HamanoMar 23, 2017
  15. Ævar Arnfjörð BjarmasonMar 23, 2017
  16. Junio C HamanoMar 23, 2017
  17. 14/16 ref-filter: add --no-contains option to tag/branch/for-each-refÆvar Arnfjörð Bjarmason, Mar 23, 2017
  18. 15/16 ref-filter: reflow recently changed branch/tag/for-each-ref docsÆvar Arnfjörð Bjarmason, Mar 23, 2017
  19. 10/16 parse-options: add OPT_NONEG to the "contains" optionÆvar Arnfjörð Bjarmason, Mar 23, 2017
  20. 16/16 tag: add tests for --with and --withoutÆvar Arnfjörð Bjarmason, Mar 23, 2017
  21. 09/16 tag: add more incompatibles mode testsÆvar Arnfjörð Bjarmason, Mar 23, 2017
  22. Junio C HamanoMar 23, 2017
  23. Ævar Arnfjörð BjarmasonMar 23, 2017
  24. Junio C HamanoMar 23, 2017
  25. Ævar Arnfjörð BjarmasonMar 23, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.