unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#35508: 27.0.50; Fine-ordering of functions on hooks
@ 2019-04-30 20:37 Stefan Monnier
  2019-04-30 21:37 ` Drew Adams
  2019-05-01 18:00 ` Eli Zaretskii
  0 siblings, 2 replies; 11+ messages in thread
From: Stefan Monnier @ 2019-04-30 20:37 UTC (permalink / raw)
  To: 35508

Package: Emacs
Version: 27.0.50


Occasionally it's important to control the relative ordering of
functions on hooks.  It's usually a bad idea, but sometimes alternatives
are worse.  When such needs show up, packages usually resort to ugly
tricks such as using some other hook (e.g. post-command-hook) to try and
detect when the ordering was broken and re-impose the desired ordering
by hand.

An example I saw recently is in `c-after-font-lock-init`.

A few months ago the idea came up again (tho I can't remember in
which context), which finally motivated me to look at this.

So, I suggest the patch below which generalizes the `append` argument of
`add-hook` to a `depth` argument similar to the `depth` used in
`add-function`.

Any objection/comment?


        Stefan


diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
index 97e9be9918..9d98cf64fd 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).
+
+But 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 f6676e0e0b..aee5d7c91a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1555,6 +1555,10 @@ never worked reliably, and now causes an error.
 \f
 * Lisp Changes in Emacs 27.1
 
+** The 'append' arg of 'add-hook' is generalized to a finer notion of 'depth'
+This allows to more precisely control the ordering of functions,
+as was already possible in 'add-function'.
+
 ** 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..3caceee279 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/subr.el b/lisp/subr.el
index f68f9dd419..f11cbaabdc 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1602,12 +1602,20 @@ 'user-original-login-name
 \f
 ;;;; 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, 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.
@@ -1620,6 +1628,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
@@ -1632,12 +1641,22 @@ 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)))
+      (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..36470d0650 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -373,5 +373,25 @@ 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)))
+  (add-hook 'subr-tests--hook 'f7 t)
+  (should (equal subr-tests--hook '(f5 f6 f2 f1 f4 f3 f7)))
+  )
+
 (provide 'subr-tests)
 ;;; subr-tests.el ends here





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-04-30 20:37 bug#35508: 27.0.50; Fine-ordering of functions on hooks Stefan Monnier
@ 2019-04-30 21:37 ` Drew Adams
  2019-04-30 22:31   ` Stefan Monnier
  2019-05-01 18:00 ` Eli Zaretskii
  1 sibling, 1 reply; 11+ messages in thread
From: Drew Adams @ 2019-04-30 21:37 UTC (permalink / raw)
  To: Stefan Monnier, 35508

> +But 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.

Commenting on just this part.  If by "default" you mean
that providing nil or no DEPTH arg has the same effect
as providing a DEPTH of 0, then I don't think that
defaulting to 0 is backward-compatible.

Currently the default behavior is to prepend.
Presumably 0 does not mean prepend but insert ~halfway.
And currently if you provide 0 as the APPEND arg you
get the effect of 100, not 0 (IIUC).

I think that if you add this feature the doc should say
something about an expected/possible use case - when
"it is important to control the relative ordering".
(I'm curious, for example, never having come across the
need.)

The way you describe it makes it seem like it will be
common to supply a specific number rather than just
appending (i.e. at the end).  Will it be common to need
a number other than 100?

BTW, do similar considerations not apply to advice?
There we have before, after, and replace, essentially.
Why don't we need -100 to 100 for ordering there?
(Again, just curious.)





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-04-30 21:37 ` Drew Adams
@ 2019-04-30 22:31   ` Stefan Monnier
  0 siblings, 0 replies; 11+ messages in thread
From: Stefan Monnier @ 2019-04-30 22:31 UTC (permalink / raw)
  To: Drew Adams; +Cc: 35508

> (I'm curious, for example, never having come across the need.)

Just look at the patch, it includes an example.

> The way you describe it makes it seem like it will be
> common to supply a specific number rather than just
> appending (i.e. at the end).  Will it be common to need
> a number other than 100?

100 (resp -100) should almost never be used, since they basically mean that
noone should ever have the right to come after (resp before), which is
rather presumptuous.

> BTW, do similar considerations not apply to advice?

As the NEWS entry mentions, this semantics mimics that already existing
in add-function (and hence advice-add).


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-04-30 20:37 bug#35508: 27.0.50; Fine-ordering of functions on hooks Stefan Monnier
  2019-04-30 21:37 ` Drew Adams
@ 2019-05-01 18:00 ` Eli Zaretskii
  2019-05-01 20:29   ` Stefan Monnier
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2019-05-01 18:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 35508

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Tue, 30 Apr 2019 16:37:08 -0400
> 
> 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?  I agree that it's usually
a bad idea, so maybe we should resist the temptation.  If the worse
comes to worst, a Lisp program could concoct the entire hook list in
any order it sees fit, right?

> +The place where the function is added depends on the DEPTH
> +parameter.  DEPTH defaults to 0.

So from now on, omitting DEPTH will not necessarily put the function
at the beginning of the hook list?  That's backward-incompatible, no?

In any case, this default is insufficiently tested by the tests you
propose.

>                                         By convention, 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.

So using 100 more than once makes the last one "win"?

> +For backward compatibility reasons, a symbol other than nil is
> +interpreted as a DEPTH of 90.

This is not explicitly tested by the test.

Thanks.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-05-01 18:00 ` Eli Zaretskii
@ 2019-05-01 20:29   ` Stefan Monnier
  2019-05-08 18:32     ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2019-05-01 20:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 35508

>> 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 agree that it's usually a bad idea, so maybe we should resist the
> temptation.

So far people haven't resisted the temptation, but have instead worked
around the lack of direct support for it, either by ad-hoc ways to
detect mis-ordering and re-set the ordering accordingly, or by hoping
for the best.

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

>> +The place where the function is added depends on the DEPTH
>> +parameter.  DEPTH defaults to 0.
>
> So from now on, omitting DEPTH will not necessarily put the function
> at the beginning of the hook list?

Indeed.  Same for `t` not always going to the very end.

> That's backward-incompatible, no?

Sure.

> In any case, this default is insufficiently tested by the tests
> you propose.

What other tests do you suggest?

> 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 can a test to try and check that `90` corresponds to `t`, if you want,
although this property is trivially verified by looking at the code.
[ I tend to prefer tests that try and catch tricky interactions rather
  than straightforward bases cases.  E.g. the tests I included are the
  ones that failed during development ;-)  ]


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-05-01 20:29   ` Stefan Monnier
@ 2019-05-08 18:32     ` Stefan Monnier
  2019-05-11 12:05       ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2019-05-08 18:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 35508

>>> 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.
 \f
 * 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.
 \f
 * 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
 \f
 ;;;; 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





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-05-08 18:32     ` Stefan Monnier
@ 2019-05-11 12:05       ` Eli Zaretskii
  2019-05-11 13:26         ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2019-05-11 12:05 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 35508

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 35508@debbugs.gnu.org
> Date: Wed, 08 May 2019 14:32:43 -0400
> 
> 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?

Thanks.  Should we perhaps change 100 to 110 and 90 to 100?  And
perhaps not document the 110 value?  Just a thought.

No other objections.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-05-11 12:05       ` Eli Zaretskii
@ 2019-05-11 13:26         ` Stefan Monnier
  2019-05-11 13:54           ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2019-05-11 13:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 35508

>> Other objections?
> Thanks.  Should we perhaps change 100 to 110 and 90 to 100?

You mean make it go https://en.wikipedia.org/wiki/Up_to_eleven ?  ;-)

> And perhaps not document the 110 value?  Just a thought.

I think we do want to document values greater than what `t` does: it can
be important (e.g. for syntax-ppss-flush-cache) to make sure we stay
closer to the end than those hooks appended via `t` for weaker reasons
(e.g. because they don't want to be before some other function, although
they don't really care if they're the very last one or not).

Also I think it's important to use the same convention as for add-function.

But what I wonder is whether we should enforce the convention: currently
we don't in add-function (and in this add-hook patch), so you can use
a depth of 8345 if you feel like: it's really just a convention.

Also, maybe the docs should insist on the fact that 100/-100 should
basically never be used since they imply that you're 100% sure that
noone will ever need to come before/after you, and you can never be sure
100%.


        Stefan






^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-05-11 13:26         ` Stefan Monnier
@ 2019-05-11 13:54           ` Eli Zaretskii
  2019-05-13 13:29             ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2019-05-11 13:54 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 35508

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 35508@debbugs.gnu.org
> Date: Sat, 11 May 2019 09:26:10 -0400
> 
> >> Other objections?
> > Thanks.  Should we perhaps change 100 to 110 and 90 to 100?
> 
> You mean make it go https://en.wikipedia.org/wiki/Up_to_eleven ?  ;-)

Yes.  The idea is that 100 is easier to remember than either 90 or
110.

> > And perhaps not document the 110 value?  Just a thought.
> 
> I think we do want to document values greater than what `t` does: it can
> be important (e.g. for syntax-ppss-flush-cache) to make sure we stay
> closer to the end than those hooks appended via `t` for weaker reasons
> (e.g. because they don't want to be before some other function, although
> they don't really care if they're the very last one or not).
> 
> Also I think it's important to use the same convention as for add-function.

We can always document that in comments.

> But what I wonder is whether we should enforce the convention: currently
> we don't in add-function (and in this add-hook patch), so you can use
> a depth of 8345 if you feel like: it's really just a convention.
> 
> Also, maybe the docs should insist on the fact that 100/-100 should
> basically never be used since they imply that you're 100% sure that
> noone will ever need to come before/after you, and you can never be sure
> 100%.

We could have checkdoc complain about values we don't want to see in
application code.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-05-11 13:54           ` Eli Zaretskii
@ 2019-05-13 13:29             ` Stefan Monnier
  2019-05-29 19:56               ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2019-05-13 13:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 35508

>> >> Other objections?
>> > Thanks.  Should we perhaps change 100 to 110 and 90 to 100?
>> You mean make it go https://en.wikipedia.org/wiki/Up_to_eleven ?  ;-)
> Yes.  The idea is that 100 is easier to remember than either 90 or
> 110.

How 'bout 42, then?

More seriously: since I don't want `t` to be equivalent to the maximum
depth, there are 2 numbers to be "remembered".  In order to follow the
same convention as add-function, I'd really much prefer to keep 100 as
the maximum.  As for the value corresponding to `t`, I'm definitely not
wedded to 90, it could be any value in the ]0,100[ interval.
66?  50?  10?  99?  π?

>> Also I think it's important to use the same convention as for add-function.
> We can always document that in comments.

Will do.

> We could have checkdoc complain about values we don't want to see in
> application code.

I think that'd be more trouble than it's worth.
But I'll warn against using 100 (or -100), in the doc.


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35508: 27.0.50; Fine-ordering of functions on hooks
  2019-05-13 13:29             ` Stefan Monnier
@ 2019-05-29 19:56               ` Stefan Monnier
  0 siblings, 0 replies; 11+ messages in thread
From: Stefan Monnier @ 2019-05-29 19:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 35508-done

> 66?  50?  10?  99?  π?

For lack of a better idea, I kept 90.

> I think that'd be more trouble than it's worth.
> But I'll warn against using 100 (or -100), in the doc.

Done and pushed,


        Stefan






^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-05-29 19:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-30 20:37 bug#35508: 27.0.50; Fine-ordering of functions on hooks Stefan Monnier
2019-04-30 21:37 ` Drew Adams
2019-04-30 22:31   ` Stefan Monnier
2019-05-01 18:00 ` Eli Zaretskii
2019-05-01 20:29   ` Stefan Monnier
2019-05-08 18:32     ` Stefan Monnier
2019-05-11 12:05       ` Eli Zaretskii
2019-05-11 13:26         ` Stefan Monnier
2019-05-11 13:54           ` Eli Zaretskii
2019-05-13 13:29             ` Stefan Monnier
2019-05-29 19:56               ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).