"Philippe Blain via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Philippe Blain <levraiphilippeblain@xxxxxxxxx>
>
> Using a named enum allows casting an integer to the enum type in both
> GDB and LLDB:
>
> $ gdb -q -ex 'b wt-status.c:44' -ex r --args ./git status
> (gdb) p (enum color_wt_status) slot
> $1 = WT_STATUS_ONBRANCH
>
> $ lldb -o 'b wt-status.c:44' -o r -- ./git status
> (lldb) p (color_wt_status) slot
> (color_wt_status) $0 = WT_STATUS_ONBRANCH
>
> In LLDB, it's also required to cast in the reversed direction, i.e.
> cast an enum constant into its corresponding integer:
>
> (lldb) p (int) color_wt_status::WT_STATUS_ONBRANCH
> (int) $1 = 8
>
> Name the enum listing the different RECURSE_SUBMODULES_* modes, to make
> debugging easier. For example, when stepping through a part of the code
> where an int is compared with a constant in this enum, it allows casting
> the int to the enum type or vice-versa, after quickly checking where the
> enum constant is declared and learning the enum name.
Makes sense, and besides just making debugging easier, this would also
make code easier to read once we use the enum in more places (instead of
just int).
> As to not make this patch a debug-only change, convert the
> 'fetch_recurse' member of 'struct submodule' to use the newly named
> enum.
[...]
> submodule-config.h | 2 +-
> submodule.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/submodule-config.h b/submodule-config.h
> index 65875b94ea5..55a4c3e0bd5 100644
> --- a/submodule-config.h
> +++ b/submodule-config.h
> @@ -37,7 +37,7 @@ struct submodule {
> const char *path;
> const char *name;
> const char *url;
> - int fetch_recurse;
> + enum submodule_recurse_mode fetch_recurse;
> const char *ignore;
> const char *branch;
> struct submodule_update_strategy update_strategy;
> diff --git a/submodule.h b/submodule.h
> index 6bd2c99fd99..55cf6f01d0c 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -13,7 +13,7 @@ struct repository;
> struct string_list;
> struct strbuf;
>
> -enum {
> +enum submodule_recurse_mode {
> RECURSE_SUBMODULES_ONLY = -5,
> RECURSE_SUBMODULES_CHECK = -4,
> RECURSE_SUBMODULES_ERROR = -3,
Like Junio, I think it would be nice to use the enum in more places, but
frankly there are so many sites that would need to change that I don't
think it's reasonable for one person to change them all.
So I'm happy to take this first step and do the rest of the refactoring
incrementally. I still think the enum's values are too disjointed and
should eventually be broken up, but that's a refactoring for later.
Reviewed-by: Glen Choo <chooglen@xxxxxxxxxx>
