On Tue, Mar 17, 2020 at 4:40 PM Taylor Blau <me@xxxxxxxxxxxx> wrote:
> In a subsequent commit, we will add configuration options that are
> specific to each kind of object filter, in which case it is handy to
> have a function that translates between 'enum
> list_objects_filter_choice' and an appropriate configuration-friendly
> string.
> ---
Missing sign-off (but perhaps that's intentional since this is RFC).
> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> @@ -15,6 +15,31 @@ static int parse_combine_filter(
> +const char *list_object_filter_config_name(enum list_objects_filter_choice c)
> +{
> + switch (c) {
> + case LOFC_BLOB_NONE:
> + return "blob:none";
> + case LOFC_BLOB_LIMIT:
> + return "blob:limit";
> + case LOFC_TREE_DEPTH:
> + return "tree:depth";
> + case LOFC_SPARSE_OID:
> + return "sparse:oid";
> + case LOFC_COMBINE:
> + return "combine";
> + case LOFC_DISABLED:
> + case LOFC__COUNT:
> + /*
> + * Include these to catch all enumerated values, but
> + * break to treat them as a bug. Any new values of this
> + * enum will cause a compiler error, as desired.
> + */
In general, people will see a warning, not an error, unless they
specifically use -Werror (or such) to turn the warning into an error,
so this statement is misleading. Also, while some compilers may
complain, others may not. So, although the comment claims that we will
notice an unhandled enum constant at compile-time, that isn't
necessarily the case.
Moreover, the comment itself, in is present form, is rather
superfluous since its merely repeating what the BUG() invocation just
below it already tells me. In fact, as a reader of this code, I would
be more interested in knowing why those two cases do not have string
equivalents which are returned (although perhaps even that would be
obvious to someone familiar with the code, hence the comment can
probably be dropped altogether).
> + break;
> + }
> + BUG("list_object_filter_choice_name: invalid argument '%d'", c);
> +}
