From: Richard Hansen <rhansen@rhansen.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 60333@debbugs.gnu.org
Subject: bug#60333: [PATCH] whitespace: Update bob, eob markers in base and indirect buffers
Date: Thu, 29 Dec 2022 01:54:21 -0500 [thread overview]
Message-ID: <819d60c0-af5d-6936-d7ed-dc8a898adb3c@rhansen.org> (raw)
In-Reply-To: <jwvpmc337nc.fsf-monnier+emacs@gnu.org>
[-- Attachment #1.1.1: Type: text/plain, Size: 699 bytes --]
On 12/28/22 09:51, Stefan Monnier wrote:
>> When a buffer is changed, update `whitespace-bob-marker' and
>> `whitespace-eob-marker' not just in the current buffer, but
>> also in the base buffer and all of its indirect buffers (if
>> any) (Bug#46982).
>
> That doesn't sound right. We should only update them in those indirect buffers
> which also use whitespace eob/bob markers. It should be their individual
> responsibility to register hook functions to update their own markers.
The changelog was worded poorly; it does what you expect it to do.
Attached is an updated patch with hopefully better wording; please let
me know if it still needs improvement.
[-- Attachment #1.1.2: v2-0001-whitespace-Update-bob-eob-markers-in-base-and-ind.patch --]
[-- Type: text/x-patch, Size: 8006 bytes --]
From 1e5efbdac0201aa87873923ba62a605f977b4e5f Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 15 Dec 2022 01:48:06 -0500
Subject: [PATCH v2] whitespace: Update bob, eob markers in base and indirect
buffers
When a buffer is changed, call `whitespace--update-bob-eob'
not just on the current buffer but also on the base buffer and
all of its indirect buffers where applicable (Bug#46982).
* lisp/whitespace.el (whitespace--indirect-buffers,
whitespace--refresh-indirect-buffers,
whitespace-unload-function): Track the relationships between
base buffers and their indirect buffers.
(whitespace--update-bob-eob-all, whitespace-color-on,
whitespace-color-off): Define a new function that calls
`whitespace--update-bob-eob' on the base buffer and all of its
indirect buffers that have `whitespace--update-bob-eob-all' in
their `after-change-functions', and use this new function
instead of `whitespace--update-bob-eob' in
`after-change-functions'.
---
lisp/whitespace.el | 58 +++++++++++++++++++++++++++++++++--
test/lisp/whitespace-tests.el | 35 +++++++++++++++++----
2 files changed, 84 insertions(+), 9 deletions(-)
diff --git a/lisp/whitespace.el b/lisp/whitespace.el
index b747293eb4..bc9d4b7998 100644
--- a/lisp/whitespace.el
+++ b/lisp/whitespace.el
@@ -2104,6 +2104,26 @@ whitespace--clone
(marker-insertion-type whitespace-eob-marker)))))
+(defvar whitespace--indirect-buffers nil
+ "Plist mapping a base buffer to a list of its indirect buffers.
+
+Used to work around Bug#46982.")
+
+
+(defun whitespace--refresh-indirect-buffers ()
+ "Refresh `whitespace--indirect-buffers'.
+
+Used to work around Bug#46982."
+ (setq whitespace--indirect-buffers nil)
+ ;; Keep track of all buffers -- not just those with
+ ;; `whitespace-mode' enabled -- in case `whitespace-mode' is enabled
+ ;; later.
+ (dolist (buf (buffer-list))
+ (let ((base (buffer-base-buffer buf)))
+ (when base
+ (push buf (plist-get whitespace--indirect-buffers base))))))
+
+
(defun whitespace-color-on ()
"Turn on color visualization."
(when (whitespace-style-face-p)
@@ -2118,7 +2138,7 @@ whitespace-color-on
(setq-local whitespace-buffer-changed nil)
(add-hook 'post-command-hook #'whitespace-post-command-hook nil t)
(add-hook 'before-change-functions #'whitespace-buffer-changed nil t)
- (add-hook 'after-change-functions #'whitespace--update-bob-eob
+ (add-hook 'after-change-functions #'whitespace--update-bob-eob-all
;; The -1 ensures that it runs before any
;; `font-lock-mode' hook functions.
-1 t)
@@ -2215,8 +2235,8 @@ whitespace-color-off
(when (whitespace-style-face-p)
(remove-hook 'post-command-hook #'whitespace-post-command-hook t)
(remove-hook 'before-change-functions #'whitespace-buffer-changed t)
- (remove-hook 'after-change-functions #'whitespace--update-bob-eob
- t)
+ (remove-hook 'after-change-functions
+ #'whitespace--update-bob-eob-all t)
(remove-hook 'clone-buffer-hook #'whitespace--clone t)
(remove-hook 'clone-indirect-buffer-hook #'whitespace--clone t)
(font-lock-remove-keywords nil whitespace-font-lock-keywords)
@@ -2401,6 +2421,32 @@ whitespace--variable-watcher
(when whitespace-mode
(font-lock-flush)))))
+(defun whitespace--update-bob-eob-all (&optional beg end &rest _)
+ "Call `whitespace--update-bob-eob' for the base and its indirect buffers.
+
+This function is intended to be used in `after-change-functions'.
+The `whitespace--update-bob-eob' function is only called for a
+buffer if the buffer has this function in its `after-change-functions'
+hook. See `after-change-functions' for the meaning of BEG and
+END."
+ ;; Change hooks do not run for the base buffer when editing an
+ ;; indirect buffer, or for indirect buffers when editing the base
+ ;; buffer, even though the change affects all of them simultaneously
+ ;; (Bug#46982). Work around that limitation by manually updating
+ ;; them all here. `whitespace--update-bob-eob' is idempotent, so if
+ ;; Bug#46982 is fixed this should continue to work correctly (though
+ ;; it will be doing unnecessary work).
+ (let* ((base (or (buffer-base-buffer) (current-buffer)))
+ (indirect (plist-get whitespace--indirect-buffers base)))
+ (dolist (buf (cons base indirect))
+ (with-current-buffer buf
+ (when (memq #'whitespace--update-bob-eob-all
+ after-change-functions)
+ ;; Positions in a base buffer always match positions in
+ ;; indirect buffers (even if narrowing differs) so there is
+ ;; no need to translate BEG or END.
+ (whitespace--update-bob-eob beg end))))))
+
(defun whitespace--update-bob-eob (&optional beg end &rest _)
"Update `whitespace-bob-marker' and `whitespace-eob-marker'.
Also apply `font-lock-multiline' text property. If BEG and END
@@ -2589,6 +2635,10 @@ whitespace-warn-read-only
\f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+(whitespace--refresh-indirect-buffers)
+(add-hook 'buffer-list-update-hook
+ #'whitespace--refresh-indirect-buffers)
+
(defvar whitespace--watched-vars
'(fill-column indent-tabs-mode tab-width whitespace-line-column))
@@ -2605,6 +2655,8 @@ whitespace-unload-function
(dolist (buf (buffer-list))
(set-buffer buf)
(whitespace-mode -1)))
+ (remove-hook 'buffer-list-update-hook
+ #'whitespace--refresh-indirect-buffers)
nil) ; continue standard unloading
diff --git a/test/lisp/whitespace-tests.el b/test/lisp/whitespace-tests.el
index 9a9fb55e4f..513328f5c7 100644
--- a/test/lisp/whitespace-tests.el
+++ b/test/lisp/whitespace-tests.el
@@ -388,12 +388,9 @@ whitespace-tests--indirect-clone-markers
(execute-kbd-macro (kbd "z RET M-< a"))
(whitespace-tests--check-markers indirect 1 8))
(kill-buffer indirect)
- ;; When the buffer was modified above, the new "a" character at
- ;; the beginning moved the base buffer's markers by one. Emacs
- ;; did not run the base buffer's `after-change-functions' after
- ;; the indirect buffer was edited (Bug#46982), so the end result
- ;; is just the shift by one.
- (whitespace-tests--check-markers base 3 5))))
+ ;; The base buffer's markers have also been updated thanks to a
+ ;; workaround for Bug#46982.
+ (whitespace-tests--check-markers base 1 8))))
(ert-deftest whitespace-tests--regular-clone-markers ()
"Test `whitespace--clone' on regular clones."
@@ -411,6 +408,32 @@ whitespace-tests--regular-clone-markers
(kill-buffer clone)
(whitespace-tests--check-markers orig 2 4))))
+(ert-deftest whitespace-tests--indirect-mutual-marker-update ()
+ "Edits should update markers in base and all indirect buffers."
+ (whitespace-tests--with-test-buffer '(face empty)
+ (insert "\nx\n\n")
+ (let* ((indirects (list (clone-indirect-buffer nil nil)
+ (clone-indirect-buffer nil nil)))
+ (bufs (cons (current-buffer) indirects)))
+ (dolist (editbuf bufs)
+ (dolist (buf bufs)
+ (whitespace-tests--check-markers buf 2 4))
+ (ert-with-buffer-selected editbuf
+ (buffer-enable-undo)
+ (undo-boundary)
+ (with-undo-amalgamate
+ (execute-kbd-macro (kbd "z RET M-< a"))))
+ (dolist (buf bufs)
+ (whitespace-tests--check-markers buf 1 8))
+ (ert-with-buffer-selected editbuf
+ (execute-kbd-macro (kbd "C-_")))
+ (dolist (buf bufs)
+ (whitespace-tests--check-markers buf 2 4)))
+ ;; `unwind-protect' is not used to clean up `indirects' because
+ ;; the buffers should only be killed on success.
+ (dolist (buf indirects)
+ (kill-buffer buf)))))
+
(provide 'whitespace-tests)
;;; whitespace-tests.el ends here
--
2.39.0
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2022-12-29 6:54 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-26 6:21 bug#60333: [PATCH] whitespace: Update bob, eob markers in base and indirect buffers Richard Hansen
2022-12-27 6:34 ` Ihor Radchenko
2022-12-27 12:24 ` Eli Zaretskii
2022-12-27 14:50 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-28 13:48 ` Ihor Radchenko
2022-12-28 14:44 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-29 8:11 ` Richard Hansen
2023-01-12 9:33 ` Eli Zaretskii
2023-01-12 16:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-29 11:20 ` Ihor Radchenko
2022-12-29 14:30 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-29 20:45 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-28 14:51 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-29 6:54 ` Richard Hansen [this message]
2023-01-12 9:32 ` Eli Zaretskii
2023-01-12 16:15 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
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=819d60c0-af5d-6936-d7ed-dc8a898adb3c@rhansen.org \
--to=rhansen@rhansen.org \
--cc=60333@debbugs.gnu.org \
--cc=monnier@iro.umontreal.ca \
/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).