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 <p...@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
