bug#45393: 27.1; Make remove-hook (interactive

[Drew Adams <drew.adams@oracle.com>]

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

1. I think this is a fine idea.

For one thing, it'll help with the gotcha of
someone misguidedly using a lambda form as a
hook function (which requires using the same
form for removal).  Instead of having to type
the lambda form exactly for `remove-hook', it
can be retrieved from the history.

2. Please add a space char at the end of the
"Function:" prompt.

3. The doc string should maybe start with a
description of the interactive use.  I think
that's typical.  And that description should
describe each arg (instead of the interactive
description just referring to the Lisp
description).

On the other hand, 90% of the uses will not
be interactive, so maybe that general rule
shouldn't be followed here?

4. I suggest doing the same for `add-hook',
and letting `remove-hook' use the last hook
added interactively as default when reading
the hook.

Attached is some quick-and-dirty code that
does that, to show what I mean.  If the idea
is accepted then we can work out a patch.

When `add-hook' is used interactively (only),
the hook var and function are recorded in
vars `last-add-hook-var' and `hook-history'.

[-- Attachment #2: throw-add-remove-hook.el --]
[-- Type: application/octet-stream, Size: 6081 bytes --]

(defvar last-add-hook-var nil
  "Last hook variable used with `add-hook' interactively.")
(defvar hook-history ()
  "History of hook variables used as minibuffer input.")

(defun add-hook (hook function &optional append local msgp)
  "Add to the value of HOOK the function FUNCTION.
FUNCTION is not added if already present.
FUNCTION is added (if necessary) at the beginning of the hook list
unless the optional argument APPEND is non-nil, in which case
FUNCTION is added at the end.

The optional fourth argument, LOCAL, if non-nil, says to modify
the hook's buffer-local value rather than its global value.
This makes the hook buffer-local, and it makes t a member of the
buffer-local value.  That acts as a flag to run the hook
functions of the global value as well as in the local value.

HOOK should be a symbol, and FUNCTION may be any valid function.  If
HOOK is void, it is first set to nil.  If HOOK's value is a single
function, it is changed to a list of functions.

Interactively, prompt for the various arguments (skipping local
unless HOOK has both local and global functions).  With a prefix
argument, append, else prepend."
  (interactive
   (let* ((hook (intern
		 (completing-read "Hook variable: "
				  obarray #'boundp t nil
				  'hook-history last-add-hook-var)))
          (local
           (and
            (local-variable-p hook)
            (symbol-value hook)
            (or (not (default-value hook)) ; No need to prompt if there's nothing global
                (y-or-n-p (format "%s has a buffer-local binding, use that? " hook)))))
	  (function (read-from-minibuffer "Function: " nil nil 'READ)))
     (list hook function current-prefix-arg local t)))
  (or (boundp hook) (set hook nil))
  (or (default-boundp hook) (set-default hook nil))
  (if local (unless (local-variable-if-set-p hook)
	      (set (make-local-variable hook) (list t)))
    ;; Detect the case where make-local-variable was used on a hook
    ;; and do what we used to do.
    (unless (and (consp (symbol-value hook)) (memq t (symbol-value hook)))
      (setq local t)))
  (let ((hook-value (if local (symbol-value hook) (default-value hook))))
    ;; If the hook value is a single function, turn it into a list.
    (when (or (not (listp hook-value)) (functionp hook-value))
      (setq hook-value (list hook-value)))
    ;; Do the actual addition if necessary
    (unless (member function hook-value)
      (when (stringp function)
	(setq function (purecopy function)))
      (setq hook-value
	    (if append
		(append hook-value (list function))
	      (cons function hook-value))))
    ;; Set the actual variable
    (if local
	(progn
	  ;; If HOOK isn't a permanent local,
	  ;; but FUNCTION wants to survive a change of modes,
	  ;; mark HOOK as partially permanent.
	  (and (symbolp function)
	       (get function 'permanent-local-hook)
	       (not (get hook 'permanent-local))
	       (put hook 'permanent-local 'permanent-local-hook))
	  (set hook hook-value))
      (set-default hook hook-value)))
  (when msgp (setq last-add-hook-var  hook)))


(defun remove-hook (hook function &optional local)
  "Remove from the value of HOOK the function FUNCTION.
HOOK should be a symbol, and FUNCTION may be any valid function.  If
FUNCTION isn't the value of HOOK, or, if FUNCTION doesn't appear in the
list of hooks to run in HOOK, then nothing is done.  See `add-hook'.

The optional third argument, LOCAL, if non-nil, says to modify
the hook's buffer-local value rather than its default value.

Interactively, prompt for the various arguments (skipping local
unless HOOK has both local and global functions).  If multiple
functions have the same representation under `princ', remove the
first one."
  (interactive
   (let* ((hook (intern
		 (completing-read "Hook variable: "
				  obarray #'boundp t nil 'hook-history
				  (and last-add-hook-var
				       (symbol-name last-add-hook-var)))))
          (local
           (and
            (local-variable-p hook)
            (symbol-value hook)
            (or (not (default-value hook)) ; No need to prompt if there's nothing global
                (y-or-n-p (format "%s has a buffer-local binding, use that? " hook)))))
          (fn-alist (mapcar
                     (lambda (x) (cons (with-output-to-string (prin1 x)) x))
                     (if local (symbol-value hook) (default-value hook))))
          (function (alist-get (completing-read
                                (format "%s hook to remove: "
                                        (if local "Buffer-local" "Global"))
                                fn-alist
                                nil t)
                               fn-alist nil nil 'string=)))
     (list hook function local)))
  (or (boundp hook) (set hook nil))
  (or (default-boundp hook) (set-default hook nil))
  ;; Do nothing if LOCAL is t but this hook has no local binding.
  (unless (and local (not (local-variable-p hook)))
    ;; Detect the case where make-local-variable was used on a hook
    ;; and do what we used to do.
    (when (and (local-variable-p hook)
	       (not (and (consp (symbol-value hook))
			 (memq t (symbol-value hook)))))
      (setq local t))
    (let ((hook-value (if local (symbol-value hook) (default-value hook))))
      ;; Remove the function, for both the list and the non-list cases.
      (if (or (not (listp hook-value)) (eq (car hook-value) 'lambda))
	  (if (equal hook-value function) (setq hook-value nil))
	(setq hook-value (delete function (copy-sequence hook-value))))
      ;; If the function is on the global hook, we need to shadow it locally
      ;;(when (and local (member function (default-value hook))
      ;;	       (not (member (cons 'not function) hook-value)))
      ;;  (push (cons 'not function) hook-value))
      ;; Set the actual variable
      (if (not local)
	  (set-default hook hook-value)
	(if (equal hook-value '(t))
	    (kill-local-variable hook)
	  (set hook hook-value))))))