Re: master 19a3b499f84: ; * lisp/loadup.el: Don't prohibit advice when ls-lisp is loaded.

[Jens Schmidt]

On 2023-12-07  21:06, Stefan Monnier wrote:

> [...]

Thanks for the explanation.

> My WiP patch is attached.

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.

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

- AFAICT you call `files--insert-directory-program' as a predicate
  only.  Probably we should rename it to `...-p' or
  `files--use-insert-directory-program'?

- And here is a feeble attempt to provide a docstring:

  "Return wether `insert-directory' should use an external program.
Take into consideration `ls-lisp-use-insert-directory-program' if that is
available."

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

- 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.  Plus
  it seems to violate the order promise done in the docstring of the
  function ("... sorted in the `string<' order").

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

                                                                Jens