bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 4402 bytes --]

Modifying a buffer via a keyboard macro does not seem to run the `after-change-functions' buffer modification hook, at least not in a batch `ert' test.

To reproduce, save the code below as test.el and run it with:

emacs -Q -batch -l ert -t test.el -f ert-run-tests-batch-and-exit

;; -*- lexical-binding: t; -*-
(require 'ert)
(require 'ert-x)
(ert-deftest test ()
   (ert-with-test-buffer ()
     (let ((acf 0))
       (add-hook 'after-change-functions
                 (lambda (&rest _) (setq acf (1+ acf)))
                 nil t)
       (execute-kbd-macro (kbd "x"))
       (should (equal (buffer-string) "x"))
       ;; This check fails:
       (should (equal acf 1)))))

If the `execute-kbd-macro' call is replaced with the following it *does* work as expected:

       (ert-simulate-command '(self-insert-command 1 ?x))

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
  of 2022-06-25 built on sprinkles
Repository revision: ec1fffdeca9c87a92b8c35545121b4ee3eec3ece
Repository branch: master
System Description: Ubuntu 20.04.4 LTS

Configured using:
  'configure --with-modules=yes --with-x=yes --with-x-toolkit=gtk3
  --with-xwidgets=yes --with-pgtk --enable-checking=yes,glyphs
  --enable-check-lisp-object-type 'CFLAGS=-O2 -g3''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY
PDUMPER PGTK PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS XIM XWIDGETS GTK3 ZLIB

Important settings:
   value of $LC_MONETARY: en_US.UTF-8
   value of $LC_NUMERIC: en_US.UTF-8
   value of $LC_TIME: root.UTF-8
   value of $LANG: en_US.UTF-8
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   line-number-mode: t
   indent-tabs-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message yank-media rmc dired
dired-loaddefs rfc822 mml url url-proxy url-privacy url-expand
url-methods url-history url-cookie rx generate-lisp-file url-domsuf
url-util url-parse auth-source eieio eieio-core eieio-loaddefs json map
url-vars mm-view mml-smime smime gnutls puny dig mailcap mml-sec
password-cache epa epg rfc6068 epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev easy-mmode nnheader gnus-util
text-property-search time-date seq cl-seq subr-x byte-opt bytecomp
byte-compile cconv range gmm-utils mailheader sendmail derived rfc2047
rfc2045 ietf-drums mm-util cl-macs inline gv pcase mail-prsvr
cl-loaddefs cl-lib mail-utils iso-transl tooltip eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/pgtk-win pgtk-win term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice simple cl-generic indonesian
philippine cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese composite
emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help
abbrev obarray oclosure cl-preloaded button loaddefs faces cus-face
macroexp files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads xwidget-internal dbusbind inotify dynamic-setting
system-font-setting font-render-setting cairo gtk pgtk lcms2 multi-tty
make-network-process emacs)

Memory information:
((conses 16 230999 17175)
  (symbols 48 9468 0)
  (strings 32 20964 1914)
  (string-bytes 1 755167)
  (vectors 16 8263)
  (vector-slots 8 149910 14088)
  (floats 8 26 18)
  (intervals 56 223 0)
  (buffers 992 10))

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Lars Ingebrigtsen]

Richard Hansen <rhansen@rhansen.org> writes:

> Modifying a buffer via a keyboard macro does not seem to run the
> `after-change-functions' buffer modification hook, at least not in a
> batch `ert' test.

I think there might be some confusion about what the current buffer is.
If you eval this in *scratch*:

(with-temp-buffer
  (execute-kbd-macro (kbd "x")))

You'll end up with an "x" in *scratch* buffer, not in the *temp* buffer.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 256 bytes --]

On 2022-06-25 07:51, Lars Ingebrigtsen wrote:
> If you eval this in *scratch*:
> 
> (with-temp-buffer
>    (execute-kbd-macro (kbd "x")))
> 
> You'll end up with an "x" in *scratch* buffer, not in the *temp* buffer.

Is that behavior intentional?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Eli Zaretskii]

> Cc: 56210@debbugs.gnu.org
> Date: Sat, 25 Jun 2022 14:06:27 -0400
> From: Richard Hansen <rhansen@rhansen.org>
> 
> On 2022-06-25 07:51, Lars Ingebrigtsen wrote:
> > If you eval this in *scratch*:
> > 
> > (with-temp-buffer
> >    (execute-kbd-macro (kbd "x")))
> > 
> > You'll end up with an "x" in *scratch* buffer, not in the *temp* buffer.
> 
> Is that behavior intentional?

Maybe we should step back and ask what do you think the below do?

  (execute-kbd-macro (kbd "x"))

What is, in your mental model, the effect of the above in Emacs?

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 1786 bytes --]

On 2022-06-25 14:56, Eli Zaretskii wrote:
> Maybe we should step back and ask what do you think the below do?
> 
>    (execute-kbd-macro (kbd "x"))
> 
> What is, in your mental model, the effect of the above in Emacs?

I would expect that to be roughly equivalent to typing "x" on the physical keyboard, which would insert an event into some sort of event queue.  (I'm not familiar enough with Emacs to know whether that queue is per-buffer, per-window, per-frame, per-terminal, per-process, or something else.)  Under normal circumstances, the event would be dispatched to the buffer associated with the `selected-window' of the `selected-frame', and insert an "x".

I think my understanding of "current buffer" is incorrect.  My current mental model of `with-current-buffer' is that it behaves as if the window's associated buffer is briefly switched to the specified buffer (like "C-x b" does) except it doesn't actually go to the expense of rendering the other buffer in the window (or update the toolbar, etc.).  Looking at the documentation of `set-window-buffer', that mental model appears to be wrong.

But if my mental model is wrong, why does my original code insert "x" into the temporary buffer?  It is not due to something specific to `ert-with-test-buffer', because if I evaluate the following in *scratch* then I see the same behavior that Lars saw with `with-temp-buffer' (the "x" is inserted into *scratch*):

     (ert-with-test-buffer () (execute-kbd-macro (kbd "x")))

Maybe `ert-run-tests-batch-and-exit' is setting it up to more closely simulate a user interacting with Emacs?  Or maybe the behavior is caused by batch mode?

Regardless, given that the temporary buffer is modified in my original example, shouldn't `after-change-functions' run?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Eli Zaretskii]

> Date: Sat, 25 Jun 2022 16:34:11 -0400
> Cc: larsi@gnus.org, 56210@debbugs.gnu.org
> From: Richard Hansen <rhansen@rhansen.org>
> 
> On 2022-06-25 14:56, Eli Zaretskii wrote:
> > Maybe we should step back and ask what do you think the below do?
> > 
> >    (execute-kbd-macro (kbd "x"))
> > 
> > What is, in your mental model, the effect of the above in Emacs?
> 
> I would expect that to be roughly equivalent to typing "x" on the physical keyboard, which would insert an event into some sort of event queue.  (I'm not familiar enough with Emacs to know whether that queue is per-buffer, per-window, per-frame, per-terminal, per-process, or something else.)  Under normal circumstances, the event would be dispatched to the buffer associated with the `selected-window' of the `selected-frame', and insert an "x".

That is correct.

> I think my understanding of "current buffer" is incorrect.  My current mental model of `with-current-buffer' is that it behaves as if the window's associated buffer is briefly switched to the specified buffer (like "C-x b" does) except it doesn't actually go to the expense of rendering the other buffer in the window (or update the toolbar, etc.).  Looking at the documentation of `set-window-buffer', that mental model appears to be wrong.

Yes, with-temp-buffer doesn't associate the temporary buffer with the
selected window.  And execute-kbd-macro starts a command loop, which
always makes sure the selected-window's buffer is current.

> But if my mental model is wrong, why does my original code insert "x" into the temporary buffer? It is not due to something specific to `ert-with-test-buffer', because if I evaluate the following in *scratch* then I see the same behavior that Lars saw with `with-temp-buffer' (the "x" is inserted into *scratch*):
> 
>      (ert-with-test-buffer () (execute-kbd-macro (kbd "x")))
> 
> Maybe `ert-run-tests-batch-and-exit' is setting it up to more closely simulate a user interacting with Emacs?  Or maybe the behavior is caused by batch mode?

Could be, you will have to investigate what ERT does in those cases.
Its macro complexity is above my pay grade.

> Regardless, given that the temporary buffer is modified in my original example, shouldn't `after-change-functions' run?

Is the temporary buffer indeed modified, and is the hook set in that
buffer?

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 1515 bytes --]

On 2022-06-26 04:10, Eli Zaretskii wrote:
>> Regardless, given that the temporary buffer is modified in my
>> original example, shouldn't `after-change-functions' run?
> 
> Is the temporary buffer indeed modified, and is the hook set in that
> buffer?

Yes on both accounts.  See updated test below.

I changed the test to switch to the temporary buffer in the selected window and it now reliably inserts "x" into the temporary buffer.  It still doesn't run the `after-change-functions' hook, however.  Updated test code:

;; -*- lexical-binding: t; -*-
(require 'ert)
(require 'ert-x)
(defvar-local acf 0)
(defun acf (&rest _) (setq-local acf (1+ acf)))
(ert-deftest test ()
   (ert-with-test-buffer ()
     (let ((b (current-buffer)))
       (save-window-excursion
         (with-current-buffer-window b
             `(display-buffer-below-selected
               (body-function
                . ,(lambda (window)
                     (select-window window t)
                     (should (eq (current-buffer) b))
                     (setq-local acf 0)
                     ;; Note that LOCAL is t:
                     (add-hook 'after-change-functions #'acf nil t)
                     (should (memq #'acf after-change-functions))
                     (execute-kbd-macro (kbd "x"))
                     (should (equal (buffer-string) "x"))
                     ;; The above checks pass, this check fails:
                     (should (equal acf 1)))))
             nil)))))

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Lars Ingebrigtsen]

Richard Hansen <rhansen@rhansen.org> writes:

> I changed the test to switch to the temporary buffer in the selected
> window and it now reliably inserts "x" into the temporary buffer.  It
> still doesn't run the `after-change-functions' hook, however.  Updated
> test code:

It's pretty mysterious -- but does the same thing happen if you execute
a keyboard macro interactively?  Or if you run the same code outside of
an ert context?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 1541 bytes --]

On 2022-06-27 03:50, Lars Ingebrigtsen wrote:
> does the same thing happen if you execute a keyboard macro 
> interactively?

That works.  Evaluating the following in *scratch* does not signal an error:

     (let* ((acf 0)
            (incacf (lambda (&rest _) (setq acf (1+ acf)))))
       (add-hook 'after-change-functions incacf nil t)
       (unwind-protect
           (execute-kbd-macro (kbd "x"))
         (remove-hook 'after-change-functions incacf t))
       (when (/= acf 1) (error "a-c-f didn't run")))

The above code shows that the hook runs synchronously as expected.  (The hook runs before `execute-kbd-macro' returns, not after some idle time or when execution returns to the main command loop.)

> Or if you run the same code outside of an ert context?
It does not work.  Evaluating the following in *scratch* signals an error:

     (save-window-excursion
       (with-temp-buffer
         (let ((b (current-buffer)))
           (with-current-buffer-window b
               `(display-buffer-below-selected
                 (body-function
                  . ,(lambda (window)
                       (select-window window t)
                       (let ((acf 0))
                         (add-hook 'after-change-functions
                                   (lambda (&rest _) (setq acf (1+ acf)))
                                   nil t)
                         (execute-kbd-macro (kbd "x"))
                         (when (/= acf 1) (error "a-c-f didn't run"))))))
               nil))))

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Eli Zaretskii]

> Date: Sun, 26 Jun 2022 23:21:24 -0400
> Cc: larsi@gnus.org, 56210@debbugs.gnu.org
> From: Richard Hansen <rhansen@rhansen.org>
> 
> > Is the temporary buffer indeed modified, and is the hook set in that
> > buffer?
> 
> Yes on both accounts.  See updated test below.
> 
> I changed the test to switch to the temporary buffer in the selected window and it now reliably inserts "x" into the temporary buffer.  It still doesn't run the `after-change-functions' hook, however.  Updated test code:
> 
> ;; -*- lexical-binding: t; -*-
> (require 'ert)
> (require 'ert-x)
> (defvar-local acf 0)
> (defun acf (&rest _) (setq-local acf (1+ acf)))
> (ert-deftest test ()
>    (ert-with-test-buffer ()
>      (let ((b (current-buffer)))
>        (save-window-excursion
>          (with-current-buffer-window b
>              `(display-buffer-below-selected
>                (body-function
>                 . ,(lambda (window)
>                      (select-window window t)
>                      (should (eq (current-buffer) b))
>                      (setq-local acf 0)
>                      ;; Note that LOCAL is t:
>                      (add-hook 'after-change-functions #'acf nil t)
>                      (should (memq #'acf after-change-functions))
>                      (execute-kbd-macro (kbd "x"))
>                      (should (equal (buffer-string) "x"))
>                      ;; The above checks pass, this check fails:
>                      (should (equal acf 1)))))
>              nil)))))

Your test calls with-current-buffer-window, which calls
temp-buffer-window-setup, which inhibits modification hooks:

  (defun temp-buffer-window-setup (buffer-or-name)
    "Set up temporary buffer specified by BUFFER-OR-NAME.
  Return the buffer."
    (let ((old-dir default-directory)
	  (buffer (get-buffer-create buffer-or-name)))
      (with-current-buffer buffer
	(kill-all-local-variables)
	(setq default-directory old-dir)
	(delete-all-overlays)
	(setq buffer-read-only nil)
	(setq buffer-file-name nil)
	(setq buffer-undo-list t)
	(let ((inhibit-read-only t)
	      (inhibit-modification-hooks t))  <<<<<<<<<<<<<<<<<<<<<<
	  (erase-buffer)
	  (run-hooks 'temp-buffer-window-setup-hook))
	;; Return the buffer.
	buffer)))

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 566 bytes --]

On 6/27/22 12:54, Eli Zaretskii wrote:
> Your test calls with-current-buffer-window, which calls
> temp-buffer-window-setup, which inhibits modification hooks:

Yup, that's it!  If I set `inhibit-modification-hooks' back to nil then it works.  Thank you.

I thought I had a `(should (null inhibit-modification-hooks))` at some point in the past to rule that out, but I must not have had that inside the `with-current-buffer-window'.

Perhaps the documentation should be updated to indicate that the modification hooks are inhibited?  I can cook up a patch.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Eli Zaretskii]

> Date: Mon, 27 Jun 2022 13:07:44 -0400
> Cc: larsi@gnus.org, 56210@debbugs.gnu.org
> From: Richard Hansen <rhansen@rhansen.org>
> 
> > Your test calls with-current-buffer-window, which calls
> > temp-buffer-window-setup, which inhibits modification hooks:
> 
> Yup, that's it!  If I set `inhibit-modification-hooks' back to nil then it works.  Thank you.
> 
> I thought I had a `(should (null inhibit-modification-hooks))` at some point in the past to rule that out, but I must not have had that inside the `with-current-buffer-window'.
> 
> Perhaps the documentation should be updated to indicate that the modification hooks are inhibited?  I can cook up a patch.

Is it really a good idea?  In general, all the 'with-SOMETHING' macros
are likely to inhibit modification hooks, since they erase the
temporary buffers high and low.  We'd need to stick these factoids in
umpteen APIs, and we'll still forget some.  E.g., in this case, even
if temp-buffer-window-setup was documented to inhibit modification
hooks. how would you know it is relevant to your case -- it is not
explicitly called anywhere.

I think the rule of thumb should be to assume that any temporary
buffer has these kinds of hooks disabled, and if one needs them
enabled, one should enable them explicitly.

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 231 bytes --]

On 6/27/22 13:17, Eli Zaretskii wrote:
> I think the rule of thumb should be to assume that any temporary
> buffer has these kinds of hooks disabled, and if one needs them
> enabled, one should enable them explicitly.

SGTM.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 702 bytes --]

On 6/27/22 13:07, Richard Hansen wrote:
> On 6/27/22 12:54, Eli Zaretskii wrote:
>> Your test calls with-current-buffer-window, which calls 
>> temp-buffer-window-setup, which inhibits modification hooks:
> 
> Yup, that's it!  If I set `inhibit-modification-hooks' back to nil 
> then it works.  Thank you.

However, that doesn't explain why the original test code fails.  If you add a `(should (null inhibit-modification-hooks))` just before the call to `execute-kbd-macro' it will not fail there.

But that original test code is incorrect anyway (the selected window is not explicitly switched to the temporary buffer before running the keyboard macro), so it doesn't really matter to me.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> I thought I had a `(should (null inhibit-modification-hooks))` at
>> some point in the past to rule that out, but I must not have had
>> that inside the `with-current-buffer-window'.
>> 
>> Perhaps the documentation should be updated to indicate that the
>> modification hooks are inhibited?  I can cook up a patch.
>
> Is it really a good idea?  In general, all the 'with-SOMETHING' macros
> are likely to inhibit modification hooks, since they erase the
> temporary buffers high and low.

It's really surprising that with-current-buffer-window does this -- it
sounds like a general variation on with-selected-window, but it's not:
It's meant to be used to pop up help buffers, and this should be
documented.

with-current-buffer/with-selected-window etc do not inhibit any hooks in
the buffers them handle.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Richard Hansen <rhansen@rhansen.org>,  56210@debbugs.gnu.org
> Date: Tue, 28 Jun 2022 13:31:23 +0200
> 
> It's really surprising that with-current-buffer-window does this -- it
> sounds like a general variation on with-selected-window, but it's not:
> It's meant to be used to pop up help buffers, and this should be
> documented.

I didn't write that code, and have no sentiments for it (although I
generally assume that all the temporary buffers disable such hooks for
performance reasons).  I just explained what Richard saw, and it took
me some non-trivial time.

Maybe we should modify with-current-buffer-window to not have such
global effects on the body?

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I didn't write that code, and have no sentiments for it (although I
> generally assume that all the temporary buffers disable such hooks for
> performance reasons).  I just explained what Richard saw, and it took
> me some non-trivial time.
>
> Maybe we should modify with-current-buffer-window to not have such
> global effects on the body?

I think that would be good, but somebody should investigate why whoever
added that binding in temp-buffer-window-setup did it in the first
place.  Perhaps there's some obscure reason why it makes sense -- it's
not at all obvious to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#56210: 29.0.50; Keyboard macros do not trigger after-change-functions

[Richard Hansen]

[-- Attachment #1.1: Type: text/plain, Size: 3205 bytes --]

On 2022-06-27 12:54, Eli Zaretskii wrote:
>> I changed the test to switch to the temporary buffer in the 
>> selected window and it now reliably inserts "x" into the temporary 
>> buffer.  It still doesn't run the `after-change-functions' hook, 
>> however.  Updated test code:
>>
>> ;; -*- lexical-binding: t; -*-
>> (require 'ert)
>> (require 'ert-x)
>> (defvar-local acf 0)
>> (defun acf (&rest _) (setq-local acf (1+ acf)))
>> (ert-deftest test ()
>>     (ert-with-test-buffer ()
>>       (let ((b (current-buffer)))
>>         (save-window-excursion
>>           (with-current-buffer-window b
>>               `(display-buffer-below-selected
>>                 (body-function
>>                  . ,(lambda (window)
>>                       (select-window window t)
>>                       (should (eq (current-buffer) b))
>>                       (setq-local acf 0)
>>                       ;; Note that LOCAL is t:
>>                       (add-hook 'after-change-functions #'acf nil t)
>>                       (should (memq #'acf after-change-functions))
>>                       (execute-kbd-macro (kbd "x"))
>>                       (should (equal (buffer-string) "x"))
>>                       ;; The above checks pass, this check fails:
>>                       (should (equal acf 1)))))
>>               nil)))))
> 
> Your test calls with-current-buffer-window, which calls 
> temp-buffer-window-setup, which inhibits modification hooks:

I looked into this a bit more.  It turns out that `inhibit-modification-hooks' is actually nil in the body of `with-current-buffer-window', as expected.  It is t in the above test code because of `display-buffer-below-selected', which calls `window--display-buffer', which does the following:

     (when (functionp (cdr (assq 'body-function alist)))
       (let ((inhibit-read-only t)
             (inhibit-modification-hooks t))
         (funcall (cdr (assq 'body-function alist)) window)))

The `body-function' action alist entry was added in Emacs 28.1 by commit 3273e2ace78 [1] for bug#39822 [2].  Presumably the modification hooks are inhibited during the execution of `body-function' because `body-function' was added to replace the (now-deprecated) `with-displayed-buffer-window' macro, which inhibits them.  The `with-displayed-buffer-window' macro has inhibited them ever since the macro was first added to Emacs 25.1 by commit f0f70ec0bc5 [3] for bug#17809 [4].

I'm guessing that `body-function' is only intended for initial buffer set-up, thus modification hooks are inhibited.  However, the body of `with-current-buffer-window' is also only intended for initial buffer set-up (it actually runs before `body-function' and before the buffer is displayed), so the inconsistency is unexpected and awkward.  Either both should inhibit the modification hooks, or neither should.


[1] https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3273e2ace788a58bef77cef936021d151815ea94
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=39822
[3] https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=f0f70ec0bc55e452ea29b5cf3f532740966b0192
[4] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17809

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]