From: Junio C Hamano <gitster@pobox•com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail•com>
Cc: git@vger•kernel.org, Lars Hjemli <hjemli@gmail•com>,
Jeff King <peff@peff•net>,
Christian Couder <christian.couder@gmail•com>,
Carlos Rica <jasampler@gmail•com>,
Samuel Tardieu <sam@rfc1149•net>,
Tom Grennan <tmgrennan@gmail•com>,
Karthik Nayak <karthik.188@gmail•com>
Subject: Re: [PATCH v3 12/16] tag: implicitly supply --list given another list-like option
Date: Thu, 23 Mar 2017 10:29:39 -0700 [thread overview]
Message-ID: <xmqqlgrvewak.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170323130529.11361-13-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Thu, 23 Mar 2017 13:05:25 +0000")
Æ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?
> @@ -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.
next prev parent reply other threads:[~2017-03-23 17:29 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-23 13:05 [PATCH v3 00/16] Various changes to the "tag" command & related Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 01/16] tag doc: move the description of --[no-]merged earlier Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 02/16] tag doc: split up the --[no-]merged documentation Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 03/16] tag doc: reword --[no-]merged to talk about commits, not tips Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 04/16] ref-filter: make combining --merged & --no-merged an error Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 05/16] ref-filter: add test for --contains on a non-commit Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 06/16] tag: remove a TODO item from the test suite Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 07/16] tag tests: fix a typo in a test description Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 08/16] for-each-ref: partly change <object> to <commit> in help Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 09/16] tag: add more incompatibles mode tests Ævar Arnfjörð Bjarmason
2017-03-23 17:13 ` Junio C Hamano
2017-03-23 20:37 ` Ævar Arnfjörð Bjarmason
2017-03-23 21:36 ` Junio C Hamano
2017-03-23 22:13 ` Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 10/16] parse-options: add OPT_NONEG to the "contains" option Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 11/16] tag: change misleading --list <pattern> documentation Ævar Arnfjörð Bjarmason
2017-03-23 17:16 ` Junio C Hamano
2017-03-23 13:05 ` [PATCH v3 12/16] tag: implicitly supply --list given another list-like option Ævar Arnfjörð Bjarmason
2017-03-23 17:29 ` Junio C Hamano [this message]
2017-03-23 20:32 ` Ævar Arnfjörð Bjarmason
2017-03-23 21:33 ` Junio C Hamano
2017-03-23 13:05 ` [PATCH v3 13/16] tag: change --point-at to default to HEAD Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 14/16] ref-filter: add --no-contains option to tag/branch/for-each-ref Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 15/16] ref-filter: reflow recently changed branch/tag/for-each-ref docs Ævar Arnfjörð Bjarmason
2017-03-23 13:05 ` [PATCH v3 16/16] tag: add tests for --with and --without Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqlgrvewak.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=avarab@gmail$(echo .)com \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=hjemli@gmail$(echo .)com \
--cc=jasampler@gmail$(echo .)com \
--cc=karthik.188@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=sam@rfc1149$(echo .)net \
--cc=tmgrennan@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox