From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.help Subject: Re: Question about defcustom and load-history Date: Mon, 06 May 2019 12:37:39 -0400 Message-ID: References: <87r2anzfbk.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="41772"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) To: help-gnu-emacs@gnu.org Original-X-From: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Mon May 06 18:38:22 2019 Return-path: Envelope-to: geh-help-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hNgd9-000Ak0-Dh for geh-help-gnu-emacs@m.gmane.org; Mon, 06 May 2019 18:38:19 +0200 Original-Received: from localhost ([127.0.0.1]:59228 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hNgd8-00088r-BX for geh-help-gnu-emacs@m.gmane.org; Mon, 06 May 2019 12:38:18 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:51869) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hNgcj-00086m-Dk for help-gnu-emacs@gnu.org; Mon, 06 May 2019 12:37:55 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hNgch-00036x-Uj for help-gnu-emacs@gnu.org; Mon, 06 May 2019 12:37:53 -0400 Original-Received: from [195.159.176.226] (port=57310 helo=blaine.gmane.org) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hNgch-00035Q-Mt for help-gnu-emacs@gnu.org; Mon, 06 May 2019 12:37:51 -0400 Original-Received: from list by blaine.gmane.org with local (Exim 4.89) (envelope-from ) id 1hNgcd-000A9E-VK for help-gnu-emacs@gnu.org; Mon, 06 May 2019 18:37:47 +0200 X-Injected-Via-Gmane: http://gmane.org/ Cancel-Lock: sha1:deRPZ1u9VDyALU7OGkuHl3S7p8g= X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 195.159.176.226 X-BeenThere: help-gnu-emacs@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Users list for the GNU Emacs text editor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "help-gnu-emacs" Xref: news.gmane.org gmane.emacs.help:120231 Archived-At: Mauro Aranda writes: >>> In the falsy case, i.e., when the variable has an initial default value, >>> a `defvar' form is evaled resulting in one entry, and then the symbol is >>> pushed again to `current-load-list' from where it probably gets to >>> `load-history'. >> >> Indeed the manual handling of current-load-list should likely be moved >> to the true branch of the `if`. > > Hello Tassilo and Stefan, > > Thank you both for your answers. > > After reading Tassilo explanation, I went on to have a look at > 'custom.el' and found out that if a custom variable has > 'custom-initialize-default' as the :initialize function, a 'defvar' is > evaled too. So, those custom variables appear thrice in 'load-history'. > > For example, loading 'ansi-color', you'll see > 'ansi-color-faces-vector' and 'ansi-color-names-vector' triplicated. > > This doesn't look intentional to me. Should I file a bug report? I haven't seen a corresponding bug-report, so I'll reply here. I installed the patch below into `master` which should fix this problem for good. Stefan * lisp/custom.el: Avoid adding vars to load-history multiple times Avoid the abuse of (eval `(defvar ...)) which tends to end up adding redundant entries in `load-history`, as discussed in https://lists.gnu.org/r/help-gnu-emacs/2019-03/msg00237.html (custom-initialize-default): Don't add to load-history. (custom-declare-variable): Use internal--define-uninitialized-variable and only add the var to load-history once. Do it before calling `initialize` so the special-variable-p flag is set. * src/eval.c (Finternal__define_uninitialized_variable): New function. (Fdefvar, Fdefconst): Use it. (syms_of_eval): Defsubr' it. diff --git a/lisp/custom.el b/lisp/custom.el index 53b8045f05..29bf9e570a 100644 --- a/lisp/custom.el +++ b/lisp/custom.el @@ -56,8 +56,14 @@ custom-initialize-default the car of that and use it as the default binding for symbol. Otherwise, EXP will be evaluated and used as the default binding for symbol." - (eval `(defvar ,symbol ,(let ((sv (get symbol 'saved-value))) - (if sv (car sv) exp))))) + (condition-case nil + (default-toplevel-value symbol) ;Test presence of default value. + (void-variable + ;; The var is not initialized yet. + (set-default-toplevel-value + symbol (eval (let ((sv (get symbol 'saved-value))) + (if sv (car sv) exp)) + t))))) (defun custom-initialize-set (symbol exp) "Initialize SYMBOL based on EXP. @@ -188,18 +194,13 @@ custom-declare-variable (t (custom-handle-keyword symbol keyword value 'custom-variable)))))) + ;; Set the docstring, record the var on load-history, as well + ;; as set the special-variable-p flag. + (internal--define-uninitialized-variable symbol doc) (put symbol 'custom-requests requests) ;; Do the actual initialization. (unless custom-dont-initialize (funcall initialize symbol default))) - ;; Use defvar to set the docstring as well as the special-variable-p flag. - ;; FIXME: We should reproduce more of `defvar's behavior, such as the warning - ;; when the var is currently let-bound. - (if (not (default-boundp symbol)) - ;; Don't use defvar to avoid setting a default-value when undesired. - (when doc (put symbol 'variable-documentation doc)) - (eval `(defvar ,symbol nil ,@(when doc (list doc))))) - (push symbol current-load-list) (run-hooks 'custom-define-hook) symbol) diff --git a/src/eval.c b/src/eval.c index 3fd9a40a3a..567c32e0d7 100644 --- a/src/eval.c +++ b/src/eval.c @@ -715,6 +715,25 @@ DEFUN ("set-default-toplevel-value", Fset_default_toplevel_value, return Qnil; } +DEFUN ("internal--define-uninitialized-variable", + Finternal__define_uninitialized_variable, + Sinternal__define_uninitialized_variable, 1, 2, 0, + doc: /* Define SYMBOL as a variable, with DOC as its docstring. +This is like `defvar' and `defconst' but without affecting the variable's +value. */) + (Lisp_Object symbol, Lisp_Object doc) +{ + XSYMBOL (symbol)->u.s.declared_special = true; + if (!NILP (doc)) + { + if (!NILP (Vpurify_flag)) + doc = Fpurecopy (doc); + Fput (symbol, Qvariable_documentation, doc); + } + LOADHIST_ATTACH (symbol); + return Qnil; +} + DEFUN ("defvar", Fdefvar, Sdefvar, 1, UNEVALLED, 0, doc: /* Define SYMBOL as a variable, and return SYMBOL. You are not required to define a variable in order to use it, but @@ -754,32 +773,25 @@ usage: (defvar SYMBOL &optional INITVALUE DOCSTRING) */) { if (!NILP (XCDR (tail)) && !NILP (XCDR (XCDR (tail)))) error ("Too many arguments"); + Lisp_Object exp = XCAR (tail); tem = Fdefault_boundp (sym); + tail = XCDR (tail); /* Do it before evaluating the initial value, for self-references. */ - XSYMBOL (sym)->u.s.declared_special = true; + Finternal__define_uninitialized_variable (sym, CAR (tail)); if (NILP (tem)) - Fset_default (sym, eval_sub (XCAR (tail))); + Fset_default (sym, eval_sub (exp)); else { /* Check if there is really a global binding rather than just a let binding that shadows the global unboundness of the var. */ union specbinding *binding = default_toplevel_binding (sym); if (binding && EQ (specpdl_old_value (binding), Qunbound)) { - set_specpdl_old_value (binding, eval_sub (XCAR (tail))); + set_specpdl_old_value (binding, eval_sub (exp)); } } - tail = XCDR (tail); - tem = Fcar (tail); - if (!NILP (tem)) - { - if (!NILP (Vpurify_flag)) - tem = Fpurecopy (tem); - Fput (sym, Qvariable_documentation, tem); - } - LOADHIST_ATTACH (sym); } else if (!NILP (Vinternal_interpreter_environment) && (SYMBOLP (sym) && !XSYMBOL (sym)->u.s.declared_special)) @@ -827,19 +839,12 @@ usage: (defconst SYMBOL INITVALUE [DOCSTRING]) */) docstring = XCAR (XCDR (XCDR (args))); } + Finternal__define_uninitialized_variable (sym, docstring); tem = eval_sub (XCAR (XCDR (args))); if (!NILP (Vpurify_flag)) tem = Fpurecopy (tem); - Fset_default (sym, tem); - XSYMBOL (sym)->u.s.declared_special = true; - if (!NILP (docstring)) - { - if (!NILP (Vpurify_flag)) - docstring = Fpurecopy (docstring); - Fput (sym, Qvariable_documentation, docstring); - } - Fput (sym, Qrisky_local_variable, Qt); - LOADHIST_ATTACH (sym); + Fset_default (sym, tem); /* FIXME: set-default-toplevel-value? */ + Fput (sym, Qrisky_local_variable, Qt); /* FIXME: Why? */ return sym; } @@ -4198,6 +4203,7 @@ alist of active lexical bindings. */); defsubr (&Sdefvaralias); DEFSYM (Qdefvaralias, "defvaralias"); defsubr (&Sdefconst); + defsubr (&Sinternal__define_uninitialized_variable); defsubr (&Smake_var_non_special); defsubr (&Slet); defsubr (&SletX);