bug#46218: 28.0.50; find-dired errors with non-directory output

bug#46218: 28.0.50; find-dired errors with non-directory output

[Stephen Berman]

[-- Attachment #1: Type: text/plain, Size: 4068 bytes --]

0. emacs -Q
1. In the following find-dired invocation adjust `<emacs/lisp>' to the
emacs lisp source directory and then evaluate the sexp:
(find-dired "<emacs/lisp>" "-name \"*.el\" -exec grep 'const' {} \";\"")
=> error in process filter: Invalid use of ‘\’ in replacement text

This also happens with Emacs 27 but not with Emacs 26.

TL;DR: see the attached patch, justification of which follows.

AFAICT this error (which is signalled repeatedly with the above
invocation) happens when the find-dired output includes a line
containing escaped characters that also matches the regexp for the first
five columns of the standard Dired display of directory entries used by
find-dired-filter.  This function calls replace-match to add spaces to
the output, and if the matches contain escaped characters, replace-match
raises an error.  While escaped characters don't occur in the first five
columns of the standard Dired display, they could well occur in
non-directory output such as that produced by the above find-dired
invocation.  It seems that find-dired-filter does not expect such
output.  (Indeed, by default find-dired does not handle such output
well,
cf. https://stat.ethz.ch/pipermail/ess-help/2021-January/012868.html,
which points out issues with the above find-dired invocation; this
prompted me to submit this bug report.)

This problem was exposed, or at least made much easier to encounter, by
this change:

commit fb20043b2fec8e8aff6354ec1396fd5ba688b76b
Author:     Sebastian Reuße <seb@wirrsal.net>
AuthorDate: Sat Dec 30 12:41:23 2017 +0200
Commit:     Eli Zaretskii <eliz@gnu.org>
CommitDate: Sat Dec 30 12:41:23 2017 +0200

    Fix output alignment in 'find-dired' for "ls -h"
    
    * lisp/find-dired.el (find-dired-filter): Fix alignment of
    the file size column when the -h ls option is used in
    'find-ls-option'.  (Bug#29803)

index 3b0613b280..bf815d500d 100644
--- a/lisp/find-dired.el
+++ b/lisp/find-dired.el
@@ -295,7 +295,7 @@ find-dired-filter
 		    (l-opt (and (consp find-ls-option)
 				(string-match "l" (cdr find-ls-option))))
 		    (ls-regexp (concat "^ +[^ \t\r\n]+\\( +[^ \t\r\n]+\\) +"
-				       "[^ \t\r\n]+ +[^ \t\r\n]+\\( +[0-9]+\\)")))
+				       "[^ \t\r\n]+ +[^ \t\r\n]+\\( +[^[:space:]]+\\)")))
 		(goto-char beg)
 		(insert string)
 		(goto-char beg)

When I revert this change, I don't get the error with the above
find-dired invocation anymore.  But since the change fixed a bug, it
can't just be reverted.  One alternative is to replace `[^[:space:]]' by
something less permissive.  In fact, in the bug thread the author of the
patch first proposed the more restrictive `[0-9.,]+[kMGT]?' to match the
file size column, and indeed, with this the above error does not occur.
But he noted that file size could be displayed in various ways,
e.g. with the `ls' option `--si', and thus chose the most permissive
regexp.  But since that can result in the above error, a better
alternative would be a compromise between too permissive and possibly
too restrictive, e.g.: `[0-9.,]+[[:alpha:]]?'.  With this the above
error also does not occur and there is also no misalignment, as can be
verified by evaluating the following (suitably adjusted) sexp after
making this compromise change in find-dired-filter:

(let ((find-ls-option '("-exec ls -ldh {} +" . "-ldh")))
  (find-dired "<emacs/lisp>" "-type f"))

The Dired fields are aligned just as with the permissive regexp and in
contrast to the output when using `[0-9]'.

But in fact, it appears possible to return to the simple `[0-9]',
i.e. reverting the above commit.  It turns out that there are cases of
misaligned directory lines with find-dired even with the current
permissive regexp added by that commit (but also with the above
compromise, and of course also with the simple `[0-9]').  Evaluating the
following code illustrates this with the output attached below it:

(let ((find-ls-option '("-exec ls -ldh {} +" . "-ldh")))
  (find-dired "/tmp" "-type f"))


[-- Attachment #2: find-dired output --]
[-- Type: text/plain, Size: 416 bytes --]

  /tmp/:
  find . \( -type f \) -exec ls -ldh \{\} +
  find: ‘./tmp.YhFgsj7n1h’: Permission denied
  -r--r--r--   1 root  root       11 Jan 31 09:48 .X0-lock
  -rw-------   1 steve steve      434 Jan 31 09:48 .xfsm-ICE-GF8VX0
  -rw-------   1 steve steve     1.3M Jan 31 10:01 Temp-2ef70396-223f-4fd3-b201-609f73a0034a/mesa_shader_cache/index
  
  find exited abnormally with code 1 at Sun Jan 31 18:08:37

[-- Attachment #3: Type: text/plain, Size: 731 bytes --]


(The above code was actually used by the author of the above commit to
argue that a more permissive regexp was needed, but in his test run the
file owner and group fields were the same for each entry and there was
no misalignment.)

The misalignment here can be repaired by evaluating
`(dired--align-all-files)' in the find-dired output buffer.  But it also
works to add this sexp to find-dired-sentinel.  Doing that thus obviates
using the current permissive regexp in find-dired-filter to avoid the
misalignment reported in Bug#29803, and also avoids (or rather: repairs)
the misalignment just illustrated, but in addition also avoids the
replace-match error of the current bug report.  In short I propose the
following patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: find-dired patch --]
[-- Type: text/x-patch, Size: 834 bytes --]

diff --git a/lisp/find-dired.el b/lisp/find-dired.el
index adc5672eca..7f4d8c0bcd 100644
--- a/lisp/find-dired.el
+++ b/lisp/find-dired.el
@@ -327,7 +327,7 @@ find-dired-filter
 		    (l-opt (and (consp find-ls-option)
 				(string-match "l" (cdr find-ls-option))))
 		    (ls-regexp (concat "^ +[^ \t\r\n]+\\( +[^ \t\r\n]+\\) +"
-				       "[^ \t\r\n]+ +[^ \t\r\n]+\\( +[^[:space:]]+\\)")))
+				       "[^ \t\r\n]+ +[^ \t\r\n]+\\( +[0-9]+\\)")))
 		(goto-char beg)
 		(insert string)
 		(goto-char beg)
@@ -392,6 +392,7 @@ find-dired-sentinel
 	      ;; process is dead, we can delete it now.  Otherwise it
 	      ;; will stay around until M-x `list-processes'.
 	      (delete-process proc)
+	      (dired--align-all-files)
 	      (force-mode-line-update))))
 	  (message "find-dired %s finished." buf))))


[-- Attachment #5: Type: text/plain, Size: 1020 bytes --]


Since the current permissive regexp that leads to the error reported
here first appeared in Emacs 27, i.e. that change introduced a
regression against Emacs 26, the proposed fix (or a better one) should
probably be applied to the emacs-27 branch.


In GNU Emacs 28.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.24.17, cairo version 1.17.3)
 of 2021-01-29 built on strobe-jhalfs
Repository revision: 47147db9b0f40c77657aff625048bbef5d32fb05
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Linux From Scratch SVN-20200401

Configured using:
 'configure --with-xwidgets 'CFLAGS=-Og -g3'
 PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LCMS2 LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SOUND
THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

bug#46218: 28.0.50; find-dired errors with non-directory output

[Stephen Berman]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On Sun, 31 Jan 2021 20:08:18 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:

> 0. emacs -Q
> 1. In the following find-dired invocation adjust `<emacs/lisp>' to the
> emacs lisp source directory and then evaluate the sexp:
> (find-dired "<emacs/lisp>" "-name \"*.el\" -exec grep 'const' {} \";\"")
> => error in process filter: Invalid use of ‘\’ in replacement text
>
> This also happens with Emacs 27 but not with Emacs 26.
[...]

It occurred to me that invoking `(dired--align-all-files)' in
find-dired-sentinel might render the realigning code in
find-dired-filter superfluous.  But it turns out -- if I didn't make a
mistake or overlook something in testing -- that simply removing the
realigning code in find-dired-filter, without adding
`(dired--align-all-files)' to find-dired-sentinel, fixes all the
problems reported in this bug as well as in Bug#29803.  That is, I am
now proposing the following patch instead of the one in my previous
post:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: find-dired-filter patch --]
[-- Type: text/x-patch, Size: 1347 bytes --]

diff --git a/lisp/find-dired.el b/lisp/find-dired.el
index adc5672eca..68f7752110 100644
--- a/lisp/find-dired.el
+++ b/lisp/find-dired.el
@@ -323,11 +323,7 @@ find-dired-filter
 	    (save-restriction
 	      (widen)
 	      (let ((buffer-read-only nil)
-		    (beg (point-max))
-		    (l-opt (and (consp find-ls-option)
-				(string-match "l" (cdr find-ls-option))))
-		    (ls-regexp (concat "^ +[^ \t\r\n]+\\( +[^ \t\r\n]+\\) +"
-				       "[^ \t\r\n]+ +[^ \t\r\n]+\\( +[^[:space:]]+\\)")))
+		    (beg (point-max)))
 		(goto-char beg)
 		(insert string)
 		(goto-char beg)
@@ -342,18 +338,6 @@ find-dired-filter
 		(goto-char (- beg 3))	; no error if < 0
 		(while (search-forward " ./" nil t)
 		  (delete-region (point) (- (point) 2)))
-		;; Pad the number of links and file size.  This is a
-		;; quick and dirty way of getting the columns to line up
-		;; most of the time, but it's not foolproof.
-		(when l-opt
-		  (goto-char beg)
-		  (goto-char (line-beginning-position))
-		  (while (re-search-forward ls-regexp nil t)
-		    (replace-match (format "%4s" (match-string 1))
-				   nil nil nil 1)
-		    (replace-match (format "%9s" (match-string 2))
-				   nil nil nil 2)
-		    (forward-line 1)))
 		;; Find all the complete lines in the unprocessed
 		;; output and process it to add text properties.
 		(goto-char (point-max))

bug#46218: 28.0.50; find-dired errors with non-directory output

[Lars Ingebrigtsen]

Stephen Berman <stephen.berman@gmx.net> writes:

> It occurred to me that invoking `(dired--align-all-files)' in
> find-dired-sentinel might render the realigning code in
> find-dired-filter superfluous.  But it turns out -- if I didn't make a
> mistake or overlook something in testing -- that simply removing the
> realigning code in find-dired-filter, without adding
> `(dired--align-all-files)' to find-dired-sentinel, fixes all the
> problems reported in this bug as well as in Bug#29803.  That is, I am
> now proposing the following patch instead of the one in my previous
> post:

Thanks; pushed to Emacs 29 (since it fixes the backtrace), but I have to
admit I've never understood what find-dired is supposed to be good for,
anyway.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no