From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"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>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 4/8] tag: Implicitly supply --list given another list-like option
Date: Sat, 18 Mar 2017 10:32:52 +0000 [thread overview]
Message-ID: <20170318103256.27141-5-avarab@gmail.com> (raw)
In-Reply-To: <20170318103256.27141-1-avarab@gmail.com>
Change the "tag" command to implicitly turn on its --list mode when
provided with a list-like option such as --contains, --points-at etc.
This is for consistency with how "branch" works. When "branch" is
given a list-like option, such as --contains, it implicitly provides
--list. Before this change "tag" would error out on those sorts of
invocations. I.e. while both of these worked for "branch":
git branch --contains v2.8.0 <pattern>
git branch --list --contains v2.8.0 <pattern>
Only the latter form worked for "tag":
git tag --contains v2.8.0 '*rc*'
git tag --list --contains v2.8.0 '*rc*'
Now "tag", like "branch" will implicitly supply --list in when a
list-like option is provided, and no other conflicting non-list
options (such as -d) are present on the command-line.
Spelunking through the history via:
git log --reverse -p -G'only allowed with' -- '*builtin*tag*c'
Reveals that there was no good reason for not allowing this in the
first place. The --contains option added in 32c35cfb1e ("git-tag: Add
--contains option", 2009-01-26) made this an error, and all the other
subsequent list-like options that were added copied its pattern of
making this usage an error.
The only tests that break as a result of this change are tests that
were explicitly checking that this "branch-like" usage wasn't
permitted. Change those failing tests to check that this invocation
mode is permitted, add extra tests for the list-like options we
weren't testing, and add tests to ensure that e.g. we don't toggle the
--list in the presence of other conflicting non-list options.
With this change errors messages such as "--contains option is only
allowed with -l" don't make sense anymore, since options like
--contain turn on -l. Instead we error out when list-like options such
as --contain are used in conjunction with conflicting options such as
-d or -v.
This change does not consider "-n" a list-like option, even though
that might be logical. Permitting it would allow:
git tag -n 100
As a synonym for:
git tag -n --list 100
Which, while not technically ambiguous as the option must already be
provided as -n<num> rather than -n <num>, would be confusing.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Documentation/git-tag.txt | 10 +++++++---
builtin/tag.c | 25 +++++++++++++++----------
t/t7004-tag.sh | 39 +++++++++++++++++++++++++++++++++++----
3 files changed, 57 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 848e8c1b73..2acd3b6beb 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -96,6 +96,10 @@ OPTIONS
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.
++
+This option is implicitly supplied if any other list-like option such
+as `--contains` is provided. See the documentation for each of those
+options for details.
--sort=<key>::
Sort based on the key given. Prefix `-` to sort in
@@ -124,10 +128,10 @@ This option is only applicable when listing tags without annotation lines.
--contains [<commit>]::
Only list tags which contain the specified commit (HEAD if not
- specified).
+ specified). Implies `--list`.
--points-at <object>::
- Only list tags of the given object.
+ Only list tags of the given object. Implies `--list`.
-m <msg>::
--message=<msg>::
@@ -178,7 +182,7 @@ This option is only applicable when listing tags without annotation lines.
--[no-]merged [<commit>]::
Only list tags whose tips are reachable, or not reachable
if `--no-merged` is used, from the specified commit (`HEAD`
- if not specified).
+ if not specified). Implies `--list`.
CONFIGURATION
-------------
diff --git a/builtin/tag.c b/builtin/tag.c
index 0bba3fd070..3483636e59 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -454,8 +454,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
}
create_tag_object = (opt.sign || annotate || msg.given || msgfile);
- if (argc == 0 && !cmdmode && !create_tag_object)
- cmdmode = 'l';
+ if (!cmdmode && !create_tag_object) {
+ if (argc == 0)
+ cmdmode = 'l';
+ else if (filter.with_commit || filter.points_at.nr || filter.merge_commit)
+ cmdmode = 'l';
+ }
if ((create_tag_object || force) && (cmdmode || (!cmdmode && !argc)))
usage_with_options(git_tag_usage, options);
@@ -483,15 +487,16 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (column_active(colopts))
stop_column_filter();
return ret;
+ } else {
+ if (filter.lines != -1)
+ die(_("-n option is only allowed in list mode."));
+ if (filter.with_commit)
+ die(_("--contains option is only allowed in list mode."));
+ if (filter.points_at.nr)
+ die(_("--points-at option is only allowed in list mode."));
+ if (filter.merge_commit)
+ die(_("--merged and --no-merged options are only allowed in list mode."));
}
- if (filter.lines != -1)
- die(_("-n option is only allowed with -l."));
- if (filter.with_commit)
- die(_("--contains option is only allowed with -l."));
- if (filter.points_at.nr)
- die(_("--points-at option is only allowed with -l."));
- if (filter.merge_commit)
- die(_("--merged and --no-merged option are only allowed with -l"));
if (cmdmode == 'd')
return for_each_tag_name(argv, delete_tag, NULL);
if (cmdmode == 'v') {
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index d36cd51fe2..5c94932f0f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1472,6 +1472,11 @@ test_expect_success 'checking that initial commit is in all tags' "
test_cmp expected actual
"
+test_expect_success 'checking that --contains can be used in non-list mode' '
+ git tag --contains $hash1 v* >actual &&
+ test_cmp expected actual
+'
+
# mixing modes and options:
test_expect_success 'mixing incompatibles modes and options is forbidden' '
@@ -1491,15 +1496,31 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
test_must_fail git tag -a -s -m -F -l &&
test_must_fail git tag -l -v &&
test_must_fail git tag -n 100 &&
+ 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 &&
test_must_fail git tag -v -s
'
+for option in --contains --merged --no-merged --points-at
+do
+ test_expect_success "mixing incompatible modes with $option is forbidden" "
+ test_must_fail git tag -d $option HEAD &&
+ test_must_fail git tag -d $option HEAD some-tag &&
+ test_must_fail git tag -v $option HEAD
+ "
+ test_expect_success "Doing 'git tag --list-like $option <commit> <pattern> is permitted" "
+ git tag -n $option HEAD HEAD &&
+ git tag $option HEAD HEAD
+ "
+done
+
# check points-at
-test_expect_success '--points-at cannot be used in non-list mode' '
- test_must_fail git tag --points-at=v4.0 foo
+test_expect_success '--points-at can be used in non-list mode' '
+ echo v4.0 >expect &&
+ git tag --points-at=v4.0 "v*" >actual &&
+ test_cmp expect actual
'
test_expect_success '--points-at finds lightweight tags' '
@@ -1776,8 +1797,13 @@ test_expect_success 'setup --merged test tags' '
git tag mergetest-3 HEAD
'
-test_expect_success '--merged cannot be used in non-list mode' '
- test_must_fail git tag --merged=mergetest-2 foo
+test_expect_success '--merged can be used in non-list mode' '
+ cat >expect <<-\EOF &&
+ mergetest-1
+ mergetest-2
+ EOF
+ git tag --merged=mergetest-2 "mergetest*" >actual &&
+ test_cmp expect actual
'
test_expect_success '--merged shows merged tags' '
@@ -1797,6 +1823,11 @@ test_expect_success '--no-merged show unmerged tags' '
test_cmp expect actual
'
+test_expect_success '--no-merged can be used in non-list mode' '
+ git tag --no-merged=mergetest-2 mergetest-* >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'ambiguous branch/tags not marked' '
git tag ambiguous &&
git branch ambiguous &&
--
2.11.0
next prev parent reply other threads:[~2017-03-18 10:34 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-18 10:32 [PATCH 0/8] Various changes to the "tag" command Ævar Arnfjörð Bjarmason
2017-03-18 10:32 ` [PATCH 1/8] tag: Remove a TODO item from the test suite Ævar Arnfjörð Bjarmason
2017-03-18 18:14 ` Junio C Hamano
2017-03-18 18:42 ` [PATCH 0/2] doc/SubmittingPatches: A couple of minor improvements Ævar Arnfjörð Bjarmason
2017-03-18 18:42 ` [PATCH 1/2] doc/SubmittingPatches: clarify the casing convention for "area: change..." Ævar Arnfjörð Bjarmason
2017-03-18 19:04 ` Junio C Hamano
2017-03-18 19:16 ` Ævar Arnfjörð Bjarmason
2017-03-18 20:07 ` Junio C Hamano
2017-03-18 18:42 ` [PATCH 2/2] doc/SubmittingPatches: show how to get a CLI commit summary Ævar Arnfjörð Bjarmason
2017-03-18 19:07 ` Junio C Hamano
2017-03-21 14:21 ` [PATCH v2 0/2] A couple of minor improvements Ævar Arnfjörð Bjarmason
2017-03-21 14:21 ` [PATCH v2 1/2] doc/SubmittingPatches: clarify the casing convention for "area: change..." Ævar Arnfjörð Bjarmason
2017-03-21 14:21 ` [PATCH v2 2/2] doc/SubmittingPatches: show how to get a CLI commit summary Ævar Arnfjörð Bjarmason
2017-03-21 15:51 ` SZEDER Gábor
2017-03-21 17:58 ` Junio C Hamano
2017-03-21 18:47 ` Ævar Arnfjörð Bjarmason
2017-03-21 20:01 ` SZEDER Gábor
2017-03-21 20:13 ` Junio C Hamano
2017-03-19 0:48 ` [PATCH 1/8] tag: Remove a TODO item from the test suite Jakub Narębski
2017-03-19 6:43 ` Ævar Arnfjörð Bjarmason
2017-03-18 10:32 ` [PATCH 2/8] tag: Refactor the options handling code to be less bizarro Ævar Arnfjörð Bjarmason
2017-03-18 18:35 ` Junio C Hamano
2017-03-18 19:13 ` Ævar Arnfjörð Bjarmason
2017-03-18 19:27 ` Junio C Hamano
2017-03-18 20:00 ` Ævar Arnfjörð Bjarmason
2017-03-18 10:32 ` [PATCH 3/8] tag: Change misleading --list <pattern> documentation Ævar Arnfjörð Bjarmason
2017-03-18 18:43 ` Junio C Hamano
2017-03-18 19:49 ` Ævar Arnfjörð Bjarmason
2017-03-20 3:44 ` Jeff King
2017-03-20 15:55 ` Junio C Hamano
2017-03-20 17:07 ` Junio C Hamano
2017-03-20 16:09 ` Ævar Arnfjörð Bjarmason
2017-03-20 16:11 ` Jeff King
2017-03-18 10:32 ` Ævar Arnfjörð Bjarmason [this message]
2017-03-20 3:55 ` [PATCH 4/8] tag: Implicitly supply --list given another list-like option Jeff King
2017-03-20 9:16 ` Ævar Arnfjörð Bjarmason
2017-03-18 10:32 ` [PATCH 5/8] tag: Implicitly supply --list given the -n option Ævar Arnfjörð Bjarmason
2017-03-20 4:02 ` Jeff King
2017-03-18 10:32 ` [PATCH 6/8] ref-filter: Add --no-contains option to tag/branch/for-each-ref Ævar Arnfjörð Bjarmason
2017-03-20 4:25 ` Jeff King
2017-03-20 9:32 ` Ævar Arnfjörð Bjarmason
2017-03-20 19:52 ` Jeff King
2017-03-20 20:04 ` Ævar Arnfjörð Bjarmason
2017-03-18 10:32 ` [PATCH 7/8] tag: Add tests for --with and --without Ævar Arnfjörð Bjarmason
2017-03-18 10:32 ` [PATCH 8/8] tag: Change --point-at to default to HEAD Ævar Arnfjörð Bjarmason
2017-03-18 18:54 ` Junio C Hamano
2017-03-18 19:52 ` Ævar Arnfjörð Bjarmason
2017-03-20 4:26 ` [PATCH 0/8] Various changes to the "tag" command Jeff King
2017-03-20 15:57 ` Junio C Hamano
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=20170318103256.27141-5-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=hjemli@gmail.com \
--cc=jasampler@gmail.com \
--cc=peff@peff.net \
--cc=sam@rfc1149.net \
--cc=tmgrennan@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).