* [PATCH iproute2-next] allow overriding color option in environment
@ 2023-09-15 21:07 Stephen Hemminger
2023-09-16 13:11 ` Andrea Claudi
0 siblings, 1 reply; 2+ messages in thread
From: Stephen Hemminger @ 2023-09-15 21:07 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
For ip, tc, and bridge command introduce a new way to enable
automatic colorization via environment variable. The default
is what ever is in the config.
Example:
$ IP_COLOR=auto ip -br show addr
The idea is that distributions can ship with the same default
as before "none" but that users can override if needed
without resorting to aliasing every command.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
bridge/bridge.c | 2 +-
include/color.h | 1 +
ip/ip.c | 2 +-
lib/color.c | 36 +++++++++++++++++++++++++++---------
man/man8/bridge.8 | 7 +++++++
man/man8/ip.8 | 14 +++++++++-----
man/man8/tc.8 | 6 ++++++
tc/tc.c | 2 +-
8 files changed, 53 insertions(+), 17 deletions(-)
diff --git a/bridge/bridge.c b/bridge/bridge.c
index 339101a874b1..f9a245cb3670 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -102,7 +102,7 @@ static int batch(const char *name)
int
main(int argc, char **argv)
{
- int color = CONF_COLOR;
+ int color = default_color("BRIDGE_COLOR");
while (argc > 1) {
const char *opt = argv[1];
diff --git a/include/color.h b/include/color.h
index 17ec56f3d7b4..8eea534f38e1 100644
--- a/include/color.h
+++ b/include/color.h
@@ -20,6 +20,7 @@ enum color_opt {
COLOR_OPT_ALWAYS = 2
};
+int default_color(const char *argv0);
bool check_enable_color(int color, int json);
bool matches_color(const char *arg, int *val);
int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...);
diff --git a/ip/ip.c b/ip/ip.c
index 860ff957c3b3..e15d5fe52d92 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -168,7 +168,7 @@ int main(int argc, char **argv)
const char *libbpf_version;
char *batch_file = NULL;
char *basename;
- int color = CONF_COLOR;
+ int color = default_color("IP_COLOR");
/* to run vrf exec without root, capabilities might be set, drop them
* if not needed as the first thing.
diff --git a/lib/color.c b/lib/color.c
index 59976847295c..9a579f34977d 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -93,6 +93,32 @@ bool check_enable_color(int color, int json)
return false;
}
+static bool match_color_value(const char *arg, int *val)
+{
+ if (*arg == '\0' || !strcmp(arg, "always"))
+ *val = COLOR_OPT_ALWAYS;
+ else if (!strcmp(arg, "auto"))
+ *val = COLOR_OPT_AUTO;
+ else if (!strcmp(arg, "never"))
+ *val = COLOR_OPT_NEVER;
+ else
+ return false;
+ return true;
+}
+
+int default_color(const char *env)
+{
+ char *name;
+ int val;
+ size_t i;
+
+ name = getenv(env);
+ if (name && match_color_value(name, &val))
+ return val;
+
+ return CONF_COLOR;
+}
+
bool matches_color(const char *arg, int *val)
{
char *dup, *p;
@@ -108,15 +134,7 @@ bool matches_color(const char *arg, int *val)
if (matches(dup, "-color"))
return false;
- if (*p == '\0' || !strcmp(p, "always"))
- *val = COLOR_OPT_ALWAYS;
- else if (!strcmp(p, "auto"))
- *val = COLOR_OPT_AUTO;
- else if (!strcmp(p, "never"))
- *val = COLOR_OPT_NEVER;
- else
- return false;
- return true;
+ return match_color_value(p, val);
}
static void set_color_palette(void)
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index c52c9331e2c2..58bb1ddbd26a 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -319,6 +319,13 @@ precedence. This flag is ignored if
.B \-json
is also given.
+
+The default color setting is
+.B never
+but can be overridden by the
+.B BRIDGE_COLOR
+environment variable.
+
.TP
.BR "\-j", " \-json"
Output results in JavaScript Object Notation (JSON).
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 72227d44fd30..df572f47d96d 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -197,11 +197,15 @@ precedence. This flag is ignored if
.B \-json
is also given.
-Used color palette can be influenced by
-.BR COLORFGBG
-environment variable
-(see
-.BR ENVIRONMENT ).
+The default color setting is
+.B never
+but can be overridden by the
+.B IP_COLOR
+environment variable.
+
+The color palette used can be adjusted with
+.B COLORFGBG
+environment variable.
.TP
.BR "\-t" , " \-timestamp"
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index d436d46472af..39ac6dcd1631 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -805,6 +805,12 @@ precedence. This flag is ignored if
.B \-json
is also given.
+The default color setting is
+.B never
+but can be overridden by the
+.B TC_COLOR
+environment variable.
+
.TP
.BR "\-j", " \-json"
Display results in JSON format.
diff --git a/tc/tc.c b/tc/tc.c
index 082c6677d34a..b7cd60d68a38 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -253,7 +253,7 @@ int main(int argc, char **argv)
{
const char *libbpf_version;
char *batch_file = NULL;
- int color = CONF_COLOR;
+ int color = default_color("TC_COLOR");
int ret;
while (argc > 1) {
--
2.39.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH iproute2-next] allow overriding color option in environment
2023-09-15 21:07 [PATCH iproute2-next] allow overriding color option in environment Stephen Hemminger
@ 2023-09-16 13:11 ` Andrea Claudi
0 siblings, 0 replies; 2+ messages in thread
From: Andrea Claudi @ 2023-09-16 13:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
On Fri, Sep 15, 2023 at 02:07:00PM -0700, Stephen Hemminger wrote:
> For ip, tc, and bridge command introduce a new way to enable
> automatic colorization via environment variable. The default
> is what ever is in the config.
>
> Example:
> $ IP_COLOR=auto ip -br show addr
>
> The idea is that distributions can ship with the same default
> as before "none" but that users can override if needed
> without resorting to aliasing every command.
>
I'm OK with this, but can we please use a single environment variable to
control color output for all iproute tools? I can't see why anyone would
want to have color output on by default for ip and not for tc and
bridge...
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> bridge/bridge.c | 2 +-
> include/color.h | 1 +
> ip/ip.c | 2 +-
> lib/color.c | 36 +++++++++++++++++++++++++++---------
> man/man8/bridge.8 | 7 +++++++
> man/man8/ip.8 | 14 +++++++++-----
> man/man8/tc.8 | 6 ++++++
> tc/tc.c | 2 +-
> 8 files changed, 53 insertions(+), 17 deletions(-)
>
> diff --git a/bridge/bridge.c b/bridge/bridge.c
> index 339101a874b1..f9a245cb3670 100644
> --- a/bridge/bridge.c
> +++ b/bridge/bridge.c
> @@ -102,7 +102,7 @@ static int batch(const char *name)
> int
> main(int argc, char **argv)
> {
> - int color = CONF_COLOR;
> + int color = default_color("BRIDGE_COLOR");
>
> while (argc > 1) {
> const char *opt = argv[1];
> diff --git a/include/color.h b/include/color.h
> index 17ec56f3d7b4..8eea534f38e1 100644
> --- a/include/color.h
> +++ b/include/color.h
> @@ -20,6 +20,7 @@ enum color_opt {
> COLOR_OPT_ALWAYS = 2
> };
>
> +int default_color(const char *argv0);
> bool check_enable_color(int color, int json);
> bool matches_color(const char *arg, int *val);
> int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...);
> diff --git a/ip/ip.c b/ip/ip.c
> index 860ff957c3b3..e15d5fe52d92 100644
> --- a/ip/ip.c
> +++ b/ip/ip.c
> @@ -168,7 +168,7 @@ int main(int argc, char **argv)
> const char *libbpf_version;
> char *batch_file = NULL;
> char *basename;
> - int color = CONF_COLOR;
> + int color = default_color("IP_COLOR");
>
> /* to run vrf exec without root, capabilities might be set, drop them
> * if not needed as the first thing.
> diff --git a/lib/color.c b/lib/color.c
> index 59976847295c..9a579f34977d 100644
> --- a/lib/color.c
> +++ b/lib/color.c
> @@ -93,6 +93,32 @@ bool check_enable_color(int color, int json)
> return false;
> }
>
> +static bool match_color_value(const char *arg, int *val)
> +{
> + if (*arg == '\0' || !strcmp(arg, "always"))
This actually produces an unexpected result.
IP_COLOR= ip address
results in colorized output, while it should be colorless, in my
opinion. However this works ok when this code path is exercised by the
'--color' option, as with:
ip -c address
users actually expect to have color on the output.
> + *val = COLOR_OPT_ALWAYS;
> + else if (!strcmp(arg, "auto"))
> + *val = COLOR_OPT_AUTO;
> + else if (!strcmp(arg, "never"))
> + *val = COLOR_OPT_NEVER;
> + else
> + return false;
> + return true;
> +}
> +
> +int default_color(const char *env)
> +{
> + char *name;
> + int val;
> + size_t i;
> +
You may want to get rid of the i var above:
color.c: In function ‘default_color’:
color.c:113:16: warning: unused variable ‘i’ [-Wunused-variable]
113 | size_t i;
| ^
> + name = getenv(env);
> + if (name && match_color_value(name, &val))
> + return val;
> +
> + return CONF_COLOR;
> +}
> +
> bool matches_color(const char *arg, int *val)
> {
> char *dup, *p;
> @@ -108,15 +134,7 @@ bool matches_color(const char *arg, int *val)
> if (matches(dup, "-color"))
> return false;
>
> - if (*p == '\0' || !strcmp(p, "always"))
> - *val = COLOR_OPT_ALWAYS;
> - else if (!strcmp(p, "auto"))
> - *val = COLOR_OPT_AUTO;
> - else if (!strcmp(p, "never"))
> - *val = COLOR_OPT_NEVER;
> - else
> - return false;
> - return true;
> + return match_color_value(p, val);
> }
>
> static void set_color_palette(void)
> diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
> index c52c9331e2c2..58bb1ddbd26a 100644
> --- a/man/man8/bridge.8
> +++ b/man/man8/bridge.8
> @@ -319,6 +319,13 @@ precedence. This flag is ignored if
> .B \-json
> is also given.
>
> +
> +The default color setting is
> +.B never
> +but can be overridden by the
> +.B BRIDGE_COLOR
> +environment variable.
> +
> .TP
> .BR "\-j", " \-json"
> Output results in JavaScript Object Notation (JSON).
> diff --git a/man/man8/ip.8 b/man/man8/ip.8
> index 72227d44fd30..df572f47d96d 100644
> --- a/man/man8/ip.8
> +++ b/man/man8/ip.8
> @@ -197,11 +197,15 @@ precedence. This flag is ignored if
> .B \-json
> is also given.
>
> -Used color palette can be influenced by
> -.BR COLORFGBG
> -environment variable
> -(see
> -.BR ENVIRONMENT ).
> +The default color setting is
> +.B never
> +but can be overridden by the
> +.B IP_COLOR
> +environment variable.
> +
> +The color palette used can be adjusted with
> +.B COLORFGBG
> +environment variable.
>
> .TP
> .BR "\-t" , " \-timestamp"
> diff --git a/man/man8/tc.8 b/man/man8/tc.8
> index d436d46472af..39ac6dcd1631 100644
> --- a/man/man8/tc.8
> +++ b/man/man8/tc.8
> @@ -805,6 +805,12 @@ precedence. This flag is ignored if
> .B \-json
> is also given.
>
> +The default color setting is
> +.B never
> +but can be overridden by the
> +.B TC_COLOR
> +environment variable.
> +
> .TP
> .BR "\-j", " \-json"
> Display results in JSON format.
> diff --git a/tc/tc.c b/tc/tc.c
> index 082c6677d34a..b7cd60d68a38 100644
> --- a/tc/tc.c
> +++ b/tc/tc.c
> @@ -253,7 +253,7 @@ int main(int argc, char **argv)
> {
> const char *libbpf_version;
> char *batch_file = NULL;
> - int color = CONF_COLOR;
> + int color = default_color("TC_COLOR");
> int ret;
>
> while (argc > 1) {
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-09-16 13:11 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-15 21:07 [PATCH iproute2-next] allow overriding color option in environment Stephen Hemminger
2023-09-16 13:11 ` Andrea Claudi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).