On Fri, Feb 10, 2023 at 10:20 PM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
>
>
> On Fri, Feb 10 2023, Masahiro Yamada wrote:
>
> > git-compat-util.h implements most of is*() macros.
> >
> > Add isblank() and isgraph(), which are useful to clean up wildmatch.c
> > in a consistent way (in this and later commits).
>
> You are on a journey to fix wildmatch.c, so...
>
> > The same issue already exists for isspace().
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > ---
> > [...]
> > -#ifdef isblank
> > -# define ISBLANK(c) (ISASCII(c) && isblank(c))
> > -#else
> > -# define ISBLANK(c) ((c) == ' ' || (c) == '\t')
> > -#endif
> > -
> > -#ifdef isgraph
> > -# define ISGRAPH(c) (ISASCII(c) && isgraph(c))
> > -#else
> > -# define ISGRAPH(c) (ISASCII(c) && isprint(c) && !isspace(c))
> > -#endif
> > -
> > +#define ISBLANK(c) (ISASCII(c) && isblank(c))
> > +#define ISGRAPH(c) (ISASCII(c) && isgraph(c))
> > #define ISPRINT(c) (ISASCII(c) && isprint(c))
> > #define ISDIGIT(c) (ISASCII(c) && isdigit(c))
> > #define ISALNUM(c) (ISASCII(c) && isalnum(c))
>
> You make this change, but not others in tree.
>
> Personally I wouldn't mind seeing this expanded to fix the various other
I wouldn't mind seeing somebody use this for tree-wide cleanups.
I do not see any reason to do that in this patch set, either.
> trivially convertable cases in-tree, e.g.:
>
> $ git -P grep -n "(' '|'\\\t').*\|\|.*(' '|'\\\t')"
> builtin/am.c:602: if (*sb.buf == '\t' || *sb.buf == ' ')
> compat/regex/regex_internal.h:60:# define isblank(ch) ((ch) == ' ' || (ch) == '\t')
> compat/regex/regex_internal.h:73: return (c == ' ' || c == '\t');
> config.c:893: while (c == ' ' || c == '\t')
> fsck.c:837: while (*p == ' ' || *p == '\t')
> mailinfo.c:749: (line->buf[0] == ' ' || line->buf[0] == '\t')) {
> sequencer.c:2476: (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || !*p) &&
> sequencer.c:2536: (*bol == ' ' || *bol == '\t')) {
> sequencer.c:2540: (*bol == ' ' || *bol == '\t')) {
> sequencer.c:2594: if (is_command(TODO_PICK, &bol) && (*bol == ' ' || *bol == '\t'))
> sequencer.c:2597: (*bol == ' ' || *bol == '\t'))
> t/helper/test-json-writer.c:443: if (c == '\n' || c == '\r' || c == ' ' || c == '\t')
> t/helper/test-json-writer.c:449: while (*buf == ' ' || *buf == '\t')
> t/t4256/1/mailinfo.c:737: (line->buf[0] == ' ' || line->buf[0] == '\t')) {
> t/t4256/1/mailinfo.c.orig:726: (line->buf[0] == ' ' || line->buf[0] == '\t')) {
> trailer.c:630: if (c != line && (*c == ' ' || *c == '\t')) {
> wildmatch.c:34:# define ISBLANK(c) ((c) == ' ' || (c) == '\t')
>
> Some of those are false positves, but most are not.
--
Best Regards
Masahiro Yamada
