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

From: Stefan Monnier <[email-masked]>
Date: Tue, 11 Feb 2025 22:43:45 -0500

>> And I guess we could use the same approach to run the
>> `modification-hooks` from text properties also *after* the modification.
>
> I'm still not happy with the loose way in which these hooks are called.

Think of it this way: while `subst-char-in-region` can turn

   foo
   bar

into

   foo bar

by modifying a single character, the major mode needs to be able to
handle the situation where the user replaces `foo\nbar` with `foo bar`.
So while it's better for `subst-char-in-region` to use tighter bounds,
it should not be needed for correctness.

IOW, if your hook function misbehaves when the region is not tight
enough, it's (also) a problem in your hook function.

> I imagine that any modification hook that's useful will essentially be
> implementing a half-working version of what calling them strictly would
> achieve.  My biggest problem is that I don't have a reference for how
> significant the performance issues are, so the limitations end up
> feeling much more impactful.

Most hook functions like coarse regions.  E.g. the one used by font-lock
rounds up the region to whole lines; the one used by `syntax-ppss` only
cares about the BEG arg and considers everything after it as lost; the
one used by Eglot also likes them rather coarse to keep the description
of changes (sent to the LSP server) smaller.

The cost of running too many hook functions prompted the introduction of
`combine-after-change-calls` in Emacs-20, and `combine-change-calls` in
Emacs-27.

> At the very least, the documentation of precisely when these functions
> are called and what sort of guarantees they give should be updated,
> since it currently reads
>> ‘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.
> in (info "(elisp) Overlay Properties").

Agreed.  I'd be in favor of changing the code also to avoid running the
after change when it turns out it's outside the bounds of the overlay.

        Stefan