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 08:33:55 -0500

> The specific place where this caused a problem for me is while filling
> paragraphs with latex fragments in org-mode.  Latex fragments are
> overlays with the `modification-hooks' property set to delete the
> overlay, so when a paragraph is filled, that calls
>   (subst-char-in-region ?\n ?\s ...)
> on the entire paragraph which then calls `modify_text' on the entire
> paragraph (sans the first line), which causes all latex fragments to
> disappear.

I'm probably misunderstanding something, but what you describe here
sounds like a bug in Org.

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

> (ert-deftest test-subst-char-in-region-before-change-functions ()
>   (let ((hook (lambda (_beg _end) (error "Poor man's read-only"))))
>     (with-temp-buffer
>       (insert (propertize "foo" 'modification-hooks (list hook)) "," "bar")
>       (should-error (subst-char-in-region (point-min) (point-max) ?o ?e)))
>
>     (with-temp-buffer
>       (insert "foo" (propertize "," 'modification-hooks (list hook)) "bar")
>       ;; My best approximation of `should-not-error'.
>       (should (ignore-errors
>                 (subst-char-in-region (point-min) (point-max) ?o ?e)
>                 t))
>       (should (equal "fee,bar" (buffer-string))))))

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.

        Stefan