From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#35508: 27.0.50; Fine-ordering of functions on hooks Date: Wed, 08 May 2019 14:32:43 -0400 Message-ID: References: <831s1iqclm.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="166784"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: 35508@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed May 08 20:51:28 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hORf5-000hG1-8S for geb-bug-gnu-emacs@m.gmane.org; Wed, 08 May 2019 20:51:27 +0200 Original-Received: from localhost ([127.0.0.1]:41738 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hORf1-0000Te-2i for geb-bug-gnu-emacs@m.gmane.org; Wed, 08 May 2019 14:51:25 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:56002) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hORes-0000TJ-8v for bug-gnu-emacs@gnu.org; Wed, 08 May 2019 14:51:16 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hORen-0008Rp-3P for bug-gnu-emacs@gnu.org; Wed, 08 May 2019 14:51:12 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:48703) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hOReg-0008KL-IP for bug-gnu-emacs@gnu.org; Wed, 08 May 2019 14:51:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hOReg-00017B-DU for bug-gnu-emacs@gnu.org; Wed, 08 May 2019 14:51:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 08 May 2019 18:51:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 35508 X-GNU-PR-Package: emacs Original-Received: via spool by 35508-submit@debbugs.gnu.org id=B35508.15573414304242 (code B ref 35508); Wed, 08 May 2019 18:51:02 +0000 Original-Received: (at 35508) by debbugs.gnu.org; 8 May 2019 18:50:30 +0000 Original-Received: from localhost ([127.0.0.1]:34012 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hORe9-00016L-5g for submit@debbugs.gnu.org; Wed, 08 May 2019 14:50:30 -0400 Original-Received: from mail.iro.umontreal.ca ([132.204.27.60]:11064 helo=mail01.iro.umontreal.ca) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hORe7-000168-1Y for 35508@debbugs.gnu.org; Wed, 08 May 2019 14:50:27 -0400 Original-Received: from mail01.iro.umontreal.ca (mail01.iro.umontreal.ca [127.0.0.1]) by mail01.iro.umontreal.ca (Postfix) with ESMTP id 23CC38943CDC for <35508@debbugs.gnu.org>; Wed, 8 May 2019 14:33:10 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; h=content-type:content-type:mime-version:user-agent:in-reply-to :date:date:references:message-id:subject:subject:to:from:from; s=dkim; t=1557340364; x=1558204365; bh=37EiSvg+de24e4xWYltqogZq g2Ho/sM5sOvjCA3APOQ=; b=hOjfzp8BGXxZT9gjf3xsrPatUzsl7ivdQLI+Hfot bVFgNkKV5hFfz9yf5h6n3wnzoAtrrXrXcGsh1HYZTy4GdMZvGHF4E3mtOTHd9wDW rqLEpLusByfTiV+ZXSIxAR3ch30qd8nGKN0l363xzW6Xg1kQB87iz2et67HsI5lf WN3x87hxN6gXMutVSboNmURt7baSUxUqUCuG2A+UcMAomHOannYZCglmh6i1O61T xVdVrLKiYUhzu2FoMo+cWMuIaRvvq9TDLc0EMy6TktvRa8FloJXRIdFx+i+2pncF 9NnkIWoP9xC4NqiJqmSsXBORIcIdGs70GrMDDIxHdi1IPw== X-Virus-Scanned: amavisd-new at iro.umontreal.ca Original-Received: from mail01.iro.umontreal.ca ([127.0.0.1]) by mail01.iro.umontreal.ca (mail01.iro.umontreal.ca [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KPWwhQZ_FKFJ for <35508@debbugs.gnu.org>; Wed, 8 May 2019 14:32:44 -0400 (EDT) Original-Received: from alfajor (modemcable213.149-175-137.mc.videotron.ca [137.175.149.213]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id C388D8943CA2; Wed, 8 May 2019 14:32:43 -0400 (EDT) In-Reply-To: (Stefan Monnier's message of "Wed, 01 May 2019 16:29:00 -0400") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:158957 Archived-At: >>> 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