Re: a fix for color ls with wrapped lines.

[Jim Meyering]

Jim Meyering <address@hidden> wrote:
...
> Here's an alternate patch that seems to do the job.
> But it may have bugs, because I did it far too quickly
> and haven't reviewed it at all.
>
> This will need at least tests corresponding to -1,
> -l (two: the name itself spans, or the -> link_name crosses
> the line_length limit), -C formats.

I've added only one test, which now feels like more than enough.
However, I did manually confirm that this works in 1-column mode, too.
I expect to push this today.

>From 2b5393cc0d38c315f71c301a7a3566a19395bfc3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 31 Dec 2008 19:17:31 +0100
Subject: [PATCH] ls: clean up after wrapped+colored file names with clear-to-EOL

This change addresses a relatively unusual case: ls --color, with
a highlighted name appearing first in the last row of a terminal
emulator (possibly followed by other lines of output) such that it
is wrapped onto the following line, as the terminal emulator scrolls
the output.  That would cause the entire following line to be
highlighted, even if the name happened to use only one position.
The least-invasive patch would made colorized output larger for
all uses.  The approach taken below is more invasive, but limits
the increase in overhead to lines that are expected to wrap.
* src/ls.c (enum indicator_no): Add C_CLR_TO_EOL.
(indicator_name): Add "cl".
(color_indicator): Add default escape codes for "cl".
(print_long_format): Propagate width to print_name_with_quoting.
(print_name_with_quoting): Print new C_CLR_TO_EOL string if needed.
Return the width of what we're printing.
(print_file_name_and_frills): Propagate width.
(print_type_indicator): Return bool (aka width).
(print_many_per_line): Pass column position to print_* function.
(print_current_files): Likewise.
(print_horizontal): Likewise.
(print_with_commas): Likewise.
* src/dircolors.c (slack_codes): Add "CLRTOEOL".
(ls_codes): Add "cl".
* tests/ls/color-clear-to-eol: New file.  Test for this fix.
* tests/Makefile.am (TESTS): Add ls/color-clear-to-eol.
* THANKS: Update.
Reported by Alexander V. Lukyanov.  See thread for details:
http://thread.gmane.org/gmane.linux.kernel/740021/focus=14824
Thanks to Jan Engelhardt for helping me reproduce the problem.

Demonstrate with this in an 80-column xterm:
  seq 200 # to start in the "bottom" row
  touch zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.foo
  env LS_COLORS='*.foo=0;31;42' ls -og --color=always

Before the fix, you'd see something like this:
(where the file name is printed in red on a green background,
and each "=" denotes a space on a green background)
...
  -rw-r--r--  1       0 Feb  5 11:31 zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz\
  zzzzzzzzzzzzzz.foo===================================================

After the patch, the trailing green spaces are gone:

  -rw-r--r--  1       0 Feb  5 11:31 zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz\
  zzzzzzzzzzzzzz.foo
---
 THANKS                      |    2 +
 src/dircolors.c             |    4 +-
 src/ls.c                    |   67 ++++++++++++++++++++++++++----------------
 tests/Makefile.am           |    1 +
 tests/ls/color-clear-to-eol |   41 ++++++++++++++++++++++++++
 5 files changed, 87 insertions(+), 28 deletions(-)
 create mode 100755 tests/ls/color-clear-to-eol

diff --git a/THANKS b/THANKS
index 13faf10..da1fc15 100644
--- a/THANKS
+++ b/THANKS
@@ -22,6 +22,7 @@ Albert Hopkins                      address@hidden
 Alberto Accomazzi                   address@hidden
 aldomel                             address@hidden
 Alen Muzinic                        address@hidden
+Alexander V. Lukyanov               address@hidden
 Allen Hewes                         address@hidden
 Axel Dörfler                        address@hidden
 Alexandre Duret-Lutz                address@hidden
@@ -243,6 +244,7 @@ James Tanis                         address@hidden
 James Youngman                      address@hidden
 Jamie Lokier                        address@hidden
 Jamie McClelland                    address@hidden
+Jan Engelhardt                      address@hidden
 Jan Fedak                           address@hidden
 Jan Moringen                        address@hidden
 Jan Nieuwenhuizen                   address@hidden
diff --git a/src/dircolors.c b/src/dircolors.c
index 0980e3d..7dad7fd 100644
--- a/src/dircolors.c
+++ b/src/dircolors.c
@@ -64,14 +64,14 @@ static const char *const slack_codes[] =
   "CHR", "CHAR", "DOOR", "EXEC", "LEFT", "LEFTCODE", "RIGHT", "RIGHTCODE",
   "END", "ENDCODE", "SUID", "SETUID", "SGID", "SETGID", "STICKY",
   "OTHER_WRITABLE", "OWR", "STICKY_OTHER_WRITABLE", "OWT", "CAPABILITY",
-  "HARDLINK", NULL
+  "HARDLINK", "CLRTOEOL", NULL
 };

 static const char *const ls_codes[] =
 {
   "no", "no", "fi", "rs", "di", "ln", "ln", "ln", "or", "mi", "pi", "pi",
   "so", "bd", "bd", "cd", "cd", "do", "ex", "lc", "lc", "rc", "rc", "ec", "ec",
-  "su", "su", "sg", "sg", "st", "ow", "ow", "tw", "tw", "ca", "hl", NULL
+  "su", "su", "sg", "sg", "st", "ow", "ow", "tw", "tw", "ca", "hl", "cl", NULL
 };
 #define array_len(Array) (sizeof (Array) / sizeof *(Array))
 verify (array_len (slack_codes) == array_len (ls_codes));
diff --git a/src/ls.c b/src/ls.c
index b03aebc..59d1fae 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -231,18 +231,21 @@ static size_t calculate_columns (bool by_columns);
 static void print_current_files (void);
 static void print_dir (char const *name, char const *realname,
                       bool command_line_arg);
-static void print_file_name_and_frills (const struct fileinfo *f);
+static size_t print_file_name_and_frills (const struct fileinfo *f,
+                                         size_t start_col);
 static void print_horizontal (void);
 static int format_user_width (uid_t u);
 static int format_group_width (gid_t g);
 static void print_long_format (const struct fileinfo *f);
 static void print_many_per_line (void);
-static void print_name_with_quoting (const char *p, mode_t mode,
-                                    int linkok, bool stat_ok,
-                                    enum filetype type,
-                                    struct obstack *stack, nlink_t nlink);
+static size_t print_name_with_quoting (const char *p, mode_t mode,
+                                      int linkok, bool stat_ok,
+                                      enum filetype type,
+                                      struct obstack *stack,
+                                      nlink_t nlink,
+                                      size_t start_col);
 static void prep_non_filename_text (void);
-static void print_type_indicator (bool stat_ok, mode_t mode,
+static bool print_type_indicator (bool stat_ok, mode_t mode,
                                  enum filetype type);
 static void print_with_commas (void);
 static void queue_directory (char const *name, char const *realname,
@@ -521,14 +524,15 @@ enum indicator_no
     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, C_CAP, C_HARDLINK
+    C_STICKY, C_OTHER_WRITABLE, C_STICKY_OTHER_WRITABLE, C_CAP, C_HARDLINK,
+    C_CLR_TO_EOL
   };

 static const char *const indicator_name[]=
   {
     "lc", "rc", "ec", "rs", "no", "fi", "di", "ln", "pi", "so",
     "bd", "cd", "mi", "or", "ex", "do", "su", "sg", "st",
-    "ow", "tw", "ca", "hl", NULL
+    "ow", "tw", "ca", "hl", "cl", NULL
   };

 struct color_ext_type
@@ -563,6 +567,7 @@ static struct bin_str color_indicator[] =
     { LEN_STR_PAIR ("30;42") },                /* tw: ow w/ sticky: black on 
green */
     { LEN_STR_PAIR ("30;41") },                /* ca: black on red */
     { LEN_STR_PAIR ("44;37") },                /* hl: white on blue */
+    { LEN_STR_PAIR ("\033[K") },       /* cl: clear to end of line */
   };

 /* FIXME: comment  */
@@ -3284,7 +3289,7 @@ print_current_files (void)
     case one_per_line:
       for (i = 0; i < cwd_n_used; i++)
        {
-         print_file_name_and_frills (sorted_file[i]);
+         print_file_name_and_frills (sorted_file[i], 0);
          putchar ('\n');
        }
       break;
@@ -3636,9 +3641,9 @@ print_long_format (const struct fileinfo *f)
     }

   DIRED_FPUTS (buf, stdout, p - buf);
-  print_name_with_quoting (f->name, FILE_OR_LINK_MODE (f), f->linkok,
-                          f->stat_ok, f->filetype, &dired_obstack,
-                          f->stat.st_nlink);
+  size_t w = print_name_with_quoting (f->name, FILE_OR_LINK_MODE (f), 
f->linkok,
+                                     f->stat_ok, f->filetype, &dired_obstack,
+                                     f->stat.st_nlink, p - buf);

   if (f->filetype == symbolic_link)
     {
@@ -3647,7 +3652,7 @@ print_long_format (const struct fileinfo *f)
          DIRED_FPUTS_LITERAL (" -> ", stdout);
          print_name_with_quoting (f->linkname, f->linkmode, f->linkok - 1,
                                   f->stat_ok, f->filetype, NULL,
-                                  f->stat.st_nlink);
+                                  f->stat.st_nlink, (p - buf) + w + 4);
          if (indicator_style != none)
            print_type_indicator (true, f->linkmode, unknown);
        }
@@ -3822,10 +3827,11 @@ quote_name (FILE *out, const char *name, struct 
quoting_options const *options,
   return len;
 }

-static void
+static size_t
 print_name_with_quoting (const char *p, mode_t mode, int linkok,
                         bool stat_ok, enum filetype type,
-                        struct obstack *stack, nlink_t nlink)
+                        struct obstack *stack, nlink_t nlink,
+                        size_t start_col)
 {
   bool used_color_this_time
     = (print_with_color
@@ -3834,7 +3840,8 @@ print_name_with_quoting (const char *p, mode_t mode, int 
linkok,
   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);

-  dired_pos += quote_name (stdout, p, filename_quoting_options, NULL);
+  size_t width = quote_name (stdout, p, filename_quoting_options, NULL);
+  dired_pos += width;

   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);
@@ -3843,7 +3850,11 @@ print_name_with_quoting (const char *p, mode_t mode, int 
linkok,
     {
       process_signals ();
       prep_non_filename_text ();
+      if (start_col / line_length != (start_col + width - 1) / line_length)
+       put_indicator (&color_indicator[C_CLR_TO_EOL]);
     }
+
+  return width;
 }

 static void
@@ -3863,8 +3874,8 @@ prep_non_filename_text (void)
    Also print file size, inode number, and filetype indicator character,
    as requested by switches.  */

-static void
-print_file_name_and_frills (const struct fileinfo *f)
+static size_t
+print_file_name_and_frills (const struct fileinfo *f, size_t start_col)
 {
   char buf[MAX (LONGEST_HUMAN_READABLE + 1, INT_BUFSIZE_BOUND (uintmax_t))];

@@ -3880,11 +3891,14 @@ print_file_name_and_frills (const struct fileinfo *f)
   if (print_scontext)
     printf ("%*s ", format == with_commas ? 0 : scontext_width, f->scontext);

-  print_name_with_quoting (f->name, FILE_OR_LINK_MODE (f), f->linkok,
-                          f->stat_ok, f->filetype, NULL, f->stat.st_nlink);
+  size_t width = print_name_with_quoting (f->name, FILE_OR_LINK_MODE (f),
+                                         f->linkok, f->stat_ok, f->filetype,
+                                         NULL, f->stat.st_nlink, start_col);

   if (indicator_style != none)
-    print_type_indicator (f->stat_ok, f->stat.st_mode, f->filetype);
+    width += print_type_indicator (f->stat_ok, f->stat.st_mode, f->filetype);
+
+  return width;
 }

 /* Given these arguments describing a file, return the single-byte
@@ -3921,12 +3935,13 @@ get_type_indicator (bool stat_ok, mode_t mode, enum 
filetype type)
   return c;
 }

-static void
+static bool
 print_type_indicator (bool stat_ok, mode_t mode, enum filetype type)
 {
   char c = get_type_indicator (stat_ok, mode, type);
   if (c)
     DIRED_PUTCHAR (c);
+  return !!c;
 }

 #ifdef HAVE_CAP
@@ -4128,7 +4143,7 @@ print_many_per_line (void)
          struct fileinfo const *f = sorted_file[filesno];
          size_t name_length = length_of_file_name_and_frills (f);
          size_t max_name_length = line_fmt->col_arr[col++];
-         print_file_name_and_frills (f);
+         print_file_name_and_frills (f, pos);

          filesno += rows;
          if (filesno >= cwd_n_used)
@@ -4153,7 +4168,7 @@ print_horizontal (void)
   size_t max_name_length = line_fmt->col_arr[0];

   /* Print first entry.  */
-  print_file_name_and_frills (f);
+  print_file_name_and_frills (f, 0);

   /* Now the rest.  */
   for (filesno = 1; filesno < cwd_n_used; ++filesno)
@@ -4172,7 +4187,7 @@ print_horizontal (void)
        }

       f = sorted_file[filesno];
-      print_file_name_and_frills (f);
+      print_file_name_and_frills (f, pos);

       name_length = length_of_file_name_and_frills (f);
       max_name_length = line_fmt->col_arr[col];
@@ -4210,7 +4225,7 @@ print_with_commas (void)
          putchar (separator);
        }

-      print_file_name_and_frills (f);
+      print_file_name_and_frills (f, pos);
       pos += len;
     }
   putchar ('\n');
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 99d2856..024eb48 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -325,6 +325,7 @@ TESTS =                                             \
   ln/misc                                      \
   ln/sf-1                                      \
   ln/target-1                                  \
+  ls/color-clear-to-eol                                \
   ls/color-dtype-dir                           \
   ls/dangle                                    \
   ls/dired                                     \
diff --git a/tests/ls/color-clear-to-eol b/tests/ls/color-clear-to-eol
new file mode 100755
index 0000000..ff9530d
--- /dev/null
+++ b/tests/ls/color-clear-to-eol
@@ -0,0 +1,41 @@
+#!/bin/sh
+# ensure that ls --color works well when a colored name is wrapped
+
+# Copyright (C) 2009 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/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  ls --version
+fi
+
+. $srcdir/test-lib.sh
+
+long_name=zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.foo
+touch $long_name || framework_failure
+
+color_code='0;31;42'
+c_pre='\e[0m\e['"${color_code}m"
+c_post='\e[0m\e[K\n\e[m'
+printf "$c_pre$long_name$c_post" > exp || framework_failure
+
+fail=0
+env TERM=xterm COLUMNS=80 LS_COLORS="*.foo=$color_code" TIME_STYLE=+T \
+  ls -og --color=always $long_name > out || fail=1
+sed 's/.*T //' out > k && mv k out
+
+compare out exp || fail=1
+
+Exit $fail
--
1.6.2.rc0.186.g417c