GNU bug report logs - #32817
27.0.50; Dotted pair syntax for directory-local variables

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 32817 in the body.
You can then email your comments to 32817 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; Dotted pair syntax for directory-local variables
Date: Mon, 24 Sep 2018 02:14:41 +0300

As indicated in https://lists.gnu.org/archive/html/emacs-devel/2018-09/msg00817.html
the current syntax is confusing.

This patch fixes add-dir-local-variable to output syntax corresponding
to examples in (info "(emacs) Directory Variables"):

diff --git a/lisp/files-x.el b/lisp/files-x.el
index 92532e85f4..8603b6299c 100644
--- a/lisp/files-x.el
+++ b/lisp/files-x.el
@@ -492,15 +492,31 @@ modify-dir-local-variable
       ;; Insert modified alist of directory-local variables.
       (insert ";;; Directory Local Variables\n")
       (insert ";;; For more information see (info \"(emacs) Directory Variables\")\n\n")
-      (pp (sort variables
+      (princ (add-dir-local-variables-to-string
+              (sort variables
 		    (lambda (a b)
 		      (cond
 		       ((null (car a)) t)
 		       ((null (car b)) nil)
 		       ((and (symbolp (car a)) (stringp (car b))) t)
 		       ((and (symbolp (car b)) (stringp (car a))) nil)
-		   (t (string< (car a) (car b))))))
-	  (current-buffer)))))
+		       (t (string< (car a) (car b)))))))
+             (current-buffer))
+      (goto-char (point-min))
+      (indent-sexp))))
+
+(defun add-dir-local-variables-to-string (variables)
+  (format "(%s)" (mapconcat
+                  (lambda (mode-variable)
+                    (format "(%S . %s)"
+                            (car mode-variable)
+                            (format "(%s)" (mapconcat
+                                            (lambda (variable-value)
+                                              (format "(%s . %S)"
+                                                      (car variable-value)
+                                                      (cdr variable-value)))
+                                            (cdr mode-variable) "\n"))))
+                  variables "\n")))
 
 ;;;###autoload
 (defun add-dir-local-variable (mode variable value)

Message #12 received at 32817 <at> debbugs.gnu.org (full text, mbox):

From: Phil Sainty <psainty <at> orcon.net.nz>
To: Juri Linkov <juri <at> linkov.net>, 32817 <at> debbugs.gnu.org
Subject: Re: bug#32817: 27.0.50; Dotted pair syntax for directory-local
 variables
Date: Wed, 26 Sep 2018 00:10:06 +1200

Hi Juri,

On 24/09/18 11:14, Juri Linkov wrote:
> As indicated in https://lists.gnu.org/archive/html/emacs-devel/2018-09/msg00817.html
> the current syntax is confusing.
> 
> This patch fixes add-dir-local-variable to output syntax corresponding
> to examples in (info "(emacs) Directory Variables"):

Thanks for implementing this.


> +(defun add-dir-local-variables-to-string (variables)

`add-dir-local-variables-to-string' is quite a confusing name.
It makes it sound like the dir-local variables will be added to
a string (which doesn't make a lot of sense), when it's actually
formatting all of the dir-locals *as* a string.

This is on account of the "add-" prefix, which I think is unnecessary
-- the functionality isn't really tied to `add-dir-local-variable';
it would work regardless of where its argument came from.

`dir-locals-to-string' seems a better name to me, and is consistent
with other dir-locals-* functions.

I think it should also have a docstring.


> +  (format "(%s)" (mapconcat
> +                  (lambda (mode-variable)

"mode-variables" (plural), I think.

> +                    (format "(%S . %s)"
> +                            (car mode-variable)
> +                            (format "(%s)" (mapconcat
> +                                            (lambda (variable-value)
> +                                              (format "(%s . %S)"
> +                                                      (car variable-value)

Why is the variable symbol "%s" when the mode symbol was "%S" ?

> +                                                      (cdr variable-value)))
> +                                            (cdr mode-variable) "\n"))))
> +                  variables "\n")))


regards,

-Phil

Message #15 received at 32817 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Phil Sainty <psainty <at> orcon.net.nz>
Cc: 32817 <at> debbugs.gnu.org
Subject: Re: bug#32817: 27.0.50;
 Dotted pair syntax for directory-local variables
Date: Tue, 25 Sep 2018 22:41:51 +0300

>> +(defun add-dir-local-variables-to-string (variables)
>
> `add-dir-local-variables-to-string' is quite a confusing name.
> It makes it sound like the dir-local variables will be added to
> a string (which doesn't make a lot of sense), when it's actually
> formatting all of the dir-locals *as* a string.
>
> This is on account of the "add-" prefix, which I think is unnecessary
> -- the functionality isn't really tied to `add-dir-local-variable';
> it would work regardless of where its argument came from.
>
> `dir-locals-to-string' seems a better name to me, and is consistent
> with other dir-locals-* functions.

My intention was to use this prefix to add the new function
to the add-dir-local-variable* namespace.  But I agree that
dir-locals-* is more suitable, so I renamed the function.

>> +  (format "(%s)" (mapconcat
>> +                  (lambda (mode-variable)
>
> "mode-variables" (plural), I think.

Fixed as well.

>> +                    (format "(%S . %s)"
>> +                            (car mode-variable)
>> +                            (format "(%s)" (mapconcat
>> +                                            (lambda (variable-value)
>> +                                              (format "(%s . %S)"
>> +                                                      (car variable-value)
>
> Why is the variable symbol "%s" when the mode symbol was "%S" ?

The mode symbol is "%S" to support string values of a subdirectory like
"src/imported" in the example in (info "(emacs) Directory Variables")
But admittedly add-dir-local-variable doesn't support adding 3 levels
subdirectory-mode-variables anyway, it could be used only to create
an initial subdirectory-variable-value to allow the user adding mode
manually.  But for any case I changed the variable symbol "%s" to "%S".

Message #18 received at 32817 <at> debbugs.gnu.org (full text, mbox):

From: Phil Sainty <psainty <at> orcon.net.nz>
To: Juri Linkov <juri <at> linkov.net>
Cc: 32817 <at> debbugs.gnu.org
Subject: Re: bug#32817: 27.0.50; Dotted pair syntax for directory-local
 variables
Date: Thu, 27 Sep 2018 03:01:41 +1200

Do we want a NEWS entry for this?

It's a user-visible change, even if the *effective* behaviour is
identical.

I'm mostly thinking it's worth flagging in NEWS for the sake of
users who have previously rejected the `add-dir-local-variable'
command on the basis that it wrecks their .dir-locals.el
formatting.


-Phil

Message #21 received at 32817 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Phil Sainty <psainty <at> orcon.net.nz>
Cc: 32817 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#32817: 27.0.50;
 Dotted pair syntax for directory-local variables
Date: Wed, 26 Sep 2018 18:50:49 +0300

> From: Phil Sainty <psainty <at> orcon.net.nz>
> Date: Thu, 27 Sep 2018 03:01:41 +1200
> Cc: 32817 <at> debbugs.gnu.org
> 
> Do we want a NEWS entry for this?

Probably.

Message #24 received at 32817 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Phil Sainty <psainty <at> orcon.net.nz>, 32817 <at> debbugs.gnu.org
Subject: Re: bug#32817: 27.0.50;
 Dotted pair syntax for directory-local variables
Date: Thu, 27 Sep 2018 03:09:25 +0300

>> Do we want a NEWS entry for this?
>
> Probably.

I added a NEWS entry like Phil suggested.

Message #27 received at 32817 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: psainty <at> orcon.net.nz, 32817 <at> debbugs.gnu.org
Subject: Re: bug#32817: 27.0.50;
 Dotted pair syntax for directory-local variables
Date: Thu, 27 Sep 2018 09:47:47 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Phil Sainty <psainty <at> orcon.net.nz>,  32817 <at> debbugs.gnu.org
> Date: Thu, 27 Sep 2018 03:09:25 +0300
> 
> >> Do we want a NEWS entry for this?
> >
> > Probably.
> 
> I added a NEWS entry like Phil suggested.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.