bug#35508: 27.0.50; Fine-ordering of functions on hooks

[Stefan Monnier]

>>> Occasionally it's important to control the relative ordering of
>>> functions on hooks.  It's usually a bad idea, but sometimes alternatives
>>> are worse.
>> Could you please give a couple of examples?
>
> The patch includes the post-self-insert-hook example, I already mentioned
> the after-change-functions example (where cc-mode wants his hook to
> come before font-lock's), and I could add the case of
> syntax-propertize's before-change-functions which needs to "come last".

I updated the patch so we use it for syntax-propertize's
before-change-functions as well as a comment showing where we'd use it
in cc-mode (using it in CC-mode would take more work in order to make
sure it doesn't break on older Emacsen).

>> If the worse comes to worst, a Lisp program could concoct
>> the entire hook list in any order it sees fit, right?
> I don't understand what you mean here.

[ Still don't understand.  ]

>> That's backward-incompatible, no?
> Sure.

I added a note in the "incompatible changes" section of NEWS.

>> In any case, this default is insufficiently tested by the tests
>> you propose.
> What other tests do you suggest?

I added a test to make sure nil is treated like 0.

>> So using 100 more than once makes the last one "win"?
> Yes.  This is so that when using `t` more than once, the last one wins,
> just as it used to.
>
>>> +For backward compatibility reasons, a symbol other than nil is
>>> +interpreted as a DEPTH of 90.
>> This is not explicitly tested by the test.

I added a test to make sure t is treated like 90.

Other objections?


        Stefan


diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
index 97e9be9918..3a2a7f6397 100644
--- a/doc/lispref/modes.texi
+++ b/doc/lispref/modes.texi
@@ -142,7 +142,7 @@ Setting Hooks
 (add-hook 'lisp-interaction-mode-hook 'auto-fill-mode)
 @end example
 
-@defun add-hook hook function &optional append local
+@defun add-hook hook function &optional depth local
 This function is the handy way to add function @var{function} to hook
 variable @var{hook}.  You can use it for abnormal hooks as well as for
 normal hooks.  @var{function} can be any Lisp function that can accept
@@ -167,9 +167,16 @@ Setting Hooks
 in which they are executed does not matter.  Any dependence on the order
 is asking for trouble.  However, the order is predictable: normally,
 @var{function} goes at the front of the hook list, so it is executed
-first (barring another @code{add-hook} call).  If the optional argument
-@var{append} is non-@code{nil}, the new hook function goes at the end of
-the hook list and is executed last.
+first (barring another @code{add-hook} call).
+
+In some cases, it is important to control the relative ordering of
+functions on the hook.  The optional argument @var{depth} lets you indicate
+where the function should be inserted in the list: it should then be a number
+between -100 and 100 where the higher the value, the closer to the end of the
+list the function should go.  The @var{depth} defaults to 0 and for backward
+compatibility when @var{depth} is a non-nil symbol it is interpreted as a depth
+of 90.  Furthermore, when @var{depth} is strictly greater than 0 the function
+is added @emph{after} rather than before functions of the same depth.
 
 @code{add-hook} can handle the cases where @var{hook} is void or its
 value is a single function; it sets or changes the value to a list of
diff --git a/etc/NEWS b/etc/NEWS
index d10a553244..ebb1dacf22 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1452,6 +1452,12 @@ documentation of the new mode and its commands.
 
 * Incompatible Lisp Changes in Emacs 27.1
 
+** add-hook does not always add to the front or the end any more.
+The replacement of `append` with `depth` implies that the function is not
+always added to the very front (when append/depth is nil) or the very end (when
+append/depth is t) any more because other functions on the hook may have
+specified higher/lower depths.
+
 ** In 'compilation-error-regexp-alist' the old undocumented feature
 where 'line' could be a function of 2 arguments has been dropped.
 
@@ -1583,6 +1589,12 @@ valid event type.
 
 * Lisp Changes in Emacs 27.1
 
++++
+** The 'append' arg of 'add-hook' is generalized to a finer notion of 'depth'
+This allows to control the ordering of functions more precisely,
+as was already possible in 'add-function' and `advice-add`.
+
+---
 ** New 'help-fns-describe-variable-functions' hook.
 Makes it possible to add metadata information to 'describe-variable'.
 
diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index 3be09d87b4..5fb9d751e2 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -564,13 +564,6 @@ electric-pair-post-self-insert-function
                       (matching-paren (char-after))))
          (save-excursion (newline 1 t)))))))
 
-;; Prioritize this to kick in after
-;; `electric-layout-post-self-insert-function': that considerably
-;; simplifies interoperation when `electric-pair-mode',
-;; `electric-layout-mode' and `electric-indent-mode' are used
-;; together.  Use `vc-region-history' on these lines for more info.
-(put 'electric-pair-post-self-insert-function   'priority  50)
-
 (defun electric-pair-will-use-region ()
   (and (use-region-p)
        (memq (car (electric-pair-syntax-info last-command-event))
@@ -622,8 +615,14 @@ electric-pair-mode
   (if electric-pair-mode
       (progn
        (add-hook 'post-self-insert-hook
-                 #'electric-pair-post-self-insert-function)
-        (electric--sort-post-self-insertion-hook)
+                 #'electric-pair-post-self-insert-function
+                  ;; Prioritize this to kick in after
+                  ;; `electric-layout-post-self-insert-function': that
+                  ;; considerably simplifies interoperation when
+                  ;; `electric-pair-mode', `electric-layout-mode' and
+                  ;; `electric-indent-mode' are used together.
+                  ;; Use `vc-region-history' on these lines for more info.
+                  50)
        (add-hook 'self-insert-uses-region-functions
                  #'electric-pair-will-use-region))
     (remove-hook 'post-self-insert-hook
diff --git a/lisp/electric.el b/lisp/electric.el
index 07da2f1d9e..53e53bd975 100644
--- a/lisp/electric.el
+++ b/lisp/electric.el
@@ -190,17 +190,6 @@ electric--after-char-pos
                            (eq (char-before) last-command-event)))))
       pos)))
 
-(defun electric--sort-post-self-insertion-hook ()
-  "Ensure order of electric functions in `post-self-insertion-hook'.
-
-Hooks in this variable interact in non-trivial ways, so a
-relative order must be maintained within it."
-  (setq-default post-self-insert-hook
-                (sort (default-value 'post-self-insert-hook)
-                      #'(lambda (fn1 fn2)
-                          (< (or (if (symbolp fn1) (get fn1 'priority)) 0)
-                             (or (if (symbolp fn2) (get fn2 'priority)) 0))))))
-
 ;;; Electric indentation.
 
 ;; Autoloading variables is generally undesirable, but major modes
@@ -297,8 +286,6 @@ electric-indent-post-self-insert-function
                 (indent-according-to-mode)
               (error (throw 'indent-error nil)))))))))
 
-(put 'electric-indent-post-self-insert-function 'priority  60)
-
 (defun electric-indent-just-newline (arg)
   "Insert just a newline, without any auto-indentation."
   (interactive "*P")
@@ -341,8 +328,8 @@ electric-indent-mode
         (remove-hook 'post-self-insert-hook
                      #'electric-indent-post-self-insert-function))
     (add-hook 'post-self-insert-hook
-              #'electric-indent-post-self-insert-function)
-    (electric--sort-post-self-insertion-hook)))
+              #'electric-indent-post-self-insert-function
+              60)))
 
 ;;;###autoload
 (define-minor-mode electric-indent-local-mode
@@ -472,8 +459,6 @@ electric-layout-post-self-insert-function-1
               ('after-stay (save-excursion (funcall nl-after)))
               ('around (funcall nl-before) (funcall nl-after))))))))
 
-(put 'electric-layout-post-self-insert-function 'priority  40)
-
 ;;;###autoload
 (define-minor-mode electric-layout-mode
   "Automatically insert newlines around some chars.
@@ -482,8 +467,8 @@ electric-layout-mode
   :global t :group 'electricity
   (cond (electric-layout-mode
          (add-hook 'post-self-insert-hook
-                   #'electric-layout-post-self-insert-function)
-         (electric--sort-post-self-insertion-hook))
+                   #'electric-layout-post-self-insert-function
+                   40))
         (t
          (remove-hook 'post-self-insert-hook
                       #'electric-layout-post-self-insert-function))))
@@ -623,8 +608,6 @@ electric-quote-post-self-insert-function
                     (replace-match (string q>>))
                     (setq last-command-event q>>))))))))))
 
-(put 'electric-quote-post-self-insert-function 'priority 10)
-
 ;;;###autoload
 (define-minor-mode electric-quote-mode
   "Toggle on-the-fly requoting (Electric Quote mode).
@@ -651,8 +634,8 @@ electric-quote-mode
         (remove-hook 'post-self-insert-hook
                      #'electric-quote-post-self-insert-function))
     (add-hook 'post-self-insert-hook
-              #'electric-quote-post-self-insert-function)
-    (electric--sort-post-self-insertion-hook)))
+              #'electric-quote-post-self-insert-function
+              10)))
 
 ;;;###autoload
 (define-minor-mode electric-quote-local-mode
diff --git a/lisp/emacs-lisp/syntax.el b/lisp/emacs-lisp/syntax.el
index d09d6c1225..d3cb04d1ca 100644
--- a/lisp/emacs-lisp/syntax.el
+++ b/lisp/emacs-lisp/syntax.el
@@ -298,7 +298,7 @@ syntax-propertize
         ;; between syntax-ppss and syntax-propertize, we also have to make
         ;; sure the flush function is installed here (bug#29767).
         (add-hook 'before-change-functions
-                 #'syntax-ppss-flush-cache t t))
+                 #'syntax-ppss-flush-cache 99 t))
       (save-excursion
         (with-silent-modifications
           (make-local-variable 'syntax-propertize--done) ;Just in case!
@@ -429,7 +430,7 @@ syntax-ppss-flush-cache
        ;; Unregister if there's no cache left.  Sadly this doesn't work
        ;; because `before-change-functions' is temporarily bound to nil here.
        ;; (unless cache
-       ;;   (remove-hook 'before-change-functions 'syntax-ppss-flush-cache t))
+       ;;   (remove-hook 'before-change-functions #'syntax-ppss-flush-cache t))
        (setcar cell last)
        (setcdr cell cache)))
     ))
@@ -533,13 +534,14 @@ syntax-ppss
 
              ;; Setup the before-change function if necessary.
              (unless (or ppss-cache ppss-last)
-                ;; We should be either the very last function on
-                ;; before-change-functions or the very first on
-                ;; after-change-functions.
                 ;; Note: combine-change-calls-1 needs to be kept in sync
                 ;; with this!
                (add-hook 'before-change-functions
-                         'syntax-ppss-flush-cache t t))
+                         #'syntax-ppss-flush-cache
+                          ;; We should be either the very last function on
+                          ;; before-change-functions or the very first on
+                          ;; after-change-functions.
+                          99 t))
 
              ;; Use the best of OLD-POS and CACHE.
              (if (or (not old-pos) (< old-pos pt-min))
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index ea865e0b0f..2aef071cf8 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -653,6 +653,8 @@ c-basic-common-init
     (make-local-hook 'after-change-functions))
   (add-hook 'before-change-functions 'c-before-change nil t)
   (setq c-just-done-before-change nil)
+  ;; FIXME: We should use the new `depth' arg in Emacs-27, e.g. a depth of -10
+  ;; would do since font-lock uses a(n implicit) depth of 0.
   (add-hook 'after-change-functions 'c-after-change nil t)
   (when (boundp 'font-lock-extend-after-change-region-function)
     (set (make-local-variable 'font-lock-extend-after-change-region-function)
diff --git a/lisp/subr.el b/lisp/subr.el
index be21dc67a0..e4bf4998ef 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1604,12 +1604,20 @@ 'user-original-login-name
 
 ;;;; Hook manipulation functions.
 
-(defun add-hook (hook function &optional append local)
+(defun add-hook (hook function &optional depth local)
   "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 place where the function is added depends on the DEPTH
+parameter.  DEPTH defaults to 0.  By convention, it should be
+a number between -100 and 100 where 100 means that the function
+should be at the very end of the list, whereas -100 means that
+the function should always come first.  When two functions have
+the same depth, the new one gets added after the old one if
+depth is strictly positive and before otherwise.
+
+For backward compatibility reasons, a symbol other than nil is
+interpreted as a DEPTH of 90.
 
 The optional fourth argument, LOCAL, if non-nil, says to modify
 the hook's buffer-local value rather than its global value.
@@ -1622,6 +1630,7 @@ add-hook
 function, it is changed to a list of functions."
   (or (boundp hook) (set hook nil))
   (or (default-boundp hook) (set-default hook nil))
+  (unless (numberp depth) (setq depth (if depth 90 0)))
   (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
@@ -1634,12 +1643,25 @@ add-hook
       (setq hook-value (list hook-value)))
     ;; Do the actual addition if necessary
     (unless (member function hook-value)
-      (when (stringp function)
+      (when (stringp function)          ;FIXME: Why?
        (setq function (purecopy function)))
+      (when (or (get hook 'hook--depth-alist) (not (zerop depth)))
+        ;; Note: The main purpose of the above `when' test is to avoid running
+        ;; this `setf' before `gv' is loaded during bootstrap.
+        (setf (alist-get function (get hook 'hook--depth-alist)
+                         0 'remove #'equal)
+              depth))
       (setq hook-value
-           (if append
+           (if (< 0 depth)
                (append hook-value (list function))
-             (cons function hook-value))))
+             (cons function hook-value)))
+      (let ((depth-alist (get hook 'hook--depth-alist)))
+        (when depth-alist
+          (setq hook-value
+                (sort (if (< 0 depth) hook-value (copy-sequence hook-value))
+                      (lambda (f1 f2)
+                        (< (alist-get f1 depth-alist 0 nil #'equal)
+                           (alist-get f2 depth-alist 0 nil #'equal))))))))
     ;; Set the actual variable
     (if local
        (progn
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index c458eef2f9..d832958363 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -373,5 +373,31 @@ subr-test--frames-1
   (should (equal (flatten-tree '(1 ("foo" "bar") 2))
                  '(1 "foo" "bar" 2))))
 
+(defvar subr-tests--hook nil)
+
+(ert-deftest subr-tests-add-hook-depth ()
+  "Test the `depth' arg of `add-hook'."
+  (setq-default subr-tests--hook nil)
+  (add-hook 'subr-tests--hook 'f1)
+  (add-hook 'subr-tests--hook 'f2)
+  (should (equal subr-tests--hook '(f2 f1)))
+  (add-hook 'subr-tests--hook 'f3 t)
+  (should (equal subr-tests--hook '(f2 f1 f3)))
+  (add-hook 'subr-tests--hook 'f4 50)
+  (should (equal subr-tests--hook '(f2 f1 f4 f3)))
+  (add-hook 'subr-tests--hook 'f5 -50)
+  (should (equal subr-tests--hook '(f5 f2 f1 f4 f3)))
+  (add-hook 'subr-tests--hook 'f6)
+  (should (equal subr-tests--hook '(f5 f6 f2 f1 f4 f3)))
+  ;; Make sure `t' is equivalent to 90.
+  (add-hook 'subr-tests--hook 'f7 90)
+  (add-hook 'subr-tests--hook 'f8 t)
+  (should (equal subr-tests--hook '(f5 f6 f2 f1 f4 f3 f7 f8)))
+  ;; Make sue `nil' is equivalent to 0.
+  (add-hook 'subr-tests--hook 'f9 0)
+  (add-hook 'subr-tests--hook 'f10)
+  (should (equal subr-tests--hook '(f5 f6 f10 f9 f2 f1 f4 f3 f7 f8)))
+  )
+
 (provide 'subr-tests)
 ;;; subr-tests.el ends here