| Message ID | 1380577106-13006-1-git-send-email-eric@regit.org |
|---|---|
| State | RFC |
| Headers | show |
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. -- 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
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,
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
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");
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(-)