Re: [PATCH] added colored print out of log levels

Re: [PATCH] added colored print out of log levels

[Sascha Hauer]

On Wed, Feb 14, 2018 at 08:26:46AM +0100, Eugen.Wiens@JUMO.net wrote:
>    Hi Sascha,
> 
>    Sascha Hauer <s.hauer@pengutronix.de> schrieb am 13.02.2018 09:37:17:
> 
>    > Von: Sascha Hauer <s.hauer@pengutronix.de>
>    > An: Eugen Wiens <eugen.wiens@jumo.net>
>    > Kopie: barebox@lists.infradead.org
>    > Datum: 13.02.2018 09:37
>    > Betreff: Re: [PATCH] added colored print out of log levels
>    >
>    > HI Eugen,
>    >
>    > On Fri, Feb 09, 2018 at 01:28:45PM +0100, Eugen Wiens wrote:
>    > > When the system is booting the warnings and errors are not be
>    > quickly discovered.
>    > > With this improvement the errors are colored red, the warnings
>    > yellow and the notices blue.
>    >
>    > I like this because I like colors ;)
>    > However, some people do not. Can we make this honour the value of the
>    > global.allow_color variable?
>    That sounds good for me.
> 
>    > This variable is currently only used in the
>    > environment, so we would have to move it to C code by adding a
>    > globalvar_add_simple_bool().
>    Where is the best place to call this function, inside the file
>    "console_common.c" or

That would be a suitable place. You could rename loglevel_init to
console_common_init for example.

>    on a more generic place. Which file is the best one to learn the handling
>    of the
>    function "globalvar_add_simple_bool"?

You pass it an int pointer which is treated as boolean variable. Setting
the globalvar on the command line directly changes the integer variable.
Other than that, grep the source, there are not so many users of that
function.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] added colored print out of log levels

[Eugen Wiens]

When the system is booting the warnings and errors are not be quickly discovered.
With this improvement the errors are colored red, the warnings yellow and the notices blue.

Signed-off-by: Eugen Wiens <eugen.wiens@jumo.net>
---
 common/Kconfig          |  8 ++++++++
 common/console_common.c | 29 ++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/common/Kconfig b/common/Kconfig
index 57418ca..461ad35 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1000,6 +1000,14 @@ config DEFAULT_LOGLEVEL
 	  7    debug-level messages (debug)
 	  8    verbose debug messages (vdebug)
 
+config PRINT_COLORED_LOG_LEVEL
+	prompt "enable colored printing of the log level"
+	bool
+	default n
+	help
+		Enable the print out of the colored log level.
+
+
 config DEBUG_INFO
 	bool
 	prompt "enable debug symbols"
diff --git a/common/console_common.c b/common/console_common.c
index d051458..19b183f 100644
--- a/common/console_common.c
+++ b/common/console_common.c
@@ -31,7 +31,21 @@
 #include <malloc.h>
 #include <asm-generic/div64.h>
 
-#ifndef CONFIG_CONSOLE_NONE
+#ifndef CONFIG_CONFIG_CONSOLE_NONE
+
+#ifdef CONFIG_PRINT_COLORED_LOG_LEVEL
+#define MAX_CHARACTERS_COLORED_LOG_LEVEL 	30
+#define MAX_LOG_LEVEL 						MSG_NOTICE
+
+static const char colored_log_level[MAX_LOG_LEVEL+2][MAX_CHARACTERS_COLORED_LOG_LEVEL] = {
+	"\033[31mEMERG:\033[0m ",		/* color red for log level MSG_EMERG */
+	"\033[31mALERT:\033[0m ",		/* color red for log level MSG_ALERT */
+	"\033[31mCRITICAL:\033[0m ",	/* color red for log level MSG_CRIT */
+	"\033[31mERROR:\033[0m ",		/* color red for log level MSG_ERR */
+	"\033[33mWARNING:\033[0m ",		/* color yellow for log level MSG_WARNING */
+	"\033[34mNOTICE:\033[0m ", 		/* color blue for log level MSG_NOTICE */
+};
+#endif
 
 int barebox_loglevel = CONFIG_DEFAULT_LOGLEVEL;
 
@@ -102,6 +116,15 @@ nolog:
 	puts(str);
 }
 
+static void print_colored_log_level(const int level)
+{
+#ifdef CONFIG_PRINT_COLORED_LOG_LEVEL
+	if (level <= MAX_LOG_LEVEL) {
+		pr_puts(level, colored_log_level[level]);
+	}
+#endif
+}
+
 int pr_print(int level, const char *fmt, ...)
 {
 	va_list args;
@@ -111,6 +134,8 @@ int pr_print(int level, const char *fmt, ...)
 	if (!IS_ENABLED(CONFIG_LOGBUF) && level > barebox_loglevel)
 		return 0;
 
+	print_colored_log_level(level);
+
 	va_start(args, fmt);
 	i = vsprintf(printbuffer, fmt, args);
 	va_end(args);
@@ -129,6 +154,8 @@ int dev_printf(int level, const struct device_d *dev, const char *format, ...)
 	if (!IS_ENABLED(CONFIG_LOGBUF) && level > barebox_loglevel)
 		return 0;
 
+	print_colored_log_level(level);
+
 	if (dev->driver && dev->driver->name)
 		ret += sprintf(printbuffer, "%s ", dev->driver->name);
 
-- 
2.7.4


Diese E-Mail kann vertrauliche und/oder rechtlich geschützte Informationen beinhalten und ist ausschließlich für die im Verteiler genannten Personen bestimmt. Das unerlaubte Kopieren sowie die unbefugte Weitergabe dieser Mail sind nicht gestattet. Bitte benachrichtigen Sie uns gegebenenfalls telefonisch oder mit Antwort-Mail, falls Sie nicht der richtige Adressat dieser E-Mail sind. Bitte löschen Sie diese Nachricht und alle Anhänge dazu unverzüglich. Falls nicht ausdrücklich vermerkt, ist diese E-Mail keine rechtlich bindende Vereinbarung. 

Kommanditgesellschaft: JUMO GmbH & Co. KG, Sitz: 36039 Fulda, Amtsgericht Fulda HRA 302, Persönlich haftende Gesellschafterin: M. K. JUCHHEIM GmbH, Sitz: 36039 Fulda, Amtsgericht Fulda HRB 17, Geschäftsführer: Dipl.-Ing. Bernhard Juchheim, Dipl.-Kfm. Michael Juchheim 
Ust.-Id.-Nr.: DE 112411234 
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] added colored print out of log levels

[Sascha Hauer]

HI Eugen,

On Fri, Feb 09, 2018 at 01:28:45PM +0100, Eugen Wiens wrote:
> When the system is booting the warnings and errors are not be quickly discovered.
> With this improvement the errors are colored red, the warnings yellow and the notices blue.

I like this because I like colors ;)
However, some people do not. Can we make this honour the value of the
global.allow_color variable? This variable is currently only used in the
environment, so we would have to move it to C code by adding a
globalvar_add_simple_bool().

> 
> Signed-off-by: Eugen Wiens <eugen.wiens@jumo.net>
> ---
>  common/Kconfig          |  8 ++++++++
>  common/console_common.c | 29 ++++++++++++++++++++++++++++-
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 57418ca..461ad35 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -1000,6 +1000,14 @@ config DEFAULT_LOGLEVEL
>  	  7    debug-level messages (debug)
>  	  8    verbose debug messages (vdebug)
>  
> +config PRINT_COLORED_LOG_LEVEL
> +	prompt "enable colored printing of the log level"
> +	bool
> +	default n
> +	help
> +		Enable the print out of the colored log level.

I don't think it's worth having an extra config option for this.
If we get concerned about the code size, then we should rather
add a global color control Kconfig option and let everything color
related depend on this.

Something like:

config CONSOLE_ALLOW_COLOR
	prompt "Allow colored console output"
	bool

static unsigned int allow_color;

bool __console_allow_color(void)
{
	return allow_color;
}

Some initcall:
	globalvar_add_simple_bool("allow_color", &allow_color);

include/:

static inline bool console_allow_color(void)
{
	if (!IS_ENABLED(CONFIG_CONSOLE_ALLOW_COLOR))
		return false;
	return __console_allow_color();
}

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox