[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when l
From: |
Stefan Monnier |
Subject: |
Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded. |
Date: |
Sat, 09 Dec 2023 15:09:35 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
> Uh. I'm still struggling to understand all bits, but here are already
> some comments.
>
> - The docstring of `ls-lisp--insert-directory' still makes references
> to its previous advice nature, this probably should be ironed out.
I haven't really attacked the task of updating the docs yet (neither
docstrings nor manual).
> - The docstring of `insert-directory-program' does not very explicitly
> mention that its value could be nil. (Nothing introduced by your
> patch, just noticed it.)
>
> - And the custom spec of `insert-directory-program' isn't prepared for
> nil as well, but that is probably OK since it should be used for a
> generated default value only?
While I did change the code so that setting `insert-directory-program`
to nil forces the use of ls-lisp, I did it in a minimalist way so far.
I'm not sure if we want to encourage such a setting. In a sense, it
"comes for free" and it's nice to make the code robust against such
a setting.
> - AFAICT you call `files--insert-directory-program' as a predicate
> only. Probably we should rename it to `...-p' or
> `files--use-insert-directory-program'?
Yeah, it wasn't intended that way but that's indeed how things turned
out.
> - Not sure whether that makes a difference, but: Without your patch the
> advice was conditional on whether ls-lisp was loaded, now the usage of
> `ls-lisp--insert-directory' is conditional on whether
> `ls-lisp-use-insert-directory-program' is bound. What if a user has
> a .emacs with a customization or `setq' of `ls-l-u-i-d-p' to nil, which
> she uses both on Windows and Unix?
Indeed, this is a change in behavior.
Note that those users already encountered this behavior in the past when
their non-Windows system loaded `ls-lisp` at run time, e.g. because of the
use of Tramp or via `help-enable-autoload`, or ...
> - Your fix in `file-expand-wildcards'
>
> (if (equal "" nondir)
> (push (or dir nondir) contents)
>
>
> is hard to digest at 11PM, so a comment won't hurt here, I guess.
Good point, thanks.
> Plus it seems to violate the order promise done in the docstring of
> the function ("... sorted in the `string<' order").
Oh, indeed I completely missed that, thank you.
> - Now the logic gets harder and harder, so I may be wrong here. The
> docstring of `dired-insert-directory' seems to handle FILE-LIST
> exclusive to WILDCARD, as if WILDCARDS do not get expanded if
> FILE-LIST is non-nil:
>
> If FILE-LIST is non-nil, list only those files.
> Otherwise, if WILDCARD is non-nil, expand wildcards;
>
> but your patch changes that, no?
Hmm... IIUC the current code treats a "*" in the directory part as
a wildcard regardless of FILE-LIST and WILDCARD, and my patch tries to
fix that so as to better obey the docstring which says that "*" should
only be treated as wildcards if WILDCARD is non-nil and FILE-LIST
is nil.
Hopefully this will get more clear once I clean up the patch and cut it
into meaningful commits.
Stefan
Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded., Jens Schmidt, 2023/12/06
- Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded., Stefan Monnier, 2023/12/07
- Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded., Jens Schmidt, 2023/12/07
- Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded.,
Stefan Monnier <=
- Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded., Stefan Monnier, 2023/12/09
- Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded., Stefan Monnier, 2023/12/10
- Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded., Jens Schmidt, 2023/12/12
- Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded., Stefan Monnier, 2023/12/21
Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded., Richard Stallman, 2023/12/06