Re: [PATCH] describe: fix matching to actually match all patterns

[Jacob Keller <jacob.keller@gmail.com>]

On Fri, Sep 15, 2017 at 10:53 PM, Max Kirillov <max@max630.net> wrote:
> `git describe --match` with multiple patterns matches only first pattern.
> If it fails, next patterns are not tried.
>
> Fix it, add test cases and update existing test which has wrong
> expectation.
>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
>  builtin/describe.c  | 9 ++++++---
>  t/t6120-describe.sh | 6 +++++-
>  2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/describe.c b/builtin/describe.c
> index 89ea1cdd60..94ff2fba0b 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -155,18 +155,21 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
>          * pattern.
>          */
>         if (patterns.nr) {
> +               int found = 0;
>                 struct string_list_item *item;
>
>                 if (!is_tag)
>                         return 0;
>
>                 for_each_string_list_item(item, &patterns) {
> -                       if (!wildmatch(item->string, path + 10, 0))
> +                       if (!wildmatch(item->string, path + 10, 0)) {
> +                               found = 1;
>                                 break;

I see what was wrong. The "if we got here" check is inside the loop,
so after the first wildmatch we never loop again. The fix is to add an
additional variable to store when we found something and exit the
loop, and ensure that we actually did the whole loop without finding a
match.

Thanks for the fix and proper tests!

Regards,
Jake


> +                       }
> +               }
>
> -                       /* If we get here, no pattern matched. */
> +               if (!found)
>                         return 0;
> -               }
>         }
>
>         /* Is it annotated? */
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index aa74eb8f0d..25110ea55d 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -182,10 +182,14 @@ 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-*" --match="test2-*" HEAD^
>
>  check_describe "test2-lightweight-*" --long --tags --match="test1-*" --no-match --match="test2-*" HEAD^
>
> +check_describe "test1-lightweight-*" --long --tags --match="test1-*" --match="test3-*" HEAD
> +
> +check_describe "test1-lightweight-*" --long --tags --match="test3-*" --match="test1-*" HEAD
> +
>  test_expect_success 'name-rev with exact tags' '
>         echo A >expect &&
>         tag_object=$(git rev-parse refs/tags/A) &&
> --
> 2.11.0.1122.gc3fec58.dirty
>