add-hook and defvar

[Artur Malabarba]

While it's nice that `add-hook' tries to do the right thing even when
the variable isn't defined yet, that's a bit of a false assurance.
Although it's documented in the docstring, it seems undesirable that
it sets the variable. This means that if the hook's defvar were to
specify an initial value, this value won't take effect because of that
preceding `add-hook' invocation.

In this situation, add-hook doesn't so much "add" as it actually sets
the value of the hook.

The following patch is one way of having `add-hook' always add,
without overriding the "future" initial value of the hook. It is not
complete, it's intended as a possible outline (hopefully a
conversation starter).

The big disadvantage is that it's not backwards compatible. Any code
out there that relies on the variable being set by `add-hook' will
break.
There's also the disadvantage that it adds a symbol-property check to
every `defvar' call.

Any thoughts?

---
 lisp/subr.el | 68 ++++++++++++++++++++++++++++++++----------------------------
 src/eval.c   | 11 +++++++++-
 2 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index deadca6..a9a7aef 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1389,38 +1389,42 @@ 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."
-  (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))))
+  (if (not (boundp hook))
+      ;; If the hook is not defined yet, store the value until it is.
+      (push (list function append (when local (current-buffer)))
+            (get hook 'pre-hooked-functions))
+
+    ;; Everything below this line wasn't changed (indentation only).
+    (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)))))

 (defun remove-hook (hook function &optional local)
   "Remove from the value of HOOK the function FUNCTION.
diff --git a/src/eval.c b/src/eval.c
index e828da9..119ffe0 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -729,7 +729,7 @@ To define a user option, use `defcustom' instead
of `defvar'.
 usage: (defvar SYMBOL &optional INITVALUE DOCSTRING)  */)
   (Lisp_Object args)
 {
-  Lisp_Object sym, tem, tail;
+  Lisp_Object sym, tem, tail, pre_hooked;

   sym = XCAR (args);
   tail = XCDR (args);
@@ -763,6 +763,15 @@ usage: (defvar SYMBOL &optional INITVALUE DOCSTRING)  */)
         tem = Fpurecopy (tem);
       Fput (sym, Qvariable_documentation, tem);
     }
+      /* In the final version, this should probably be
+         Qpre_hooked_functions. */
+      pre_hooked = Fget (sym, intern ("pre-hooked-functions"));;
+      if (!NILP (pre_hooked))
+        {
+          /* do something like add-hook on each element of the
+             pre_hooked list. */
+        }
+
       LOADHIST_ATTACH (sym);
     }
   else if (!NILP (Vinternal_interpreter_environment)
-- 
2.3.0