From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Ahern <dsahern@gmail.com>
Subject: Re: [iproute PATCH v4] Make colored output configurable
Date: Thu, 16 Aug 2018 07:06:07 -0600
Message-ID: <46aa7a15-0317-10e4-0404-580b38fe2efb@gmail.com>
References: <20180816093703.23022-1-phil@nwl.cc>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org, Till Maas <opensource@till.name>
To: Phil Sutter <phil@nwl.cc>,
        Stephen Hemminger <stephen@networkplumber.org>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pg1-f193.google.com ([209.85.215.193]:38927 "EHLO
        mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S2403787AbeHPQEk (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 16 Aug 2018 12:04:40 -0400
Received: by mail-pg1-f193.google.com with SMTP id a11-v6so2048854pgw.6
        for <netdev@vger.kernel.org>; Thu, 16 Aug 2018 06:06:11 -0700 (PDT)
In-Reply-To: <20180816093703.23022-1-phil@nwl.cc>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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.