bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: ls --color=always and unterminated last line of output


From: Jim Meyering
Subject: Re: ls --color=always and unterminated last line of output
Date: Thu, 31 Dec 2009 20:17:35 +0100

C de-Avillez wrote:

> Original Ubuntu bug:
> https://bugs.launchpad.net/ubuntu/+source/coreutils/+bug/494663
>
> Reporter complains last line of a 'ls --color=always' is unterminated,
> causing grief on a pipe to 'ls -FXR'.
>
> Indeed it is unterminated:
>
> address@hidden:/usr/src/buildd/coreutils/src$ ls --color=always|tail -2 |
> od -Xc
> 0000000        2e736579        5b1b0a6f        0000006d
>           y   e   s   .   o  \n 033   [   m
> 0000011
> address@hidden:/usr/src/buildd/coreutils/src$
>
> I am not sure this *is* a bug, and would like input from the list.
>
> A _few_ tests I performed, on a hacked version of 'ls' where I took out
> the resetting of colours, do not show any impact in terms of terminal
> colouring. Obviously, these tests are not even near being a  complete
> test set
>
> On the other hand, the only usage I can see for a 'ls --color=always' is
> to produce human output -- for example, as the reporter states -- 'ls
> --color=always | less -FXR'. I am pretty sure an human can discard an
> empty last line.

Here's a potential solution.
The ls.c change not pretty, but I think it is correct.
I'm not sure this belongs in coreutils-8.3, but I'm considering it.

>From bbeb3d4f3db5366fbfed6675e964d47dc9efa9d3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 31 Dec 2009 20:09:06 +0100
Subject: [PATCH] ls --color: don't emit a final no-op escape sequence

* src/ls.c (main): With --color, avoid emitting the final color-
resetting escape sequence when it would be a no-op.
* tests/ls/color-clear-to-eol: Adjust expected output accordingly.
* tests/ls/color-dtype-dir: Likewise.
* tests/ls/multihardlink: Likewise.
* tests/ls/stat-free-symlinks: Likewise.
* tests/misc/ls-misc: Likewise.
Reported by Jim Avera in
http://bugs.launchpad.net/ubuntu/+source/coreutils/+bug/494663
---
 src/ls.c                    |   10 +++++++++-
 tests/ls/color-clear-to-eol |    2 +-
 tests/ls/color-dtype-dir    |    4 ----
 tests/ls/multihardlink      |   11 +++++------
 tests/ls/stat-free-symlinks |    1 -
 tests/misc/ls-misc          |   11 +++++------
 6 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index 6c1f275..fd9b382 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1438,7 +1438,15 @@ main (int argc, char **argv)
       int j;

       if (used_color)
-        restore_default_color ();
+        {
+          /* Skip the restore when it would be a no-op, i.e.,
+             when left is "\033[" and right is "m".  */
+          if (!(color_indicator[C_LEFT].len == 2
+                && memcmp (color_indicator[C_LEFT].string, "\033[", 2) == 0
+                && color_indicator[C_RIGHT].len == 1
+                && color_indicator[C_RIGHT].string[0] == 'm'))
+            restore_default_color ();
+        }
       fflush (stdout);

       /* Restore the default signal handling.  */
diff --git a/tests/ls/color-clear-to-eol b/tests/ls/color-clear-to-eol
index aa8cf4e..baa8c18 100755
--- a/tests/ls/color-clear-to-eol
+++ b/tests/ls/color-clear-to-eol
@@ -29,7 +29,7 @@ touch $long_name || framework_failure
 e='\33'
 color_code='0;31;42'
 c_pre="$e[0m$e[${color_code}m"
-c_post="$e[0m$e[K\n$e[m"
+c_post="$e[0m$e[K\n"
 printf "$c_pre$long_name$c_post\n" > exp || framework_failure

 env TERM=xterm COLUMNS=80 LS_COLORS="*.foo=$color_code" TIME_STYLE=+T \
diff --git a/tests/ls/color-dtype-dir b/tests/ls/color-dtype-dir
index 526d9e9..9ad282b 100755
--- a/tests/ls/color-dtype-dir
+++ b/tests/ls/color-dtype-dir
@@ -36,7 +36,6 @@ chmod o+t sticky || framework_failure

 ls --color=always > out || fail=1
 cat -A out > o1 || fail=1
-echo >> o1 || fail=1
 mv o1 out || fail=1

 cat <<\EOF > exp || fail=1
@@ -44,7 +43,6 @@ cat <<\EOF > exp || fail=1
 ^[[34;42mother-writable^[[0m$
 out$
 ^[[37;44msticky^[[0m$
-^[[m
 EOF

 compare out exp || fail=1
@@ -56,7 +54,6 @@ rm exp

 LS_COLORS="ow=:" ls --color=always > out || fail=1
 cat -A out > o1 || fail=1
-echo >> o1 || fail=1
 mv o1 out || fail=1

 cat <<\EOF > exp || fail=1
@@ -64,7 +61,6 @@ cat <<\EOF > exp || fail=1
 ^[[01;34mother-writable^[[0m$
 out$
 ^[[37;44msticky^[[0m$
-^[[m
 EOF

 compare out exp || fail=1
diff --git a/tests/ls/multihardlink b/tests/ls/multihardlink
index cbe6330..c6b078c 100755
--- a/tests/ls/multihardlink
+++ b/tests/ls/multihardlink
@@ -30,7 +30,6 @@ code_mh='44;37'
 code_ex='01;32'
 code_png='01;35'
 c0=$(printf '\033[0m')
-c_end=$(printf '\033[m')
 c_mh=$(printf '\033[%sm' $code_mh)
 c_ex=$(printf '\033[%sm' $code_ex)
 c_png=$(printf '\033[%sm' $code_png)
@@ -44,7 +43,7 @@ compare out out_ok || fail=1
 LS_COLORS="mh=$code_mh" ls -U1 --color=always file1 file2 > out || fail=1
 printf "$c0${c_mh}file1$c0
 ${c_mh}file2$c0
-$c_end" > out_ok || framework_failure
+" > out_ok || framework_failure
 compare out out_ok || fail=1

 # hard links and png (hard link coloring takes precedence)
@@ -53,7 +52,7 @@ LS_COLORS="mh=$code_mh:*.png=$code_png" ls -U1 --color=always 
file1 file2.png \
   > out || fail=1
 printf "$c0${c_mh}file1$c0
 ${c_mh}file2.png$c0
-$c_end" > out_ok || framework_failure
+" > out_ok || framework_failure
 compare out out_ok || fail=1

 # hard links and exe (exe coloring takes precedence)
@@ -63,7 +62,7 @@ LS_COLORS="mh=$code_mh:*.png=$code_png:ex=$code_ex" \
 chmod a-x file2.png || framework_failure
 printf "$c0${c_ex}file1$c0
 ${c_ex}file2.png$c0
-$c_end" > out_ok || framework_failure
+" > out_ok || framework_failure
 compare out out_ok || fail=1

 # hard links and png (hard link coloring disabled => png coloring enabled)
@@ -71,7 +70,7 @@ LS_COLORS="mh=00:*.png=$code_png" ls -U1 --color=always file1 
file2.png > out \
   || fail=1
 printf "file1
 $c0${c_png}file2.png$c0
-$c_end" > out_ok || framework_failure
+" > out_ok || framework_failure
 compare out out_ok || fail=1

 # hard links and png (hard link coloring not enabled explicitly => png 
coloring)
@@ -79,7 +78,7 @@ LS_COLORS="*.png=$code_png" ls -U1 --color=always file1 
file2.png > out \
   || fail=1
 printf "file1
 $c0${c_png}file2.png$c0
-$c_end" > out_ok || framework_failure
+" > out_ok || framework_failure
 compare out out_ok || fail=1

 Exit $fail
diff --git a/tests/ls/stat-free-symlinks b/tests/ls/stat-free-symlinks
index 86d871e..59d533f 100755
--- a/tests/ls/stat-free-symlinks
+++ b/tests/ls/stat-free-symlinks
@@ -46,7 +46,6 @@ grep '^stat("x"' err && fail=1
 {
   printf '\033[0m\033[01;address@hidden'
   printf '\033[01;32mx\033[0m*\n'
-  printf '\033[m'
 } > exp || fail=1

 compare out exp || fail=1
diff --git a/tests/misc/ls-misc b/tests/misc/ls-misc
index a734d5f..82eda11 100755
--- a/tests/misc/ls-misc
+++ b/tests/misc/ls-misc
@@ -156,10 +156,10 @@ my @Tests =

      # Test for a bug that was fixed in coreutils-4.5.4.
      ['sl-F-color', '-F --color=always d',
-                                 {OUT => "$e\e[01;address@hidden"},
+                                 {OUT => "$e\e[01;address@hidden"},
                                   $slink_d, $unlink_d],
      ['sl-dF-color', '-dF --color=always d',
-                                 {OUT => "$e\e[01;address@hidden"},
+                                 {OUT => "$e\e[01;address@hidden"},
                                   $slink_d, $unlink_d],

      # A listing with no output should have no color sequences at all.
@@ -169,12 +169,12 @@ my @Tests =

      # Test for a bug fixed after coreutils-6.9.
      ['sl-target', '--color=always d',
-      {OUT => "$e\e[01;34mX$e\n\e[m"}, $target, $target2],
+      {OUT => "$e\e[01;34mX$e\n"}, $target, $target2],

      # Test for another bug fixed after coreutils-6.9.
      # This one bites only for a system/file system with d_type support.
      ['sl-dangle', '--color=always d',
-      {OUT => "$e\e[40;31;01mX$e\n\e[m"},
+      {OUT => "$e\e[40;31;01mX$e\n"},
       {PRE => sub {
                 mkdir 'd',0755 or die "d: $!\n";
                 symlink 'non-existent', 'd/X' or die "d/X: $!\n";
@@ -189,7 +189,7 @@ my @Tests =
      # To demonstrate it, the file in question (with executable bit set)
      # must not be a command line argument.
      ['color-exe1', '--color=always j',
-                                 {OUT => "$e\e[01;32md$e\n\e[m"},
+                                 {OUT => "$e\e[01;32md$e\n"},
                                   $exe_in_subdir, $remove_j],

      # From St├ęphane Chazelas.
@@ -207,7 +207,6 @@ my @Tests =
             . "\e[30;43msetgid$e\n"
             . "\e[37;41msetuid$e\n"
             . "\e[37;44msticky$e\n"
-            . "\e[m"
          },

         {POST => sub {
--
1.6.6.325.g6f5f




reply via email to

[Prev in Thread] Current Thread [Next in Thread]