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

From: Stefan Kangas <[email-masked]>
Date: Thu, 20 Feb 2025 02:37:45 -0800

Tassilo Horn <[email-masked]> writes:

> Stefan Kangas <[email-masked]> writes:
>
>>> The benefits of supporting the `replace-region-contents` API are much
>>> smaller than the "sounded minor" annoyances we'd have to pay for the
>>> rest of Emacs's future.
>>
>> AFAIU, the `replace-buffer-contents` API exists because:
>>
>>> There are many tools (e.g. auto-formatters) that take buffer
>>> contents, reformat them, and write the reformatted output somewhere.
>>> However, there is no good way how to apply the modified output to the
>>> source buffer.  The naive way (erasing and re-inserting the buffer
>>> contents) loses point and markers.  Therefore there should be a
>>> function (e.g. `replace-buffer-contents') that calculates a minimal
>>> diff between the old and the new contents and uses editing operations
>>> (insert, delete, etc.) to apply the diff.  Ideally this would be done
>>> without any external tools (e.g. 'diff').
>>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25355
>
> That's true.  I use it frequently (through json-pretty-print-buffer) to
> format minimized JSON replies from REST-APIs.  It would be a bummer if
> point moved to the end of the buffer contents after formatting.  But it
> can be unbearable slow if the region is very large and the replacement
> text differs a lot from the current text.  That's why it has these
> strange MAX-SECS and MAX-COSTS arguments.
>
>> This sounds quite specialized to my mind, and the API is also quite
>> complicated.  I think it's a fine addition for this use case, but I'd
>> suggest renaming it to something that highlights its specialized
>> nature.
>>
>> The `replace-region-contents' function builds on that, and similarly
>> has a complicated calling convention.  I think it should similarly be
>> renamed to highlight its specialized nature.
>
> Fine with me but we have to consider that it's already used in the wild.
> I remember submitting patches to at least restclient.el (which only uses
> it through json-pretty-print-buffer so wouldn't be affected by renaming)
> and hindent.el, see
> https://github.com/mihaimaruseac/hindent/blob/master/elisp/hindent.el#L220.
> Maybe there are other callers in third-party packages.

We could just leave the old names as aliases, so nothing should break.
Anyways, it's not highly important to rename it, I just think that it
might help clarify its intended use.

>> Meanwhile, according to what we have learned in Bug#71370, many users
>> are currently using this:
>>
>>     (setf (buffer-substring START END) "foo")
>
> That's actually quite elegant (although it gives an obsolecence
> warning).  It never occured to me that (buffer-substring START END) was
> a generalized variable.

I think the main "problem" is that ELisp programmers are more used to
functions than generalized variables, so some people (like you and me
probably) are less used to them.  I also wasn't aware of this until we
decided to obsolete it a few years ago.

>> I think this shows the need for a more low-level operation, and
>> `replace-region` seems to fit this use case exactly.  So I would be in
>> favor of installing it.
>
> IMHO, it should at least reference replace-region-contents from its
> docstring and explain the difference (and vice-versa).

Fully agreed.