[{"id":16773,"web_url":"https://patchwork.libcamera.org/comment/16773/","msgid":"<YJK718XcqlJq1J8H@pendragon.ideasonboard.com>","date":"2021-05-05T15:37:59","subject":"Re: [libcamera-devel] [PATCH] libcamera: log: add colors to log\n\tlevels","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Marco,\n\nThank you for the patch.\n\nOn Wed, May 05, 2021 at 05:33:18PM +0200, Marco Felsch wrote:\n> Add colored logs if the output belongs to a terminal. This makes it easier\n> to identitify warnings and/or errors.\n> \n> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>\n> ---\n> Hi all,\n> \n> this is an reworked version of [1]. I used by sob and auther since I did\n> more changes than I kept from [1].\n> \n> [1] 20210203181746.22028-1-m.cichy@pengutronix.de\n> \n>  src/libcamera/log.cpp | 28 ++++++++++++++++++++++++++++\n>  1 file changed, 28 insertions(+)\n> \n> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> index 94175ab3..3dffd004 100644\n> --- a/src/libcamera/log.cpp\n> +++ b/src/libcamera/log.cpp\n> @@ -19,6 +19,7 @@\n>  #include <string.h>\n>  #include <syslog.h>\n>  #include <time.h>\n> +#include <unistd.h>\n>  #include <unordered_set>\n>  \n>  #include <libcamera/logging.h>\n> @@ -98,6 +99,26 @@ static const char *log_severity_name(LogSeverity severity)\n>  \t\treturn \"UNKWN\";\n>  }\n>  \n> +static const char *log_severity_color_name(LogSeverity severity)\n> +{\n> +\tstatic const char *const names[] = {\n> +\t\t\"\\033[1m\\033[37mDEBUG\\033[0m\",\n> +\t\t\"\\033[1m\\033[32m INFO\\033[0m\",\n> +\t\t\"\\033[1m\\033[33m WARN\\033[0m\",\n> +\t\t\"\\033[1m\\033[31mERROR\\033[0m\",\n> +\t\t\"\\033[1m\\033[35mFATAL\\033[0m\",\n> +\t};\n> +\n> +\t/* Only print colored output if output really belongs to a terminal */\n> +\tif (!isatty(fileno(stderr)))\n\nWhat is the stream that the logger logs to isn't stderr ?\n\n> +\t\treturn log_severity_name(severity);\n> +\n> +\tif (static_cast<unsigned int>(severity) < std::size(names))\n> +\t\treturn names[severity];\n> +\telse\n> +\t\treturn \"\\033[1m\\033[32mUNKWN\\033[0m\";\n> +}\n> +\n>  /**\n>   * \\brief Log output\n>   *\n> @@ -197,6 +218,13 @@ void LogOutput::write(const LogMessage &msg)\n>  \t\twriteSyslog(msg.severity(), str);\n>  \t\tbreak;\n>  \tcase LoggingTargetStream:\n> +\t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> +\t\t    + std::to_string(Thread::currentId()) + \"] \"\n> +\t\t    + log_severity_color_name(msg.severity()) + \" \"\n> +\t\t    + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> +\t\t    + msg.msg();\n> +\t\twriteStream(str);\n> +\t\tbreak;\n>  \tcase LoggingTargetFile:\n>  \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n>  \t\t    + std::to_string(Thread::currentId()) + \"] \"","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BF065BDE79\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  5 May 2021 15:38:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42FB5602C0;\n\tWed,  5 May 2021 17:38:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4780A602BD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 May 2021 17:38:04 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C4ABD549;\n\tWed,  5 May 2021 17:38:03 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UPGmb4EJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620229084;\n\tbh=ydYvd9PM2xAX1Gggx0bfgOPpkbAwz7275IuwVZGGbw4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UPGmb4EJBPOLVn+vpWyT8Bv75aZeHvaKyLt+UroYfAXtzJuLEbtvvBUR34B8vBZdA\n\tFuvxXwG0ZCWf/lywx/R61EoJ5AKfjPoFha1TbAlxojnW5cM9fg+gaTr+JOOM0TbhXp\n\tDc2WqWvyjZbjkEMKD0wSNof7E7nBS9dFrB0zLlto=","Date":"Wed, 5 May 2021 18:37:59 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Marco Felsch <m.felsch@pengutronix.de>","Message-ID":"<YJK718XcqlJq1J8H@pendragon.ideasonboard.com>","References":"<20210505153318.8519-1-m.felsch@pengutronix.de>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210505153318.8519-1-m.felsch@pengutronix.de>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: log: add colors to log\n\tlevels","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16775,"web_url":"https://patchwork.libcamera.org/comment/16775/","msgid":"<20210505154444.obfgbudnbzm6esg7@pengutronix.de>","date":"2021-05-05T15:44:44","subject":"Re: [libcamera-devel] [PATCH] libcamera: log: add colors to log\n\tlevels","submitter":{"id":89,"url":"https://patchwork.libcamera.org/api/people/89/","name":"Marco Felsch","email":"m.felsch@pengutronix.de"},"content":"On 21-05-05 18:37, Laurent Pinchart wrote:\n> Hi Marco,\n> \n> Thank you for the patch.\n> \n> On Wed, May 05, 2021 at 05:33:18PM +0200, Marco Felsch wrote:\n> > Add colored logs if the output belongs to a terminal. This makes it easier\n> > to identitify warnings and/or errors.\n> > \n> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>\n> > ---\n> > Hi all,\n> > \n> > this is an reworked version of [1]. I used by sob and auther since I did\n> > more changes than I kept from [1].\n> > \n> > [1] 20210203181746.22028-1-m.cichy@pengutronix.de\n> > \n> >  src/libcamera/log.cpp | 28 ++++++++++++++++++++++++++++\n> >  1 file changed, 28 insertions(+)\n> > \n> > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> > index 94175ab3..3dffd004 100644\n> > --- a/src/libcamera/log.cpp\n> > +++ b/src/libcamera/log.cpp\n> > @@ -19,6 +19,7 @@\n> >  #include <string.h>\n> >  #include <syslog.h>\n> >  #include <time.h>\n> > +#include <unistd.h>\n> >  #include <unordered_set>\n> >  \n> >  #include <libcamera/logging.h>\n> > @@ -98,6 +99,26 @@ static const char *log_severity_name(LogSeverity severity)\n> >  \t\treturn \"UNKWN\";\n> >  }\n> >  \n> > +static const char *log_severity_color_name(LogSeverity severity)\n> > +{\n> > +\tstatic const char *const names[] = {\n> > +\t\t\"\\033[1m\\033[37mDEBUG\\033[0m\",\n> > +\t\t\"\\033[1m\\033[32m INFO\\033[0m\",\n> > +\t\t\"\\033[1m\\033[33m WARN\\033[0m\",\n> > +\t\t\"\\033[1m\\033[31mERROR\\033[0m\",\n> > +\t\t\"\\033[1m\\033[35mFATAL\\033[0m\",\n> > +\t};\n> > +\n> > +\t/* Only print colored output if output really belongs to a terminal */\n> > +\tif (!isatty(fileno(stderr)))\n> \n> What is the stream that the logger logs to isn't stderr ?\n\nHi Laurent,\n\nAccording Logger::parseLogFile this can't be the case.\n\nRegards,\n  Marco\n\n> \n> > +\t\treturn log_severity_name(severity);\n> > +\n> > +\tif (static_cast<unsigned int>(severity) < std::size(names))\n> > +\t\treturn names[severity];\n> > +\telse\n> > +\t\treturn \"\\033[1m\\033[32mUNKWN\\033[0m\";\n> > +}\n> > +\n> >  /**\n> >   * \\brief Log output\n> >   *\n> > @@ -197,6 +218,13 @@ void LogOutput::write(const LogMessage &msg)\n> >  \t\twriteSyslog(msg.severity(), str);\n> >  \t\tbreak;\n> >  \tcase LoggingTargetStream:\n> > +\t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> > +\t\t    + std::to_string(Thread::currentId()) + \"] \"\n> > +\t\t    + log_severity_color_name(msg.severity()) + \" \"\n> > +\t\t    + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > +\t\t    + msg.msg();\n> > +\t\twriteStream(str);\n> > +\t\tbreak;\n> >  \tcase LoggingTargetFile:\n> >  \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> >  \t\t    + std::to_string(Thread::currentId()) + \"] \"\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B2E3BBDE7C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  5 May 2021 15:44:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C788602C0;\n\tWed,  5 May 2021 17:44:47 +0200 (CEST)","from metis.ext.pengutronix.de (metis.ext.pengutronix.de\n\t[IPv6:2001:67c:670:201:290:27ff:fe1d:cc33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7AF92602BD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 May 2021 17:44:45 +0200 (CEST)","from pty.hi.pengutronix.de ([2001:67c:670:100:1d::c5])\n\tby metis.ext.pengutronix.de with esmtps\n\t(TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92)\n\t(envelope-from <mfe@pengutronix.de>)\n\tid 1leJhd-0005fu-3w; Wed, 05 May 2021 17:44:45 +0200","from mfe by pty.hi.pengutronix.de with local (Exim 4.89)\n\t(envelope-from <mfe@pengutronix.de>)\n\tid 1leJhc-0006Bf-Na; Wed, 05 May 2021 17:44:44 +0200"],"Date":"Wed, 5 May 2021 17:44:44 +0200","From":"Marco Felsch <m.felsch@pengutronix.de>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210505154444.obfgbudnbzm6esg7@pengutronix.de>","References":"<20210505153318.8519-1-m.felsch@pengutronix.de>\n\t<YJK718XcqlJq1J8H@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YJK718XcqlJq1J8H@pendragon.ideasonboard.com>","X-Sent-From":"Pengutronix Hildesheim","X-URL":"http://www.pengutronix.de/","X-IRC":"#ptxdist @freenode","X-Accept-Language":"de,en","X-Accept-Content-Type":"text/plain","X-Uptime":"17:42:30 up 154 days, 5:48, 49 users, load average: 0.24, 0.16, \n\t0.13","User-Agent":"NeoMutt/20170113 (1.7.2)","X-SA-Exim-Connect-IP":"2001:67c:670:100:1d::c5","X-SA-Exim-Mail-From":"mfe@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.ext.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [PATCH] libcamera: log: add colors to log\n\tlevels","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16803,"web_url":"https://patchwork.libcamera.org/comment/16803/","msgid":"<70e238ad-8883-dc5c-2c9c-5b8d88d04302@ideasonboard.com>","date":"2021-05-06T10:22:37","subject":"Re: [libcamera-devel] [PATCH] libcamera: log: add colors to log\n\tlevels","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Marco,\n\nOn 05/05/2021 16:33, Marco Felsch wrote:\n> Add colored logs if the output belongs to a terminal. This makes it easier\n> to identitify warnings and/or errors.\n> \n> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>\n> ---\n> Hi all,\n> \n> this is an reworked version of [1]. I used by sob and auther since I did\n> more changes than I kept from [1].\n> \n> [1] 20210203181746.22028-1-m.cichy@pengutronix.de\n\nPersonally, I'm very pleased to see this being worked on. It's a small\ntouch but it really does help ease the eyes when trying to follow the logs.\n\nI've carried a cherry-pick of [1] on some of my branches for exactly\nthat reason.\n\n\n\n> \n>  src/libcamera/log.cpp | 28 ++++++++++++++++++++++++++++\n>  1 file changed, 28 insertions(+)\n> \n> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> index 94175ab3..3dffd004 100644\n> --- a/src/libcamera/log.cpp\n> +++ b/src/libcamera/log.cpp\n> @@ -19,6 +19,7 @@\n>  #include <string.h>\n>  #include <syslog.h>\n>  #include <time.h>\n> +#include <unistd.h>\n>  #include <unordered_set>\n>  \n>  #include <libcamera/logging.h>\n> @@ -98,6 +99,26 @@ static const char *log_severity_name(LogSeverity severity)\n>  \t\treturn \"UNKWN\";\n>  }\n>  \n> +static const char *log_severity_color_name(LogSeverity severity)\n> +{\n> +\tstatic const char *const names[] = {\n> +\t\t\"\\033[1m\\033[37mDEBUG\\033[0m\",\n> +\t\t\"\\033[1m\\033[32m INFO\\033[0m\",\n> +\t\t\"\\033[1m\\033[33m WARN\\033[0m\",\n> +\t\t\"\\033[1m\\033[31mERROR\\033[0m\",\n> +\t\t\"\\033[1m\\033[35mFATAL\\033[0m\",\n> +\t};\n\nI find this quite ... painful to read?\n\nWe're duplicating the names from log_severity_name, and we're hardcoding\nthe ansi tty colours inline which are ... well quite unreadable (to an\nuntrained eye).\n\nAt the very least those escape codes should be defined out to named\ntypes in my opinion\n\n#define TTY_CLEAR \"\\033[0m\"\nor\nCOLOUR(BLUE).\n\nA helper function might be useful, as then it could always return \"\"\ninstead of the colour in a single place if the colour was disabled, but\nthat's not helpful for the const chars :-(\n\nAt least a simple macro could potentially make this:\n\n+\tstatic const char *const names[] = {\n+\t\tCOLOUR(GREEN)  \"DEBUG\" TTY_CLEAR(),\n+\t\tCOLOUR(BLACK)  \" INFO\" TTY_CLEAR(),\n+\t\tCOLOUR(YELLOW) \" WARN\" TTY_CLEAR(),\n+\t\tCOLOUR(BLUE)   \"ERROR\" TTY_CLEAR(),\n+\t\tCOLOUR(RED)    \"FATAL\" TTY_CLEAR(),\n+\t};\n\nif the helper function isn't possible.\n\n\n> +\n> +\t/* Only print colored output if output really belongs to a terminal */\n> +\tif (!isatty(fileno(stderr)))\n> +\t\treturn log_severity_name(severity);\n> +\n> +\tif (static_cast<unsigned int>(severity) < std::size(names))\n> +\t\treturn names[severity];\n> +\telse\n> +\t\treturn \"\\033[1m\\033[32mUNKWN\\033[0m\";\n> +}\n> +\n>  /**\n>   * \\brief Log output\n>   *\n> @@ -197,6 +218,13 @@ void LogOutput::write(const LogMessage &msg)\n>  \t\twriteSyslog(msg.severity(), str);\n>  \t\tbreak;\n>  \tcase LoggingTargetStream:\n> +\t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> +\t\t    + std::to_string(Thread::currentId()) + \"] \"\n> +\t\t    + log_severity_color_name(msg.severity()) + \" \"\n> +\t\t    + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> +\t\t    + msg.msg();\n> +\t\twriteStream(str);\n> +\t\tbreak;\n>  \tcase LoggingTargetFile:\n>  \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n>  \t\t    + std::to_string(Thread::currentId()) + \"] \"\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B75B2BF81D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  6 May 2021 10:22:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 229FA6890C;\n\tThu,  6 May 2021 12:22:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27DC7602BA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  6 May 2021 12:22:52 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9F9354A5;\n\tThu,  6 May 2021 12:22:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tsOHJa0y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620296571;\n\tbh=nVmLjeevuqkmtYa8/qC+p4x1CdPOqAjMjudYNPBU9k4=;\n\th=Reply-To:To:References:From:Subject:Date:In-Reply-To:From;\n\tb=tsOHJa0yofMS3ycvuVUOGoXDlvB4/DOePAjfp1B28oW7hh8k18c0tX48LJmQlEbdS\n\tUQ8HYJVHs7csb545tWIHJTwq5bkf53NtuxWTBi6C6cB4zjWg8EvL3GxWq+WypkAYGG\n\t6KE2IzA+LNSmNc5bPYFcgckhueRuGTQeakNJeIYo=","To":"Marco Felsch <m.felsch@pengutronix.de>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210505153318.8519-1-m.felsch@pengutronix.de>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Organization":"Ideas on Board","Message-ID":"<70e238ad-8883-dc5c-2c9c-5b8d88d04302@ideasonboard.com>","Date":"Thu, 6 May 2021 11:22:37 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210505153318.8519-1-m.felsch@pengutronix.de>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: log: add colors to log\n\tlevels","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16804,"web_url":"https://patchwork.libcamera.org/comment/16804/","msgid":"<YJPFYaI/D7KoAQWV@pendragon.ideasonboard.com>","date":"2021-05-06T10:30:57","subject":"Re: [libcamera-devel] [PATCH] libcamera: log: add colors to log\n\tlevels","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Marco,\n\nOn Wed, May 05, 2021 at 05:44:44PM +0200, Marco Felsch wrote:\n> On 21-05-05 18:37, Laurent Pinchart wrote:\n> > On Wed, May 05, 2021 at 05:33:18PM +0200, Marco Felsch wrote:\n> > > Add colored logs if the output belongs to a terminal. This makes it easier\n> > > to identitify warnings and/or errors.\n> > > \n> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>\n> > > ---\n> > > Hi all,\n> > > \n> > > this is an reworked version of [1]. I used by sob and auther since I did\n> > > more changes than I kept from [1].\n> > > \n> > > [1] 20210203181746.22028-1-m.cichy@pengutronix.de\n> > > \n> > >  src/libcamera/log.cpp | 28 ++++++++++++++++++++++++++++\n> > >  1 file changed, 28 insertions(+)\n> > > \n> > > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> > > index 94175ab3..3dffd004 100644\n> > > --- a/src/libcamera/log.cpp\n> > > +++ b/src/libcamera/log.cpp\n> > > @@ -19,6 +19,7 @@\n> > >  #include <string.h>\n> > >  #include <syslog.h>\n> > >  #include <time.h>\n> > > +#include <unistd.h>\n> > >  #include <unordered_set>\n> > >  \n> > >  #include <libcamera/logging.h>\n> > > @@ -98,6 +99,26 @@ static const char *log_severity_name(LogSeverity severity)\n> > >  \t\treturn \"UNKWN\";\n> > >  }\n> > >  \n> > > +static const char *log_severity_color_name(LogSeverity severity)\n> > > +{\n> > > +\tstatic const char *const names[] = {\n> > > +\t\t\"\\033[1m\\033[37mDEBUG\\033[0m\",\n> > > +\t\t\"\\033[1m\\033[32m INFO\\033[0m\",\n> > > +\t\t\"\\033[1m\\033[33m WARN\\033[0m\",\n> > > +\t\t\"\\033[1m\\033[31mERROR\\033[0m\",\n> > > +\t\t\"\\033[1m\\033[35mFATAL\\033[0m\",\n> > > +\t};\n> > > +\n> > > +\t/* Only print colored output if output really belongs to a terminal */\n> > > +\tif (!isatty(fileno(stderr)))\n> > \n> > What is the stream that the logger logs to isn't stderr ?\n> \n> Hi Laurent,\n> \n> According Logger::parseLogFile this can't be the case.\n\nBut we also have Logger::logSetStream(), which can be used by\napplications to redirect the log output to a custom stream. It gets\ncalled by the global logSetStream() function (declared in the public\ninclude/libcamera/logging.h header).\n\n> > > +\t\treturn log_severity_name(severity);\n> > > +\n> > > +\tif (static_cast<unsigned int>(severity) < std::size(names))\n> > > +\t\treturn names[severity];\n> > > +\telse\n> > > +\t\treturn \"\\033[1m\\033[32mUNKWN\\033[0m\";\n> > > +}\n> > > +\n> > >  /**\n> > >   * \\brief Log output\n> > >   *\n> > > @@ -197,6 +218,13 @@ void LogOutput::write(const LogMessage &msg)\n> > >  \t\twriteSyslog(msg.severity(), str);\n> > >  \t\tbreak;\n> > >  \tcase LoggingTargetStream:\n> > > +\t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> > > +\t\t    + std::to_string(Thread::currentId()) + \"] \"\n> > > +\t\t    + log_severity_color_name(msg.severity()) + \" \"\n> > > +\t\t    + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > > +\t\t    + msg.msg();\n> > > +\t\twriteStream(str);\n> > > +\t\tbreak;\n> > >  \tcase LoggingTargetFile:\n> > >  \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> > >  \t\t    + std::to_string(Thread::currentId()) + \"] \"","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8B6DBBDE7F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  6 May 2021 10:31:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1088D68918;\n\tThu,  6 May 2021 12:31:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CF142602BA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  6 May 2021 12:31:02 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4D8174A5;\n\tThu,  6 May 2021 12:31:02 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZKBxDpnq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620297062;\n\tbh=IUVxXq3w1WBAA9C40gwGEmpMIHw+JW0xRAM7F4cfQrw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZKBxDpnqzwaDNXA+CIo1VE6opD19b154RLC6Pldw1x5qjk4F8LVcii0nspp7KPBGC\n\tPaMfvDJqKpb8fk68wSdEa5O9j5gDMa5lit3W2vpWyR4Rt+Goivclw+QVKtrm9/UbcE\n\tzMifSeWgNp6DX5KqTxt/3OyVN3/zjg9qQJpCEfeU=","Date":"Thu, 6 May 2021 13:30:57 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Marco Felsch <m.felsch@pengutronix.de>","Message-ID":"<YJPFYaI/D7KoAQWV@pendragon.ideasonboard.com>","References":"<20210505153318.8519-1-m.felsch@pengutronix.de>\n\t<YJK718XcqlJq1J8H@pendragon.ideasonboard.com>\n\t<20210505154444.obfgbudnbzm6esg7@pengutronix.de>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210505154444.obfgbudnbzm6esg7@pengutronix.de>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: log: add colors to log\n\tlevels","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]