From: Philipp Stephani <p.stephani2@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>, 21730@debbugs.gnu.org, mwd@md5i.com
Subject: bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions
Date: Wed, 14 Sep 2016 19:11:57 +0000 [thread overview]
Message-ID: <CAArVCkQdroFVLejs2-Bzc0==bEo2cb2fsPHaGJRdFJD5xhdj-g@mail.gmail.com> (raw)
In-Reply-To: <CAArVCkTA5_hxvxszdYX1QWSoG382zg+mW=4U3uhiXmTBcPCSgw@mail.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
next prev parent reply other threads:[~2016-09-14 19:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-22 4:04 bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions Michael Welsh Duggan
2015-10-22 14:40 ` Eli Zaretskii
2015-10-27 4:19 ` Michael Welsh Duggan
2015-10-31 13:07 ` Michael Welsh Duggan
2015-10-31 13:32 ` Eli Zaretskii
2016-09-14 17:01 ` Philipp Stephani
[not found] ` <CAArVCkTA5_hxvxszdYX1QWSoG382zg+mW=4U3uhiXmTBcPCSgw@mail.gmail.com>
2016-09-14 17:09 ` Eli Zaretskii
2016-09-14 18:43 ` Philipp Stephani
2016-09-14 19:23 ` Eli Zaretskii
2016-09-14 19:48 ` Philipp Stephani
2016-09-15 14:22 ` Eli Zaretskii
2016-09-14 19:11 ` Philipp Stephani [this message]
2016-09-14 19:25 ` Eli Zaretskii
2017-07-25 2:06 ` Sergio Durigan Junior
2017-07-25 14:21 ` Eli Zaretskii
2017-07-25 23:15 ` Sergio Durigan Junior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAArVCkQdroFVLejs2-Bzc0==bEo2cb2fsPHaGJRdFJD5xhdj-g@mail.gmail.com' \
--to=p.stephani2@gmail.com \
--cc=21730@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=mwd@md5i.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).