bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#61901: 30.0.50; [PATCH] Add permanently-enabled-local-variable-dirs


From: Eli Zaretskii
Subject: bug#61901: 30.0.50; [PATCH] Add permanently-enabled-local-variable-dirs variable.
Date: Thu, 02 Mar 2023 08:57:21 +0200

> Date: Wed, 01 Mar 2023 22:20:33 +0000
> From:  Antero Mejr via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> This patch allows users to trust directories to load dir-local variables
> from, so they don't have to do something lile this:
> (defun risky-local-variable-p (sym &optional _ignored) nil)
> as suggested here:
> https://emacs.stackexchange.com/questions/10983/remember-permission-to-execute-risky-local-variables
> 
> It also works over TRAMP if enable-remote-dir-locals is true.

Thanks, IMO this is a very useful feature.

> --- a/doc/lispref/variables.texi
> +++ b/doc/lispref/variables.texi
> @@ -1974,6 +1974,12 @@ File Local Variables
>  symbols.
>  @end defvar
>  
> +@defvar permanently-enabled-local-variable-dirs
> +This is a list of trusted directories that contain local variables.
> +Local variables in these directories will always be enabled, regardless
> +of whether they are risky.
> +@end defvar

This should explicitly allude to the '.dir-locals.el' files in those
directories, since otherwise talking about "directories that contain
variables" could be confusing.

I also suggest to rename the variable to something like
'permanently-safe-local-variable-directories', or maybe just
'safe-local-variable-directories' which IMO should express the purpose
better.

> -Also see the `permanently-enabled-local-variables' variable."
> +Also see the `permanently-enabled-local-variables' and
> +'permanently-enabled-local-variable-dirs' variables."
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We quote `like this' in doc strings, to produce links in the *Help*
buffers.

> +(defcustom permanently-enabled-local-variable-dirs '()
> +  "A list of directories that contain local variables that are always
> +enabled, regardless of whether they are risky."

The first line of a doc string should be a single complete sentence.
(This is because the various apropos commands show only the first line
of the doc string.)

> @@ -3730,7 +3739,9 @@ hack-local-variables-confirm
>  !  -- to apply the local variables list, and permanently mark these
>        values (*) as safe (in the future, they will be set automatically.)
>  i  -- to ignore the local variables list, and permanently mark these
> -      values (*) as ignored\n\n")
> +      values (*) as ignored
> ++  -- to apply the local variables list, and permanently trust "
> +                    name "\n\n")

"permanently trust name" sounds confusing (what is "name"?).  How
about this variant:

  +  -- to apply the local variables list, and permanently trust
        all directory-local variables in this directory

> @@ -3762,8 +3773,13 @@ hack-local-variables-confirm
>              char)
>         (when offer-save
>              (push ?i exit-chars)
> -            (push ?! exit-chars))
> +            (push ?! exit-chars)
> +            (push ?+ exit-chars))
>         (setq char (read-char-choice prompt exit-chars))
> +          (when (and offer-save (= char ?+))
> +            (customize-push-and-save
> +             'permanently-enabled-local-variable-dirs
> +             (list dir-name)))

Bother: AFAIU here we modify the user's custom file without asking for
an explicit permission.  Should we ask for permission?

Last, but not least: this change is larger than what we can accept
without you assigning to FSF the copyright for your changes, and I
don't see any copyright assignment in your name on file.  Would you be
willing to do the legal paperwork for such an assignment?  If yes, I
will send you the form to start the paperwork rolling; when it is
completed, we can install your changes.

Thanks.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]