[libcamera-devel] [PATCH] libcamera: log: add colors to log levels

[Laurent Pinchart]

Hi Marian,

Thank you for the patch.

On Wed, Feb 03, 2021 at 10:58:42PM +0300, Andrey Konovalov wrote:
> On 03.02.2021 22:26, Jean-Michel Hautbois wrote:
> > On 03/02/2021 20:13, Marian Cichy wrote:
> >> Hi Sebastian,
> >>
> >> thanks, I also just realized that I probably should sign-off, right?
> >>
> >> should I send a V2?

That's right. A Signed-off-by line is required as explained in
Documentation/contributing.rst.

> >> On 2/3/21 8:09 PM, Sebastian Fricke wrote:
> >>> Hey Marian,
> >>>
> >>> Thank you for the patch.
> >>> This looks better than before, I tested it on my machine and it worked
> >>> without problems.
> >>> If you like to you can add:
> >>> Tested-by: Sebastian Fricke <sebastian.fricke at posteo.net>
> >>>
> >>> Greetings,
> >>> Sebastian
> >>>
> >>> On 03.02.2021 19:17, Marian Cichy wrote:
> >>>> mono-colored wall of logs can be hard to read and doesn't show the level
> >>>> of failure at a first glance. By adding colors to the log level
> >>>> categories, it is much easier to scroll through logs and find important
> >>>> entries.
> >>>> ---
> >>>> src/libcamera/log.cpp | 10 +++++-----
> >>>> 1 file changed, 5 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
> >>>> index 45c7c2d2..3ad9c3de 100644
> >>>> --- a/src/libcamera/log.cpp
> >>>> +++ b/src/libcamera/log.cpp
> >>>> @@ -85,11 +85,11 @@ static int log_severity_to_syslog(LogSeverity
> >>>> severity)
> >>>> static const char *log_severity_name(LogSeverity severity)
> >>>> {
> >>>>      static const char *const names[] = {
> >>>> -        "DEBUG",
> >>>> -        " INFO",
> >>>> -        " WARN",
> >>>> -        "ERROR",
> >>>> -        "FATAL",
> >>>> +        "\033[1m\033[37mDEBUG\033[0m",
> >>>> +        "\033[1m\033[32m INFO\033[0m",
> >>>> +        "\033[1m\033[33m WARN\033[0m",
> >>>> +        "\033[1m\033[31mERROR\033[0m",
> >>>> +        "\033[1m\033[35mFATAL\033[0m",
> >>>>      };
> > 
> > I like the idea, but I would prefer having the possibility to
> > enable/disable it at runtime, as for copy/pasting debug logs it is
> > easier to read uncolored :-).
> 
> I second this proposal!
> Maybe something similar to GST_DEBUG_NO_COLOR in gstreamer.

This sounds good to me, but I think we should go one step further and
disable colours by default when the log target isn't a terminal.
libcamera supports 3 log targets so far:

- LoggingTargetSyslog
- LoggingTargetFile
- LoggingTargetStream

When logging to syslog or to a file, I think we should disable colours
by default. When logging to a stream, we should check if the stream
refers to a terminal, and disable colours if it doesn't. The question is
how to do so. The isatty() function is a good start, but it takes a file
descriptor, and as far as I can tell, there's no way to get a file
descriptor from an std::ostream.

-- 
Regards,

Laurent Pinchart