Re: Not sure how to best reply re: dir_colors situation

[Jim Meyering]

Kamil Dudka wrote:
> Subject: [PATCH] ls --color: do not colorize files with multiple hard links 
> by default
>
> * src/ls.c: Rename hl->mh, do not colorize files with multiple hard links
> by default.
> * src/dircolors.c: Rename HARDLINK -> MULTIHARDLINK, hl -> mh.
> * src/dircolors.hin: Do not colorize files with multiple hard links by 
> default.
> * tests/Makefile.am: Rename the test case accordingly.
> * tests/ls/multihardlink: Additionally test default ls' behavior.
> * NEWS: Mention the change in behavior.
...

Thanks, Kamil, and thanks to Pádraig for reviewing it.
I've shortened log message lines to fit within 72-col
limit (remember: they get TAB prepended in the generated
ChangeLog file).

Also there was too much duplication in that test,
so I factored it.

First the incremental:

diff --git a/tests/ls/multihardlink b/tests/ls/multihardlink
index a00664d..d237a71 100755
--- a/tests/ls/multihardlink
+++ b/tests/ls/multihardlink
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Ensure "ls --color" properly colorizes hard linked files.

-# Copyright (C) 2008 Free Software Foundation, Inc.
+# Copyright (C) 2008-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
@@ -28,6 +28,10 @@ touch file file1 || framework_failure
 ln file1 file2 || skip_test_ "can't create hard link"
 code_mh='44;37'
 code_png='01;35'
+c0=$(printf '\033[0m')
+c_end=$(printf '\033[m')
+c_mh=$(printf '\033[%sm' $code_mh)
+c_png=$(printf '\033[%sm' $code_png)
 fail=0

 # regular file - not hard linked
@@ -37,34 +41,34 @@ compare out out_ok || fail=1

 # hard links
 LS_COLORS="mh=$code_mh" ls -U1 --color=always file1 file2 > out || fail=1
-printf "\033[0m\033[44;37mfile1\033[0m
-\033[44;37mfile2\033[0m
-\033[m" > out_ok || framework_failure
+printf "$c0${c_mh}file1$c0
+${c_mh}file2$c0
+$c_end" > out_ok || framework_failure
 compare out out_ok || fail=1

 # hard links and png (hard link coloring takes precedence)
 mv file2 file2.png || framework_failure
 LS_COLORS="mh=$code_mh:*.png=$code_png" ls -U1 --color=always file1 file2.png \
   > out || fail=1
-printf "\033[0m\033[44;37mfile1\033[0m
-\033[44;37mfile2.png\033[0m
-\033[m" > out_ok || framework_failure
+printf "$c0${c_mh}file1$c0
+${c_mh}file2.png$c0
+$c_end" > out_ok || framework_failure
 compare out out_ok || fail=1

 # hard links and png (hard link coloring disabled => png coloring enabled)
 LS_COLORS="mh=00:*.png=$code_png" ls -U1 --color=always file1 file2.png > out \
   || fail=1
 printf "file1
-\033[0m\033[01;35mfile2.png\033[0m
-\033[m" > out_ok || framework_failure
+$c0${c_png}file2.png$c0
+$c_end" > out_ok || framework_failure
 compare out out_ok || fail=1

 # hard links and png (hard link coloring not enabled explicitly => png 
coloring)
 LS_COLORS="*.png=$code_png" ls -U1 --color=always file1 file2.png > out \
   || fail=1
 printf "file1
-\033[0m\033[01;35mfile2.png\033[0m
-\033[m" > out_ok || framework_failure
+$c0${c_png}file2.png$c0
+$c_end" > out_ok || framework_failure
 compare out out_ok || fail=1

 Exit $fail


Kamil,
I've merged that into yours.
Here's the result that I'll push once you have a chance to look at it:

>From 0df338f6719ec2bcf1e1dea2d8b12dc66daf8a1e Mon Sep 17 00:00:00 2001
From: Kamil Dudka <address@hidden>
Date: Wed, 10 Jun 2009 19:44:43 +0200
Subject: [PATCH] ls --color: do not colorize files with multiple hard links by 
default

* src/ls.c: Rename hl->mh, do not colorize files with multiple
hard links by default.
* src/dircolors.c: Rename HARDLINK -> MULTIHARDLINK, hl -> mh.
* src/dircolors.hin: Do not colorize files with multiple hard links by
default.
* tests/Makefile.am: Rename the test case accordingly.
* tests/ls/multihardlink: Additionally test ls' default behavior
and factor out some duplication.
* NEWS: Mention the change in behavior.
---
 NEWS                   |    9 ++++++
 src/dircolors.c        |    4 +-
 src/dircolors.hin      |    2 +-
 src/ls.c               |   10 +++---
 tests/Makefile.am      |    2 +-
 tests/ls/hardlink      |   60 --------------------------------------
 tests/ls/multihardlink |   74 ++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 92 insertions(+), 69 deletions(-)
 delete mode 100755 tests/ls/hardlink
 create mode 100755 tests/ls/multihardlink

diff --git a/NEWS b/NEWS
index 29b09a0..0455d59 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,15 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   truncate -s failed to skip all whitespace in the option argument in
   some locales.

+** Changes in behavior
+
+  ls --color: files with multiple hard links are no longer colored differently
+  by default. That can be enabled by changing the LS_COLORS environment
+  variable. You can control that using the MULTIHARDLINK dircolors input
+  variable which corresponds to the 'mh' LS_COLORS item. Note these variables
+  were renamed from 'HARDLINK' and 'hl' which were available since
+  coreutils-7.1 when this feature was introduced.
+
 ** New features

   chroot now accepts the options --userspec and --groups.
diff --git a/src/dircolors.c b/src/dircolors.c
index f01d557..39c8e64 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", "CLRTOEOL", NULL
+  "MULTIHARDLINK", "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", "cl", NULL
+  "su", "su", "sg", "sg", "st", "ow", "ow", "tw", "tw", "ca", "mh", "cl", NULL
 };
 verify (ARRAY_CARDINALITY (slack_codes) == ARRAY_CARDINALITY (ls_codes));

diff --git a/src/dircolors.hin b/src/dircolors.hin
index 5621259..30fd878 100644
--- a/src/dircolors.hin
+++ b/src/dircolors.hin
@@ -70,7 +70,7 @@ RESET 0               # reset to "normal" color
 DIR 01;34      # directory
 LINK 01;36     # symbolic link.  (If you set this to 'target' instead of a
                # numerical value, the color is as for the file pointed to.)
-HARDLINK 44;37 # regular file with more than one link
+MULTIHARDLINK 00       # regular file with more than one link
 FIFO 40;33     # pipe
 SOCK 01;35     # socket
 DOOR 01;35     # door
diff --git a/src/ls.c b/src/ls.c
index 838431c..48bc47e 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -529,7 +529,7 @@ 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_MULTIHARDLINK,
     C_CLR_TO_EOL
   };

@@ -537,7 +537,7 @@ 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", "cl", NULL
+    "ow", "tw", "ca", "mh", "cl", NULL
   };

 struct color_ext_type
@@ -571,7 +571,7 @@ static struct bin_str color_indicator[] =
     { LEN_STR_PAIR ("34;42") },                /* ow: other-writable: blue on 
green */
     { 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 */
+    { 0, NULL },                       /* mh: disabled by default */
     { LEN_STR_PAIR ("\033[K") },       /* cl: clear to end of line */
   };

@@ -4106,8 +4106,8 @@ print_color_indicator (const char *name, mode_t mode, int 
linkok,
            type = C_CAP;
          else if ((mode & S_IXUGO) != 0)
            type = C_EXEC;
-         else if (is_colored (C_HARDLINK) && (1 < nlink))
-           type = C_HARDLINK;
+         else if (is_colored (C_MULTIHARDLINK) && (1 < nlink))
+           type = C_MULTIHARDLINK;
        }
       else if (S_ISDIR (mode))
        {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a0ed986..2405ce4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -343,10 +343,10 @@ TESTS =                                           \
   ls/dired                                     \
   ls/file-type                                 \
   ls/follow-slink                              \
-  ls/hardlink                                  \
   ls/infloop                                   \
   ls/inode                                     \
   ls/m-option                                  \
+  ls/multihardlink                             \
   ls/no-arg                                    \
   ls/no-cap                                    \
   ls/proc-selinux-segfault                     \
diff --git a/tests/ls/hardlink b/tests/ls/hardlink
deleted file mode 100755
index b120914..0000000
--- a/tests/ls/hardlink
+++ /dev/null
@@ -1,60 +0,0 @@
-#!/bin/sh
-# Ensure "ls --color" properly colorizes hard linked files.
-
-# Copyright (C) 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
-# 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
-working_umask_or_skip_
-
-touch file file1 || framework_failure
-ln file1 file2 || skip_test_ "can't create hard link"
-code_hl='44;37'
-code_png='01;35'
-fail=0
-
-# regular file - not hard linked
-LS_COLORS="hl=$code_hl" ls -U1 --color=always file > out || fail=1
-printf "file\n" > out_ok || fail=1
-compare out out_ok || fail=1
-
-# hard links
-LS_COLORS="hl=$code_hl" ls -U1 --color=always file1 file2 > out || fail=1
-printf "\033[0m\033[44;37mfile1\033[0m
-\033[44;37mfile2\033[0m
-\033[m" > out_ok || fail=1
-compare out out_ok || fail=1
-
-# hard links and png
-mv file2 file2.png || framework_failure
-LS_COLORS="hl=$code_hl:*.png=$code_png" ls -U1 --color=always file1 file2.png 
> out || fail=1
-printf "\033[0m\033[44;37mfile1\033[0m
-\033[44;37mfile2.png\033[0m
-\033[m" > out_ok || fail=1
-compare out out_ok || fail=1
-
-# hard links and png (hard links highlighting disabled)
-LS_COLORS="hl=:*.png=$code_png" ls -U1 --color=always file1 file2.png > out || 
fail=1
-printf "file1
-\033[0m\033[01;35mfile2.png\033[0m
-\033[m" > out_ok || fail=1
-compare out out_ok || fail=1
-
-Exit $fail
diff --git a/tests/ls/multihardlink b/tests/ls/multihardlink
new file mode 100755
index 0000000..d237a71
--- /dev/null
+++ b/tests/ls/multihardlink
@@ -0,0 +1,74 @@
+#!/bin/sh
+# Ensure "ls --color" properly colorizes hard linked files.
+
+# Copyright (C) 2008-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
+working_umask_or_skip_
+
+touch file file1 || framework_failure
+ln file1 file2 || skip_test_ "can't create hard link"
+code_mh='44;37'
+code_png='01;35'
+c0=$(printf '\033[0m')
+c_end=$(printf '\033[m')
+c_mh=$(printf '\033[%sm' $code_mh)
+c_png=$(printf '\033[%sm' $code_png)
+fail=0
+
+# regular file - not hard linked
+LS_COLORS="mh=$code_mh" ls -U1 --color=always file > out || fail=1
+printf "file\n" > out_ok || framework_failure
+compare out out_ok || fail=1
+
+# hard links
+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
+compare out out_ok || fail=1
+
+# hard links and png (hard link coloring takes precedence)
+mv file2 file2.png || framework_failure
+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
+compare out out_ok || fail=1
+
+# hard links and png (hard link coloring disabled => png coloring enabled)
+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
+compare out out_ok || fail=1
+
+# hard links and png (hard link coloring not enabled explicitly => png 
coloring)
+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
+compare out out_ok || fail=1
+
+Exit $fail
--
1.6.3.2.349.g338ec3