bug#55894: 26.3; [PATCH] `find-lisp.el' bugs

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> Date: Fri, 10 Jun 2022 22:06:58 +0000
> 
> `find-lisp.el' is an old file that doesn't seem to have gotten much
> love.  It's apparently been updated just to change things like use of
> (set (make-local-variable...)) to (setq-local...).
> 
> (I wasn't aware of this library till I saw a blog post by Micky Petersen:
> https://www.masteringemacs.org/article/working-multiple-files-dired.)
> 
> The attached patch fixes a few bugs that prevent using it in some
> important ways: (1) opening a Dired buffer that explicitly lists its
> listed files or dirs (i.e. a snapshot Dired buffer, where DIRNAME is a
> cons), (2) using its listing with a `dired-actual-switches' value that
> contains switch `F', (3) properly handling a listed file or dir name
> that starts with a space char.

Thanks.

>  (defun find-lisp-find-dired-subdirectories (dir)
>    "Find all subdirectories of DIR."
> -  (interactive "DFind subdirectories of directory: ")
> +  (interactive "DDired descendent dirs of directory: ")

I find the new prompt much more cryptic than the old one.  Can this be
improved, please?  For example, would

  Find and dired subdirectories of directory:

be okay?

> -  "Run find (Lisp version) and go into Dired mode on a buffer of the output."
> -  (let ((dired-buffers dired-buffers)
> -     (regexp find-lisp-regexp))
> -    ;; Expand DIR ("" means default-directory), and make sure it has a
> -    ;; trailing slash.
> +                                          directory-predicate buffer-name
> +                                          &optional other-window)
> +  "Run Lisp version of `find', then dired the result."

Here, the existing doc string explains better what the function does.
I don't think relying too much on the assumption that "dired
SOMETHING" should be clear to users is a good idea.  One cannot find
such a verb in a dictionary, and we don't use that widely enough in
our documentation.  So explanations of that term would help clarifying
the meaning.

>  (defun find-lisp-find-dired-insert-file (file buffer)
> +  "Insert line for FILE in BUFFER.
> +FILE is a file or a directory name.
> +If `dired-actual-switches' is non-nil in BUFFER then preserve it."

What does it mean in this context to "preserve it"?  This part of the
doc string is not clear.

>  (defun find-lisp-format (file-name file-attr switches now)
> -  "Format one line of long ls output for file FILE-NAME.
> +  "Format one line of long `ls' output for file or directory FILE-NAME.

The comment says this was lifted from ls-lisp.el, but the latter has
since improved its formatting capabilities _a_lot_.  How about making
this version more like what ls-lisp.el does nowadays.  In particular,
this:

> -         (if (stringp file-type)     ; is a symbolic link
> -             (concat " -> " file-type)
> -           "")
> +            (and (eq t file-type)  (memq ?F switches)
> +                 "/")                  ; Add `/' for dir if `F' switch
> +         (and (stringp file-type)
> +                 (concat " -> " file-type)) ; Add " -> " for symbolic link
>           "\n")))

is treated much more thoroughly now in ls-lisp.el, recognizing more
file types than just symlinks and directories.  I think we should do
the same here.