Subject: Re: bug#58506: Use ".dir-locals.eld" and ".dir-locals-2.eld" when they exist

From: Stefan Kangas <[email-masked]>
Date: Fri, 14 Oct 2022 07:36:12 -0500

Eli Zaretskii <[email-masked]> writes:

>> diff --git a/lisp/files.el b/lisp/files.el
>> index 94d110f0b7..bfd1e5e8e1 100644
>> --- a/lisp/files.el
>> +++ b/lisp/files.el
>> @@ -4403,11 +4403,14 @@ dir-locals--all-files
>>                                          (dosified-file-name dir-locals-file)
>>                                        dir-locals-file)
>>                                      directory))
>> -           (file-2 (when (string-match "\\.el\\'" file-1)
>> +           (is-el (string-match (rx ".el" eos) file-1))
>> +           (file-2 (when is-el
>>                       (replace-match "-2.el" t nil file-1)))
>>            (out nil))
>>        ;; The order here is important.
>> -      (dolist (f (list file-2 file-1))
>> +      ;; Support *.eld files, too.
>> +      (dolist (f (list (and is-el (concat file-2 "d")) file-2
>> +                       (and is-el (concat file-1 "d")) file-1))
>
> This conses 2 strings, which is a pity, because this code is called a
> lot.  Can this be done without so much consing for such a simple job?

Actually, 2 strings are already consed up, so this brings it up to 4.

How about something like this instead?  It brings us down to 0, as long
as users don't mess with the value of `dir-locals-file'.  Maybe it's
even worth installing even if we don't add support for *.eld?

diff --git a/lisp/files.el b/lisp/files.el
index 94d110f0b7..dedf9c4848 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4393,27 +4393,34 @@ dir-locals-file

 See Info node `(elisp)Directory Local Variables' for details.")

+(defvar dir-locals--file-last nil)
+(defvar dir-locals--files nil)
 (defun dir-locals--all-files (directory)
   "Return a list of all readable dir-locals files in DIRECTORY.
 The returned list is sorted by increasing priority.  That is,
 values specified in the last file should take precedence over
 those in the first."
   (when (file-readable-p directory)
-    (let* ((file-1 (expand-file-name (if (eq system-type 'ms-dos)
-                                        (dosified-file-name dir-locals-file)
-                                      dir-locals-file)
-                                    directory))
-           (file-2 (when (string-match "\\.el\\'" file-1)
-                     (replace-match "-2.el" t nil file-1)))
-          (out nil))
-      ;; The order here is important.
-      (dolist (f (list file-2 file-1))
-        (when (and f
-                   (file-readable-p f)
-                   ;; FIXME: Aren't file-regular-p and
-                   ;; file-directory-p mutually exclusive?
-                   (file-regular-p f)
-                   (not (file-directory-p f)))
+    ;; Users might be misguidedly messing with the `dir-locals-file'
+    ;; variable, as ELisp doesn't bother enforcing defconst.
+    (unless (equal dir-locals-file dir-locals--file-last)
+      ;; Caching this avoids consing.
+      (setq dir-locals--files
+            (delq nil
+                  (let* ((file-1 (if (eq system-type 'ms-dos)
+                                     (dosified-file-name dir-locals-file)
+                                   dir-locals-file))
+                         (is-el (string-match "\\.el\\'" file-1))
+                         (file-2 (when is-el
+                                   (replace-match "-2.el" t nil file-1))))
+                    ;; The order here is important.
+                    ;; Support *.eld files, too.
+                    (list file-2 (and is-el (concat file-2 "d"))
+                          file-1 (and is-el (concat file-1 "d")))))))
+    (let ((default-directory directory) out)
+      (dolist (f dir-locals--files)
+        (when (and (file-readable-p f)
+                   (file-regular-p f))
           (push f out)))
       out)))