[libmnl PATCH] debug: don't colorize output on non tty

[libmnl PATCH] debug: don't colorize output on non tty

[Eric Leblond]

When output is not a tty (pipe or redirect to a file), the color
display is causing the output to be unreadable:
  02 00 00 00  |        |  extra header  |
 |ESC[1;31m00008ESC[0m|ESC[1;32m--ESC[0m|ESC[1;34m00001ESC[0m|   |len |flags| type|
This patch tests if the output is a terminal and only add color in
this case. It also displays space instead of char 0 if a letter is
not existing.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 src/nlmsg.c | 69 ++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 26 deletions(-)

diff --git a/src/nlmsg.c b/src/nlmsg.c
index fdb7af8..07f19ba 100644
--- a/src/nlmsg.c
+++ b/src/nlmsg.c
@@ -281,28 +281,45 @@ mnl_nlmsg_fprintf_payload(FILE *fd, const struct nlmsghdr *nlh,
 			fprintf(fd, "|  extra header  |\n");
 		/* this seems like an attribute header. */
 		} else if (rem == 0 && (attr->nla_type & NLA_TYPE_MASK) != 0) {
-			fprintf(fd, "|%c[%d;%dm"
-				    "%.5u"
-				    "%c[%dm"
-				    "|"
-				    "%c[%d;%dm"
-				    "%c%c"
-				    "%c[%dm"
-				    "|"
-				    "%c[%d;%dm"
-				    "%.5u"
-				    "%c[%dm|\t",
-				27, 1, 31,
-				attr->nla_len,
-				27, 0,
-				27, 1, 32,
-				attr->nla_type & NLA_F_NESTED ? 'N' : '-',
-				attr->nla_type &
-					NLA_F_NET_BYTEORDER ? 'B' : '-',
-				27, 0,
-				27, 1, 34,
-				attr->nla_type & NLA_TYPE_MASK,
-				27, 0);
+			if (isatty(fileno(fd))) {
+				fprintf(fd, "|%c[%d;%dm"
+					    "%.5u"
+					    "%c[%dm"
+					    "|"
+					    "%c[%d;%dm"
+					    "%c%c"
+					    "%c[%dm"
+					    "|"
+					    "%c[%d;%dm"
+					    "%.5u"
+					    "%c[%dm|\t",
+					27, 1, 31,
+					attr->nla_len,
+					27, 0,
+					27, 1, 32,
+					attr->nla_type &
+						NLA_F_NESTED ? 'N' : '-',
+					attr->nla_type &
+						NLA_F_NET_BYTEORDER ? 'B' : '-',
+					27, 0,
+					27, 1, 34,
+					attr->nla_type & NLA_TYPE_MASK,
+					27, 0);
+			} else {
+				fprintf(fd, "|"
+					    "%.5u"
+					    "|"
+					    "%c%c"
+					    "|"
+					    "%.5u"
+					    "|\t",
+					attr->nla_len,
+					attr->nla_type &
+						NLA_F_NESTED ? 'N' : '-',
+					attr->nla_type &
+						NLA_F_NET_BYTEORDER ? 'B' : '-',
+					attr->nla_type & NLA_TYPE_MASK);
+			}
 			fprintf(fd, "|len |flags| type|\n");
 
 			if (!(attr->nla_type & NLA_F_NESTED)) {
@@ -317,10 +334,10 @@ mnl_nlmsg_fprintf_payload(FILE *fd, const struct nlmsghdr *nlh,
 				0xff & b[i+2],	0xff & b[i+3]);
 			fprintf(fd, "|      data      |");
 			fprintf(fd, "\t %c %c %c %c\n",
-				isalnum(b[i]) ? b[i] : 0,
-				isalnum(b[i+1]) ? b[i+1] : 0,
-				isalnum(b[i+2]) ? b[i+2] : 0,
-				isalnum(b[i+3]) ? b[i+3] : 0);
+				isalnum(b[i]) ? b[i] : 32,
+				isalnum(b[i+1]) ? b[i+1] : 32,
+				isalnum(b[i+2]) ? b[i+2] : 32,
+				isalnum(b[i+3]) ? b[i+3] : 32);
 		}
 	}
 	fprintf(fd, "----------------\t------------------\n");
-- 
1.8.4.rc3

Re: [libmnl PATCH] debug: don't colorize output on non tty

[Pablo Neira Ayuso]

Hi Eric,

On Mon, Sep 30, 2013 at 11:38:26PM +0200, Eric Leblond wrote:
> When output is not a tty (pipe or redirect to a file), the color
> display is causing the output to be unreadable:
>   02 00 00 00  |        |  extra header  |
>  |ESC[1;31m00008ESC[0m|ESC[1;32m--ESC[0m|ESC[1;34m00001ESC[0m|   |len |flags| type|
> This patch tests if the output is a terminal and only add color in
> this case. It also displays space instead of char 0 if a letter is
> not existing.

In both cases, you can use less -r to interpret the colors, is that
enough to address what you're noticing?

The chunk to replace char 0 by 32 looks fine to me.

Let me know.

Re: [libmnl PATCH] debug: don't colorize output on non tty

[Eric Leblond]

Hi Pablo,

Le mercredi 02 octobre 2013 à 17:25 +0200, Pablo Neira Ayuso a écrit :
> Hi Eric,
> 
> On Mon, Sep 30, 2013 at 11:38:26PM +0200, Eric Leblond wrote:
> > When output is not a tty (pipe or redirect to a file), the color
> > display is causing the output to be unreadable:
> >   02 00 00 00  |        |  extra header  |
> >  |ESC[1;31m00008ESC[0m|ESC[1;32m--ESC[0m|ESC[1;34m00001ESC[0m|   |len |flags| type|
> > This patch tests if the output is a terminal and only add color in
> > this case. It also displays space instead of char 0 if a letter is
> > not existing.
> 
> In both cases, you can use less -r to interpret the colors, is that
> enough to address what you're noticing?

No, I don't think this is enough. First, you've got to know -r option of
less ;) Second, this is really painful if you want to redirect the
output to a file to edit it with a standard editor (that was my case).
At last, git and a lot of software are doing that so some user are used
to that behavior.

> The chunk to replace char 0 by 32 looks fine to me.

OK.

BR,
-- 
Eric Leblond <eric@regit.org>

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [libmnl PATCH] debug: don't colorize output on non tty

[Pablo Neira Ayuso]

Hi Eric,

On Wed, Oct 02, 2013 at 05:47:20PM +0200, Eric Leblond wrote:
> Hi Pablo,
> 
> Le mercredi 02 octobre 2013 à 17:25 +0200, Pablo Neira Ayuso a écrit :
> > Hi Eric,
> > 
> > On Mon, Sep 30, 2013 at 11:38:26PM +0200, Eric Leblond wrote:
> > > When output is not a tty (pipe or redirect to a file), the color
> > > display is causing the output to be unreadable:
> > >   02 00 00 00  |        |  extra header  |
> > >  |ESC[1;31m00008ESC[0m|ESC[1;32m--ESC[0m|ESC[1;34m00001ESC[0m|   |len |flags| type|
> > > This patch tests if the output is a terminal and only add color in
> > > this case. It also displays space instead of char 0 if a letter is
> > > not existing.
> > 
> > In both cases, you can use less -r to interpret the colors, is that
> > enough to address what you're noticing?
> 
> No, I don't think this is enough. First, you've got to know -r option of
> less ;) Second, this is really painful if you want to redirect the
> output to a file to edit it with a standard editor (that was my case).
> At last, git and a lot of software are doing that so some user are used
> to that behavior.

I can see that git allows you to enable/disable colors on demand. I
think we can have a new interface that includes a flags field for
this, so we can achieve the same way of working.

I'd like to keep some way to request colors, I usually redirect the
output to files and use less -r to watch them. Some days, when you
spent more than 8 hours in front of the computer, some colors help,
really :-).
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html