I agree with Jim that patch 1/4 is no longer needed.
Also, the first hunk of patch 3/4 should be removed
as that also has already been fixed, in a different way.
Some thoughts on patch 2/4:
> +static int is_tty;
It'd be nicer if we didn't have to introduce this static variable on
POSIX hosts, so that the non-Microsoft developer didn't have to worry
about the usual problems with global state.
More generally, it'd be nicer if the Windows-specific stuff were
segregated better, so that it's easier for Microsoft developers to
find and for non-Microsoft developers to skip.
How about something like this instead?
#include "colorize.h"
Where colorize.h looks like this:
#define PR_SGR_START(s) PR_SGR_FMT( SGR_START, (s))
#define PR_SGR_END(s) PR_SGR_FMT( SGR_END, (s))
#define PR_SGR_START_IF(s) PR_SGR_FMT_IF(SGR_START, (s))
#define PR_SGR_END_IF(s) PR_SGR_FMT_IF(SGR_END, (s))
static int
should_colorize (int fd)
{
char const *t;
return isatty (fd) && (t = getenv ("TERM")) && !STREQ (t, "dumb");
}
And there's a new file ms/colorize.h that contains
something like this:
void PR_SGR_START (char const *);
void PR_SGR_END (char const *);
void PR_SGR_START_IF (char const *);
void PR_SGR_END_IF (char const *);
int should_colorize (int);
and a new file ms/colorize.c that contains implementations of the
above. This is just a sketch; the exact interface could no doubt be
improved.
This way, the static var could be in ms/colorize.c, and the
Microsoft-specific code could all be in the ms subdirectory. When
compiling on Windows, we prepend -Ims so that its colorize.h is
included.
