Re: [PATCH][mingw] Enable colorized diagnostics

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2017-10-09 at 11:01 +0000, JonY wrote:
> On 10/08/2017 11:39 AM, Liu Hao wrote:
> > On 2017/9/28 4:09, Joseph Myers wrote:
> > > On Thu, 28 Sep 2017, Liu Hao wrote:
> > > 
> > > > Colorized diagnostics used to be disabled for MinGW targets (on
> > > > which
> > > > the macro `_WIN32` is defined), and this patch enables it.
> > > 
> > > I'd hope this is all to do with MinGW host, and nothing to do
> > > with the
> > > target.
> > > 
> > 
> > Ping? Are there any more opinions about this?
> > 
> 
> I'm not sure if it should be enabled by default due to the
> interleaving
> problem, but seeing as the user has to go out to set GCC_COLORS to
> use
> this feature, I suppose it is OK.
> 
> I will commit soon if there are no more comments.

I have some concerns about adding non-trivial host-specific code to the
diagnostics subsystem.

I occasionally make changes to the files you're touching, but I don't
have access to the host in question, so I can't test that I don't break
things on MinGW.

Is it OK if this is the MinGW team's problem, and not mine?  (i.e. can
you please clean up after me if I break something on MinGW?).

Also, you might want to add some selftests to the code e.g. for
find_esc_head and find_esc_terminator.  I looked at the docs for the
Windows console API and unfortunately there doesn't seem to be a way to
set up a dummy console for unit-testing the parts of the code that call
the console API directly.  But find_esc_head and find_esc_terminator
don't directly call the API, and hence you can write some selftest
functions for them.  (there's probably a much more involved way to test
this using mocks/stubs for the console API, but that's probably
overkill).

Dave