Re: [PATCH 02/10] parse-options.[ch]: consistently use "enum parse_opt_flags"

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Tue, Sep 28 2021, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>
>>> Use the "enum parse_opt_flags" instead of an "int flags" as arguments
>>> to the various functions in parse-options.c. This will help to catch
>>> cases where we're not handling cases in switch statements, and

 ...

> But I think this is perfectly good use of enums, we use this
> enums-as-bitfields pattern in various other places,
> e.g. builtin/rebase.c's "flags", the "commit_graph_write_flags",
> "expire_reflog_flags" & "todo_item_flags", just to name a few from some
> quick grepping.

Many codepaths already misusing is not an excuse to add another ;-)

But ...

> One advantage is that integrates nicely with some wider C
> tooling. E.g. before this series, starting "git stash show" under gdb
> and inspecting flags:
>
>     (gdb) p flags
>     $1 = 9
>
> And after:
>
>     (gdb) p flags
>     $1 = (PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_UNKNOWN)

... this is a pleasant surprise---the last time I checked, debuggers
were that clever to notice that the distinct values are names for
individual bits.  I can buy this as an argument for using enums for
names for individual bits.  For this to work, obviously, variable
and struct members need to be given the appropriate type.

So I agree with the change in 2/10.

Except that one place there was a change related to a different enum
that is a true enumeration in this step.  It belongs to 3/10, I
think.  Also, the sales pitch for this step in the proposed commit
log message needs rewriting---this will "not" help to catch cases
where we're not handling cases in switch statements; if you are
selling it because you think it will help debuggers and other
tooling, let's describe it as such.

Even though I think debuggers are overrated ;-)

Thanks.