GNU bug report logs - #36539
elec-pair.elc gets surreptitiously loaded (almost) unconditionally at start up.

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 36539 in the body.
You can then email your comments to 36539 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: Alan Mackenzie <acm <at> muc.de>
To: bug-gnu-emacs <at> gnu.org
Subject: elec-pair.elc gets surreptitiously loaded (almost) unconditionally
 at start up.
Date: Sun, 7 Jul 2019 14:14:14 +0000

Hello, Emacs.

Current master branch, emacs -Q:

elec-pair.elc has already been loaded at startup.  It seems this is not
intended - elec-pair.el is not loaded in loadup.el.

The reason for this (almost) unconditional loading seems to be a
`require' form in the `defined-derive-mode' for emacs-lisp-mode in
.../progmodes/elisp-mode.el.

This is not good.

A decision should be taken as to whether elec-pair.elc should be loaded
at start up, and if so, it should be part of the dump.  If not, the
`require' form in elisp-mode.el should be replaced by an
`eval-after-load' form.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sun, 07 Jul 2019 12:57:16 -0400

[Message part 1 (text/plain, inline)]

tags 36539 + patch
quit

Alan Mackenzie <acm <at> muc.de> writes:

> A decision should be taken as to whether elec-pair.elc should be loaded
> at start up, and if so, it should be part of the dump.  If not, the
> `require' form in elisp-mode.el should be replaced by an
> `eval-after-load' form.

I think it's clear that elec-pair should not be part of the dump, since
electric-pair-mode is not turned on by default.  Looking at the git log,
it seems I added the require to avoid some bootstrapping problems with
Bug#24901, but we can move the electric-pair-text-pairs manipulation to
a hook instead:

[0001-Don-t-load-elec-pair-in-elisp-mode-Bug-36539.patch (text/x-diff, inline)]

From cf6057cb5bf6a44d718349776e370b225bc7079f Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 7 Jul 2019 12:22:37 -0400
Subject: [PATCH] Don't load elec-pair in elisp-mode (Bug#36539)

* lisp/progmodes/elisp-mode.el (emacs-lisp-set-electric-text-pairs):
New function.
(emacs-lisp-mode): Add it to electric-pair-mode-hook.
---
 lisp/progmodes/elisp-mode.el | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index c86277a309..7c77c34b50 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -233,6 +233,12 @@ emacs-lisp-mode-hook
   :type 'hook
   :group 'lisp)
 
+(defun emacs-lisp-set-electric-text-pairs ()
+  (defvar electric-pair-text-pairs)
+  (setq-local electric-pair-text-pairs
+              (append '((?\` . ?\') (?‘ . ?’)) electric-pair-text-pairs))
+  (remove-hook 'electric-pair-mode-hook #'emacs-lisp-set-electric-text-pairs t))
+
 ;;;###autoload
 (define-derived-mode emacs-lisp-mode prog-mode "Emacs-Lisp"
   "Major mode for editing Lisp code to run in Emacs.
@@ -245,12 +251,8 @@ emacs-lisp-mode
   (defvar project-vc-external-roots-function)
   (lisp-mode-variables nil nil 'elisp)
   (add-hook 'after-load-functions #'elisp--font-lock-flush-elisp-buffers)
-  (unless noninteractive
-    (require 'elec-pair)
-    (defvar electric-pair-text-pairs)
-    (setq-local electric-pair-text-pairs
-                (append '((?\` . ?\') (?‘ . ?’)) electric-pair-text-pairs))
-    (setq-local electric-quote-string t))
+  (add-hook 'electric-pair-mode-hook #'emacs-lisp-set-electric-text-pairs t)
+  (setq-local electric-quote-string t)
   (setq imenu-case-fold-search nil)
   (add-function :before-until (local 'eldoc-documentation-function)
                 #'elisp-eldoc-documentation-function)
-- 
2.11.0

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

From: Alan Mackenzie <acm <at> muc.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sun, 7 Jul 2019 18:29:41 +0000

Hello, Noam.

On Sun, Jul 07, 2019 at 12:57:16 -0400, Noam Postavsky wrote:
> tags 36539 + patch
> quit

> Alan Mackenzie <acm <at> muc.de> writes:

> > A decision should be taken as to whether elec-pair.elc should be loaded
> > at start up, and if so, it should be part of the dump.  If not, the
> > `require' form in elisp-mode.el should be replaced by an
> > `eval-after-load' form.

> I think it's clear that elec-pair should not be part of the dump, since
> electric-pair-mode is not turned on by default.  Looking at the git log,
> it seems I added the require to avoid some bootstrapping problems with
> Bug#24901, but we can move the electric-pair-text-pairs manipulation to
> a hook instead:

Just as a matter of interest, I commented out the pertinent form in
emacs-lisp-mode, did a make bootstrap, emacs -Q, and elec-pair.elc was
still present in my Emacs.  :-(

I don't know why, or how, but it is.  With your patch (below), have you
tested whether or not elec-pair.elc hasn't been loaded on starting
Emacs?  If it hasn't been loaded, you've found some trick that eludes
me.

> >>From cf6057cb5bf6a44d718349776e370b225bc7079f Mon Sep 17 00:00:00 2001
> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Sun, 7 Jul 2019 12:22:37 -0400
> Subject: [PATCH] Don't load elec-pair in elisp-mode (Bug#36539)
> 
> * lisp/progmodes/elisp-mode.el (emacs-lisp-set-electric-text-pairs):
> New function.
> (emacs-lisp-mode): Add it to electric-pair-mode-hook.
> ---
>  lisp/progmodes/elisp-mode.el | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
> index c86277a309..7c77c34b50 100644
> --- a/lisp/progmodes/elisp-mode.el
> +++ b/lisp/progmodes/elisp-mode.el
> @@ -233,6 +233,12 @@ emacs-lisp-mode-hook
>    :type 'hook
>    :group 'lisp)
>  
> +(defun emacs-lisp-set-electric-text-pairs ()
> +  (defvar electric-pair-text-pairs)
> +  (setq-local electric-pair-text-pairs
> +              (append '((?\` . ?\') (?‘ . ?’)) electric-pair-text-pairs))
> +  (remove-hook 'electric-pair-mode-hook #'emacs-lisp-set-electric-text-pairs t))
> +
>  ;;;###autoload
>  (define-derived-mode emacs-lisp-mode prog-mode "Emacs-Lisp"
>    "Major mode for editing Lisp code to run in Emacs.
> @@ -245,12 +251,8 @@ emacs-lisp-mode
>    (defvar project-vc-external-roots-function)
>    (lisp-mode-variables nil nil 'elisp)
>    (add-hook 'after-load-functions #'elisp--font-lock-flush-elisp-buffers)
> -  (unless noninteractive
> -    (require 'elec-pair)
> -    (defvar electric-pair-text-pairs)
> -    (setq-local electric-pair-text-pairs
> -                (append '((?\` . ?\') (?‘ . ?’)) electric-pair-text-pairs))
> -    (setq-local electric-quote-string t))
> +  (add-hook 'electric-pair-mode-hook #'emacs-lisp-set-electric-text-pairs t)

Shouldn't that be .....-text pairs nil t), to make a local value of the
                                   ^^^
hook?  The remove-hook (above) seems to be expecting a local value.

> +  (setq-local electric-quote-string t)
>    (setq imenu-case-fold-search nil)
>    (add-function :before-until (local 'eldoc-documentation-function)
>                  #'elisp-eldoc-documentation-function)
> -- 
> 2.11.0
> 

As a matter of interest, what is that 2.11.0 that I keep seeing at the
bottom of patches?

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: npostavs <at> gmail.com, 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sun, 07 Jul 2019 21:45:28 +0300

> Date: Sun, 7 Jul 2019 18:29:41 +0000
> From: Alan Mackenzie <acm <at> muc.de>
> Cc: 36539 <at> debbugs.gnu.org
> 
> Just as a matter of interest, I commented out the pertinent form in
> emacs-lisp-mode, did a make bootstrap, emacs -Q, and elec-pair.elc was
> still present in my Emacs.  :-(

I cannot reproduce this.  After applying Noam's patch and just saying
"make", "emacs -Q" doesn't load elec-pair anymore, which I verified
both by calling featurep and by running under GDB with a breakpoint in
Fload.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sun, 07 Jul 2019 14:47:51 -0400

[Message part 1 (text/plain, inline)]

Alan Mackenzie <acm <at> muc.de> writes:

> Just as a matter of interest, I commented out the pertinent form in
> emacs-lisp-mode, did a make bootstrap, emacs -Q, and elec-pair.elc was
> still present in my Emacs.  :-(
>
> I don't know why, or how, but it is.  With your patch (below), have you
> tested whether or not elec-pair.elc hasn't been loaded on starting
> Emacs?  If it hasn't been loaded, you've found some trick that eludes
> me.

How are you checking?  M-: (featurep 'elec-pair) RET returns nil for me.

Assuming you have help-enable-completion-auto-load set to t (the
default), it's pretty easy to load various things just by hitting TAB in
response to describe-function or describe-variable.

>> +  (add-hook 'electric-pair-mode-hook #'emacs-lisp-set-electric-text-pairs t)
>
> Shouldn't that be .....-text pairs nil t), to make a local value of the
>                                    ^^^
> hook?  The remove-hook (above) seems to be expecting a local value.

Oops, yes, thanks for catching that.

>> +  (setq-local electric-quote-string t)
>>    (setq imenu-case-fold-search nil)
>>    (add-function :before-until (local 'eldoc-documentation-function)
>>                  #'elisp-eldoc-documentation-function)
>> -- 
>> 2.11.0
>> 
>
> As a matter of interest, what is that 2.11.0 that I keep seeing at the
> bottom of patches?

Looks like 'git format-patch' adds the git version at the bottom.

$ git --version
git version 2.11.0

[0001-Don-t-load-elec-pair-in-elisp-mode-Bug-36539.patch (text/plain, attachment)]

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

From: Alan Mackenzie <acm <at> muc.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sun, 7 Jul 2019 19:10:52 +0000

Hello again, Noam.

On Sun, Jul 07, 2019 at 14:47:51 -0400, Noam Postavsky wrote:
> Alan Mackenzie <acm <at> muc.de> writes:

> > Just as a matter of interest, I commented out the pertinent form in
> > emacs-lisp-mode, did a make bootstrap, emacs -Q, and elec-pair.elc was
> > still present in my Emacs.  :-(

> > I don't know why, or how, but it is.  With your patch (below), have you
> > tested whether or not elec-pair.elc hasn't been loaded on starting
> > Emacs?  If it hasn't been loaded, you've found some trick that eludes
> > me.

> How are you checking?  M-: (featurep 'elec-pair) RET returns nil for me.

Yes, it worked for me, too.  :-(

> Assuming you have help-enable-completion-auto-load set to t (the
> default), it's pretty easy to load various things just by hitting TAB in
> response to describe-function or describe-variable.

That was my problem.  I didn't know about
help-enable-completion-auto-load (which says it was new in 26.3).  I
think I will disable this in my .emacs - it seems it will cause me more
problems than it will help.  I've always used C-h f and C-h v to see an
Elisp file's autoloads.  Thanks for the tip!.

[ .... ]

> >> +  (setq-local electric-quote-string t)
> >>    (setq imenu-case-fold-search nil)
> >>    (add-function :before-until (local 'eldoc-documentation-function)
> >>                  #'elisp-eldoc-documentation-function)
> >> -- 
> >> 2.11.0

> > As a matter of interest, what is that 2.11.0 that I keep seeing at the
> > bottom of patches?

> Looks like 'git format-patch' adds the git version at the bottom.

Ah, so that's it.  Thanks!

> $ git --version
> git version 2.11.0

Thanks for the revised patch.  I'll try it.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: npostavs <at> gmail.com, 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sun, 7 Jul 2019 19:14:55 +0000

Hello, Eli.

Thanks for the reply.

On Sun, Jul 07, 2019 at 21:45:28 +0300, Eli Zaretskii wrote:
> > Date: Sun, 7 Jul 2019 18:29:41 +0000
> > From: Alan Mackenzie <acm <at> muc.de>
> > Cc: 36539 <at> debbugs.gnu.org

> > Just as a matter of interest, I commented out the pertinent form in
> > emacs-lisp-mode, did a make bootstrap, emacs -Q, and elec-pair.elc was
> > still present in my Emacs.  :-(

> I cannot reproduce this.  After applying Noam's patch and just saying
> "make", "emacs -Q" doesn't load elec-pair anymore, which I verified
> both by calling featurep and by running under GDB with a breakpoint in
> Fload.

As Noam suggested, I was getting caught by
help-enable-completion-auto-load being t by default.  I tried to check
whether elec-pair.el had been loaded with C-h f electric-pair- <tab>,
which, of course loaded the file before giving me the complete list of
functions.

So, just pilot error on my part.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: npostavs <at> gmail.com, 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sun, 07 Jul 2019 22:17:46 +0300

> Date: Sun, 7 Jul 2019 19:10:52 +0000
> From: Alan Mackenzie <acm <at> muc.de>
> Cc: 36539 <at> debbugs.gnu.org
> 
> That was my problem.  I didn't know about
> help-enable-completion-auto-load (which says it was new in 26.3).  I
> think I will disable this in my .emacs - it seems it will cause me more
> problems than it will help.  I've always used C-h f and C-h v to see an
> Elisp file's autoloads.  Thanks for the tip!.

This option allows you to _disable_ auto-loading as side effect of
Help commands.  Emacs 26.1 and 26.2 also auto-load them, but don't let
you disable that.  So whatever you were used to do with C-h f was
already working against you since Emacs 26.1; 26.3 lets you control
that.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: npostavs <at> gmail.com, 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sat, 13 Jul 2019 14:54:40 +0300

> Date: Sun, 7 Jul 2019 19:10:52 +0000
> From: Alan Mackenzie <acm <at> muc.de>
> Cc: 36539 <at> debbugs.gnu.org
> 
> > How are you checking?  M-: (featurep 'elec-pair) RET returns nil for me.
> 
> Yes, it worked for me, too.  :-(
> 
> > Assuming you have help-enable-completion-auto-load set to t (the
> > default), it's pretty easy to load various things just by hitting TAB in
> > response to describe-function or describe-variable.
> 
> That was my problem.  I didn't know about
> help-enable-completion-auto-load (which says it was new in 26.3).  I
> think I will disable this in my .emacs - it seems it will cause me more
> problems than it will help.  I've always used C-h f and C-h v to see an
> Elisp file's autoloads.  Thanks for the tip!.
> 
> [ .... ]
> 
> > >> +  (setq-local electric-quote-string t)
> > >>    (setq imenu-case-fold-search nil)
> > >>    (add-function :before-until (local 'eldoc-documentation-function)
> > >>                  #'elisp-eldoc-documentation-function)
> > >> -- 
> > >> 2.11.0
> 
> > > As a matter of interest, what is that 2.11.0 that I keep seeing at the
> > > bottom of patches?
> 
> > Looks like 'git format-patch' adds the git version at the bottom.
> 
> Ah, so that's it.  Thanks!
> 
> > $ git --version
> > git version 2.11.0
> 
> Thanks for the revised patch.  I'll try it.

Any reasons this is not yet installed on master?

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Alan Mackenzie <acm <at> muc.de>, 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Sat, 13 Jul 2019 18:41:23 -0400

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

> Any reasons this is not yet installed on master?

Actually, I noticed a problem with it: electric-pair-mode is a global
mode, so using the local value of the hook isn't right.  I think the
updated patch below is correct.

[0001-Don-t-load-elec-pair-in-elisp-mode-Bug-36539.patch (text/x-diff, inline)]

From ebec942f5734f2ce6916dc14706704dc1a93e62d Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 7 Jul 2019 12:22:37 -0400
Subject: [PATCH] Don't load elec-pair in elisp-mode (Bug#36539)

* lisp/progmodes/elisp-mode.el (emacs-lisp-set-electric-text-pairs):
New function.
(emacs-lisp-mode): Add it to electric-pair-mode-hook, if elec-pair
hasn't been loaded yet.
---
 lisp/progmodes/elisp-mode.el | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index c86277a309..5204c78131 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -233,6 +233,17 @@ emacs-lisp-mode-hook
   :type 'hook
   :group 'lisp)
 
+(defun emacs-lisp-set-electric-text-pairs ()
+  (defvar electric-pair-text-pairs)
+  (let ((elisp-pairs (append '((?\` . ?\') (?‘ . ?’))
+                             electric-pair-text-pairs)))
+    (save-current-buffer
+      (dolist (buf (buffer-list))
+        (set-buffer buf)
+        (when (derived-mode-p 'emacs-lisp-mode)
+          (setq-local electric-pair-text-pairs elisp-pairs)))))
+  (remove-hook 'electric-pair-mode-hook #'emacs-lisp-set-electric-text-pairs))
+
 ;;;###autoload
 (define-derived-mode emacs-lisp-mode prog-mode "Emacs-Lisp"
   "Major mode for editing Lisp code to run in Emacs.
@@ -245,12 +256,12 @@ emacs-lisp-mode
   (defvar project-vc-external-roots-function)
   (lisp-mode-variables nil nil 'elisp)
   (add-hook 'after-load-functions #'elisp--font-lock-flush-elisp-buffers)
-  (unless noninteractive
-    (require 'elec-pair)
-    (defvar electric-pair-text-pairs)
-    (setq-local electric-pair-text-pairs
-                (append '((?\` . ?\') (?‘ . ?’)) electric-pair-text-pairs))
-    (setq-local electric-quote-string t))
+  (if (boundp 'electric-pair-text-pairs)
+      (setq-local electric-pair-text-pairs
+                  (append '((?\` . ?\') (?‘ . ?’))
+                          electric-pair-text-pairs))
+    (add-hook 'electric-pair-mode-hook #'emacs-lisp-set-electric-text-pairs))
+  (setq-local electric-quote-string t)
   (setq imenu-case-fold-search nil)
   (add-function :before-until (local 'eldoc-documentation-function)
                 #'elisp-eldoc-documentation-function)
-- 
2.11.0

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Alan Mackenzie <acm <at> muc.de>, 36539 <at> debbugs.gnu.org
Subject: Re: bug#36539: elec-pair.elc gets surreptitiously loaded (almost)
 unconditionally at start up.
Date: Thu, 18 Jul 2019 20:09:57 -0400

tags 36539 fixed
close 36539 27.1
quit

> Actually, I noticed a problem with it: electric-pair-mode is a global
> mode, so using the local value of the hook isn't right.  I think the
> updated patch below is correct.

> Subject: [PATCH] Don't load elec-pair in elisp-mode (Bug#36539)
>
> * lisp/progmodes/elisp-mode.el (emacs-lisp-set-electric-text-pairs):
> New function.
> (emacs-lisp-mode): Add it to electric-pair-mode-hook, if elec-pair
> hasn't been loaded yet.

Pushed to master.

8f54998827 2019-07-18T20:06:27-04:00 "Don't load elec-pair in elisp-mode (Bug#36539)"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=8f54998827e416fe1fca7bc384bf0a6281b73f1b

Previous Next

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