Re: [PATCH] submodule.h: use a named enum for RECURSE_SUBMODULES_*

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

"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Philippe Blain <levraiphilippeblain@gmail.com>
>
> Using a named enum allows casting an integer to the enum type in both
> GDB and LLDB:
>
>     (gdb) p (enum diff_submodule_format) options->submodule_format
>     $1 = DIFF_SUBMODULE_LOG

Hmph, this was somewhat unexpected and quite disappointing.

Because that's not what those "Let's move away from #define'd
constants and use more enums" said when they sold their "enum" to
us.  In the diff_options struct, the .submodule_format member is of
type enum already, so, if we trust what they told us when they sold
their enums, "p options->submodule_format" should be enough to give
us "DIFF_SUBMODULE_LOG", not "1", as its output.  Do you really need
the cast in the above example?

> Name the enum listing the different RECURSE_SUBMODULES_* modes, to make
> debugging easier.

Even though this patch may be a good single step in the right
direction, until it is _used_ to declare a variable or a member of a
struct of that enum type, it probably is not useful as much as it
could be.  The user needs to know which enum is stored in that "int"
variable or member the user is interested in to cast it to.

That is, many changes like this one.

diff --git i/builtin/pull.c w/builtin/pull.c
index c8457619d8..f2fd4784df 100644
--- i/builtin/pull.c
+++ w/builtin/pull.c
@@ -71,7 +71,7 @@ static const char * const pull_usage[] = {
 /* Shared options */
 static int opt_verbosity;
 static char *opt_progress;
-static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static enum submodule_recurse_mode recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 
 /* Options passed to git-merge or git-rebase */
 static enum rebase_type opt_rebase = -1;