| Message ID | 20230915210700.83077-1-stephen@networkplumber.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next] allow overriding color option in environment | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
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 >
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) {
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(-)