[PATCH v3 00/16] Various changes to the "tag" command & related

[PATCH v3 00/16] Various changes to the "tag" command & related

[Ævar Arnfjörð Bjarmason]

Hopefully the last version of this. Changes since v2 in
<20170321125901.10652-1-avarab@gmail.com>:

Ævar Arnfjörð Bjarmason (16):
  tag doc: move the description of --[no-]merged earlier
  tag doc: split up the --[no-]merged documentation
  tag doc: reword --[no-]merged to talk about commits, not tips
  ref-filter: make combining --merged & --no-merged an error

No changes.

  ref-filter: add test for --contains on a non-commit

Minor grammar fixes in the commit message, as pointed out on-list.

  tag: remove a TODO item from the test suite
  tag tests: fix a typo in a test description
  for-each-ref: partly change <object> to <commit> in help

No changes.

  tag: add more incompatibles mode tests

Reworded the commit message to be less confusing, as pointed out
on-list & added a few more tests for incompatible model, e.g. modes,
e.g. "-l -v -d".

  parse-options: add OPT_NONEG to the "contains" option

NEW: Junio pointed out that my new --without should have a OPT_NONEG,
which made me notice that --with and --contains have also had the same
bug for ages without anyone noticing, add OPT_NONEG to them all.

  tag: change misleading --list <pattern> documentation

Hopefully solves the sticking point with the "-l <pat> -l <pat>" test
I'm adding, there's now a big fat comment saying this was never
intended behavior but we want to test it blah blah.

Also change `git tag <pattern>` -> `git tag <tagname>`. See also my
just-sent "[PATCH] branch doc: Change `git branch <pattern>` to use
`<branchname>`".

  tag: implicitly supply --list given another list-like option

Minor typo fix in commit message, as pointed out on-list.

Squashed the "tag: implicitly supply --list given the -n option" patch
into this, as discussed on-list.

  tag: change --point-at to default to HEAD
  ref-filter: add --no-contains option to tag/branch/for-each-ref

The new --no-contains & --without options also have OPT_NONEG now.

  ref-filter: reflow recently changed branch/tag/for-each-ref docs

Add missing Signed-off-by.

  tag: add tests for --with and --without

Add a couple of extra tests asserting that --no-with and --no-without
error out.

 Documentation/git-branch.txt           |  33 +++--
 Documentation/git-for-each-ref.txt     |  12 +-
 Documentation/git-tag.txt              |  60 +++++---
 builtin/branch.c                       |   5 +-
 builtin/for-each-ref.c                 |   5 +-
 builtin/tag.c                          |  27 ++--
 contrib/completion/git-completion.bash |   4 +-
 parse-options.h                        |   6 +-
 ref-filter.c                           |  30 +++-
 ref-filter.h                           |   1 +
 t/t3200-branch.sh                      |   4 +
 t/t3201-branch-contains.sh             |  61 +++++++-
 t/t6302-for-each-ref-filter.sh         |  20 +++
 t/t7004-tag.sh                         | 245 +++++++++++++++++++++++++++++++--
 14 files changed, 441 insertions(+), 72 deletions(-)

-- 
2.11.0

[PATCH v3 01/16] tag doc: move the description of --[no-]merged earlier

[Ævar Arnfjörð Bjarmason]

Move the documentation for the --merged & --no-merged options earlier
in the documentation, to sit along the other switches, and right next
to the similar --contains and --points-at switches.

It makes more sense to group the options together, not have some
options after the like of <tagname>, <object>, <format> etc.

This was originally put there when the --merged & --no-merged options
were introduced in 5242860f54 ("tag.c: implement '--merged' and
'--no-merged' options", 2015-09-10). It's not apparent from that
commit that the documentation is being placed apart from other
options, rather than along with them, so this was likely missed in the
initial review.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-tag.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 525737a5d8..33f18ea5fb 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -124,6 +124,11 @@ This option is only applicable when listing tags without annotation lines.
 	Only list tags which contain the specified commit (HEAD if not
 	specified).
 
+--[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).
+
 --points-at <object>::
 	Only list tags of the given object.
 
@@ -173,11 +178,6 @@ This option is only applicable when listing tags without annotation lines.
 	that of linkgit:git-for-each-ref[1].  When unspecified,
 	defaults to `%(refname:strip=2)`.
 
---[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).
-
 CONFIGURATION
 -------------
 By default, 'git tag' in sign-with-default mode (-s) will use your
-- 
2.11.0

[PATCH v3 02/16] tag doc: split up the --[no-]merged documentation

[Ævar Arnfjörð Bjarmason]

Split up the --[no-]merged documentation into documentation that
documents each option independently. This is in line with how "branch"
and "for-each-ref" are documented, and makes subsequent changes to
discuss the limits & caveats of each option easier to read.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-tag.txt | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 33f18ea5fb..68b0ab2410 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -124,10 +124,13 @@ This option is only applicable when listing tags without annotation lines.
 	Only list tags which contain the specified commit (HEAD if not
 	specified).
 
---[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).
+--merged [<commit>]::
+	Only list tags whose tips are reachable from the specified commit
+	(`HEAD` if not specified).
+
+--no-merged [<commit>]::
+	Only list tags whose tips are not reachable from the specified
+	commit (`HEAD` if not specified).
 
 --points-at <object>::
 	Only list tags of the given object.
-- 
2.11.0

[PATCH v3 03/16] tag doc: reword --[no-]merged to talk about commits, not tips

[Ævar Arnfjörð Bjarmason]

Change the wording for the --merged and --no-merged options to talk
about "commits" instead of "tips".

This phrasing was copied from the "branch" documentation in commit
5242860f54 ("tag.c: implement '--merged' and '--no-merged' options",
2015-09-10). Talking about the "tip" is branch nomenclature, not
something usually associated with tags.

This phrasing might lead the reader to believe that these options
might find tags pointing to trees or blobs, let's instead be explicit
and only talk about commits.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-tag.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 68b0ab2410..3abf912782 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -125,11 +125,11 @@ This option is only applicable when listing tags without annotation lines.
 	specified).
 
 --merged [<commit>]::
-	Only list tags whose tips are reachable from the specified commit
-	(`HEAD` if not specified).
+	Only list tags whose commits are reachable from the specified
+	commit (`HEAD` if not specified).
 
 --no-merged [<commit>]::
-	Only list tags whose tips are not reachable from the specified
+	Only list tags whose commits are not reachable from the specified
 	commit (`HEAD` if not specified).
 
 --points-at <object>::
-- 
2.11.0

[PATCH v3 04/16] ref-filter: make combining --merged & --no-merged an error

[Ævar Arnfjörð Bjarmason]

Change the behavior of specifying --merged & --no-merged to be an
error, instead of silently picking the option that was provided last.

Subsequent changes of mine add a --no-contains option in addition to
the existing --contains. Providing both of those isn't an error, and
has actual meaning.

Making its cousins have different behavior in this regard would be
confusing to the user, especially since we'd be silently disregarding
some of their command-line input.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-branch.txt       |  6 ++++--
 Documentation/git-for-each-ref.txt |  6 ++++--
 Documentation/git-tag.txt          |  4 ++--
 ref-filter.c                       | 11 ++++++++++-
 t/t3200-branch.sh                  |  4 ++++
 t/t6302-for-each-ref-filter.sh     |  4 ++++
 t/t7004-tag.sh                     |  4 ++++
 7 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 092f1bcf9f..e465298571 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -215,11 +215,13 @@ start-point is either a local or remote-tracking branch.
 
 --merged [<commit>]::
 	Only list branches whose tips are reachable from the
-	specified commit (HEAD if not specified). Implies `--list`.
+	specified commit (HEAD if not specified). Implies `--list`,
+	incompatible with `--no-merged`.
 
 --no-merged [<commit>]::
 	Only list branches whose tips are not reachable from the
-	specified commit (HEAD if not specified). Implies `--list`.
+	specified commit (HEAD if not specified). Implies `--list`,
+	incompatible with `--merged`.
 
 <branchname>::
 	The name of the branch to create or delete.
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 111e1be6f5..4d55893712 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -69,11 +69,13 @@ OPTIONS
 
 --merged [<object>]::
 	Only list refs whose tips are reachable from the
-	specified commit (HEAD if not specified).
+	specified commit (HEAD if not specified),
+	incompatible with `--no-merged`.
 
 --no-merged [<object>]::
 	Only list refs whose tips are not reachable from the
-	specified commit (HEAD if not specified).
+	specified commit (HEAD if not specified),
+	incompatible with `--merged`.
 
 --contains [<object>]::
 	Only list refs which contain the specified commit (HEAD if not
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 3abf912782..448fdf3743 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -126,11 +126,11 @@ This option is only applicable when listing tags without annotation lines.
 
 --merged [<commit>]::
 	Only list tags whose commits are reachable from the specified
-	commit (`HEAD` if not specified).
+	commit (`HEAD` if not specified), incompatible with `--no-merged`.
 
 --no-merged [<commit>]::
 	Only list tags whose commits are not reachable from the specified
-	commit (`HEAD` if not specified).
+	commit (`HEAD` if not specified), incompatible with `--merged`.
 
 --points-at <object>::
 	Only list tags of the given object.
diff --git a/ref-filter.c b/ref-filter.c
index 9c82b5b9d6..d7efae7b53 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2084,8 +2084,17 @@ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
 {
 	struct ref_filter *rf = opt->value;
 	unsigned char sha1[20];
+	int no_merged = starts_with(opt->long_name, "no");
 
-	rf->merge = starts_with(opt->long_name, "no")
+	if (rf->merge) {
+		if (no_merged) {
+			return opterror(opt, "is incompatible with --merged", 0);
+		} else {
+			return opterror(opt, "is incompatible with --no-merged", 0);
+		}
+	}
+
+	rf->merge = no_merged
 		? REF_FILTER_MERGED_OMIT
 		: REF_FILTER_MERGED_INCLUDE;
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 9f353c0efc..fe62e7c775 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -978,6 +978,10 @@ test_expect_success '--merged catches invalid object names' '
 	test_must_fail git branch --merged 0000000000000000000000000000000000000000
 '
 
+test_expect_success '--merged is incompatible with --no-merged' '
+	test_must_fail git branch --merged HEAD --no-merged HEAD
+'
+
 test_expect_success 'tracking with unexpected .fetch refspec' '
 	rm -rf a b c d &&
 	git init a &&
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index a09a1a46ef..d36d5dc124 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -421,4 +421,8 @@ test_expect_success 'check %(if:notequals=<string>)' '
 	test_cmp expect actual
 '
 
+test_expect_success '--merged is incompatible with --no-merged' '
+	test_must_fail git for-each-ref --merged HEAD --no-merged HEAD
+'
+
 test_done
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f5..45790664c1 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1748,6 +1748,10 @@ test_expect_success '--merged cannot be used in non-list mode' '
 	test_must_fail git tag --merged=mergetest-2 foo
 '
 
+test_expect_success '--merged is incompatible with --no-merged' '
+	test_must_fail git tag --merged HEAD --no-merged HEAD
+'
+
 test_expect_success '--merged shows merged tags' '
 	cat >expect <<-\EOF &&
 	mergetest-1
-- 
2.11.0

[PATCH v3 05/16] ref-filter: add test for --contains on a non-commit

[Ævar Arnfjörð Bjarmason]

Change the tag test suite to test for --contains on a tree & blob. It
only accepts commits and will spew out "<object> is a tree, not a
commit".

It's sufficient to test this just for the "tag" and "branch" commands,
because it covers all the machinery shared between "branch" and
"for-each-ref".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3201-branch-contains.sh | 9 +++++++++
 t/t7004-tag.sh             | 4 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index 7f3ec47241..daa3ae82b7 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -130,6 +130,15 @@ test_expect_success 'implicit --list conflicts with modification options' '
 
 '
 
+test_expect_success 'Assert that --contains only works on commits, not trees & blobs' '
+	test_must_fail git branch --contains master^{tree} &&
+	blob=$(git hash-object -w --stdin <<-\EOF
+	Some blob
+	EOF
+	) &&
+	test_must_fail git branch --contains $blob
+'
+
 # We want to set up a case where the walk for the tracking info
 # of one branch crosses the tip of another branch (and make sure
 # that the latter walk does not mess up our flag to see if it was
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 45790664c1..3439913488 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1461,7 +1461,9 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
 	test_must_fail git tag -n 100 &&
 	test_must_fail git tag -l -m msg &&
 	test_must_fail git tag -l -F some file &&
-	test_must_fail git tag -v -s
+	test_must_fail git tag -v -s &&
+	test_must_fail git tag --contains tag-tree &&
+	test_must_fail git tag --contains tag-blob
 '
 
 # check points-at
-- 
2.11.0

[PATCH v3 06/16] tag: remove a TODO item from the test suite

[Ævar Arnfjörð Bjarmason]

Change the test for "git tag -l" to not have an associated TODO
comment saying that it should return non-zero if there's no tags.

This was added in commit ef5a6fb597 ("Add test-script for git-tag",
2007-06-28) when the tests for "tag" were initially added, but at this
point changing this would be inconsistent with how "git tag" is a
synonym for "git tag -l", and would needlessly break external code
that relies on this porcelain command.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t7004-tag.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 3439913488..830eff948e 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -16,7 +16,6 @@ tag_exists () {
 	git show-ref --quiet --verify refs/tags/"$1"
 }
 
-# todo: git tag -l now returns always zero, when fixed, change this test
 test_expect_success 'listing all tags in an empty tree should succeed' '
 	git tag -l &&
 	git tag
@@ -136,7 +135,6 @@ test_expect_success \
 	'listing a tag using a matching pattern should output that tag' \
 	'test $(git tag -l mytag) = mytag'
 
-# todo: git tag -l now returns always zero, when fixed, change this test
 test_expect_success \
 	'listing tags using a non-matching pattern should suceed' \
 	'git tag -l xxx'
-- 
2.11.0

[PATCH v3 07/16] tag tests: fix a typo in a test description

[Ævar Arnfjörð Bjarmason]

Change "suceed" to "succeed" in a test description. The typo has been
here since the code was originally added in commit ef5a6fb597 ("Add
test-script for git-tag", 2007-06-28).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t7004-tag.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 830eff948e..63ee2cf727 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -136,7 +136,7 @@ test_expect_success \
 	'test $(git tag -l mytag) = mytag'
 
 test_expect_success \
-	'listing tags using a non-matching pattern should suceed' \
+	'listing tags using a non-matching pattern should succeed' \
 	'git tag -l xxx'
 
 test_expect_success \
-- 
2.11.0

[PATCH v3 08/16] for-each-ref: partly change <object> to <commit> in help

[Ævar Arnfjörð Bjarmason]

Change mentions of <object> to <commit> in the help output of
for-each-ref as appropriate.

Both --[no-]merged and --contains only take commits, but --points-at
can take any object, such as a tag pointing to a tree or blob.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/for-each-ref.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index df41fa0350..1a5ed20f59 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -8,8 +8,8 @@
 static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
 	N_("git for-each-ref [--points-at <object>]"),
-	N_("git for-each-ref [(--merged | --no-merged) [<object>]]"),
-	N_("git for-each-ref [--contains [<object>]]"),
+	N_("git for-each-ref [(--merged | --no-merged) [<commit>]]"),
+	N_("git for-each-ref [--contains [<commit>]]"),
 	NULL
 };
 
-- 
2.11.0

[PATCH v3 09/16] tag: add more incompatibles mode tests

[Ævar Arnfjörð Bjarmason]

Amend the test suite to test for more invalid uses like "-l -a"
etc. This change tests this code path in builtin/tag.c:

    `(argc == 0 && !cmdmode)`
    ->
    `((create_tag_object || force) && (cmdmode != 0))`

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t7004-tag.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 63ee2cf727..92af8bb7e6 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1455,8 +1455,24 @@ test_expect_success 'checking that initial commit is in all tags' "
 
 test_expect_success 'mixing incompatibles modes and options is forbidden' '
 	test_must_fail git tag -a &&
+	test_must_fail git tag -a -l &&
+	test_must_fail git tag -s &&
+	test_must_fail git tag -s -l &&
+	test_must_fail git tag -m &&
+	test_must_fail git tag -m -l &&
+	test_must_fail git tag -m "hlagh" &&
+	test_must_fail git tag -m "hlagh" -l &&
+	test_must_fail git tag -F &&
+	test_must_fail git tag -F -l &&
+	test_must_fail git tag -f &&
+	test_must_fail git tag -f -l &&
+	test_must_fail git tag -a -s -m -F &&
+	test_must_fail git tag -a -s -m -F -l &&
 	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 &&
+	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 &&
-- 
2.11.0

Re: [PATCH v3 09/16] tag: add more incompatibles mode tests

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Amend the test suite to test for more invalid uses like "-l -a"
> etc. This change tests this code path in builtin/tag.c:
>
>     `(argc == 0 && !cmdmode)`
>     ->
>     `((create_tag_object || force) && (cmdmode != 0))`

What is this notation trying to tell us?  A range of lines starting
with the first one to the second one?  If so perhaps

	Add tests to detect more invalid combinations like "-a -l",
	to test code that defaults to the list mode when no other
	conflicting options are given and that detects conflicting
	requests.

would be easier to understand, I would think.

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/t7004-tag.sh | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 63ee2cf727..92af8bb7e6 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1455,8 +1455,24 @@ test_expect_success 'checking that initial commit is in all tags' "
>  
>  test_expect_success 'mixing incompatibles modes and options is forbidden' '
>  	test_must_fail git tag -a &&
> +	test_must_fail git tag -a -l &&
> +	test_must_fail git tag -s &&
> +	test_must_fail git tag -s -l &&
> +	test_must_fail git tag -m &&
> +	test_must_fail git tag -m -l &&
> +	test_must_fail git tag -m "hlagh" &&
> +	test_must_fail git tag -m "hlagh" -l &&
> +	test_must_fail git tag -F &&
> +	test_must_fail git tag -F -l &&
> +	test_must_fail git tag -f &&
> +	test_must_fail git tag -f -l &&
> +	test_must_fail git tag -a -s -m -F &&
> +	test_must_fail git tag -a -s -m -F -l &&
>  	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 &&
> +	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 &&

Re: [PATCH v3 09/16] tag: add more incompatibles mode tests

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 23, 2017 at 6:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Amend the test suite to test for more invalid uses like "-l -a"
>> etc. This change tests this code path in builtin/tag.c:
>>
>>     `(argc == 0 && !cmdmode)`
>>     ->
>>     `((create_tag_object || force) && (cmdmode != 0))`
>
> What is this notation trying to tell us?  A range of lines starting
> with the first one to the second one?

Yeah.

> If so perhaps
>
>         Add tests to detect more invalid combinations like "-a -l",
>         to test code that defaults to the list mode when no other
>         conflicting options are given and that detects conflicting
>         requests.

I think the only people who'll ever dig up this commit is someone
trying to figure out why one of these invocations dies after 'git
blame'-ing this code. Surely it'll be more useful to them to see what
specific code this was aiming to test, rather than having more vague
prose like that...

> would be easier to understand, I would think.
>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  t/t7004-tag.sh | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
>> index 63ee2cf727..92af8bb7e6 100755
>> --- a/t/t7004-tag.sh
>> +++ b/t/t7004-tag.sh
>> @@ -1455,8 +1455,24 @@ test_expect_success 'checking that initial commit is in all tags' "
>>
>>  test_expect_success 'mixing incompatibles modes and options is forbidden' '
>>       test_must_fail git tag -a &&
>> +     test_must_fail git tag -a -l &&
>> +     test_must_fail git tag -s &&
>> +     test_must_fail git tag -s -l &&
>> +     test_must_fail git tag -m &&
>> +     test_must_fail git tag -m -l &&
>> +     test_must_fail git tag -m "hlagh" &&
>> +     test_must_fail git tag -m "hlagh" -l &&
>> +     test_must_fail git tag -F &&
>> +     test_must_fail git tag -F -l &&
>> +     test_must_fail git tag -f &&
>> +     test_must_fail git tag -f -l &&
>> +     test_must_fail git tag -a -s -m -F &&
>> +     test_must_fail git tag -a -s -m -F -l &&
>>       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 &&
>> +     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 &&

Re: [PATCH v3 09/16] tag: add more incompatibles mode tests

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Thu, Mar 23, 2017 at 6:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>
>>> Amend the test suite to test for more invalid uses like "-l -a"
>>> etc. This change tests this code path in builtin/tag.c:
>>>
>>>     `(argc == 0 && !cmdmode)`
>>>     ->
>>>     `((create_tag_object || force) && (cmdmode != 0))`
>>
>> What is this notation trying to tell us?  A range of lines starting
>> with the first one to the second one?
>
> Yeah.

The reason why I asked about the original notation was because it
looked as if you are saying "the former is changed to the latter,
and this is to make sure it does not regress".  

Compared to that, spelling out why in a readable sentence would be
much more helpful for later readers of the log output, no matter
what they want to do with the information.

Re: [PATCH v3 09/16] tag: add more incompatibles mode tests

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 23, 2017 at 10:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Thu, Mar 23, 2017 at 6:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>>
>>>> Amend the test suite to test for more invalid uses like "-l -a"
>>>> etc. This change tests this code path in builtin/tag.c:
>>>>
>>>>     `(argc == 0 && !cmdmode)`
>>>>     ->
>>>>     `((create_tag_object || force) && (cmdmode != 0))`
>>>
>>> What is this notation trying to tell us?  A range of lines starting
>>> with the first one to the second one?
>>
>> Yeah.
>
> The reason why I asked about the original notation was because it
> looked as if you are saying "the former is changed to the latter,
> and this is to make sure it does not regress".
>
> Compared to that, spelling out why in a readable sentence would be
> much more helpful for later readers of the log output, no matter
> what they want to do with the information.

Ah, I see. Will reword as "between lines .. and ...". I think it's
worth showing the actual code it's trying to test since it's short
enough, and anyone digging into these tests would want to know what
part of the code the tests were trying to test.

[PATCH v3 10/16] parse-options: add OPT_NONEG to the "contains" option

[Ævar Arnfjörð Bjarmason]

Add the OPT_NONEG flag to the "contains" option and its hidden synonym
"with". Since this was added in commit 694a577519 ("git-branch
--contains=commit", 2007-11-07) giving --no-{contains,with} hasn't
been an error, but has emitted the help output since
filter.with_commit wouldn't get set.

Now git will emit "error: unknown option `no-{contains,with}'" at the
top of the help output.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 parse-options.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/parse-options.h b/parse-options.h
index dcd8a0926c..9f48f554ba 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -258,7 +258,7 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
 	  PARSE_OPT_LASTARG_DEFAULT | flag, \
 	  parse_opt_commits, (intptr_t) "HEAD" \
 	}
-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
-#define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
+#define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 
 #endif
-- 
2.11.0

[PATCH v3 11/16] tag: change misleading --list <pattern> documentation

[Ævar Arnfjörð Bjarmason]

Change the documentation for --list so that it's described as a
toggle, not as an option that takes a <pattern> as an argument.

Junio initially documented this in b867c7c23a ("git-tag: -l to list
tags (usability).", 2006-02-17), but later Jeff King changed "tag" to
accept multiple patterns in 588d0e834b ("tag: accept multiple patterns
for --list", 2011-06-20).

However, documenting this as "-l <pattern>" was never correct, as
these both worked before Jeff's change:

    git tag -l 'v*'
    git tag 'v*' -l

One would expect an option that was documented like that to only
accept:

    git tag --list
    git tag --list 'v*rc*'

And after Jeff's change, one that took multiple patterns:

    git tag --list 'v*rc*' --list '*2.8*'

But since it's actually a toggle all of these work as well, and
produce identical output to the last example above:

    git tag --list 'v*rc*' '*2.8*'
    git tag --list 'v*rc*' '*2.8*' --list --list --list
    git tag --list 'v*rc*' '*2.8*' --list -l --list -l --list

Now the documentation is more in tune with how the "branch" command
describes its --list option since commit cddd127b9a ("branch:
introduce --list option", 2011-08-28).

Change the test suite to assert that these invocations work for the
cases that weren't already being tested for.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-tag.txt | 16 +++++++++-------
 t/t7004-tag.sh            | 25 +++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 448fdf3743..2640c6955c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -87,13 +87,15 @@ OPTIONS
 	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.
 
--l <pattern>::
---list <pattern>::
-	List tags with names that match the given pattern (or all if no
-	pattern is given).  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.
+-l::
+--list::
+	Activate the list mode. `git tag <tagname>` would try to
+	create a tag, use `git tag --list <pattern>...` to list
+	matching branches, (or all if no pattern is given).
++
+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.
 
 --sort=<key>::
 	Sort based on the key given.  Prefix `-` to sort in
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 92af8bb7e6..75681b2cad 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -118,6 +118,18 @@ test_expect_success 'listing all tags if one exists should succeed' '
 	git tag
 '
 
+cat >expect <<EOF
+mytag
+EOF
+test_expect_success 'Multiple -l or --list options are equivalent to one -l option' '
+	git tag -l -l >actual &&
+	test_cmp expect actual &&
+	git tag --list --list >actual &&
+	test_cmp expect actual &&
+	git tag --list -l --list >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'listing all tags if one exists should output that tag' '
 	test $(git tag -l) = mytag &&
 	test $(git tag) = mytag
@@ -336,6 +348,19 @@ test_expect_success 'tag -l can accept multiple patterns' '
 	test_cmp expect actual
 '
 
+# Between v1.7.7 & v2.13.0 a fair reading of the git-tag documentation
+# could leave you with the impression that "-l <pattern> -l <pattern>"
+# was how we wanted to accept multiple patterns.
+#
+# This test should not imply that this is a sane thing to support. but
+# since the documentation was worded like it was let's at least find
+# out if we're going to break this long-documented form of taking
+# multiple patterns.
+test_expect_success 'tag -l <pattern> -l <pattern> works, as our buggy documentation previously suggested' '
+	git tag -l "v1*" -l "v0*" >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'listing tags in column' '
 	COLUMNS=40 git tag -l --column=row >actual &&
 	cat >expected <<\EOF &&
-- 
2.11.0

Re: [PATCH v3 11/16] tag: change misleading --list <pattern> documentation

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> --l <pattern>::
> ---list <pattern>::
> -	List tags with names that match the given pattern (or all if no
> -	pattern is given).  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.
> +-l::
> +--list::
> +	Activate the list mode. `git tag <tagname>` would try to
> +	create a tag, use `git tag --list <pattern>...` to list
> +	matching branches, (or all if no pattern is given).
> ++
> +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.

Reads much easier, especially with two separate paragraphs.  The
first one may want to match what I suggested for the "branch"
update, though.

Thanks.

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

[Ævar Arnfjörð Bjarmason]

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-tag.txt | 17 +++++++++------
 builtin/tag.c             | 18 ++++++++++------
 t/t7004-tag.sh            | 55 ++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 73 insertions(+), 17 deletions(-)

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.
 
 -l::
 --list::
@@ -96,6 +97,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 +129,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 +140,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..3c686961db 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -454,8 +454,14 @@ 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 ||
+			 filter.lines != -1)
+			cmdmode = 'l';
+	}
 
 	if ((create_tag_object || force) && (cmdmode != 0))
 		usage_with_options(git_tag_usage, options);
@@ -485,13 +491,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 75681b2cad..5823de16aa 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -643,6 +643,11 @@ test_expect_success \
 	git tag -n0 -l tag-one-line >actual &&
 	test_cmp expect actual &&
 
+	git tag -n0 | grep "^tag-one-line" >actual &&
+	test_cmp expect actual &&
+	git tag -n0 tag-one-line >actual &&
+	test_cmp expect actual &&
+
 	echo "tag-one-line    A msg" >expect &&
 	git tag -n1 -l | grep "^tag-one-line" >actual &&
 	test_cmp expect actual &&
@@ -656,6 +661,17 @@ test_expect_success \
 	test_cmp expect actual
 '
 
+test_expect_success 'The -n 100 invocation means -n --list 100, not -n100' '
+	>expect &&
+	git tag -n 100 >actual &&
+	test_cmp expect actual &&
+
+	git tag -m "A msg" 100 &&
+	echo "100             A msg" >expect &&
+	git tag -n 100 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success \
 	'listing the zero-lines message of a non-signed tag should succeed' '
 	git tag -m "" tag-zero-lines &&
@@ -1476,6 +1492,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' '
@@ -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 &&
 	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 &&
@@ -1505,10 +1525,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' '
@@ -1785,8 +1820,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' '
@@ -1810,6 +1850,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

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

[Junio C Hamano]

Æ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.

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

[Ævar Arnfjörð Bjarmason]

On Thu, Mar 23, 2017 at 6:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Æ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".

>> @@ -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.

>>       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.

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> 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,...

Then what you did is the right thing to do.  Thanks.

[PATCH v3 13/16] tag: change --point-at to default to HEAD

[Ævar Arnfjörð Bjarmason]

Change the --points-at option to default to HEAD for consistency with
its siblings --contains, --merged etc. which default to
HEAD. Previously we'd get:

    $ git tag --points-at 2>&1 | head -n 1
    error: option `points-at' requires a value

This changes behavior added in commit ae7706b9ac (tag: add --points-at
list option, 2012-02-08).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-tag.txt | 3 ++-
 builtin/tag.c             | 3 ++-
 t/t7004-tag.sh            | 9 ++++++++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 491dba065c..6de440f139 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -140,7 +140,8 @@ 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. Implies `--list`.
+	Only list tags of the given object (HEAD if not
+	specified). Implies `--list`.
 
 -m <msg>::
 --message=<msg>::
diff --git a/builtin/tag.c b/builtin/tag.c
index 3c686961db..8bf6d85176 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -431,7 +431,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			     N_("field name to sort on"), &parse_opt_ref_sorting),
 		{
 			OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
-			N_("print only tags of the object"), 0, parse_opt_object_name
+			N_("print only tags of the object"), PARSE_OPT_LASTARG_DEFAULT,
+			parse_opt_object_name, (intptr_t) "HEAD"
 		},
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
 		OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 5823de16aa..3529c3009c 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1534,7 +1534,8 @@ do
 	"
 	test_expect_success "Doing 'git tag --list-like $option <commit> <pattern> is permitted" "
 		git tag -n $option HEAD HEAD &&
-		git tag $option HEAD HEAD
+		git tag $option HEAD HEAD &&
+		git tag $option
 	"
 done
 
@@ -1546,6 +1547,12 @@ test_expect_success '--points-at can be used in non-list mode' '
 	test_cmp expect actual
 '
 
+test_expect_success '--points-at is a synonym for --points-at HEAD' '
+	echo v4.0 >expect &&
+	git tag --points-at >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '--points-at finds lightweight tags' '
 	echo v4.0 >expect &&
 	git tag --points-at v4.0 >actual &&
-- 
2.11.0

[PATCH v3 14/16] ref-filter: add --no-contains option to tag/branch/for-each-ref

[Ævar Arnfjörð Bjarmason]

Change the tag, branch & for-each-ref commands to have a --no-contains
option in addition to their longstanding --contains options.

This allows for finding the last-good rollout tag given a known-bad
<commit>. Given a hypothetically bad commit cf5c7253e0, the git
version to revert to can be found with this hacky two-liner:

    (git tag -l 'v[0-9]*'; git tag -l --contains cf5c7253e0 'v[0-9]*') |
        sort | uniq -c | grep -E '^ *1 ' | awk '{print $2}' | tail -n 10

With this new --no-contains option the same can be achieved with:

    git tag -l --no-contains cf5c7253e0 'v[0-9]*' | sort | tail -n 10

As the filtering machinery is shared between the tag, branch &
for-each-ref commands, implement this for those commands too. A
practical use for this with "branch" is e.g. finding branches which
were branched off between v2.8.0 and v2.10.0:

    git branch --contains v2.8.0 --no-contains v2.10.0

The "describe" command also has a --contains option, but its semantics
are unrelated to what tag/branch/for-each-ref use --contains for. A
--no-contains option for "describe" wouldn't make any sense, other
than being exactly equivalent to not supplying --contains at all,
which would be confusing at best.

Add a --without option to "tag" as an alias for --no-contains, for
consistency with --with and --contains.  The --with option is
undocumented, and possibly the only user of it is
Junio (<xmqqefy71iej.fsf@gitster.mtv.corp.google.com>). But it's
trivial to support, so let's do that.

The additions to the the test suite are inverse copies of the
corresponding --contains tests. With this change --no-contains for
tag, branch & for-each-ref is just as well tested as the existing
--contains option.

In addition to those tests, add a test for "tag" which asserts that
--no-contains won't find tree/blob tags, which is slightly
unintuitive, but consistent with how --contains works & is documented.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-branch.txt           |  16 +++-
 Documentation/git-for-each-ref.txt     |   6 +-
 Documentation/git-tag.txt              |   6 +-
 builtin/branch.c                       |   5 +-
 builtin/for-each-ref.c                 |   3 +-
 builtin/tag.c                          |   8 +-
 contrib/completion/git-completion.bash |   4 +-
 parse-options.h                        |   2 +
 ref-filter.c                           |  19 +++--
 ref-filter.h                           |   1 +
 t/t3201-branch-contains.sh             |  54 +++++++++++++-
 t/t6302-for-each-ref-filter.sh         |  16 ++++
 t/t7004-tag.sh                         | 130 ++++++++++++++++++++++++++++++++-
 13 files changed, 245 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index e465298571..e4b5d5c3e1 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,8 @@ SYNOPSIS
 'git branch' [--color[=<when>] | --no-color] [-r | -a]
 	[--list] [-v [--abbrev=<length> | --no-abbrev]]
 	[--column[=<options>] | --no-column]
-	[(--merged | --no-merged | --contains) [<commit>]] [--sort=<key>]
+	[(--merged | --no-merged) [<commit>]]
+	[--contains [<commit]] [--no-contains [<commit>]] [--sort=<key>]
 	[--points-at <object>] [--format=<format>] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
@@ -35,7 +36,7 @@ as branch creation.
 
 With `--contains`, shows only the branches that contain the named commit
 (in other words, the branches whose tip commits are descendants of the
-named commit).  With `--merged`, only branches merged into the named
+named commit), `--no-contains` inverts it. With `--merged`, only branches merged into the named
 commit (i.e. the branches whose tip commits are reachable from the named
 commit) will be listed.  With `--no-merged` only branches not merged into
 the named commit will be listed.  If the <commit> argument is missing it
@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.
 	Only list branches which contain the specified commit (HEAD
 	if not specified). Implies `--list`.
 
+--no-contains [<commit>]::
+	Only list branches which don't contain the specified commit
+	(HEAD if not specified). Implies `--list`.
+
 --merged [<commit>]::
 	Only list branches whose tips are reachable from the
 	specified commit (HEAD if not specified). Implies `--list`,
@@ -298,13 +303,16 @@ If you are creating a branch that you want to checkout immediately, it is
 easier to use the git checkout command with its `-b` option to create
 a branch and check it out with a single command.
 
-The options `--contains`, `--merged` and `--no-merged` serve three related
-but different purposes:
+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`
+serve four related but different purposes:
 
 - `--contains <commit>` is used to find all branches which will need
   special attention if <commit> were to be rebased or amended, since those
   branches contain the specified <commit>.
 
+- `--no-contains <commit>` is the inverse of that, i.e. branches that don't
+  contain the specified <commit>.
+
 - `--merged` is used to find all branches which can be safely deleted,
   since those branches are fully contained by HEAD.
 
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 4d55893712..03e187a105 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
 		   [--points-at <object>] [(--merged | --no-merged) [<object>]]
-		   [--contains [<object>]]
+		   [--contains [<object>]] [--no-contains [<object>]]
 
 DESCRIPTION
 -----------
@@ -81,6 +81,10 @@ OPTIONS
 	Only list refs which contain the specified commit (HEAD if not
 	specified).
 
+--no-contains [<object>]::
+	Only list refs which don't contain the specified commit (HEAD
+	if not specified).
+
 --ignore-case::
 	Sorting and filtering refs are case insensitive.
 
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 6de440f139..028a8ce4a9 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
+'git tag' [-n[<num>]] -l [--contains <commit>] [--contains <commit>] [--points-at <object>]
 	[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
 	[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
 'git tag' -v [--format=<format>] <tagname>...
@@ -131,6 +131,10 @@ This option is only applicable when listing tags without annotation lines.
 	Only list tags which contain the specified commit (HEAD if not
 	specified). Implies `--list`.
 
+--no-contains [<commit>]::
+	Only list tags which don't contain the specified commit (HEAD if
+	not specified). Implies `--list`.
+
 --merged [<commit>]::
 	Only list tags whose commits are reachable from the specified
 	commit (`HEAD` if not specified), incompatible with `--no-merged`.
diff --git a/builtin/branch.c b/builtin/branch.c
index 52688f2e1b..0552c42ad1 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -562,7 +562,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
 			FILTER_REFS_REMOTES),
 		OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
+		OPT_WITHOUT(&filter.no_commit, N_("print only branches that don't contain the commit")),
 		OPT__ABBREV(&filter.abbrev),
 
 		OPT_GROUP(N_("Specific git-branch actions:")),
@@ -618,7 +620,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
-	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
+	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
+	    filter.no_commit)
 		list = 1;
 
 	if (!!delete + !!rename + !!new_upstream +
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1a5ed20f59..eca365bf89 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,7 +9,7 @@ static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
 	N_("git for-each-ref [--points-at <object>]"),
 	N_("git for-each-ref [(--merged | --no-merged) [<commit>]]"),
-	N_("git for-each-ref [--contains [<commit>]]"),
+	N_("git for-each-ref [--contains [<commit>]] [--no-contains [<commit>]]"),
 	NULL
 };
 
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_MERGED(&filter, N_("print only refs that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which don't contain the commit")),
 		OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END(),
 	};
diff --git a/builtin/tag.c b/builtin/tag.c
index 8bf6d85176..dbc6f5b74b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -22,7 +22,7 @@
 static const char * const git_tag_usage[] = {
 	N_("git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]"),
 	N_("git tag -d <tagname>..."),
-	N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
+	N_("git tag -l [-n[<num>]] [--contains <commit>] [--no-contains <commit>] [--points-at <object>]"
 		"\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
 	N_("git tag -v [--format=<format>] <tagname>..."),
 	NULL
@@ -424,7 +424,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(N_("Tag listing options")),
 		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_NO_CONTAINS(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
+		OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_MERGED(&filter, N_("print only tags that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
 		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
@@ -458,7 +460,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (!cmdmode) {
 		if (argc == 0)
 			cmdmode = 'l';
-		else if (filter.with_commit ||
+		else if (filter.with_commit || filter.no_commit ||
 			 filter.points_at.nr || filter.merge_commit ||
 			 filter.lines != -1)
 			cmdmode = 'l';
@@ -495,6 +497,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die(_("-n option is only allowed in list mode"));
 	if (filter.with_commit)
 		die(_("--contains option is only allowed in list mode"));
+	if (filter.no_commit)
+		die(_("--no-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)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fc32286a43..ec8fce5820 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1093,7 +1093,7 @@ _git_branch ()
 	--*)
 		__gitcomp "
 			--color --no-color --verbose --abbrev= --no-abbrev
-			--track --no-track --contains --merged --no-merged
+			--track --no-track --contains --no-contains --merged --no-merged
 			--set-upstream-to= --edit-description --list
 			--unset-upstream --delete --move --remotes
 			--column --no-column --sort= --points-at
@@ -2862,7 +2862,7 @@ _git_tag ()
 		__gitcomp "
 			--list --delete --verify --annotate --message --file
 			--sign --cleanup --local-user --force --column --sort=
-			--contains --points-at --merged --no-merged --create-reflog
+			--contains --no-contains --points-at --merged --no-merged --create-reflog
 			"
 		;;
 	esac
diff --git a/parse-options.h b/parse-options.h
index 9f48f554ba..af711227ae 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -259,6 +259,8 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
 	  parse_opt_commits, (intptr_t) "HEAD" \
 	}
 #define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 
 #endif
diff --git a/ref-filter.c b/ref-filter.c
index d7efae7b53..1e39273005 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1487,6 +1487,7 @@ struct ref_filter_cbdata {
 	struct ref_array *array;
 	struct ref_filter *filter;
 	struct contains_cache contains_cache;
+	struct contains_cache no_contains_cache;
 };
 
 /*
@@ -1586,11 +1587,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
 }
 
 static int commit_contains(struct ref_filter *filter, struct commit *commit,
-			   struct contains_cache *cache)
+			   struct commit_list *list, struct contains_cache *cache)
 {
 	if (filter->with_commit_tag_algo)
-		return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
-	return is_descendant_of(commit, filter->with_commit);
+		return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
+	return is_descendant_of(commit, list);
 }
 
 /*
@@ -1780,13 +1781,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 	 * obtain the commit using the 'oid' available and discard all
 	 * non-commits early. The actual filtering is done later.
 	 */
-	if (filter->merge_commit || filter->with_commit || filter->verbose) {
+	if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
 		commit = lookup_commit_reference_gently(oid->hash, 1);
 		if (!commit)
 			return 0;
-		/* We perform the filtering for the '--contains' option */
+		/* We perform the filtering for the '--contains' option... */
 		if (filter->with_commit &&
-		    !commit_contains(filter, commit, &ref_cbdata->contains_cache))
+		    !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))
+			return 0;
+		/* ...or for the `--no-contains' option */
+		if (filter->no_commit &&
+		    commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))
 			return 0;
 	}
 
@@ -1887,6 +1892,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 	filter->kind = type & FILTER_REFS_KIND_MASK;
 
 	init_contains_cache(&ref_cbdata.contains_cache);
+	init_contains_cache(&ref_cbdata.no_contains_cache);
 
 	/*  Simple per-ref filtering */
 	if (!filter->kind)
@@ -1911,6 +1917,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 	}
 
 	clear_contains_cache(&ref_cbdata.contains_cache);
+	clear_contains_cache(&ref_cbdata.no_contains_cache);
 
 	/*  Filters that need revision walking */
 	if (filter->merge_commit)
diff --git a/ref-filter.h b/ref-filter.h
index e738c5dfd3..dde40f6849 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -53,6 +53,7 @@ struct ref_filter {
 	const char **name_patterns;
 	struct sha1_array points_at;
 	struct commit_list *with_commit;
+	struct commit_list *no_commit;
 
 	enum {
 		REF_FILTER_MERGED_NONE = 0,
diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index daa3ae82b7..0ef1b6fdcc 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='branch --contains <commit>, --merged, and --no-merged'
+test_description='branch --contains <commit>, --no-contains <commit> --merged, and --no-merged'
 
 . ./test-lib.sh
 
@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '
 
 '
 
+test_expect_success 'branch --no-contains=master' '
+
+	git branch --no-contains=master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'branch --no-contains master' '
+
+	git branch --no-contains master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains=side' '
 
 	git branch --contains=side >actual &&
@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '
 
 '
 
+test_expect_success 'branch --no-contains=side' '
+
+	git branch --no-contains=side >actual &&
+	{
+		echo "  master"
+	} >expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'branch --contains with pattern implies --list' '
 
 	git branch --contains=master master >actual &&
@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '
 
 '
 
+test_expect_success 'branch --no-contains with pattern implies --list' '
+
+	git branch --no-contains=master master >actual &&
+	>expect &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'side: branch --merged' '
 
 	git branch --merged >actual &&
@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '
 test_expect_success 'implicit --list conflicts with modification options' '
 
 	test_must_fail git branch --contains=master -d &&
-	test_must_fail git branch --contains=master -m foo
+	test_must_fail git branch --contains=master -m foo &&
+	test_must_fail git branch --no-contains=master -d &&
+	test_must_fail git branch --no-contains=master -m foo
 
 '
 
@@ -136,7 +172,8 @@ test_expect_success 'Assert that --contains only works on commits, not trees & b
 	Some blob
 	EOF
 	) &&
-	test_must_fail git branch --contains $blob
+	test_must_fail git branch --contains $blob &&
+	test_must_fail git branch --no-contains $blob
 '
 
 # We want to set up a case where the walk for the tracking info
@@ -168,4 +205,15 @@ test_expect_success 'branch --merged with --verbose' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'branch --contains combined with --no-contains' '
+	git branch --contains zzz --no-contains topic >actual &&
+	cat >expect <<-\EOF &&
+	  master
+	  side
+	  zzz
+	EOF
+	test_cmp expect actual
+
+'
+
 test_done
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index d36d5dc124..fc067ed672 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -93,6 +93,22 @@ test_expect_success 'filtering with --contains' '
 	test_cmp expect actual
 '
 
+test_expect_success 'filtering with --no-contains' '
+	cat >expect <<-\EOF &&
+	refs/tags/one
+	EOF
+	git for-each-ref --format="%(refname)" --no-contains=two >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'filtering with --contains and --no-contains' '
+	cat >expect <<-\EOF &&
+	refs/tags/two
+	EOF
+	git for-each-ref --format="%(refname)" --contains=two --no-contains=three >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '%(color) must fail' '
 	test_must_fail git for-each-ref --format="%(color)%(refname)"
 '
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 3529c3009c..8a6e8032da 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1424,6 +1424,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' "
 	test_cmp expected actual
 "
 
+# All the --contains tests above, but with --no-contains
+test_expect_success 'checking that first commit is not listed in any tag with --no-contains  (hash)' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (tag)' "
+	git tag -l --no-contains v1.0 v* >actual &&
+	test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (relative)' "
+	git tag -l --no-contains HEAD~2 v* >actual &&
+	test_cmp expected actual
+"
+
 cat > expected <<EOF
 v2.0
 EOF
@@ -1433,6 +1450,17 @@ test_expect_success 'checking that second commit only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+EOF
+
+test_expect_success 'inverse of the last test, with --no-contains' "
+	git tag -l --no-contains $hash2 v* >actual &&
+	test_cmp expected actual
+"
 
 cat > expected <<EOF
 EOF
@@ -1442,6 +1470,19 @@ test_expect_success 'checking that third commit has no tags' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'conversely --no-contains on the third commit lists all tags' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
 # how about a simple merge?
 
 test_expect_success 'creating simple branch' '
@@ -1463,6 +1504,19 @@ test_expect_success 'checking that branch head only has one tag' "
 	test_cmp expected actual
 "
 
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'checking that branch head with --no-contains lists all but one tag' "
+	git tag -l --no-contains $hash4 v* >actual &&
+	test_cmp expected actual
+"
+
 test_expect_success 'merging original branch into this branch' '
 	git merge --strategy=ours master &&
         git tag v4.0
@@ -1484,6 +1538,20 @@ v1.0.1
 v1.1.3
 v2.0
 v3.0
+EOF
+
+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' "
+	git tag -l --no-contains $hash3 v* >actual &&
+	test_cmp expected actual
+"
+
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+v3.0
 v4.0
 EOF
 
@@ -1497,6 +1565,12 @@ test_expect_success 'checking that --contains can be used in non-list mode' '
 	test_cmp expected actual
 '
 
+test_expect_success 'checking that initial commit is in all tags with --no-contains' "
+	>expected &&
+	git tag -l --no-contains $hash1 v* >actual &&
+	test_cmp expected actual
+"
+
 # mixing modes and options:
 
 test_expect_success 'mixing incompatibles modes and options is forbidden' '
@@ -1522,10 +1596,13 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
 	test_must_fail git tag -l -F some file &&
 	test_must_fail git tag -v -s &&
 	test_must_fail git tag --contains tag-tree &&
-	test_must_fail git tag --contains tag-blob
+	test_must_fail git tag --contains tag-blob &&
+	test_must_fail git tag --no-contains tag-tree &&
+	test_must_fail git tag --no-contains tag-blob &&
+	test_must_fail git tag --contains --no-contains
 '
 
-for option in --contains --merged --no-merged --points-at
+for option in --contains --no-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 &&
@@ -1792,7 +1869,7 @@ run_with_limited_stack () {
 test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
 
 # we require ulimit, this excludes Windows
-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '
+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '
 	>expect &&
 	i=1 &&
 	while test $i -lt 8000
@@ -1808,7 +1885,9 @@ EOF"
 	git checkout master &&
 	git tag far-far-away HEAD^ &&
 	run_with_limited_stack git tag --contains HEAD >actual &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	run_with_limited_stack git tag --no-contains HEAD >actual &&
+	test_line_count ">" 10 actual
 '
 
 test_expect_success '--format should list tags as per format given' '
@@ -1870,4 +1949,47 @@ test_expect_success 'ambiguous branch/tags not marked' '
 	test_cmp expect actual
 '
 
+test_expect_success '--contains combined with --no-contains' '
+	(
+		git init no-contains &&
+		cd no-contains &&
+		test_commit v0.1 &&
+		test_commit v0.2 &&
+		test_commit v0.3 &&
+		test_commit v0.4 &&
+		test_commit v0.5 &&
+		cat >expected <<-\EOF &&
+		v0.2
+		v0.3
+		v0.4
+		EOF
+		git tag --contains v0.2 --no-contains v0.5 >actual &&
+		test_cmp expected actual
+	)
+'
+
+# As the docs say, list tags which contain a specified *commit*. We
+# don't recurse down to tags for trees or blobs pointed to by *those*
+# commits.
+test_expect_success 'Does --[no-]contains stop at commits? Yes!' '
+	cd no-contains &&
+	blob=$(git rev-parse v0.3:v0.3.t) &&
+	tree=$(git rev-parse v0.3^{tree}) &&
+	git tag tag-blob $blob &&
+	git tag tag-tree $tree &&
+	git tag --contains v0.3 >actual &&
+	cat >expected <<-\EOF &&
+	v0.3
+	v0.4
+	v0.5
+	EOF
+	test_cmp expected actual &&
+	git tag --no-contains v0.3 >actual &&
+	cat >expected <<-\EOF &&
+	v0.1
+	v0.2
+	EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
2.11.0

[PATCH v3 15/16] ref-filter: reflow recently changed branch/tag/for-each-ref docs

[Ævar Arnfjörð Bjarmason]

Reflow the recently changed branch/tag-for-each-ref
documentation. This change shows no changes under --word-diff, except
the innocuous change of moving git-tag.txt's "[--sort=<key>]" around
slightly.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-branch.txt | 15 ++++++++-------
 Documentation/git-tag.txt    |  7 ++++---
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index e4b5d5c3e1..5e175ec339 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -10,9 +10,9 @@ SYNOPSIS
 [verse]
 'git branch' [--color[=<when>] | --no-color] [-r | -a]
 	[--list] [-v [--abbrev=<length> | --no-abbrev]]
-	[--column[=<options>] | --no-column]
+	[--column[=<options>] | --no-column] [--sort=<key>]
 	[(--merged | --no-merged) [<commit>]]
-	[--contains [<commit]] [--no-contains [<commit>]] [--sort=<key>]
+	[--contains [<commit]] [--no-contains [<commit>]]
 	[--points-at <object>] [--format=<format>] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
@@ -36,11 +36,12 @@ as branch creation.
 
 With `--contains`, shows only the branches that contain the named commit
 (in other words, the branches whose tip commits are descendants of the
-named commit), `--no-contains` inverts it. With `--merged`, only branches merged into the named
-commit (i.e. the branches whose tip commits are reachable from the named
-commit) will be listed.  With `--no-merged` only branches not merged into
-the named commit will be listed.  If the <commit> argument is missing it
-defaults to `HEAD` (i.e. the tip of the current branch).
+named commit), `--no-contains` inverts it. With `--merged`, only branches
+merged into the named commit (i.e. the branches whose tip commits are
+reachable from the named commit) will be listed.  With `--no-merged` only
+branches not merged into the named commit will be listed.  If the <commit>
+argument is missing it defaults to `HEAD` (i.e. the tip of the current
+branch).
 
 The command's second form creates a new branch head named <branchname>
 which points to the current `HEAD`, or <start-point> if given.
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 028a8ce4a9..3b018f40c8 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,9 +12,10 @@ SYNOPSIS
 'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [--contains <commit>] [--points-at <object>]
-	[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
-	[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
+'git tag' [-n[<num>]] -l [--contains <commit>] [--contains <commit>]
+	[--points-at <object>] [--column[=<options>] | --no-column]
+	[--create-reflog] [--sort=<key>] [--format=<format>]
+	[--[no-]merged [<commit>]] [<pattern>...]
 'git tag' -v [--format=<format>] <tagname>...
 
 DESCRIPTION
-- 
2.11.0

[PATCH v3 16/16] tag: add tests for --with and --without

[Ævar Arnfjörð Bjarmason]

Change the test suite to test for these synonyms for --contains and
--no-contains, respectively.

Before this change there were no tests for them at all. This doesn't
exhaustively test for them as well as their --contains and
--no-contains synonyms, but at least it's something.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t7004-tag.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8a6e8032da..6143113dbb 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1599,10 +1599,12 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
 	test_must_fail git tag --contains tag-blob &&
 	test_must_fail git tag --no-contains tag-tree &&
 	test_must_fail git tag --no-contains tag-blob &&
-	test_must_fail git tag --contains --no-contains
+	test_must_fail git tag --contains --no-contains &&
+	test_must_fail git tag --no-with HEAD &&
+	test_must_fail git tag --no-without HEAD
 '
 
-for option in --contains --no-contains --merged --no-merged --points-at
+for option in --contains --with --no-contains --without --merged --no-merged --points-at
 do
 	test_expect_success "mixing incompatible modes with $option is forbidden" "
 		test_must_fail git tag -d $option HEAD &&
-- 
2.11.0