custom-set-variables considered harmful

custom-set-variables considered harmful

[Stefan Monnier]

I keep seeing people copy and pasting part of calls to
custom-set-variables, and getting all confused about it.

I think we should stop using it.  Here's a proposal for that.

When writing customizations, instead of writing

    (custom-set-variables
     ;; Big ugly warning which doesn't help enough.
     '(VAR1 VAL1)
     '(VAR2 VAL2 nil '(REQUEST) COMMENT)
     '(VAR3 VAL3)
     ...)

we write

    (autogenerated-custom-settings
      ;; Big warning, still, but less important.
      (setq VAR1 VAL1)
      (require 'REQUEST)
      (customize-set-variable VAR2 VAL2 COMMENT)
      (customize-set-variable VAR3 VAL3)
      ...)

where `autogenerated-custom-settings` is a macro which turns that call
back into the form expected by custom-set-variables.

The idea is basically to use a syntax which happens to look close to
what you'd write by hand if you weren't using Customize, so that users
who copy&paste snippets of code end up copying more or less valid code,
instead of copying "code" like

    '(toto 2)

and then be surprised that it doesn't do anything.

[ The difference between VAR1 and VAR3 is that VAR1 doesn't have
  a setter.  I guess in some cases we should emit `(setq-default VAR VAL)`
  also.  ]

One question, tho: could someone explain to me what the NOW is used
for, really?  I've read the docstrings, comments, and the code that
seems related to it, and I have some idea of what it supposedly does,
but I don't have a clear idea of a scenario where it's used.


        Stefan

Re: custom-set-variables considered harmful

[Philippe Vaucher]

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

> The idea is basically to use a syntax which happens to look close to
> what you'd write by hand if you weren't using Customize, so that users
> who copy&paste snippets of code end up copying more or less valid code,
> instead of copying "code" like
>
>     '(toto 2)
>
> and then be surprised that it doesn't do anything.
>

Good idea! It's also consistent with "Show saved lisp expression" from
customize buffers.

In my case, I always delete the whole custom-set-variables block and move
the relevant parts in my config, but have to transform the sexps into their
`setq` equivalent, which is annoying.

Philippe

[-- Attachment #2: Type: text/html, Size: 1017 bytes --]

Re: custom-set-variables considered harmful

[Clément Pit-Claudel]

On 2017-11-06 04:02, Philippe Vaucher wrote:
> but have to transform the sexps into their `setq` equivalent, which is annoying.

And often they're not equivalent, either, which causes plenty of issues for beginners.

Re: custom-set-variables considered harmful

[raman]

Here i a useful defmacro that helps with setting things that are
customized via custom without having to think about it:

(defmacro c-setq (variable value)
  "Exactly like setq, but handles custom."
  `(funcall (or (get ',variable 'custom-set) 'set-default) ',variable ,value))
-- 

Re: custom-set-variables considered harmful

[Stefan Monnier]

> Here i a useful defmacro that helps with setting things that are
> customized via custom without having to think about it:

> (defmacro c-setq (variable value)
>   "Exactly like setq, but handles custom."
>   `(funcall (or (get ',variable 'custom-set) 'set-default) ',variable ,value))

I think you should use customize-set-variable instead
(or make your macro use it).


        Stefan

Re: custom-set-variables considered harmful

[Vivek Dasmohapatra]

> The idea is basically to use a syntax which happens to look close to
> what you'd write by hand if you weren't using Customize, so that users
> who copy&paste snippets of code end up copying more or less valid code,
> instead of copying "code" like

Won't they then be surprised by the normal value sanitisation/hook-firing
etc that some settings rely on not happening and have even less idea
why it's not working when they inevitably cargo-cult the contents?

Re: custom-set-variables considered harmful

[Stefan Monnier]

> Won't they then be surprised by the normal value sanitisation/hook-firing
> etc that some settings rely on not happening and have even less idea
> why it's not working when they inevitably cargo-cult the contents?

Hmm... what you say rings some kind of bell, but not clearly enough:
could give a scenario to clarify the issue?


        Stefan

Re: custom-set-variables considered harmful

[Vivek Dasmohapatra]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1378 bytes --]

On Wed, 8 Nov 2017, Stefan Monnier wrote:

>> Won't they then be surprised by the normal value sanitisation/hook-firing

> Hmm... what you say rings some kind of bell, but not clearly enough:
> could give a scenario to clarify the issue?

Sure, take something like glyphless-char-display-control which says:

==========================================================================
Do not set its value directly from Lisp; the value takes effect
only via a custom ‘:set’
function (‘update-glyphless-char-display’), which updates
‘glyphless-char-display’.

You can customize this variable.
==========================================================================

Or a variable which has a restricted set of possible values, as determined
by its defcustom declaration...

In the former case (or similar cases) the user might copy the setq and then
be surprised that it does not work as expected. In the latter, they would
copy the setq, alter the value (possibly to an "illegal" value) and then
be surprised it didnt work.

Maybe instead of setq a (users-will-copy-this-even-though-we-said-not-to …)
form which _could_ be copied out of the custom-set-variables expression
independently and would still trigger the relevant custom-related hooks
and value-sanitisation (I believe someone suggested custom-set or similar
instead of setq which might well do that).

Re: custom-set-variables considered harmful

[Stefan Monnier]

I think you misunderstood my suggestion.  E.g. look at how VAR3 is
handled in my example:

     '(VAR3 VAL3)

is turned into

      (customize-set-variable VAR3 VAL3)

rather than into

      (setq VAR3 VAL3)

The idea was that VAR3 is a variable with a :setter.

> In the former case (or similar cases) the user might copy the setq and then
> be surprised that it does not work as expected.

This is already a problem since '(VAR VAL3) doesn't do what the user
intended when copied outside of the custom-set-variables block.  The new
doesn't aim to guarantee that copy&paste always works.  It just aims to
get a bit closer.

E.g. note that using customize-set-variable doesn't solve the problem
completely either (there are differences in terms of *when* the settings
is applied), but it will often work.

> In the latter, they would copy the setq, alter the value (possibly to
> an "illegal" value) and then be surprised it didnt work.

While the change I propose doesn't solve this problem, it doesn't make
it worse either, AFAICT.


        Stefan

Re: custom-set-variables considered harmful

[Vivek Dasmohapatra]

On Wed, 8 Nov 2017, Stefan Monnier wrote:

> I think you misunderstood my suggestion.  E.g. look at how VAR3 is
> handled in my example:
>
>     '(VAR3 VAL3)
>
> is turned into
>
>      (customize-set-variable VAR3 VAL3)

Ah, ok. Fair enough.

Re: custom-set-variables considered harmful

[Stefan Monnier]

> When writing customizations, instead of writing
>
>     (custom-set-variables
>      ;; Big ugly warning which doesn't help enough.
>      '(VAR1 VAL1)
>      '(VAR2 VAL2 nil '(REQUEST) COMMENT)
>      '(VAR3 VAL3)
>      ...)
>
> we write
>
>     (autogenerated-custom-settings
>       ;; Big warning, still, but less important.
>       (setq VAR1 VAL1)
>       (require 'REQUEST)
>       (customize-set-variable VAR2 VAL2 COMMENT)
>       (customize-set-variable VAR3 VAL3)
>       ...)

Here's a sample patch to do that.  Comments?


        Stefan


diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index edf3545cad..0f9b38974d 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -4374,6 +4382,16 @@ custom-file
 if only the first line of the docstring is shown."))
   :group 'customize)
 
+(defcustom custom-mimic-plain-elisp t
+  "If non-nil, save user settings with the new format.
+This new format tries to mimick the code that would be written by hand
+in plain Elisp.  But it relies on `custom-autogenerated-user-settings' which
+is a new macro in Emacs-27, so settings saved with this will either
+require a recent enough Emacs, or some ad-hoc hack such
+as (defalias 'custom-autogenerated-user-settings #'ignore)."
+  :type 'boolean
+  :group 'customize)
+
 (defun custom-file (&optional no-error)
   "Return the file name for saving customizations."
   (if (or (null user-init-file)
@@ -4505,6 +4523,7 @@ custom-save-variables
   "Save all customized variables in `custom-file'."
   (save-excursion
     (custom-save-delete 'custom-set-variables)
+    (custom-save-delete 'custom-autogenerated-user-settings)
     (let ((standard-output (current-buffer))
 	  (saved-list (make-list 1 0))
 	  sort-fold-case)
@@ -4519,8 +4538,12 @@ custom-save-variables
       (setq saved-list (sort (cdr saved-list) 'string<))
       (unless (bolp)
 	(princ "\n"))
-      (princ "(custom-set-variables
- ;; custom-set-variables was added by Custom.
+      (princ (if custom-mimic-plain-elisp
+                 "(custom-autogenerated-user-settings
+ ;; This custom-autogenerated-user-settings was added by Custom."
+               "(custom-set-variables
+ ;; This custom-set-variables was added by Custom."))
+      (princ "
  ;; If you edit it by hand, you could mess it up, so be careful.
  ;; Your init file should contain only one such instance.
  ;; If there is more than one, they won't work right.\n")
@@ -4555,28 +4578,43 @@ custom-save-variables
 	    ;; with the customize facility.
 	    (unless (bolp)
 	      (princ "\n"))
-	    (princ " '(")
-	    (prin1 symbol)
-	    (princ " ")
-	    (let ((val (prin1-to-string (car value))))
-	      (if (< (length val) 60)
-		  (insert val)
-		(newline-and-indent)
-		(let ((beginning-of-val (point)))
-		  (insert val)
-		  (save-excursion
-		    (goto-char beginning-of-val)
-		    (indent-pp-sexp 1)))))
-	    (when (or now requests comment)
+            (if custom-mimic-plain-elisp
+                ;; Do the inverse conversion of
+                ;; custom-autogenerated-user-settings.
+                (let* ((e (cond
+                           ((get symbol 'custom-set)
+                            `(customize-set-variable ',symbol ,(car value)))
+                           ((local-variable-if-set-p
+                             symbol (get-buffer-create "*scratch*"))
+                            `(setq-default ,symbol ,(car value)))
+                           (t `(setq ,symbol ,(car value))))))
+                  (dolist (e (nconc (mapcar (lambda (r) `(require ',r)) requests)
+                                    (if comment (list comment e) (list e))))
+                    (princ " ")
+                    (pp e)
+                    (unless (bolp) (princ "\n"))))
+	      (princ " '(")
+	      (prin1 symbol)
 	      (princ " ")
-	      (prin1 now)
-	      (when (or requests comment)
-		(princ " ")
-		(prin1 requests)
-		(when comment
+	      (let ((val (prin1-to-string (car value))))
+	        (if (< (length val) 60)
+		    (insert val)
+		  (newline-and-indent)
+		  (let ((beginning-of-val (point)))
+		    (insert val)
+		    (save-excursion
+		      (goto-char beginning-of-val)
+		      (indent-pp-sexp 1)))))
+	      (when (or now requests comment)
+	        (princ " ")
+	        (prin1 now)
+	        (when (or requests comment)
 		  (princ " ")
-		  (prin1 comment))))
-	    (princ ")"))))
+		  (prin1 requests)
+		  (when comment
+		    (princ " ")
+		    (prin1 comment))))
+	      (princ ")")))))
       (if (bolp)
 	  (princ " "))
       (princ ")")
@@ -4586,7 +4624,7 @@ custom-save-variables
 (defun custom-save-faces ()
   "Save all customized faces in `custom-file'."
   (save-excursion
-    (custom-save-delete 'custom-reset-faces)
+    (custom-save-delete 'custom-reset-faces) ;FIXME: never written!?
     (custom-save-delete 'custom-set-faces)
     (let ((standard-output (current-buffer))
 	  (saved-list (make-list 1 0))
@@ -4738,9 +4776,9 @@ customize-menu-create
 
 (defvar tool-bar-map)
 
-;;; `custom-tool-bar-map' used to be set up here.  This will fail to
-;;; DTRT when `display-graphic-p' returns nil during compilation.  Hence
-;;; we set this up lazily in `Custom-mode'.
+;; `custom-tool-bar-map' used to be set up here.  This will fail to
+;; DTRT when `display-graphic-p' returns nil during compilation.  Hence
+;; we set this up lazily in `Custom-mode'.
 (defvar custom-tool-bar-map nil
   "Keymap for toolbar in Custom mode.")
 
diff --git a/lisp/custom.el b/lisp/custom.el
index 352fc6bd53..f541b51420 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -1,4 +1,4 @@
-;;; custom.el --- tools for declaring and initializing options
+;;; custom.el --- tools for declaring and initializing options  -*- lexical-binding:t -*-
 ;;
 ;; Copyright (C) 1996-1997, 1999, 2001-2017 Free Software Foundation,
 ;; Inc.
@@ -926,6 +933,35 @@ custom-fix-face-spec
 	  (nreverse result))
       spec)))
 \f
+(defmacro custom-autogenerated-user-settings (&rest body)
+  "Install user customizations of variable values specified in ARGS.
+Expect the format output by `custom-save-variables'."
+  (let* ((settings '())
+         (requests '())
+         (comment nil)
+         (mk (lambda (x e)
+               (push
+                `'(,x ,e
+                      . ,(when (or comment requests)
+                           `(nil ,(prog1 (nreverse requests)
+                                    (setq requests '()))
+                                 . ,(when comment
+                                      (prog1 (list comment)
+                                        (setq comment nil))))))
+                settings))))
+    (dolist (e body)
+      (pcase e
+        (`(,(or 'setq 'setq-default) ,x ,e) (funcall mk x e))
+        (`(require ',p) (push p requests))
+        (`(customize-set-variable ',x ,e) (funcall mk x e))
+        (`(,x ,(and v (or 1 -1))) (funcall mk x (> v 0)))
+        ((pred stringp)
+         (and comment (message "Dropping extra comment: %S" comment))
+         (setq comment e))
+        (_
+         (message "Unrecognized expression in custom-autogenerated-user-settings: %S" e))))
+    `(custom-set-variables ,@(nreverse settings))))
+
 (defun custom-set-variables (&rest args)
   "Install user customizations of variable values specified in ARGS.
 These settings are registered as theme `user'.
@@ -940,7 +976,7 @@ custom-set-variables
 REQUEST is a list of features we must require in order to
 handle SYMBOL properly.
 COMMENT is a comment string about SYMBOL."
-  (apply 'custom-theme-set-variables 'user args))
+  (apply #'custom-theme-set-variables 'user args))
 
 (defun custom-theme-set-variables (theme &rest args)
   "Initialize variables for theme THEME according to settings in ARGS.
@@ -1505,7 +1541,7 @@ custom-reset-variables
     (VARIABLE IGNORED)
 
 This means reset VARIABLE.  (The argument IGNORED is ignored)."
-    (apply 'custom-theme-reset-variables 'user args))
+    (apply #'custom-theme-reset-variables 'user args))
 
 ;;; The End.
 

Re: custom-set-variables considered harmful

[Elias Mårtenson]

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

On 14 November 2017 at 00:26, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > When writing customizations, instead of writing
> >
> >     (custom-set-variables
> >      ;; Big ugly warning which doesn't help enough.
> >      '(VAR1 VAL1)
> >      '(VAR2 VAL2 nil '(REQUEST) COMMENT)
> >      '(VAR3 VAL3)
> >      ...)
> >
> > we write
> >
> >     (autogenerated-custom-settings
> >       ;; Big warning, still, but less important.
> >       (setq VAR1 VAL1)
> >       (require 'REQUEST)
> >       (customize-set-variable VAR2 VAL2 COMMENT)
> >       (customize-set-variable VAR3 VAL3)
> >       ...)
>
> Here's a sample patch to do that.  Comments?
>

Is there a particular reson you're not using ‘customize-set-variable’ for
all values?
It would be more consistent, and also avoid problems in case a package is
changed
and a setter function is added to a variable which previously did not use
it.

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 1513 bytes --]

Re: custom-set-variables considered harmful

[Stefan Monnier]

> Is there a particular reson you're not using ‘customize-set-variable’
> for all values?

Because I want the code to look as much as possible like "manually
written Elisp", and `setq` is what is used in 99% of the Elisp
customization code.

> It would be more consistent, and also avoid problems
> in case a package is changed and a setter function is added to
> a variable which previously did not use it.

Note that as long as the `setq` is within the magical
custom-autogenerated-user-settings form, it will behave correctly even
if the variable has a setter function.  The difference will only affect
those users who take this code and then copy it elsewhere.


        Stefan

Re: custom-set-variables considered harmful

[Clément Pit-Claudel]

On 2017-11-24 02:16, Stefan Monnier wrote:
>> Is there a particular reson you're not using ‘customize-set-variable’
>> for all values?
> 
> Because I want the code to look as much as possible like "manually
> written Elisp", and `setq` is what is used in 99% of the Elisp
> customization code.

I think that's a bug, though — multiple packages of mine have specific code to deal with this (users using setq instead of customize-set-variable).

>> It would be more consistent, and also avoid problems
>> in case a package is changed and a setter function is added to
>> a variable which previously did not use it.
> 
> Note that as long as the `setq` is within the magical
> custom-autogenerated-user-settings form, it will behave correctly even
> if the variable has a setter function.

I like the direction of this patch very much, but I'm not a fan of this: it seems too error-prone.  It'd be much better if copying from the special form was safe (at least in the vast majority of cases).

> The difference will only affect
> those users who take this code and then copy it elsewhere.

But isn't one of the selling points of this patch to make copying easier?

Thanks,
Clément.

Re: custom-set-variables considered harmful

[Stefan Monnier]

> I think that's a bug, though — multiple packages of mine have specific code
> to deal with this (users using setq instead of customize-set-variable).

Variables which need customize-set-variable will be printed as
(customize-set-variable 'VAR VAL) so that shouldn't be a problem.

> I like the direction of this patch very much, but I'm not a fan of this: it
> seems too error-prone.  It'd be much better if copying from the special form
> was safe (at least in the vast majority of cases).

I consider it as safe in the vast majority of cases.


        Stefan

Re: custom-set-variables considered harmful

[Elias Mårtenson]

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

On 24 Nov 2017 3:17 pm, "Stefan Monnier" <monnier@iro.umontreal.ca> wrote:

> Is there a particular reson you're not using ‘customize-set-variable’
> for all values?

Because I want the code to look as much as possible like "manually
written Elisp", and `setq` is what is used in 99% of the Elisp
customization code.


I usually try to recommend people that they always use c-s-v instead of
setq. It's easier to explain and more consistent.

The fact that a lot of people use setq isn't really a great argument for
using it in this case. The reason people use setq isn't because they want
to, but because they don't understand how custom works. I see this as a
great opportunity to make c-s-v more visible and teach people how to use
it.

Note that as long as the `setq` is within the magical
custom-autogenerated-user-settings form, it will behave correctly even
if the variable has a setter function.  The difference will only affect
those users who take this code and then copy it elsewhere.


But setq doesn't have any concrete benefit other than familiarity to people
who don't understand custom. I've spent more time than I wish on Freenode
#emacs teaching people about custom. If c-s-v was used consistently that
effort would be made a lot easier.

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 2232 bytes --]

RE: custom-set-variables considered harmful

[Drew Adams]

> The fact that a lot of people use setq isn't really
> a great argument for using it in this case. The
> reason people use setq isn't because they want to,
> but because they don't understand how custom works.
> I see this as a great opportunity to make c-s-v
> more visible and teach people how to use it. 
>
> Note that as long as the `setq` is within the
> magical custom-autogenerated-user-settings form,
> it will behave correctly even if the variable has
> a setter function.  The difference will only affect
> those users who take this code and then copy it elsewhere.
>
> But setq doesn't have any concrete benefit other than
> familiarity to people who don't understand custom.
> I've spent more time than I wish on Freenode #emacs
> teaching people about custom. If c-s-v was used
> consistently that effort would be made a lot easier. 

Couldn't have said it better myself.  +10, all of it.

Re: custom-set-variables considered harmful

[Philippe Vaucher]

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

>
>
> Note that as long as the `setq` is within the magical
> custom-autogenerated-user-settings form, it will behave correctly even
> if the variable has a setter function.  The difference will only affect
> those users who take this code and then copy it elsewhere.
>
>
> But setq doesn't have any concrete benefit other than familiarity to
> people who don't understand custom. I've spent more time than I wish on
> Freenode #emacs teaching people about custom. If c-s-v was used
> consistently that effort would be made a lot easier.
>

The problem I have with c-s-v is that I don't see its benefits for configs
like this:

(use-package ido
  :bind ("C-b" . ido-switch-buffer)
  :init
  (setq ido-enable-flex-matching t)
  (setq ido-create-new-buffer 'always)
  (setq ido-use-url-at-point nil)
  (setq ido-use-filename-at-point nil)

  ;; Disable auto searching for files unless called explicitly
  (setq ido-auto-merge-delay-time 99999)

  ;; Avoid "size too big" errors
  (setq ido-max-directory-size 300000)

  ;; Always propose old buffers as well
  (setq ido-use-virtual-buffers nil)

  ;; Don't remember history
  (setq ido-enable-last-directory-history nil)
  :config
  (ido-mode t)
  (ido-everywhere t))


Can you maybe enlighten me about what problem c-s-v solves that would have
me start to use it?

Thanks,
Philippe

[-- Attachment #2: Type: text/html, Size: 4101 bytes --]

Re: custom-set-variables considered harmful

[John Wiegley]

>>>>> "PV" == Philippe Vaucher <philippe.vaucher@gmail.com> writes:

VP> The problem I have with c-s-v is that I don't see its benefits for configs
VP> like this:

Just FYI, in the latest use-package (on github) you can now say:

  (use-package ido
    :bind ("C-b" . ido-switch-buffer)
    :custom ((ido-enable-flex-matching t)
             (ido-create-new-buffer 'always)
             (ido-use-url-at-point nil)
             (ido-use-filename-at-point nil)
  
             ;; Disable auto searching for files unless called explicitly   
             (ido-auto-merge-delay-time 99999)
  
             ;; Avoid "size too big" errors
             (ido-max-directory-size 300000)
  
             ;; Always propose old buffers as well
             (ido-use-virtual-buffers nil)
  
             ;; Don't remember history
             (ido-enable-last-directory-history nil))
    :config
    (ido-mode t)
    (ido-everywhere t))

Which uses custom-set-variables under the hood, taking advantage of any :set
functions related to those variables.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: custom-set-variables considered harmful

[Philippe Vaucher]

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

>
> VP> The problem I have with c-s-v is that I don't see its benefits for
> configs
> VP> like this:
>
> Just FYI, in the latest use-package (on github) you can now say:
>
> (snip)
>
> Which uses custom-set-variables under the hood, taking advantage of any
> :set
> functions related to those variables.
>

Ah, I agree that looks cleaner. Thanks!

Philippe

[-- Attachment #2: Type: text/html, Size: 670 bytes --]

Re: custom-set-variables considered harmful

[Elias Mårtenson]

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

On 28 November 2017 at 22:12, Philippe Vaucher <philippe.vaucher@gmail.com>
wrote:

>
>> Note that as long as the `setq` is within the magical
>> custom-autogenerated-user-settings form, it will behave correctly even
>> if the variable has a setter function.  The difference will only affect
>> those users who take this code and then copy it elsewhere.
>>
>>
>> But setq doesn't have any concrete benefit other than familiarity to
>> people who don't understand custom. I've spent more time than I wish on
>> Freenode #emacs teaching people about custom. If c-s-v was used
>> consistently that effort would be made a lot easier.
>>
>
> The problem I have with c-s-v is that I don't see its benefits for configs
> like this:
>
>
[ lines of ‘setq’ invocations on ido variables ]

Can you maybe enlighten me about what problem c-s-v solves that would have
> me start to use it?
>

Your explicit ‘setq’ calls are perfectly fine as long as the variables you
set do not have a setting function
assigned to them. However, if they do have setter functions, then the
behaviour will not be correct.
It has been suggested that you manually look at the ‘defcustom’ definitions
of the variables so that
you know which ones doesn't work with ‘setq’.

The only reason people can get away with not doing this is because there
are so few custom variables
that use setter functions.

Thus, if you use ‘setq’ instead of ‘custom-set-variable’ you need to
manually check every single
variable to ensure they don't have setter functions assigned. If you want
to be thorough (nobody is)
you also need to check them every time your modules are updated. This is
not just a theoretical
situation. It happened in gnu-apl-mode, a library for which I am the
maintainer. In it, the
variable ‘gnu-apl-mode-map-prefix’ got a setter function after the fact.

Or, you could simply use ‘custom-set-variable’ everywhere and spare
yourself the trouble.

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 3702 bytes --]

Re: custom-set-variables considered harmful

[Stefan Monnier]

> Thus, if you use ‘setq’ instead of ‘custom-set-variable’ you need to
> manually check every single variable to ensure they don't have setter
> functions assigned.  If you want to be thorough (nobody is) you also
> need to check them every time your modules are updated.  This is not
> just a theoretical situation.

Note that there are also many situations where the var has a setter, yet
doing (setq foo val) on it from your ~/.emacs will still work correctly
(as long as you do it before loading the library).
I'd argue it's actually the most frequent case.

> It happened in gnu-apl-mode, a library for which I am the maintainer.
> In it, the variable ‘gnu-apl-mode-map-prefix’ got a setter function
> after the fact.

Looking at gnu-apl-mode.el, I get the impression that this is no
exception:

    (setq gnu-apl-mode-map-prefix "C-")
    (require 'gnu-apl-mode)

will result in the exact same state as

    (customize-set-variable 'gnu-apl-mode-map-prefix "C-")
    (require 'gnu-apl-mode)

after gnu-apl-mode is loaded, OTOH you indeed need to use
customize-set-variable because a setq would have no real effect.


        Stefan

RE: custom-set-variables considered harmful

[Drew Adams]

> > Thus, if you use ‘setq’ instead of ‘custom-set-variable’ you need to
> > manually check every single variable to ensure they don't have setter
> > functions assigned.  If you want to be thorough (nobody is) you also
> > need to check them every time your modules are updated.  This is not
> > just a theoretical situation.
> 
> Note that there are also many situations where the var has a setter, yet
> doing (setq foo val) on it from your ~/.emacs will still work correctly
> (as long as you do it before loading the library).
> I'd argue it's actually the most frequent case.
> 
> > It happened in gnu-apl-mode, a library for which I am the maintainer.
> > In it, the variable ‘gnu-apl-mode-map-prefix’ got a setter function
> > after the fact.
> 
> Looking at gnu-apl-mode.el, I get the impression that this is no
> exception:
> 
>     (setq gnu-apl-mode-map-prefix "C-")
>     (require 'gnu-apl-mode)
> 
> will result in the exact same state as
> 
>     (customize-set-variable 'gnu-apl-mode-map-prefix "C-")
>     (require 'gnu-apl-mode)
> 
> after gnu-apl-mode is loaded, OTOH you indeed need to use
> customize-set-variable because a setq would have no real effect.

It may be the case that many uses of `setq' amount to the
same thing as using `c-s-var*'.  But I'm not sure what
that changes.

Here's a crazy possibility (not a suggestion, but maybe
food for thought): Make `setq' do `customize-set-variable'
for an option and do what it does now otherwise.

(Ducks for cover...)

Re: custom-set-variables considered harmful

[Robert Weiner]

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

On Wed, Nov 29, 2017 at 10:03 AM, Drew Adams <drew.adams@oracle.com> wrote:

>
> Here's a crazy possibility (not a suggestion, but maybe
> food for thought): Make `setq' do `customize-set-variable'
> for an option and do what it does now otherwise.
>

​I have been wondering about this too.  Are there any arguments for not
having setq run the setter of a custom option when available?  Is it a
performance issue?
Is it a load ordering issue?

If there is no fundamental issue, then this would be great because one
wouldn't have to worry about the type of a variable when setting it.

Bob

[-- Attachment #2: Type: text/html, Size: 1653 bytes --]

Re: custom-set-variables considered harmful

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > ​I have been wondering about this too.  Are there any arguments for not
  > having setq run the setter of a custom option when available?

That change would create a risk of bugs in many parts of the code.
Each differnt kind of setter function creates a different risk of
possible errors.

That change would mean that making an ordinary let binding could run
arbitrary Lisp code.  Unwinding the binding could also run arbitrary
Lisp code.

Finally, it could cause errors in places that set the variable and
expect that to do nothing but put a new value in it.  This includes
the definitions of setter functions, right?

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Skype: No way! See https://stallman.org/skype.html.

RE: custom-set-variables considered harmful

[Drew Adams]

>> ​I have been wondering about this too.  Are there any arguments for
>> not having setq run the setter of a custom option when available?
> 
> That change would create a risk of bugs in many parts of the code.
> Each differnt kind of setter function creates a different risk of
> possible errors.
> 
> That change would mean that making an ordinary let binding could run
> arbitrary Lisp code.  Unwinding the binding could also run arbitrary
> Lisp code.
> 
> Finally, it could cause errors in places that set the variable and
> expect that to do nothing but put a new value in it.  This includes
> the definitions of setter functions, right?

As I said, I don't argue for doing this.

And yes, there would need to also be a "basic setq",
which has the current behavior, to be able to use in
custom functions (e.g. setters) and some other places.
I figured that much was obvious.

The other considerations you mention are less clear to me.

I suppose another possibility might be to come up with
another function, with a short name, that does what
we're talking about: does `customize-set-variable' for
an option (except in `custom*' code) and does `setq'
otherwise.  And then encourage people to use that new
function instead of `setq' most places.  I kinda doubt
that would help much, though.

But again, I'm not proposing any of this.  I mentioned
it only as food for thought.

The problem to solve (which is admittedly a small one,
since as Stefan pointed out most options don't use
special :set functions) is that users seem to often
use `setq' for options without even being aware of
`customize-set-variable' or why it might be needed in
some cases.

Re: custom-set-variables considered harmful

[Teemu Likonen]

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

Drew Adams [2017-11-29 07:03:39-08] wrote:

> Here's a crazy possibility (not a suggestion, but maybe food for
> thought): Make `setq' do `customize-set-variable' for an option and do
> what it does now otherwise.

That would be a job for setf, I think, because it is generalized
assignment macro. Common Lisp's setf works with setter functions and in
the same spirit it could work with Emacs's custom setters.

-- 
/// Teemu Likonen   - .-..   <https://keybase.io/tlikonen> //
// PGP: 4E10 55DC 84E9 DFF6 13D7 8557 719D 69D3 2453 9450 ///

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: custom-set-variables considered harmful

[Elias Mårtenson]

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

On 29 November 2017 at 23:00, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > Thus, if you use ‘setq’ instead of ‘custom-set-variable’ you need to
> > manually check every single variable to ensure they don't have setter
> > functions assigned.  If you want to be thorough (nobody is) you also
> > need to check them every time your modules are updated.  This is not
> > just a theoretical situation.
>
> Note that there are also many situations where the var has a setter, yet
> doing (setq foo val) on it from your ~/.emacs will still work correctly
> (as long as you do it before loading the library).
> I'd argue it's actually the most frequent case.
>

It is. But why does that make a difference? There are plenty of other
things that
works almost all the time, even though it's not recommended.

All I'm trying to say is that we currently have two methods for setting
custom
variables:

  - setq - works almost all the time
  - custom-set-variable - works all the time

I'm not saying that anyone should make setq not work. I'm simply suggesting
that
perhaps it's a good idea to recommend users that they use the one, simple,
mechanism that exists and works all the time.

Wouldn't that make things simpler in the long run?

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 1861 bytes --]

Re: custom-set-variables considered harmful

[Scott Randby]

On 11/29/2017 11:35 AM, Elias Mårtenson wrote:> 
> All I'm trying to say is that we currently have two methods for setting custom
> variables:
> 
>   - setq - works almost all the time
>   - custom-set-variable - works all the time
> 
> I'm not saying that anyone should make setq not work. I'm simply suggesting that
> perhaps it's a good idea to recommend users that they use the one, simple,
> mechanism that exists and works all the time.
> 
> Wouldn't that make things simpler in the long run?

My question about this is what do we do if our initialization file is broken into pieces? I use an org-mode file which I tangle to an initialization file, and I have setq in various pieces of code all over the place in the org-mode file. My understanding is that you should only use one custom-set-variables or else bad things could happen. I do not want to abandon my org-mode file. How do I keep modularization and use custom-set-variables?

Scott Randby

Re: custom-set-variables considered harmful

[Stefan Monnier]

> custom-set-variables or else bad things could happen. I do not want to
> abandon my org-mode file. How do I keep modularization and use
> custom-set-variables?

That's a completely different matter.  We're here arguing between `setq`
and `customize-set-variable`.  Both of those apply to a *single*
var anyway: do not confuse customize-set-variable an
custom-set-variables.


        Stefan

Re: custom-set-variables considered harmful

[Scott Randby]

On 11/29/2017 05:08 PM, Stefan Monnier wrote:
> That's a completely different matter.  We're here arguing between `setq`
> and `customize-set-variable`.  Both of those apply to a *single*
> var anyway: do not confuse customize-set-variable an
> custom-set-variables.

Okay, thanks for the clarification. I think I misunderstood earlier messages in this thread.

Scott 

Re: custom-set-variables considered harmful

[Philippe Vaucher]

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

> Can you maybe enlighten me about what problem c-s-v solves that would have
>> me start to use it?
>>
>
> Your explicit ‘setq’ calls are perfectly fine as long as the variables you
> set do not have a setting function
> assigned to them. However, if they do have setter functions, then the
> behaviour will not be correct.
> It has been suggested that you manually look at the ‘defcustom’
> definitions of the variables so that
> you know which ones doesn't work with ‘setq’.
>
> The only reason people can get away with not doing this is because there
> are so few custom variables
> that use setter functions.
>

Ah, well in my case it was just ignorance that setter functions can exist.
That and the fact that "it just works" 99% of the time with most variables
and if you `setq` the variable prior to loading the package makes it hard
to notice there's a problem.

I'll try to migrate my config toward use-package `:custom` directive.

Philippe

[-- Attachment #2: Type: text/html, Size: 1648 bytes --]

Re: custom-set-variables considered harmful

[Stefan Monnier]

> Here's a sample patch to do that.  Comments?

Based on the feedback, I added a config var to control the use of `setq'.
I also added a backward compatibility hack which makes it so that your
"new style config" will partly work in Emacs<27.

Any objection to installing the patch below into `master`?


        Stefan


diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index 4965adfd56..d7166294c2 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -4374,6 +4382,23 @@ custom-file
 if only the first line of the docstring is shown."))
   :group 'customize)
 
+(defcustom custom-mimic-plain-elisp nil
+  "If non-nil, save user settings with the new format.
+This new format tries to mimick the code that would be written by hand
+in plain Elisp.  But it relies on `custom-autogenerated-user-settings' which
+is a new macro in Emacs-27, so settings saved with this will not work
+reliably in Emacs<27 (although a backward compatibility trick is used
+which should make them work to some extent)."
+  :type 'boolean
+  :group 'customize)
+
+(defcustom custom-mimic-plain-elisp-use-setq nil
+  "If non-nil, use `setq' when possible in generated code.
+If nil, `custom-mimic-plain-elisp' will only use `customize-set-variable',
+which is the more reliable option."
+  :type 'boolean
+  :group 'customize)
+
 (defun custom-file (&optional no-error)
   "Return the file name for saving customizations."
   (if (or (null user-init-file)
@@ -4501,10 +4526,32 @@ custom-save-delete
 
 (defvar sort-fold-case) ; defined in sort.el
 
+(defun custom--mimic-plain-elisp (symbol exp requests comment)
+  "Print the code corresponding to setting SYMBOL to EXP."
+  ;; Do the inverse conversion of custom-autogenerated-user-settings.
+  (let* ((e (cond
+             ((eq (get symbol 'custom-set) #'custom-set-minor-mode)
+              `(,symbol ,(if (memq exp '(t nil))
+                             (if exp 1 -1)
+                           '(if ,exp 1 -1))))
+             ((or (not custom-mimic-plain-elisp-use-setq)
+                  (get symbol 'custom-set))
+              `(customize-set-variable ',symbol ,exp))
+             ((local-variable-if-set-p
+               symbol (get-buffer-create "*scratch*"))
+              `(setq-default ,symbol ,exp))
+             (t `(setq ,symbol ,exp)))))
+    (dolist (e (nconc (mapcar (lambda (r) `(require ',r)) requests)
+                      (if comment (list comment e) (list e))))
+      (princ " ")
+      (pp e)
+      (unless (bolp) (princ "\n")))))
+
 (defun custom-save-variables ()
   "Save all customized variables in `custom-file'."
   (save-excursion
     (custom-save-delete 'custom-set-variables)
+    (custom-save-delete 'custom-autogenerated-user-settings)
     (let ((standard-output (current-buffer))
 	  (saved-list (make-list 1 0))
 	  sort-fold-case)
@@ -4519,8 +4566,21 @@ custom-save-variables
       (setq saved-list (sort (cdr saved-list) 'string<))
       (unless (bolp)
 	(princ "\n"))
-      (princ "(custom-set-variables
- ;; custom-set-variables was added by Custom.
+      (when (and custom-mimic-plain-elisp
+                 (not (save-excursion
+                        (re-search-backward
+                         "custom-autogenerated-user-settings" nil t))))
+        (princ ";; Backward compatibility definition for Emacs<27")
+        (princ ";; auto-added by `custom-save-variables'.\n")
+        (pp '(unless (fboundp 'custom-autogenerated-user-settings)
+               (defalias 'custom-autogenerated-user-settings 'progn)))
+        (unless (bolp) (princ "\n")))
+      (princ (if custom-mimic-plain-elisp
+                 "(custom-autogenerated-user-settings
+ ;; This custom-autogenerated-user-settings was added by Custom."
+               "(custom-set-variables
+ ;; This custom-set-variables was added by Custom."))
+      (princ "
  ;; If you edit it by hand, you could mess it up, so be careful.
  ;; Your init file should contain only one such instance.
  ;; If there is more than one, they won't work right.\n")
@@ -4555,6 +4555,8 @@
 	    ;; with the customize facility.
 	    (unless (bolp)
 	      (princ "\n"))
+            (if custom-mimic-plain-elisp
+                (custom--mimic-plain-elisp symbol (car value) requests comment)
 	    (princ " '(")
 	    (prin1 symbol)
 	    (princ " ")
@@ -4576,7 +4578,7 @@
 		(when comment
 		  (princ " ")
 		  (prin1 comment))))
-	    (princ ")"))))
+	      (princ ")")))))
       (if (bolp)
 	  (princ " "))
       (princ ")")
diff --git a/lisp/custom.el b/lisp/custom.el
index 352fc6bd53..b641740266 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -1,4 +1,4 @@
-;;; custom.el --- tools for declaring and initializing options
+;;; custom.el --- tools for declaring and initializing options  -*- lexical-binding:t -*-
 ;;
 ;; Copyright (C) 1996-1997, 1999, 2001-2017 Free Software Foundation,
 ;; Inc.
@@ -926,6 +933,36 @@ custom-fix-face-spec
 	  (nreverse result))
       spec)))
 \f
+(defmacro custom-autogenerated-user-settings (&rest body)
+  "Install user customizations of variable values specified in ARGS.
+Expect the format output by `custom-save-variables'."
+  (let* ((settings '())
+         (requests '())
+         (comment nil)
+         (mk (lambda (x e)
+               (push
+                `'(,x ,e
+                      . ,(when (or comment requests)
+                           `(nil ,(prog1 (nreverse requests)
+                                    (setq requests '()))
+                                 . ,(when comment
+                                      (prog1 (list comment)
+                                        (setq comment nil))))))
+                settings))))
+    (dolist (e body)
+      (pcase e
+        (`(,(or 'setq 'setq-default) ,x ,e) (funcall mk x e))
+        (`(require ',p) (push p requests))
+        (`(customize-set-variable ',x ,e) (funcall mk x e))
+        (`(,x ,(and v (or 1 -1))) (funcall mk x (> v 0)))
+        (`(,x (if ,e 1 -1)) (funcall mk x e))
+        ((pred stringp)
+         (and comment (message "Dropping extra comment: %S" comment))
+         (setq comment e))
+        (_
+         (message "Unrecognized expression in custom-autogenerated-user-settings: %S" e))))
+    `(custom-set-variables ,@(nreverse settings))))
+
 (defun custom-set-variables (&rest args)
   "Install user customizations of variable values specified in ARGS.
 These settings are registered as theme `user'.

Re: custom-set-variables considered harmful

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 04 Dec 2017 08:48:06 -0500
> 
> Based on the feedback, I added a config var to control the use of `setq'.
> I also added a backward compatibility hack which makes it so that your
> "new style config" will partly work in Emacs<27.

Thanks.

> Any objection to installing the patch below into `master`?

No objections from me, but:

 . please add :version tags to defcustoms
 . please add some documentation, including a NEWS entry

A couple of nits about doc strings below.

> +(defcustom custom-mimic-plain-elisp nil
> +  "If non-nil, save user settings with the new format.

IME, saying "new" in documentation is not a very good idea: it conveys
no useful information, and after some time it is no longer "new".  How
about something like the below?

  If non-nil, save user settings using format that mimics hand-written code.

Then you could lose the next sentence of the doc string:

> +This new format tries to mimick the code that would be written by hand
> +in plain Elisp.

> +                 But it relies on `custom-autogenerated-user-settings' which
> +is a new macro in Emacs-27, so settings saved with this will not work
> +reliably in Emacs<27 (although a backward compatibility trick is used
> +which should make them work to some extent)."

This sounds like a comment, not sure if it will be useful to users.

> +(defcustom custom-mimic-plain-elisp-use-setq nil
> +  "If non-nil, use `setq' when possible in generated code.

I'd suggest "in code generated by `custom-mimic-plain-elisp'".

> +(defmacro custom-autogenerated-user-settings (&rest body)
> +  "Install user customizations of variable values specified in ARGS.

Ehm... which "ARGS" are those?  There's just BODY.

Re: custom-set-variables considered harmful

[Robert Weiner]

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

On Mon, Dec 4, 2017 at 8:48 AM, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> Any objection to installing the patch below into `master`?
>
>         Stefan
>
>
> +(defcustom custom-mimic-plain-elisp nil

+  "If non-nil, save user settings with the new format.
> +This new format tries to mimick the code that would be written by hand
> +in plain Elisp.  But it relies on `custom-autogenerated-user-settings'
> which
> +is a new macro in Emacs-27, so settings saved with this will not work
> +reliably in Emacs<27 (although a backward compatibility trick is used
> +which should make them work to some extent)."
>


I think there must be a better way to name what you are doing here
and to explain it in the doc string.  Generally, a reader won't
know what 'new format' or 'plain Elisp' mean.  See if you can
define and clarify them.  Also, the mimic term just feels unfamiliar
vis-a-vis the rest of the Emacs code base.


> +  :type 'boolean
> +  :group 'customize)
>

​Don't we typically add '-flag​' for booleans nowadays?

​​
> +
> ​
>
> +
> ​​
> (defcustom custom-mimic-plain-elisp-use-setq nil
> +
> ​​
>   "If non-nil, use `setq' when possible in generated code.
> +
> ​​
> If nil, `custom-mimic-plain-elisp' will only use `customize-set-variable',
> ​​
> +
> ​​
> which is the more reliable option."
>

​Here the documentation is much clearer but the naming could still
be improved.  What about, `custom-use-setq-flag'?  And clarify
the contexts where this is used, does it apply to the let bindings
that RMS mentioned?


> +(defun custom--mimic-plain-elisp (symbol exp requests comment)
>

​Since this prints things, its name should mention that.
How about, 'custom--print-option-setter'?
​
Thanks for developing this and all your great work.

Bob

[-- Attachment #2: Type: text/html, Size: 5236 bytes --]

Re: custom-set-variables considered harmful

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Based on the feedback, I added a config var to control the use of `setq'.
  > I also added a backward compatibility hack which makes it so that your
  > "new style config" will partly work in Emacs<27.

Could you please summarize what this will do?

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Skype: No way! See https://stallman.org/skype.html.

Re: custom-set-variables considered harmful

[Stefan Monnier]

>> Based on the feedback, I added a config var to control the use of `setq'.
>> I also added a backward compatibility hack which makes it so that your
>> "new style config" will partly work in Emacs<27.

> Could you please summarize what this will do?

If you refer to the compatibility hack, it just defines
custom-autogenerated-user-settings as a alias for `progn', so it will
usually set the config vars just fine but will fail to mark them as set
by Custom.  Occasionally it will also fail to set the vars properly, of
course (e.g. because they're not set in the right order), but it should
be a fairly rare occurrence.


        Stefan

Re: custom-set-variables considered harmful

[John Wiegley]

>>>>> "SM" == Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

SM> The idea is basically to use a syntax which happens to look close to what
SM> you'd write by hand if you weren't using Customize, so that users who
SM> copy&paste snippets of code end up copying more or less valid code,
SM> instead of copying "code" like

SM>     '(toot 2)

SM> and then be surprised that it doesn't do anything.

I like the idea. It turns the body of the customization block into a different
DSL than "oddly specialized list" so pieces of it can be safely excerpted. I
assume the ordering becomes important, so the REQUIRE statement only affects
the immediately following setting?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2