Re: [PATCH v2 2/3] ref-filter: add new "describe" atom

Junio C Hamano
2023-07-15T19:00:32Z

Kousik Sanagavarapu <five231003@gmail.com> writes:

> conveys it better or is it too much unnecessary stuff to and should we
> just do
>
> struct {
> const char **describe_args;
> } describe;
>
> leaving the describe_opts array as is and changing the how the switch is
> written.

I think this struct can be replaced with a single

const char **describe_args;

and then

>> > +static int describe_atom_parser(struct ref_format *format UNUSED,
>> > + struct used_atom *atom,
>> > + const char *arg, struct strbuf *err)
>> > +{
>> > + const char *describe_opts[] = {
>> > + "",
>> > + "tags",
>> > + "abbrev",
>> > + "match",
>> > + "exclude",
>> > + NULL
>> > + };

this array can simply go away. Then you can

>> > + struct strvec args = STRVEC_INIT;
>> > + for (;;) {
>> > + int found = 0;
>> > + const char *argval;
>> > + size_t arglen = 0;
>> > + int optval = 0;
>> > + int opt;
>> > +
>> > + if (!arg)
>> > + break;
>> > +
>> > + for (opt = D_BARE; !found && describe_opts[opt]; opt++) {

rewrite this "for" loop plus the "switch" inside to an if/else
if/else cascade:

if (match_atom_bool_arg(arg, "tags", &arg, &optval)) {
... do "tags" thing ...
} else if (match_atom_arg_value(arg, "abbrev", ...)) {
... do "abbrev" thing ...
} else if ...

That way, you do not need any enum anywhere and there is no reason
to have desribe_opts[] array, either.