list-objects-filter-options: support 'auto' mode for --filter

In a following commit, we are going to allow passing "auto" as a
<filterspec> to the `--filter=<filterspec>` option, but only for some
commands. Other commands that support the `--filter=<filterspec>`
option should still die() when 'auto' is passed.

Let's set up the "list-objects-filter-options.{c,h}" infrastructure to
support that:

- Add a new `unsigned int allow_auto_filter : 1;` flag to
  `struct list_objects_filter_options` which specifies if "auto" is
  accepted or not.
- Change gently_parse_list_objects_filter() to parse "auto" if it's
  accepted.
- Make sure we die() if "auto" is combined with another filter.
- Update list_objects_filter_release() to preserve the
  allow_auto_filter flag, as this function is often called (via
  opt_parse_list_objects_filter) to reset the struct before parsing a
  new value.

Let's also update `list-objects-filter.c` to recognize the new
`LOFC_AUTO` choice. Since "auto" must be resolved to a concrete filter
before filtering actually begins, initializing a filter with
`LOFC_AUTO` is invalid and will trigger a BUG().

Note that ideally combining "auto" with "auto" could be allowed, but in
practice, it's probably not worth the added code complexity. And if we
really want it, nothing prevents us to allow it in future work.

If we ever want to give a meaning to combining "auto" with a different
filter too, nothing prevents us to do that in future work either.

While at it, let's add a new "u-list-objects-filter-options.c" file for
`struct list_objects_filter_options` related unit tests. For now it
only tests gently_parse_list_objects_filter() though.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Makefile b/Makefile
index 89d8d73ec0a21be02ae8d23122598c7497fde085..85b2ff09f4da6ab8fc38eda55245de2d386145df 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -1507,6 +1507,7 @@ CLAR_TEST_SUITES += u-dir
 CLAR_TEST_SUITES += u-example-decorate
 CLAR_TEST_SUITES += u-hash
 CLAR_TEST_SUITES += u-hashmap
+CLAR_TEST_SUITES += u-list-objects-filter-options
 CLAR_TEST_SUITES += u-mem-pool
 CLAR_TEST_SUITES += u-oid-array
 CLAR_TEST_SUITES += u-oidmap

diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 7420bf81fe0103196f46e5850077f239a46c8e8d..f13ae5caeb6d21fa8c0e56110e3f001af2c4c2b0 100644 (file)
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -20,6 +20,8 @@ const char *list_object_filter_config_name(enum list_objects_filter_choice c)
        case LOFC_DISABLED:
                /* we have no name for "no filter at all" */
                break;
+       case LOFC_AUTO:
+               return "auto";
        case LOFC_BLOB_NONE:
                return "blob:none";
        case LOFC_BLOB_LIMIT:
@@ -52,7 +54,17 @@ int gently_parse_list_objects_filter(
        if (filter_options->choice)
                BUG("filter_options already populated");
 
-       if (!strcmp(arg, "blob:none")) {
+       if (!strcmp(arg, "auto")) {
+               if (!filter_options->allow_auto_filter) {
+                       strbuf_addstr(
+                               errbuf,
+                               _("'auto' filter not supported by this command"));
+                       return 1;
+               }
+               filter_options->choice = LOFC_AUTO;
+               return 0;
+
+       } else if (!strcmp(arg, "blob:none")) {
                filter_options->choice = LOFC_BLOB_NONE;
                return 0;
 
@@ -146,10 +158,20 @@ static int parse_combine_subfilter(
 
        decoded = url_percent_decode(subspec->buf);
 
-       result = has_reserved_character(subspec, errbuf) ||
-               gently_parse_list_objects_filter(
+       result = has_reserved_character(subspec, errbuf);
+       if (result)
+               goto cleanup;
+
+       result = gently_parse_list_objects_filter(
                        &filter_options->sub[new_index], decoded, errbuf);
+       if (result)
+               goto cleanup;
+
+       result = (filter_options->sub[new_index].choice == LOFC_AUTO);
+       if (result)
+               strbuf_addstr(errbuf, _("an 'auto' filter cannot be combined"));
 
+cleanup:
        free(decoded);
        return result;
 }
@@ -263,6 +285,9 @@ void parse_list_objects_filter(
        } else {
                struct list_objects_filter_options *sub;
 
+               if (filter_options->choice == LOFC_AUTO)
+                       die(_("an 'auto' filter is incompatible with any other filter"));
+
                /*
                 * Make filter_options an LOFC_COMBINE spec so we can trivially
                 * add subspecs to it.
@@ -277,6 +302,9 @@ void parse_list_objects_filter(
                if (gently_parse_list_objects_filter(sub, arg, &errbuf))
                        die("%s", errbuf.buf);
 
+               if (sub->choice == LOFC_AUTO)
+                       die(_("an 'auto' filter is incompatible with any other filter"));
+
                strbuf_addch(&filter_options->filter_spec, '+');
                filter_spec_append_urlencode(filter_options, arg);
        }
@@ -317,6 +345,7 @@ void list_objects_filter_release(
        struct list_objects_filter_options *filter_options)
 {
        size_t sub;
+       unsigned int allow_auto_filter = filter_options->allow_auto_filter;
 
        if (!filter_options)
                return;
@@ -326,6 +355,7 @@ void list_objects_filter_release(
                list_objects_filter_release(&filter_options->sub[sub]);
        free(filter_options->sub);
        list_objects_filter_init(filter_options);
+       filter_options->allow_auto_filter = allow_auto_filter;
 }
 
 void partial_clone_register(

diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 7b2108b98662c9937eec96dbe4d8bef97e291158..77d7bbc846e1c1bbe5f2ca11fb119c0cf25b136d 100644 (file)
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -18,6 +18,7 @@ enum list_objects_filter_choice {
        LOFC_SPARSE_OID,
        LOFC_OBJECT_TYPE,
        LOFC_COMBINE,
+       LOFC_AUTO,
        LOFC__COUNT /* must be last */
 };
 
@@ -50,6 +51,11 @@ struct list_objects_filter_options {
         */
        unsigned int no_filter : 1;
 
+       /*
+        * Is LOFC_AUTO a valid option?
+        */
+       unsigned int allow_auto_filter : 1;
+
        /*
         * BEGIN choice-specific parsed values from within the filter-spec. Only
         * some values will be defined for any given choice.

diff --git a/list-objects-filter.c b/list-objects-filter.c
index acd65ebb734523461730952ed01e9dd36d82dfee..78316e7f90c8d4969e169948d9b508e6ca37484c 100644 (file)
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -745,6 +745,13 @@ static void filter_combine__init(
        filter->finalize_omits_fn = filter_combine__finalize_omits;
 }
 
+static void filter_auto__init(
+       struct list_objects_filter_options *filter_options UNUSED,
+       struct filter *filter UNUSED)
+{
+       BUG("LOFC_AUTO should have been resolved before initializing the filter");
+}
+
 typedef void (*filter_init_fn)(
        struct list_objects_filter_options *filter_options,
        struct filter *filter);
@@ -760,6 +767,7 @@ static filter_init_fn s_filters[] = {
        filter_sparse_oid__init,
        filter_object_type__init,
        filter_combine__init,
+       filter_auto__init,
 };
 
 struct filter *list_objects_filter__init(

diff --git a/t/meson.build b/t/meson.build
index 459c52a48972e441e9325616940f5bca13b9ec83..0bd66cc6cea5d71ad802205b0c553b38c875ad9e 100644 (file)
--- a/t/meson.build
+++ b/t/meson.build
@@ -4,6 +4,7 @@ clar_test_suites = [
   'unit-tests/u-example-decorate.c',
   'unit-tests/u-hash.c',
   'unit-tests/u-hashmap.c',
+  'unit-tests/u-list-objects-filter-options.c',
   'unit-tests/u-mem-pool.c',
   'unit-tests/u-oid-array.c',
   'unit-tests/u-oidmap.c',

diff --git a/t/unit-tests/u-list-objects-filter-options.c b/t/unit-tests/u-list-objects-filter-options.c
new file mode 100644 (file)
index 0000000..f7d7370
--- /dev/null
+++ b/t/unit-tests/u-list-objects-filter-options.c
@@ -0,0 +1,53 @@
+#include "unit-test.h"
+#include "list-objects-filter-options.h"
+#include "strbuf.h"
+
+/* Helper to test gently_parse_list_objects_filter() */
+static void check_gentle_parse(const char *filter_spec,
+                              int expect_success,
+                              int allow_auto,
+                              enum list_objects_filter_choice expected_choice)
+{
+       struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
+       struct strbuf errbuf = STRBUF_INIT;
+       int ret;
+
+       filter_options.allow_auto_filter = allow_auto;
+
+       ret = gently_parse_list_objects_filter(&filter_options, filter_spec, &errbuf);
+
+       if (expect_success) {
+               cl_assert_equal_i(ret, 0);
+               cl_assert_equal_i(expected_choice, filter_options.choice);
+               cl_assert_equal_i(errbuf.len, 0);
+       } else {
+               cl_assert(ret != 0);
+               cl_assert(errbuf.len > 0);
+       }
+
+       strbuf_release(&errbuf);
+       list_objects_filter_release(&filter_options);
+}
+
+void test_list_objects_filter_options__regular_filters(void)
+{
+       check_gentle_parse("blob:none", 1, 0, LOFC_BLOB_NONE);
+       check_gentle_parse("blob:none", 1, 1, LOFC_BLOB_NONE);
+       check_gentle_parse("blob:limit=5k", 1, 0, LOFC_BLOB_LIMIT);
+       check_gentle_parse("blob:limit=5k", 1, 1, LOFC_BLOB_LIMIT);
+       check_gentle_parse("combine:blob:none+tree:0", 1, 0, LOFC_COMBINE);
+       check_gentle_parse("combine:blob:none+tree:0", 1, 1, LOFC_COMBINE);
+}
+
+void test_list_objects_filter_options__auto_allowed(void)
+{
+       check_gentle_parse("auto", 1, 1, LOFC_AUTO);
+       check_gentle_parse("auto", 0, 0, 0);
+}
+
+void test_list_objects_filter_options__combine_auto_fails(void)
+{
+       check_gentle_parse("combine:auto+blob:none", 0, 1, 0);
+       check_gentle_parse("combine:blob:none+auto", 0, 1, 0);
+       check_gentle_parse("combine:auto+auto", 0, 1, 0);
+}