Subject: Re: bug#32817: 27.0.50; Dotted pair syntax for directory-local variables

From: Phil Sainty <[email-masked]>
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