[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#55894: 26.3; [PATCH] `find-lisp.el' bugs
From: |
Eli Zaretskii |
Subject: |
bug#55894: 26.3; [PATCH] `find-lisp.el' bugs |
Date: |
Sat, 11 Jun 2022 09:01:55 +0300 |
> 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.