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

From: Thuna <[email-masked]>
Date: Fri, 07 Feb 2025 16:19:36 +0100

>> I'm assuming that there isn't any particular reason why `modify_text'
>> should be called on the whole paragraph, but if there is, then that's ok
>> too, I suppose.  I don't see any reason why this should be breaking
>> stuff, but I haven't actually checked.
>
> The reason is that running the modification hooks N times (to replace
> N chars) would be expensive.

What if we called `modify_text' for each contiguous region of FROMCHARs?
It should be faster than calling it for each character and even
occasionally (though I wouldn't count on it) faster than calling it over
the whole region since it would avoid running hooks unnecessarily.  Do
you know of a way we could benchmark it?

> The `modification-hooks` text property is weaker than the
> `modification-hooks` overlay property: if you use an overlay here, then
> you should be able to get the behavior your test wants by checking in
> your `hook` the value of the second argument (which indicates if it's
> the "before change" or the "after change" case): while the "before
> change" call covers the whole region (actually, IIRC it covers the
> region between the first substituted chars and the end argument), the
> "after change" should be more specific and cover only the region between
> the first and the last chars that were substituted.

And that is the intended and desired behavior?  Personally I find it
quite unintuitive to have `modification-hooks' be "Hooks that are called
when some characters in region _might_ be modified".

I don't think that it's just me, either; in buffer-tests.el's
`overlay-modification-hooks' there is a comment stating
> Replacing text never calls `insert-in-front-hooks' or
> `insert-behind-hooks'.  It calls `modification-hooks' if the overlay
> covers any text that has changed.
which seems to suggest that the tests there were also written with the
understanding that `modification-hooks' should only be called for
characters actually being replaced.  Of course, those tests are for
`replace-match', but still, I see no reason why the two should have
different semantics (sans performance considerations, of course).