Subject: Re: bug#76108: Call `modify_text' only on the text being replaced in `subst-char-in-region'

From: Stefan Monnier <[email-masked]>
Date: Fri, 07 Feb 2025 14:44:16 -0500

>> You have to understand the distinction between the "before" and the
>> "after" calls (which correspond to the `before-change-functions` and the
>> `after-change-functions` hooks): the "before" call is there to announce
>> that some change*s* *may* happen in the specified region.
> It still reads to me as though it's called before some changes _will_
> happen - that is, when changing text, you call before-change-functions
> (and co.) on the text, change it, and call after-change-functions (and
> co.) afterwards - but I understand that it's up to interpretation and
> mine just happens to not match with the implementation.

Your interpretation is right for the simple case.  But the API is
designed to accommodate other cases, and `subst-char-in-region` takes
advantage of that.

Of course, calling the `before-change-functions` without calling
`after-change-functions` is something we strive to avoid, and similarly
we try to make the region passed to `before-change-functions` as small
as possible, but both of those are done for performance/optimization
reasons, they are not needed for correctness.

> Now, I am not entirely sure how I've managed to miss this, but
> `subst-char-in-region' actually _also_ messes up the after calls.  I
> have no idea how or why this is happening, but a buffer whose contents
> are
>
>   "foo bar\nquux baz ban"
>
> with an overlay from 14 to 17 (that is, around the word "baz") with the
> modification hook
>
>   (lambda (&rest args) (print args))
>
> reports being called with the arguments
>
>   (#<overlay from 17 to 17 in temp> nil 8 21)
>   (#<overlay from 14 to 17 in temp> t 8 9 1)
>
> when I do
>
>   (subst-char-in-region (point-min) (point-max) ?\n ?\s)

The `nil 8 21` and `t 8 9 1` are the values I'd expect, but the `17 to
17` seems very odd and the fact that the hook is called at all for the
"after" case sounds like a bug, as you point out.

> Maybe `report_overlay_modification' calls everything called in the
> before call as well, but I honestly can't tell.

That would be my guess, indeed.
[ Haven't looked at the code yet.  ]

        Stefan