master a63d905: Fix misuses of `make-local-variable` on hooks

[Stefan Monnier]

branch: master
commit a63d90517549b1940ffbe3438a614afc7ea1aa6d
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    Fix misuses of `make-local-variable` on hooks
    
    * lisp/vc/smerge-mode.el (smerge-ediff):
    * lisp/progmodes/python.el (python-pdbtrack-setup-tracking):
    * lisp/net/tramp-smb.el (tramp-smb-call-winexe):
    * lisp/net/secrets.el (secrets-mode):
    * lisp/mail/rmail.el (rmail-variables):
    * lisp/ielm.el (inferior-emacs-lisp-mode):
    * lisp/erc/erc-log.el (erc-log-setup-logging): Use `add-hook`.
    
    * lisp/eshell/em-unix.el (eshell/diff):
    * lisp/eshell/em-hist.el (eshell-hist-initialize): Don't
    `make-local-variable` on hooks.
---
 lisp/dframe.el           |  3 +++
 lisp/erc/erc-log.el      |  2 +-
 lisp/eshell/em-hist.el   | 40 +++++++++++++---------------------------
 lisp/eshell/em-unix.el   | 11 +++++------
 lisp/gnus/nnimap.el      |  2 +-
 lisp/ielm.el             |  7 ++++---
 lisp/mail/rmail.el       |  3 +--
 lisp/net/secrets.el      |  4 ++--
 lisp/net/tramp-smb.el    |  3 +--
 lisp/progmodes/python.el |  4 ++--
 lisp/vc/smerge-mode.el   | 38 ++++++++++++++++++++------------------
 lisp/vc/vc-dispatcher.el |  4 +++-
 12 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/lisp/dframe.el b/lisp/dframe.el
index efe2bc5..417477b 100644
--- a/lisp/dframe.el
+++ b/lisp/dframe.el
@@ -287,6 +287,9 @@ CREATE-HOOK is a hook to run after creating a frame."
       ;; Correct use of `temp-buffer-show-function': Bob Weiner
       (if (and (boundp 'temp-buffer-show-hook)
               (boundp 'temp-buffer-show-function))
+         ;; FIXME: Doesn't this get us into an inf-loop when the
+          ;; `temp-buffer-show-function' runs `temp-buffer-show-hook'
+          ;; (as is normally the case)?
          (progn (make-local-variable 'temp-buffer-show-hook)
                 (setq temp-buffer-show-hook temp-buffer-show-function)))
       (make-local-variable 'temp-buffer-show-function)
diff --git a/lisp/erc/erc-log.el b/lisp/erc/erc-log.el
index 2166123..de0a16e 100644
--- a/lisp/erc/erc-log.el
+++ b/lisp/erc/erc-log.el
@@ -267,7 +267,7 @@ The current buffer is given by BUFFER."
     (with-current-buffer buffer
       (auto-save-mode -1)
       (setq buffer-file-name nil)
-      (set (make-local-variable 'write-file-functions) 
'(erc-save-buffer-in-logs))
+      (add-hook 'write-file-functions #'erc-save-buffer-in-logs nil t)
       (when erc-log-insert-log-on-open
        (ignore-errors
          (save-excursion
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 5cee1ba..bdc21c9 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -75,8 +75,7 @@
 (defcustom eshell-hist-load-hook nil
   "A list of functions to call when loading `eshell-hist'."
   :version "24.1"                      ; removed eshell-hist-initialize
-  :type 'hook
-  :group 'eshell-hist)
+  :type 'hook)
 
 (defcustom eshell-hist-unload-hook
   (list
@@ -84,8 +83,7 @@
     (lambda ()
       (remove-hook 'kill-emacs-hook 'eshell-save-some-history))))
   "A hook that gets run when `eshell-hist' is unloaded."
-  :type 'hook
-  :group 'eshell-hist)
+  :type 'hook)
 
 (defcustom eshell-history-file-name
   (expand-file-name "history" eshell-directory-name)
@@ -93,20 +91,17 @@
 See also `eshell-read-history' and `eshell-write-history'.
 If it is nil, Eshell will use the value of HISTFILE."
   :type '(choice (const :tag "Use HISTFILE" nil)
-                file)
-  :group 'eshell-hist)
+                file))
 
 (defcustom eshell-history-size 128
   "Size of the input history ring.  If nil, use envvar HISTSIZE."
   :type '(choice (const :tag "Use HISTSIZE" nil)
-                integer)
-  :group 'eshell-hist)
+                integer))
 
 (defcustom eshell-hist-ignoredups nil
   "If non-nil, don't add input matching the last on the input ring.
 This mirrors the optional behavior of bash."
-  :type 'boolean
-  :group 'eshell-hist)
+  :type 'boolean)
 
 (defcustom eshell-save-history-on-exit t
   "Determine if history should be automatically saved.
@@ -118,8 +113,7 @@ If set to `ask', ask if any Eshell buffers are open at exit 
time.
 If set to t, history will always be saved, silently."
   :type '(choice (const :tag "Never" nil)
                 (const :tag "Ask" ask)
-                (const :tag "Always save" t))
-  :group 'eshell-hist)
+                (const :tag "Always save" t)))
 
 (defcustom eshell-input-filter 'eshell-input-filter-default
   "Predicate for filtering additions to input history.
@@ -128,8 +122,7 @@ the input history list.  Default is to save anything that 
isn't all
 whitespace."
   :type '(radio (function-item eshell-input-filter-default)
                 (function-item eshell-input-filter-initial-space)
-                (function :tag "Other function"))
-  :group 'eshell-hist)
+                (function :tag "Other function")))
 
 (put 'eshell-input-filter 'risky-local-variable t)
 
@@ -138,31 +131,26 @@ whitespace."
 Otherwise, typing <M-p> and <M-n> will always go to the next history
 element, regardless of any text on the command line.  In that case,
 <C-c M-r> and <C-c M-s> still offer that functionality."
-  :type 'boolean
-  :group 'eshell-hist)
+  :type 'boolean)
 
 (defcustom eshell-hist-move-to-end t
   "If non-nil, move to the end of the buffer before cycling history."
-  :type 'boolean
-  :group 'eshell-hist)
+  :type 'boolean)
 
 (defcustom eshell-hist-event-designator
   "^!\\(!\\|-?[0-9]+\\|\\??[^:^$%*?]+\\??\\|#\\)"
   "The regexp used to identifier history event designators."
-  :type 'regexp
-  :group 'eshell-hist)
+  :type 'regexp)
 
 (defcustom eshell-hist-word-designator
   "^:?\\([0-9]+\\|[$^%*]\\)?\\(-[0-9]*\\|[$^%*]\\)?"
   "The regexp used to identify history word designators."
-  :type 'regexp
-  :group 'eshell-hist)
+  :type 'regexp)
 
 (defcustom eshell-hist-modifier
   "^\\(:\\([hretpqx&g]\\|s/\\([^/]*\\)/\\([^/]*\\)/\\)\\)*"
   "The regexp used to identity history modifiers."
-  :type 'regexp
-  :group 'eshell-hist)
+  :type 'regexp)
 
 (defcustom eshell-hist-rebind-keys-alist
   '(([(control ?p)]   . eshell-previous-input)
@@ -180,8 +168,7 @@ element, regardless of any text on the command line.  In 
that case,
   "History keys to bind differently if point is in input text."
   :type '(repeat (cons (vector :tag "Keys to bind"
                               (repeat :inline t sexp))
-                      (function :tag "Command")))
-  :group 'eshell-hist)
+                      (function :tag "Command"))))
 
 ;;; Internal Variables:
 
@@ -308,7 +295,6 @@ Returns nil if INPUT is prepended by blank space, otherwise 
non-nil."
 
   (add-hook 'kill-emacs-hook #'eshell-save-some-history)
 
-  (make-local-variable 'eshell-input-filter-functions)
   (add-hook 'eshell-input-filter-functions #'eshell-add-to-history nil t))
 
 (defun eshell-save-some-history ()
diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el
index 68aa680..937b8bf 100644
--- a/lisp/eshell/em-unix.el
+++ b/lisp/eshell/em-unix.el
@@ -1007,18 +1007,17 @@ Show wall-clock time elapsed during execution of 
COMMAND.")
               (throw 'eshell-replace-command
                      (eshell-parse-command "*diff" orig-args))))
          (when (fboundp 'diff-mode)
-           (make-local-variable 'compilation-finish-functions)
            (add-hook
             'compilation-finish-functions
-            `(lambda (buff msg)
+            (lambda (buff _msg)
                (with-current-buffer buff
                  (diff-mode)
-                 (set (make-local-variable 'eshell-diff-window-config)
-                      ,config)
-                 (local-set-key [?q] 'eshell-diff-quit)
+                 (set (make-local-variable 'eshell-diff-window-config) config)
+                 (local-set-key [?q] #'eshell-diff-quit)
                  (if (fboundp 'turn-on-font-lock-if-enabled)
                      (turn-on-font-lock-if-enabled))
-                 (goto-char (point-min))))))
+                 (goto-char (point-min))))
+            nil t))
          (pop-to-buffer (current-buffer))))))
   nil)
 
diff --git a/lisp/gnus/nnimap.el b/lisp/gnus/nnimap.el
index 8a88e0e..7984998 100644
--- a/lisp/gnus/nnimap.el
+++ b/lisp/gnus/nnimap.el
@@ -365,7 +365,7 @@ textual parts.")
     (mm-disable-multibyte)
     (buffer-disable-undo)
     (gnus-add-buffer)
-    (set (make-local-variable 'after-change-functions) nil)
+    (set (make-local-variable 'after-change-functions) nil) ;FIXME: Why?
     (set (make-local-variable 'nnimap-object)
         (make-nnimap :server (nnoo-current-server 'nnimap)
                      :initial-resync 0))
diff --git a/lisp/ielm.el b/lisp/ielm.el
index b3654b9..91d025d 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -533,9 +533,10 @@ Customized bindings may be defined in `ielm-map', which 
currently contains:
   (set (make-local-variable 'paragraph-start) comint-prompt-regexp)
   (setq comint-input-sender 'ielm-input-sender)
   (setq comint-process-echoes nil)
-  (set (make-local-variable 'completion-at-point-functions)
-       '(comint-replace-by-expanded-history
-         ielm-complete-filename elisp-completion-at-point))
+  (dolist (f '(elisp-completion-at-point
+               ielm-complete-filename
+               comint-replace-by-expanded-history))
+    (add-hook 'completion-at-point-functions f nil t))
   (add-hook 'eldoc-documentation-functions
             #'elisp-eldoc-var-docstring nil t)
   (add-hook 'eldoc-documentation-functions
diff --git a/lisp/mail/rmail.el b/lisp/mail/rmail.el
index 86084b0..2c972ee 100644
--- a/lisp/mail/rmail.el
+++ b/lisp/mail/rmail.el
@@ -1492,8 +1492,7 @@ If so restore the actual mbox message collection."
   (setq require-final-newline nil)
   (make-local-variable 'version-control)
   (setq version-control 'never)
-  (make-local-variable 'kill-buffer-hook)
-  (add-hook 'kill-buffer-hook 'rmail-mode-kill-summary)
+  (add-hook 'kill-buffer-hook #'rmail-mode-kill-summary nil t)
   (make-local-variable 'file-precious-flag)
   (setq file-precious-flag t)
   (make-local-variable 'desktop-save-buffer)
diff --git a/lisp/net/secrets.el b/lisp/net/secrets.el
index dc1b468..f98ded4 100644
--- a/lisp/net/secrets.el
+++ b/lisp/net/secrets.el
@@ -795,8 +795,8 @@ In this mode, widgets represent the search results.
   (set (make-local-variable 'revert-buffer-function)
        #'secrets-show-collections)
   ;; When we toggle, we must set temporary widgets.
-  (set (make-local-variable 'tree-widget-after-toggle-functions)
-       '(secrets-tree-widget-after-toggle-function)))
+  (add-hook 'tree-widget-after-toggle-functions
+            #'secrets-tree-widget-after-toggle-function nil t))
 
 ;; It doesn't make sense to call it interactively.
 (put 'secrets-mode 'disabled t)
diff --git a/lisp/net/tramp-smb.el b/lisp/net/tramp-smb.el
index 3220e51..a040508 100644
--- a/lisp/net/tramp-smb.el
+++ b/lisp/net/tramp-smb.el
@@ -2143,8 +2143,7 @@ Removes smb prompt.  Returns nil if an error message has 
appeared."
     "%s %s"
     tramp-smb-winexe-shell-command tramp-smb-winexe-shell-command-switch))
 
-  (set (make-local-variable 'kill-buffer-hook)
-       '(tramp-smb-kill-winexe-function))
+  (add-hook 'kill-buffer-hook #'tramp-smb-kill-winexe-function nil t)
 
   ;; Suppress "^M".  Shouldn't we specify utf8?
   (set-process-coding-system (tramp-get-connection-process vec) 'raw-text-dos)
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 53b6540..d6feba2 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -4011,8 +4011,8 @@ Argument OUTPUT is a string with the output from the 
comint process."
   "Setup pdb tracking in current buffer."
   (make-local-variable 'python-pdbtrack-buffers-to-kill)
   (make-local-variable 'python-pdbtrack-tracked-buffer)
-  (add-to-list (make-local-variable 'comint-input-filter-functions)
-               #'python-pdbtrack-comint-input-filter-function)
+  (add-hook 'comint-input-filter-functions
+            #'python-pdbtrack-comint-input-filter-function nil t)
   (add-to-list (make-local-variable 'comint-output-filter-functions)
                #'python-pdbtrack-comint-output-filter-function)
   (add-function :before (process-sentinel (get-buffer-process 
(current-buffer)))
diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index d0a83fd..fe7724d 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -1352,24 +1352,26 @@ buffer names."
     ;; Do a few further adjustments and take precautions for exit.
     (set (make-local-variable 'smerge-ediff-windows) config)
     (set (make-local-variable 'smerge-ediff-buf) buf)
-    (set (make-local-variable 'ediff-quit-hook)
-        (lambda ()
-          (let ((buffer-A ediff-buffer-A)
-                (buffer-B ediff-buffer-B)
-                (buffer-C ediff-buffer-C)
-                (buffer-Ancestor ediff-ancestor-buffer)
-                (buf smerge-ediff-buf)
-                (windows smerge-ediff-windows))
-            (ediff-cleanup-mess)
-            (with-current-buffer buf
-              (erase-buffer)
-              (insert-buffer-substring buffer-C)
-              (kill-buffer buffer-A)
-              (kill-buffer buffer-B)
-              (kill-buffer buffer-C)
-              (when (bufferp buffer-Ancestor) (kill-buffer buffer-Ancestor))
-              (set-window-configuration windows)
-              (message "Conflict resolution finished; you may save the 
buffer")))))
+    (add-hook 'ediff-quit-hook
+             (lambda ()
+               (let ((buffer-A ediff-buffer-A)
+                     (buffer-B ediff-buffer-B)
+                     (buffer-C ediff-buffer-C)
+                     (buffer-Ancestor ediff-ancestor-buffer)
+                     (buf smerge-ediff-buf)
+                     (windows smerge-ediff-windows))
+                 (ediff-cleanup-mess)
+                 (with-current-buffer buf
+                   (erase-buffer)
+                   (insert-buffer-substring buffer-C)
+                   (kill-buffer buffer-A)
+                   (kill-buffer buffer-B)
+                   (kill-buffer buffer-C)
+                   (when (bufferp buffer-Ancestor)
+                     (kill-buffer buffer-Ancestor))
+                   (set-window-configuration windows)
+                   (message "Conflict resolution finished; you may save the 
buffer"))))
+             nil t)
     (message "Please resolve conflicts now; exit ediff when done")))
 
 (defun smerge-makeup-conflict (pt1 pt2 pt3 &optional pt4)
diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index 99bf5bf..932b915 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -138,7 +138,9 @@ preserve the setting."
 ;; Variables the user doesn't need to know about.
 
 (defvar vc-log-operation nil)
-(defvar vc-log-after-operation-hook nil)
+(defvar vc-log-after-operation-hook nil
+  "Name of the hook run at the end of `vc-finish-logentry'.
+BEWARE: Despite its name, this variable is not itself a hook!")
 (defvar vc-log-fileset)
 
 ;; In a log entry buffer, this is a local variable