Subject: bug#15992: 'ls' ignores term capabilities when generating color.

From: Pádraig Brady <[email-masked]>
Date: Wed, 16 Apr 2014 16:56:04 +0100

On 04/16/2014 02:51 PM, Guilherme de Almeida Suckevicz wrote:
> Hi Pádraig,
> 
> I made a patch for this issue according to the tips above.
> I hope that is ok!
> 
> [guilherme@almeida build]$ diff ../src/ls.c ../src/ls-orig.c

Note the patch is reversed, also we prefer unified format,
also we'd much prefer a patch against the git repo.
For general notes on supplying patches see:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=README-hacking;hb=HEAD

> 2372,2382c2339
> <     {
> <       /* If COLORTERM is not set, check if the term type is know. */
> <       if (getenv ("COLORTERM") == NULL)
> <     {
> <       if (! know_term_type ())
> <         {
> <           print_with_color = false;
> <           return;
> <         }
> <     }

          you want a return here too

> <     }

I've cleaned up the patch and added a test.
If you're Ok with it I'll push.

thanks,
Pádraig.

>From 3dbeb4e844889f25dcd0eff7a3b766bdcff560d8 Mon Sep 17 00:00:00 2001
From: Guilherme de Almeida Suckevicz <[email-masked]>
Date: Wed, 16 Apr 2014 16:38:49 +0100
Subject: [PATCH] ls: don't output colors with unknown TERM env variable

--colors controls whether to output colors depending on
whether we're connected to a terminal or not, while this
change gives control over which terminals we output colors to.

* NEWS: Mention the change in behavior.
* src/ls.c (known_term_type): A new function to search the static
list from dircolors.h
(parse_ls_colors): Honor the TERM when both LS_COLORS and COLORTERM
are non empty.
* tests/ls/color-term.sh: A new test.
* tests/local.mk: Reference the new test.
Fixes http://bugs.gnu.org/15992
---
 NEWS                   |    6 ++++++
 src/ls.c               |   36 +++++++++++++++++++++++++++++++++++-
 tests/local.mk         |    1 +
 tests/ls/color-term.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 1 deletions(-)
 create mode 100755 tests/ls/color-term.sh

diff --git a/NEWS b/NEWS
index 1d3ecf1..55f0b58 100644
--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,12 @@ GNU coreutils NEWS                                    -*- outline -*-
   od accepts a new option: --endian=TYPE to handle inputs with different byte
   orders, or to provide consistent output on systems with disparate endianness.
 
+** Changes in behavior
+
+  ls with none of LS_COLORS or COLORTERM environment variables set,
+  will now honor an empty or unknown TERM environment variable,
+  and not output colors even with --colors=always.
+
 ** Improvements
 
   chroot has better --userspec and --group look-ups, with numeric IDs never
diff --git a/src/ls.c b/src/ls.c
index 5d87dd3..25e10fa 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -108,6 +108,7 @@
 #include "xstrtol.h"
 #include "areadlink.h"
 #include "mbsalign.h"
+#include "dircolors.h"
 
 /* Include <sys/capability.h> last to avoid a clash of <sys/types.h>
    include guards with some premature versions of libcap.
@@ -2326,6 +2327,30 @@ enum parse_state
     PS_FAIL
   };
 
+
+/* Check if the content of TERM is a valid name in dircolors.  */
+
+static bool
+known_term_type (void)
+{
+  char const *term = getenv ("TERM");
+  if (! term || ! *term)
+    return false;
+
+  char const *line = G_line;
+  while (line - G_line < sizeof (G_line))
+    {
+      if (STRNCMP_LIT (line, "TERM ") == 0)
+        {
+          if (STREQ (term, line + 5))
+            return true;
+        }
+      line += strlen (line) + 1;
+    }
+
+  return false;
+}
+
 static void
 parse_ls_color (void)
 {
@@ -2336,7 +2361,16 @@ parse_ls_color (void)
   struct color_ext_type *ext;	/* Extension we are working on */
 
   if ((p = getenv ("LS_COLORS")) == NULL || *p == '\0')
-    return;
+    {
+      /* LS_COLORS takes precedence, but if that's not set then
+         honor the COLORTERM and TERM env variables so that
+         we only go with the internal ANSI color codes if the
+         former is non empty or the latter is set to a known value.  */
+      char const *colorterm = getenv ("COLORTERM");
+      if (! (colorterm && *colorterm) && ! known_term_type ())
+        print_with_color = false;
+      return;
+    }
 
   ext = NULL;
   strcpy (label, "??");
diff --git a/tests/local.mk b/tests/local.mk
index 26aef50..d58b603 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -537,6 +537,7 @@ all_tests =					\
   tests/ls/color-clear-to-eol.sh		\
   tests/ls/color-dtype-dir.sh			\
   tests/ls/color-norm.sh			\
+  tests/ls/color-term.sh			\
   tests/ls/dangle.sh				\
   tests/ls/dired.sh				\
   tests/ls/file-type.sh				\
diff --git a/tests/ls/color-term.sh b/tests/ls/color-term.sh
new file mode 100755
index 0000000..a8e0afa
--- /dev/null
+++ b/tests/ls/color-term.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+# Ensure "ls --color" doesn't output colors for TERM=dumb
+
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ ls
+
+# Output time as something constant
+export TIME_STYLE="+norm"
+
+touch exe || framework_failure_
+chmod u+x exe || framework_failure_
+
+# output colors
+LS_COLORS='' COLORTERM='nonempty' TERM='' ls --color=always exe >> out || fail=1
+LS_COLORS='' COLORTERM='' TERM='xterm' ls --color=always exe >> out || fail=1
+
+# Don#t output colors
+LS_COLORS='' COLORTERM='' TERM='dumb' ls --color=always exe >> out || fail=1
+LS_COLORS='' COLORTERM='' TERM='' ls --color=always exe >> out || fail=1
+
+cat -A out > out.display || framework_failure_
+mv out.display out || framework_failure_
+
+cat <<\EOF > exp || framework_failure_
+^[[0m^[[01;32mexe^[[0m$
+^[[0m^[[01;32mexe^[[0m$
+exe$
+exe$
+EOF
+
+compare exp out || fail=1
+
+Exit $fail
-- 
1.7.7.6