bug#65929: 29.1; modification-hooks run when deleting after the overlay

bug#65929: 29.1; modification-hooks run when deleting after the overlay

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

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

[-- Attachment #2: yasnippet-tests-trace.txt --]
[-- Type: text/plain, Size: 6229 bytes --]

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

bug#65929: 29.1; modification-hooks run when deleting after the overlay

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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)))))