All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Emily Shaffer <emilyshaffer@google.com>,
	Glen Choo <chooglen@google.com>,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Philippe Blain <levraiphilippeblain@gmail.com>
Subject: [PATCH v2] submodule.h: use a named enum for RECURSE_SUBMODULES_*
Date: Mon, 04 Apr 2022 17:10:11 +0000	[thread overview]
Message-ID: <pull.1111.v2.git.1649092211419.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1111.git.1641410782015.gitgitgadget@gmail.com>

From: Philippe Blain <levraiphilippeblain@gmail.com>

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.

As to not make this patch a debug-only change, convert the
'fetch_recurse' member of 'struct submodule' to use the newly named
enum.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
    submodule.h: use a named enum for RECURSE_SUBMODULES_*
    
    Changes since v1:
    
     * converted one user of the enum from an 'int' to the new enum type, so
       the patch is not debug-only, as suggested by Glen.
     * updated the commit message to use an 'int' as example of a local
       variable being compared with an enum constant, instead of using a
       struct member which is already of enum type, as pointed out by Junio
     * added a little bit more explanation in the commit message as to how
       this patch can help when debugging.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1111%2Fphil-blain%2Fsubmodule-recurse-enum-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1111/phil-blain/submodule-recurse-enum-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1111

Range-diff vs v1:

 1:  4c787d4054e ! 1:  e0b211b88f5 submodule.h: use a named enum for RECURSE_SUBMODULES_*
     @@ Commit message
          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
     +        $ 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) p (diff_submodule_format) options->submodule_format
     -        (diff_submodule_format) $1 = DIFF_SUBMODULE_LOG
     +        $ 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) diff_submodule_format::DIFF_SUBMODULE_SHORT
     -        (int) $0 = 0
     +        (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.
     +    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.
     +
     +    As to not make this patch a debug-only change, convert the
     +    'fetch_recurse' member of 'struct submodule' to use the newly named
     +    enum.
      
          Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
      
     + ## submodule-config.h ##
     +@@ submodule-config.h: 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;
     +
       ## submodule.h ##
      @@ submodule.h: struct repository;
       struct string_list;


 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,

base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907
-- 
gitgitgadget

  parent reply	other threads:[~2022-04-04 21:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05 19:26 [PATCH] submodule.h: use a named enum for RECURSE_SUBMODULES_* Philippe Blain via GitGitGadget
2022-01-05 21:20 ` Junio C Hamano
2022-01-07 13:27   ` Philippe Blain
2022-01-07 17:43     ` Glen Choo
2022-01-18 18:20   ` Glen Choo
2022-04-04 17:10 ` Philippe Blain via GitGitGadget [this message]
2022-04-04 17:52   ` [PATCH v2] " Glen Choo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1111.v2.git.1649092211419.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=chooglen@google.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.