bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

Tags: patch


When selecting a number of .el files in Dired and byte-compiling them
I'll usually also select a .dir-locals.el file that won't compile.
Would annotating them as non-byte-compilable files be sensible?


In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.34, cairo version 1.17.6) of 2022-10-13 built on rhea
Repository revision: 36505800b5a0c8d7b4af5ceb54483cfdfdbf6925
Repository branch: local
System Description: Fedora Linux 36 (Workstation Edition)

Configured using:
 'configure --with-pgtk --with-imagemagick'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Prevent-.dir-locals.el-from-being-byte-compiled.patch --]
[-- Type: text/patch, Size: 1029 bytes --]

From 8bf2e39c36a66df7e415800caa77c9116648effb Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Wed, 5 Oct 2022 19:45:27 +0200
Subject: [PATCH] Prevent .dir-locals.el from being byte-compiled

* lisp/files-x.el (modify-dir-local-variable): Add a 'no-byte-compile'
file local variable when generating a .dir-locals.el file.
---
 lisp/files-x.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/files-x.el b/lisp/files-x.el
index da1e44e250..f6d5d6cc27 100644
--- a/lisp/files-x.el
+++ b/lisp/files-x.el
@@ -489,7 +489,7 @@ modify-dir-local-variable
                               dir-locals-directory-cache))
 
       ;; Insert modified alist of directory-local variables.
-      (insert ";;; Directory Local Variables\n")
+      (insert ";;; Directory Local Variables            -*- no-byte-compile: t -*-\n")
       (insert ";;; For more information see (info \"(emacs) Directory Variables\")\n\n")
       (princ (dir-locals-to-string
               (sort variables
-- 
2.37.3

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Stefan Kangas]

Philip Kaludercic <philipk@posteo.net> writes:

> When selecting a number of .el files in Dired and byte-compiling them
> I'll usually also select a .dir-locals.el file that won't compile.
> Would annotating them as non-byte-compilable files be sensible?

Makes sense to me, FWIW.  Though Stefan M might point out that these
files should actually be called ".dir-locals.eld".  Which, BTW, we
should probably add support for.  And "*.eld" files should never be
compiled, I think?

(Don't forget to add the cookie to our .dir-locals.el as well.)

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Philip Kaludercic]

Stefan Kangas <stefankangas@gmail.com> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> When selecting a number of .el files in Dired and byte-compiling them
>> I'll usually also select a .dir-locals.el file that won't compile.
>> Would annotating them as non-byte-compilable files be sensible?
>
> Makes sense to me, FWIW.  Though Stefan M might point out that these
> files should actually be called ".dir-locals.eld".  Which, BTW, we
> should probably add support for.  And "*.eld" files should never be
> compiled, I think?

AFAIR this was not done to avoid newer versions of Emacs creating
.dir-locals.eld files that older versions of Emacs won't notice.

> (Don't forget to add the cookie to our .dir-locals.el as well.)

Good point, will do.

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Stefan Kangas]

Philip Kaludercic <philipk@posteo.net> writes:

> AFAIR this was not done to avoid newer versions of Emacs creating
> .dir-locals.eld files that older versions of Emacs won't notice.

If we add support to read them when they exist in Emacs 29, it will be
safe to start creating them in some future version 30+.

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Juri Linkov]

>> When selecting a number of .el files in Dired and byte-compiling them
>> I'll usually also select a .dir-locals.el file that won't compile.
>> Would annotating them as non-byte-compilable files be sensible?
>
> Makes sense to me, FWIW.  Though Stefan M might point out that these
> files should actually be called ".dir-locals.eld".  Which, BTW, we
> should probably add support for.  And "*.eld" files should never be
> compiled, I think?

It's already handled by auto-mode-alist:

  (defvar auto-mode-alist
       ...
       ;; .dir-locals.el is not really Elisp.  Could use the
       ;; `dir-locals-file' constant if it weren't defined below.
       ("\\.dir-locals\\(?:-2\\)?\\.el\\'" . lisp-data-mode)

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Philip Kaludercic]

Juri Linkov <juri@linkov.net> writes:

>>> When selecting a number of .el files in Dired and byte-compiling them
>>> I'll usually also select a .dir-locals.el file that won't compile.
>>> Would annotating them as non-byte-compilable files be sensible?
>>
>> Makes sense to me, FWIW.  Though Stefan M might point out that these
>> files should actually be called ".dir-locals.eld".  Which, BTW, we
>> should probably add support for.  And "*.eld" files should never be
>> compiled, I think?
>
> It's already handled by auto-mode-alist:
>
>   (defvar auto-mode-alist
>        ...
>        ;; .dir-locals.el is not really Elisp.  Could use the
>        ;; `dir-locals-file' constant if it weren't defined below.
>        ("\\.dir-locals\\(?:-2\\)?\\.el\\'" . lisp-data-mode)

Yes, but this is not related to the situation I mentioned with Dired.

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

[Stefan Kangas]

Severity: wishlist

[Spun out from Bug#58486.]

Juri Linkov <juri@linkov.net> writes:

> It's already handled by auto-mode-alist:
>
>   (defvar auto-mode-alist
>        ...
>        ;; .dir-locals.el is not really Elisp.  Could use the
>        ;; `dir-locals-file' constant if it weren't defined below.
>        ("\\.dir-locals\\(?:-2\\)?\\.el\\'" . lisp-data-mode)

It's nice if we can use the "*.eld" extension for files that are not
supposed to be executed.  But it's hard to just change that outright, as
a file ".dir-locals.eld" won't be used on old versions of Emacs.

So how about doing something like the below, as a future compatibility
patch?  Then, in some undefined future version, we can consider creating
".dir-locals.eld" files by default, instead.

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))
         (when (and f
                    (file-readable-p f)
                    ;; FIXME: Aren't file-regular-p and

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

[Eli Zaretskii]

> Cc: Juri Linkov <juri@linkov.net>, Philip Kaludercic <philipk@posteo.net>,
>  Stefan Monnier <monnier@iro.umontreal.ca>
> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Fri, 14 Oct 2022 11:22:23 +0200
> 
> Severity: wishlist
> 
> [Spun out from Bug#58486.]
> 
> Juri Linkov <juri@linkov.net> writes:
> 
> > It's already handled by auto-mode-alist:
> >
> >   (defvar auto-mode-alist
> >        ...
> >        ;; .dir-locals.el is not really Elisp.  Could use the
> >        ;; `dir-locals-file' constant if it weren't defined below.
> >        ("\\.dir-locals\\(?:-2\\)?\\.el\\'" . lisp-data-mode)
> 
> It's nice if we can use the "*.eld" extension for files that are not
> supposed to be executed.  But it's hard to just change that outright, as
> a file ".dir-locals.eld" won't be used on old versions of Emacs.
> 
> So how about doing something like the below, as a future compatibility
> patch?  Then, in some undefined future version, we can consider creating
> ".dir-locals.eld" files by default, instead.
> 
> 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?

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

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> 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)))

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

[Lars Ingebrigtsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> So how about doing something like the below, as a future compatibility
> patch?  Then, in some undefined future version, we can consider creating
> ".dir-locals.eld" files by default, instead.

It sort of makes sense to me, but if we're doing a name change, I'd
rather make it a functional change, too.

It's been pointed out many times that the .dir-locals.el file format is
bad -- it's not extensible, and many things have been proposed for extra
functionality that's impossible to add today.

(For instance -- variables set before the major mode happens, and adding
to list variables.)

So if we introduce .dir-locals.eld, we should take the opportunity to
add a brand new, sensible, extensible syntax, too.

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

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So if we introduce .dir-locals.eld, we should take the opportunity to
> add a brand new, sensible, extensible syntax, too.

That makes sense.

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

[Robert Pluim]

>>>>> On Fri, 14 Oct 2022 13:25:22 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> So if we introduce .dir-locals.eld, we should take the opportunity to
    Lars> add a brand new, sensible, extensible syntax, too.

We could use an almost syntax-less language with lots of structuring
parentheses (LOSP for short). And weʼd have to call the file
'please-pwn-my-Emacs.el'

Seriously, having files popping up safe-variable warnings is scary
enough already, do we really need to complicate things by adding
yet-another not-quite-lisp syntax? Is the support for `eval' in
.dir-locals.el not enough?

Robert
-- 

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

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> Seriously, having files popping up safe-variable warnings is scary
> enough already, do we really need to complicate things by adding
> yet-another not-quite-lisp syntax? Is the support for `eval' in
> .dir-locals.el not enough?

It's not -- there's a bunch of bug reports around these issues
*handwaves at debbugs*.

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

[Robert Pluim]

>>>>> On Fri, 14 Oct 2022 15:10:04 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Robert Pluim <rpluim@gmail.com> writes:
    >> Seriously, having files popping up safe-variable warnings is scary
    >> enough already, do we really need to complicate things by adding
    >> yet-another not-quite-lisp syntax? Is the support for `eval' in
    >> .dir-locals.el not enough?

    Lars> It's not -- there's a bunch of bug reports around these issues
    Lars> *handwaves at debbugs*.

My debbugs-gnu-foo must be too weak: I can only find two, neither of
which look like theyʼd be fixed by changing the syntax accepted by
.dir-locals.el

Robert
-- 

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

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

>     Lars> It's not -- there's a bunch of bug reports around these issues
>     Lars> *handwaves at debbugs*.
>
> My debbugs-gnu-foo must be too weak: I can only find two,

Isn't two a bunch?  😀

> neither of which look like theyʼd be fixed by changing the syntax
> accepted by .dir-locals.el

I suspect that by "syntax" you're thinking about syntax syntax -- I'm
talking semantics syntax.

The .dir-locals.eld file will be Lisp forms, of course.

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

[Robert Pluim]

>>>>> On Fri, 14 Oct 2022 15:29:36 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Robert Pluim <rpluim@gmail.com> writes:
    Lars> It's not -- there's a bunch of bug reports around these issues
    Lars> *handwaves at debbugs*.
    >> 
    >> My debbugs-gnu-foo must be too weak: I can only find two,

    Lars> Isn't two a bunch?  😀

Maybe, but if one of them is "Lars canʼt reproduce, waiting for
feedback", is it really open?

    >> neither of which look like theyʼd be fixed by changing the syntax
    >> accepted by .dir-locals.el

    Lars> I suspect that by "syntax" you're thinking about syntax syntax -- I'm
    Lars> talking semantics syntax.

    Lars> The .dir-locals.eld file will be Lisp forms, of course.

Then I still fail to see how itʼs different from `eval', unless youʼre
planning on adding  "run these forms before/after setting the mode"
semantics.

Robert
-- 

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

[Philip Kaludercic]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Fri, 14 Oct 2022 13:25:22 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:
>
>     Lars> So if we introduce .dir-locals.eld, we should take the opportunity to
>     Lars> add a brand new, sensible, extensible syntax, too.
>
> We could use an almost syntax-less language with lots of structuring
> parentheses (LOSP for short). And weʼd have to call the file
> 'please-pwn-my-Emacs.el'
>
> Seriously, having files popping up safe-variable warnings is scary
> enough already, do we really need to complicate things by adding
> yet-another not-quite-lisp syntax? Is the support for `eval' in
> .dir-locals.el not enough?

What I would like to see if some kind of extensibility in the syntax.
Perhaps using methods.  I have often seen projects using `eval' just
because the options they are setting are not trivial assignments, but
conventional modifications like prepending an item to a list or setting
a symbol property (check out the .dir-locals.el for Guix if you want to
see a massive "Do you want to accept all these variables" prompt).

> Robert

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

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> What I would like to see if some kind of extensibility in the syntax.
> Perhaps using methods.  I have often seen projects using `eval' just
> because the options they are setting are not trivial assignments, but
> conventional modifications like prepending an item to a list or setting
> a symbol property (check out the .dir-locals.el for Guix if you want to
> see a massive "Do you want to accept all these variables" prompt).

Yes, exactly -- people have to resort to using `eval' (which is
something people should use as little as possible) because our syntax
doesn't allow for simple things like adding values to a list.

Post-mode variables and list concatenation are two things we'd like to
have, but I'm sure there's a whole bunch of stuff people will come up
with if the syntax allowed for (safe) extensions.

I don't know what the new syntax would look like -- the current syntax
is, er, very implementation-friendly and user-hostile.  I.e., it's easy
for Emacs to parse, and difficult for people to write:

((nil . ((tab-width . 8)
         (sentence-end-double-space . t)
         (fill-column . 70)
	 (emacs-lisp-docstring-fill-column . 65)
         (vc-git-annotate-switches . "-w")
         (bug-reference-url-format . "https://debbugs.gnu.org/%s")
	 (diff-add-log-use-relative-names . t)))
 (c-mode . ((c-file-style . "GNU")
            (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK"))
            (electric-quote-comment . nil)
            (electric-quote-string . nil)
            (indent-tabs-mode . t)
	    (mode . bug-reference-prog))))

Perhaps a more imperative style would be nice.  Err...  something
like...

(in-mode c-mode
  (set c-file-style "GNU")
  (set-early treesit-thing t)
  (add-to-list odd-list 3)
  (minor-mode indent-tabs-mode)
  (minor-mode blink-parentheses-mode))

`safep' would have to be a bit adjusted -- a `safep' for `odd-list'
would be (cl-every #'oddp) etc.

And we'd make the parser backwards/forwards compatible -- i.e., elements
that are unknown to the Emacs version running would just be ignored.

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

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> What I would like to see if some kind of extensibility in the syntax.
>> Perhaps using methods.  I have often seen projects using `eval' just
>> because the options they are setting are not trivial assignments, but
>> conventional modifications like prepending an item to a list or setting
>> a symbol property (check out the .dir-locals.el for Guix if you want to
>> see a massive "Do you want to accept all these variables" prompt).
>
> Yes, exactly -- people have to resort to using `eval' (which is
> something people should use as little as possible) because our syntax
> doesn't allow for simple things like adding values to a list.
>
> Post-mode variables and list concatenation are two things we'd like to
> have, but I'm sure there's a whole bunch of stuff people will come up
> with if the syntax allowed for (safe) extensions.
>
> I don't know what the new syntax would look like -- the current syntax
> is, er, very implementation-friendly and user-hostile.  I.e., it's easy
> for Emacs to parse, and difficult for people to write:
>
> ((nil . ((tab-width . 8)
>          (sentence-end-double-space . t)
>          (fill-column . 70)
> 	 (emacs-lisp-docstring-fill-column . 65)
>          (vc-git-annotate-switches . "-w")
>          (bug-reference-url-format . "https://debbugs.gnu.org/%s")
> 	 (diff-add-log-use-relative-names . t)))
>  (c-mode . ((c-file-style . "GNU")
>             (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK"))
>             (electric-quote-comment . nil)
>             (electric-quote-string . nil)
>             (indent-tabs-mode . t)
> 	    (mode . bug-reference-prog))))

Since we are admitting that this is data (-> .eld), not code is it even
necessary to worry about how difficult it is two write.  Couldn't adding
a comment referencing `add-dir-local-variable', or if necessary some
other, new function be enough?

> Perhaps a more imperative style would be nice.  Err...  something
> like...
>
> (in-mode c-mode
>   (set c-file-style "GNU")
>   (set-early treesit-thing t)
>   (add-to-list odd-list 3)
>   (minor-mode indent-tabs-mode)
>   (minor-mode blink-parentheses-mode))

My only objection to "something like" this is that it appears to be Lisp
code, and people could get surprised when they try to add something that
isn't (eg. a regular `add-to-list' call that had a quote -- beginners
often get tripped up on these minor syntactic points).

> `safep' would have to be a bit adjusted -- a `safep' for `odd-list'
> would be (cl-every #'oddp) etc.
>
> And we'd make the parser backwards/forwards compatible -- i.e., elements
> that are unknown to the Emacs version running would just be ignored.

Ideally there would be a warning or some way you could find out that
variables aren't loaded, e.g. if you have a typo in
`blink-parenthesis-mode' vs. `blink-parentheses-mode'.

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

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> My only objection to "something like" this is that it appears to be Lisp
> code, and people could get surprised when they try to add something that
> isn't (eg. a regular `add-to-list' call that had a quote -- beginners
> often get tripped up on these minor syntactic points).

Then I guess `add-to-list' wouldn't be a good "directive" here.

>> And we'd make the parser backwards/forwards compatible -- i.e., elements
>> that are unknown to the Emacs version running would just be ignored.
>
> Ideally there would be a warning or some way you could find out that
> variables aren't loaded, e.g. if you have a typo in
> `blink-parenthesis-mode' vs. `blink-parentheses-mode'.

Well, that would make things less backwards/forwards compatible --
people would be getting warnings when flipping between different Emacs
versions.

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

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> My only objection to "something like" this is that it appears to be Lisp
>> code, and people could get surprised when they try to add something that
>> isn't (eg. a regular `add-to-list' call that had a quote -- beginners
>> often get tripped up on these minor syntactic points).
>
> Then I guess `add-to-list' wouldn't be a good "directive" here.

But, yes, that's a good point about making these directives look too
much like Lisp code, so perhaps a different kind of syntax would be
nice...  but what?

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

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> My only objection to "something like" this is that it appears to be Lisp
>> code, and people could get surprised when they try to add something that
>> isn't (eg. a regular `add-to-list' call that had a quote -- beginners
>> often get tripped up on these minor syntactic points).
>
> Then I guess `add-to-list' wouldn't be a good "directive" here.

Another idea could be to take inspiration from Guile's "Sandboxed
Evaluation"[0] and provide a "safe subset" of Elisp that can be
evaluated (with some additional checks).

E.g. the following would allow evaluating `add-to-list' if the list if
safe and the value is self-evaluating:

--8<---------------cut here---------------start------------->8---
(cl-defmethod safe-eval ((_fn (eql 'add-to-list)) &rest args)
  (when-let* ((list-var (nth 0 args))
	      (element (nth 1 args))
	      ((macroexp-const-p element))
	      (append (nth 2 args))
	      (new-list (if append
			    (append (symbol-value list-var) (list element))
			  (cons element (symbol-value list-var))))
	      ((safe-local-variable-p list-var new-list)))
    (add-to-list list-var element append)))
--8<---------------cut here---------------end--------------->8---

[0] https://www.gnu.org/software/guile/docs/docs-2.2/guile-ref/Sandboxed-Evaluation.html

>>> And we'd make the parser backwards/forwards compatible -- i.e., elements
>>> that are unknown to the Emacs version running would just be ignored.
>>
>> Ideally there would be a warning or some way you could find out that
>> variables aren't loaded, e.g. if you have a typo in
>> `blink-parenthesis-mode' vs. `blink-parentheses-mode'.
>
> Well, that would make things less backwards/forwards compatible --
> people would be getting warnings when flipping between different Emacs
> versions.

No, what I had in mind was not to trigger warnings but either to
highlight unused variables or provide a command that would check it for
you.

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

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> Another idea could be to take inspiration from Guile's "Sandboxed
> Evaluation"[0] and provide a "safe subset" of Elisp that can be
> evaluated (with some additional checks).
>
> E.g. the following would allow evaluating `add-to-list' if the list if
> safe and the value is self-evaluating:

Oh, that's a good idea.  I wonder whether anybody's written an
interpreter for a "safe" version of Emacs Lisp -- then people could put
`if' statements etc also into these files.

We already mark functions as being side-effect-free, so it seems like
code like

(if (cl-oddp (% (random) 2))
    (setq ...))

would be "safe" together with the safep markup for assignments we
already have.  We could make a safe restricted language subset for use
both here and in similar circumstances.

> No, what I had in mind was not to trigger warnings but either to
> highlight unused variables or provide a command that would check it for
> you.

Oh, right.  That's another good idea.  😀

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

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Another idea could be to take inspiration from Guile's "Sandboxed
>> Evaluation"[0] and provide a "safe subset" of Elisp that can be
>> evaluated (with some additional checks).
>>
>> E.g. the following would allow evaluating `add-to-list' if the list if
>> safe and the value is self-evaluating:
>
> Oh, that's a good idea.  I wonder whether anybody's written an
> interpreter for a "safe" version of Emacs Lisp -- then people could put
> `if' statements etc also into these files.

There is unsafep, but that is too strict for what we want.  E.g.

  (unsafep '(setq tab-width 3)) ;; => (global-variable tab-width)

even though we would want this to work.  I've attached an incomplete
sketch of how this could look like


[-- Attachment #2: safe-eval.el --]
[-- Type: text/plain, Size: 3796 bytes --]

;;; safe-eval.el --- Evaluation of Safe Emacs Lisp   -*- lexical-binding: t; -*-

;; Copyright (C) 2022  Philip Kaludercic

;; Author: Philip Kaludercic <philipk@posteo.net>
;; Created: 15Oct22
;; Keywords: lisp

;; This program is free software; you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; This program is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with this program.  If not, see <https://www.gnu.org/licenses/>.

;;; Commentary:

;; An evaluator for safe Emacs Lisp code.

;;; Code:

(eval-when-compile (require 'pcase))
(require 'cl-lib)
(require 'seq)
(require 'macroexp)

(cl-defgeneric safe-eval-p (form)
  "Return non-nil if it is safe to evaluate FORM.
A FORM is safe if it is not a function call, has no side-effects
or a method has been defined to verify its safety."
  (:method :around (form)
	   "Macroexpand FORM before testing for safety."
	   (cl-call-next-method (macroexpand-all form)))
  ;; Some basic logic
  (:method
   ((form (head if)))
   "A `if' FORM is safe if all arguments are safe."
   (pcase-let ((`(if ,(pred safe-eval-p) ,(pred safe-eval-p) . ,else) form))
     (seq-every-p #'safe-eval-p else)))
  (:method
   ((form (head when)))
   "A `when' FORM is safe if all arguments are safe."
   (pcase-let ((`(when ,(pred safe-eval-p) . ,body) form))
     (seq-every-p #'safe-eval-p body)))
  (:method
   ((form (head unless)))
   "A `unless' FORM is safe if all arguments are safe."
   (pcase-let ((`(unless ,(pred safe-eval-p) . ,body) form))
     (seq-every-p #'safe-eval-p body)))
  ;; Common state modifiers
  (:method
   ((form (head setq)))
   "A `setq' FORM is safe the new value is a safe value."
   (pcase-let ((`(setq ,var ,val) form))
     (and (safe-eval-p val)
	  (safe-local-variable-p var val))))
  (:method
   ((form (head add-hook)))
   "A form with `add-hook' must modify a hook locally."
   (pcase-let* ((`(add-hook ',hook
			    ,(or `#',func `',func)
			    ,(pred macroexp-const-p)
			    ,(and (pred macroexp-const-p)
				  (pred identity)))
		 form)
		(new-hook (cons func (symbol-value hook))))
     (and (safe-local-variable-p hook new-hook)
	  (macroexp-const-p func))))
  (:method
   ((form (head add-to-list)))
   "A `add-to-hook' FORM is safe the new list is has a safe value."
   (pcase-let* ((`(add-to-list ',list-var ,element ,append)
		 form)
		(old-list (symbol-value list-var))
		(new-list (if append
			      (append old-list (list element))
			    (cons element old-list))))
     ;; FIXME: `new-list' contains `element' before evaluation.
     (and (safe-local-variable-p list-var new-list)
	  (safe-eval-p element)
	  (macroexp-const-p append))))
  ;; ;; Fallback
  (:method
   ((form t))
   "A fallback handler to check if FORM is side-effect free."
   (or (not (consp form))
       (and (get (car form) 'side-effect-free)
	    (seq-every-p #'safe-eval-p (cdr form))))))

;;;###autoload
(defun safe-eval (form)
  "Evaluate FORM is it is safe per `safe-eval-p'.
If it is not safe, it will be silently ignored."
  (when (safe-eval-p form)
    (eval form t)))

;;;###autoload
(defun safe-eval-file (filename)
  "Evaluate the safe contents of FILENAME.
All files deemed unsafe by `safe-eval-p' are silently ignored.'"
  (with-temp-buffer
    (insert-file-contents filename)
    (while (not (eobp))
      (safe-eval (read (current-buffer))))))

(provide 'safe-eval)
;;; safe-eval.el ends here

[-- Attachment #3: Type: text/plain, Size: 1059 bytes --]


> We already mark functions as being side-effect-free, so it seems like
> code like
>
> (if (cl-oddp (% (random) 2))
>     (setq ...))
>
> would be "safe" together with the safep markup for assignments we
> already have.  We could make a safe restricted language subset for use
> both here and in similar circumstances.

That is a good point, but I think more tagging should be done.  Ideally
this would read as regular elisp (which is kind of ironic considering
that we are discussing an .eld file), so it would be nice if
mode-specific modifications could be done by just writing

--8<---------------cut here---------------start------------->8---
(when (derived-mode-p 'c-mode)
  (setq tab-width 8))
--8<---------------cut here---------------end--------------->8---

or something like that.

>> No, what I had in mind was not to trigger warnings but either to
>> highlight unused variables or provide a command that would check it for
>> you.
>
> Oh, right.  That's another good idea.  😀

One idea would be to use Flymake.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> There is unsafep, but that is too strict for what we want.  E.g.
>
>   (unsafep '(setq tab-width 3)) ;; => (global-variable tab-width)

We have our own safety predicates for setting variables, so the
`unsafep` test should only be applied to the computation of the value to
which we want to set the variable.  After checking it's safe, we can run
that code to get the value, and then we can call the variable's own
safety predicate to check that the computed value is safe.


        Sefan

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

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> There is unsafep, but that is too strict for what we want.  E.g.
>
>   (unsafep '(setq tab-width 3)) ;; => (global-variable tab-width)
>
> even though we would want this to work.  I've attached an incomplete
> sketch of how this could look like

Cool; looks very promising.

>> We already mark functions as being side-effect-free, so it seems like
>> code like
>>
>> (if (cl-oddp (% (random) 2))
>>     (setq ...))
>>
>> would be "safe" together with the safep markup for assignments we
>> already have.  We could make a safe restricted language subset for use
>> both here and in similar circumstances.
>
> That is a good point, but I think more tagging should be done.  Ideally
> this would read as regular elisp (which is kind of ironic considering
> that we are discussing an .eld file),

Yes, so perhaps we should come up with a new extension for this "new
language", i.e., "safe Lisp".  Err...  ".dir-locals.els"?

> so it would be nice if
> mode-specific modifications could be done by just writing
>
> (when (derived-mode-p 'c-mode)
>   (setq tab-width 8))
>
> or something like that.

Yes, our side-effect-free tagging isn't very complete at present -- 
probably because it's not used that much (in a visible way).  I mean,
the byte compiler uses the data to warn, for instance.  But this would
give people an impetus to do further tagging.  It looks like
`derived-mode-p' is side-effect-free, for instance.

>>> No, what I had in mind was not to trigger warnings but either to
>>> highlight unused variables or provide a command that would check it for
>>> you.
>>
>> Oh, right.  That's another good idea.  😀
>
> One idea would be to use Flymake.

That's possible, but I think it should be possible to just use font
locking, too.

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

[Stefan Kangas]

Philip Kaludercic <philipk@posteo.net> writes:

> My only objection to "something like" this is that it appears to be Lisp
> code, and people could get surprised when they try to add something that
> isn't (eg. a regular `add-to-list' call that had a quote -- beginners
> often get tripped up on these minor syntactic points).

We could add a comment at the top of the file to explain this.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I don't know what the new syntax would look like -- the current syntax
> is, er, very implementation-friendly and user-hostile.  I.e., it's easy
> for Emacs to parse, and difficult for people to write:

As Philip points out, "hard to write" is circumvented by
`add-dir-local-variable`.
The more serious problem IMO is that it's hard for humans to read.

[ Also, while it's not ELisp code, I think it's code more than it's
  data.  And indeed this discussion is about making it more like
  code.  ]

> ((nil . ((tab-width . 8)
>          (sentence-end-double-space . t)
>          (fill-column . 70)
> 	 (emacs-lisp-docstring-fill-column . 65)
>          (vc-git-annotate-switches . "-w")
>          (bug-reference-url-format . "https://debbugs.gnu.org/%s")
> 	 (diff-add-log-use-relative-names . t)))
>  (c-mode . ((c-file-style . "GNU")
>             (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK"))
>             (electric-quote-comment . nil)
>             (electric-quote-string . nil)
>             (indent-tabs-mode . t)
> 	    (mode . bug-reference-prog))))

I think the above is better written:

    ((nil
      (tab-width . 8)
      (sentence-end-double-space . t)
      (fill-column . 70)
      (emacs-lisp-docstring-fill-column . 65)
      (vc-git-annotate-switches . "-w")
      (bug-reference-url-format . "https://debbugs.gnu.org/%s")
      (diff-add-log-use-relative-names . t))
     (c-mode
      (c-file-style . "GNU")
      (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK"))
      (electric-quote-comment . nil)
      (electric-quote-string . nil)
      (indent-tabs-mode . t)
      (mode . bug-reference-prog)))

> Perhaps a more imperative style would be nice.  Err...  something
> like...
>
> (in-mode c-mode
>   (set c-file-style "GNU")
>   (set-early treesit-thing t)
>   (add-to-list odd-list 3)
>   (minor-mode indent-tabs-mode)
>   (minor-mode blink-parentheses-mode))

I fully agree with Philip here that it looks too much like ELisp.
We should either make it use a proper subset of ELisp, or make it use
a syntax that's sufficiently different.

Maybe something like:

    (c-mode
     (:set c-file-style "GNU")
     (:set treesit-thing t)
     (:set odd-list (cons 3 odd-list))
     (:minor-mode indent-tabs-mode -1) ;; Disable
     (:minor-mode blink-parentheses-mode))

[ I dropped the "set-early" because I still haven't heard any good
  reason why we'd need that nor what that would really mean (e.g. how it
  could be implemented).  ]

> `safep' would have to be a bit adjusted -- a `safep' for `odd-list'
> would be (cl-every #'oddp) etc.

Sorry, I don't know what problem you're alluding to.
Why would `safep` need to be adjusted?


        Stefan

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

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> As Philip points out, "hard to write" is circumvented by
> `add-dir-local-variable`.

My ever-reliable statistics team is informing me that 99.74% of
.dir-locals files are written by hand.

> We should either make it use a proper subset of ELisp, or make it use
> a syntax that's sufficiently different.
>
> Maybe something like:
>
>     (c-mode
>      (:set c-file-style "GNU")
>      (:set treesit-thing t)
>      (:set odd-list (cons 3 odd-list))
>      (:minor-mode indent-tabs-mode -1) ;; Disable
>      (:minor-mode blink-parentheses-mode))

Yes, that's much better than my initial suggestion.  But I like the
safe-lisp approach better.

> [ I dropped the "set-early" because I still haven't heard any good
>   reason why we'd need that nor what that would really mean (e.g. how it
>   could be implemented).  ]

Some major modes react to variables to change how they work.  So
you'd say

(setq org-thingamabob-syntax-version 2)
(org-thingamabob-mode)

And this has to be set before the mode is called, because the mode is
very expensive and re-interpreting the file afterwards is ungood.

>> `safep' would have to be a bit adjusted -- a `safep' for `odd-list'
>> would be (cl-every #'oddp) etc.
>
> Sorry, I don't know what problem you're alluding to.
> Why would `safep` need to be adjusted?

It's not necessary, but it'd be nice to be able to say "this element is
safe to add to the list" instead of saying "after adding this element to
the list, the resulting list is safe".

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

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> As Philip points out, "hard to write" is circumvented by
>> `add-dir-local-variable`.
>
> My ever-reliable statistics team is informing me that 99.74% of
> .dir-locals files are written by hand.

Really?  I guess by virtue of participating in this very discussion I'm
not the average Emacs user, but I'd be surprised if
`add-dir-local-variable' is that unknown.

>> We should either make it use a proper subset of ELisp, or make it use
>> a syntax that's sufficiently different.
>>
>> Maybe something like:
>>
>>     (c-mode
>>      (:set c-file-style "GNU")
>>      (:set treesit-thing t)
>>      (:set odd-list (cons 3 odd-list))
>>      (:minor-mode indent-tabs-mode -1) ;; Disable
>>      (:minor-mode blink-parentheses-mode))

And I assume that if you don't wrap the block in a (foo-mode ...)
construct, that the settings will apply to all modes, right?

> Yes, that's much better than my initial suggestion.  But I like the
> safe-lisp approach better.

... why not both?  I can imagine that safe-lisp will take a while before
it becomes usable, just because something like `safe-eval-p' has to be
quite exhaustive, and at the very least handle all special forms.  Elisp
isn't a Scheme so that means that a number of methods have to be
implemented...

Meanwhile .dir-locals.eld with Stefan's syntax seems like a good
improvement over the current syntax -- especially if you are right about
how many people write these files from hand.

>> [ I dropped the "set-early" because I still haven't heard any good
>>   reason why we'd need that nor what that would really mean (e.g. how it
>>   could be implemented).  ]
>
> Some major modes react to variables to change how they work.  So
> you'd say
>
> (setq org-thingamabob-syntax-version 2)
> (org-thingamabob-mode)
>
> And this has to be set before the mode is called, because the mode is
> very expensive and re-interpreting the file afterwards is ungood.
>
>>> `safep' would have to be a bit adjusted -- a `safep' for `odd-list'
>>> would be (cl-every #'oddp) etc.
>>
>> Sorry, I don't know what problem you're alluding to.
>> Why would `safep` need to be adjusted?
>
> It's not necessary, but it'd be nice to be able to say "this element is
> safe to add to the list" instead of saying "after adding this element to
> the list, the resulting list is safe".

I have seen a lot of packages that either forget or don't bother to
specify safe values even if they are applicable.  Making it easier to do
so could help motivate maintainers to specify these.

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

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

>> Yes, that's much better than my initial suggestion.  But I like the
>> safe-lisp approach better.
>
> ... why not both?  I can imagine that safe-lisp will take a while before
> it becomes usable, just because something like `safe-eval-p' has to be
> quite exhaustive, and at the very least handle all special forms.  Elisp
> isn't a Scheme so that means that a number of methods have to be
> implemented...

We're not in a hurry here.  😀  If we're aiming for a `safe-eval-p'
solution, then adding another syntax that will then be deprecated again
seems counter-productive.

But Stefan's syntax does look attractive, and allows both manual editing
and automatic editing, which a safe-lisp approach doesn't.  And the
safe-list approach also makes backwards compatibility more iffy -- if we
introduce `cond' in safe-lisp in Emacs 33, we'd still want the file to
not bug out in Emacs 32.

So safe-lisp would just have to ignore forms that are unknown to it,
which is less attractive than just ignoring

  (:some-new-thing ...)

in Stefan's syntax.

Uhm uhm hm.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> [ I dropped the "set-early" because I still haven't heard any good
>>   reason why we'd need that nor what that would really mean (e.g. how it
>>   could be implemented).  ]
>
> Some major modes react to variables to change how they work.  So
> you'd say
>
> (setq org-thingamabob-syntax-version 2)
> (org-thingamabob-mode)

We've been solving this problem for the last 30 years without
introducing a new kind of "set before mode", so I don't understand
your example.

If `org-thingamabob-mode` is a major mode, then it starts with
`kill-all-local-variables`, so either you mark
`org-thingamabob-syntax-version` as persistent-local or you're screwed
from the outset (you can try some convoluted hack using
`change-major-mode-hook` but it's gonna be ugly).

OTOH using `:after-hook` or `hack-local-variables-hook` works right now
without very much effort.

> And this has to be set before the mode is called, because the mode is
> very expensive and re-interpreting the file afterwards is ungood.

So: delay the expensive part.  It's usually pretty easy, and most of the
time it's a good idea for all kinds of other reasons.

>>> `safep' would have to be a bit adjusted -- a `safep' for `odd-list'
>>> would be (cl-every #'oddp) etc.
>> Sorry, I don't know what problem you're alluding to.
>> Why would `safep` need to be adjusted?
> It's not necessary, but it'd be nice to be able to say "this element is
> safe to add to the list" instead of saying "after adding this element to
> the list, the resulting list is safe".

Ah, I see.  My intuition is YAGNI but I haven't thought too much
about it.


        Stefan

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

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> We've been solving this problem for the last 30 years without
> introducing a new kind of "set before mode", so I don't understand
> your example.

See bug#57003.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-10-16 15:44:44] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> We've been solving this problem for the last 30 years without
>> introducing a new kind of "set before mode", so I don't understand
>> your example.
> See bug#57003.

I don't see anything new there.  We've been solving this problem
for years.  Ihor just wasn't aware of it.


        Stefan

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

[Juri Linkov]

> I think the above is better written:
>
>     ((nil
>       (tab-width . 8)
>       (sentence-end-double-space . t)
>       (fill-column . 70)
>       (emacs-lisp-docstring-fill-column . 65)
>       (vc-git-annotate-switches . "-w")
>       (bug-reference-url-format . "https://debbugs.gnu.org/%s")
>       (diff-add-log-use-relative-names . t))
>      (c-mode
>       (c-file-style . "GNU")
>       (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK"))
>       (electric-quote-comment . nil)
>       (electric-quote-string . nil)
>       (indent-tabs-mode . t)
>       (mode . bug-reference-prog)))

So the dotted pair notation should be avoided only at the first level.

Then what about making it closer to the syntax of display-buffer-alist rules
where CONDITION is passed to buffer-match-p that can match not only major-mode,
but also a regular expression of a buffer name.  And display-buffer-alist's
ACTION alist already looks like dir-locals's alist.

> Maybe something like:
>
>     (c-mode
>      (:set c-file-style "GNU")
>      (:set treesit-thing t)
>      (:set odd-list (cons 3 odd-list))
>      (:minor-mode indent-tabs-mode -1) ;; Disable
>      (:minor-mode blink-parentheses-mode))

The drawback of the current syntax is that `mode` and `eval`
are used like variables, but really are keywords.  Using the
keyword syntax `:` will help to resolve this ambiguity.

Then for backwards-compatibility dir-locals could support both
the current dotted pairs and new :keyword syntax, where unrecognized
keywords are simply ignored in older versions.

Also should file-local variables support the same :keyword syntax?

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> I think the above is better written:
>>
>>     ((nil
>>       (tab-width . 8)
>>       (sentence-end-double-space . t)
>>       (fill-column . 70)
>>       (emacs-lisp-docstring-fill-column . 65)
>>       (vc-git-annotate-switches . "-w")
>>       (bug-reference-url-format . "https://debbugs.gnu.org/%s")
>>       (diff-add-log-use-relative-names . t))
>>      (c-mode
>>       (c-file-style . "GNU")
>>       (c-noise-macro-names . ("INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK"))
>>       (electric-quote-comment . nil)
>>       (electric-quote-string . nil)
>>       (indent-tabs-mode . t)
>>       (mode . bug-reference-prog)))
>
> So the dotted pair notation should be avoided only at the first level.

That's one way to look at it.
The other is that the dotted syntax doesn't make much sense when what
you have after the `.` is always a list.

> Then what about making it closer to the syntax of display-buffer-alist rules
> where CONDITION is passed to buffer-match-p that can match not only major-mode,
> but also a regular expression of a buffer name.

No opinion on that.  But indeed, it could be useful to specify a kind of
local `auto-mode-alist` additions.

>
>> Maybe something like:
>>
>>     (c-mode
>>      (:set c-file-style "GNU")
>>      (:set treesit-thing t)
>>      (:set odd-list (cons 3 odd-list))
>>      (:minor-mode indent-tabs-mode -1) ;; Disable
>>      (:minor-mode blink-parentheses-mode))
>
> The drawback of the current syntax is that `mode` and `eval`
> are used like variables, but really are keywords.  Using the
> keyword syntax `:` will help to resolve this ambiguity.

I think that's minor.  The fact that it only contains values rather than
(safely computable) expressions is the more problematic part.

> Also should file-local variables support the same :keyword syntax?

I don't think there's as much need for changes there, so I'd rather wait
for the dir-locals part to stabilize before trying to see if/how to move
it over to file-local variables.


        Stefan

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> It sort of makes sense to me, but if we're doing a name change, I'd
> rather make it a functional change, too.

While I can see why that might sound attractive, AFAICT the functional
change is very far from being clear, so it seems unlikely it will happen
any time soon.


        Stefan

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

[Philip Kaludercic]

Stefan Kangas <stefankangas@gmail.com> writes:

> Severity: wishlist
>
> [Spun out from Bug#58486.]
>
> Juri Linkov <juri@linkov.net> writes:
>
>> It's already handled by auto-mode-alist:
>>
>>   (defvar auto-mode-alist
>>        ...
>>        ;; .dir-locals.el is not really Elisp.  Could use the
>>        ;; `dir-locals-file' constant if it weren't defined below.
>>        ("\\.dir-locals\\(?:-2\\)?\\.el\\'" . lisp-data-mode)
>
> It's nice if we can use the "*.eld" extension for files that are not
> supposed to be executed.  But it's hard to just change that outright, as
> a file ".dir-locals.eld" won't be used on old versions of Emacs.

FWIW it could be possible to mitigate this situation using Compat from
ELPA.  It could make use of the `hack-local-variables-hook' and
"transparently" add support for .dir-locals.eld.  Another idea would be
to provide such a feature as a package that could intentionally be
installed from ELPA for older versions.

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> When selecting a number of .el files in Dired and byte-compiling them
>>> I'll usually also select a .dir-locals.el file that won't compile.
>>> Would annotating them as non-byte-compilable files be sensible?
>>
>> Makes sense to me, FWIW.  Though Stefan M might point out that these
>> files should actually be called ".dir-locals.eld".  Which, BTW, we
>> should probably add support for.  And "*.eld" files should never be
>> compiled, I think?
>
> It's already handled by auto-mode-alist:
>
>   (defvar auto-mode-alist
>        ...
>        ;; .dir-locals.el is not really Elisp.  Could use the
>        ;; `dir-locals-file' constant if it weren't defined below.
>        ("\\.dir-locals\\(?:-2\\)?\\.el\\'" . lisp-data-mode)

To stay in the spirit of bug#58075, if we consider the major mode
specified in `auto-mode-alist` (and `mode:` cookies) as a proxy for
"file type", maybe we should arrange to try and avoid byte-compiling
those `.el` files whose major mode is `lisp-data-mode` (or: is not
`emacs-lisp-mode`)?


        Stefan

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Juri Linkov]

>>   (defvar auto-mode-alist
>>        ...
>>        ;; .dir-locals.el is not really Elisp.  Could use the
>>        ;; `dir-locals-file' constant if it weren't defined below.
>>        ("\\.dir-locals\\(?:-2\\)?\\.el\\'" . lisp-data-mode)
>
> To stay in the spirit of bug#58075, if we consider the major mode
> specified in `auto-mode-alist` (and `mode:` cookies) as a proxy for
> "file type", maybe we should arrange to try and avoid byte-compiling
> those `.el` files whose major mode is `lisp-data-mode` (or: is not
> `emacs-lisp-mode`)?

I don't know if such case is possible that auto-mode-alist specifies
lisp-data-mode for a file type to avoid byte-compiling, but
some file of that type needs to override its file type's default
by using a cookie `no-byte-compile: nil`?

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> To stay in the spirit of bug#58075, if we consider the major mode
>> specified in `auto-mode-alist` (and `mode:` cookies) as a proxy for
>> "file type", maybe we should arrange to try and avoid byte-compiling
>> those `.el` files whose major mode is `lisp-data-mode` (or: is not
>> `emacs-lisp-mode`)?
>
> I don't know if such case is possible that auto-mode-alist specifies
> lisp-data-mode for a file type to avoid byte-compiling, but
> some file of that type needs to override its file type's default
> by using a cookie `no-byte-compile: nil`?

I don't understand what you're saying.  The auto-mode-alist's purpose is
not "to avoid byte-compiling" but to indicate what kind of content is to
be found in the file.  And all I'm suggesting is that we should take
advantage of that info in more places, for example when we decide
whether to byte-compile something.


        Stefan

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> * lisp/files-x.el (modify-dir-local-variable): Add a 'no-byte-compile'
> file local variable when generating a .dir-locals.el file.

Looks good to me.

bug#58486: [PATCH] Prevent .dir-locals.el from being byte-compiled

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> * lisp/files-x.el (modify-dir-local-variable): Add a 'no-byte-compile'
>> file local variable when generating a .dir-locals.el file.
>
> Looks good to me.

It has been pushed.