[PATCH] describe: bail of --contains --all is used with --exclude or --match

[PATCH] describe: bail of --contains --all is used with --exclude or --match

[Jacob Keller]

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

If you try to use git describe --contains with --all, the exclude and
match patterns are silently ignored.

This results in unexpected behavior, as you may try to provide patterns
and expect it to change the result.

Check for this, and have describe die when it encounters this, instead
of silently ignoring the provided options.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---

I just found this while trying to use it, the patterns weren't being applied
properly.

This is pretty quick/dirty, I haven't had time to write a test, or anything.

 builtin/describe.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/describe.c b/builtin/describe.c
index 1409cedce2fb..1bf4b6e3d0ae 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -589,6 +589,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 				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);
+		} else {
+			if (patterns.nr || exclude_patterns.nr)
+				die(_("--contains with --all does not support --match or --exclude"));
 		}
 		if (argc)
 			argv_array_pushv(&args, argv);
-- 
2.18.0.219.gaf81d287a9da

Re: [PATCH] describe: bail of --contains --all is used with --exclude or --match

[Eric Sunshine]

On Tue, Feb 26, 2019 at 4:54 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
> describe: bail of --contains --all is used with --exclude or --match

s/of/if/

> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> diff --git a/builtin/describe.c b/builtin/describe.c
> @@ -589,6 +589,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
>                         for_each_string_list_item(item, &exclude_patterns)
>                                 argv_array_pushf(&args, "--exclude=refs/tags/%s", item->string);
> +               } else {
> +                       if (patterns.nr || exclude_patterns.nr)
> +                               die(_("--contains with --all does not support --match or --exclude"));
>                 }

Could be folded into an 'else if':

    } else if (patterns.nr || exclude_patterns.nr) {
        die(...);
    }

but not worth a re-roll.