bug#60333: [PATCH] whitespace: Update bob, eob markers in base and indirect buffers

[Richard Hansen <rhansen@rhansen.org>]

[-- Attachment #1.1.1: Type: text/plain, Size: 1480 bytes --]

Attached patch:

     whitespace: Update bob, eob markers in base and indirect buffers

     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).
     * 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, and use this new function instead of
     `whitespace--update-bob-eob' in `after-change-functions'.

See Bug#46982 for additional context.  This can also be thought of as a 
continuation of the fix for Bug#59618 made in commit 
d76d7a3bebf1ff0b06a38f7f96d316752844ed10.

I'm not sure if this patch belongs on master, on emacs-29, or nowhere:
   * The bug it fixes is very minor, so it might not be worth the added 
complexity.
   * It touches code that has been the source of a few recent bugs, so 
I'm wary of introducing new bugs onto emacs-29.
   * This is a workaround for Bug#46982; it would be better to fix that 
bug instead (although fixing that bug might be infeasible for reasons of 
backwards compatibility).

[-- Attachment #1.1.2: 0001-whitespace-Update-bob-eob-markers-in-base-and-indire.patch --]
[-- Type: text/x-patch, Size: 7944 bytes --]

From eb0caae32d2cb3d8d1d568d1054a33850c368477 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 15 Dec 2022 01:48:06 -0500
Subject: [PATCH] whitespace: Update bob, eob markers in base and indirect
 buffers

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).
* 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, 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 --]