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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55894 in the body.
You can then email your comments to 55894 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 26.3; [PATCH] `find-lisp.el' bugs
Date: Fri, 10 Jun 2022 22:06:58 +0000

[Message part 1 (text/plain, inline)]

`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.

[#1 is an important use case.  Because a `find' command (Lisp or not)
can take quite a while, it can be useful to make a snapshot of its
result, i.e., save the list of files and later restore a Dired buffer
for that explicit set of files, without rerunning `find'.  For that use
case, at least, these bugs need to be fixed.]

The patch does the following.  1-2 are improvements, 3-4 are bug fixes.

1. Add other-window versions of commands `find-lisp-find-dired' and
   `find-lisp-find-dired-subdirectories'.

   (Really, both command names should be shortened.  Really, most names
   in this library should be shortened/improved.  And really, the name
   `find-lisp-find-dired' is not good - besides a confusing combination
   of name components, it doesn't tell you that it's only about files,
   not files and dirs - its companion for listing only subdirs has that
   in its name.)

2. `find-lisp-find-dired-internal': Added optional arg OTHER-WINDOW.

3. `find-lisp-find-dired-insert-file':

   * Add a doc string.
   * Put text property `dired-filename' on the inserted file/dir name.
     (This fixes the mistreatment of file/dir names that start with a
     space char.)
   * Start with `dired-actual-switches', instead of just "".  In
     particular, this is to keep switch `F'.

4. `find-lisp-format': If `F' listing switch is present then append `/'
   to listed directories.

Attached is a patch against master today.

Other changes could also be made, both to clean up the code and to
improve the behavior.  But that will wait for someone else.



In GNU Emacs 26.3 (build 1, x86_64-w64-mingw32)
 of 2019-08-29
Repository revision: 96dd0196c28bc36779584e47fffcca433c9309cd
Windowing system distributor `Microsoft Corp.', version 10.0.19044
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

[find-lisp-2022-06-10a.patch (application/octet-stream, attachment)]

Message #8 received at 55894 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 55894 <at> debbugs.gnu.org
Subject: Re: bug#55894: 26.3; [PATCH] `find-lisp.el' bugs
Date: Sat, 11 Jun 2022 09:01:55 +0300

> From: Drew Adams <drew.adams <at> 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.

Message #11 received at 55894 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "55894 <at> debbugs.gnu.org" <55894 <at> debbugs.gnu.org>
Subject: RE: [External] : Re: bug#55894: 26.3; [PATCH] `find-lisp.el' bugs
Date: Sat, 11 Jun 2022 16:46:35 +0000

Summary:

 1. Thanks for considering making some improvements
    to `find-lisp.el', and starting with the fixes
    I suggested.

 2. I'm OK with changes you suggested.  Please do
    whatever you think helps best.

Some detailed feedback below.
___

Yes, Emacs typically uses "subdirectory" to mean a
descendent directory (i.e., any level), and not just
a child directory.  So might as well be consistent
with that.

I don't think that "sub" generally suggests such a
meaning, however.  It's typically ambiguous whether
it means child or descendant - which, in effect,
means that it means descendant.

When appropriate we'd be better off using "parent"
and "child" or "ancestor" and "descendant".

But yes, it's no doubt too late to change, and Emacs
is certainly not going to change this everywhere.

Perhaps we could start to rename places (e.g. doc,
if not fun/var names) where we really mean children
to use "child" and "children" instead of "sub___".
Maybe we consistently do that already; dunno.  
___

The existing prompts could be improved, IMO,
by quoting the command names `find' and `ls',
rather than just using them as words.  And I
think "Run Lisp version of `find'..." is
clearer than "Run find (Lisp version)...".

I agree about not using "dired" as a verb, at
least in the first line of a doc string.  I was
trying to avoid a too-long first line.  I'm OK
with whatever you do here.
___

> > +If `dired-actual-switches' is non-nil in BUFFER then preserve it."
> What does it mean in this context to "preserve it"?  

I don't have a good suggestion for the wording,
and I don't insist that the point be communicated
in the doc string.

The point is that `dired-actual-switches', and not
"", is used as the starting point.

The problem with not making this fix is that the
code inserts subdir names without trailing "/",
and yet if `dired-actual-switches' (e.g. from
`dired-listing-switches') contains switch `F' then
functions such as `dired-move-to-end-of-filename'
don't work, because they're off-by-one for dir
names - e.g.:

(and (dired-check-switches dired-actual-switches
                           "F" "classify") ; used-F 
     (or (memq file-type '(?d ?s ?p)) executable)
     (forward-char -1))

> 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.

Please do whatever's right here.  I didn't write
that comment, and I didn't try to bring `find-lisp'
completely up-to-date.  I'm quite sure there are
lots of ways the library could/should be improved.

Perhaps the `ls-lisp' code takes care of the `F'
bug.  If not, that should be done in `find-lisp'.

Message #14 received at 55894 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 55894 <at> debbugs.gnu.org
Subject: Re: [External] : Re: bug#55894: 26.3; [PATCH] `find-lisp.el' bugs
Date: Sat, 11 Jun 2022 20:00:34 +0300

> From: Drew Adams <drew.adams <at> oracle.com>
> CC: "55894 <at> debbugs.gnu.org" <55894 <at> debbugs.gnu.org>
> Date: Sat, 11 Jun 2022 16:46:35 +0000
> 
> Summary:
> 
>  1. Thanks for considering making some improvements
>     to `find-lisp.el', and starting with the fixes
>     I suggested.
> 
>  2. I'm OK with changes you suggested.  Please do
>     whatever you think helps best.

Does this mean you won't be submitting a modified patch which takes
into account the comments?

Message #17 received at 55894 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "55894 <at> debbugs.gnu.org" <55894 <at> debbugs.gnu.org>
Subject: RE: [External] : Re: bug#55894: 26.3; [PATCH] `find-lisp.el' bugs
Date: Sat, 11 Jun 2022 17:47:17 +0000

> > Summary:
> >
> >  1. Thanks for considering making some improvements
> >     to `find-lisp.el', and starting with the fixes
> >     I suggested.
> >
> >  2. I'm OK with changes you suggested.  Please do
> >     whatever you think helps best.
> 
> Does this mean you won't be submitting a modified patch which takes
> into account the comments?

I guess so, yes.

I pointed out a couple of bugs.  And I suggested
fixes for those bugs, as a patch.  That should at
least make clear what those problems are and serve
as food for thought in case you want to fix them.

I'm OK with your making whatever doc-string changes
you like.

I'm OK with your integrating `ls-lisp' code or
otherwise updating `find-lisp' to take advantage of
`ls-lisp' code.  That would be welcome.

My overall request is to fix the bugs I pointed to.
I made a suggestion as to how they can be fixed.
HTH.

I'm also OK with your doing nothing.  In that case,
I'll have to create a file (e.g. `find-lisp+.el')
with those fixes, for `find-lisp' to work with my
other code (e.g. `dired+.el' commands that make
snapshot Dired buffers from `find' listings (e.g.
from `find-(dired|lisp)').

(I could add such a file anyway, for use with older
Emacs versions, but I'm OK with foregoing use of
`find-lisp' with Emacs versions prior to whatever
Emacs release you might fix this for.)

Message #20 received at 55894 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "55894 <at> debbugs.gnu.org" <55894 <at> debbugs.gnu.org>
Subject: Re: bug#55894: 26.3; [PATCH] `find-lisp.el' bugs
Date: Fri, 17 Jun 2022 02:38:43 -0700

Drew Adams <drew.adams <at> oracle.com> writes:

>> Does this mean you won't be submitting a modified patch which takes
>> into account the comments?
>
> I guess so, yes.
[...]
> I'm also OK with your doing nothing.  In that case,
> I'll have to create a file (e.g. `find-lisp+.el')
> with those fixes, for `find-lisp' to work with my
> other code (e.g. `dired+.el' commands that make
> snapshot Dired buffers from `find' listings (e.g.
> from `find-(dired|lisp)').

Why would you do all that work instead of just submitting a modified
patch?

Message #23 received at 55894 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "55894 <at> debbugs.gnu.org" <55894 <at> debbugs.gnu.org>
Subject: RE: [External] : Re: bug#55894: 26.3; [PATCH] `find-lisp.el' bugs
Date: Fri, 17 Jun 2022 14:57:33 +0000

> >> Does this mean you won't be submitting a modified
> >> patch which takes into account the comments?
> >
> > I guess so, yes.
> [...]
> > I'm also OK with your doing nothing.  In that case,
> > I'll have to create a file (e.g. `find-lisp+.el')
> > with those fixes, for `find-lisp' to work with my
> > other code (e.g. `dired+.el' commands that make
> > snapshot Dired buffers from `find' listings (e.g.
> > from `find-(dired|lisp)').
> 
> Why would you do all that work instead of just
> submitting a modified patch?

I'm not promising to do any such work.

Why would you waste your precious time sending
such an email, instead of just fixing the bug?
Is it that you want to argue?  Why make this
about me, and not the bug?

1. I'm an Emacs user.  I reported a bug.
2. I even suggested fixes for it, showing code.
   That's a suggestion.  The reported problems
   are not a suggestion - they're problems.
3. I said I'm fine with whatever doc-string
   changes someone might want to make.
   Not that it matters whether I'm fine with
   that.  Just politely letting you know: please
   feel free to go ahead and fix the bug however
   you like.

No good deed goes unpunished, apparently.  Forget
the suggested fixes, if you like.  Concentrate on
the bug, please.  Thx.

It's Emacs's bug.

Whether I want to bother to work-around the bug
locally is up to me.  Please don't worry about
whether or why I might or might not want to.
Not your problem.

But I can say that I prefer that Emacs itself
be fixed, for the benefit of all.  If you don't
want to fix it, I can adjust.  It won't be the
first (or last) time.  Not a big deal.

It's a minor bug in a minor library that no one
has paid attention to for quite a while.  One
reason for filing the bug was to draw attention
to the library, which could be made more useful.
Or not.

At ease.

Message #26 received at 55894 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 55894 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#55894: 26.3; [PATCH] `find-lisp.el' bugs
Date: Tue, 06 Sep 2022 12:54:27 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> 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.

I've now pushed Drew's patch, adjusting for Eli's comments (and I did
some other tweaks to the doc strings) to Emacs 29.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.