From: Richard Hansen <rhansen@rhansen.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>,
Ihor Radchenko <yantar92@posteo.net>
Cc: 59618@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>,
arsen@aarsen.me, stefankangas@gmail.com
Subject: bug#59618: [PATCH] Re: bug#59618: 29.0.50; global-whitespace-mode + org-capture: "Marker does not point anywhere"
Date: Sun, 18 Dec 2022 04:24:27 -0500 [thread overview]
Message-ID: <764d74f1-a1e8-50c7-fd8e-373acced75e5@rhansen.org> (raw)
In-Reply-To: <jwvilidvjnz.fsf-monnier+emacs@gnu.org>
[-- Attachment #1.1.1: Type: text/plain, Size: 171 bytes --]
Attached is a patch that should fix this issue. I have some other
`whitespace-mode' fixes in progress; I'll open new issues (or reply to
existing issues) for those.
[-- Attachment #1.1.2: 0001-whitespace-Avoid-mutating-original-buffer-s-markers-.patch --]
[-- Type: text/x-patch, Size: 6859 bytes --]
From c22833b22cb0a1cb631bfc1fe04881d7ee9bf3d1 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Tue, 13 Dec 2022 01:33:43 -0500
Subject: [PATCH] whitespace: Avoid mutating original buffer's markers in
clones
* lisp/whitespace.el (whitespace--clone): New hook function that is
run after cloning a buffer that copies `whitespace-bob-marker' and
`whitespace-eob-marker' and changes the copies to point to the new
buffer (Bug#59618).
(whitespace-color-on): Register the hook function.
(whitespace-color-off): Unregister the hook function.
---
lisp/whitespace.el | 15 +++++++
test/lisp/whitespace-tests.el | 75 +++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+)
diff --git a/lisp/whitespace.el b/lisp/whitespace.el
index 25ea07e9db..2c1520ec6a 100644
--- a/lisp/whitespace.el
+++ b/lisp/whitespace.el
@@ -2093,6 +2093,17 @@ whitespace-style-face-p
t))
+(defun whitespace--clone ()
+ "Hook function run after `make-indirect-buffer' and `clone-buffer'."
+ (when (whitespace-style-face-p)
+ (setq-local whitespace-bob-marker
+ (copy-marker (marker-position whitespace-bob-marker)
+ (marker-insertion-type whitespace-bob-marker)))
+ (setq-local whitespace-eob-marker
+ (copy-marker (marker-position whitespace-eob-marker)
+ (marker-insertion-type whitespace-eob-marker)))))
+
+
(defun whitespace-color-on ()
"Turn on color visualization."
(when (whitespace-style-face-p)
@@ -2111,6 +2122,8 @@ whitespace-color-on
;; The -1 ensures that it runs before any
;; `font-lock-mode' hook functions.
-1 t)
+ (add-hook 'clone-buffer-hook #'whitespace--clone nil t)
+ (add-hook 'clone-indirect-buffer-hook #'whitespace--clone nil t)
;; Add whitespace-mode color into font lock.
(setq
whitespace-font-lock-keywords
@@ -2204,6 +2217,8 @@ whitespace-color-off
(remove-hook 'before-change-functions #'whitespace-buffer-changed t)
(remove-hook 'after-change-functions #'whitespace--update-bob-eob
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)
(font-lock-flush)))
diff --git a/test/lisp/whitespace-tests.el b/test/lisp/whitespace-tests.el
index fb53543c9e..330fc335ea 100644
--- a/test/lisp/whitespace-tests.el
+++ b/test/lisp/whitespace-tests.el
@@ -42,6 +42,13 @@ whitespace-tests--with-test-buffer
'(whitespace-mode 1))
,@body)))
+(defmacro whitespace--with-buffer-selected (buffer-or-name &rest body)
+ (declare (debug (form body)) (indent 1))
+ `(save-window-excursion
+ (with-current-buffer (or ,buffer-or-name (current-buffer))
+ (with-selected-window (display-buffer (current-buffer))
+ ,@body))))
+
(defun whitespace-tests--faceup (&rest lines)
"Convenience wrapper around `faceup-test-font-lock-buffer'.
Returns non-nil if the concatenated LINES match the current
@@ -327,6 +334,74 @@ whitespace-tests--empty-bob-eob-read-only-buffer
"«:whitespace-empty:\n"
"»")))))
+(ert-deftest whitespace-tests--indirect-clone-breaks-base-markers ()
+ "Specific regression test for Bug#59618."
+ (whitespace-tests--with-test-buffer '(face empty)
+ (insert "\nx\n\n")
+ (let ((base (current-buffer))
+ ;; `unwind-protect' is not used to clean up `indirect'
+ ;; because the buffer should only be killed on success.
+ (indirect (clone-indirect-buffer (buffer-name) nil)))
+ (should (eq (marker-buffer whitespace-bob-marker) base))
+ (should (eq (marker-buffer whitespace-eob-marker) base))
+ (whitespace--with-buffer-selected indirect
+ ;; Mutate the indirect buffer to update its bob/eob markers.
+ (execute-kbd-macro (kbd "z RET M-< a")))
+ ;; With Bug#59618, the above mutation would cause the base
+ ;; buffer's markers to point inside the indirect buffer because
+ ;; the indirect buffer erroneously shared marker objects with
+ ;; the base buffer. Killing the indirect buffer would then
+ ;; invalidate those markers (make them point nowhere).
+ (kill-buffer indirect)
+ (should (eq (marker-buffer whitespace-bob-marker) base))
+ (should (eq (marker-buffer whitespace-eob-marker) base)))))
+
+(defun whitespace-tests--check-markers (buf bpos epos)
+ (with-current-buffer buf
+ (should (eq (marker-buffer whitespace-bob-marker) buf))
+ (should (eq (marker-position whitespace-bob-marker) bpos))
+ (should (eq (marker-buffer whitespace-eob-marker) buf))
+ (should (eq (marker-position whitespace-eob-marker) epos))))
+
+(ert-deftest whitespace-tests--indirect-clone-markers ()
+ "Test `whitespace--clone' on indirect clones."
+ (whitespace-tests--with-test-buffer '(face empty)
+ (insert "\nx\n\n")
+ (let ((base (current-buffer))
+ ;; `unwind-protect' is not used to clean up `indirect'
+ ;; because the buffer should only be killed on success.
+ (indirect (clone-indirect-buffer nil nil)))
+ (whitespace-tests--check-markers base 2 4)
+ (whitespace--with-buffer-selected indirect
+ (whitespace-tests--check-markers indirect 2 4)
+ ;; Mutate the buffer to trigger `after-change-functions' and
+ ;; thus `whitespace--update-bob-eob'.
+ (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))))
+
+(ert-deftest whitespace-tests--regular-clone-markers ()
+ "Test `whitespace--clone' on regular clones."
+ (whitespace-tests--with-test-buffer '(face empty)
+ (insert "\nx\n\n")
+ (let ((orig (current-buffer))
+ ;; `unwind-protect' is not used to clean up `clone' because
+ ;; the buffer should only be killed on success.
+ (clone (clone-buffer)))
+ (whitespace-tests--check-markers orig 2 4)
+ (whitespace--with-buffer-selected clone
+ (whitespace-tests--check-markers clone 2 4)
+ (execute-kbd-macro (kbd "z RET M-< a"))
+ (whitespace-tests--check-markers clone 1 8))
+ (kill-buffer clone)
+ (whitespace-tests--check-markers orig 2 4))))
+
(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-18 9:24 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-26 14:42 bug#59618: 29.0.50; global-whitespace-mode + org-capture: "Marker does not point anywhere" Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-26 19:58 ` Eli Zaretskii
2022-11-26 20:11 ` Arsen Arsenović via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-27 0:27 ` Ihor Radchenko
2022-11-27 6:13 ` Stefan Kangas
2022-11-27 6:20 ` Ihor Radchenko
2022-11-27 6:54 ` Stefan Kangas
2022-12-01 12:06 ` Eli Zaretskii
2022-12-01 15:02 ` Stefan Kangas
2022-12-13 6:25 ` Richard Hansen
2022-12-13 6:34 ` Ihor Radchenko
2022-12-13 6:38 ` Richard Hansen
2022-12-13 6:48 ` Ihor Radchenko
2022-12-14 2:42 ` Richard Hansen
2022-12-14 12:09 ` Eli Zaretskii
2022-12-14 14:15 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-14 15:08 ` Ihor Radchenko
2022-12-14 17:40 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-14 17:48 ` Ihor Radchenko
2022-12-14 20:10 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-18 9:24 ` Richard Hansen [this message]
2022-12-20 0:41 ` bug#59618: [PATCH] " Richard Hansen
2022-12-20 13:52 ` Eli Zaretskii
2022-12-21 13:34 ` Ihor Radchenko
2022-12-21 14:03 ` Eli Zaretskii
2022-11-27 0:28 ` Ihor Radchenko
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=764d74f1-a1e8-50c7-fd8e-373acced75e5@rhansen.org \
--to=rhansen@rhansen.org \
--cc=59618@debbugs.gnu.org \
--cc=arsen@aarsen.me \
--cc=eliz@gnu.org \
--cc=monnier@iro.umontreal.ca \
--cc=stefankangas@gmail.com \
--cc=yantar92@posteo.net \
/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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.