All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Phil Sutter <phil@nwl.cc>
Cc: David Ahern <dsahern@gmail.com>,
	netdev@vger.kernel.org, Till Maas <opensource@till.name>
Subject: Re: [iproute PATCH v2 4/4] lib: Enable colored output only for TTYs
Date: Wed, 15 Aug 2018 12:30:57 -0700	[thread overview]
Message-ID: <20180815123057.4e86f3be@xeon-e3> (raw)
In-Reply-To: <20180815165115.GU32448@orbyte.nwl.cc>

On Wed, 15 Aug 2018 18:51:15 +0200
Phil Sutter <phil@nwl.cc> wrote:

> On Wed, Aug 15, 2018 at 10:43:25AM -0600, David Ahern wrote:
> > On 8/15/18 10:39 AM, Phil Sutter wrote:  
> > > On Wed, Aug 15, 2018 at 10:24:31AM -0600, David Ahern wrote:  
> > >> On 8/15/18 10:21 AM, Phil Sutter wrote:  
> > >>> Add an additional prerequisite to check_enable_color() to make sure
> > >>> stdout actually points to an open TTY device. Otherwise calls like
> > >>>
> > >>> | ip -color a s >/tmp/foo
> > >>>
> > >>> will print color escape sequences into that file. Allow to override this
> > >>> check by specifying '-color' flag more than once.
> > >>>
> > >>> Signed-off-by: Phil Sutter <phil@nwl.cc>
> > >>> ---
> > >>> Changes since v1:
> > >>> - Allow to override isatty() check by specifying '-color' flag more than
> > >>>   once.  
> > >>
> > >> That adds overhead to my workflow where I almost always have to pipe the
> > >> output of ip to a pager.  
> > > 
> > > alias ip='ip -color -color'  
> > 
> > no. Don't impact existing users.  
> 
> That's a possible fix for *your* workflow. If applied to the shell
> handling that workflow, it won't impact existing users.
> 
> > > Another alternative may be to introduce -autocolor flag. Establishing
> > > the same syntax as used by 'ls' is not as trivial due to the simple
> > > commandline parsing used in 'ip'.  
> > 
> > I disagree with ignoring or overriding an argument a user passes in. You
> > are guessing what is the correct output and you are guessing wrong.
> > There is nothing wrong with piping output to a file and the viewing that
> > file through 'less -R'.
> > 
> > If a user does not want the color codes in the file, then that user can
> > drop the -color arg. iproute2 commands should not be guessing.  
> 
> OK, I got it. Should I respin the fixes or will you apply the series
> partially?
> 
> Thanks, Phil

Please follow the color conventions of grep and ls to have
consistent user experience.
	-c  == --color=always
	
and add never and auto.

      parent reply	other threads:[~2018-08-15 22:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-15 16:21 [iproute PATCH v2 0/4] A bunch of fixes regarding colored output Phil Sutter
2018-08-15 16:21 ` [iproute PATCH 1/4] tc: Fix typo in check for " Phil Sutter
2018-08-15 16:21 ` [iproute PATCH 2/4] bridge: Fix " Phil Sutter
2018-08-15 16:21 ` [iproute PATCH 3/4] Merge common code for conditionally " Phil Sutter
2018-08-15 16:21 ` [iproute PATCH v2 4/4] lib: Enable colored output only for TTYs Phil Sutter
2018-08-15 16:24   ` David Ahern
2018-08-15 16:39     ` Phil Sutter
2018-08-15 16:43       ` David Ahern
2018-08-15 16:51         ` Phil Sutter
2018-08-15 16:57           ` David Ahern
2018-08-15 17:58             ` Phil Sutter
2018-08-15 19:30           ` Stephen Hemminger [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180815123057.4e86f3be@xeon-e3 \
    --to=stephen@networkplumber.org \
    --cc=dsahern@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=opensource@till.name \
    --cc=phil@nwl.cc \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.