GNU bug report logs - #65929
29.1; modification-hooks run when deleting after the overlay

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 65929 in the body.
You can then email your comments to 65929 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.1; modification-hooks run when deleting after the overlay
Date: Wed, 13 Sep 2023 18:37:12 -0400

[Message part 1 (text/plain, inline)]

Package: Emacs
Version: 29.1


As found via yasnippet's test suite, the new overlay code in Emacs-29
sometimes mistakenly runs the `modification-hooks` when the text *right
after* an (empty) overlay is deleted.  This can be seen in the attached
trace obtained while running yasnippet's
`delete-numberless-inner-snippet-issue-562` test.

In that trace we see a call

    (yas--on-protection-overlay-modification #<overlay from 1 to 1 in  *temp*-896992> nil 1 5)

i.e. a call where the `modification-hooks` function
(`yas--on-protection-overlay-modification`) is called for a deletion of
text between 1 and 5 while the overlay itself does not contain any of
those characters since the overlay is empty, at BOB.

According to the Texinfo doc:

    ‘modification-hooks’
         This property’s value is a list of functions to be called if any
         character within the overlay is changed or if text is inserted
         strictly within the overlay.

The behavior in Emacs<29 obeys the Texinfo doc.


        Stefan

[yasnippet-tests-trace.txt (text/plain, inline)]

1 -> (yas-skip-and-clear-field)
| 2 -> (yas-current-field)
| 2 <- yas-current-field: #1=#s(yas--field nil #<marker at 1 in *temp*-896992> #<marker at 5 in  *temp*-896992> #s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 8 in  *temp*-896992> nil nil nil nil #1#) nil nil nil #s(yas--exit #<marker at 8 in *temp*-896992> #s(yas--field 2 #<marker at 8 in  *temp*-896992> #<marker at 11 in  *temp*-896992> nil nil nil nil nil)))
| 2 -> (yas--skip-and-clear #1=#s(yas--field nil #<marker at 1 in *temp*-896992> #<marker at 5 in  *temp*-896992> #s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 8 in  *temp*-896992> nil nil nil nil #1#) nil nil nil #s(yas--exit #<marker at 8 in *temp*-896992> #s(yas--field 2 #<marker at 8 in  *temp*-896992> #<marker at 11 in  *temp*-896992> nil nil nil nil nil))))
| | 3 -> (yas--mark-this-and-children-modified #1=#s(yas--field nil #<marker at 1 in  *temp*-896992> #<marker at 5 in  *temp*-896992> #s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 8 in *temp*-896992> nil nil nil nil #1#) nil nil nil #s(yas--exit #<marker at 8 in  *temp*-896992> #s(yas--field 2 #<marker at 8 in *temp*-896992> #<marker at 11 in  *temp*-896992> nil nil nil nil nil))))
| | 3 <- yas--mark-this-and-children-modified: nil
| | 3 -> (delete-region #<marker at 1 in  *temp*-896992> #<marker at 5 in  *temp*-896992>)
| | | 4 -> (yas--on-field-overlay-modification #<overlay from 1 to 5 in  *temp*-896992> nil 1 5)
| | | | 5 -> (yas-active-snippets 1 5)
| | | | | 6 -> (overlays-in 1 12)
| | | | | 6 <- overlays-in: (#<overlay from 1 to 5 in  *temp*-896992> #<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 12 in  *temp*-896992> #<overlay from 5 to 6 in *temp*-896992>)
| | | | | 6 -> (overlays-in 1 5)
| | | | | 6 <- overlays-in: (#<overlay from 1 to 5 in  *temp*-896992> #<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 12 in  *temp*-896992>)
| | | | 5 <- yas-active-snippets: (#s(yas--snippet nil (#2=#s(yas--field 2 #<marker at 8 in  *temp*-896992> #<marker at 11 in *temp*-896992> nil nil nil nil nil) #1=#s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 8 in  *temp*-896992> nil nil nil nil #3=#s(yas--field nil #<marker at 1 in  *temp*-896992> #<marker at 5 in  *temp*-896992> #1# nil nil t #4=#s(yas--exit #<marker at 8 in  *temp*-896992> #2#))) #3#) #4# 6 #<overlay from 1 to 12 in  *temp*-896992> #3# nil nil))
| | | 4 <- yas--on-field-overlay-modification: (#s(yas--snippet nil (#2=#s(yas--field 2 #<marker at 8 in  *temp*-896992> #<marker at 11 in  *temp*-896992> nil nil nil nil nil) #1=#s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 8 in *temp*-896992> nil nil nil nil #3=#s(yas--field nil #<marker at 1 in  *temp*-896992> #<marker at 5 in  *temp*-896992> #1# nil nil t #4=#s(yas--exit #<marker at 8 in  *temp*-896992> #2#))) #3#) #4# 6 #<overlay from 1 to 12 in  *temp*-896992> #3# nil nil))
| | | 4 -> (yas--on-protection-overlay-modification #<overlay from 1 to 1 in  *temp*-896992> nil 1 5)
| | | 4 <- yas--on-protection-overlay-modification: nil
| | | 4 -> (yas--on-field-overlay-modification #<overlay from 1 to 1 in  *temp*-896992> t 1 1 4)
| | | | 5 -> (yas-active-snippets 1 1)
| | | | | 6 -> (overlays-in 1 8)
| | | | | 6 <- overlays-in: (#<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 8 in  *temp*-896992> #<overlay from 1 to 2 in  *temp*-896992>)
| | | | | 6 -> (overlays-in 1 1)
| | | | | 6 <- overlays-in: (#<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 1 in  *temp*-896992>)
| | | | 5 <- yas-active-snippets: (#s(yas--snippet nil (#2=#s(yas--field 2 #<marker at 4 in  *temp*-896992> #<marker at 7 in *temp*-896992> nil nil nil nil nil) #1=#s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 4 in  *temp*-896992> nil nil nil nil #3=#s(yas--field nil #<marker at 1 in  *temp*-896992> #<marker at 1 in  *temp*-896992> #1# nil nil t #4=#s(yas--exit #<marker at 4 in  *temp*-896992> #2#))) #3#) #4# 6 #<overlay from 1 to 8 in  *temp*-896992> #3# nil nil))
| | | 4 <- yas--on-field-overlay-modification: (#s(yas--snippet nil (#2=#s(yas--field 2 #<marker at 4 in  *temp*-896992> #<marker at 7 in  *temp*-896992> nil nil nil nil nil) #1=#s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 4 in *temp*-896992> nil nil nil nil #3=#s(yas--field nil #<marker at 1 in  *temp*-896992> #<marker at 1 in  *temp*-896992> #1# nil nil t #4=#s(yas--exit #<marker at 4 in  *temp*-896992> #2#))) #3#) #4# 6 #<overlay from 1 to 8 in  *temp*-896992> #3# nil nil))
| | | 4 -> (yas--on-protection-overlay-modification #<overlay from 1 to 1 in  *temp*-896992> t 1 1 4)
| | | | 5 -> (yas-active-snippets)
| | | | | 6 -> (overlays-in 1 8)
| | | | | 6 <- overlays-in: (#<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 8 in  *temp*-896992> #<overlay from 1 to 2 in  *temp*-896992>)
| | | | | 6 -> (overlays-in 1 2)
| | | | | 6 <- overlays-in: (#<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 1 in  *temp*-896992> #<overlay from 1 to 8 in  *temp*-896992> #<overlay from 1 to 2 in  *temp*-896992>)
| | | | 5 <- yas-active-snippets: (#s(yas--snippet nil (#2=#s(yas--field 2 #<marker at 4 in  *temp*-896992> #<marker at 7 in *temp*-896992> nil nil nil nil nil) #1=#s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 4 in  *temp*-896992> nil nil nil nil #3=#s(yas--field nil #<marker at 1 in  *temp*-896992> #<marker at 1 in  *temp*-896992> #1# nil nil t #4=#s(yas--exit #<marker at 4 in  *temp*-896992> #2#))) #3#) #4# 6 #<overlay from 1 to 8 in  *temp*-896992> #3# nil nil))
| | | | 5 -> (yas--commit-snippet #s(yas--snippet nil (#2=#s(yas--field 2 #<marker at 4 in  *temp*-896992> #<marker at 7 in *temp*-896992> nil nil nil nil nil) #1=#s(yas--field 3 #<marker at 1 in  *temp*-896992> #<marker at 4 in  *temp*-896992> nil nil nil nil #3=#s(yas--field nil #<marker at 1 in  *temp*-896992> #<marker at 1 in  *temp*-896992> #1# nil nil t #4=#s(yas--exit #<marker at 4 in  *temp*-896992> #2#))) #3#) #4# 6 #<overlay from 1 to 8 in  *temp*-896992> #3# nil nil))
| | | | 5 <- yas--commit-snippet: nil
| | | 4 <- yas--on-protection-overlay-modification: nil
| | 3 <- delete-region: nil

Message #8 received at 65929 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 65929 <at> debbugs.gnu.org
Subject: Re: bug#65929: 29.1; modification-hooks run when deleting after the
 overlay
Date: Wed, 13 Sep 2023 18:52:33 -0400

I pushed the patch below to `emacs-29`.
It fixes the bug for the yasnippet test suite and adds a corresponding
regression test in our test suite.


        Stefan


diff --git a/src/buffer.c b/src/buffer.c
index 252231357bc..db362ffe7b0 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -4091,7 +4091,7 @@ report_overlay_modification (Lisp_Object start, Lisp_Object end, bool after,
 	    }
 	  /* Test for intersecting intervals.  This does the right thing
 	     for both insertion and deletion.  */
-	  if (! insertion || (end_arg > obegin && begin_arg < oend))
+	  if (end_arg > obegin && begin_arg < oend)
 	    {
 	      Lisp_Object prop = Foverlay_get (overlay, Qmodification_hooks);
 	      if (!NILP (prop))
diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el
index 45914b2b6b0..aa30533c6a0 100644
--- a/test/src/buffer-tests.el
+++ b/test/src/buffer-tests.el
@@ -142,6 +142,7 @@ overlay-modification-hooks
                 (expected-calls . ((modification-hooks (nil 3 4))
                                    (modification-hooks (t 3 4 1)))))
                ((replace . "4"))
+               ((replace . "4") (overlay-beg . 4)) ;bug#65929
                ((replace . "12")
                 (expected-calls . ((modification-hooks (nil 1 3))
                                    (modification-hooks (t 1 2 2)))))

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.