GNU bug report logs - #76313
New function `replace-region`

Previous Next

Full log

View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Philipp Stephani <phst <at> google.com>, 76313 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Tassilo Horn <tsdh <at> gnu.org>
Subject: bug#76313: New function `replace-region`
Date: Fri, 21 Feb 2025 11:31:00 -0500

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

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

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.


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.