Re: [PATCH v3 5/5] describe: teach describe negative pattern matches

2017-01-18 Thread Junio C Hamano
Jacob Keller  writes:

> From: Jacob Keller 
>
> 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 tag that introduces commit abcdef via:
>
>   git describe --contains --match="v*" --exclude="*rc*"
>
> Add documentation and tests for this change.
>
> Signed-off-by: Jacob Keller 
> ---

The above is much better than 3/5 with a concrete example (compared
to the vague "certain kinds of references").  It also does not have
the "we check this first and then that" ;-).

> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
> index 7ad41e2f6ade..21a43b78924a 100644
> --- a/Documentation/git-describe.txt
> +++ b/Documentation/git-describe.txt
> @@ -88,6 +88,14 @@ OPTIONS
>   patterns will be considered. Use `--no-match` to clear and reset the
>   list of patterns.
>  
> +--exclude ::
> + 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. Use `--no-exclude` to clear and
> + reset the list of patterns.
> +

Similar to 3/5, perhaps we want to say something about interaction
between this one and --match?



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

2017-01-17 Thread Jacob Keller
From: Jacob Keller 

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 tag that introduces commit abcdef via:

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

Add documentation and tests for this change.

Signed-off-by: Jacob Keller 
---
 Documentation/git-describe.txt |  8 
 builtin/describe.c | 21 +
 t/t6120-describe.sh|  8 
 3 files changed, 37 insertions(+)

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index 7ad41e2f6ade..21a43b78924a 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -88,6 +88,14 @@ OPTIONS
patterns will be considered. Use `--no-match` to clear and reset the
list of patterns.
 
+--exclude ::
+   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. 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;
 
@@ -130,6 +131,22 @@ static int get_name(const char *path, const struct 
object_id *oid, int flag, voi
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, _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  most recent tags (default: 10)")),
OPT_STRING_LIST(0, "match", , N_("pattern"),
   N_("only consider tags matching ")),
+   OPT_STRING_LIST(0, "exclude", _patterns, N_("pattern"),
+  N_("do not consider tags matching ")),
OPT_BOOL(0, "always",,
N_("show abbreviated commit object as fallback")),
{OPTION_STRING, 0, "dirty",  , N_("mark"),
@@ -458,6 +477,8 @@ int cmd_describe(int argc, const char **argv, const char 
*prefix)
argv_array_push(, "--tags");
for_each_string_list_item(item, )
argv_array_pushf(, "--refs=refs/tags/%s", 
item->string);
+   for_each_string_list_item(item, _patterns)
+   argv_array_pushf(, 
"--exclude=refs/tags/%s", item->string);
}
if (argc)
argv_array_pushv(, argv);
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.403.g196674b8396b