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 <guito.li...@gmail.com> 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
