ls --color no longer outputs unnecessary escape sequences

In --color mode, plain files do not get any color, not even white.
When no highlighting is required, ls outputs no escape sequence at all.
* src/ls.c (print_with_color):
(used_color): New global.
(indicator_no) [C_RESET]: New enum value.
(indicator_name) ["rs"]: Corresponding new string.
(color_indicator): Make the 'normal' and 'file' markers be NULL.
Use "rs" (C_RESET) to reset to ordinary colors.
(process_signals): Restore default colors only if necessary.
(main): Don't call prep_non_filename_text here.
(print_name_with_quoting): Call it here, instead.
(prep_non_filename_text): Use C_RESET, not C_NORM.
(print_color_indicator): Return bool, not void.
Print nothing, when possible.
(put_indicator): Call prep_non_filename_text the first time.
* tests/misc/ls-misc: Test for above.
* tests/ls/color-dtype-dir: Adapt: no escapes around regular file name.
* TODO: Remove item.
* NEWS: Mention this.

diff --git a/NEWS b/NEWS
index af27aab..34fda4e 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -6,13 +6,16 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   configure --enable-no-install-program=groups now works.
 
-
   ls no longer segfaults on files in /proc when linked with an older version
   of libselinux.  E.g., ls -l /proc/sys would dereference a NULL pointer.
 
   "rmdir --ignore-fail-on-non-empty" detects and ignores the failure
   in more cases when a directory is empty.
 
+** Improvements
+
+  ls --color no longer outputs unnecessary escape sequences
+
 ** Consistency
 
   mkdir and split now write --verbose output to stdout, not stderr.

diff --git a/TODO b/TODO
index 24369c6..8c6b6fc 100644 (file)
--- a/TODO
+++ b/TODO
@@ -133,9 +133,6 @@ Changes expected to go in, someday.
 
   Pending copyright papers:
   ------------------------
-  ls --color: Ed Avis' patch to suppress escape sequences for
-    non-highlighted files
-
   getpwnam from Bruce Korb
 
   pb (progress bar) from Miika Pekkarinen

diff --git a/src/ls.c b/src/ls.c
index ff49278..0eb2b27 100644 (file)
--- a/src/ls.c
+++ b/src/ls.c
@@ -207,7 +207,7 @@ static bool file_ignored (char const *name);
 static uintmax_t gobble_file (char const *name, enum filetype type,
                              ino_t inode, bool command_line_arg,
                              char const *dirname);
-static void print_color_indicator (const char *name, mode_t mode, int linkok,
+static bool print_color_indicator (const char *name, mode_t mode, int linkok,
                                   bool stat_ok, enum filetype type);
 static void put_indicator (const struct bin_str *ind);
 static void add_ignore_pattern (const char *pattern);
@@ -488,6 +488,12 @@ ARGMATCH_VERIFY (indicator_style_args, indicator_style_types);
 
 static bool print_with_color;
 
+/* Whether we used any colors in the output so far.  If so, we will
+   need to restore the default color later.  If not, we will need to
+   call prep_non_filename_text before using color for the first time. */
+
+static bool used_color = false;
+
 enum color_type
   {
     color_never,               /* 0: default or --color=never */
@@ -506,14 +512,15 @@ enum Dereference_symlink
 
 enum indicator_no
   {
-    C_LEFT, C_RIGHT, C_END, C_NORM, C_FILE, C_DIR, C_LINK, C_FIFO, C_SOCK,
+    C_LEFT, C_RIGHT, C_END, C_RESET, C_NORM, C_FILE, C_DIR, C_LINK,
+    C_FIFO, C_SOCK,
     C_BLK, C_CHR, C_MISSING, C_ORPHAN, C_EXEC, C_DOOR, C_SETUID, C_SETGID,
     C_STICKY, C_OTHER_WRITABLE, C_STICKY_OTHER_WRITABLE
   };
 
 static const char *const indicator_name[]=
   {
-    "lc", "rc", "ec", "no", "fi", "di", "ln", "pi", "so",
+    "lc", "rc", "ec", "rs", "no", "fi", "di", "ln", "pi", "so",
     "bd", "cd", "mi", "or", "ex", "do", "su", "sg", "st",
     "ow", "tw", NULL
   };
@@ -530,8 +537,9 @@ static struct bin_str color_indicator[] =
     { LEN_STR_PAIR ("\033[") },                /* lc: Left of color sequence */
     { LEN_STR_PAIR ("m") },            /* rc: Right of color sequence */
     { 0, NULL },                       /* ec: End color (replaces lc+no+rc) */
-    { LEN_STR_PAIR ("0") },            /* no: Normal */
-    { LEN_STR_PAIR ("0") },            /* fi: File: default */
+    { LEN_STR_PAIR ("0") },            /* rs: Reset to ordinary colors */
+    { 0, NULL },                       /* no: Normal */
+    { 0, NULL },                       /* fi: File: default */
     { LEN_STR_PAIR ("01;34") },                /* di: Directory: bright blue */
     { LEN_STR_PAIR ("01;36") },                /* ln: Symlink: bright cyan */
     { LEN_STR_PAIR ("33") },           /* pi: Pipe: yellow/brown */
@@ -1071,7 +1079,8 @@ process_signals (void)
       int stops;
       sigset_t oldset;
 
-      restore_default_color ();
+      if (used_color)
+       restore_default_color ();
       fflush (stdout);
 
       sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
@@ -1211,8 +1220,6 @@ main (int argc, char **argv)
            }
 #endif
        }
-
-      prep_non_filename_text ();
     }
 
   if (dereference == DEREF_UNDEFINED)
@@ -1327,7 +1334,8 @@ main (int argc, char **argv)
     {
       int j;
 
-      restore_default_color ();
+      if (used_color)
+       restore_default_color ();
       fflush (stdout);
 
       /* Restore the default signal handling.  */
@@ -3790,8 +3798,9 @@ print_name_with_quoting (const char *p, mode_t mode, int linkok,
                         bool stat_ok, enum filetype type,
                         struct obstack *stack)
 {
-  if (print_with_color)
-    print_color_indicator (p, mode, linkok, stat_ok, type);
+  bool used_color_this_time
+    = (print_with_color
+       && print_color_indicator (p, mode, linkok, stat_ok, type));
 
   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);
@@ -3801,7 +3810,7 @@ print_name_with_quoting (const char *p, mode_t mode, int linkok,
   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);
 
-  if (print_with_color)
+  if (used_color_this_time)
     {
       process_signals ();
       prep_non_filename_text ();
@@ -3816,7 +3825,7 @@ prep_non_filename_text (void)
   else
     {
       put_indicator (&color_indicator[C_LEFT]);
-      put_indicator (&color_indicator[C_NORM]);
+      put_indicator (&color_indicator[C_RESET]);
       put_indicator (&color_indicator[C_RIGHT]);
     }
 }
@@ -3891,7 +3900,8 @@ print_type_indicator (bool stat_ok, mode_t mode, enum filetype type)
     DIRED_PUTCHAR (c);
 }
 
-static void
+/* Returns whether any color sequence was printed. */
+static bool
 print_color_indicator (const char *name, mode_t mode, int linkok,
                       bool stat_ok, enum filetype filetype)
 {
@@ -3968,9 +3978,19 @@ print_color_indicator (const char *name, mode_t mode, int linkok,
        }
     }
 
-  put_indicator (&color_indicator[C_LEFT]);
-  put_indicator (ext ? &(ext->seq) : &color_indicator[type]);
-  put_indicator (&color_indicator[C_RIGHT]);
+  {
+    const struct bin_str *const s
+      = ext ? &(ext->seq) : &color_indicator[type];
+    if (s->string != NULL)
+      {
+       put_indicator (&color_indicator[C_LEFT]);
+       put_indicator (s);
+       put_indicator (&color_indicator[C_RIGHT]);
+       return true;
+      }
+    else
+      return false;
+  }
 }
 
 /* Output a color indicator (which may contain nulls).  */
@@ -3980,6 +4000,12 @@ put_indicator (const struct bin_str *ind)
   size_t i;
   const char *p;
 
+  if (! used_color)
+    {
+      used_color = true;
+      prep_non_filename_text ();
+    }
+
   p = ind->string;
 
   for (i = ind->len; i != 0; --i)

diff --git a/tests/ls/color-dtype-dir b/tests/ls/color-dtype-dir
index 7a25c06..b969701 100755 (executable)
--- a/tests/ls/color-dtype-dir
+++ b/tests/ls/color-dtype-dir
@@ -4,7 +4,7 @@
 # directories the same as the first one -- but only on a file system
 # with dirent.d_type support.
 
-# Copyright (C) 2006-2007 Free Software Foundation, Inc.
+# Copyright (C) 2006-2008 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
@@ -43,7 +43,7 @@ mv o1 out || fail=1
 cat <<\EOF > exp || fail=1
 ^[[0m^[[01;34md^[[0m$
 ^[[34;42mother-writable^[[0m$
-^[[0mout^[[0m$
+out$
 ^[[37;44msticky^[[0m$
 ^[[m
 EOF

diff --git a/tests/misc/ls-misc b/tests/misc/ls-misc
index 9290eb4..4182fa1 100755 (executable)
--- a/tests/misc/ls-misc
+++ b/tests/misc/ls-misc
@@ -1,7 +1,7 @@
 #!/bin/sh
 # -*- perl -*-
 
-# Copyright (C) 1998, 2000-2007 Free Software Foundation, Inc.
+# Copyright (C) 1998, 2000-2008 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
@@ -55,8 +55,14 @@ system (qq(touch setuid && chmod u+s setuid && $test -u setuid &&
   or (warn "$program_name: cannot create setuid/setgid/sticky files,"
       . "so can't run this test\n"), exit 77;
 
-my $mkdir = {PRE => sub {mkdir 'd',0755 or die "d: $!\n"}};
-my $rmdir = {POST => sub {rmdir 'd' or die "d: $!\n"}};
+sub mkdir_d {mkdir 'd',0755 or die "d: $!\n"}
+sub rmdir_d {rmdir 'd'      or die "d: $!\n"}
+my $mkdir = {PRE => sub {mkdir_d}};
+my $rmdir = {POST => sub {rmdir_d}};
+my $mkdir_reg = {PRE => sub {mkdir_d; open (FH, '>d/f') && close FH
+                              or die "d/f: $!\n" }};
+my $rmdir_reg = {POST => sub {unlink 'd/f' or die "d/f: $!\n";
+                             rmdir 'd' or die "d: $!\n"}};
 
 my $mkdir2 = {PRE => sub {mkdir 'd',0755 or die "d: $!\n";
                          mkdir 'd/e',0755 or die "d/e: $!\n" }};
@@ -132,6 +138,11 @@ my @Tests =
                                 {OUT => "\e[0m\e[01;36md\e[0m\@\n\e[m"},
                                  $slink_d, $unlink_d],
 
+     # A listing with no output should have no color sequences at all.
+     ['no-c-empty', '--color=always d', {OUT => ""}, $mkdir, $rmdir],
+     # A listing with only regular files should have no color sequences at all.
+     ['no-c-reg', '--color=always d', {OUT => "f\n"}, $mkdir_reg, $rmdir_reg],
+
      # Test for a bug fixed after coreutils-6.9.
      ['sl-target', '--color=always d',
       {OUT => "\e[0m\e[01;34mX\e[0m\n\e[m"}, $target, $target2],