Subject: Re: bug#76313: New function `replace-region`

From: Stefan Kangas <[email-masked]>
Date: Tue, 4 Mar 2025 19:58:05 -0800

Stefan Monnier <[email-masked]> writes:

> I suggest we start by merging `replace-region-contents` and `replace-buffer-contents`.
> The downsides I see of each interface is:
>
> - `replace-region-contents` takes a REPLACE-FN whereas
>   it's often easier to compute the replacement beforehand.
> - `replace-buffer-contents` operates only on the "whole"
>   buffer, so requires the use of narrowing if we need something else.
> - `replace-buffer-contents` can't accept a string as the replacement
>   because it already takes a string to mean the name of a buffer.

I'll add that MAX-SECS and MAX-COSTS is a significant complication in
the interfaces of both functions.  I think this is because they were
neither designed as nor intended to be low-level operations.  This is
the main reason why I think `replace-region` is still needed, though I
also agree with the above points.

See also Bug#71370, where we are struggling to justify the obsoletion of
the generalized variable `buffer-substring`, because we do not have
anything to replace it with.  According to the statistics posted there
by Andrea, that is the most popular generalized variable by far, so
there are clearly more than a few users that want something like it.

> I think it's hard to fix the problems of `replace-buffer-contents` in
> a backward incompatible way, so I suggest we "standardize" on
> `replace-region-contents` instead where we replace its REPLACE-FN
> argument with a REPLACE argument which can be either a string, or a
> buffer, or a function (this latter one could be deprecated, but we'd
> keep it for backward compatibility).
>
> This is backward-compatible, so we would obsolete only `replace-buffer-contents`.

This is orthogonal to `replace-region`, is it not?  It sounds to me like
a good change, in any case.

> Compared to my notion of ideal this would still have three problems:
>
> - It's much slower than insert+delete-region, so it's not a replacement
>   for the function I proposed.  We could add a "fast" option to it, but
>   this fast option (which would presumably be the most common use case)
>   would require every caller to pass an extra argument, which
>   would be inconvenient (would encourage people to use the expensive
>   code without being aware of the risk).
> - It lacks an option to `insert-and-inherit` (needed by
>   `minibuffer--replace`).  We could add one more optional arg for it.
>   It would be kind of "far" (after MAX-SECS MAX-COSTS), so it's not
>   ideal, but it's not the end of the world.
> - It still uses this MAX-COSTS argument which noone seems to understand.
>
> BTW, the code of `replace-buffer-contents` does:
>
>        del_range (...);
>        Finsert_buffer_substring (...);
>
> This means that As it currently stands, when there is no similarity
> between the before and after contents, `replace-buffer-contents`
> preserves markers less carefully than the `replace-region` code
> I sent 🙁.
>
> The above would be better replaced by a call to a simple&fast
> `replace-region`, tho contrary to the one I sent, that one needs to take
> its replacement text from a region of a buffer rather than from
> a string.  I guess we could let the STRING (aka REPLACE) arg of
> `replace-region` be a triplet (BUFFER FROM TO), or we could add another
> function of the form
>
>     (replace-region-from-buffer BEG END
>                                 BUFFER BUFFER-BEG BUFFER-END
>                                 &optional INHERIT)
>
> or require the use of narrowing in the source buffer to implicitly pass
> the bounds.

FWIW, I think that I'd personally rather see another function than
complicating the `replace-region` interface just for this.  OTOH, if
it's needed only here perhaps we could just inline it.