[PATCH v4 0/5] extend git-describe pattern matching

[PATCH v4 0/5] extend git-describe pattern matching

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Teach git describe and git name-rev the ability to match multiple
patterns inclusively. Additionally, teach these commands to also accept
negative patterns to exclude any refs which match.

The pattern lists for positive and negative patterns are inclusive. This
means that for the positive patterns, a reference will be considered as
long as it matches at least one of the match patterns. It need not match
all given patterns. Additionally for negative patterns, we will not
consider any ref which matches any negative pattern, even if it matches
one of the positive patterns.

Together this allows the ability to express far more sets of tags than a
single match pattern alone. It does not provide quite the same depth as
would teaching full regexp but it is simpler and easy enough to
understand.

- v2
* use --exclude instead of --discard
* use modern style in tests

- v3
* fix broken test (sorry for the thrash!)

- v4
* update documentation and commit messages at Junio's suggestion
* add completion

After even more thought, I do not like the way that git-log works with
--exclude, and do not believe it gives enough extra expressive power to
bother with the complexity. The current implementation of --match and
--exclude is relatively easy to explain and makes some sense. Since we
did not have a historical reasoning in the same way that git-log does, I
do not think using something like an exclude_list or similar is worth
the added complexity.

-- interdiff since v3 --
diff --git c/Documentation/git-describe.txt w/Documentation/git-describe.txt
index 21a43b78924a..8755f3af7bcd 100644
--- c/Documentation/git-describe.txt
+++ w/Documentation/git-describe.txt
@@ -93,7 +93,9 @@ OPTIONS
 	the "refs/tags/" prefix. This can be used to narrow the tag space and
 	find only tags matching some meaningful criteria. If given multiple
 	times, a list of patterns will be accumulated and tags matching any
-	of the patterns will be excluded. Use `--no-exclude` to clear and
+	of the patterns will be excluded. When combined with --match a tag will
+	be considered when it matches at least one --match pattern and does not
+	match any of the --exclude patterns. Use `--no-exclude` to clear and
 	reset the list of patterns.
 
 --always::
diff --git c/Documentation/git-name-rev.txt w/Documentation/git-name-rev.txt
index 301b4a8d55e6..da83f280ed88 100644
--- c/Documentation/git-name-rev.txt
+++ w/Documentation/git-name-rev.txt
@@ -33,9 +33,11 @@ OPTIONS
 --exclude=<pattern>::
 	Do not use any ref whose name matches a given shell pattern. The
 	pattern can be one of branch name, tag name or fully qualified ref
-	name. If given multiple times, exclude refs that match any of the given
-	shell patterns. Use `--no-exclude` to clear the list of exclude
-	patterns.
+	name. If given multiple times, a ref will be excluded when it matches
+	any of the given patterns. When used together with --refs, a ref will
+	be used as a match only when it matches at least one --ref pattern and
+	does not match any --exclude patterns. Use `--no-exclude` to clear the
+	list of exclude patterns.
 
 --all::
 	List all commits reachable from all refs
diff --git c/Documentation/technical/api-parse-options.txt w/Documentation/technical/api-parse-options.txt
index 6914f54f5f44..36768b479e16 100644
--- c/Documentation/technical/api-parse-options.txt
+++ w/Documentation/technical/api-parse-options.txt
@@ -168,10 +168,10 @@ There are some macros to easily define options:
 	Introduce an option with string argument.
 	The string argument is put into `str_var`.
 
-`OPT_STRING_LIST(short, long, &list, arg_str, description)`::
+`OPT_STRING_LIST(short, long, &struct string_list, arg_str, description)`::
 	Introduce an option with string argument.
-	The string argument is stored as an element in `&list` which must be a
-	struct string_list. Reset the list using `--no-option`.
+	The string argument is stored as an element in `string_list`.
+	Use of `--no-option` will clear the list of preceding values.
 
 `OPT_INTEGER(short, long, &int_var, description)`::
 	Introduce an option with integer argument.
diff --git c/builtin/name-rev.c w/builtin/name-rev.c
index da4a0d7c0fdf..ea8ef48102f8 100644
--- c/builtin/name-rev.c
+++ w/builtin/name-rev.c
@@ -166,10 +166,11 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
 
 		/* See if any of the patterns match. */
 		for_each_string_list_item(item, &data->ref_filters) {
-			/*
-			 * We want to check every pattern even if we already
-			 * found a match, just in case one of the later
-			 * patterns could abbreviate the output.
+			/* Check every pattern even after we found a match so
+			 * that we can determine when we should abbreviate the
+			 * output. We will abbreviate the output when any of
+			 * the patterns match a subpath, even if one of the
+			 * patterns matches fully.
 			 */
 			switch (subpath_matches(path, item->string)) {
 			case -1: /* did not match */
diff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash
index 6721ff80fb13..835d7fcfd4f2 100644
--- c/contrib/completion/git-completion.bash
+++ w/contrib/completion/git-completion.bash
@@ -1198,6 +1198,7 @@ _git_describe ()
 		__gitcomp "
 			--all --tags --contains --abbrev= --candidates=
 			--exact-match --debug --long --match --always
+			--exclude
 			"
 		return
 	esac

Jacob Keller (5):
  doc: add documentation for OPT_STRING_LIST
  name-rev: extend --refs to accept multiple patterns
  name-rev: add support to exclude refs by pattern match
  describe: teach --match to accept multiple patterns
  describe: teach describe negative pattern matches

 Documentation/git-describe.txt                | 15 +++++++-
 Documentation/git-name-rev.txt                | 13 ++++++-
 Documentation/technical/api-parse-options.txt |  5 +++
 builtin/describe.c                            | 51 +++++++++++++++++++++----
 builtin/name-rev.c                            | 54 +++++++++++++++++++++------
 contrib/completion/git-completion.bash        |  1 +
 t/t6007-rev-list-cherry-pick-file.sh          | 38 +++++++++++++++++++
 t/t6120-describe.sh                           | 27 ++++++++++++++
 8 files changed, 183 insertions(+), 21 deletions(-)

-- 
2.11.0.488.g1cece4bcb7a5

[PATCH v4 1/5] doc: add documentation for OPT_STRING_LIST

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Commit c8ba16391655 ("parse-options: add OPT_STRING_LIST helper",
2011-06-09) added the OPT_STRING_LIST as a way to accumulate a repeated
list of strings. However, this was not documented in the
api-parse-options documentation. Add documentation now so that future
developers may learn of its existence.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/technical/api-parse-options.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 27bd701c0d68..36768b479e16 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -168,6 +168,11 @@ There are some macros to easily define options:
 	Introduce an option with string argument.
 	The string argument is put into `str_var`.
 
+`OPT_STRING_LIST(short, long, &struct string_list, arg_str, description)`::
+	Introduce an option with string argument.
+	The string argument is stored as an element in `string_list`.
+	Use of `--no-option` will clear the list of preceding values.
+
 `OPT_INTEGER(short, long, &int_var, description)`::
 	Introduce an option with integer argument.
 	The integer is put into `int_var`.
-- 
2.11.0.488.g1cece4bcb7a5

[PATCH v4 2/5] name-rev: extend --refs to accept multiple patterns

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Teach git name-rev to take multiple --refs stored as a string list of
patterns. The list of patterns will be matched inclusively, and each ref
only needs to match one pattern to be included. A ref will only be
excluded if it does not match any of the given patterns. Additionally,
if any of the patterns would allow abbreviation, then we will abbreviate
the ref, even if another pattern is more strict and would not have
allowed abbreviation on its own.

Add tests and documentation for this change. The tests expected output
is dynamically generated, but this is in order to avoid hard-coding
a commit object id in the test results (as the expected output is to
simply leave the commit object unnamed).

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/git-name-rev.txt       |  4 +++-
 builtin/name-rev.c                   | 42 +++++++++++++++++++++++++-----------
 t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++++++++++++
 3 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index ca28fb8e2a07..7433627db12d 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -26,7 +26,9 @@ OPTIONS
 
 --refs=<pattern>::
 	Only use refs whose names match a given shell pattern.  The pattern
-	can be one of branch name, tag name or fully qualified ref name.
+	can be one of branch name, tag name or fully qualified ref name. If
+	given multiple times, use refs whose names match any of the given shell
+	patterns. Use `--no-refs` to clear any previous ref patterns given.
 
 --all::
 	List all commits reachable from all refs
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index cd89d48b65e8..85897ea1f714 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -108,7 +108,7 @@ static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)
 struct name_ref_data {
 	int tags_only;
 	int name_only;
-	const char *ref_filter;
+	struct string_list ref_filters;
 };
 
 static struct tip_table {
@@ -150,16 +150,34 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
 	if (data->tags_only && !starts_with(path, "refs/tags/"))
 		return 0;
 
-	if (data->ref_filter) {
-		switch (subpath_matches(path, data->ref_filter)) {
-		case -1: /* did not match */
-			return 0;
-		case 0:  /* matched fully */
-			break;
-		default: /* matched subpath */
-			can_abbreviate_output = 1;
-			break;
+	if (data->ref_filters.nr) {
+		struct string_list_item *item;
+		int matched = 0;
+
+		/* See if any of the patterns match. */
+		for_each_string_list_item(item, &data->ref_filters) {
+			/* Check every pattern even after we found a match so
+			 * that we can determine when we should abbreviate the
+			 * output. We will abbreviate the output when any of
+			 * the patterns match a subpath, even if one of the
+			 * patterns matches fully.
+			 */
+			switch (subpath_matches(path, item->string)) {
+			case -1: /* did not match */
+				break;
+			case 0: /* matched fully */
+				matched = 1;
+				break;
+			default: /* matched subpath */
+				matched = 1;
+				can_abbreviate_output = 1;
+				break;
+			}
 		}
+
+		/* If none of the patterns matched, stop now */
+		if (!matched)
+			return 0;
 	}
 
 	add_to_tip_table(oid->hash, path, can_abbreviate_output);
@@ -306,11 +324,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 {
 	struct object_array revs = OBJECT_ARRAY_INIT;
 	int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
-	struct name_ref_data data = { 0, 0, NULL };
+	struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };
 	struct option opts[] = {
 		OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
 		OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
-		OPT_STRING(0, "refs", &data.ref_filter, N_("pattern"),
+		OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
 				   N_("only use refs matching <pattern>")),
 		OPT_GROUP(""),
 		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh
index 1408b608eb03..d9827a6389a3 100755
--- a/t/t6007-rev-list-cherry-pick-file.sh
+++ b/t/t6007-rev-list-cherry-pick-file.sh
@@ -99,6 +99,32 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '
 	test_cmp actual.named expect
 '
 
+test_expect_success 'name-rev multiple --refs combine inclusive' '
+	git rev-list --left-right --cherry-pick F...E -- bar >actual &&
+	git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \
+		<actual >actual.named &&
+	test_cmp actual.named expect
+'
+
+cat >expect <<EOF
+<tags/F
+EOF
+
+test_expect_success 'name-rev --refs excludes non-matched patterns' '
+	git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect &&
+	git rev-list --left-right --cherry-pick F...E -- bar >actual &&
+	git name-rev --stdin --name-only --refs="*tags/F" \
+		<actual >actual.named &&
+	test_cmp actual.named expect
+'
+
+test_expect_success 'name-rev --no-refs clears the refs list' '
+	git rev-list --left-right --cherry-pick F...E -- bar >expect &&
+	git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
+		<expect >actual &&
+	test_cmp actual expect
+'
+
 cat >expect <<EOF
 +tags/F
 =tags/D
-- 
2.11.0.488.g1cece4bcb7a5

Re: [PATCH v4 2/5] name-rev: extend --refs to accept multiple patterns

[Junio C Hamano]

Jacob Keller <jacob.e.keller@intel.com> writes:

> +	if (data->ref_filters.nr) {
> +		struct string_list_item *item;
> +		int matched = 0;
> +
> +		/* See if any of the patterns match. */
> +		for_each_string_list_item(item, &data->ref_filters) {
> +			/* Check every pattern even after we found a match so
> +			 * that we can determine when we should abbreviate the
> +			 * output. We will abbreviate the output when any of
> +			 * the patterns match a subpath, even if one of the
> +			 * patterns matches fully.
> +			 */

This describe "what" we do, which we can read from the code.  What I
asked you to mention was "why", which cannot be read from the code.

	/*
	 * Check all patterns even after finding a match, 
	 * so that we can see if a match with a subpath exists.
	 * When a user asked for 'refs/tags/v*' and 'v1.*', both
	 * of which match, the user is showing her willingness
	 * to accept a shortened output by having the 'v1.*' in
	 * the acceptable refnames, so we shouldn't stop when seeing
	 * 'refs/tags/v1.4' matches 'refs/tags/v*'.  We should show
	 * it as 'v1.4'.
	 */

or something like that, perhaps?

Re: [PATCH v4 2/5] name-rev: extend --refs to accept multiple patterns

[Junio C Hamano]

Jacob Keller <jacob.e.keller@intel.com> writes:

> From: Jacob Keller <jacob.keller@gmail.com>
>
> Teach git name-rev to take multiple --refs stored as a string list of
> patterns. The list of patterns will be matched inclusively, and each ref
> only needs to match one pattern to be included. A ref will only be
> excluded if it does not match any of the given patterns. Additionally,
> if any of the patterns would allow abbreviation, then we will abbreviate
> the ref, even if another pattern is more strict and would not have
> allowed abbreviation on its own.
>
> Add tests and documentation for this change. The tests expected output
> is dynamically generated, but this is in order to avoid hard-coding
> a commit object id in the test results (as the expected output is to
> simply leave the commit object unnamed).

Makes sense.  

I do not see anything that requires "... generated, but" there,
though, as if it is a bad thing to do to prepare expected output
dynamically.  I'd just reword "generated.  This is..." to make it
neutral.

Re: [PATCH v4 2/5] name-rev: extend --refs to accept multiple patterns

[Jacob Keller]

On Thu, Jan 19, 2017 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Teach git name-rev to take multiple --refs stored as a string list of
>> patterns. The list of patterns will be matched inclusively, and each ref
>> only needs to match one pattern to be included. A ref will only be
>> excluded if it does not match any of the given patterns. Additionally,
>> if any of the patterns would allow abbreviation, then we will abbreviate
>> the ref, even if another pattern is more strict and would not have
>> allowed abbreviation on its own.
>>
>> Add tests and documentation for this change. The tests expected output
>> is dynamically generated, but this is in order to avoid hard-coding
>> a commit object id in the test results (as the expected output is to
>> simply leave the commit object unnamed).
>
> Makes sense.
>
> I do not see anything that requires "... generated, but" there,
> though, as if it is a bad thing to do to prepare expected output
> dynamically.  I'd just reword "generated.  This is..." to make it
> neutral.

Makes sense. I was commenting this way since I was requested that
someone on the list preferred non-dynamic test expectations.

Thanks,
Jake

Re: [PATCH v4 2/5] name-rev: extend --refs to accept multiple patterns

[Jacob Keller]

On Thu, Jan 19, 2017 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Teach git name-rev to take multiple --refs stored as a string list of
>> patterns. The list of patterns will be matched inclusively, and each ref
>> only needs to match one pattern to be included. A ref will only be
>> excluded if it does not match any of the given patterns. Additionally,
>> if any of the patterns would allow abbreviation, then we will abbreviate
>> the ref, even if another pattern is more strict and would not have
>> allowed abbreviation on its own.
>>
>> Add tests and documentation for this change. The tests expected output
>> is dynamically generated, but this is in order to avoid hard-coding
>> a commit object id in the test results (as the expected output is to
>> simply leave the commit object unnamed).
>
> Makes sense.
>
> I do not see anything that requires "... generated, but" there,
> though, as if it is a bad thing to do to prepare expected output
> dynamically.  I'd just reword "generated.  This is..." to make it
> neutral.

To clarify my earlier comment, I think the SQUASH?? you queued looks great.

Thanks,
Jake

[PATCH v4 3/5] name-rev: add support to exclude refs by pattern match

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Extend git-name-rev to support excluding refs which match shell patterns
using --exclude. These patterns can be used to limit the scope of refs
by excluding any ref that matches one of the --exclude patterns. A ref
will only be used for naming when it matches at least one --ref pattern
but does not match any of the --exclude patterns. Thus, --exlude
patterns are given precedence over --ref patterns.

For example, suppose you wish to name a series of commits based on an
official release tag of the form "v*" but excluding any pre-release tags
which match "*rc*". You can use the following to do so:

  git name-rev --refs="v*" --exclude="*rc*" --all

Add tests and update Documentation for this change.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/git-name-rev.txt       |  9 +++++++++
 builtin/name-rev.c                   | 14 +++++++++++++-
 t/t6007-rev-list-cherry-pick-file.sh | 12 ++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index 7433627db12d..da83f280ed88 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -30,6 +30,15 @@ OPTIONS
 	given multiple times, use refs whose names match any of the given shell
 	patterns. Use `--no-refs` to clear any previous ref patterns given.
 
+--exclude=<pattern>::
+	Do not use any ref whose name matches a given shell pattern. The
+	pattern can be one of branch name, tag name or fully qualified ref
+	name. If given multiple times, a ref will be excluded when it matches
+	any of the given patterns. When used together with --refs, a ref will
+	be used as a match only when it matches at least one --ref pattern and
+	does not match any --exclude patterns. Use `--no-exclude` to clear the
+	list of exclude patterns.
+
 --all::
 	List all commits reachable from all refs
 
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 85897ea1f714..ea8ef48102f8 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -109,6 +109,7 @@ struct name_ref_data {
 	int tags_only;
 	int name_only;
 	struct string_list ref_filters;
+	struct string_list exclude_filters;
 };
 
 static struct tip_table {
@@ -150,6 +151,15 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
 	if (data->tags_only && !starts_with(path, "refs/tags/"))
 		return 0;
 
+	if (data->exclude_filters.nr) {
+		struct string_list_item *item;
+
+		for_each_string_list_item(item, &data->exclude_filters) {
+			if (subpath_matches(path, item->string) >= 0)
+				return 0;
+		}
+	}
+
 	if (data->ref_filters.nr) {
 		struct string_list_item *item;
 		int matched = 0;
@@ -324,12 +334,14 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 {
 	struct object_array revs = OBJECT_ARRAY_INIT;
 	int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
-	struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };
+	struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
 	struct option opts[] = {
 		OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
 		OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
 		OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
 				   N_("only use refs matching <pattern>")),
+		OPT_STRING_LIST(0, "exclude", &data.exclude_filters, N_("pattern"),
+				   N_("ignore refs matching <pattern>")),
 		OPT_GROUP(""),
 		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
 		OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh
index d9827a6389a3..29597451967d 100755
--- a/t/t6007-rev-list-cherry-pick-file.sh
+++ b/t/t6007-rev-list-cherry-pick-file.sh
@@ -118,6 +118,18 @@ test_expect_success 'name-rev --refs excludes non-matched patterns' '
 	test_cmp actual.named expect
 '
 
+cat >expect <<EOF
+<tags/F
+EOF
+
+test_expect_success 'name-rev --exclude excludes matched patterns' '
+	git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect &&
+	git rev-list --left-right --cherry-pick F...E -- bar >actual &&
+	git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \
+		<actual >actual.named &&
+	test_cmp actual.named expect
+'
+
 test_expect_success 'name-rev --no-refs clears the refs list' '
 	git rev-list --left-right --cherry-pick F...E -- bar >expect &&
 	git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
-- 
2.11.0.488.g1cece4bcb7a5

Re: [PATCH v4 3/5] name-rev: add support to exclude refs by pattern match

[Junio C Hamano]

Jacob Keller <jacob.e.keller@intel.com> writes:

> From: Jacob Keller <jacob.keller@gmail.com>
>
> Extend git-name-rev to support excluding refs which match shell patterns
> using --exclude. These patterns can be used to limit the scope of refs
> by excluding any ref that matches one of the --exclude patterns. A ref
> will only be used for naming when it matches at least one --ref pattern

s/--ref pattern/--refs pattern/

> but does not match any of the --exclude patterns. Thus, --exlude

s/--exlude/--exclude/

> patterns are given precedence over --ref patterns.

s/--ref pattern/--refs pattern/

No need to resend.  Thanks.

[PATCH v4 4/5] describe: teach --match to accept multiple patterns

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Teach `--match` to be accepted multiple times, accumulating a list of
patterns to match into a string list. Each pattern is inclusive, such
that a tag need only match one of the provided patterns to be
considered for matching.

This extension is useful as it enables more flexibility in what tags
match, and may avoid the need to run the describe command multiple
times to get the same result.

Add tests and update the documentation for this change.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/git-describe.txt |  5 ++++-
 builtin/describe.c             | 30 +++++++++++++++++++++++-------
 t/t6120-describe.sh            | 19 +++++++++++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e4ac448ff565..7ad41e2f6ade 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -83,7 +83,10 @@ OPTIONS
 --match <pattern>::
 	Only consider tags matching the given `glob(7)` pattern,
 	excluding the "refs/tags/" prefix.  This can be used to avoid
-	leaking private tags from the repository.
+	leaking private tags from the repository. If given multiple times, a
+	list of patterns will be accumulated, and tags matching any of the
+	patterns will be considered. Use `--no-match` to clear and reset the
+	list of patterns.
 
 --always::
 	Show uniquely abbreviated commit object as fallback.
diff --git a/builtin/describe.c b/builtin/describe.c
index 01490a157efc..5cc9e9abe798 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -28,7 +28,7 @@ static int abbrev = -1; /* unspecified */
 static int max_candidates = 10;
 static struct hashmap names;
 static int have_util;
-static const char *pattern;
+static struct string_list patterns = STRING_LIST_INIT_NODUP;
 static int always;
 static const char *dirty;
 
@@ -129,9 +129,24 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
 	if (!all && !is_tag)
 		return 0;
 
-	/* Accept only tags that match the pattern, if given */
-	if (pattern && (!is_tag || wildmatch(pattern, path + 10, 0, NULL)))
-		return 0;
+	/*
+	 * If we're given patterns, accept only tags which match at least one
+	 * pattern.
+	 */
+	if (patterns.nr) {
+		struct string_list_item *item;
+
+		if (!is_tag)
+			return 0;
+
+		for_each_string_list_item(item, &patterns) {
+			if (!wildmatch(item->string, path + 10, 0, NULL))
+				break;
+
+			/* If we get here, no pattern matched. */
+			return 0;
+		}
+	}
 
 	/* Is it annotated? */
 	if (!peel_ref(path, peeled.hash)) {
@@ -404,7 +419,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			    N_("only output exact matches"), 0),
 		OPT_INTEGER(0, "candidates", &max_candidates,
 			    N_("consider <n> most recent tags (default: 10)")),
-		OPT_STRING(0, "match",       &pattern, N_("pattern"),
+		OPT_STRING_LIST(0, "match", &patterns, N_("pattern"),
 			   N_("only consider tags matching <pattern>")),
 		OPT_BOOL(0, "always",        &always,
 			N_("show abbreviated commit object as fallback")),
@@ -430,6 +445,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		die(_("--long is incompatible with --abbrev=0"));
 
 	if (contains) {
+		struct string_list_item *item;
 		struct argv_array args;
 
 		argv_array_init(&args);
@@ -440,8 +456,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			argv_array_push(&args, "--always");
 		if (!all) {
 			argv_array_push(&args, "--tags");
-			if (pattern)
-				argv_array_pushf(&args, "--refs=refs/tags/%s", pattern);
+			for_each_string_list_item(item, &patterns)
+				argv_array_pushf(&args, "--refs=refs/tags/%s", item->string);
 		}
 		if (argc)
 			argv_array_pushv(&args, argv);
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 85f269411cb3..9e5db9b87a1f 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -182,6 +182,10 @@ check_describe "test2-lightweight-*" --tags --match="test2-*"
 
 check_describe "test2-lightweight-*" --long --tags --match="test2-*" HEAD^
 
+check_describe "test1-lightweight-*" --long --tags --match="test1-*" --match="test2-*" HEAD^
+
+check_describe "test2-lightweight-*" --long --tags --match="test1-*" --no-match --match="test2-*" HEAD^
+
 test_expect_success 'name-rev with exact tags' '
 	echo A >expect &&
 	tag_object=$(git rev-parse refs/tags/A) &&
@@ -206,4 +210,19 @@ test_expect_success 'describe --contains with the exact tags' '
 	test_cmp expect actual
 '
 
+test_expect_success 'describe --contains and --match' '
+	echo "A^0" >expect &&
+	tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+	test_must_fail git describe --contains --match="B" $tagged_commit &&
+	git describe --contains --match="B" --match="A" $tagged_commit >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'describe --contains and --no-match' '
+	echo "A^0" >expect &&
+	tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+	git describe --contains --match="B" --no-match $tagged_commit >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.11.0.488.g1cece4bcb7a5

[PATCH v4 5/5] describe: teach describe negative pattern matches

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Teach git-describe the `--exclude` option which will allow specifying
a glob pattern of tags to ignore. This can be combined with the
`--match` patterns to enable more flexibility in determining which tags
to consider.

For example, suppose you wish to find the first official release tag
that contains a certain commit. If we assume that official release tags
are of the form "v*" and pre-release candidates include "*rc*" in their
name, we can now find the first release tag that introduces the commit
abcdef:

  git describe --contains --match="v*" --exclude="*rc*" abcdef

Add documentation, tests, and completion for this change.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/git-describe.txt         | 10 ++++++++++
 builtin/describe.c                     | 21 +++++++++++++++++++++
 contrib/completion/git-completion.bash |  1 +
 t/t6120-describe.sh                    |  8 ++++++++
 4 files changed, 40 insertions(+)

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index 7ad41e2f6ade..8755f3af7bcd 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -88,6 +88,16 @@ OPTIONS
 	patterns will be considered. Use `--no-match` to clear and reset the
 	list of patterns.
 
+--exclude <pattern>::
+	Do not consider tags matching the given `glob(7)` pattern, excluding
+	the "refs/tags/" prefix. This can be used to narrow the tag space and
+	find only tags matching some meaningful criteria. If given multiple
+	times, a list of patterns will be accumulated and tags matching any
+	of the patterns will be excluded. When combined with --match a tag will
+	be considered when it matches at least one --match pattern and does not
+	match any of the --exclude patterns. Use `--no-exclude` to clear and
+	reset the list of patterns.
+
 --always::
 	Show uniquely abbreviated commit object as fallback.
 
diff --git a/builtin/describe.c b/builtin/describe.c
index 5cc9e9abe798..6769446e1f57 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -29,6 +29,7 @@ static int max_candidates = 10;
 static struct hashmap names;
 static int have_util;
 static struct string_list patterns = STRING_LIST_INIT_NODUP;
+static struct string_list exclude_patterns = STRING_LIST_INIT_NODUP;
 static int always;
 static const char *dirty;
 
@@ -129,6 +130,22 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
 	if (!all && !is_tag)
 		return 0;
 
+	/*
+	 * If we're given exclude patterns, first exclude any tag which match
+	 * any of the exclude pattern.
+	 */
+	if (exclude_patterns.nr) {
+		struct string_list_item *item;
+
+		if (!is_tag)
+			return 0;
+
+		for_each_string_list_item(item, &exclude_patterns) {
+			if (!wildmatch(item->string, path + 10, 0, NULL))
+				return 0;
+		}
+	}
+
 	/*
 	 * If we're given patterns, accept only tags which match at least one
 	 * pattern.
@@ -421,6 +438,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			    N_("consider <n> most recent tags (default: 10)")),
 		OPT_STRING_LIST(0, "match", &patterns, N_("pattern"),
 			   N_("only consider tags matching <pattern>")),
+		OPT_STRING_LIST(0, "exclude", &exclude_patterns, N_("pattern"),
+			   N_("do not consider tags matching <pattern>")),
 		OPT_BOOL(0, "always",        &always,
 			N_("show abbreviated commit object as fallback")),
 		{OPTION_STRING, 0, "dirty",  &dirty, N_("mark"),
@@ -458,6 +477,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			argv_array_push(&args, "--tags");
 			for_each_string_list_item(item, &patterns)
 				argv_array_pushf(&args, "--refs=refs/tags/%s", item->string);
+			for_each_string_list_item(item, &exclude_patterns)
+				argv_array_pushf(&args, "--exclude=refs/tags/%s", item->string);
 		}
 		if (argc)
 			argv_array_pushv(&args, argv);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6721ff80fb13..835d7fcfd4f2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1198,6 +1198,7 @@ _git_describe ()
 		__gitcomp "
 			--all --tags --contains --abbrev= --candidates=
 			--exact-match --debug --long --match --always
+			--exclude
 			"
 		return
 	esac
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 9e5db9b87a1f..167491fd5b0d 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -218,6 +218,14 @@ test_expect_success 'describe --contains and --match' '
 	test_cmp expect actual
 '
 
+test_expect_success 'describe --exclude' '
+	echo "c~1" >expect &&
+	tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+	test_must_fail git describe --contains --match="B" $tagged_commit &&
+	git describe --contains --match="?" --exclude="A" $tagged_commit >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'describe --contains and --no-match' '
 	echo "A^0" >expect &&
 	tagged_commit=$(git rev-parse "refs/tags/A^0") &&
-- 
2.11.0.488.g1cece4bcb7a5