Web lists-archives.com

[PATCH] tag: Implicitly supply --list given another list-like option




Change these invocations which currently error out without the -l, to
behave as if though -l was provided:

    git tag -l [--contains|--points-at|--[no-]merged] <commit>

I ran into what turned out to be not-a-bug in "branch" where it,
unlike "tag" before this patch, accepts input like:

    git branch --contains v2.8.0 <pattern>

Jeff King pointed out in
<20170310124247.jvrmmcz2pbv4qf3o@xxxxxxxxxxxxxxxxxxxxx> in reply to
that::

    The difference between "branch" and "tag" here is that "branch
    --contains" implies "--list" (and the argument becomes a pattern).
    Whereas "tag --contains" just detects the situation and complains.

    If anything, I'd think we would consider teaching "tag" to behave
    more like "branch".

I agree. This change does that, the only tests that broke as a result
of this were tests that were explicitly checking that this
"branch-like" usage wasn't permitted, i.e. no actual breakages
occurred, and I can't imagine an invocation that would negatively
impact backwards compatibility, i.e. these invocations all just
errored out before.

Spelunking through the history via:

    git log --reverse -p -G'only allowed with' -- '*builtin*tag*c'

Reveals that there was no good reason for this in the first place. The
--contains option added in v1.6.1.1-243-g32c35cfb1e made this an
error, and all the other subsequent options I'm changing here just
copy/pasted its pattern.

I've changed the failing tests to check that this invocation mode is
permitted instead, and added extra tests for the list-like options we
weren't testing.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---

Junio: This will merge conflict with my in-flight --no-contains
patch. I can re-send either one depending on which you want to accept
first, this patch will need an additional test for --no-contains. I
just wanted to get this on the ML for review before the --no-contains
patch hit "master".

 Documentation/git-tag.txt |  3 +++
 builtin/tag.c             | 12 ++++++------
 t/t7004-tag.sh            | 25 +++++++++++++++++++++----
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 525737a5d8..c80d9e11ba 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -94,6 +94,9 @@ OPTIONS
 	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.
++
+We supply this option implicitly if any other list-like option is
+provided. E.g. `--contains`, `--points-at` etc.
 
 --sort=<key>::
 	Sort based on the key given.  Prefix `-` to sort in
diff --git a/builtin/tag.c b/builtin/tag.c
index ad29be6923..6ab65bcf6b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -454,6 +454,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	}
 	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
 
+	/* We implicitly supply --list with --contains, --points-at,
+	   --merged and --no-merged, just like git-branch */
+	if (filter.with_commit || filter.points_at.nr || filter.merge_commit)
+		cmdmode = 'l';
+
+	/* Just plain "git tag" is like "git tag --list" */
 	if (argc == 0 && !cmdmode)
 		cmdmode = 'l';
 
@@ -486,12 +492,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	}
 	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 b4698ab5f5..e0306ee5a8 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1453,6 +1453,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' '
@@ -1466,8 +1471,10 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
 
 # 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' '
@@ -1744,8 +1751,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' '
@@ -1765,6 +1777,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