Re: custom-set-variables considered harmful

[Robert Weiner]

On Mon, Dec 4, 2017 at 8:48 AM, Stefan Monnier <address@hidden> 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