All of lore.kernel.org
 help / color / mirror / Atom feed
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>,
	"Karthik Nayak" <karthik.188@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 11/16] tag: implicitly supply --list given another list-like option
Date: Tue, 21 Mar 2017 12:58:56 +0000	[thread overview]
Message-ID: <20170321125901.10652-12-avarab@gmail.com> (raw)
In-Reply-To: <20170321125901.10652-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 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,. 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 tests to ensure that e.g. we don't toggle the
list mode 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 |  8 ++++++--
 builtin/tag.c             | 17 +++++++++++------
 t/t7004-tag.sh            | 39 +++++++++++++++++++++++++++++++++++----
 3 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index d534c57156..b9dffc4128 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,7 +128,7 @@ 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`.
 
 --merged [<commit>]::
 	Only list tags whose commits are reachable from the specified
@@ -135,7 +139,7 @@ This option is only applicable when listing tags without annotation lines.
 	commit (`HEAD` if not specified), incompatible with `--merged`.
 
 --points-at <object>::
-	Only list tags of the given object.
+	Only list tags of the given object. Implies `--list`.
 
 -m <msg>::
 --message=<msg>::
diff --git a/builtin/tag.c b/builtin/tag.c
index ad29be6923..ec987ae3c7 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -454,8 +454,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	}
 	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
 
-	if (argc == 0 && !cmdmode)
-		cmdmode = 'l';
+	if (!cmdmode) {
+		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 != 0))
 		usage_with_options(git_tag_usage, options);
@@ -485,13 +490,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		return ret;
 	}
 	if (filter.lines != -1)
-		die(_("-n option is only allowed with -l."));
+		die(_("-n option is only allowed in list mode"));
 	if (filter.with_commit)
-		die(_("--contains option is only allowed with -l."));
+		die(_("--contains option is only allowed in list mode"));
 	if (filter.points_at.nr)
-		die(_("--points-at option is only allowed with -l."));
+		die(_("--points-at option is only allowed in list mode"));
 	if (filter.merge_commit)
-		die(_("--merged and --no-merged option are only allowed with -l"));
+		die(_("--merged and --no-merged options are only allowed in list mode"));
 	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 1de7185be0..dd793e43aa 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,6 +1496,7 @@ 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 &&
@@ -1498,10 +1504,25 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
 	test_must_fail git tag --contains tag-blob
 '
 
+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' '
@@ -1778,8 +1799,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 is incompatible with --no-merged' '
@@ -1803,6 +1829,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


  parent reply	other threads:[~2017-03-21 13:01 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21 12:58 [PATCH v2 00/16] Various changes to the "tag" command & related Ævar Arnfjörð Bjarmason
2017-03-21 12:58 ` [PATCH v2 01/16] tag doc: move the description of --[no-]merged earlier Ævar Arnfjörð Bjarmason
2017-03-21 18:22   ` Junio C Hamano
2017-03-21 12:58 ` [PATCH v2 02/16] tag doc: split up the --[no-]merged documentation Ævar Arnfjörð Bjarmason
2017-03-21 18:26   ` Junio C Hamano
2017-03-21 12:58 ` [PATCH v2 03/16] tag doc: reword --[no-]merged to talk about commits, not tips Ævar Arnfjörð Bjarmason
2017-03-21 12:58 ` [PATCH v2 04/16] ref-filter: make combining --merged & --no-merged an error Ævar Arnfjörð Bjarmason
2017-03-21 12:58 ` [PATCH v2 05/16] ref-filter: add test for --contains on a non-commit Ævar Arnfjörð Bjarmason
2017-03-21 18:29   ` Junio C Hamano
2017-03-21 12:58 ` [PATCH v2 06/16] tag: remove a TODO item from the test suite Ævar Arnfjörð Bjarmason
2017-03-21 12:58 ` [PATCH v2 07/16] tag tests: fix a typo in a test description Ævar Arnfjörð Bjarmason
2017-03-21 12:58 ` [PATCH v2 08/16] for-each-ref: partly change <object> to <commit> in help Ævar Arnfjörð Bjarmason
2017-03-21 18:32   ` Junio C Hamano
2017-03-21 18:50     ` Ævar Arnfjörð Bjarmason
2017-03-21 19:16       ` Junio C Hamano
2017-03-21 12:58 ` [PATCH v2 09/16] tag: add more incompatibles mode tests Ævar Arnfjörð Bjarmason
2017-03-21 18:32   ` Junio C Hamano
2017-03-21 18:58     ` Ævar Arnfjörð Bjarmason
2017-03-21 12:58 ` [PATCH v2 10/16] tag: change misleading --list <pattern> documentation Ævar Arnfjörð Bjarmason
2017-03-21 18:45   ` Junio C Hamano
2017-03-22 19:32     ` Ævar Arnfjörð Bjarmason
2017-03-22 21:09       ` Junio C Hamano
2017-03-22 22:08         ` Ævar Arnfjörð Bjarmason
2017-03-22 22:26           ` Junio C Hamano
2017-03-22 22:36             ` Jeff King
2017-03-22 23:43               ` Ævar Arnfjörð Bjarmason
2017-03-23  0:46                 ` Junio C Hamano
2017-03-22 21:15       ` Junio C Hamano
2017-03-21 12:58 ` Ævar Arnfjörð Bjarmason [this message]
2017-03-21 18:48   ` [PATCH v2 11/16] tag: implicitly supply --list given another list-like option Junio C Hamano
2017-03-21 12:58 ` [PATCH v2 12/16] tag: change --point-at to default to HEAD Ævar Arnfjörð Bjarmason
2017-03-21 18:48   ` Junio C Hamano
2017-03-21 12:58 ` [PATCH v2 13/16] ref-filter: add --no-contains option to tag/branch/for-each-ref Ævar Arnfjörð Bjarmason
2017-03-21 18:51   ` Junio C Hamano
2017-03-21 19:03     ` Ævar Arnfjörð Bjarmason
2017-03-21 12:58 ` [PATCH v2 14/16] ref-filter: reflow recently changed branch/tag/for-each-ref docs Ævar Arnfjörð Bjarmason
2017-03-21 18:53   ` Junio C Hamano
2017-03-21 19:12     ` Ævar Arnfjörð Bjarmason
2017-03-21 12:59 ` [PATCH v2 15/16] tag: implicitly supply --list given the -n option Ævar Arnfjörð Bjarmason
2017-03-21 18:59   ` Junio C Hamano
2017-03-21 19:11     ` Ævar Arnfjörð Bjarmason
2017-03-21 19:24       ` Junio C Hamano
2017-03-21 19:33         ` Ævar Arnfjörð Bjarmason
2017-03-21 12:59 ` [PATCH v2 16/16] tag: add tests for --with and --without Ævar Arnfjörð Bjarmason
2017-03-21 18:22 ` [PATCH v2 00/16] Various changes to the "tag" command & related 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=20170321125901.10652-12-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=karthik.188@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.