This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: color diagnostics markers


On 8 April 2013 15:23, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Apr 05, 2013 at 11:51:43PM +0200, Manuel López-Ibáñez wrote:
>> In this patch the default is "never", because for some reason "auto"
>> triggers colorization during regression testing. I have not found a
>
> That reason is obvious, dejagnu (expect?) creates pseudo terminals, so
> isatty is true, we'd need to just use -fno-diagnostics-color by default
> for the testsuite (IMHO not a big deal).
>

Fine for me.

> Anyway, I've kept the default as never for now, but am sending my review
> comments in form of a new diff, which fixes formatting, avoids memory leaks
> and changes it to introduce more color names (for caret, locus, quoted
> text), change default of note color (for some color compatibility with
> clang, bold green is there used for caret lines, for notes they use
> bold black apparently, but that doesn't work too well on white-on-black
> terminals).  Right now the patch is unfinished, because there is no support
> for the new %[locus]%s:%d:%d%[] style diagnostics strings (where
> %[locus] and %[] stand for switching to "locus" color and resetting color
> %back) in the -Wformat code (and gettext).  I'm wondering if instead of the
> %[colorname] and %[] it wouldn't be better to just have some %r or whatever
> letter isn't taken yet which would consume a const char * colorname from
> %va_arg, and some other letter with no argument that would do color reset.
> Ideas for best unused letters for that?  Perhaps then -Wformat support for
> it would be easier.  I.e. instead of:
> pp_printf ("%[locus]%s:%d:%d[]", loc.file, loc.line, loc.column);
> one would write:
> pp_printf ("%r%s:%d:%d%R", "locus", loc.file, loc.line, loc.column);

Thanks for working on this, your improvements are quite nice.

About %r versus %[colorname], I just don't see the user-case for
dynamic color names.

In fact, I would be fine with something like:

pp_start_color()
pp_stop_color()
pp_wrap_in_color("")

It is a bit more verbose, but also clearer when reading the code. And
no need for %[colorname] or %r or -Wformat support.

Cheers,

Manuel.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]