unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions
@ 2023-10-21 12:08 Mauro Aranda
  2023-10-21 12:16 ` Mauro Aranda
  0 siblings, 1 reply; 8+ messages in thread
From: Mauro Aranda @ 2023-10-21 12:08 UTC (permalink / raw)
  To: 66663

Severity: wishlist

This is a feature request, which is a part of trying to land an Easy
Customization interface to editing dir locals files:
https://lists.gnu.org/archive/html/emacs-devel/2023-09/msg01306.html

The need is to be able to specify the file to modify, so that the
.dir-locals.el or .dir-locals-2.el can be customized. Currently, the
situation is:

emacs -Q
Go to a directory without a .dir-locals.el file
Use add-directory-local-variable to add a variable.  It goes to
.dir-locals.el, which is fine.  But it would be nice if I could select
to add it to .dir-locals-2.el instead.

Similarly, if both .dir-locals.el and .dir-locals-2.el exist, only the
.dir-locals-2.el can be modified with *-dir-local-variables, according
to my testing.  It'd be nice to be able to modify either.

The patch I'll send once I get assigned a Bug# is my first approach to
it.  I made the commands take an optional argument, which indicates: "I
want to modify that other file".  The intention is:
With no prefix arg:
- If no dir locals file exists, modify .dir-locals.el
- If one of them exists, modify that one.
- If the two exists, modify .dir-locals-2.el

This is just how today everything works, AFAICT.

With a prefix arg:
- If no dir locals file exists, modify .dir-locals-2.el
- If one of them exists, modify the other one.
- If the two exists, modify .dir-locals.el

Additionally, I made modify-dir-local-variable take a 5th argument,
optional, which can be a boolean or a string (a filename).  The string
use case is to be able to directly specify the file to modify, which is
useful so one doesn't have to check which file exists and which doesn't
(it'll prove useful in the customize code).  I'm not posting the code
for cus-edit.el because some part of the code depends if this proposed
change gets accepted.






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions
  2023-10-21 12:08 bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions Mauro Aranda
@ 2023-10-21 12:16 ` Mauro Aranda
  2023-10-21 12:45   ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Mauro Aranda @ 2023-10-21 12:16 UTC (permalink / raw)
  To: 66663; +Cc: Juri Linkov

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

tags 66663 patch
quit


Here's the patch.  Juri, could you take a look to see if this approach
is fine? Thank you.

[-- Attachment #2: 0001-Allow-specifying-the-dir-locals-file-to-edit-Bug-666.patch --]
[-- Type: text/x-patch, Size: 10254 bytes --]

From 1fa03835062bfd3519687eadc179d5647eb61f25 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sat, 21 Oct 2023 09:14:25 -0300
Subject: [PATCH] Allow specifying the dir locals file to edit (Bug#66663)

* lisp/files-x.el (modify-dir-local-variable): Take a 5th optional
argument, to manipulate which dir locals file gets selected.
(add-dir-local-variable, delete-dir-local-variable)
(copy-file-locals-to-dir-locals): Take a prefix argument and pass it
to modify-dir-local-variable.
* etc/NEWS: Announce the change.
* doc/emacs/custom.texi (Directory Variables): Document the new
functionality.
---
 doc/emacs/custom.texi | 14 ++++++-
 etc/NEWS              | 10 +++++
 lisp/files-x.el       | 91 +++++++++++++++++++++++++++++++++++--------
 3 files changed, 97 insertions(+), 18 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 8c30f26bbf7..df6bcad506a 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -1507,7 +1507,19 @@ Directory Variables
 entry defining the directory-local variable.  @kbd{M-x
 delete-dir-local-variable} deletes an entry.  @kbd{M-x
 copy-file-locals-to-dir-locals} copies the file-local variables in the
-current file into @file{.dir-locals.el}.
+current file into @file{.dir-locals.el}, or @file{.dir-locals-2.el} if
+that file is also present.
+
+Since both @file{.dir-locals.el} and @file{.dir-locals-2.el} file
+might exist in the same directory, there may be some clash about which
+file you want to modify when executing the above three commands.  To
+solve that, all three of them take a prefix argument, to indicate
+which file you want to modify.  When both files exist, a prefix
+argument means to prefer to modify @file{.dir-locals.el} instead of
+@file{.dir-locals-2.el}.  When one of the files doesn't exist, and
+you're adding a variable or copying the file-local variables, a prefix
+argument means to modify (i.e., create) the file that doesn't yet
+exist.
 
 @findex dir-locals-set-class-variables
 @findex dir-locals-set-directory-class
diff --git a/etc/NEWS b/etc/NEWS
index 3d4cdd876b3..5c5ae0581d2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -187,6 +187,11 @@ right-aligned to is controlled by the new user option
 It can be used to add, remove and reorder functions that change
 the appearance of every tab on the tab bar.
 
++++
+** New optional argument for modifying directory local variables
+The commands 'add-dir-local-variable', 'delete-dir-local-variable' and
+'copy-file-locals-to-dir-locals' now take an optional prefix argument,
+to indicate which file you want to modify.
 ** Miscellaneous
 
 ---
@@ -1337,6 +1342,11 @@ Since circular alias chains now cannot occur, 'function-alias-p',
 'indirect-function' and 'indirect-variable' will never signal an error.
 Their 'noerror' arguments have no effect and are therefore obsolete.
 
+---
+** New optional argument to 'modify-dir-local-variable'
+A 5th argument, optional, has been added to
+'modify-dir-local-variable'.  It can be used to specify which
+dir-locals file to modify.
 \f
 * Changes in Emacs 30.1 on Non-Free Operating Systems
 
diff --git a/lisp/files-x.el b/lisp/files-x.el
index 3ba7632d253..c0379fe1084 100644
--- a/lisp/files-x.el
+++ b/lisp/files-x.el
@@ -31,6 +31,7 @@
 ;;; Code:
 
 (eval-when-compile (require 'subr-x)) ; for string-trim-right
+(declare-function dosified-file-name "dos-fns" (file-name))
 
 \f
 ;;; Commands to add/delete file-local/directory-local variables.
@@ -410,7 +411,7 @@ delete-file-local-variable-prop-line
 
 (defvar auto-insert) ; from autoinsert.el
 
-(defun modify-dir-local-variable (mode variable value op)
+(defun modify-dir-local-variable (mode variable value op &optional file)
   "Modify directory-local VARIABLE in .dir-locals.el depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -422,7 +423,12 @@ modify-dir-local-variable
 this file in the current directory.
 
 If OP is `delete' then delete all existing settings of VARIABLE
-from the MODE alist ignoring the input argument VALUE."
+from the MODE alist ignoring the input argument VALUE.
+
+FILE specifies what file to modify.  It can be a string, the name of the
+dir-locals file to modify.  It can also be any other non-nil value, in which
+case the file to modify is .dir-locals.el, if .dir-locals-2.el exists, or
+.dir-locals-2.el if .dir-locals.el exists but the former doesn't."
   (catch 'exit
     (unless enable-local-variables
       (throw 'exit (message "Directory-local variables are disabled")))
@@ -432,7 +438,8 @@ modify-dir-local-variable
            (variables-file
             ;; If there are several .dir-locals, the user probably
             ;; wants to edit the last one (the highest priority).
-            (cond ((stringp dir-or-cache)
+            (cond ((stringp file) file)
+                  ((stringp dir-or-cache)
                    (car (last (dir-locals--all-files dir-or-cache))))
                   ((consp dir-or-cache)	; result from cache
                    ;; If cache element has an mtime, assume it came
@@ -441,9 +448,34 @@ modify-dir-local-variable
                    (if (nth 2 dir-or-cache)
                        (car (last (dir-locals--all-files (car dir-or-cache))))
                      (cadr dir-or-cache)))
-                  ;; Try to make a proper file-name.
-                  (t (expand-file-name dir-locals-file))))
+                  ;; Try to make a proper file-name, respecting FILE.
+                  (t (let* ((pri (expand-file-name
+                                  (if (eq system-type 'ms-dos)
+                                      (dosified-file-name dir-locals-file)
+                                    dir-locals-file)))
+                            (sec (replace-regexp-in-string ".el$" "-2.el" pri))
+                            (sec-exists-p (file-exists-p sec)))
+                       (if file
+                           ;; When alternating, prefer .dir-locals.el if
+                           ;; .dir-locals-2.el exists, or .dir-locals-2.el
+                           ;; if it doesn't exist (or neither exist).
+                           (if sec-exists-p pri sec)
+                         (if sec-exists-p sec pri))))))
            variables)
+      ;; When alternating, FILE is non-nil and not a string.
+      ;; We check for `dir-or-cache' non-nil, because we handled the nil
+      ;; case in the `cond' above.
+      (when (and file (not (stringp file))
+                 dir-or-cache
+                 (stringp variables-file))
+        ;; If we got the .dir-locals-2.el file, make it
+        ;; the .dir-locals.el file.
+        (if (string-match "[^2]\\(\\.el$\\)" variables-file)
+            (setq variables-file (replace-match "-2.el" t nil variables-file 1))
+          ;; And if we got the .dir-locals.el file, make it the
+          ;; .dir-locals-2.el file.
+          (setq variables-file
+                (replace-regexp-in-string "-2.el$" ".el" variables-file))))
       ;; I can't be bothered to handle this case right now.
       ;; Dir locals were set directly from a class.  You need to
       ;; directly modify the class in dir-locals-class-alist.
@@ -528,32 +560,57 @@ dir-locals-to-string
            variables "\n")))
 
 ;;;###autoload
-(defun add-dir-local-variable (mode variable value)
-  "Add directory-local VARIABLE with its VALUE and MODE to .dir-locals.el."
+(defun add-dir-local-variable (mode variable value &optional alt)
+  "Add directory-local VARIABLE with its VALUE and MODE to .dir-locals.el.
+
+A prefix argument changes the file to modify.  When given and .dir-locals.el
+exists but .dir-locals-2.el does not, it modifies .dir-locals-2.el, possibly
+creating it.  On the contrary, when both files exist, a prefix argument means
+to prefer to add VARIABLE to .dir-locals.el.
+
+When called from Lisp, ALT may also be the expanded name of the dir-locals file
+where to add VARIABLE."
   (interactive
    (let (variable)
      (list
       (read-file-local-variable-mode)
       (setq variable (read-file-local-variable "Add directory-local variable"))
-      (read-file-local-variable-value variable))))
-  (modify-dir-local-variable mode variable value 'add-or-replace))
+      (read-file-local-variable-value variable)
+      current-prefix-arg)))
+   (modify-dir-local-variable mode variable value 'add-or-replace alt))
 
 ;;;###autoload
-(defun delete-dir-local-variable (mode variable)
-  "Delete all MODE settings of file-local VARIABLE from .dir-locals.el."
+(defun delete-dir-local-variable (mode variable &optional alt)
+  "Delete all MODE settings of dir-local VARIABLE from .dir-locals.el.
+
+A prefix argument changes the file to modify.  If given and if both
+.dir-locals-2.el and .dir-locals.el exist, prefer to delete VARIABLE from
+.dir-locals.el.
+
+When called from Lisp, ALT may also be the expanded name of the dir-locals file
+from where to delete VARIABLE."
   (interactive
    (list
     (read-file-local-variable-mode)
-    (read-file-local-variable "Delete directory-local variable")))
-  (modify-dir-local-variable mode variable nil 'delete))
+    (read-file-local-variable "Delete directory-local variable")
+    current-prefix-arg))
+  (modify-dir-local-variable mode variable nil 'delete alt))
 
 ;;;###autoload
-(defun copy-file-locals-to-dir-locals ()
-  "Copy file-local variables to .dir-locals.el."
-  (interactive)
+(defun copy-file-locals-to-dir-locals (&optional alt)
+  "Copy file-local variables to .dir-locals.el.
+
+A prefix argument changes the file to modify.  When given and .dir-locals.el
+exists but .dir-locals-2.el does not, it modifies .dir-locals-2.el, possibly
+creating it.  On the contrary, if both files exist, a prefix argument means
+to prefer to copy the variables to .dir-locals.el.
+
+When called from Lisp, ALT may also be the expanded name of the dir-locals file
+where to copy the file-local variables."
+  (interactive "P")
   (dolist (elt file-local-variables-alist)
     (unless (assq (car elt) dir-local-variables-alist)
-      (add-dir-local-variable major-mode (car elt) (cdr elt)))))
+      (add-dir-local-variable major-mode (car elt) (cdr elt) alt))))
 
 ;;;###autoload
 (defun copy-dir-locals-to-file-locals ()
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions
  2023-10-21 12:16 ` Mauro Aranda
@ 2023-10-21 12:45   ` Eli Zaretskii
  2023-10-21 13:49     ` Mauro Aranda
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-10-21 12:45 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 66663, juri

> Cc: Juri Linkov <juri@linkov.net>
> Date: Sat, 21 Oct 2023 09:16:28 -0300
> From: Mauro Aranda <maurooaranda@gmail.com>
> 
> +Since both @file{.dir-locals.el} and @file{.dir-locals-2.el} file
> +might exist in the same directory, there may be some clash about which
> +file you want to modify when executing the above three commands.  To
> +solve that, all three of them take a prefix argument, to indicate
> +which file you want to modify.  When both files exist, a prefix
> +argument means to prefer to modify @file{.dir-locals.el} instead of
> +@file{.dir-locals-2.el}.  When one of the files doesn't exist, and
> +you're adding a variable or copying the file-local variables, a prefix
> +argument means to modify (i.e., create) the file that doesn't yet
> +exist.

I think a better UI is to ask the user when the command is invoked
with a prefix argument.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions
  2023-10-21 12:45   ` Eli Zaretskii
@ 2023-10-21 13:49     ` Mauro Aranda
  2023-10-21 14:06       ` Mauro Aranda
  0 siblings, 1 reply; 8+ messages in thread
From: Mauro Aranda @ 2023-10-21 13:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66663, juri

On 21/10/23 09:45, Eli Zaretskii wrote:
 >> Cc: Juri Linkov <juri@linkov.net>
 >> Date: Sat, 21 Oct 2023 09:16:28 -0300
 >> From: Mauro Aranda <maurooaranda@gmail.com>
 >>
 >> +Since both @file{.dir-locals.el} and @file{.dir-locals-2.el} file
 >> +might exist in the same directory, there may be some clash about which
 >> +file you want to modify when executing the above three commands.  To
 >> +solve that, all three of them take a prefix argument, to indicate
 >> +which file you want to modify.  When both files exist, a prefix
 >> +argument means to prefer to modify @file{.dir-locals.el} instead of
 >> +@file{.dir-locals-2.el}.  When one of the files doesn't exist, and
 >> +you're adding a variable or copying the file-local variables, a prefix
 >> +argument means to modify (i.e., create) the file that doesn't yet
 >> +exist.
 >
 > I think a better UI is to ask the user when the command is invoked
 > with a prefix argument.

Yes, that makes sense.  I'll try that approach.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions
  2023-10-21 13:49     ` Mauro Aranda
@ 2023-10-21 14:06       ` Mauro Aranda
  2023-10-21 18:21         ` Juri Linkov
  0 siblings, 1 reply; 8+ messages in thread
From: Mauro Aranda @ 2023-10-21 14:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66663, juri

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

On 21/10/23 10:49, Mauro Aranda wrote:
 > On 21/10/23 09:45, Eli Zaretskii wrote:
 >  >> Cc: Juri Linkov <juri@linkov.net>
 >  >> Date: Sat, 21 Oct 2023 09:16:28 -0300
 >  >> From: Mauro Aranda <maurooaranda@gmail.com>
 >  >>
 >  >> +Since both @file{.dir-locals.el} and @file{.dir-locals-2.el} file
 >  >> +might exist in the same directory, there may be some clash about 
which
 >  >> +file you want to modify when executing the above three commands.  To
 >  >> +solve that, all three of them take a prefix argument, to indicate
 >  >> +which file you want to modify.  When both files exist, a prefix
 >  >> +argument means to prefer to modify @file{.dir-locals.el} instead of
 >  >> +@file{.dir-locals-2.el}.  When one of the files doesn't exist, and
 >  >> +you're adding a variable or copying the file-local variables, a 
prefix
 >  >> +argument means to modify (i.e., create) the file that doesn't yet
 >  >> +exist.
 >  >
 >  > I think a better UI is to ask the user when the command is invoked
 >  > with a prefix argument.
 >
 > Yes, that makes sense.  I'll try that approach.

I reworked the patch to implement that UI instead.  Patch attached.

[-- Attachment #2: 0001-Allow-specifying-the-dir-locals-file-to-edit-Bug-666.patch --]
[-- Type: text/x-patch, Size: 9732 bytes --]

From 077a6a8b83cd1fb6efa9f169831ad6adec52312a Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sat, 21 Oct 2023 11:02:36 -0300
Subject: [PATCH] Allow specifying the dir locals file to edit (Bug#66663)

* lisp/files-x.el (modify-dir-local-variable): Take a 5th optional
argument, the filename of the dir locals file to modify.
(add-dir-local-variable, delete-dir-local-variable)
(copy-file-locals-to-dir-locals): Optionally read the filename to
modify, and pass it to modify-dir-local-variable.
* etc/NEWS: Announce the change.
* doc/emacs/custom.texi (Directory Variables): Document the new
functionality.
---
 doc/emacs/custom.texi |   8 ++-
 etc/NEWS              |  10 ++++
 lisp/files-x.el       | 110 ++++++++++++++++++++++++++++++------------
 3 files changed, 95 insertions(+), 33 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 8c30f26bbf7..adecc873163 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -1507,7 +1507,13 @@ Directory Variables
 entry defining the directory-local variable.  @kbd{M-x
 delete-dir-local-variable} deletes an entry.  @kbd{M-x
 copy-file-locals-to-dir-locals} copies the file-local variables in the
-current file into @file{.dir-locals.el}.
+current file into @file{.dir-locals.el}, or @file{.dir-locals-2.el} if
+that file is also present.
+
+With a prefix argument, all three commands prompt for the file you
+want to modify.  Although it doesn't have to exist, you must enter a
+valid filename, either @file{.dir-locals.el} or
+@file{.dir-locals-2.el}.
 
 @findex dir-locals-set-class-variables
 @findex dir-locals-set-directory-class
diff --git a/etc/NEWS b/etc/NEWS
index 3d4cdd876b3..d48466be305 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -187,6 +187,11 @@ right-aligned to is controlled by the new user option
 It can be used to add, remove and reorder functions that change
 the appearance of every tab on the tab bar.
 
++++
+** New optional argument for modifying directory local variables
+The commands 'add-dir-local-variable', 'delete-dir-local-variable' and
+'copy-file-locals-to-dir-locals' now take an optional prefix argument,
+to enter the file you want to modify.
 ** Miscellaneous
 
 ---
@@ -1337,6 +1342,11 @@ Since circular alias chains now cannot occur, 'function-alias-p',
 'indirect-function' and 'indirect-variable' will never signal an error.
 Their 'noerror' arguments have no effect and are therefore obsolete.
 
+---
+** New optional argument to 'modify-dir-local-variable'
+A 5th argument, optional, has been added to
+'modify-dir-local-variable'.  It can be used to specify which
+dir-locals file to modify.
 \f
 * Changes in Emacs 30.1 on Non-Free Operating Systems
 
diff --git a/lisp/files-x.el b/lisp/files-x.el
index 3ba7632d253..ef3adcb9cd3 100644
--- a/lisp/files-x.el
+++ b/lisp/files-x.el
@@ -31,6 +31,7 @@
 ;;; Code:
 
 (eval-when-compile (require 'subr-x)) ; for string-trim-right
+(declare-function dosified-file-name "dos-fns" (file-name))
 
 \f
 ;;; Commands to add/delete file-local/directory-local variables.
@@ -410,7 +411,7 @@ delete-file-local-variable-prop-line
 
 (defvar auto-insert) ; from autoinsert.el
 
-(defun modify-dir-local-variable (mode variable value op)
+(defun modify-dir-local-variable (mode variable value op &optional file)
   "Modify directory-local VARIABLE in .dir-locals.el depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -422,28 +423,37 @@ modify-dir-local-variable
 this file in the current directory.
 
 If OP is `delete' then delete all existing settings of VARIABLE
-from the MODE alist ignoring the input argument VALUE."
+from the MODE alist ignoring the input argument VALUE.
+
+Optional argument FILE, when non-nil, specifies what file to modify.  It
+should be an expanded filename."
   (catch 'exit
     (unless enable-local-variables
       (throw 'exit (message "Directory-local variables are disabled")))
-    (let* ((dir-or-cache (and (buffer-file-name)
-                              (not (file-remote-p (buffer-file-name)))
-                              (dir-locals-find-file (buffer-file-name))))
-           (variables-file
-            ;; If there are several .dir-locals, the user probably
-            ;; wants to edit the last one (the highest priority).
-            (cond ((stringp dir-or-cache)
-                   (car (last (dir-locals--all-files dir-or-cache))))
-                  ((consp dir-or-cache)	; result from cache
-                   ;; If cache element has an mtime, assume it came
-                   ;; from a file.  Otherwise, assume it was set
-                   ;; directly.
-                   (if (nth 2 dir-or-cache)
-                       (car (last (dir-locals--all-files (car dir-or-cache))))
-                     (cadr dir-or-cache)))
-                  ;; Try to make a proper file-name.
-                  (t (expand-file-name dir-locals-file))))
-           variables)
+    (let ((variables-file
+           (if (stringp file)
+               file
+             (let ((dir-or-cache
+                    (and (buffer-file-name)
+                         (not (file-remote-p (buffer-file-name)))
+                         (dir-locals-find-file (buffer-file-name)))))
+               ;; If there are several .dir-locals, the user probably
+               ;; wants to edit the last one (the highest priority).
+               (cond
+                ((stringp dir-or-cache)
+                 (car (last (dir-locals--all-files dir-or-cache))))
+                ((consp dir-or-cache)	; result from cache
+                 ;; If cache element has an mtime, assume it came
+                 ;; from a file.  Otherwise, assume it was set
+                 ;; directly.
+                 (if (nth 2 dir-or-cache)
+                     (car (last (dir-locals--all-files (car dir-or-cache))))
+                   (cadr dir-or-cache)))
+                ;; Try to make a proper file-name.
+                (t (expand-file-name (if (eq system-type 'ms-dos)
+                                         (dosified-file-name dir-locals-file)
+                                       dir-locals-file)))))))
+          variables)
       ;; I can't be bothered to handle this case right now.
       ;; Dir locals were set directly from a class.  You need to
       ;; directly modify the class in dir-locals-class-alist.
@@ -527,33 +537,69 @@ dir-locals-to-string
                                      (cdr mode-variables) "\n"))))
            variables "\n")))
 
+(defun read-dir-locals-file ()
+  "Read a dir-locals filename using completion.
+Intended to be used in the `interactive' spec of `add-dir-local-variable',
+`delete-dir-local-variable' and `copy-file-locals-to-dir-locals'.
+
+Returns the filename, expanded."
+  (expand-file-name
+   (read-file-name "File" nil nil
+                   (lambda (fname)
+                     (member (file-name-nondirectory fname)
+                             (list dir-locals-file
+                                   (replace-regexp-in-string
+                                    ".el$" "-2.el" dir-locals-file))))
+                   dir-locals-file)))
+
 ;;;###autoload
-(defun add-dir-local-variable (mode variable value)
-  "Add directory-local VARIABLE with its VALUE and MODE to .dir-locals.el."
+(defun add-dir-local-variable (mode variable value &optional file)
+  "Add directory-local VARIABLE with its VALUE and MODE to .dir-locals.el.
+
+With a prefix argument, prompt for the file to modify.
+
+When called from Lisp, FILE may be the expanded name of the dir-locals file
+where to add VARIABLE."
   (interactive
    (let (variable)
      (list
       (read-file-local-variable-mode)
       (setq variable (read-file-local-variable "Add directory-local variable"))
-      (read-file-local-variable-value variable))))
-  (modify-dir-local-variable mode variable value 'add-or-replace))
+      (read-file-local-variable-value variable)
+      (when current-prefix-arg
+        (read-dir-locals-file)))))
+  (modify-dir-local-variable mode variable value 'add-or-replace file))
 
 ;;;###autoload
-(defun delete-dir-local-variable (mode variable)
-  "Delete all MODE settings of file-local VARIABLE from .dir-locals.el."
+(defun delete-dir-local-variable (mode variable &optional file)
+  "Delete all MODE settings of dir-local VARIABLE from .dir-locals.el.
+
+With a prefix argument, prompt for the file to modify.
+
+When called from Lisp, FILE may be the expanded name of the dir-locals file
+from where to delete VARIABLE."
   (interactive
    (list
     (read-file-local-variable-mode)
-    (read-file-local-variable "Delete directory-local variable")))
-  (modify-dir-local-variable mode variable nil 'delete))
+    (read-file-local-variable "Delete directory-local variable")
+    (when current-prefix-arg
+      (read-dir-locals-file))))
+  (modify-dir-local-variable mode variable nil 'delete file))
 
 ;;;###autoload
-(defun copy-file-locals-to-dir-locals ()
-  "Copy file-local variables to .dir-locals.el."
-  (interactive)
+(defun copy-file-locals-to-dir-locals (&optional file)
+  "Copy file-local variables to .dir-locals.el.
+
+With a prefix argument, prompt for the file to modify.
+
+When called from Lisp, FILE may be the expanded name of the dir-locals file
+where to copy the file-local variables."
+  (interactive
+   (list (when current-prefix-arg
+           (read-dir-locals-file))))
   (dolist (elt file-local-variables-alist)
     (unless (assq (car elt) dir-local-variables-alist)
-      (add-dir-local-variable major-mode (car elt) (cdr elt)))))
+      (add-dir-local-variable major-mode (car elt) (cdr elt) file))))
 
 ;;;###autoload
 (defun copy-dir-locals-to-file-locals ()
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions
  2023-10-21 14:06       ` Mauro Aranda
@ 2023-10-21 18:21         ` Juri Linkov
  2023-10-21 22:10           ` Mauro Aranda
  0 siblings, 1 reply; 8+ messages in thread
From: Juri Linkov @ 2023-10-21 18:21 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: Eli Zaretskii, 66663

> I reworked the patch to implement that UI instead.  Patch attached.

Thanks, some comments:

> +(defun read-dir-locals-file ()
> +  "Read a dir-locals filename using completion.
> +Intended to be used in the `interactive' spec of `add-dir-local-variable',
> +`delete-dir-local-variable' and `copy-file-locals-to-dir-locals'.
> +
> +Returns the filename, expanded."
> +  (expand-file-name
> +   (read-file-name "File" nil nil

The prompt needs a colon.

> +                   (lambda (fname)
> +                     (member (file-name-nondirectory fname)
> +                             (list dir-locals-file
> +                                   (replace-regexp-in-string
> +                                    ".el$" "-2.el" dir-locals-file))))
> +                   dir-locals-file)))

This needs to be run in the project's root directory
where the dir-locals file is saved.  Probably it's not easy
to find the root without using project.el.  But when one of the
files already exists (either .dir-locals.el or .dir-locals-2.el),
then better to use the directory of the existing file by default.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions
  2023-10-21 18:21         ` Juri Linkov
@ 2023-10-21 22:10           ` Mauro Aranda
  2023-10-22 17:55             ` Juri Linkov
  0 siblings, 1 reply; 8+ messages in thread
From: Mauro Aranda @ 2023-10-21 22:10 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Eli Zaretskii, 66663

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

On 21/10/23 15:21, Juri Linkov wrote:
 >> +(defun read-dir-locals-file ()
 >> +  "Read a dir-locals filename using completion.
 >> +Intended to be used in the `interactive' spec of 
`add-dir-local-variable',
 >> +`delete-dir-local-variable' and `copy-file-locals-to-dir-locals'.
 >> +
 >> +Returns the filename, expanded."
 >> +  (expand-file-name
 >> +   (read-file-name "File" nil nil
 >
 > The prompt needs a colon.

Fixed.

 >> +                   (lambda (fname)
 >> +                     (member (file-name-nondirectory fname)
 >> +                             (list dir-locals-file
 >> + (replace-regexp-in-string
 >> +                                    ".el$" "-2.el" dir-locals-file))))
 >> +                   dir-locals-file)))
 >
 > This needs to be run in the project's root directory
 > where the dir-locals file is saved.  Probably it's not easy
 > to find the root without using project.el.  But when one of the
 > files already exists (either .dir-locals.el or .dir-locals-2.el),
 > then better to use the directory of the existing file by default.

In this new patch, I went for using it if already available and we
couldn't find an existing file by other means.

[-- Attachment #2: 0001-Allow-specifying-the-dir-locals-file-to-edit-Bug-666.patch --]
[-- Type: text/x-patch, Size: 9967 bytes --]

From 8a084557ca4647e82effc7007974ac1b1c66a27a Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sat, 21 Oct 2023 11:02:36 -0300
Subject: [PATCH] Allow specifying the dir locals file to edit (Bug#66663)

* lisp/files-x.el (modify-dir-local-variable): Take a 5th optional
argument, the filename of the dir locals file to modify.
(read-dir-locals-file): New function.
(add-dir-local-variable, delete-dir-local-variable)
(copy-file-locals-to-dir-locals): Optionally read the filename to
modify, and pass it to modify-dir-local-variable.
* etc/NEWS: Announce the change.
* doc/emacs/custom.texi (Directory Variables): Document the new
functionality.
---
 doc/emacs/custom.texi |   8 ++-
 etc/NEWS              |  10 ++++
 lisp/files-x.el       | 117 ++++++++++++++++++++++++++++++------------
 3 files changed, 102 insertions(+), 33 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 8c30f26bbf7..adecc873163 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -1507,7 +1507,13 @@ Directory Variables
 entry defining the directory-local variable.  @kbd{M-x
 delete-dir-local-variable} deletes an entry.  @kbd{M-x
 copy-file-locals-to-dir-locals} copies the file-local variables in the
-current file into @file{.dir-locals.el}.
+current file into @file{.dir-locals.el}, or @file{.dir-locals-2.el} if
+that file is also present.
+
+With a prefix argument, all three commands prompt for the file you
+want to modify.  Although it doesn't have to exist, you must enter a
+valid filename, either @file{.dir-locals.el} or
+@file{.dir-locals-2.el}.
 
 @findex dir-locals-set-class-variables
 @findex dir-locals-set-directory-class
diff --git a/etc/NEWS b/etc/NEWS
index 5d42d88fb60..a3639620ebb 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -187,6 +187,11 @@ right-aligned to is controlled by the new user option
 It can be used to add, remove and reorder functions that change
 the appearance of every tab on the tab bar.
 
++++
+** New optional argument for modifying directory local variables
+The commands 'add-dir-local-variable', 'delete-dir-local-variable' and
+'copy-file-locals-to-dir-locals' now take an optional prefix argument,
+to enter the file you want to modify.
 ** Miscellaneous
 
 ---
@@ -1358,6 +1363,11 @@ Their 'noerror' arguments have no effect and are therefore obsolete.
 When supplied with ':default-language LANGUAGE', rules after it will
 default to use 'LANGUAGE'.
 
+---
+** New optional argument to 'modify-dir-local-variable'
+A 5th argument, optional, has been added to
+'modify-dir-local-variable'.  It can be used to specify which
+dir-locals file to modify.
 \f
 * Changes in Emacs 30.1 on Non-Free Operating Systems
 
diff --git a/lisp/files-x.el b/lisp/files-x.el
index 3ba7632d253..f6fbd44ce21 100644
--- a/lisp/files-x.el
+++ b/lisp/files-x.el
@@ -31,6 +31,8 @@
 ;;; Code:
 
 (eval-when-compile (require 'subr-x)) ; for string-trim-right
+(declare-function dosified-file-name "dos-fns" (file-name))
+(declare-function project-root "project" (project))
 
 \f
 ;;; Commands to add/delete file-local/directory-local variables.
@@ -410,7 +412,7 @@ delete-file-local-variable-prop-line
 
 (defvar auto-insert) ; from autoinsert.el
 
-(defun modify-dir-local-variable (mode variable value op)
+(defun modify-dir-local-variable (mode variable value op &optional file)
   "Modify directory-local VARIABLE in .dir-locals.el depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -422,28 +424,37 @@ modify-dir-local-variable
 this file in the current directory.
 
 If OP is `delete' then delete all existing settings of VARIABLE
-from the MODE alist ignoring the input argument VALUE."
+from the MODE alist ignoring the input argument VALUE.
+
+Optional argument FILE, when non-nil, specifies what file to modify.  It
+should be an expanded filename."
   (catch 'exit
     (unless enable-local-variables
       (throw 'exit (message "Directory-local variables are disabled")))
-    (let* ((dir-or-cache (and (buffer-file-name)
-                              (not (file-remote-p (buffer-file-name)))
-                              (dir-locals-find-file (buffer-file-name))))
-           (variables-file
-            ;; If there are several .dir-locals, the user probably
-            ;; wants to edit the last one (the highest priority).
-            (cond ((stringp dir-or-cache)
-                   (car (last (dir-locals--all-files dir-or-cache))))
-                  ((consp dir-or-cache)	; result from cache
-                   ;; If cache element has an mtime, assume it came
-                   ;; from a file.  Otherwise, assume it was set
-                   ;; directly.
-                   (if (nth 2 dir-or-cache)
-                       (car (last (dir-locals--all-files (car dir-or-cache))))
-                     (cadr dir-or-cache)))
-                  ;; Try to make a proper file-name.
-                  (t (expand-file-name dir-locals-file))))
-           variables)
+    (let ((variables-file
+           (if (stringp file)
+               file
+             (let ((dir-or-cache
+                    (and (buffer-file-name)
+                         (not (file-remote-p (buffer-file-name)))
+                         (dir-locals-find-file (buffer-file-name)))))
+               ;; If there are several .dir-locals, the user probably
+               ;; wants to edit the last one (the highest priority).
+               (cond
+                ((stringp dir-or-cache)
+                 (car (last (dir-locals--all-files dir-or-cache))))
+                ((consp dir-or-cache)	; result from cache
+                 ;; If cache element has an mtime, assume it came
+                 ;; from a file.  Otherwise, assume it was set
+                 ;; directly.
+                 (if (nth 2 dir-or-cache)
+                     (car (last (dir-locals--all-files (car dir-or-cache))))
+                   (cadr dir-or-cache)))
+                ;; Try to make a proper file-name.
+                (t (expand-file-name (if (eq system-type 'ms-dos)
+                                         (dosified-file-name dir-locals-file)
+                                       dir-locals-file)))))))
+          variables)
       ;; I can't be bothered to handle this case right now.
       ;; Dir locals were set directly from a class.  You need to
       ;; directly modify the class in dir-locals-class-alist.
@@ -527,33 +538,75 @@ dir-locals-to-string
                                      (cdr mode-variables) "\n"))))
            variables "\n")))
 
+(defun read-dir-locals-file ()
+  "Read a dir-locals filename using completion.
+Intended to be used in the `interactive' spec of `add-dir-local-variable',
+`delete-dir-local-variable' and `copy-file-locals-to-dir-locals'.
+
+Returns the filename, expanded."
+  (let* ((pri dir-locals-file)
+         (sec (replace-regexp-in-string ".el$" "-2.el" dir-locals-file))
+         (dir (or (locate-dominating-file default-directory pri)
+                  (locate-dominating-file default-directory sec))))
+    (expand-file-name
+     (read-file-name
+      "File: "
+      (cond (dir)
+            ((when-let ((proj (and (featurep 'project) (project-current))))
+               (project-root proj))))
+      nil
+      (lambda (fname)
+        (member (file-name-nondirectory fname) (list pri sec)))
+      dir-locals-file))))
+
 ;;;###autoload
-(defun add-dir-local-variable (mode variable value)
-  "Add directory-local VARIABLE with its VALUE and MODE to .dir-locals.el."
+(defun add-dir-local-variable (mode variable value &optional file)
+  "Add directory-local VARIABLE with its VALUE and MODE to .dir-locals.el.
+
+With a prefix argument, prompt for the file to modify.
+
+When called from Lisp, FILE may be the expanded name of the dir-locals file
+where to add VARIABLE."
   (interactive
    (let (variable)
      (list
       (read-file-local-variable-mode)
       (setq variable (read-file-local-variable "Add directory-local variable"))
-      (read-file-local-variable-value variable))))
-  (modify-dir-local-variable mode variable value 'add-or-replace))
+      (read-file-local-variable-value variable)
+      (when current-prefix-arg
+        (read-dir-locals-file)))))
+  (modify-dir-local-variable mode variable value 'add-or-replace file))
 
 ;;;###autoload
-(defun delete-dir-local-variable (mode variable)
-  "Delete all MODE settings of file-local VARIABLE from .dir-locals.el."
+(defun delete-dir-local-variable (mode variable &optional file)
+  "Delete all MODE settings of dir-local VARIABLE from .dir-locals.el.
+
+With a prefix argument, prompt for the file to modify.
+
+When called from Lisp, FILE may be the expanded name of the dir-locals file
+from where to delete VARIABLE."
   (interactive
    (list
     (read-file-local-variable-mode)
-    (read-file-local-variable "Delete directory-local variable")))
-  (modify-dir-local-variable mode variable nil 'delete))
+    (read-file-local-variable "Delete directory-local variable")
+    (when current-prefix-arg
+      (read-dir-locals-file))))
+  (modify-dir-local-variable mode variable nil 'delete file))
 
 ;;;###autoload
-(defun copy-file-locals-to-dir-locals ()
-  "Copy file-local variables to .dir-locals.el."
-  (interactive)
+(defun copy-file-locals-to-dir-locals (&optional file)
+  "Copy file-local variables to .dir-locals.el.
+
+With a prefix argument, prompt for the file to modify.
+
+When called from Lisp, FILE may be the expanded name of the dir-locals file
+where to copy the file-local variables."
+  (interactive
+   (list (when current-prefix-arg
+           (read-dir-locals-file))))
   (dolist (elt file-local-variables-alist)
     (unless (assq (car elt) dir-local-variables-alist)
-      (add-dir-local-variable major-mode (car elt) (cdr elt)))))
+      (add-dir-local-variable major-mode (car elt) (cdr elt) file))))
 
 ;;;###autoload
 (defun copy-dir-locals-to-file-locals ()
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions
  2023-10-21 22:10           ` Mauro Aranda
@ 2023-10-22 17:55             ` Juri Linkov
  0 siblings, 0 replies; 8+ messages in thread
From: Juri Linkov @ 2023-10-22 17:55 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: Eli Zaretskii, 66663

close 66663 30.0.50
thanks

>> This needs to be run in the project's root directory
>> where the dir-locals file is saved.  Probably it's not easy
>> to find the root without using project.el.  But when one of the
>> files already exists (either .dir-locals.el or .dir-locals-2.el),
>> then better to use the directory of the existing file by default.
>
> In this new patch, I went for using it if already available and we
> couldn't find an existing file by other means.

Thanks, I've tested your patch, and it works nicely, so now pushed to master.





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-10-22 17:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-21 12:08 bug#66663: 30.0.50; Allow dir locals file selection in *-dir-local-variable functions Mauro Aranda
2023-10-21 12:16 ` Mauro Aranda
2023-10-21 12:45   ` Eli Zaretskii
2023-10-21 13:49     ` Mauro Aranda
2023-10-21 14:06       ` Mauro Aranda
2023-10-21 18:21         ` Juri Linkov
2023-10-21 22:10           ` Mauro Aranda
2023-10-22 17:55             ` Juri Linkov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).