Re: ls --color feature request: ordinary files uncoloured

[Ed Avis]

Here is an updated patch against coreutils-5.2.0, including changelog
and news entries.  All the 'make check' tests pass, at least for me.

diff -ru coreutils-5.2.0/ChangeLog coreutils-5.2.0-new/ChangeLog
--- coreutils-5.2.0/ChangeLog   2004-02-17 16:05:39.000000000 +0000
+++ coreutils-5.2.0-new/ChangeLog       2004-02-22 13:36:13.163611440 +0000
@@ -1,3 +1,11 @@
+2004-02-22  Ed Avis  <address@hidden>
+
+       * src/ls.c: ordinary files are written with no ANSI escape
+       sequences, even with --color.  The escape sequence to reset the
+       terminal is not output if no filenames were printed with color.
+       In particular this means nothing printed on stdout for 'ls --color
+       nonexistent-file'.
+
 2004-02-17  Jim Meyering  <address@hidden>
 
        * Version 5.2.0.
diff -ru coreutils-5.2.0/NEWS coreutils-5.2.0-new/NEWS
--- coreutils-5.2.0/NEWS        2004-02-19 18:04:27.000000000 +0000
+++ coreutils-5.2.0-new/NEWS    2004-02-22 13:42:28.437561128 +0000
@@ -1,4 +1,11 @@
 GNU coreutils NEWS                                    -*- outline -*-
+* Major changes since 5.2.0
+
+  ls --color now leaves ordinary files uncolored, unless you have
+  explicitly chosen a color for them in the LS_COLORS environment
+  variable.  And if it did not print any color escape sequences, it
+  will not print the escape sequence to reset the terminal afterwards.
+
 * Major changes in release 5.2.0 (2004-02-19) [stable]
 
 ** Bug fixes
diff -ru coreutils-5.2.0/src/ls.c coreutils-5.2.0-new/src/ls.c
--- coreutils-5.2.0/src/ls.c    2004-02-01 19:31:00.000000000 +0000
+++ coreutils-5.2.0-new/src/ls.c        2004-02-22 13:34:13.349825904 +0000
@@ -227,7 +227,7 @@
 static int file_interesting (const struct dirent *next);
 static uintmax_t gobble_file (const char *name, enum filetype type,
                              int explicit_arg, const char *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);
 static void put_indicator (const struct bin_str *ind);
 static int put_indicator_direct (const struct bin_str *ind);
 static void add_ignore_pattern (const char *pattern);
@@ -483,6 +483,13 @@
 
 static int 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
+   print some stuff (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 */
@@ -501,13 +508,13 @@
 
 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
   };
 
 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", NULL
   };
 
@@ -523,8 +530,9 @@
     { 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 */
@@ -977,7 +985,8 @@
   signal (sig, SIG_IGN);
 #endif
 
-  restore_default_color ();
+  if (used_color)
+    restore_default_color ();
 
   /* SIGTSTP is special, since the application can receive that signal more
      than once.  In this case, don't set the signal handler to the default.
@@ -1035,7 +1044,6 @@
      may have just reset it -- e.g., if LS_COLORS is invalid.  */
   if (print_with_color)
     {
-      prep_non_filename_text ();
       /* Avoid following symbolic links when possible.  */
       if (color_indicator[C_ORPHAN].string != NULL
          || (color_indicator[C_MISSING].string != NULL
@@ -1192,7 +1200,7 @@
     }
 
   /* Restore default color before exiting */
-  if (print_with_color)
+  if (used_color)
     {
       put_indicator (&color_indicator[C_LEFT]);
       put_indicator (&color_indicator[C_RIGHT]);
@@ -3408,8 +3416,8 @@
 print_name_with_quoting (const char *p, mode_t mode, int linkok,
                         struct obstack *stack)
 {
-  if (print_with_color)
-    print_color_indicator (p, mode, linkok);
+  const bool used_color_this_time
+    = print_with_color && print_color_indicator (p, mode, linkok);
 
   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);
@@ -3419,7 +3427,7 @@
   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);
 
-  if (print_with_color)
+  if (used_color_this_time)
     prep_non_filename_text ();
 }
 
@@ -3431,7 +3439,7 @@
   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]);
     }
 }
@@ -3492,7 +3500,8 @@
     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)
 {
   int type = C_FILE;
@@ -3544,10 +3553,20 @@
            }
        }
     }
-
-  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).  */
@@ -3557,6 +3576,12 @@
   register size_t i;
   register const char *p;
 
+  if (! used_color)
+    {
+      used_color = true;
+      prep_non_filename_text ();
+    }
+  
   p = ind->string;
 
   for (i = ind->len; i != 0; --i)
@@ -3573,6 +3598,12 @@
   if (ind->len == 0)
     return 0;
 
+  if (! used_color)
+    {
+      used_color = true;
+      prep_non_filename_text ();
+    }
+
   len = ind->len;
   return (full_write (STDOUT_FILENO, ind->string, len) != len);
 }
diff -ru coreutils-5.2.0/tests/ls-2/tests coreutils-5.2.0-new/tests/ls-2/tests
--- coreutils-5.2.0/tests/ls-2/tests    2004-01-26 15:53:05.000000000 +0000
+++ coreutils-5.2.0-new/tests/ls-2/tests        2004-02-22 13:23:58.406311528 
+0000
@@ -32,6 +32,7 @@
                            $ENV{LS_COLORS}='ln=01;36:di=01;34:or=40;31;01'
                            }};
 my $unlink_d = {POST => sub {unlink 'd' or die "d: $!\n"}};
+my $creat_d = {PRE => sub {local *FH; open(FH, '>d') or die "d: $!\n"}};
 
 sub make_j_d ()
 {
@@ -85,6 +86,9 @@
                                 {OUT => "\e[0m\e[01;address@hidden"},
                                  $slink_d, $unlink_d],
 
+     # A listing with only plain files should have no color sequences at all.
+     ['no-color', '--color=always d', {OUT => "d\n"}, $creat_d, $unlink_d],
+
      # Test for a bug that was introduced in coreutils-4.5.4; fixed in 4.5.5.
      # To demonstrate it, the file in question (with executable bit set)
      # must not be a command line argument.


-- 
Ed Avis <address@hidden>