[iproute PATCH] lib/color: introduce freely configurable color strings

[iproute PATCH] lib/color: introduce freely configurable color strings

[Jan Engelhardt]

Implement fine-grained control over color codes for iproute, very
similar to the GCC_COLORS environment variable.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 lib/color.c   | 127 ++++++++++++++++++++++++--------------------------
 man/man8/ip.8 |  25 ++++++++--
 2 files changed, 81 insertions(+), 71 deletions(-)

diff --git a/lib/color.c b/lib/color.c
index 59976847..a11129f4 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -1,4 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdarg.h>
 #include <stdlib.h>
@@ -13,71 +14,37 @@
 
 static void set_color_palette(void);
 
-enum color {
-	C_RED,
-	C_GREEN,
-	C_YELLOW,
-	C_BLUE,
-	C_MAGENTA,
-	C_CYAN,
-	C_WHITE,
-	C_BOLD_RED,
-	C_BOLD_GREEN,
-	C_BOLD_YELLOW,
-	C_BOLD_BLUE,
-	C_BOLD_MAGENTA,
-	C_BOLD_CYAN,
-	C_BOLD_WHITE,
-	C_CLEAR
-};
-
-static const char * const color_codes[] = {
-	"\e[31m",
-	"\e[32m",
-	"\e[33m",
-	"\e[34m",
-	"\e[35m",
-	"\e[36m",
-	"\e[37m",
-	"\e[1;31m",
-	"\e[1;32m",
-	"\e[1;33m",
-	"\e[1;34m",
-	"\e[1;35m",
-	"\e[1;36m",
-	"\e[1;37m",
-	"\e[0m",
-	NULL,
-};
-
-/* light background */
-static enum color attr_colors_light[] = {
-	C_CYAN,
-	C_YELLOW,
-	C_MAGENTA,
-	C_BLUE,
-	C_GREEN,
-	C_RED,
+enum {
+	C_IFACE,
+	C_LLADDR,
+	C_V4ADDR,
+	C_V6ADDR,
+	C_OPERUP,
+	C_OPERDN,
 	C_CLEAR,
+	C_MAX,
 };
 
-/* dark background */
-static enum color attr_colors_dark[] = {
-	C_BOLD_CYAN,
-	C_BOLD_YELLOW,
-	C_BOLD_MAGENTA,
-	C_BOLD_BLUE,
-	C_BOLD_GREEN,
-	C_BOLD_RED,
-	C_CLEAR
+static const char default_colors_for_black[] =
+	"iface=36:lladdr=33:v4addr=35:v6addr=34:operup=32:operdn=31:clear=0";
+static const char default_colors_for_white[] =
+	"iface=1;36:lladdr=1;33:v4addr=1;35:v6addr=1;34:operup=1;32:operdn=1;31:clear=0";
+static struct color_code {
+	const char match[8], *code;
+	int len;
+} color_codes[C_MAX] = {
+	{"iface="}, {"lladdr="}, {"v4addr="}, {"v6addr="}, {"operup="},
+	{"operdn="}, {"clear=", "0", 1},
 };
 
-static int is_dark_bg;
 static int color_is_enabled;
 
 static void enable_color(void)
 {
-	color_is_enabled = 1;
+	/* Without $TERM analysis by terminfo, the next best option is...: */
+	const char *s = getenv("COLORTERM"), *s2 = getenv("COLORFGBG");
+	color_is_enabled = (s != NULL && strtoul(s, NULL, 0) != 0) ||
+	                   (s2 != NULL && *s2 != '\0');
 	set_color_palette();
 }
 
@@ -121,17 +88,43 @@ bool matches_color(const char *arg, int *val)
 
 static void set_color_palette(void)
 {
-	char *p = getenv("COLORFGBG");
+	const char *initstr = default_colors_for_black;
+	const char *p = getenv("COLORFGBG");
 
 	/*
 	 * COLORFGBG environment variable usually contains either two or three
 	 * values separated by semicolons; we want the last value in either case.
-	 * If this value is 0-6 or 8, background is dark.
+	 * If this value is 7, background is bright.
 	 */
-	if (p && (p = strrchr(p, ';')) != NULL
-		&& ((p[1] >= '0' && p[1] <= '6') || p[1] == '8')
-		&& p[2] == '\0')
-		is_dark_bg = 1;
+	if (p && (p = strrchr(p, ';')) != NULL && (p[1] == '7' && p[2] == '\0'))
+		initstr = default_colors_for_white;
+	p = getenv("IPROUTE_COLORS");
+	if (p != NULL)
+		initstr = p;
+
+	for (p = initstr; *p != '\0'; ) {
+		unsigned int key = C_MAX;
+		const char *code = NULL;
+		for (size_t j = 0; j < ARRAY_SIZE(color_codes); ++j) {
+			if (strncmp(p, color_codes[j].match,
+			    strlen(color_codes[j].match)) != 0)
+				continue;
+			key = j;
+			code = p + strlen(color_codes[j].match);
+			break;
+		}
+
+		const char *next = strchr(p, ':');
+		if (next == NULL)
+			next = p + strlen(p);
+		if (key != C_MAX) {
+			color_codes[key].code = code;
+			color_codes[key].len  = next - code;
+		}
+		p = next;
+		if (*next != '\0')
+			++p;
+	}
 }
 
 __attribute__((format(printf, 3, 4)))
@@ -147,11 +140,13 @@ int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...)
 		goto end;
 	}
 
-	ret += fprintf(fp, "%s", color_codes[is_dark_bg ?
-		attr_colors_dark[attr] : attr_colors_light[attr]]);
-
+	const struct color_code *k = &color_codes[attr];
+	if (k->code != NULL && *k->code != '\0')
+		ret += fprintf(fp, "\e[%.*sm", k->len, k->code);
 	ret += vfprintf(fp, fmt, args);
-	ret += fprintf(fp, "%s", color_codes[C_CLEAR]);
+	k = &color_codes[C_CLEAR];
+	if (k->code != NULL && *k->code != '\0')
+		ret += fprintf(fp, "\e[%.*sm", k->len, k->code);
 
 end:
 	va_end(args);
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index c9f7671e..90efeadf 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -199,8 +199,7 @@ precedence. This flag is ignored if
 .B \-json
 is also given.
 
-Used color palette can be influenced by
-.BR COLORFGBG
+The used color palette can be influenced by the \fBIPROUTE_COLORS\fP
 environment variable
 (see
 .BR ENVIRONMENT ).
@@ -359,15 +358,31 @@ or, if the objects of this class cannot be listed,
 .SH ENVIRONMENT
 .TP
 .B COLORFGBG
-If set, it's value is used for detection whether background is dark or
-light and use contrast colors for it.
+If set, its value is used to detect whether the background is dark or
+light and to select a different palette for IPROUTE_COLORS.
 
-COLORFGBG environment variable usually contains either two or three
+The COLORFGBG environment variable usually contains either two or three
 values separated by semicolons; we want the last value in either case.
 If this value is 0-6 or 8, chose colors suitable for dark background:
 
 COLORFGBG=";0" ip -c a
+.TP
+\fBCOLORTERM\fP
+If set, its value is used to determine whether to enable color when
+--color=auto is in effect. iproute does not otherwise make use of terminfo and
+as such does not evaluate the TERM environment variable.
+.TP
+\fBIPROUTE_COLORS\fP
+Its value is a colon-separated list of capabilities and Select Graphic
+Rendition (SGR) substrings. SGR commands are interpreted by the terminal or
+terminal emulator. (See the section in the documentation of your text terminal
+for permitted values and their meanings as character attributes. The
+console_codes(4) manpage gives an overview of ECMA codes.) These substring
+values are integers in decimal representation and can be concatenated with
+semicolons.
 
+Default:
+\fIiface=36:lladdr=33:v4addr=35:v6addr=34:operup=32:operdn=31:clear=0\fP
 .SH EXIT STATUS
 Exit status is 0 if command was successful, and 1 if there is a syntax error.
 If an error was reported by the kernel exit status is 2.
-- 
2.28.0

Re: [iproute PATCH] lib/color: introduce freely configurable color strings

[Stephen Hemminger]

On Mon, 12 Oct 2020 16:50:21 +0200
Jan Engelhardt <jengelh@inai.de> wrote:

> Implement fine-grained control over color codes for iproute, very
> similar to the GCC_COLORS environment variable.
> 
> Signed-off-by: Jan Engelhardt <jengelh@inai.de>
> ---
>  lib/color.c   | 127 ++++++++++++++++++++++++--------------------------
>  man/man8/ip.8 |  25 ++++++++--
>  2 files changed, 81 insertions(+), 71 deletions(-)

I like the idea a lot.

But there are some style issues.
Checkpatch sees:

WARNING: Missing a blank line after declarations
#165: FILE: lib/color.c:46:
+	const char *s = getenv("COLORTERM"), *s2 = getenv("COLORFGBG");
+	color_is_enabled = (s != NULL && strtoul(s, NULL, 0) != 0) ||

ERROR: code indent should use tabs where possible
#166: FILE: lib/color.c:47:
+^I                   (s2 != NULL && *s2 != '\0');$

ERROR: do not use assignment in if condition
#188: FILE: lib/color.c:99:
+	if (p && (p = strrchr(p, ';')) != NULL && (p[1] == '7' && p[2] == '\0'))

WARNING: Missing a blank line after declarations
#197: FILE: lib/color.c:108:
+		const char *code = NULL;
+		for (size_t j = 0; j < ARRAY_SIZE(color_codes); ++j) {

WARNING: Missing a blank line after declarations
#207: FILE: lib/color.c:118:
+		const char *next = strchr(p, ':');
+		if (next == NULL)

WARNING: Missing a blank line after declarations
#228: FILE: lib/color.c:144:
+	const struct color_code *k = &color_codes[attr];
+	if (k->code != NULL && *k->code != '\0')


Also, please don't mix declarations in code like:

+		for (size_t j = 0; j < ARRAY_SIZE(color_codes); ++j) {

Iproute2 tries to stick to the kernel coding style, and loop variables
like this are allowed in that standard.