[iproute PATCH v4] Make colored output configurable

[iproute PATCH v4] Make colored output configurable

[Phil Sutter]

Allow for -color={never,auto,always} to have colored output disabled,
enabled only if stdout is a terminal or enabled regardless of stdout
state.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Allow to override isatty() check by specifying '-color' flag more than
  once.
- Document new behaviour in man pages.

Changes since v2:
- Implement new -color=foo syntax.
- Update commit message and man page texts accordingly.

Changes since v3:
- Fix typo in tc/tc.c causing compile error.
---
 bridge/bridge.c   |  3 +--
 include/color.h   |  7 +++++++
 ip/ip.c           |  3 +--
 lib/color.c       | 33 ++++++++++++++++++++++++++++++++-
 man/man8/bridge.8 | 13 +++++++++++--
 man/man8/ip.8     | 13 +++++++++++--
 man/man8/tc.8     | 13 +++++++++++--
 tc/tc.c           |  3 +--
 8 files changed, 75 insertions(+), 13 deletions(-)

diff --git a/bridge/bridge.c b/bridge/bridge.c
index 451d684e0bcfd..e35e5bdf7fb30 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -173,8 +173,7 @@ main(int argc, char **argv)
 			NEXT_ARG();
 			if (netns_switch(argv[1]))
 				exit(-1);
-		} else if (matches(opt, "-color") == 0) {
-			++color;
+		} else if (matches_color(opt, &color) == 0) {
 		} else if (matches(opt, "-compressvlans") == 0) {
 			++compress_vlans;
 		} else if (matches(opt, "-force") == 0) {
diff --git a/include/color.h b/include/color.h
index 4f2c918db7e43..42038dc2e7f87 100644
--- a/include/color.h
+++ b/include/color.h
@@ -12,8 +12,15 @@ enum color_attr {
 	COLOR_NONE
 };
 
+enum color_opt {
+	COLOR_OPT_NEVER = 0,
+	COLOR_OPT_AUTO = 1,
+	COLOR_OPT_ALWAYS = 2
+};
+
 void enable_color(void);
 int check_enable_color(int color, int json);
+int matches_color(const char *arg, int *val);
 void set_color_palette(void);
 int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...);
 enum color_attr ifa_family_color(__u8 ifa_family);
diff --git a/ip/ip.c b/ip/ip.c
index 38eac5ec1e17d..893c3c43ef99a 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -283,8 +283,7 @@ int main(int argc, char **argv)
 				exit(-1);
 			}
 			rcvbuf = size;
-		} else if (matches(opt, "-color") == 0) {
-			++color;
+		} else if (matches_color(opt, &color) == 0) {
 		} else if (matches(opt, "-help") == 0) {
 			usage();
 		} else if (matches(opt, "-netns") == 0) {
diff --git a/lib/color.c b/lib/color.c
index edf96e5c6ecd7..3ad1d6d647722 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -3,11 +3,13 @@
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <linux/if.h>
 
 #include "color.h"
+#include "utils.h"
 
 enum color {
 	C_RED,
@@ -79,13 +81,42 @@ void enable_color(void)
 
 int check_enable_color(int color, int json)
 {
-	if (color && !json) {
+	if (json || color == COLOR_OPT_NEVER)
+		return 1;
+
+	if (color == COLOR_OPT_ALWAYS || isatty(fileno(stdout))) {
 		enable_color();
 		return 0;
 	}
 	return 1;
 }
 
+int matches_color(const char *arg, int *val)
+{
+	char *dup, *p;
+
+	if (!val)
+		return 1;
+
+	dup = strdupa(arg);
+	p = strchrnul(dup, '=');
+	if (*p)
+		*(p++) = '\0';
+
+	if (matches(dup, "-color"))
+		return 1;
+
+	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 1;
+	return 0;
+}
+
 void set_color_palette(void)
 {
 	char *p = getenv("COLORFGBG");
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index f6d228c5ebfe7..6dfd4178a19cc 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -171,8 +171,17 @@ If there were any errors during execution of the commands, the application
 return code will be non zero.
 
 .TP
-.BR "\-c" , " -color"
-Use color output.
+.BR \-c [ color ][ = { always | auto | never }
+Configure color output. If parameter is omitted or
+.BR always ,
+color output is enabled regardless of stdout state. If parameter is
+.BR auto ,
+stdout is checked to be a terminal before enabling color output. If parameter is
+.BR never ,
+color output is disabled. If specified multiple times, the last one takes
+precedence. This flag is ignored if
+.B \-json
+is also given.
 
 .TP
 .BR "\-j", " \-json"
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 0087d18b74706..1d358879ec39c 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -187,8 +187,17 @@ to
 executes specified command over all objects, it depends if command supports this option.
 
 .TP
-.BR "\-c" , " -color"
-Use color output.
+.BR \-c [ color ][ = { always | auto | never }
+Configure color output. If parameter is omitted or
+.BR always ,
+color output is enabled regardless of stdout state. If parameter is
+.BR auto ,
+stdout is checked to be a terminal before enabling color output. If parameter is
+.BR never ,
+color output is disabled. If specified multiple times, the last one takes
+precedence. This flag is ignored if
+.B \-json
+is also given.
 
 .TP
 .BR "\-t" , " \-timestamp"
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index fd33f9b2f1806..f98398a34bd3e 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -755,8 +755,17 @@ option was specified. Classes can be filtered only by
 option.
 
 .TP
-.BR "\ -color"
-Use color output.
+.BR \-c [ color ][ = { always | auto | never }
+Configure color output. If parameter is omitted or
+.BR always ,
+color output is enabled regardless of stdout state. If parameter is
+.BR auto ,
+stdout is checked to be a terminal before enabling color output. If parameter is
+.BR never ,
+color output is disabled. If specified multiple times, the last one takes
+precedence. This flag is ignored if
+.B \-json
+is also given.
 
 .TP
 .BR "\-j", " \-json"
diff --git a/tc/tc.c b/tc/tc.c
index 4c7a128c8103e..c634eff86b6b5 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -496,8 +496,7 @@ int main(int argc, char **argv)
 				matches(argv[1], "-conf") == 0) {
 			NEXT_ARG();
 			conf_file = argv[1];
-		} else if (matches(argv[1], "-color") == 0) {
-			++color;
+		} else if (matches_color(argv[1], &color) == 0) {
 		} else if (matches(argv[1], "-timestamp") == 0) {
 			timestamp++;
 		} else if (matches(argv[1], "-tshort") == 0) {
-- 
2.18.0

Re: [iproute PATCH v4] Make colored output configurable

[David Ahern]

On 8/16/18 3:37 AM, Phil Sutter wrote:
> Allow for -color={never,auto,always} to have colored output disabled,
> enabled only if stdout is a terminal or enabled regardless of stdout
> state.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Allow to override isatty() check by specifying '-color' flag more than
>   once.
> - Document new behaviour in man pages.
> 
> Changes since v2:
> - Implement new -color=foo syntax.
> - Update commit message and man page texts accordingly.
> 
> Changes since v3:
> - Fix typo in tc/tc.c causing compile error.
> ---
>  bridge/bridge.c   |  3 +--
>  include/color.h   |  7 +++++++
>  ip/ip.c           |  3 +--
>  lib/color.c       | 33 ++++++++++++++++++++++++++++++++-
>  man/man8/bridge.8 | 13 +++++++++++--
>  man/man8/ip.8     | 13 +++++++++++--
>  man/man8/tc.8     | 13 +++++++++++--
>  tc/tc.c           |  3 +--
>  8 files changed, 75 insertions(+), 13 deletions(-)
> 
> diff --git a/bridge/bridge.c b/bridge/bridge.c
> index 451d684e0bcfd..e35e5bdf7fb30 100644
> --- a/bridge/bridge.c
> +++ b/bridge/bridge.c
> @@ -173,8 +173,7 @@ main(int argc, char **argv)
>  			NEXT_ARG();
>  			if (netns_switch(argv[1]))
>  				exit(-1);
> -		} else if (matches(opt, "-color") == 0) {
> -			++color;
> +		} else if (matches_color(opt, &color) == 0) {
>  		} else if (matches(opt, "-compressvlans") == 0) {
>  			++compress_vlans;
>  		} else if (matches(opt, "-force") == 0) {
> diff --git a/include/color.h b/include/color.h
> index 4f2c918db7e43..42038dc2e7f87 100644
> --- a/include/color.h
> +++ b/include/color.h
> @@ -12,8 +12,15 @@ enum color_attr {
>  	COLOR_NONE
>  };
>  
> +enum color_opt {
> +	COLOR_OPT_NEVER = 0,
> +	COLOR_OPT_AUTO = 1,
> +	COLOR_OPT_ALWAYS = 2
> +};

The order of AUTO and ALWAYS is backwards. Existing users who do
something like:

    ip -c addr list | less -R
or
    ip -c addr list > /tmp/addr
    less -R /tmp/addr

should not be affected by this change. That is an existing command that
works and should continue to work the same after this change.

Users who add -c but don't want the codes if stdout is not a tty are the
ones who should be doing something new - be it adding another -c or
using -c=auto.

Re: [iproute PATCH v4] Make colored output configurable

[Phil Sutter]

On Thu, Aug 16, 2018 at 07:06:07AM -0600, David Ahern wrote:
> On 8/16/18 3:37 AM, Phil Sutter wrote:
> > Allow for -color={never,auto,always} to have colored output disabled,
> > enabled only if stdout is a terminal or enabled regardless of stdout
> > state.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > Changes since v1:
> > - Allow to override isatty() check by specifying '-color' flag more than
> >   once.
> > - Document new behaviour in man pages.
> > 
> > Changes since v2:
> > - Implement new -color=foo syntax.
> > - Update commit message and man page texts accordingly.
> > 
> > Changes since v3:
> > - Fix typo in tc/tc.c causing compile error.
> > ---
> >  bridge/bridge.c   |  3 +--
> >  include/color.h   |  7 +++++++
> >  ip/ip.c           |  3 +--
> >  lib/color.c       | 33 ++++++++++++++++++++++++++++++++-
> >  man/man8/bridge.8 | 13 +++++++++++--
> >  man/man8/ip.8     | 13 +++++++++++--
> >  man/man8/tc.8     | 13 +++++++++++--
> >  tc/tc.c           |  3 +--
> >  8 files changed, 75 insertions(+), 13 deletions(-)
> > 
> > diff --git a/bridge/bridge.c b/bridge/bridge.c
> > index 451d684e0bcfd..e35e5bdf7fb30 100644
> > --- a/bridge/bridge.c
> > +++ b/bridge/bridge.c
> > @@ -173,8 +173,7 @@ main(int argc, char **argv)
> >  			NEXT_ARG();
> >  			if (netns_switch(argv[1]))
> >  				exit(-1);
> > -		} else if (matches(opt, "-color") == 0) {
> > -			++color;
> > +		} else if (matches_color(opt, &color) == 0) {
> >  		} else if (matches(opt, "-compressvlans") == 0) {
> >  			++compress_vlans;
> >  		} else if (matches(opt, "-force") == 0) {
> > diff --git a/include/color.h b/include/color.h
> > index 4f2c918db7e43..42038dc2e7f87 100644
> > --- a/include/color.h
> > +++ b/include/color.h
> > @@ -12,8 +12,15 @@ enum color_attr {
> >  	COLOR_NONE
> >  };
> >  
> > +enum color_opt {
> > +	COLOR_OPT_NEVER = 0,
> > +	COLOR_OPT_AUTO = 1,
> > +	COLOR_OPT_ALWAYS = 2
> > +};
> 
> The order of AUTO and ALWAYS is backwards. Existing users who do
> something like:

Ordering color_opt this way felt natural since the default (0)
automatically matches the existing default (no colors).

>     ip -c addr list | less -R
> or
>     ip -c addr list > /tmp/addr
>     less -R /tmp/addr
> 
> should not be affected by this change. That is an existing command that
> works and should continue to work the same after this change.
> 
> Users who add -c but don't want the codes if stdout is not a tty are the
> ones who should be doing something new - be it adding another -c or
> using -c=auto.

My code is correct in that regard: Plain -c[olor] is equivalent to
-color=always.

Cheers, Phil

Re: [iproute PATCH v4] Make colored output configurable

[Stephen Hemminger]

On Thu, 16 Aug 2018 11:37:03 +0200
Phil Sutter <phil@nwl.cc> wrote:

> Allow for -color={never,auto,always} to have colored output disabled,
> enabled only if stdout is a terminal or enabled regardless of stdout
> state.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Allow to override isatty() check by specifying '-color' flag more than
>   once.
> - Document new behaviour in man pages.
> 
> Changes since v2:
> - Implement new -color=foo syntax.
> - Update commit message and man page texts accordingly.
> 
> Changes since v3:
> - Fix typo in tc/tc.c causing compile error.
> ---
>  bridge/bridge.c   |  3 +--
>  include/color.h   |  7 +++++++
>  ip/ip.c           |  3 +--
>  lib/color.c       | 33 ++++++++++++++++++++++++++++++++-
>  man/man8/bridge.8 | 13 +++++++++++--
>  man/man8/ip.8     | 13 +++++++++++--
>  man/man8/tc.8     | 13 +++++++++++--
>  tc/tc.c           |  3 +--
>  8 files changed, 75 insertions(+), 13 deletions(-)
> 
> diff --git a/bridge/bridge.c b/bridge/bridge.c
> index 451d684e0bcfd..e35e5bdf7fb30 100644
> --- a/bridge/bridge.c
> +++ b/bridge/bridge.c
> @@ -173,8 +173,7 @@ main(int argc, char **argv)
>  			NEXT_ARG();
>  			if (netns_switch(argv[1]))
>  				exit(-1);
> -		} else if (matches(opt, "-color") == 0) {
> -			++color;
> +		} else if (matches_color(opt, &color) == 0) {
>  		} else if (matches(opt, "-compressvlans") == 0) {
>  			++compress_vlans;
>  		} else if (matches(opt, "-force") == 0) {
> diff --git a/include/color.h b/include/color.h
> index 4f2c918db7e43..42038dc2e7f87 100644
> --- a/include/color.h
> +++ b/include/color.h
> @@ -12,8 +12,15 @@ enum color_attr {
>  	COLOR_NONE
>  };
>  
> +enum color_opt {
> +	COLOR_OPT_NEVER = 0,
> +	COLOR_OPT_AUTO = 1,
> +	COLOR_OPT_ALWAYS = 2
> +};
> +
>  void enable_color(void);
>  int check_enable_color(int color, int json);
> +int matches_color(const char *arg, int *val);
>  void set_color_palette(void);
>  int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...);
>  enum color_attr ifa_family_color(__u8 ifa_family);
> diff --git a/ip/ip.c b/ip/ip.c
> index 38eac5ec1e17d..893c3c43ef99a 100644
> --- a/ip/ip.c
> +++ b/ip/ip.c
> @@ -283,8 +283,7 @@ int main(int argc, char **argv)
>  				exit(-1);
>  			}
>  			rcvbuf = size;
> -		} else if (matches(opt, "-color") == 0) {
> -			++color;
> +		} else if (matches_color(opt, &color) == 0) {
>  		} else if (matches(opt, "-help") == 0) {
>  			usage();
>  		} else if (matches(opt, "-netns") == 0) {
> diff --git a/lib/color.c b/lib/color.c
> index edf96e5c6ecd7..3ad1d6d647722 100644
> --- a/lib/color.c
> +++ b/lib/color.c
> @@ -3,11 +3,13 @@
>  #include <stdarg.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <unistd.h>
>  #include <sys/socket.h>
>  #include <sys/types.h>
>  #include <linux/if.h>
>  
>  #include "color.h"
> +#include "utils.h"
>  
>  enum color {
>  	C_RED,
> @@ -79,13 +81,42 @@ void enable_color(void)
>  
>  int check_enable_color(int color, int json)
>  {
> -	if (color && !json) {
> +	if (json || color == COLOR_OPT_NEVER)
> +		return 1;
> +
> +	if (color == COLOR_OPT_ALWAYS || isatty(fileno(stdout))) {
>  		enable_color();
>  		return 0;
>  	}
>  	return 1;
>  }
>  
> +int matches_color(const char *arg, int *val)
> +{
> +	char *dup, *p;
> +
> +	if (!val)
> +		return 1;

I am fine with this even for current version (not next).
Minor nit, is that these functions should be bool.
Most of the iproute2 uses int for booleans for historical reasons
but lets try use bool for new code.

[iproute PATCH v5 1/2] Make colored output configurable

[Phil Sutter]

Allow for -color={never,auto,always} to have colored output disabled,
enabled only if stdout is a terminal or enabled regardless of stdout
state.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Allow to override isatty() check by specifying '-color' flag more than
  once.
- Document new behaviour in man pages.

Changes since v2:
- Implement new -color=foo syntax.
- Update commit message and man page texts accordingly.

Changes since v3:
- Fix typo in tc/tc.c causing compile error.

Changes since v4:
- Make matches_color() return boolean.
---
 bridge/bridge.c   |  3 +--
 include/color.h   |  9 +++++++++
 ip/ip.c           |  3 +--
 lib/color.c       | 33 ++++++++++++++++++++++++++++++++-
 man/man8/bridge.8 | 13 +++++++++++--
 man/man8/ip.8     | 13 +++++++++++--
 man/man8/tc.8     | 13 +++++++++++--
 tc/tc.c           |  3 +--
 8 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/bridge/bridge.c b/bridge/bridge.c
index b3cab717ead30..663a35b2b2e46 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -173,8 +173,7 @@ main(int argc, char **argv)
 			NEXT_ARG();
 			if (netns_switch(argv[1]))
 				exit(-1);
-		} else if (matches(opt, "-color") == 0) {
-			++color;
+		} else if (matches_color(opt, &color)) {
 		} else if (matches(opt, "-compressvlans") == 0) {
 			++compress_vlans;
 		} else if (matches(opt, "-force") == 0) {
diff --git a/include/color.h b/include/color.h
index 4f2c918db7e43..a22a00c2277e0 100644
--- a/include/color.h
+++ b/include/color.h
@@ -2,6 +2,8 @@
 #ifndef __COLOR_H__
 #define __COLOR_H__ 1
 
+#include <stdbool.h>
+
 enum color_attr {
 	COLOR_IFNAME,
 	COLOR_MAC,
@@ -12,8 +14,15 @@ enum color_attr {
 	COLOR_NONE
 };
 
+enum color_opt {
+	COLOR_OPT_NEVER = 0,
+	COLOR_OPT_AUTO = 1,
+	COLOR_OPT_ALWAYS = 2
+};
+
 void enable_color(void);
 int check_enable_color(int color, int json);
+bool matches_color(const char *arg, int *val);
 void set_color_palette(void);
 int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...);
 enum color_attr ifa_family_color(__u8 ifa_family);
diff --git a/ip/ip.c b/ip/ip.c
index 72e858eed50d5..58c643df8a366 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -283,8 +283,7 @@ int main(int argc, char **argv)
 				exit(-1);
 			}
 			rcvbuf = size;
-		} else if (matches(opt, "-color") == 0) {
-			++color;
+		} else if (matches_color(opt, &color)) {
 		} else if (matches(opt, "-help") == 0) {
 			usage();
 		} else if (matches(opt, "-netns") == 0) {
diff --git a/lib/color.c b/lib/color.c
index edf96e5c6ecd7..9c9023587748f 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -3,11 +3,13 @@
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <linux/if.h>
 
 #include "color.h"
+#include "utils.h"
 
 enum color {
 	C_RED,
@@ -79,13 +81,42 @@ void enable_color(void)
 
 int check_enable_color(int color, int json)
 {
-	if (color && !json) {
+	if (json || color == COLOR_OPT_NEVER)
+		return 1;
+
+	if (color == COLOR_OPT_ALWAYS || isatty(fileno(stdout))) {
 		enable_color();
 		return 0;
 	}
 	return 1;
 }
 
+bool matches_color(const char *arg, int *val)
+{
+	char *dup, *p;
+
+	if (!val)
+		return false;
+
+	dup = strdupa(arg);
+	p = strchrnul(dup, '=');
+	if (*p)
+		*(p++) = '\0';
+
+	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;
+}
+
 void set_color_palette(void)
 {
 	char *p = getenv("COLORFGBG");
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 1d10cb2b6a72c..53cd3d0a3d933 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -172,8 +172,17 @@ If there were any errors during execution of the commands, the application
 return code will be non zero.
 
 .TP
-.BR "\-c" , " -color"
-Use color output.
+.BR \-c [ color ][ = { always | auto | never }
+Configure color output. If parameter is omitted or
+.BR always ,
+color output is enabled regardless of stdout state. If parameter is
+.BR auto ,
+stdout is checked to be a terminal before enabling color output. If parameter is
+.BR never ,
+color output is disabled. If specified multiple times, the last one takes
+precedence. This flag is ignored if
+.B \-json
+is also given.
 
 .TP
 .BR "\-j", " \-json"
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 0087d18b74706..1d358879ec39c 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -187,8 +187,17 @@ to
 executes specified command over all objects, it depends if command supports this option.
 
 .TP
-.BR "\-c" , " -color"
-Use color output.
+.BR \-c [ color ][ = { always | auto | never }
+Configure color output. If parameter is omitted or
+.BR always ,
+color output is enabled regardless of stdout state. If parameter is
+.BR auto ,
+stdout is checked to be a terminal before enabling color output. If parameter is
+.BR never ,
+color output is disabled. If specified multiple times, the last one takes
+precedence. This flag is ignored if
+.B \-json
+is also given.
 
 .TP
 .BR "\-t" , " \-timestamp"
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index fd33f9b2f1806..f98398a34bd3e 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -755,8 +755,17 @@ option was specified. Classes can be filtered only by
 option.
 
 .TP
-.BR "\ -color"
-Use color output.
+.BR \-c [ color ][ = { always | auto | never }
+Configure color output. If parameter is omitted or
+.BR always ,
+color output is enabled regardless of stdout state. If parameter is
+.BR auto ,
+stdout is checked to be a terminal before enabling color output. If parameter is
+.BR never ,
+color output is disabled. If specified multiple times, the last one takes
+precedence. This flag is ignored if
+.B \-json
+is also given.
 
 .TP
 .BR "\-j", " \-json"
diff --git a/tc/tc.c b/tc/tc.c
index 4c7a128c8103e..4b28e9b182c1d 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -496,8 +496,7 @@ int main(int argc, char **argv)
 				matches(argv[1], "-conf") == 0) {
 			NEXT_ARG();
 			conf_file = argv[1];
-		} else if (matches(argv[1], "-color") == 0) {
-			++color;
+		} else if (matches_color(argv[1], &color)) {
 		} else if (matches(argv[1], "-timestamp") == 0) {
 			timestamp++;
 		} else if (matches(argv[1], "-tshort") == 0) {
-- 
2.18.0

[iproute PATCH 2/2] lib: Make check_enable_color() return boolean

[Phil Sutter]

As suggested, turn return code into true/false although it's not checked
anywhere yet.

Fixes: 4d82962cccc6a ("Merge common code for conditionally colored output")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/color.h | 2 +-
 lib/color.c     | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/color.h b/include/color.h
index a22a00c2277e0..e30f28c51c844 100644
--- a/include/color.h
+++ b/include/color.h
@@ -21,7 +21,7 @@ enum color_opt {
 };
 
 void enable_color(void);
-int check_enable_color(int color, int json);
+bool check_enable_color(int color, int json);
 bool matches_color(const char *arg, int *val);
 void set_color_palette(void);
 int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...);
diff --git a/lib/color.c b/lib/color.c
index 9c9023587748f..eaf69e74d673a 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -79,16 +79,16 @@ void enable_color(void)
 	set_color_palette();
 }
 
-int check_enable_color(int color, int json)
+bool check_enable_color(int color, int json)
 {
 	if (json || color == COLOR_OPT_NEVER)
-		return 1;
+		return false;
 
 	if (color == COLOR_OPT_ALWAYS || isatty(fileno(stdout))) {
 		enable_color();
-		return 0;
+		return true;
 	}
-	return 1;
+	return false;
 }
 
 bool matches_color(const char *arg, int *val)
-- 
2.18.0

Re: [iproute PATCH v5 1/2] Make colored output configurable

[David Ahern]

On 8/17/18 10:38 AM, Phil Sutter wrote:
> Allow for -color={never,auto,always} to have colored output disabled,
> enabled only if stdout is a terminal or enabled regardless of stdout
> state.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Allow to override isatty() check by specifying '-color' flag more than
>   once.
> - Document new behaviour in man pages.
> 
> Changes since v2:
> - Implement new -color=foo syntax.
> - Update commit message and man page texts accordingly.
> 
> Changes since v3:
> - Fix typo in tc/tc.c causing compile error.
> 
> Changes since v4:
> - Make matches_color() return boolean.
> ---
>  bridge/bridge.c   |  3 +--
>  include/color.h   |  9 +++++++++
>  ip/ip.c           |  3 +--
>  lib/color.c       | 33 ++++++++++++++++++++++++++++++++-
>  man/man8/bridge.8 | 13 +++++++++++--
>  man/man8/ip.8     | 13 +++++++++++--
>  man/man8/tc.8     | 13 +++++++++++--
>  tc/tc.c           |  3 +--
>  8 files changed, 77 insertions(+), 13 deletions(-)
> 

LGTM.

Reviewed-by: David Ahern <dsahern@gmail.com>