ls: with --group-directories-first, also group symlinked dirs

* src/ls.c (is_linked_directory): A new function to
also consider symlinked directories.
(main): Rename check_symlink_color to check_symlink_mode,
and enable that with --group-directories-first.
(DIRFIRST_CHECK): Adjust to use is_linked_directory,
rather than just is_directory.
(gobble_file): Simplify to always update f->linkmode
if the stat() succeeds.
* tests/ls/group-dirs.sh: A new test.
* tests/local.mk: Reference the new test.
* NEWS: Mention the change in behavior.
Suggested by Amin Bandali in
https://lists.gnu.org/r/coreutils/2018-12/msg00017.html

diff --git a/NEWS b/NEWS
index 6a9c0bca2fb51532329b6796758b0fc84d98e010..ca296b09662c0b2e4acd9de98793aed2a5d00a2b 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -38,6 +38,8 @@ GNU coreutils NEWS                                    -*- outline -*-
   approach is still used in situations where hard links to directories
   are allowed (e.g., NetBSD when superuser).
 
+  ls --group-directories-first will also group symlinks to directories.
+
   'test -a FILE' is not supported anymore.  Long ago, there were concerns about
   the high probability of humans confusing the -a primary with the -a binary
   operator, so POSIX changed this to 'test -e FILE'.  Scripts using it were

diff --git a/src/ls.c b/src/ls.c
index 5ad9bc2db8c828de002275a259a014ed771dca68..243c1860a34cc5829e3a837b0a0befc52481cbd1 100644 (file)
--- a/src/ls.c
+++ b/src/ls.c
@@ -640,9 +640,9 @@ static struct color_ext_type *color_ext_list = NULL;
 static char *color_buf;
 
 /* True means to check for orphaned symbolic link, for displaying
-   colors.  */
+   colors, or to group symlink to directories with other dirs.  */
 
-static bool check_symlink_color;
+static bool check_symlink_mode;
 
 /* True means mention the inode number of each file.  -i  */
 
@@ -1477,13 +1477,15 @@ main (int argc, char **argv)
 
   /* Test print_with_color again, because the call to parse_ls_color
      may have just reset it -- e.g., if LS_COLORS is invalid.  */
-  if (print_with_color)
+  if (directories_first)
+    check_symlink_mode = true;
+  else if (print_with_color)
     {
       /* Avoid following symbolic links when possible.  */
       if (is_colored (C_ORPHAN)
           || (is_colored (C_EXEC) && color_symlink_as_referent)
           || (is_colored (C_MISSING) && format == long_format))
-        check_symlink_color = true;
+        check_symlink_mode = true;
     }
 
   if (dereference == DEREF_UNDEFINED)
@@ -3149,7 +3151,7 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
       || ((print_inode || format_needs_type)
           && (type == symbolic_link || type == unknown)
           && (dereference == DEREF_ALWAYS
-              || color_symlink_as_referent || check_symlink_color))
+              || color_symlink_as_referent || check_symlink_mode))
       /* Command line dereferences are already taken care of by the above
          assertion that the inode number is not yet known.  */
       || (print_inode && inode == NOT_AN_INODE_NUMBER)
@@ -3300,7 +3302,7 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
         }
 
       if (S_ISLNK (f->stat.st_mode)
-          && (format == long_format || check_symlink_color))
+          && (format == long_format || check_symlink_mode))
         {
           struct stat linkstats;
 
@@ -3315,21 +3317,11 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
           /* Avoid following symbolic links when possible, ie, when
              they won't be traced and when no indicator is needed.  */
           if (linkname
-              && (file_type <= indicator_style || check_symlink_color)
+              && (file_type <= indicator_style || check_symlink_mode)
               && stat (linkname, &linkstats) == 0)
             {
               f->linkok = true;
-
-              /* Symbolic links to directories that are mentioned on the
-                 command line are automatically traced if not being
-                 listed as files.  */
-              if (!command_line_arg || format == long_format
-                  || !S_ISDIR (linkstats.st_mode))
-                {
-                  /* Get the linked-to file's mode for the filetype indicator
-                     in long listings.  */
-                  f->linkmode = linkstats.st_mode;
-                }
+              f->linkmode = linkstats.st_mode;
             }
           free (linkname);
         }
@@ -3443,6 +3435,14 @@ is_directory (const struct fileinfo *f)
   return f->filetype == directory || f->filetype == arg_directory;
 }
 
+/* Return true if F refers to a (symlinked) directory.  */
+static bool
+is_linked_directory (const struct fileinfo *f)
+{
+  return f->filetype == directory || f->filetype == arg_directory
+         || S_ISDIR (f->linkmode);
+}
+
 /* Put the name of the file that FILENAME is a symbolic link to
    into the LINKNAME field of 'f'.  COMMAND_LINE_ARG indicates whether
    FILENAME is a command-line argument.  */
@@ -3588,8 +3588,8 @@ typedef int (*qsortFunc)(V a, V b);
 #define DIRFIRST_CHECK(a, b)                                           \
   do                                                                   \
     {                                                                  \
-      bool a_is_dir = is_directory ((struct fileinfo const *) a);      \
-      bool b_is_dir = is_directory ((struct fileinfo const *) b);      \
+      bool a_is_dir = is_linked_directory ((struct fileinfo const *) a);\
+      bool b_is_dir = is_linked_directory ((struct fileinfo const *) b);\
       if (a_is_dir && !b_is_dir)                                       \
         return -1;         /* a goes before b */                       \
       if (!a_is_dir && b_is_dir)                                       \

diff --git a/tests/local.mk b/tests/local.mk
index e494eff4e79b546f9a02d347c892f0746584121b..290e30f6296c7ab39f02ba1e5d278aa5d44b4b44 100644 (file)
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -595,6 +595,7 @@ all_tests =                                 \
   tests/ls/file-type.sh                                \
   tests/ls/follow-slink.sh                     \
   tests/ls/getxattr-speedup.sh                 \
+  tests/ls/group-dirs.sh                       \
   tests/ls/hex-option.sh                       \
   tests/ls/infloop.sh                          \
   tests/ls/inode.sh                            \

diff --git a/tests/ls/group-dirs.sh b/tests/ls/group-dirs.sh
new file mode 100755 (executable)
index 0000000..b3b266b
--- /dev/null
+++ b/tests/ls/group-dirs.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+# test --group-directories-first
+
+# Copyright (C) 2018 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 <https://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ ls
+
+# Isolate output files from directory being listed
+mkdir dir dir/b || framework_failure_
+touch dir/a || framework_failure_
+ln -s b dir/bl || framework_failure_
+
+ls --group dir > out || fail=1
+cat <<\EOF > exp
+b
+bl
+a
+EOF
+compare exp out || fail=1
+
+ls --group -d dir/* > out || fail=1
+cat <<\EOF > exp
+dir/b
+dir/bl
+dir/a
+EOF
+compare exp out || fail=1
+
+Exit $fail