bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions

[Philipp Stephani <p.stephani2@gmail.com>]

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

Philipp Stephani <p.stephani2@gmail.com> schrieb am Mi., 14. Sep. 2016 um
18:54 Uhr:

> Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 31. Okt. 2015 um 14:34 Uhr:
>
> > From: Michael Welsh Duggan <mwd@md5i.com>
> > Date: Sat, 31 Oct 2015 09:07:05 -0400
> >
> > > I've changed the (bobp) to (= point 1).  I'll run it for a couple of
> > > weeks and report back.
> >
> > This change seems to have done the trick.  I have not encountered the
> > error in several days.
>
> Thanks for testing.  I pushed the change, and I'm arking this bug as
> done.
>
>
>
> This change was  reverted in 76ef52267cf887e3e1aa6d25b3b16dd0601dd459.
> It also doesn't seem correct. cursor-sensor--detect is only used in
> pre-redisplay-functions, and the documentation of that variable says:
> "Hook run just before redisplay.
> It is called in each window that is to be redisplayed.  It takes one
> argument,
> which is the window that will be redisplayed.  When run, the
> ‘current-buffer’
> is set to the buffer displayed in that window."
> That means that (bobp) is correct and (= point 1) cannot give a different
> result, unless narrowing is in effect (then only bobp is correct).
> Given that replacing (bobp) with (= point 1) does solve this bug, the
> documentation of pre-redisplay-functions must be incorrect, i.e. the
> current buffer is not the buffer of the window passed as argument. I think
> the only way how this can happen is that a previous entry in
> pre-redisplay-functions has changed the current buffer. Probably the
> implementation of redisplay--pre-redisplay-functions should be changed from
> (with-current-buffer (window-buffer win)
>           (run-hook-with-args 'pre-redisplay-functions win))
> to
> (run-hook-wrapped 'pre-redisplay-functions
>   (lambda (func) (with-current-buffer (window-buffer win)
>      (funcall func win)
>      nil))
> or so.
>

I've attached a patch for this.

[-- Attachment #1.2: Type: text/html, Size: 3500 bytes --]

[-- Attachment #2: 0001-Restore-buffer-in-pre-redisplay-functions.txt --]
[-- Type: text/plain, Size: 3499 bytes --]

From 361a0488a67ffc601e207626b25327bf4868d24a Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Wed, 14 Sep 2016 21:08:07 +0200
Subject: [PATCH] Restore buffer in `pre-redisplay-functions'

Previously, a function in `pre-redisplay-functions' could change the
current buffer, breaking the contract of `pre-redisplay-functions'
that the current buffer is always the buffer displayed in the argument
window.

* lisp/simple.el (redisplay--pre-redisplay-functions): Fix
behavior when a function in `pre-redisplay-functions' changes the
current buffer.

* test/lisp/simple-tests.el
(pre-redisplay-function--current-buffer): Test for fixed behavior.
---
 lisp/simple.el            | 16 ++++++++++------
 test/lisp/simple-tests.el | 28 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 04a525c..9a4690e 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5425,13 +5425,17 @@ pre-redisplay-functions
 is set to the buffer displayed in that window.")
 
 (defun redisplay--pre-redisplay-functions (windows)
+  (cond
+   ((null windows) (setq windows (list (selected-window))))
+   ((nlistp windows) (setq windows (window-list-1 nil nil t))))
   (with-demoted-errors "redisplay--pre-redisplay-functions: %S"
-    (if (null windows)
-        (with-current-buffer (window-buffer (selected-window))
-          (run-hook-with-args 'pre-redisplay-functions (selected-window)))
-      (dolist (win (if (listp windows) windows (window-list-1 nil nil t)))
-        (with-current-buffer (window-buffer win)
-          (run-hook-with-args 'pre-redisplay-functions win))))))
+    (dolist (win windows)
+      (run-hook-wrapped
+       'pre-redisplay-functions
+       (lambda (func)
+         (with-current-buffer (window-buffer win)
+           (funcall func win))
+         nil)))))
 
 (add-function :before pre-redisplay-function
               #'redisplay--pre-redisplay-functions)
diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index d022240..da7a03d 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -374,5 +374,33 @@ simple-test-undo-with-switched-buffer
        (undo)
        (point)))))
 
+\f
+;;; `pre-redisplay-function'
+(ert-deftest pre-redisplay-function--current-buffer ()
+  "Verify that the functions in `pre-redisplay-functions' are called with the correct `current-buffer'.
+See Bug#21730."
+  (with-temp-buffer
+    (let ((buffer-1 (current-buffer)))
+      (with-temp-buffer
+        (let ((buffer-2 (current-buffer))
+              (pre-redisplay-functions pre-redisplay-functions)
+              (hook-1-calls 0)
+              (hook-2-calls 0))
+          (add-hook 'pre-redisplay-functions
+                    (lambda (window)
+                      (should (equal (current-buffer) (window-buffer window)))
+                      (cl-incf hook-1-calls)
+                      (set-buffer buffer-1))
+                    :append)
+          (add-hook 'pre-redisplay-functions
+                    (lambda (window)
+                      (should (equal (current-buffer) (window-buffer window)))
+                      (cl-incf hook-2-calls))
+                    :append)
+          (funcall pre-redisplay-function nil)
+          (should (equal (current-buffer) buffer-2))
+          (should (equal hook-1-calls 1))
+          (should (equal hook-2-calls 1)))))))
+
 (provide 'simple-test)
 ;;; simple-test.el ends here
-- 
2.8.0.rc3.226.g39d4020