Re: [PATCH v2 3/7] sane-ctype.h: move sane-ctype macros from git-compat-util.h

From: Jonathan Tan <jonathantanmy@google.com>
Date: May 26, 2023, 21:19 UTC
Message-ID: <20230526211911.2291886-1-jonathantanmy@google.com>
In-Reply-To: <20230523192949.1271671-3-calvinwan@google.com>

Calvin Wan <calvinwan@google.com> writes:

Show 8 quoted lines

> Splitting these macros from git-compat-util.h cleans up the file and
> allows future third-party sources to not use these overrides if they do
> not wish to.
> 
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> ---
>  git-compat-util.h | 65 +-------------------------------------------
>  sane-ctype.h      | 69 +++++++++++++++++++++++++++++++++++++++++++++++

Any specific reason for the "sane-" prefix? I think it would make more sense if it was just named ctype.h: see below.

> -/* in ctype.c, for kwset users */
> -extern const unsigned char tolower_trans_tbl[256];

I'm not sure what this has to do with sanity, but this is indeed defined in ctype.c, so it's easier to justify moving this out if the criterion was "what's in ctype.c" rather than "what's related to sane ctypes".

> -extern const signed char hexval_table[256];

And this one has nothing to do with ctypes or sanity, but rather, what's considered to be a hex character. I think we need another patch to move this to hex.h.

> -#define isxdigit(x) (hexval_table[(unsigned char)(x)] != -1)

Same for this one.

With the above suggestions, I think we do get what we want - a split between things that make ctype more sane and between other compat stuff, and also a split between those two and more platform-agnostic things like what a hex character is.