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

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

[Alan Mackenzie]

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).

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

[Noam Postavsky]

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

tags 36539 + patch
quit

Alan Mackenzie <acm@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:


[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1922 bytes --]

From cf6057cb5bf6a44d718349776e370b225bc7079f Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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

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

[Alan Mackenzie]

Hello, Noam.

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

> Alan Mackenzie <acm@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@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).

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

[Eli Zaretskii]

> Date: Sun, 7 Jul 2019 18:29:41 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: 36539@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.

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

[Noam Postavsky]

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

Alan Mackenzie <acm@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


[-- Attachment #2: updated patch --]
[-- Type: text/plain, Size: 1926 bytes --]

From c3a99c7260e07692c8d02a9cd9d38f6ef188311c Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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..78a87f2d32 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 nil 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

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

[Alan Mackenzie]

Hello again, Noam.

On Sun, Jul 07, 2019 at 14:47:51 -0400, Noam Postavsky wrote:
> Alan Mackenzie <acm@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).

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

[Alan Mackenzie]

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@muc.de>
> > Cc: 36539@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).

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

[Eli Zaretskii]

> Date: Sun, 7 Jul 2019 19:10:52 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: 36539@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.

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

[Eli Zaretskii]

> Date: Sun, 7 Jul 2019 19:10:52 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: 36539@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?

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

[Noam Postavsky]

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

Eli Zaretskii <eliz@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.


[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2374 bytes --]

From ebec942f5734f2ce6916dc14706704dc1a93e62d Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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

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

[Noam Postavsky]

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