bug#30823: 25.3; modification-hooks of overlays are not run in some cases

[Noam Postavsky]

tags 30823 + patch
quit

Ren Victor <victorhge@gmail.com> writes:

> I wrote a ert case which is encolsed.
>
> emacs -Q -batch -l ert -l bug30823.el -f ert-run-tests-batch-and-exit

Thanks, this patch seems to fix it.

>From 41cb2b33bc62a23a0561b94f3d25e1282935a08c Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 31 Mar 2018 09:33:41 -0400
Subject: [PATCH v1] Don't skip modification hooks if 1st overlay is deleted
 (Bug#30823)

The fix for Bug#21824 "Don't invoke overlay modification hooks in
wrong buffer" from 2015-11-06 prevented running of overlay hooks if
the first overlay registered when running the hooks with after=nil was
deleted (since a deleted overlay has no buffer, it was considered as
not from the current buffer).  Therefore, revert that change and
instead just inhibit modification hooks when performing message
coalescing (because in that case, we aren't doing the necessary
preparation for running modification hooks, or even running them with
after=nil at all).
* src/buffer.c (report_overlay_modification): Remove checking of
buffer overlay.
* src/xdisp.c (message_dolog): Let-bind inhibit-modification-hooks
to t around del_range_both calls
* test/src/buffer-tests.el (test-modification-hooks): New test.
---
 src/buffer.c             | 17 -----------------
 src/xdisp.c              |  9 +++++++++
 test/src/buffer-tests.el | 19 +++++++++++++++++++
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index 14837372d3..a5d65da2e8 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -4543,23 +4543,6 @@ report_overlay_modification (Lisp_Object start, 
Lisp_Object end, bool after,
     Lisp_Object *copy;
     ptrdiff_t i;
 
-    if (size)
-      {
-       Lisp_Object ovl
-         = XVECTOR (last_overlay_modification_hooks)->contents[1];
-
-       /* If the buffer of the first overlay in the array doesn't
-          match the current buffer, then these modification hooks
-          should not be run in this buffer.  This could happen when
-          some code calls some insdel functions, such as del_range_1,
-          with the PREPARE argument false -- in that case this
-          function is never called to record the overlay modification
-          hook functions in the last_overlay_modification_hooks
-          array, so anything we find there is not ours.  */
-       if (XMARKER (OVERLAY_START (ovl))->buffer != current_buffer)
-         return;
-      }
-
     USE_SAFE_ALLOCA;
     SAFE_ALLOCA_LISP (copy, size);
     memcpy (copy, XVECTOR (last_overlay_modification_hooks)->contents,
diff --git a/src/xdisp.c b/src/xdisp.c
index df5335e4ac..082b40b742 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10403,6 +10403,13 @@ message_dolog (const char *m, ptrdiff_t nbytes, bool 
nlflag, bool multibyte)
          ptrdiff_t this_bol, this_bol_byte, prev_bol, prev_bol_byte;
          printmax_t dups;
 
+          /* Since we call del_range_both passing false for PREPARE,
+             we aren't prepared to run modification hooks (we could
+             end up calling modification hooks from another buffer and
+             only with AFTER=t, Bug#21824).  */
+          ptrdiff_t count = SPECPDL_INDEX ();
+          specbind (Qinhibit_modification_hooks, Qt);
+
          insert_1_both ("\n", 1, 1, true, false, false);
 
          scan_newline (Z, Z_BYTE, BEG, BEG_BYTE, -2, false);
@@ -10448,6 +10455,8 @@ message_dolog (const char *m, ptrdiff_t nbytes, bool 
nlflag, bool multibyte)
                            -XFASTINT (Vmessage_log_max) - 1, false);
              del_range_both (BEG, BEG_BYTE, PT, PT_BYTE, false);
            }
+
+          unbind_to (count, Qnil);
        }
       BEGV = marker_position (oldbegv);
       BEGV_BYTE = marker_byte_position (oldbegv);
diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el
index f9c477fbfd..5d091875b5 100644
--- a/test/src/buffer-tests.el
+++ b/test/src/buffer-tests.el
@@ -45,6 +45,25 @@
             (should (eq buf (current-buffer))))
         (when msg-ov (delete-overlay msg-ov))))))
 
+(ert-deftest test-modification-hooks ()
+  "Test for bug#30823."
+  (let ((check-point nil)
+       (ov-delete nil)
+       (ov-set nil))
+    (with-temp-buffer
+      (insert "abc")
+      (setq ov-set (make-overlay 1 3))
+      (overlay-put ov-set 'modification-hooks
+                  (list (lambda (_o after &rest _args)
+                          (and after (setq check-point t)))))
+      (setq ov-delete (make-overlay 1 3))
+      (overlay-put ov-delete 'modification-hooks
+                  (list (lambda (o after &rest _args)
+                          (and (not after) (delete-overlay o)))))
+      (goto-char 2)
+      (insert "1")
+      (should (eq check-point t)))))
+
 (ert-deftest test-generate-new-buffer-name-bug27966 ()
   (should-not (string-equal "nil"
                             (progn (get-buffer-create "nil")
-- 
2.11.0