Re: ls --dired : be more proactive with errors [was: Re: ls/test: verify the output of ls --dired]

[Pádraig Brady]

On 17/09/2023 15:00, Sylvestre Ledru wrote:

Le 15/09/2023 à 14:53, Pádraig Brady a écrit :

On 14/09/2023 23:05, Sylvestre Ledru wrote:

Hello

Currently, GNU coreutils doesn't have any test verifying the actual
output of ls --dired.

It should generate a list of position (pairs).

//DIRED// 73 82 142 152 ...

The attached patch creates 2 files and one directory and verify that we
can find their names using the position.

Well the existing test did verify --dired output offsets,
albeit constant offsets from the non varying output.

Thanks.

Here is a second patch to improve the error management.
Currently, "ls --dired" silently fails

I implemented the first suggestion in the FIXME.

Yes silently ignoring --dired isn't ideal.

However I'm thinking --dired implying -l,
and taking precedence if specified last
would be preferable?

That's done in the attached.

cheers,
Pádraig

From e49679e6afb3accfb7f81f6b661686cdecb2c77a Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylves...@debian.org>
Date: Sun, 17 Sep 2023 15:55:57 +0200
Subject: [PATCH] ls: --dired now implies long format with hyperlinks disabled

Currently --dired is silently ignored

* src/ls.c (decode_switches): Set default format and hyperlink mode
when the --dired option is specified.
* tests/ls/dired.sh: Check that formats are implied / overridden.
* NEWS: Mention the change in behavior.
* doc/coreutils.texi (ls invocation): Adjust --dired description.
---
 NEWS               | 5 +++++
 doc/coreutils.texi | 6 +++---
 src/ls.c           | 7 ++++---
 tests/ls/dired.sh  | 8 +++++---
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/NEWS b/NEWS
index 2adea1e11..118995dbc 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   numfmt options like --suffix no longer have an arbitrary 127-byte limit.
   [bug introduced with numfmt in coreutils-8.21]
 
+** Changes in behavior
+
+  ls --dired now implies long format output without hyperlinks enabled,
+  and will take precedence over previously specified formats or hyperlink mode.
+
 
 * Noteworthy changes in release 9.4 (2023-08-29) [stable]
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 8aed79222..f7a2bda7c 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7890,9 +7890,9 @@ should either specify an explicit @option{--quoting-style=literal}
 (@option{-N}) option on the command line, or else be
 prepared to parse the escaped names.
 
-The @option{--dired} (@option{-D}) option has well-defined behavior
-only when long format is in effect and hyperlinks are disabled (e.g.,
-@option{--hyperlink=none}).
+The @option{--dired} (@option{-D}) option implies long format output
+with hyperlinks disabled, and takes precedence over previously specified
+output formats or hyperlink mode.
 
 @item --full-time
 @opindex --full-time
diff --git a/src/ls.c b/src/ls.c
index 5fe6e2725..dfb7718f7 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2054,6 +2054,8 @@ decode_switches (int argc, char **argv)
           break;
 
         case 'D':
+          format_opt = long_format;
+          print_hyperlink = false;
           dired = true;
           break;
 
@@ -2377,9 +2379,8 @@ decode_switches (int argc, char **argv)
   dirname_quoting_options = clone_quoting_options (nullptr);
   set_char_quoting (dirname_quoting_options, ':', 1);
 
-  /* --dired is meaningful only with --format=long (-l) and sans --hyperlink.
-     Otherwise, ignore it.  FIXME: warn about this?
-     Alternatively, make --dired imply --format=long?  */
+  /* --dired implies --format=long (-l) and sans --hyperlink.
+     So ignore it if those overridden.  */
   dired &= (format == long_format) & !print_hyperlink;
 
   if (eolbyte < dired)
diff --git a/tests/ls/dired.sh b/tests/ls/dired.sh
index 417d3b594..c4361d5e2 100755
--- a/tests/ls/dired.sh
+++ b/tests/ls/dired.sh
@@ -19,18 +19,20 @@
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ ls
 
+
 # Check with constant positions
 mkdir dir || framework_failure_
 
-LC_MESSAGES=C ls -lR --dired dir > out || fail=1
 cat <<EOF > exp
   dir:
   total 0
 //SUBDIRED// 2 5
 //DIRED-OPTIONS// --quoting-style=literal
 EOF
-
-compare exp out || fail=1
+for opt in '-l' '' '--hyperlink' '-x'; do
+  LC_MESSAGES=C ls $opt -R --dired dir > out || fail=1
+  compare exp out || fail=1
+done
 
 
 # Check with varying positions (due to usernames etc.)
-- 
2.41.0