Re: MS-Windows build of Grep [2/4]

[Jim Meyering]

Paul Eggert wrote:
> On 12/30/11 02:39, Paolo Bonzini wrote:
>> It can go
>> in lib/ rather than src/, but I disagree about splitting it in POSIX
>> and Windows files as long as the code for Windows is a superset of the
>> POSIX code.
>
> It's not a superset, e.g., the Windows code disagrees with the POSIX code
> if there is no TERM variable in the environment.  But I'm not sure I
> would agree even if it were a strict superset.  If there's sufficient
> clutter from a Windows port in the code, the clutter should be moved into
> a Windows-specific porting layer, even if the clutter merely adds to
> the mainline code.
>
> Anyway, the mild preference seems to be to make this more of a library
> and to put it under lib/* rather than src/*, so here's a revised patch
> to do that.  Unlike the previous patch, it creates an interface file
> colorize.h and separates that from the implementation in colorize-impl.c,
> as this has more of a library feel.  As before, I've tested this only
> under POSIX.
>
> This whole colorization business is a mess anyway.  It mishandles
> signals, for one thing.  All I'm trying to do here is to avoid having it
> be even messier with the new Microsoft support.
>
>
>>From 4c112598a1981787fecfad12890f6540b991f991 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <address@hidden>
> Date: Fri, 30 Dec 2011 11:53:40 -0800
> Subject: [PATCH] ms: move Microsoft-specific stuff to lib/ms
>
> * lib/colorize.c, lib/colorize.h, lib/colorize-impl.c:
> * lib/ms/colorize.h, lib/ms/colorize-impl.c: New files.
> * configure.ac (GREP_SRC_INCLUDES): New macro.
> * lib/Makefile.am (libgreputils_a_SOURCES): Add colorize.[ch].
> (EXTRA_DIST): New macro.
> * src/Makefile.am (DEFAULT_INCLUDES): New macro.
> * src/main.c: Include colorize.h.
> (PR_SGR_START, PR_SGR_END, PR_SGR_START_IF, PR_SGR_END_IF):
> Now static functions, not macros.
> (hstdout, norm_attr, w32_console_init, w32_sgr2attr)
> (w32_clreol) [__MINGW32__]: Move to lib/ms/colorize-impl.c.
> (pr_sgr_start, pr_sgr_end): Remove; callers changed to use new
> print_start_colorize, print_end_colorize from colorize.h.
> (init_colorize): Rename from w32_console_init and move to
> colorize module; caller changed.
> (should_colorize): Move to colorize module.

Thanks for the clean-up.
That looks fine except that it provokes this "make distcheck" failure:

    lib/colorize-impl.c:37:      return t && strcmp (t, "dumb") != 0;
    lib/ms/colorize-impl.c:65:      return ! (t && strcmp (t, "dumb") == 0);
    maint.mk: replace strcmp calls above with STREQ/STRNEQ

It's a little annoying that STREQ is already defined in two places
(we prefer to keep dfa.c self-contained, to accommodate gawk):

    $ git grep fine.STREQ
    src/dfa.c:#define STREQ(a, b) (strcmp (a, b) == 0)
    src/main.c:#define STREQ(a, b) (strcmp (a, b) == 0)

You may want to simply exempt those two files from that check.