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

Previous Next

To reply to this bug, email your comments to 76313 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: bug-gnu-emacs <at> gnu.org
Subject: New function `replace-region`
Date: Sat, 15 Feb 2025 17:18:15 -0500

[Message part 1 (text/plain, inline)]

Tags: patch

We often need to replace a chunk of buffer text with something else.
It is easy enough to do with `insert` + `delete-region`, but that
has suboptimal behavior w.r.t markers, so rather than let coders
reinvent a slightly-better replacement I propose we add a dedicated
function for it.

So I suggest the patch below.  Comments?  Objections?


        Stefan

[replace-region.patch (text/patch, attachment)]

Message #8 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sun, 16 Feb 2025 07:42:43 +0200

> Cc: monnier <at> iro.umontreal.ca
> Date: Sat, 15 Feb 2025 17:18:15 -0500
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> We often need to replace a chunk of buffer text with something else.
> It is easy enough to do with `insert` + `delete-region`, but that
> has suboptimal behavior w.r.t markers, so rather than let coders
> reinvent a slightly-better replacement I propose we add a dedicated
> function for it.
> 
> So I suggest the patch below.  Comments?  Objections?

What's wrong with replace-buffer-contents or replace-region-contents?

Message #11 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sun, 16 Feb 2025 12:17:04 -0500

>> So I suggest the patch below.  Comments?  Objections?
> What's wrong with replace-buffer-contents or replace-region-contents?

Yeah, I'm not super happy about that situation, but here are my reasons:

- On the API side: They're inconvenient replacements because they require
  either changing the narrowing or wrapping the replacement string into
  a closure.  Not the end of the world, but AFAICT, `replace-region` is
  a quite common operation (the patch I sent only replaces a couple of
  use-cases, but I know there are much more).

- On the implementation side: They are significantly more expensive (fancier).

Maybe we should merge some of them.  E.g. we could merge my
proposed `replace-region` with `replace-region-contents` as follow:

- Allow REPLACE-FN to be a string rather than a function.
- If MAX-SECS/COSTS are both nil, then use the cheap implementation.
  I.e. only do the fancy diffing when one of MAX-SECS/COSTS is specified
  (could allow a `t` value to mean "do it the fancy way with default
  settings").

I'm not sure it'd be a good idea, tho: `replace-region-contents` is
a "heavy" operation, involving narrowing and another buffer, whereas
`replace-region` is meant to be a low-level operation (if we implement
it in C, we could arguably use it as *the* low-level operation and
define `insert` and `delete-region` on top of it).


        Stefan

Message #14 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sun, 16 Feb 2025 21:30:41 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: 76313 <at> debbugs.gnu.org
> Date: Sun, 16 Feb 2025 12:17:04 -0500
> 
> >> So I suggest the patch below.  Comments?  Objections?
> > What's wrong with replace-buffer-contents or replace-region-contents?
> 
> Yeah, I'm not super happy about that situation, but here are my reasons:
> 
> - On the API side: They're inconvenient replacements because they require
>   either changing the narrowing or wrapping the replacement string into
>   a closure.  Not the end of the world, but AFAICT, `replace-region` is
>   a quite common operation (the patch I sent only replaces a couple of
>   use-cases, but I know there are much more).
> 
> - On the implementation side: They are significantly more expensive (fancier).
> 
> Maybe we should merge some of them.  E.g. we could merge my
> proposed `replace-region` with `replace-region-contents` as follow:
> 
> - Allow REPLACE-FN to be a string rather than a function.

I immediately thought of this extension when I saw your proposal.

> I'm not sure it'd be a good idea, tho: `replace-region-contents` is
> a "heavy" operation, involving narrowing and another buffer, whereas
> `replace-region` is meant to be a low-level operation (if we implement
> it in C, we could arguably use it as *the* low-level operation and
> define `insert` and `delete-region` on top of it).

On the downside, we already have 2 APIs for this, so having a 3rd one
will only make the confusion more prominent.  Who, apart of you and
maybe me, will know or remember that those two are "heavy" whereas the
3rd one isn't, especially given that replace-buffer-contents is
implemented in C?  And how to apply that knowledge to any concrete use
case?

Also, if someone wants a "simple" implementation that doesn't care
about being destructive and clobbering markers, properties, etc., why
can't they indeed do the delete-region + insert dance?

Message #17 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sun, 16 Feb 2025 21:45:03 -0500

>> I'm not sure it'd be a good idea, tho: `replace-region-contents` is
>> a "heavy" operation, involving narrowing and another buffer, whereas
>> `replace-region` is meant to be a low-level operation (if we implement
>> it in C, we could arguably use it as *the* low-level operation and
>> define `insert` and `delete-region` on top of it).
>
> On the downside, we already have 2 APIs for this, so having a 3rd one
> will only make the confusion more prominent.

We could start by merging `replace-region-contents` and
`replace-buffer-contents`.
[ We should also improve their docstrings to give some hint about what
  kind of "diff" they're able to find.  E.g. do they use char/word/line
  granularity?  ]

> Who, apart of you and maybe me, will know or remember that those two
> are "heavy" whereas the 3rd one isn't,

Indeed, their name should be upfront about it.
I'd have expected a name like `replace-region-carefully`.

> And how to apply that knowledge to any concrete use case?

Apparently current callers don't have much difficulty choosing between
insert+delete-region and `replace-region-contents`, so I'm not
too worried.
I think the complexity of the API described in the docstring does the
trick to make sure people use the more expensive ones only when they
care enough about the difference.

> Also, if someone wants a "simple" implementation that doesn't care
> about being destructive and clobbering markers, properties, etc., why
> can't they indeed do the delete-region + insert dance?

They can.  But using the new `replace-region` is simpler for them, with
the side benefit that it will behave (slightly) better.
That's kind of the point of `replace-region`: to be a no-brainer
replacement for insert+delete-region.


        Stefan

Message #20 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sun, 16 Feb 2025 23:36:57 -0500

[Message part 1 (text/plain, inline)]

> So I suggest the patch below.  Comments?  Objections?

Here's a better implementation, in C.
One advantage is that it runs the `after/before-change-functions` only once.
Also, I made it take an optional `inherit` argument so it can do the
same as `insert-and-inherit` (as is used in `minibuffer--replace`).
This version does not move point to the end of the new text.


        Stefan

[replace-region.patch (text/x-diff, inline)]

diff --git a/src/editfns.c b/src/editfns.c
index f9258392146..c86d0b890a2 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2654,6 +2654,22 @@ DEFUN ("delete-and-extract-region", Fdelete_and_extract_region,
     return empty_unibyte_string;
   return del_range_1 (XFIXNUM (start), XFIXNUM (end), 1, 1);
 }
+
+DEFUN ("replace-region", Freplace_region, Sreplace_region, 3, 4, 0,
+       doc: /* Replace region FROM..TO with STRING.
+Does not move point, like `delete-region' but unlike `insert'.
+If optional arg INHERIT, then text properties are inherited from
+context like `insert-and-inherit'.  */)
+  (Lisp_Object from, Lisp_Object to, Lisp_Object string, Lisp_Object inherit)
+{
+  validate_region (&from, &to);
+  CHECK_STRING (string);
+  /* SET_PT (XFIXNUM (to)); */
+  replace_range (XFIXNUM (from), XFIXNUM (to), string,
+		 /* FIXME: Should we update search regs?  */
+		 true, !NILP (inherit), true, false, false);
+  return Qnil;
+}
 
 /* Alist of buffers in which labeled restrictions are used.  The car
    of each list element is a buffer, the cdr is a list of triplets
@@ -4929,6 +4945,7 @@ syms_of_editfns (void)
   defsubr (&Stranslate_region_internal);
   defsubr (&Sdelete_region);
   defsubr (&Sdelete_and_extract_region);
+  defsubr (&Sreplace_region);
   defsubr (&Swiden);
   defsubr (&Snarrow_to_region);
   defsubr (&Sinternal__labeled_narrow_to_region);
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index 8d4e7bc48fa..1b0fe30f8ce 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -305,6 +305,29 @@ replace-buffer-contents-bug31837
   (should (equal (buffer-substring-no-properties (point-min) (point-max))
                  (concat (string (char-from-name "SMILE")) "1234"))))
 
+(ert-deftest editfns-tests--replace-region ()
+  (with-temp-buffer
+    (insert "here is some text")
+    (let ((m5n (copy-marker (+ (point-min) 5)))
+          (m5a (copy-marker (+ (point-min) 5) t))
+          (m6n (copy-marker (+ (point-min) 6)))
+          (m6a (copy-marker (+ (point-min) 6) t))
+          (m7n (copy-marker (+ (point-min) 7)))
+          (m7a (copy-marker (+ (point-min) 7) t)))
+      (replace-region (+ (point-min) 5) (+ (point-min) 7) "was")
+      (should (equal (buffer-string) "here was some text"))
+      (should (equal (point) (point-max)))
+      ;; Markers before the replaced text stay before.
+      (should (= m5n (+ (point-min) 5)))
+      (should (= m5a (+ (point-min) 5)))
+      ;; Markers in the replaced text can end up at either end, depending
+      ;; on whether they're advance-after-insert or not.
+      (should (= m6n (+ (point-min) 5)))
+      (should (<= (+ (point-min) 5) m6a (+ (point-min) 8)))
+      ;; Markers after the replaced text stay after.
+      (should (= m7n (+ (point-min) 8)))
+      (should (= m7a (+ (point-min) 8))))))
+
 (ert-deftest delete-region-undo-markers-1 ()
   "Make sure we don't end up with freed markers reachable from Lisp."
   ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30931#40

Message #23 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Mon, 17 Feb 2025 14:20:20 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: 76313 <at> debbugs.gnu.org
> Date: Sun, 16 Feb 2025 21:45:03 -0500
> 
> >> I'm not sure it'd be a good idea, tho: `replace-region-contents` is
> >> a "heavy" operation, involving narrowing and another buffer, whereas
> >> `replace-region` is meant to be a low-level operation (if we implement
> >> it in C, we could arguably use it as *the* low-level operation and
> >> define `insert` and `delete-region` on top of it).
> >
> > On the downside, we already have 2 APIs for this, so having a 3rd one
> > will only make the confusion more prominent.
> 
> We could start by merging `replace-region-contents` and
> `replace-buffer-contents`.

Fine by me, thanks.

Message #26 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Mon, 17 Feb 2025 10:03:26 -0500

>> >> I'm not sure it'd be a good idea, tho: `replace-region-contents` is
>> >> a "heavy" operation, involving narrowing and another buffer, whereas
>> >> `replace-region` is meant to be a low-level operation (if we implement
>> >> it in C, we could arguably use it as *the* low-level operation and
>> >> define `insert` and `delete-region` on top of it).
>> >
>> > On the downside, we already have 2 APIs for this, so having a 3rd one
>> > will only make the confusion more prominent.
>> 
>> We could start by merging `replace-region-contents` and
>> `replace-buffer-contents`.
>
> Fine by me, thanks.

So does that mean you're OK with my new `replace-region`, and that
I should open a new bug-report with a proposal for how to merge
`replace-region-contents` and `replace-buffer-contents`?

Or something else?


        Stefan

Message #29 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Mon, 17 Feb 2025 18:14:41 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: 76313 <at> debbugs.gnu.org
> Date: Mon, 17 Feb 2025 10:03:26 -0500
> 
> >> >> I'm not sure it'd be a good idea, tho: `replace-region-contents` is
> >> >> a "heavy" operation, involving narrowing and another buffer, whereas
> >> >> `replace-region` is meant to be a low-level operation (if we implement
> >> >> it in C, we could arguably use it as *the* low-level operation and
> >> >> define `insert` and `delete-region` on top of it).
> >> >
> >> > On the downside, we already have 2 APIs for this, so having a 3rd one
> >> > will only make the confusion more prominent.
> >> 
> >> We could start by merging `replace-region-contents` and
> >> `replace-buffer-contents`.
> >
> > Fine by me, thanks.
> 
> So does that mean you're OK with my new `replace-region`, and that
> I should open a new bug-report with a proposal for how to merge
> `replace-region-contents` and `replace-buffer-contents`?
> 
> Or something else?

You suggested to merge replace-region-contents with
replace-buffer-contents when replace-region was not on the table at
all.  I'm still not sure we want a 3rd API for this purpose.

Message #32 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Mon, 17 Feb 2025 15:55:04 -0500

>> So does that mean you're OK with my new `replace-region`, and that
>> I should open a new bug-report with a proposal for how to merge
>> `replace-region-contents` and `replace-buffer-contents`?
>> 
>> Or something else?
>
> You suggested to merge replace-region-contents with
> replace-buffer-contents when replace-region was not on the table at
> all.  I'm still not sure we want a 3rd API for this purpose.

I just want a simple function which does the same as
`insert+delete-region`, just as efficiently, without having to think
about which way is best (should I delete first, or insert first?
If I insert first, should I insert at FROM, or at TO, or in the
middle?  You can find pretty much all variations out there, several
times with comments showing that the coders did have to waste time
thinking about it).

`replace-buffer/region-contents` is not a valid answer because it's
a lot more costly.  Do you want me to merge them all into a single
monster `replace-region` function and mark the other two as obsolete:

    (replace-region FROM TO REPLACEMENT &optional INHERIT CAREFULLY)

- REPLACEMENT can be a string or a buffer object (not a buffer name!).
  I'd drop support for `replace-region-contents`s function
  because one can just as easily run the function before calling
  `replace-region(-contents)` (which may also save us from building
  a closure).
-  INHERIT would specify whether to "insert-and-inherit" (used by
  `minibuffer--replace`).
- CAREFULLY if non-nil requests a costly replacement which minimizes
  impact on markers and such.  It can be a symbol or a list (MAX-SECS
  MAX-COSTS), tho I hate that ill-defined MAX-COSTS thingy, so if we
  could replace it with something better defined that would be sweet.
- I'd make it return nil since it seems that the return value of
  `replace-buffer-contents` is not used anywhere (well, except in
  `files-tests-revert-buffer-with-fine-grain` where we could replace it
  with a test that some marker was indeed properly preserved rather
  than blindly believing the return value of `revert-buffer-with-fine-grain`).

I can do that.


        Stefan

Message #35 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Mon, 17 Feb 2025 22:38:22 -0500

Stefan Monnier [2025-02-17 15:55:04] wrote:
> I just want a simple function which does the same as
> `insert+delete-region`, just as efficiently, without having to think
> about which way is best (should I delete first, or insert first?
> If I insert first, should I insert at FROM, or at TO, or in the
> middle?  You can find pretty much all variations out there, several
> times with comments showing that the coders did have to waste time
> thinking about it).

IOW, just like `oddp/evenp` saves coders the trouble of wondering
whether to implement it using `%` or `logand` or whatnot.


        Stefan

Message #38 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Tue, 18 Feb 2025 14:25:17 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: 76313 <at> debbugs.gnu.org
> Date: Mon, 17 Feb 2025 15:55:04 -0500
> 
> >> So does that mean you're OK with my new `replace-region`, and that
> >> I should open a new bug-report with a proposal for how to merge
> >> `replace-region-contents` and `replace-buffer-contents`?
> >> 
> >> Or something else?
> >
> > You suggested to merge replace-region-contents with
> > replace-buffer-contents when replace-region was not on the table at
> > all.  I'm still not sure we want a 3rd API for this purpose.
> 
> I just want a simple function which does the same as
> `insert+delete-region`, just as efficiently, without having to think
> about which way is best (should I delete first, or insert first?
> If I insert first, should I insert at FROM, or at TO, or in the
> middle?  You can find pretty much all variations out there, several
> times with comments showing that the coders did have to waste time
> thinking about it).

How come we never needed it until now, even though insdel.c can
replace buffer text since day one?  And are you sure that the way you
implement it by calling replace_region, this will produce exactly the
variation that everyone wants?

I thought at some point you suggested allowing to extend the
REPLACE-FN argument of replace-region-contents to be a string, and I
agreed to that (had the same idea, actually, while reading your OP).
But now you are going back to basically your original proposal? why?

> `replace-buffer/region-contents` is not a valid answer because it's
> a lot more costly.  Do you want me to merge them all into a single
> monster `replace-region` function and mark the other two as obsolete:

No, but how about

 (replace-buffer-contents SOURCE &optional MAX-SECS MAX-COSTS CHEAP-AND-RECKLESS)

where, if CHEAP-AND-RECKLESS is non-nil, we call replace_region
disregarding what will that do to markers and overlays?  If you want
to support INHERIT, CHEAP-AND-RECKLESS could be also non-nil and non-t
to indicate that.  I can live with that; at least we stay with the
same 2 APIs, of which one is implemented on top of the other.

Message #41 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Tue, 18 Feb 2025 15:04:58 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: 76313 <at> debbugs.gnu.org
> Date: Mon, 17 Feb 2025 22:38:22 -0500
> 
> Stefan Monnier [2025-02-17 15:55:04] wrote:
> > I just want a simple function which does the same as
> > `insert+delete-region`, just as efficiently, without having to think
> > about which way is best (should I delete first, or insert first?
> > If I insert first, should I insert at FROM, or at TO, or in the
> > middle?  You can find pretty much all variations out there, several
> > times with comments showing that the coders did have to waste time
> > thinking about it).
> 
> IOW, just like `oddp/evenp` saves coders the trouble of wondering
> whether to implement it using `%` or `logand` or whatnot.

Assuming that people will know what is "odd" and what is "even" better
and easier than they know their mathematical definitions...

Message #44 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Tue, 18 Feb 2025 08:30:16 -0500

>  (replace-buffer-contents SOURCE &optional MAX-SECS MAX-COSTS CHEAP-AND-RECKLESS)

Hmm... I don't think people will be happy to replace their

    (insert foo)
    (delete-region from to)

with

    (save-restriction
      (narrow-to-region from to)
      (replace-buffer-contents foo nil nil 'cheap))

Without mentioning that it would be a backward-incompatible change because
currently `replace-buffer-contents` considers a string argument as the
name of a buffer.

> where, if CHEAP-AND-RECKLESS is non-nil, we call replace_region
> disregarding what will that do to markers and overlays?  If you want
> to support INHERIT, CHEAP-AND-RECKLESS could be also non-nil and non-t
> to indicate that.

FWIW, `minibuffer--replace` seems to want INHERIT at the same time as
"fancy".  Tho I must admit that I can't remember why I needed
INHERIT there.  Maybe because of insertion in a field?

> I can live with that; at least we stay with the
> same 2 APIs, of which one is implemented on top of the other.

`replace-region-contents` is a bad API (the REPLACE-FN part is a bad
idea, all the callers I could find would be better served if they could
pass a string instead): we should obsolete it.


        Stefan

Message #47 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Tue, 18 Feb 2025 17:36:21 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: 76313 <at> debbugs.gnu.org
> Date: Tue, 18 Feb 2025 08:30:16 -0500
> 
> >  (replace-buffer-contents SOURCE &optional MAX-SECS MAX-COSTS CHEAP-AND-RECKLESS)
> 
> Hmm... I don't think people will be happy to replace their
> 
>     (insert foo)
>     (delete-region from to)
> 
> with
> 
>     (save-restriction
>       (narrow-to-region from to)
>       (replace-buffer-contents foo nil nil 'cheap))
> 
> Without mentioning that it would be a backward-incompatible change because
> currently `replace-buffer-contents` considers a string argument as the
> name of a buffer.

Then how about reimplementing replace-region-contents in C, and making
a similar change (an additional optional arg) to it instead?

> `replace-region-contents` is a bad API

Why is it bad?

Message #50 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Tue, 18 Feb 2025 12:01:38 -0500

>> `replace-region-contents` is a bad API
> Why is it bad?

- REPLACE-FN is never useful, it's always just as easy (and marginally
  more efficient) to compute the replacement beforehand.
- It defaults to being expensive, whereas most potential users of
  `replace-region` currently use insert+delete-region and thus would
  need to pass an extra "cheaply" argument for fear of being impacted by
  a slowdown.
- MAX-COSTS is ill-defined.

Also, personally I must say that I'd be happier calling `replace-region`
than `replace-region-contents`, just because it's shorter.

Given that `replace-region-contents` is fairly new and very little used,
I don't see much gain in trying to fit within its API.


        Stefan

Message #53 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Tue, 18 Feb 2025 21:30:33 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: 76313 <at> debbugs.gnu.org
> Date: Tue, 18 Feb 2025 12:01:38 -0500
> 
> >> `replace-region-contents` is a bad API
> > Why is it bad?
> 
> - REPLACE-FN is never useful, it's always just as easy (and marginally
>   more efficient) to compute the replacement beforehand.
> - It defaults to being expensive, whereas most potential users of
>   `replace-region` currently use insert+delete-region and thus would
>   need to pass an extra "cheaply" argument for fear of being impacted by
>   a slowdown.
> - MAX-COSTS is ill-defined.
> 
> Also, personally I must say that I'd be happier calling `replace-region`
> than `replace-region-contents`, just because it's shorter.
> 
> Given that `replace-region-contents` is fairly new and very little used,
> I don't see much gain in trying to fit within its API.

So basically, you reject every attempt to compromise that I could come
up with, and keep pressing for your original proposal, whose downsides
I prefer not to sustain.  Are you really saying that your way is the
only way? why?

Message #58 received at 76313-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313-done <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Tue, 18 Feb 2025 15:04:22 -0500

> So basically, you reject every attempt to compromise that I could come
> up with, and keep pressing for your original proposal, whose downsides
> I prefer not to sustain.  Are you really saying that your way is the
> only way? why?

Never mind,


        Stefan

Message #61 received at 76313-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313-done <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Wed, 19 Feb 2025 13:59:04 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: 76313-done <at> debbugs.gnu.org
> Date: Tue, 18 Feb 2025 15:04:22 -0500
> 
> > So basically, you reject every attempt to compromise that I could come
> > up with, and keep pressing for your original proposal, whose downsides
> > I prefer not to sustain.  Are you really saying that your way is the
> > only way? why?
> 
> Never mind,

It's up to you, but I still think that by a relatively simple and
backward-compatible change to the API of replace-region-contents we
could have a new mode of that function which would do what you wanted.
I still don't understand why you rejected that, the reasons you gave
sounded minor to me, certainly not important enough to throw in the
towel.

Message #64 received at 76313-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313-done <at> debbugs.gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Wed, 19 Feb 2025 10:02:53 -0500

> It's up to you, but I still think that by a relatively simple and
> backward-compatible change to the API of replace-region-contents we
> could have a new mode of that function which would do what you wanted.
> I still don't understand why you rejected that, the reasons you gave
> sounded minor to me, certainly not important enough to throw in the
> towel.

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.

And really, this whole discussion has been much too frustrating, for
such a trivial function which everyone can feel free to reinvent badly in
their own code.


        Stefan

Message #67 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: Philipp Stephani <phst <at> google.com>, 76313 <at> debbugs.gnu.org,
 Tassilo Horn <tsdh <at> gnu.org>
Subject: Re: bug#76313: New function `replace-region`
Date: Wed, 19 Feb 2025 18:22:39 +0000

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> gnu.org> writes:

>> It's up to you, but I still think that by a relatively simple and
>> backward-compatible change to the API of replace-region-contents we
>> could have a new mode of that function which would do what you wanted.
>> I still don't understand why you rejected that, the reasons you gave
>> sounded minor to me, certainly not important enough to throw in the
>> towel.
>
> 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

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.

Meanwhile, according to what we have learned in Bug#71370, many users
are currently using this:

    (setf (buffer-substring START END) "foo")

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.

Message #70 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
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>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#76313: New function `replace-region`
Date: Thu, 20 Feb 2025 08:03:02 +0100

Stefan Kangas <stefankangas <at> gmail.com> 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.

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

Bye,
Tassilo

Message #73 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Philipp Stephani <phst <at> google.com>, 76313 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#76313: New function `replace-region`
Date: Thu, 20 Feb 2025 02:37:45 -0800

Tassilo Horn <tsdh <at> gnu.org> writes:

> Stefan Kangas <stefankangas <at> gmail.com> 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.

Message #76 received at 76313 <at> debbugs.gnu.org (full text, mbox):

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: Re: 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

Message #79 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
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: Re: bug#76313: New function `replace-region`
Date: Tue, 4 Mar 2025 19:58:05 -0800

Stefan Monnier <monnier <at> iro.umontreal.ca> 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.

Message #82 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Wed, 05 Mar 2025 15:16:36 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Tue, 4 Mar 2025 19:58:05 -0800
> Cc: Philipp Stephani <phst <at> google.com>, 76313 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, 
> 	Tassilo Horn <tsdh <at> gnu.org>
> 
> FWIW, I think that I'd personally rather see another function than
> complicating the `replace-region` interface just for this.

And I'm of the directly opposite opinion.  I think having 2 APIs for
this is too much; adding a 3rd is overboard.

I proposed some ways to avoid adding yet another API.  Yes, they are
less than ideal, but then we don't develop Emacs from scratch here, so
compromises are unavoidable.  (And it bothers me that two of the most
prolific Emacs contributors don't see t5he terrible downsides of
exponential explosion in the number of Emacs functions and variables.)

Message #85 received at 76313 <at> debbugs.gnu.org (full text, mbox):

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: Re: bug#76313: New function `replace-region`
Date: Wed, 05 Mar 2025 09:21:51 -0500

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

Mostly.  And that's the point: to try and allow forward progress while
we try and resolve the remaining disagreement.

> It sounds to me like a good change, in any case.

Eli?


        Stefan

Message #88 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#76313: New function `replace-region`
Date: Wed, 05 Mar 2025 17:16:55 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Philipp Stephani <phst <at> google.com>,  76313 <at> debbugs.gnu.org,  Eli
>  Zaretskii <eliz <at> gnu.org>,  Tassilo Horn <tsdh <at> gnu.org>
> Date: Wed, 05 Mar 2025 09:21:51 -0500
> 
> >> 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?
> 
> Mostly.  And that's the point: to try and allow forward progress while
> we try and resolve the remaining disagreement.
> 
> > It sounds to me like a good change, in any case.
> 
> Eli?

I don't think I understand what is the question I'm being asked here.

If the suggestion is to extend replace-region-contents, then you will
recall that this was your first proposal, with which I agreed in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76313#14, and again in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76313#29, but you then
said in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76313#32 you
didn't like that?

Message #91 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Wed, 5 Mar 2025 11:28:42 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Tue, 4 Mar 2025 19:58:05 -0800
>> Cc: Philipp Stephani <phst <at> google.com>, 76313 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
>> 	Tassilo Horn <tsdh <at> gnu.org>
>>
>> FWIW, I think that I'd personally rather see another function than
>> complicating the `replace-region` interface just for this.
>
> And I'm of the directly opposite opinion.  I think having 2 APIs for
> this is too much; adding a 3rd is overboard.
>
> I proposed some ways to avoid adding yet another API.  Yes, they are
> less than ideal, but then we don't develop Emacs from scratch here, so
> compromises are unavoidable.  (And it bothers me that two of the most
> prolific Emacs contributors don't see t5he terrible downsides of
> exponential explosion in the number of Emacs functions and variables.)

My proposal is to obsolete `replace-buffer-contents`, as per Stefan
Monnier's suggestion.[1]  This is orthogonal to adding `replace-region`,
but means that we only have one API.

Adding `replace-region` would then bring us up to 2 APIs.  This is the
same as what we have now.

To make things even clearer, we could consider renaming
`replace-region-contents` to `carefully-replace-region`, or something
along those lines.  Its docstring and manual entry should emphasize it's
specialized nature.  Then, we have only one API that we really recommend
for this, plus the specialized function for the rare cases when you
really need it.

(BTW, since `replace-region-contents` is currently unused in our tree,
maybe we don't even need it.  OTOH, if we rename it, it shouldn't hurt
to leave it alone, in case it will be needed again in the future.)

Footnotes:
[1]  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76313#76

Message #96 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Thu, 06 Mar 2025 10:47:40 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Wed, 5 Mar 2025 11:28:42 -0800
> Cc: monnier <at> iro.umontreal.ca, 76313 <at> debbugs.gnu.org, tsdh <at> gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> FWIW, I think that I'd personally rather see another function than
> >> complicating the `replace-region` interface just for this.
> >
> > And I'm of the directly opposite opinion.  I think having 2 APIs for
> > this is too much; adding a 3rd is overboard.
> >
> > I proposed some ways to avoid adding yet another API.  Yes, they are
> > less than ideal, but then we don't develop Emacs from scratch here, so
> > compromises are unavoidable.  (And it bothers me that two of the most
> > prolific Emacs contributors don't see t5he terrible downsides of
> > exponential explosion in the number of Emacs functions and variables.)
> 
> My proposal is to obsolete `replace-buffer-contents`, as per Stefan
> Monnier's suggestion.[1]

Why would we obsolete a function that is used in half a dozen places
in our own sources?

And anyway, obsoleting a function doesn't remove it from Emacs until
many years in the future.

> Adding `replace-region` would then bring us up to 2 APIs.  This is the
> same as what we have now.

This trick doesn't solve my problem, sorry.

> To make things even clearer, we could consider renaming
> `replace-region-contents` to `carefully-replace-region`, or something
> along those lines.  Its docstring and manual entry should emphasize it's
> specialized nature.  Then, we have only one API that we really recommend
> for this, plus the specialized function for the rare cases when you
> really need it.

I'm not opposed to renaming too much (although it does have its
non-negligible price), but it doesn't solve the problem, either: the
other APIs still exist, so the potential confusion what to use when is
still there.  I very much doubt that you will be able to catch all the
subtleties in our documentation to avoid the confusion, but feel free
to try convincing me otherwise.

> (BTW, since `replace-region-contents` is currently unused in our tree,
> maybe we don't even need it.  OTOH, if we rename it, it shouldn't hurt
> to leave it alone, in case it will be needed again in the future.)

OTOH, my suggestion to extend replace-buffer-contents instead was
rejected by Stefan Monnier more strongly than the suggestion to extend
replace-region-contents.  So it sounds like Stefan favors
replace-region-contents.

Message #99 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Fri, 7 Mar 2025 20:23:09 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> To make things even clearer, we could consider renaming
>> `replace-region-contents` to `carefully-replace-region`, or something
>> along those lines.  Its docstring and manual entry should emphasize it's
>> specialized nature.  Then, we have only one API that we really recommend
>> for this, plus the specialized function for the rare cases when you
>> really need it.
>
> I'm not opposed to renaming too much (although it does have its
> non-negligible price),

Fully agreed.  It's more painful to rename things that are used often,
of course.

> but it doesn't solve the problem, either: the
> other APIs still exist, so the potential confusion what to use when is
> still there.  I very much doubt that you will be able to catch all the
> subtleties in our documentation to avoid the confusion, but feel free
> to try convincing me otherwise.

I don't fully understand what exactly you are asking for here.  Would
you like me to propose a documentation patch on top of Stefan Monnier's
patch?

I don't know if this is sufficient, but I suggest putting
`replace-region` at the top of (info "(elisp) Replacing").  I don't
think it will be very hard to explain the specialized nature of a
renamed `replace-region-carefully`.  It's doable even if we don't rename
it, I think, although the `replace-region-contents` name is a fair bit
less self-explanatory.

I'm sure that we can come up with good documentation if we collaborate
to make it so; we usually do.

Message #102 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 08 Mar 2025 10:48:59 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Fri, 7 Mar 2025 20:23:09 +0000
> Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > but it doesn't solve the problem, either: the
> > other APIs still exist, so the potential confusion what to use when is
> > still there.  I very much doubt that you will be able to catch all the
> > subtleties in our documentation to avoid the confusion, but feel free
> > to try convincing me otherwise.
> 
> I don't fully understand what exactly you are asking for here.  Would
> you like me to propose a documentation patch on top of Stefan Monnier's
> patch?
> 
> I don't know if this is sufficient, but I suggest putting
> `replace-region` at the top of (info "(elisp) Replacing").  I don't
> think it will be very hard to explain the specialized nature of a
> renamed `replace-region-carefully`.  It's doable even if we don't rename
> it, I think, although the `replace-region-contents` name is a fair bit
> less self-explanatory.
> 
> I'm sure that we can come up with good documentation if we collaborate
> to make it so; we usually do.

I'm saying that the documentation aspect of this is not the real
problem.  Once we agree on the code and API changes, updating the
documentation is a no-brainer, and I don't expect it to trigger an
argument.

The problem here is how to provide the functionality without
complicating Emacs with yet another API, where we already have two.

Message #105 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 8 Mar 2025 01:46:20 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

> I'm saying that the documentation aspect of this is not the real
> problem.  Once we agree on the code and API changes, updating the
> documentation is a no-brainer, and I don't expect it to trigger an
> argument.
>
> The problem here is how to provide the functionality without
> complicating Emacs with yet another API, where we already have two.

OK, thanks.  That makes it more clear to me what you're saying.

I don't have any better solution to propose than:

    a) Obsoleting `replace-buffer-contents`.

    b) Renaming `replace-region-contents` to `replace-region-carefully`.

I think both of these would make sense independently of `replace-region`.
How about we move ahead with those, and then revisit the situation once
that is done?

Message #108 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 08 Mar 2025 13:28:41 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Sat, 8 Mar 2025 01:46:20 -0800
> Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I'm saying that the documentation aspect of this is not the real
> > problem.  Once we agree on the code and API changes, updating the
> > documentation is a no-brainer, and I don't expect it to trigger an
> > argument.
> >
> > The problem here is how to provide the functionality without
> > complicating Emacs with yet another API, where we already have two.
> 
> OK, thanks.  That makes it more clear to me what you're saying.
> 
> I don't have any better solution to propose than:
> 
>     a) Obsoleting `replace-buffer-contents`.
> 
>     b) Renaming `replace-region-contents` to `replace-region-carefully`.
> 
> I think both of these would make sense independently of `replace-region`.
> How about we move ahead with those, and then revisit the situation once
> that is done?

I'm not sure I understand: are you suggesting to do the above, and
_then_ add a new replace-region function?  I thought I already
explained why doing so will not resolve the problems I had with the
addition of a new API?

Message #111 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 8 Mar 2025 03:50:17 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Sat, 8 Mar 2025 01:46:20 -0800
>> Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> > I'm saying that the documentation aspect of this is not the real
>> > problem.  Once we agree on the code and API changes, updating the
>> > documentation is a no-brainer, and I don't expect it to trigger an
>> > argument.
>> >
>> > The problem here is how to provide the functionality without
>> > complicating Emacs with yet another API, where we already have two.
>>
>> OK, thanks.  That makes it more clear to me what you're saying.
>>
>> I don't have any better solution to propose than:
>>
>>     a) Obsoleting `replace-buffer-contents`.
>>
>>     b) Renaming `replace-region-contents` to `replace-region-carefully`.
>>
>> I think both of these would make sense independently of `replace-region`.
>> How about we move ahead with those, and then revisit the situation once
>> that is done?
>
> I'm not sure I understand: are you suggesting to do the above, and

Do we agree about points a) and b) above?  If yes, can we do that now?
Alternatively, can we do either a) or b)?

> I'm not sure I understand: are you suggesting to do the above, and
> _then_ add a new replace-region function?  I thought I already
> explained why doing so will not resolve the problems I had with the
> addition of a new API?

I actually don't have any proposal for how to proceed after a) and b).
I don't know what might be needed.  The best proposals that exist have
been detailed in this thread.

I think it's for you to say, either "here's what I need to feel
comfortable with adding `replace-region`", or "I see no way of making me
feel comfortable with adding `replace-region`".  I'm currently reading
you as saying the latter.

If my reading is correct, I think we should close this issue after we
have decided what to do about my a) and b) above.  And then move on.

Message #114 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 08 Mar 2025 15:15:33 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Sat, 8 Mar 2025 03:50:17 -0800
> Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> I don't have any better solution to propose than:
> >>
> >>     a) Obsoleting `replace-buffer-contents`.
> >>
> >>     b) Renaming `replace-region-contents` to `replace-region-carefully`.
> >>
> >> I think both of these would make sense independently of `replace-region`.
> >> How about we move ahead with those, and then revisit the situation once
> >> that is done?
> >
> > I'm not sure I understand: are you suggesting to do the above, and
> 
> Do we agree about points a) and b) above?  If yes, can we do that now?
> Alternatively, can we do either a) or b)?

I don't think it's important.  We should discuss this after we solve
the real problem.

> > I'm not sure I understand: are you suggesting to do the above, and
> > _then_ add a new replace-region function?  I thought I already
> > explained why doing so will not resolve the problems I had with the
> > addition of a new API?
> 
> I actually don't have any proposal for how to proceed after a) and b).
> I don't know what might be needed.  The best proposals that exist have
> been detailed in this thread.
> 
> I think it's for you to say, either "here's what I need to feel
> comfortable with adding `replace-region`", or "I see no way of making me
> feel comfortable with adding `replace-region`".  I'm currently reading
> you as saying the latter.

I already suggested how to add it without adding a new API.  And
AFAIU, Stefan Monnier almost agreed with that.

> If my reading is correct, I think we should close this issue after we
> have decided what to do about my a) and b) above.  And then move on.

If we don't make any changes to incorporate the functionality that
Stefan Monnier wanted to add, then I see no reason to do a) or b),
either.

Message #117 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org,
 Stefan Kangas <stefankangas <at> gmail.com>, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Fri, 28 Mar 2025 00:56:45 -0400

[Message part 1 (text/plain, inline)]

I just pushed to `scratch/replace-region-contents` a small series of
patches which consolidate `replace-buffer-contents` and
`replace-region-contents`, marking `replace-buffer-contents` as obsolete
and making `replace-region-contents` usable as a replacement for the
`replace-region` I'd still prefer (the difference is just a mere
`-contents` plus an extra `0` argument, but as argued here before, this
sets a trap for the unwary user who may unwittingly end up calling
a very expensive function).

The first patch is the one which makes the actual change the subsequent
patches just change other code to use the new functionality.


        Stefan

[0001-replace-region-contents-Improve-and-promote-bug-7631.patch (text/x-diff, inline)]

From 0dfaa9633304e7529571a765dd2c6a49f84768cb Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Fri, 28 Mar 2025 00:46:53 -0400
Subject: [PATCH 1/3] (replace-region-contents): Improve and promote
 (bug#76313)

Swap the role of `replace-region-contents` and `replace-buffer-contents`,
so `replace-region-contents` is the main function, implemented in C,
and `replace-buffer-contents` is a mere wrapper (marked as obsolete).
Also remove the need to rely on narrowing and on describing the
new text as a function.
Finally, allow MAX-SECS==0 to require a cheap replacement, and
add an INHERIT argument.

* src/editfns.c (kill_buffer): New function.
(Freplace_region_contents): Rename from `Freplace_buffer_contents`.
Change calling convention to that of `replace-region-contents`.
Add more options for the SOURCE argument.  Add INHERIT argument.
Skip the costly algorithm if MAX-SECS is 0.
* src/insdel.c (replace_range): Allow NEW to be a buffer.

* lisp/subr.el (replace-buffer-contents): New implementation.
* lisp/emacs-lisp/subr-x.el (replace-region-contents): Delete.

* doc/lispref/text.texi (Replacing): Document new API for
`replace-region-contents`.  Remove documentation of
`replace-buffer-contents`.

* test/src/editfns-tests.el (replace-buffer-contents-1)
(replace-buffer-contents-2, replace-buffer-contents-bug31837):
Use `replace-region-contents`.
(editfns--replace-region): Delete.
(editfns-tests--replace-region): Use `replace-region-contents`.
Adds tests for new types of SOURCE args.
---
 doc/lispref/text.texi     |  70 ++++++++---------
 etc/NEWS                  |   7 ++
 lisp/emacs-lisp/subr-x.el |  29 -------
 lisp/subr.el              |  14 ++++
 src/editfns.c             | 154 +++++++++++++++++++++++++++-----------
 src/insdel.c              |   6 ++
 test/src/editfns-tests.el |  68 ++++++++---------
 7 files changed, 205 insertions(+), 143 deletions(-)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 18ed71fd1f5..6bf2e616bc8 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -4776,29 +4776,41 @@ Transposition
 @node Replacing
 @section Replacing Buffer Text
 
-  You can use the following function to replace the text of one buffer
-with the text of another buffer:
+  You can use the following function to replace some the text of the
+current buffer:
 
-@deffn Command replace-buffer-contents source &optional max-secs max-costs
-This function replaces the accessible portion of the current buffer
-with the accessible portion of the buffer @var{source}.  @var{source}
-may either be a buffer object or the name of a buffer.  When
-@code{replace-buffer-contents} succeeds, the text of the accessible
-portion of the current buffer will be equal to the text of the
-accessible portion of the @var{source} buffer.
+@defun replace-region-contents beg end source &optional max-secs max-costs inherit
+This function replaces the region between @var{beg} and @var{end}
+of the current buffer with the text found in @var{source} which
+is usually a string or a buffer, in which case it will use the
+accessible portion of that buffer.
 
 This function attempts to keep point, markers, text properties, and
 overlays in the current buffer intact.  One potential case where this
-behavior is useful is external code formatting programs: they
-typically write the reformatted text into a temporary buffer or file,
-and using @code{delete-region} and @code{insert-buffer-substring}
-would destroy these properties.  However, the latter combination is
-typically faster (@xref{Deletion}, and @ref{Insertion}).
-
-For its working, @code{replace-buffer-contents} needs to compare the
-contents of the original buffer with that of @var{source} which is a
-costly operation if the buffers are huge and there is a high number of
-differences between them.  In order to keep
+behavior is useful is external code formatting programs: they typically
+write the reformatted text into a temporary buffer or file, and using
+@code{insert} and @code{delete-region} would destroy these properties.
+
+However, in order to do that, @code{replace-buffer-contents} needs to
+compare the contents of the original buffer with that of @var{source},
+using a costly algorithm which makes the operation much slower than
+a simple @code{insert} and @code{delete-region}.  In many cases, you may
+not need that refinement, and you will then want to pass 0 as
+@var{max-secs} argument, so as to short-circuit that costly algorithm:
+It will then be just as fast as @code{insert} and @code{delete-region}
+while still preserving point and markers marginally better.
+
+Beyond that basic usage, if you need to use as source a subset of the
+accessible portion of a buffer, @var{source} can also be a vector
+@code{[@var{sbuf} @var{sbeg} @var{send}]} where the region between
+@var{sbeg} and @var{send} in buffer @var{sbuf} is the text
+you want to use as source.
+
+If you need the inserted text to inherit text-properties
+from the adjoining text, you can pass a non-@code{nil} value as
+@var{inherit} argument.
+
+When you do want the costly refined replacement, in order to keep
 @code{replace-buffer-contents}'s runtime in bounds, it has two
 optional arguments.
 
@@ -4813,23 +4825,11 @@ Replacing
 @code{replace-buffer-contents} returns @code{t} if a non-destructive
 replacement could be performed.  Otherwise, i.e., if @var{max-secs}
 was exceeded, it returns @code{nil}.
-@end deffn
 
-@defun replace-region-contents beg end replace-fn &optional max-secs max-costs
-This function replaces the region between @var{beg} and @var{end}
-using the given @var{replace-fn}.  The function @var{replace-fn} is
-run in the current buffer narrowed to the specified region and it
-should return either a string or a buffer replacing the region.
-
-The replacement is performed using @code{replace-buffer-contents} (see
-above) which also describes the @var{max-secs} and @var{max-costs}
-arguments and the return value.
-
-Note: If the replacement is a string, it will be placed in a temporary
-buffer so that @code{replace-buffer-contents} can operate on it.
-Therefore, if you already have the replacement in a buffer, it makes
-no sense to convert it to a string using @code{buffer-substring} or
-similar.
+Note: When using the refined replacement algorithm, if the replacement
+is a string, it will be internally copied to a temporary buffer.
+Therefore, all else being equal, it is preferable to pass a buffer than
+a string as @var{source} argument.
 @end defun
 
 @node Decompression
diff --git a/etc/NEWS b/etc/NEWS
index 1bd2fd6d486..8832873a7dd 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1713,6 +1713,13 @@ Previously, its argument was always evaluated using dynamic binding.
 
 * Lisp Changes in Emacs 31.1
 
++++
+** Improve 'replace-region-contents' to accept more forms of sources.
+It has been promoted from 'subr-x' to the C code.
+You can now directly pass it a string or a buffer rather than a function.
+Actually passing it a function is now deprecated.
+'replace-buffer-contents' is also marked as obsolete.
+
 +++
 ** New macros 'static-when' and 'static-unless'.
 Like 'static-if', these macros evaluate their condition at
diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 6414ecab394..eaa8119ead7 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -281,35 +281,6 @@ string-chop-newline
   (declare (pure t) (side-effect-free t))
   (string-remove-suffix "\n" string))
 
-(defun replace-region-contents (beg end replace-fn
-                                    &optional max-secs max-costs)
-  "Replace the region between BEG and END using REPLACE-FN.
-REPLACE-FN runs on the current buffer narrowed to the region.  It
-should return either a string or a buffer replacing the region.
-
-The replacement is performed using `replace-buffer-contents'
-which also describes the MAX-SECS and MAX-COSTS arguments and the
-return value.
-
-Note: If the replacement is a string, it'll be placed in a
-temporary buffer so that `replace-buffer-contents' can operate on
-it.  Therefore, if you already have the replacement in a buffer,
-it makes no sense to convert it to a string using
-`buffer-substring' or similar."
-  (save-excursion
-    (save-restriction
-      (narrow-to-region beg end)
-      (goto-char (point-min))
-      (let ((repl (funcall replace-fn)))
-	(if (bufferp repl)
-	    (replace-buffer-contents repl max-secs max-costs)
-	  (let ((source-buffer (current-buffer)))
-	    (with-temp-buffer
-	      (insert repl)
-	      (let ((tmp-buffer (current-buffer)))
-		(set-buffer source-buffer)
-		(replace-buffer-contents tmp-buffer max-secs max-costs)))))))))
-
 ;;;###autoload
 (defmacro named-let (name bindings &rest body)
   "Looping construct taken from Scheme.
diff --git a/lisp/subr.el b/lisp/subr.el
index af9289c0216..abdac1080cb 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -7497,6 +7497,20 @@ delete-line
   "Delete the current line."
   (delete-region (pos-bol) (pos-bol 2)))
 
+(defun replace-buffer-contents (source &optional max-secs max-costs)
+  "Replace accessible portion of current buffer with that of SOURCE.
+SOURCE can be a buffer or a string that names a buffer.
+Interactively, prompt for SOURCE.
+
+The replacement is performed using `replace-region-contents'
+which also describes the MAX-SECS and MAX-COSTS arguments and the
+return value."
+  (declare (obsolete replace-region-contents "31.1"))
+  (interactive "bSource buffer: ")
+  (replace-region-contents (point-min) (point-max)
+                           (if (stringp source) (get-buffer source) source)
+                           max-secs max-costs))
+
 (defun ensure-empty-lines (&optional lines)
   "Ensure that there are LINES number of empty lines before point.
 If LINES is nil or omitted, ensure that there is a single empty
diff --git a/src/editfns.c b/src/editfns.c
index 53d6cce7c82..5dbc39767d5 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -1914,11 +1914,20 @@ #define EARLY_ABORT(ctx) compareseq_early_abort (ctx)
 #include "minmax.h"
 #include "diffseq.h"
 
-DEFUN ("replace-buffer-contents", Freplace_buffer_contents,
-       Sreplace_buffer_contents, 1, 3, "bSource buffer: ",
-       doc: /* Replace accessible portion of current buffer with that of SOURCE.
-SOURCE can be a buffer or a string that names a buffer.
-Interactively, prompt for SOURCE.
+static void
+kill_buffer (Lisp_Object buf)
+{
+  Fkill_buffer (buf);
+}
+
+DEFUN ("replace-region-contents", Freplace_region_contents,
+       Sreplace_region_contents, 3, 6, 0,
+       doc: /* Replace the region between BEG and END with that of SOURCE.
+SOURCE can be a buffer, a string, or a vector [SBUF SBEG SEND]
+denoting the subtring SBEG..SEND of buffer SBUF.
+
+If optional argument INHERIT is non-nil, the inserted text will inherit
+properties from adjoining text.
 
 As far as possible the replacement is non-destructive, i.e. existing
 buffer contents, markers, properties, and overlays in the current
@@ -1940,18 +1949,86 @@ DEFUN ("replace-buffer-contents", Freplace_buffer_contents,
 used to provide a faster but suboptimal solution.  The default value
 is 1000000.
 
+Note: If the replacement is a string, it’ll usually be placed internally
+in a temporary buffer.  Therefore, if you already have the replacement
+in a buffer, it makes no sense to convert it to a string using
+‘buffer-substring’ or similar.
+
 This function returns t if a non-destructive replacement could be
 performed.  Otherwise, i.e., if MAX-SECS was exceeded, it returns
-nil.  */)
-  (Lisp_Object source, Lisp_Object max_secs, Lisp_Object max_costs)
+nil.
+
+SOURCE can also be a function that will be called with no argument
+and with current buffer narrowed to BEG..END and should return
+a buffer or a string.  But this is deprecated.  */)
+  (Lisp_Object beg, Lisp_Object end, Lisp_Object source,
+   Lisp_Object max_secs, Lisp_Object max_costs, Lisp_Object inherit)
 {
-  struct buffer *a = current_buffer;
-  Lisp_Object source_buffer = Fget_buffer (source);
-  if (NILP (source_buffer))
-    nsberror (source);
-  struct buffer *b = XBUFFER (source_buffer);
-  if (! BUFFER_LIVE_P (b))
+  validate_region (&beg, &end);
+  ptrdiff_t min_a = XFIXNUM (beg);
+  ptrdiff_t size_a = XFIXNUM (end) - min_a;
+  eassume (size_a >= 0);
+  bool a_empty = size_a == 0;
+  bool inh = !NILP (inherit);
+
+  if (FUNCTIONP (source))
+    {
+      specpdl_ref count = SPECPDL_INDEX ();
+      record_unwind_protect (save_restriction_restore,
+			     save_restriction_save ());
+      Fnarrow_to_region (beg, end);
+      source = calln (source);
+      unbind_to (count, Qnil);
+      }
+  ptrdiff_t min_b, size_b;
+  struct buffer *b;
+  if (STRINGP (source))
+    {
+      min_b = BEG;		/* Assuming we'll copy it into a buffer.  */
+      size_b = SCHARS (source);
+      b = NULL;
+    }
+  else if (BUFFERP (source))
+    {
+      b = XBUFFER (source);
+      min_b = BUF_BEGV (b);
+      size_b = BUF_ZV (b) - min_b;
+    }
+  else
+    {
+      CHECK_TYPE (VECTORP (source),
+		  list (Qor, Qstring, Qbuffer, Qvector), source);
+      /* Let `Faref' signal an error if it's too small.  */
+      Lisp_Object insend = Faref (source, make_fixnum (2));
+      CHECK_BUFFER (AREF (source, 0));
+      CHECK_FIXNUM (AREF (source, 1));
+      CHECK_FIXNUM (insend);
+      b = XBUFFER (AREF (source, 0));
+      min_b = XFIXNUM (AREF (source, 1));
+      size_b = XFIXNUM (insend) - min_b;
+      if (size_b < 0)
+	error ("Negative sized source range");
+      if (! (BUF_BEGV (b) <= min_b && min_b + size_b <= BUF_ZV (b)))
+	args_out_of_range_3 (AREF (source, 0), AREF (source, 0), AREF (source, 1));
+    }
+  bool b_empty = size_b == 0;
+  if (b && !BUFFER_LIVE_P (b))
     error ("Selecting deleted buffer");
+
+  /* Handle trivial cases where at least one accessible portion is
+     empty.  */
+
+  if (a_empty && b_empty)
+    return Qt;
+  else if (a_empty || b_empty
+	   || EQ (max_secs, make_fixnum (0))
+	   || EQ (max_costs, make_fixnum (0)))
+    {
+      replace_range (min_a, min_a + size_a, source, true, false, inh);
+      return Qt;
+    }
+
+  struct buffer *a = current_buffer;
   if (a == b)
     error ("Cannot replace a buffer with itself");
 
@@ -1977,36 +2054,8 @@ DEFUN ("replace-buffer-contents", Freplace_buffer_contents,
 	time_limit = tlim;
     }
 
-  ptrdiff_t min_a = BEGV;
-  ptrdiff_t min_b = BUF_BEGV (b);
-  ptrdiff_t size_a = ZV - min_a;
-  ptrdiff_t size_b = BUF_ZV (b) - min_b;
-  eassume (size_a >= 0);
-  eassume (size_b >= 0);
-  bool a_empty = size_a == 0;
-  bool b_empty = size_b == 0;
-
-  /* Handle trivial cases where at least one accessible portion is
-     empty.  */
-
-  if (a_empty && b_empty)
-    return Qt;
-
-  if (a_empty)
-    {
-      Finsert_buffer_substring (source, Qnil, Qnil);
-      return Qt;
-    }
-
-  if (b_empty)
-    {
-      del_range_both (BEGV, BEGV_BYTE, ZV, ZV_BYTE, true);
-      return Qt;
-    }
-
   specpdl_ref count = SPECPDL_INDEX ();
 
-
   ptrdiff_t diags = size_a + size_b + 3;
   ptrdiff_t del_bytes = size_a / CHAR_BIT + 1;
   ptrdiff_t ins_bytes = size_b / CHAR_BIT + 1;
@@ -2020,6 +2069,22 @@ DEFUN ("replace-buffer-contents", Freplace_buffer_contents,
   unsigned char *deletions_insertions = memset (buffer + 2 * diags, 0,
 						del_bytes + ins_bytes);
 
+  /* The rest of the code is not prepared to handle a string SOURCE.  */
+  if (!b)
+    {
+      Lisp_Object name
+	= Fgenerate_new_buffer_name (build_string (" *replace-workbuf*"), Qnil);
+      Lisp_Object workbuf = Fget_buffer_create (name, Qt);
+      b = XBUFFER (workbuf);
+      record_unwind_protect (kill_buffer, workbuf);
+      record_unwind_current_buffer ();
+      set_buffer_internal (b);
+      Fset_buffer_multibyte (STRING_MULTIBYTE (source) ? Qt : Qnil);
+      CALLN (Finsert, source);
+      set_buffer_internal (a);
+    }
+  Lisp_Object source_buffer = make_lisp_ptr (b, Lisp_Vectorlike);
+
   /* FIXME: It is not documented how to initialize the contents of the
      context structure.  This code cargo-cults from the existing
      caller in src/analyze.c of GNU Diffutils, which appears to
@@ -2053,7 +2118,7 @@ DEFUN ("replace-buffer-contents", Freplace_buffer_contents,
       Lisp_Object src = CALLN (Fvector, source_buffer,
 			       make_fixnum (BUF_BEGV (b)),
 			       make_fixnum (BUF_ZV (b)));
-      replace_range (BEGV, ZV, src, true, false, false);
+      replace_range (BEGV, ZV, src, true, false, inh);
       SAFE_FREE_UNBIND_TO (count, Qnil);
       return Qnil;
     }
@@ -2102,10 +2167,9 @@ DEFUN ("replace-buffer-contents", Freplace_buffer_contents,
           eassert (beg_a <= end_a);
           eassert (beg_b <= end_b);
           eassert (beg_a < end_a || beg_b < end_b);
-          /* FIXME: Use 'replace_range'!  */
           ASET (src, 1, make_fixed_natnum (beg_b));
           ASET (src, 2, make_fixed_natnum (end_b));
-          replace_range (beg_a, end_a, src, true, false, false);
+          replace_range (beg_a, end_a, src, true, false, inh);
 	}
       --i;
       --j;
@@ -4787,7 +4851,7 @@ syms_of_editfns (void)
 
   defsubr (&Sinsert_buffer_substring);
   defsubr (&Scompare_buffer_substrings);
-  defsubr (&Sreplace_buffer_contents);
+  defsubr (&Sreplace_region_contents);
   defsubr (&Ssubst_char_in_region);
   defsubr (&Stranslate_region_internal);
   defsubr (&Sdelete_region);
diff --git a/src/insdel.c b/src/insdel.c
index 9b770725971..22b6c3e974a 100644
--- a/src/insdel.c
+++ b/src/insdel.c
@@ -1439,6 +1439,12 @@ replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
       insbeg = 0;
       inschars = SCHARS (new);
     }
+  else if (BUFFERP (new))
+    {
+      insbuf = XBUFFER (new);
+      insbeg = BUF_BEGV (insbuf);
+      inschars = BUF_ZV (insbuf) - insbeg;
+    }
   else
     {
       CHECK_VECTOR (new);
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index c3f825c6149..3da9d4e8acd 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -289,7 +289,7 @@ replace-buffer-contents-1
             (narrow-to-region 8 13)
             (goto-char 12)
             (should (looking-at " \\'"))
-            (replace-buffer-contents source)
+            (replace-region-contents (point-min) (point-max) source)
             (should (looking-at " \\'")))
           (should (equal (marker-buffer marker) (current-buffer)))
           (should (equal (marker-position marker) 16)))
@@ -306,7 +306,7 @@ replace-buffer-contents-2
     (let ((source (current-buffer)))
       (with-temp-buffer
         (insert "foo BAR baz qux")
-        (replace-buffer-contents source)
+        (replace-region-contents (point-min) (point-max) source)
         (should (equal-including-properties
                  (buffer-string)
                  "foo bar baz qux"))))))
@@ -318,44 +318,44 @@ replace-buffer-contents-bug31837
   (switch-to-buffer "b")
   (insert-char (char-from-name "SMILE"))
   (insert "5678")
-  (replace-buffer-contents "a")
+  (replace-region-contents (point-min) (point-max) (get-buffer "a"))
   (should (equal (buffer-substring-no-properties (point-min) (point-max))
                  (concat (string (char-from-name "SMILE")) "1234"))))
 
-(defun editfns--replace-region (from to string)
-  (save-excursion
-    (save-restriction
-      (narrow-to-region from to)
-      (let ((buf (current-buffer)))
-        (with-temp-buffer
-          (let ((str-buf (current-buffer)))
-            (insert string)
-            (with-current-buffer buf
-              (replace-buffer-contents str-buf))))))))
-
 (ert-deftest editfns-tests--replace-region ()
   ;; :expected-result :failed
   (with-temp-buffer
-    (insert "here is some text")
-    (let ((m5n (copy-marker (+ (point-min) 5)))
-          (m5a (copy-marker (+ (point-min) 5) t))
-          (m6n (copy-marker (+ (point-min) 6)))
-          (m6a (copy-marker (+ (point-min) 6) t))
-          (m7n (copy-marker (+ (point-min) 7)))
-          (m7a (copy-marker (+ (point-min) 7) t)))
-      (editfns--replace-region (+ (point-min) 5) (+ (point-min) 7) "be")
-      (should (equal (buffer-string) "here be some text"))
-      (should (equal (point) (point-max)))
-      ;; Markers before the replaced text stay before.
-      (should (= m5n (+ (point-min) 5)))
-      (should (= m5a (+ (point-min) 5)))
-      ;; Markers in the replaced text can end up at either end, depending
-      ;; on whether they're advance-after-insert or not.
-      (should (= m6n (+ (point-min) 5)))
-      (should (<= (+ (point-min) 5) m6a (+ (point-min) 7)))
-      ;; Markers after the replaced text stay after.
-      (should (= m7n (+ (point-min) 7)))
-      (should (= m7a (+ (point-min) 7))))))
+    (let ((tmpbuf (current-buffer)))
+      (insert "  be  ")
+      (narrow-to-region (+ (point-min) 2) (- (point-max) 2))
+      (dolist (args `((,tmpbuf)
+                      (,(vector tmpbuf (point-min) (point-max)))
+                      (,"be")
+                      (,(vector tmpbuf (point-min) (point-max)) 0)
+                      (,"be" 0)))
+        (with-temp-buffer
+          (insert "here is some text")
+          (let ((m5n (copy-marker (+ (point-min) 5)))
+                (m5a (copy-marker (+ (point-min) 5) t))
+                (m6n (copy-marker (+ (point-min) 6)))
+                (m6a (copy-marker (+ (point-min) 6) t))
+                (m7n (copy-marker (+ (point-min) 7)))
+                (m7a (copy-marker (+ (point-min) 7) t)))
+            (apply #'replace-region-contents
+                   (+ (point-min) 5) (+ (point-min) 7) args)
+            (should (equal (buffer-string) "here be some text"))
+            (should (equal (point) (point-max)))
+            ;; Markers before the replaced text stay before.
+            (should (= m5n (+ (point-min) 5)))
+            (should (= m5a (+ (point-min) 5)))
+            ;; Markers in the replaced text can end up at either end, depending
+            ;; on whether they're advance-after-insert or not.
+            (should (= m6n (+ (point-min) 5)))
+            (should (<= (+ (point-min) 5) m6a (+ (point-min) 7)))
+            ;; Markers after the replaced text stay after.
+            (should (= m7n (+ (point-min) 7)))
+            (should (= m7a (+ (point-min) 7)))))
+        (widen)))))
 
 (ert-deftest delete-region-undo-markers-1 ()
   "Make sure we don't end up with freed markers reachable from Lisp."
-- 
2.39.5

[0002-Use-the-new-replace-region-contents.patch (text/x-diff, inline)]

From 8a80713a71271130df9d8029a007827da477b88d Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Fri, 28 Mar 2025 00:48:28 -0400
Subject: [PATCH 2/3] Use the new `replace-region-contents`

* lisp/vc/vc.el (vc-diff-restore-buffer):
* lisp/progmodes/python.el (python--do-isort):
* lisp/progmodes/eglot.el (eglot--apply-text-edits):
* lisp/minibuffer.el (completion--replace):
* lisp/help-fns.el (help-fns--signature):
* lisp/files.el (revert-buffer-insert-file-contents-delicately):
* lisp/emacs-lisp/cl-lib.el (cl--set-buffer-substring):
* lisp/json.el (json-pretty-print): Use `replace-region-contents`.
---
 lisp/emacs-lisp/cl-lib.el |  8 +++-----
 lisp/emacs-lisp/gv.el     |  2 ++
 lisp/files.el             | 11 ++++++-----
 lisp/help-fns.el          |  2 +-
 lisp/json.el              | 20 +++++++++-----------
 lisp/minibuffer.el        | 31 ++-----------------------------
 lisp/progmodes/eglot.el   |  9 ++++++---
 lisp/progmodes/python.el  |  2 +-
 lisp/vc/vc.el             |  2 +-
 9 files changed, 31 insertions(+), 56 deletions(-)

diff --git a/lisp/emacs-lisp/cl-lib.el b/lisp/emacs-lisp/cl-lib.el
index 4208160bd12..4645b4dffb1 100644
--- a/lisp/emacs-lisp/cl-lib.el
+++ b/lisp/emacs-lisp/cl-lib.el
@@ -154,12 +154,10 @@ cl-pushnew
 	`(setq ,place (cl-adjoin ,x ,place ,@keys)))
     `(cl-callf2 cl-adjoin ,x ,place ,@keys)))
 
-(defun cl--set-buffer-substring (start end val)
+(defun cl--set-buffer-substring (start end val &optional inherit)
   "Delete region from START to END and insert VAL."
-  (save-excursion (delete-region start end)
-		  (goto-char start)
-		  (insert val)
-		  val))
+  (replace-region-contents start end val 0 nil inherit)
+  val)
 
 (defun cl--set-substring (str start end val)
   (if end (if (< end 0) (incf end (length str)))
diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index b44f7dc87f3..6c949f1016b 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -684,6 +684,8 @@ buffer-string
   `(insert (prog1 ,store (erase-buffer))))
 (make-obsolete-generalized-variable 'buffer-string nil "29.1")
 
+;; FIXME: Can't use `replace-region-contents' because it's not
+;; expected to be costly, so we need to pass MAX-SECS==0.
 (gv-define-simple-setter buffer-substring cl--set-buffer-substring)
 (make-obsolete-generalized-variable 'buffer-substring nil "29.1")
 
diff --git a/lisp/files.el b/lisp/files.el
index 4e3aeeb9246..bcab246aeef 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -7261,9 +7261,9 @@ revert-buffer-with-fine-grain-max-seconds
 The command tries to preserve markers, properties and overlays.
 If the operation takes more than this time, a single
 delete+insert is performed.  Actually, this value is passed as
-the MAX-SECS argument to the function `replace-buffer-contents',
+the MAX-SECS argument to the function `replace-region-contents',
 so it is not ensured that the whole execution won't take longer.
-See `replace-buffer-contents' for more details.")
+See `replace-region-contents' for more details.")
 
 (defun revert-buffer-insert-file-contents-delicately (file-name _auto-save-p)
   "Optional function for `revert-buffer-insert-file-contents-function'.
@@ -7272,11 +7272,11 @@ revert-buffer-insert-file-contents-delicately
 
 As with `revert-buffer-insert-file-contents--default-function', FILE-NAME is
 the name of the file and AUTO-SAVE-P is non-nil if this is an auto-save file.
-Since calling `replace-buffer-contents' can take a long time, depending of
+Since calling `replace-region-contents' can take a long time, depending of
 the number of changes made to the buffer, it uses the value of the variable
 `revert-buffer-with-fine-grain-max-seconds' as a maximum time to try delicately
 reverting the buffer.  If it fails, it does a delete+insert.  For more details,
-see `replace-buffer-contents'."
+see `replace-region-contents'."
   (cond
    ((not (file-exists-p file-name))
     (error (if buffer-file-number
@@ -7299,7 +7299,8 @@ revert-buffer-insert-file-contents-delicately
                   (let ((temp-buf (current-buffer)))
                     (set-buffer buf)
                     (let ((buffer-file-name nil))
-                      (replace-buffer-contents
+                      (replace-region-contents
+                       (point-min) (point-max)
                        temp-buf
                        revert-buffer-with-fine-grain-max-seconds))))))))
       ;; See comments in revert-buffer-with-fine-grain for an explanation.
diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index cd5a0a6883f..dacf1ecbbd4 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -777,7 +777,7 @@ help-fns--signature
                      (save-excursion
                        (forward-char -1)
                        (<= (current-column) (- fill-column 12)))
-                     (cl--set-buffer-substring (- beg 3) beg " ")))))
+                     (replace-region-contents (- beg 3) beg " " 0)))))
           high-doc)))))
 
 (defun help-fns--parent-mode (function)
diff --git a/lisp/json.el b/lisp/json.el
index 6e62e594910..098bf43cd99 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -803,7 +803,7 @@ json-pretty-print
         (orig-buf (current-buffer)))
     ;; Strategy: Repeatedly `json-read' from the original buffer and
     ;; write the pretty-printed snippet to a temporary buffer.
-    ;; Use `replace-buffer-contents' to swap the original
+    ;; Use `replace-region-contents' to swap the original
     ;; region with the contents of the temporary buffer so that point,
     ;; marks, etc. are kept.
     ;; Stop as soon as we get an error from `json-read'.
@@ -825,16 +825,14 @@ json-pretty-print
                             (standard-output tmp-buf))
                         (with-current-buffer tmp-buf
                           (erase-buffer) (json--print json))
-                        (save-restriction
-                          (narrow-to-region beg (point))
-                          (replace-buffer-contents
-                           tmp-buf
-                           json-pretty-print-max-secs
-                           ;; FIXME: What's a good value here?  Can we use
-                           ;; something better, e.g., by deriving a value
-                           ;; from the size of the region?
-                           64)
-                        'keep-going))
+                        (replace-region-contents
+                         beg (point) tmp-buf
+                         json-pretty-print-max-secs
+                         ;; FIXME: What's a good value here?  Can we use
+                         ;; something better, e.g., by deriving a value
+                         ;; from the size of the region?
+                         64)
+                        'keep-going)
                     ;; EOF is expected because we json-read until we hit
                     ;; the end of the narrow region.
                     (json-end-of-file nil))))))))))
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index becb2a7faba..ae50574d803 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1398,35 +1398,8 @@ completion--replace
                               newtext)
     ;; Remove all text properties.
     (set-text-properties 0 (length newtext) nil newtext))
-  ;; Maybe this should be in subr.el.
-  ;; You'd think this is trivial to do, but details matter if you want
-  ;; to keep markers "at the right place" and be robust in the face of
-  ;; after-change-functions that may themselves modify the buffer.
-  (let ((prefix-len 0))
-    ;; Don't touch markers in the shared prefix (if any).
-    (while (and (< prefix-len (length newtext))
-                (< (+ beg prefix-len) end)
-                (eq (char-after (+ beg prefix-len))
-                    (aref newtext prefix-len)))
-      (setq prefix-len (1+ prefix-len)))
-    (unless (zerop prefix-len)
-      (setq beg (+ beg prefix-len))
-      (setq newtext (substring newtext prefix-len))))
-  (let ((suffix-len 0))
-    ;; Don't touch markers in the shared suffix (if any).
-    (while (and (< suffix-len (length newtext))
-                (< beg (- end suffix-len))
-                (eq (char-before (- end suffix-len))
-                    (aref newtext (- (length newtext) suffix-len 1))))
-      (setq suffix-len (1+ suffix-len)))
-    (unless (zerop suffix-len)
-      (setq end (- end suffix-len))
-      (setq newtext (substring newtext 0 (- suffix-len))))
-    (goto-char beg)
-    (let ((length (- end beg)))         ;Read `end' before we insert the text.
-      (insert-and-inherit newtext)
-      (delete-region (point) (+ (point) length)))
-    (forward-char suffix-len)))
+  (goto-char end)
+  (replace-region-contents beg end newtext 0.1 nil 'inherit))
 
 (defcustom completion-cycle-threshold nil
   "Number of completion candidates below which cycling is used.
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index bc70db34fb5..d635914e331 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -3845,9 +3845,12 @@ eglot--apply-text-edits
                   (let ((temp (current-buffer)))
                     (with-current-buffer source
                       (save-excursion
-                        (save-restriction
-                          (narrow-to-region beg end)
-                          (replace-buffer-contents temp)))
+                        (if (> emacs-major-version 30)
+                            (replace-region-contents beg end temp)
+                          (save-restriction
+                            (narrow-to-region beg end)
+                            (with-no-warnings
+                              (replace-buffer-contents temp)))))
                       (when reporter
                         (eglot--reporter-update reporter (cl-incf done))))))))
             (mapcar (lambda (edit)
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index b6db6097d9f..de3745a036c 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -6931,7 +6931,7 @@ python--do-isort
             (unless (eq 0 status)
               (error "%s exited with status %s (maybe isort is missing?)"
                      python-interpreter status))
-            (replace-buffer-contents temp)
+            (replace-region-contents (point-min) (point-max) temp)
             (not (eq tick (buffer-chars-modified-tick)))))))))
 
 ;;;###autoload
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 565eaabff0b..5c401f0bded 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1970,7 +1970,7 @@ vc-diff-restore-buffer
 objects, and finally killing buffer ORIGINAL."
   (with-current-buffer original
     (let ((inhibit-read-only t))
-      (replace-buffer-contents new)))
+      (replace-region-contents (point-min) (point-max) new)))
   (with-current-buffer new
     (buffer-swap-text original))
   (kill-buffer original))
-- 
2.39.5

[0003-Org-Use-new-replace-region-contents.patch (text/x-diff, inline)]

From 701e45bdcd194c3447e51eb1fec50fe252bcec62 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Fri, 28 Mar 2025 00:49:33 -0400
Subject: [PATCH 3/3] Org: Use new `replace-region-contents`

* lisp/org/org-compat.el (org-replace-buffer-contents): Delete function.
(org-replace-region-contents): New function.
* lisp/org/org-src.el (org-edit-src-save, org-edit-src-exit): Use it.
---
 lisp/org/org-compat.el | 18 ++++++++++++++----
 lisp/org/org-src.el    | 21 ++++++---------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el
index 59d34b661c6..297e8f06045 100644
--- a/lisp/org/org-compat.el
+++ b/lisp/org/org-compat.el
@@ -292,10 +292,20 @@ org-buffer-text-pixel-width
       (if tree (push tree elems))
       (nreverse elems))))
 
-(if (version< emacs-version "27.1")
-    (defsubst org-replace-buffer-contents (source &optional _max-secs _max-costs)
-      (replace-buffer-contents source))
-  (defalias 'org-replace-buffer-contents #'replace-buffer-contents))
+(defalias 'org-replace-region-contents
+  (if (> emacs-major-version 30)
+      #'replace-region-contents
+    ;; The `replace-region-contents' in Emacs<31 does not accept a buffer
+    ;; as SOURCE argument and does not preserve the position well enough.
+    (lambda (beg end source &optional max-secs max-costs)
+      (save-restriction
+        (narrow-to-region beg end)
+        (let ((eobp (eobp)))
+          (with-no-warnings
+            (if (< emacs-major-version 27)
+                (replace-buffer-contents source)
+              (replace-buffer-contents source max-secs max-costs)))
+          (if eobp (goto-char (point-max))))))))
 
 (unless (fboundp 'proper-list-p)
   ;; `proper-list-p' was added in Emacs 27.1.  The function below is
diff --git a/lisp/org/org-src.el b/lisp/org/org-src.el
index 302c27ac866..d8a928b1f9f 100644
--- a/lisp/org/org-src.el
+++ b/lisp/org/org-src.el
@@ -1414,13 +1414,9 @@ org-edit-src-save
       ;; insert new contents.
       (delete-overlay overlay)
       (let ((expecting-bol (bolp)))
-	(if (version< emacs-version "27.1")
-	    (progn (delete-region beg end)
-		   (insert (with-current-buffer write-back-buf (buffer-string))))
-	  (save-restriction
-	    (narrow-to-region beg end)
-	    (org-replace-buffer-contents write-back-buf 0.1 nil)
-	    (goto-char (point-max))))
+        (goto-char end)
+        (org-replace-region-contents beg end write-back-buf 0.1 nil)
+        (cl-assert (= (point) (+ beg (buffer-size write-back-buf))))
 	(when (and expecting-bol (not (bolp))) (insert "\n")))
       (kill-buffer write-back-buf)
       (save-buffer)
@@ -1461,14 +1457,9 @@ org-edit-src-exit
        (undo-boundary)
        (goto-char beg)
        (let ((expecting-bol (bolp)))
-	 (if (version< emacs-version "27.1")
-	     (progn (delete-region beg end)
-		    (insert (with-current-buffer write-back-buf
-                              (buffer-string))))
-	   (save-restriction
-	     (narrow-to-region beg end)
-	     (org-replace-buffer-contents write-back-buf 0.1 nil)
-	     (goto-char (point-max))))
+         (goto-char end)
+         (org-replace-region-contents beg end write-back-buf 0.1 nil)
+         (cl-assert (= (point) (+ beg (buffer-size write-back-buf))))
 	 (when (and expecting-bol (not (bolp))) (insert "\n")))))
     (when write-back-buf (kill-buffer write-back-buf))
     ;; If we are to return to source buffer, put point at an
-- 
2.39.5

Message #120 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org,
 Stefan Kangas <stefankangas <at> gmail.com>, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Fri, 28 Mar 2025 11:38:19 -0400

> (the difference is just a mere `-contents` plus an extra `0` argument,
> but as argued here before, this sets a trap for the unwary user who
> may unwittingly end up calling a very expensive function).

The main part is not the `-contents` but the extra arg needed to avoid
the risk of spending half-an-hour waiting for the command to finish
looking for diffs.
Any chance we could consider a  breaking change and force a non-nil value of
one of MAX-SECS or MAX-COSTS before the function is allowed to use the
costly algorithm?

After all, it's a "soft breakage" since the algorithm doesn't make
any promises about what is recognized as "unchanged".

I took a look at the uses I could find "out there" (see below my sig)
and the overwhelming impression is that the change would have a very
minor impact, and of course would be easy to fix in
a backward-compatible manner.  [ Note that I do not suggest making
a similar change to `replace-buffer-contents`.  ]


        Stefan


There are currently no uses in Emacs itself.
I found one use in GNU ELPA:

    packages/sketch-mode/sketch-mode.el-      (with-current-buffer (print (marker-buffer s))
    packages/sketch-mode/sketch-mode.el-        (let ((inhibit-read-only t))
    packages/sketch-mode/sketch-mode.el:          (replace-region-contents s e (lambda () (concat "OPACITY: "
    packages/sketch-mode/sketch-mode.el-                                                     (format (if sketch-opacity
    packages/sketch-mode/sketch-mode.el-                                                                 "%.2f"
    packages/sketch-mode/sketch-mode.el-                                                               "%s"
    packages/sketch-mode/sketch-mode.el-                                                               )
    packages/sketch-mode/sketch-mode.el-                                                             sketch-opacity)
    packages/sketch-mode/sketch-mode.el-                                                     "\n ")))
    packages/sketch-mode/sketch-mode.el-      (put-text-property (1- e) e 'display (svg-image 
    packages/sketch-mode/sketch-mode.el-                                                (let* ((w 220)
    packages/sketch-mode/sketch-mode.el-                                                       (h (default-font-height))
    packages/sketch-mode/sketch-mode.el-                                                       (half-h (/ h 2))

Having tested this code, I know that careful preservation of markers and
such is of no importance so the proposed change would bring only a (minor)
improvement in performance.

I found one/two use in NonGNU ELPA:

    packages/parinfer-rust-mode/test/test-helper.el-    (goto-line (+ 1 (cdr (assoc 'lineNo change))))) ;; This is normally bad, but we're just doing this in a test
    packages/parinfer-rust-mode/test/test-helper.el-  (forward-char (cdr (assoc 'x change)))            ;; and we need to go to the line specified by the current change
    packages/parinfer-rust-mode/test/test-helper.el:  (replace-region-contents (point)
    packages/parinfer-rust-mode/test/test-helper.el-                           (+ (point) (length (cdr (assoc 'newText change))))
    packages/parinfer-rust-mode/test/test-helper.el-                           (lambda (&rest _args) (cdr (assoc 'oldText change)))))
    packages/parinfer-rust-mode/test/test-helper.el-
    packages/parinfer-rust-mode/test/test-helper.el-(defun apply-changes-in-buffer (change)
    packages/parinfer-rust-mode/test/test-helper.el-  "Given a list of CHANGE apply to the specified area of the buffer."
    packages/parinfer-rust-mode/test/test-helper.el-  (with-no-warnings
    packages/parinfer-rust-mode/test/test-helper.el-    (goto-line (+ 1 (cdr (assoc 'lineNo change)))))
    packages/parinfer-rust-mode/test/test-helper.el-  (forward-char (cdr (assoc 'x change)))
    packages/parinfer-rust-mode/test/test-helper.el-  (setq-local parinfer-rust--test-line-no (line-number-at-pos))
    packages/parinfer-rust-mode/test/test-helper.el:  (replace-region-contents (point)
    packages/parinfer-rust-mode/test/test-helper.el-                           (+ (point) (length (cdr (assoc 'oldText change))))
    packages/parinfer-rust-mode/test/test-helper.el-                           (lambda (&rest _args) (cdr (assoc 'newText change))))
    packages/parinfer-rust-mode/test/test-helper.el-  (setq-local parinfer-rust--test-line-no nil))

According to my tests, this would not be adversely affected either.
I found six/seven uses in MELPA:

    chroma-20240716.1131.tar-                 (match-end (match-end 0))
    chroma-20240716.1131.tar-                 (color-string (match-string-no-properties 0)))
    chroma-20240716.1131.tar:             (replace-region-contents (match-beginning 0) (match-end 0)
    chroma-20240716.1131.tar-                                      (lambda ()
    chroma-20240716.1131.tar-                                        (funcall color-conversion-fn
    chroma-20240716.1131.tar-                                                 color-string)))))))
    chroma-20240716.1131.tar-      (t
    chroma-20240716.1131.tar-       (cl-multiple-value-bind (color-string start end)
    chroma-20240716.1131.tar-           (funcall color-string-at-point-fn (point))
    chroma-20240716.1131.tar:         (replace-region-contents start end
    chroma-20240716.1131.tar-                                  (lambda ()
    chroma-20240716.1131.tar-                                    (funcall color-conversion-fn
    chroma-20240716.1131.tar-                                             color-string)))))))))

This is in a function that replaces one color name with another and
would not be significantly affected, AFAICT.

    ekg-20250313.436.tar-(defun ekg-edit-display-metadata ()
    ekg-20250313.436.tar-  "Create or edit the overlay to show metadata."
    ekg-20250313.436.tar-  (let ((o (ekg--metadata-overlay))
    ekg-20250313.436.tar-        (inhibit-read-only t))
    ekg-20250313.436.tar-    (buffer-disable-undo)
    ekg-20250313.436.tar:    (replace-region-contents (overlay-start o) (overlay-end o)
    ekg-20250313.436.tar-                             #'ekg--replace-metadata)
    ekg-20250313.436.tar-    (goto-char (overlay-end o))
    ekg-20250313.436.tar-    (insert "\n")
    ekg-20250313.436.tar-    (move-overlay o (point-min) (- (overlay-end o) 1))

AFAICT, again this wouldn't be significantly affected either.

    elisp-demos-20240128.810.tar-(with-temp-buffer
    elisp-demos-20240128.810.tar-  (insert "hello")
    elisp-demos-20240128.810.tar:  (replace-region-contents (point-min) (point-max) (lambda () '"world"))
    elisp-demos-20240128.810.tar-  (buffer-string))
    elisp-demos-20240128.810.tar-#+END_SRC
    elisp-demos-20240128.810.tar-
    elisp-demos-20240128.810.tar-#+RESULTS:
    elisp-demos-20240128.810.tar-: "world"
    elisp-demos-20240128.810.tar-

Not affected.

    hindent-20241128.1601.tar-                  (progn
    hindent-20241128.1601.tar-                    (if (fboundp 'replace-region-contents)
    hindent-20241128.1601.tar:                        (replace-region-contents
    hindent-20241128.1601.tar-                         beg end (lambda () temp))
    hindent-20241128.1601.tar-                      (let ((line (line-number-at-pos))
    hindent-20241128.1601.tar-                            (col (current-column)))
    hindent-20241128.1601.tar-                        (delete-region beg
    hindent-20241128.1601.tar-                                       end)
    hindent-20241128.1601.tar-                        (insert new-str)))
    hindent-20241128.1601.tar-                    (message "Formatted."))
    hindent-20241128.1601.tar-                  (message "Already formatted.")))))))))))

This one would be affected a bit: the package passes the region through
an external indenter/prettyprinter, so the use of careful replacement
to better preserves the markers in the mark-ring is relevant.
As the code shows, it's not the end of the world: not only it still has
the fallback insert+delete code but that fallback wasn't even careful to do
the insertion before the delete.

Also, this use case would be trivial to fix by passing the additional
MAX-SECS argument, of course (there is no backward compatibility in
this respect: all version of `replace-region-contents` have accepted
the MAX-SECS arg, contrary to `replace-buffer-contents`).

    jet-20240730.1228.tar-(defun jet-paste-cursor (thing &optional args)
    jet-20240730.1228.tar-  "Run jet for THING at cursor and ARGS pasting to current buffer."
    jet-20240730.1228.tar-  (interactive (jet-menu--interactive-args))
    jet-20240730.1228.tar-  (let ((result (string-trim (jet-menu--run thing args))))
    jet-20240730.1228.tar-    (if (use-region-p)
    jet-20240730.1228.tar:        (replace-region-contents (region-beginning) (region-end) (lambda () result))
    jet-20240730.1228.tar-      (insert result))))

This runs the `jet` command which converts data between different
representations (JSON/EDN/Transit?), so it's similar to hindent above:
it's nice to preserve things like mark-ring positions along the way, but
it's not the end of the world and shouldn't result in actual breakage,
just a slightly less refined behavior, which can be recovered by
a trivial change to the code.

    org-mpv-notes-20241222.1958.tar-(defun org-mpv-notes-replace-timestamp-with-link (begin end link)
    org-mpv-notes-20241222.1958.tar-  "Convert hh:mm:ss text within region to link with timestamp.
    org-mpv-notes-20241222.1958.tar-Region is between `BEGIN' and `END' points,
    org-mpv-notes-20241222.1958.tar-`LINK' is the new media url/path."
    org-mpv-notes-20241222.1958.tar-  (interactive "r\nsLink:")
    org-mpv-notes-20241222.1958.tar-  (save-excursion
    org-mpv-notes-20241222.1958.tar-    (let (timestamp)
    org-mpv-notes-20241222.1958.tar-      (setq link (org-link-escape link))
    org-mpv-notes-20241222.1958.tar-      (goto-char end)
    org-mpv-notes-20241222.1958.tar-      (while (re-search-backward "[^0-9]\\([0-9]+:[0-9]+:[0-9]+\\)" begin t)
    org-mpv-notes-20241222.1958.tar-        (setq timestamp (match-string 1))
    org-mpv-notes-20241222.1958.tar:        (replace-region-contents (match-beginning 1) (match-end 1)
    org-mpv-notes-20241222.1958.tar-                                 (lambda () (concat "[[mpv:" link "::" timestamp "][" timestamp "]]")))
    org-mpv-notes-20241222.1958.tar-        (search-backward "[[" begin t)))))

AFAICT this code should not affected by my proposed change.

Message #123 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 12:49:33 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  phst <at> google.com,
>   76313 <at> debbugs.gnu.org,  tsdh <at> gnu.org
> Date: Fri, 28 Mar 2025 00:56:45 -0400
> 
> I just pushed to `scratch/replace-region-contents` a small series of
> patches which consolidate `replace-buffer-contents` and
> `replace-region-contents`, marking `replace-buffer-contents` as obsolete
> and making `replace-region-contents` usable as a replacement for the
> `replace-region` I'd still prefer (the difference is just a mere
> `-contents` plus an extra `0` argument, but as argued here before, this
> sets a trap for the unwary user who may unwittingly end up calling
> a very expensive function).

Thanks, a few comments below.

> +@defun replace-region-contents beg end source &optional max-secs max-costs inherit
> +This function replaces the region between @var{beg} and @var{end}
> +of the current buffer with the text found in @var{source} which
> +is usually a string or a buffer, in which case it will use the
> +accessible portion of that buffer.
>  
>  This function attempts to keep point, markers, text properties, and
>  overlays in the current buffer intact.  One potential case where this
> -behavior is useful is external code formatting programs: they
> -typically write the reformatted text into a temporary buffer or file,
> -and using @code{delete-region} and @code{insert-buffer-substring}
> -would destroy these properties.  However, the latter combination is
> -typically faster (@xref{Deletion}, and @ref{Insertion}).
> -
> -For its working, @code{replace-buffer-contents} needs to compare the
> -contents of the original buffer with that of @var{source} which is a
> -costly operation if the buffers are huge and there is a high number of
> -differences between them.  In order to keep
> +behavior is useful is external code formatting programs: they typically
> +write the reformatted text into a temporary buffer or file, and using
> +@code{insert} and @code{delete-region} would destroy these properties.
> +
> +However, in order to do that, @code{replace-buffer-contents} needs to
                                       ^^^^^^^^^^^^^^^^^^^^^^^
The name of the function there is incorrect.

> +(defun replace-buffer-contents (source &optional max-secs max-costs)
> +  "Replace accessible portion of current buffer with that of SOURCE.
> +SOURCE can be a buffer or a string that names a buffer.
> +Interactively, prompt for SOURCE.
> +
> +The replacement is performed using `replace-region-contents'
> +which also describes the MAX-SECS and MAX-COSTS arguments and the
> +return value."
> +  (declare (obsolete replace-region-contents "31.1"))
> +  (interactive "bSource buffer: ")
> +  (replace-region-contents (point-min) (point-max)
> +                           (if (stringp source) (get-buffer source) source)
> +                           max-secs max-costs))

Since this is an interactive function, shouldn't it test SOURCE to be
of a valid type, instead of delegating to replace-region-contents?

> +Note: If the replacement is a string, it’ll usually be placed internally
> +in a temporary buffer.  Therefore, if you already have the replacement
> +in a buffer, it makes no sense to convert it to a string using
> +‘buffer-substring’ or similar.

This uses Unicode quotes in a doc string; please use ASCII characters
instead.

> +SOURCE can also be a function that will be called with no argument
                                                          ^^^^^^^^^^^
"no arguments", right?

> +and with current buffer narrowed to BEG..END and should return
                                               ^
A comma is missing there.

> +      b = XBUFFER (AREF (source, 0));
> +      min_b = XFIXNUM (AREF (source, 1));
> +      size_b = XFIXNUM (insend) - min_b;
> +      if (size_b < 0)
> +	error ("Negative sized source range");

Shouldn't we support here any order of SBEG and SEND, as we do in many
other places?

> +      if (! (BUF_BEGV (b) <= min_b && min_b + size_b <= BUF_ZV (b)))
> +	args_out_of_range_3 (AREF (source, 0), AREF (source, 0), AREF (source, 1));

Should this error message show all 3 values, instead of showing one of
them twice?  Or maybe show BEGV, ZV, and the offending position
(either SBEG or SEND)?

> +  bool b_empty = size_b == 0;
> +  if (b && !BUFFER_LIVE_P (b))
>      error ("Selecting deleted buffer");

What if b is NULL? can we continue?

> +  /* The rest of the code is not prepared to handle a string SOURCE.  */
> +  if (!b)
> +    {
> +      Lisp_Object name
> +	= Fgenerate_new_buffer_name (build_string (" *replace-workbuf*"), Qnil);
> +      Lisp_Object workbuf = Fget_buffer_create (name, Qt);
> +      b = XBUFFER (workbuf);
> +      record_unwind_protect (kill_buffer, workbuf);
> +      record_unwind_current_buffer ();
> +      set_buffer_internal (b);
> +      Fset_buffer_multibyte (STRING_MULTIBYTE (source) ? Qt : Qnil);

coding.c:code_conversion_save does something similar; perhaps some of
the buffer-local settings we use there could also be a good idea here?

> --- a/src/insdel.c
> +++ b/src/insdel.c
> @@ -1439,6 +1439,12 @@ replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
>        insbeg = 0;
>        inschars = SCHARS (new);
>      }
> +  else if (BUFFERP (new))
> +    {
> +      insbuf = XBUFFER (new);
> +      insbeg = BUF_BEGV (insbuf);
> +      inschars = BUF_ZV (insbuf) - insbeg;
> +    }

Please update the commentary to replace_range to describe this type of
NEW.

Message #126 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 12:59:28 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  phst <at> google.com,
>   76313 <at> debbugs.gnu.org,  tsdh <at> gnu.org
> Date: Fri, 28 Mar 2025 11:38:19 -0400
> 
> Any chance we could consider a  breaking change and force a non-nil value of
> one of MAX-SECS or MAX-COSTS before the function is allowed to use the
> costly algorithm?

I'd prefer a completely backward-compatible change of passing a
special value of MAX-SECS to indicate that the potentially costly
algorithm is to be bypassed.

Message #129 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 05:46:51 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
>> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  phst <at> google.com,
>>   76313 <at> debbugs.gnu.org,  tsdh <at> gnu.org
>> Date: Fri, 28 Mar 2025 11:38:19 -0400
>>
>> Any chance we could consider a  breaking change and force a non-nil value of
>> one of MAX-SECS or MAX-COSTS before the function is allowed to use the
>> costly algorithm?
>
> I'd prefer a completely backward-compatible change of passing a
> special value of MAX-SECS to indicate that the potentially costly
> algorithm is to be bypassed.

We can keep trying to shoe-horn `replace-region-contents` into something
that looks like a function intended for general use, and document it as
such.  If we go that route, I think it would make sense to make the
change Stefan M proposes.

Alternatively, we let it live on as a proper citizen of subr-x.el and
consequently remove it from the manual.  We've managed without
`replace-region` for decades already, so there's no urgent need to
elevate a design that has more than a few rough edges.

Personally, I'd prefer the latter, to avoid encouraging wider use of an
interface that might not yet be fully thought through.

Message #132 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 16:05:00 +0300

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Sat, 29 Mar 2025 05:46:51 -0700
> Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, tsdh <at> gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> >> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  phst <at> google.com,
> >>   76313 <at> debbugs.gnu.org,  tsdh <at> gnu.org
> >> Date: Fri, 28 Mar 2025 11:38:19 -0400
> >>
> >> Any chance we could consider a  breaking change and force a non-nil value of
> >> one of MAX-SECS or MAX-COSTS before the function is allowed to use the
> >> costly algorithm?
> >
> > I'd prefer a completely backward-compatible change of passing a
> > special value of MAX-SECS to indicate that the potentially costly
> > algorithm is to be bypassed.
> 
> We can keep trying to shoe-horn `replace-region-contents` into something
> that looks like a function intended for general use, and document it as
> such.  If we go that route, I think it would make sense to make the
> change Stefan M proposes.

Can you explain why it does NOT make sense to do what I proposed?  It
gives us what Stefan wanted, but avoids the backward incompatibility
(unless I'm missing something).

> Alternatively, we let it live on as a proper citizen of subr-x.el and
> consequently remove it from the manual.  We've managed without
> `replace-region` for decades already, so there's no urgent need to
> elevate a design that has more than a few rough edges.

??? I'm confused: Stefan made replace-region-contents our preferred
API, and demoted replace-buffer.  Are you suggesting to do the
opposite now?

Message #135 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 06:57:48 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Sat, 29 Mar 2025 05:46:51 -0700
>> Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, tsdh <at> gnu.org
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
>> >> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  phst <at> google.com,
>> >>   76313 <at> debbugs.gnu.org,  tsdh <at> gnu.org
>> >> Date: Fri, 28 Mar 2025 11:38:19 -0400
>> >>
>> >> Any chance we could consider a  breaking change and force a non-nil value of
>> >> one of MAX-SECS or MAX-COSTS before the function is allowed to use the
>> >> costly algorithm?
>> >
>> > I'd prefer a completely backward-compatible change of passing a
>> > special value of MAX-SECS to indicate that the potentially costly
>> > algorithm is to be bypassed.
>>
>> We can keep trying to shoe-horn `replace-region-contents` into something
>> that looks like a function intended for general use, and document it as
>> such.  If we go that route, I think it would make sense to make the
>> change Stefan M proposes.
>
> Can you explain why it does NOT make sense to do what I proposed?  It
> gives us what Stefan wanted, but avoids the backward incompatibility
> (unless I'm missing something).

My understanding is that your proposal means that the potentially costly
algorithm is used by default, and that users should explicitly say so if
they don't want it.

That would be the best way to do it, if the normal case is that you want
the costly algorithm.  But my understanding so far is that, in the
overwhelming majority of cases, you do not want this.

>> Alternatively, we let it live on as a proper citizen of subr-x.el and
>> consequently remove it from the manual.  We've managed without
>> `replace-region` for decades already, so there's no urgent need to
>> elevate a design that has more than a few rough edges.
>
> ??? I'm confused: Stefan made replace-region-contents our preferred
> API, and demoted replace-buffer.  Are you suggesting to do the
> opposite now?

I agree that it's a step forward to demote `replace-buffer-contents`.

I submit that it's worth considering that we might better off with just
a function in subr-x.el, for when you need it, than officially blessing
a problematic or suboptimal function for general use.

Personally, I would feel reluctant to use this function.  For example, I
don't know how to specify a meaningful value for MAX-COSTS.  It is
documented as specifying "the quality of the difference computation",
which does not help much.  I think our users will feel this even more.

No other buffer editing primitives have anything resembling the MAX-SECS
and MAX-COSTS arguments.  I find the design choice to expose those
arguments a bit unusual in a specialized function, and inappropriate in
a function intended for general use.

That said, I understand that they are occasionally useful, or they
wouldn't exist.  I would make them internal to the function, with
sensible defaults that users could override with some variable when they
need to.  That change could be done in a backwards-compatible way with
advertised-calling-convention', and I would suggest we do that.

With that change, and the one proposed by Stefan Monnier above, I can
see that keeping `replace-region-contents` is okay.  I would also
suggest adding the alias `replace-region` for improved ergonomics and
consistency (e.g., with `delete-region`), but that is less important.
We can live with a long name.

Message #138 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 17:28:35 +0300

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Sat, 29 Mar 2025 06:57:48 -0700
> Cc: monnier <at> iro.umontreal.ca, 76313 <at> debbugs.gnu.org, tsdh <at> gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> We can keep trying to shoe-horn `replace-region-contents` into something
> >> that looks like a function intended for general use, and document it as
> >> such.  If we go that route, I think it would make sense to make the
> >> change Stefan M proposes.
> >
> > Can you explain why it does NOT make sense to do what I proposed?  It
> > gives us what Stefan wanted, but avoids the backward incompatibility
> > (unless I'm missing something).
> 
> My understanding is that your proposal means that the potentially costly
> algorithm is used by default, and that users should explicitly say so if
> they don't want it.
> 
> That would be the best way to do it, if the normal case is that you want
> the costly algorithm.  But my understanding so far is that, in the
> overwhelming majority of cases, you do not want this.

I don't think this assumption is true.  The slow and costly algorithm
keeps the markers and other properties, whereas bypassing it will not
do so.  So the trade-off is not clear and depends on the context.

> >> Alternatively, we let it live on as a proper citizen of subr-x.el and
> >> consequently remove it from the manual.  We've managed without
> >> `replace-region` for decades already, so there's no urgent need to
> >> elevate a design that has more than a few rough edges.
> >
> > ??? I'm confused: Stefan made replace-region-contents our preferred
> > API, and demoted replace-buffer.  Are you suggesting to do the
> > opposite now?
> 
> I agree that it's a step forward to demote `replace-buffer-contents`.
> 
> I submit that it's worth considering that we might better off with just
> a function in subr-x.el, for when you need it, than officially blessing
> a problematic or suboptimal function for general use.

I don't see any blessing.  We have 2 functions before; after Stefan's
changes we will have one function and another one obsolete.

> Personally, I would feel reluctant to use this function.  For example, I
> don't know how to specify a meaningful value for MAX-COSTS.  It is
> documented as specifying "the quality of the difference computation",
> which does not help much.  I think our users will feel this even more.
> 
> No other buffer editing primitives have anything resembling the MAX-SECS
> and MAX-COSTS arguments.  I find the design choice to expose those
> arguments a bit unusual in a specialized function, and inappropriate in
> a function intended for general use.

The original version of replace-buffer-contents didn't have any such
arguments.  You called the function and had to wait for however long it
took to do its job.  We added these arguments to give Lisp programs
more control, depending on the context and the importance of keeping
the text which doesn't need to be changed.  Now we are discussing how
to give Lisp programs even more control.

> That said, I understand that they are occasionally useful, or they
> wouldn't exist.  I would make them internal to the function, with
> sensible defaults that users could override with some variable when they
> need to.  That change could be done in a backwards-compatible way with
> advertised-calling-convention', and I would suggest we do that.

I'm not sure I see how this would be better than what we have now.  It
has to be significantly better to justify incompatible changes like
that.  If we add an easy way to bypass the costly part, for the Lisp
programs that don't care enough about the consequences, I think we
will have the best of all worlds.

Message #141 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 17:58:55 -0400

>> +(defun replace-buffer-contents (source &optional max-secs max-costs)
>> +  "Replace accessible portion of current buffer with that of SOURCE.
>> +SOURCE can be a buffer or a string that names a buffer.
>> +Interactively, prompt for SOURCE.
>> +
>> +The replacement is performed using `replace-region-contents'
>> +which also describes the MAX-SECS and MAX-COSTS arguments and the
>> +return value."
>> +  (declare (obsolete replace-region-contents "31.1"))
>> +  (interactive "bSource buffer: ")
>> +  (replace-region-contents (point-min) (point-max)
>> +                           (if (stringp source) (get-buffer source) source)
>> +                           max-secs max-costs))
>
> Since this is an interactive function, shouldn't it test SOURCE to be
> of a valid type, instead of delegating to replace-region-contents?

I was being sloppy because it's obsolete anyway, but actually it's
simpler to always pass the arg to `get-buffer`, so you're quite right!

>> +Note: If the replacement is a string, it’ll usually be placed internally
>> +in a temporary buffer.  Therefore, if you already have the replacement
>> +in a buffer, it makes no sense to convert it to a string using
>> +‘buffer-substring’ or similar.
>
> This uses Unicode quotes in a doc string; please use ASCII characters
> instead.

Hmm... copy/paste from *Help*.  I think I'm going to turn off those
fancy quotes because it's not the first it bites me.

>> +      b = XBUFFER (AREF (source, 0));
>> +      min_b = XFIXNUM (AREF (source, 1));
>> +      size_b = XFIXNUM (insend) - min_b;
>> +      if (size_b < 0)
>> +	error ("Negative sized source range");
>
> Shouldn't we support here any order of SBEG and SEND, as we do in many
> other places?

Hmm... I must admit I'm not a big fan of this "feature", but OK,
I changed the code to use `validate_region` as elsewhere.

>> +      if (! (BUF_BEGV (b) <= min_b && min_b + size_b <= BUF_ZV (b)))
>> +	args_out_of_range_3 (AREF (source, 0), AREF (source, 0), AREF (source, 1));
>
> Should this error message show all 3 values, instead of showing one of
> them twice?  Or maybe show BEGV, ZV, and the offending position
> (either SBEG or SEND)?

Fixed, thanks.

>> +  bool b_empty = size_b == 0;
>> +  if (b && !BUFFER_LIVE_P (b))
>>      error ("Selecting deleted buffer");
>
> What if b is NULL? can we continue?

Yes: `b` is NULL here iff source is a string, in which case it's set
later to a locally-created temp buffer.

>> +  /* The rest of the code is not prepared to handle a string SOURCE.  */
>> +  if (!b)
>> +    {
>> +      Lisp_Object name
>> +	= Fgenerate_new_buffer_name (build_string (" *replace-workbuf*"), Qnil);
>> +      Lisp_Object workbuf = Fget_buffer_create (name, Qt);
>> +      b = XBUFFER (workbuf);
>> +      record_unwind_protect (kill_buffer, workbuf);
>> +      record_unwind_current_buffer ();
>> +      set_buffer_internal (b);
>> +      Fset_buffer_multibyte (STRING_MULTIBYTE (source) ? Qt : Qnil);
>
> coding.c:code_conversion_save does something similar; perhaps some of
> the buffer-local settings we use there could also be a good idea here?

Even better: I changed the code to call `code_conversion_save`!

> Please update the commentary to replace_range to describe this type of NEW.

Done, thanks, and pushed.


        Stefan

Message #144 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 18:20:15 -0400

>> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
>> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  phst <at> google.com,
>>   76313 <at> debbugs.gnu.org,  tsdh <at> gnu.org
>> Date: Fri, 28 Mar 2025 11:38:19 -0400
>> 
>> Any chance we could consider a  breaking change and force a non-nil value of
>> one of MAX-SECS or MAX-COSTS before the function is allowed to use the
>> costly algorithm?
>
> I'd prefer a completely backward-compatible change of passing a
> special value of MAX-SECS to indicate that the potentially costly
> algorithm is to be bypassed.

We already have that: pass a 0 for MAX-SECS.

But I've changed my mind: I don't think we should unify
`replace-region-contents` and my proposed `replace-region`.

When I sent the patches there were still some tests failing, and
I assumed these were just the result of idiotic errors on my part which
I'd fix the next day (it was late).

While that was partly true, some of the problems were a lot more
delicate: while `replace-region` is "easy" to use to do what I would
describe as "edits", `replace-region-contents` is a good bit more
delicate because its effect on point is less predictable.

When point is "in the middle" of the modified text, all is fine callers
usually don't expect any specific position for point after wards, in
that case.  But if point is at the beginning (resp. at the end) of the text
being replaced, most callers expect it to still be at the beginning
(resp. the end) of the new text afterwards.

This does hold when point is at the beginning of the text but *not* when
it's at the end.  So in a typical loop scanning a buffer with
`re-search-forward` and then calling `replace-region-contents`, it's
easy to get bitten by it when occasionally we'll end up finding a match
in the text we just inserted because the "careful" version of
`replace-region-contents` moved point back somewhat.

IOW, if you have code that uses

    (replace-region-contents BEG END SOURCE 0)

and you change it to:

    (replace-region-contents BEG END SOURCE 0.1)

thinking "oh, I can afford to spend a bit more time here, let's
enable the fancy diff algorithm", you may end up breaking your code.


        Stefan

Message #147 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sun, 30 Mar 2025 00:15:54 -0400

[Message part 1 (text/plain, inline)]

>> That would be the best way to do it, if the normal case is that you want
>> the costly algorithm.  But my understanding so far is that, in the
>> overwhelming majority of cases, you do not want this.
> I don't think this assumption is true.  The slow and costly algorithm
> keeps the markers and other properties, whereas bypassing it will not
> do so.  So the trade-off is not clear and depends on the context.

My guts agree with Stefan, but I think none of us have hard data to
confirm our intuition.

This said, I did a quick:

    grep -C1 delete-region **/*.el  |
        sed 's/-\([0-9]\+\)-/:\1:/' |
        grep insert

and this gives a crapload of matches.  A completely unscientific random
sampling suggests that the majority of them could make use of
`replace-region`, but that only a smaller proportion of them are likely
to benefit from the fancy diff of `replace-region-contents`.

See the (untested) patch below resulting from my random sampling (can
you spot which ones use the fancy diff algo and which ones don't).

To me the strongest argument in favor of having 2 separate functions is
that the semantics is sufficiently different that callers need to know
which is which (mostly because of the difference in algorithmic
complexity but also because of where point is (guaranteed or not) after
the call).

[ I noticed also that some of them could benefit from an `and-extract`
  option (i.e. return the old text), e.g. in `transpose-subr-1`.
  It would be easy/cheap to add such an option to `replace-region`, but not
  to `replace-region-contents`.  ]

>> Personally, I would feel reluctant to use this function.  For example, I
>> don't know how to specify a meaningful value for MAX-COSTS.  It is
>> documented as specifying "the quality of the difference computation",
>> which does not help much.  I think our users will feel this even more.
>> 
>> No other buffer editing primitives have anything resembling the MAX-SECS
>> and MAX-COSTS arguments.  I find the design choice to expose those
>> arguments a bit unusual in a specialized function, and inappropriate in
>> a function intended for general use.
>
> The original version of replace-buffer-contents didn't have any such
> arguments.  You called the function and had to wait for however long it
> took to do its job.  We added these arguments to give Lisp programs
> more control, depending on the context and the importance of keeping
> the text which doesn't need to be changed.  Now we are discussing how
> to give Lisp programs even more control.

[ Note: This sub-discussion is only very remotely related to
  `replace-region`.  ]
FWIW, I find the MAX-COSTS very problematic, because nobody knows what
it does.  The only thing we know about it is that it's an integer and it
defaults to 10k.  Based on the name we can assume that a larger value
pushes the limit further, so a smaller value may shorten the time to
complete (while reducing the quality of the output), but we have no idea
about the proportion.  To make it usable, the programmer would need to
know things like:

- How does it relate to the size of the replaced text?
  E.g. If the replacement is "zoomed" by a factor 4 (i.e. e.g. just
  replace each char in both the old and the new text with 4 repetitions
  of that char), does the same MAX-COSTS lead to finding the same diff
  in both cases (modulo the zoom factor)?
- If I reduce MAX-COSTS by half, is the max runtime reduced by half?
  Much less than half?  Much more than half?  Something else altogether?

>> That said, I understand that they are occasionally useful, or they
>> wouldn't exist.  I would make them internal to the function, with
>> sensible defaults that users could override with some variable when they
>> need to.  That change could be done in a backwards-compatible way with
>> advertised-calling-convention', and I would suggest we do that.
> I'm not sure I see how this would be better than what we have now.

I'd tend to agree: Dynbound implicit args aren't great, so I think the
current setup is not bad choice in this regard.  The MAX-SECS arg tends
to depend on the specific call, AFAICT, so I'm not sure there'd be much
benefit to a global, overridable, default.  As for MAX-COSTS, I don't
think we can guess what setup is best without being able to answer the
questions above.


        Stefan

[replace-region.patch (text/x-diff, inline)]

diff --git a/admin/admin.el b/admin/admin.el
index 542556a65e0..a34e4adb9e1 100644
--- a/admin/admin.el
+++ b/admin/admin.el
@@ -517,8 +517,7 @@ manual-html-fix-headers
     (setq opoint (point))
     (search-forward "</head>")
     (goto-char (match-beginning 0))
-    (delete-region opoint (point))
-    (insert manual-style-string)
+    (replace-region-contents opoint (point) manual-style-string 0)
     ;; Remove Texinfo 5 hard-coding bgcolor, text, link, vlink, alink.
     (when (re-search-forward "<body lang=\"[^\"]+\"" nil t)
       (setq opoint (point))
@@ -603,9 +602,10 @@ manual-html-fix-index-2
 		 (setq opoint (match-beginning 0))
 		 (while (and (looking-at " *&mdash;")
 			     (zerop (forward-line 1))))
-		 (delete-region opoint (point))
-		 (insert "</table>\n\n\
-<h2>Detailed Node Listing</h2>\n\n<p>")
+		 (replace-region-contents opoint (point)
+		                          "</table>\n\n\
+<h2>Detailed Node Listing</h2>\n\n<p>"
+		                          0)
 		 ;; FIXME Fragile!
 		 ;; The Emacs and Elisp manual have some text at the
 		 ;; start of the detailed menu that is not part of the menu.
@@ -651,14 +651,15 @@ manual-html-fix-index-2
 	      (save-excursion
 	        (forward-char -1)
 	        (skip-chars-backward " ")
-	        (delete-region (point) (line-end-position))
-	        (insert "</td>\n  </tr>")))
-	    (insert "  <tr>\n    ")
-	    (if table-workaround
-	        ;; This works around a Firefox bug in the mono file.
-	        (insert "<td bgcolor=\"white\">")
-	      (insert "<td>"))
-	    (insert tag "</td>\n    <td>" (or desc ""))
+	        (replace-region-contents (point) (progn (end-of-line) (point))
+	                                 "</td>\n  </tr>" 0)
+	        ))
+	    (insert "  <tr>\n    "
+	            (if table-workaround
+	                ;; This works around a Firefox bug in the mono file.
+	                "<td bgcolor=\"white\">"
+	              "<td>")
+	            tag "</td>\n    <td>" (or desc ""))
 	    (setq open-td t))
 	   ((eq (char-after) ?\n)
 	    (delete-char 1)
diff --git a/admin/gitmerge.el b/admin/gitmerge.el
index 5bfb23dc3a2..ec2e5dd267b 100644
--- a/admin/gitmerge.el
+++ b/admin/gitmerge.el
@@ -324,12 +324,8 @@ gitmerge-resolve
                   ;; match-3's first.
                   (let ((match3 (buffer-substring start3 end3))
                         (match1 (buffer-substring start1 end1)))
-                    (delete-region start3 end3)
-                    (goto-char start3)
-                    (insert match1)
-                    (delete-region start1 end1)
-                    (goto-char start1)
-                    (insert match3)))))
+                    (replace-region-contents start3 end3 match1 0)
+                    (replace-region-contents start1 end1 match3 0)))))
             ;; (pop-to-buffer (current-buffer)) (debug 'before-resolve)
             ))
           ;; Try to resolve the conflicts.
diff --git a/admin/tree-sitter/treesit-admin.el b/admin/tree-sitter/treesit-admin.el
index 86174ed2625..516fd7bb700 100644
--- a/admin/tree-sitter/treesit-admin.el
+++ b/admin/tree-sitter/treesit-admin.el
@@ -187,8 +187,7 @@ treesit-admin--verify-major-mode-queries
             (forward-line -1))
           (setq beg (point))
           (search-forward "\n\n")
-          (delete-region beg (point))
-          (insert ";;\n")
+          (replace-region-contents beg (point) ";;\n" 0)
           (dolist (mode modes)
             (insert (format ";; %s has been tested with the following grammars and version:\n" mode))
             (dolist (lang (alist-get mode mode-language-alist))
diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index 93eb086da7a..cf3af3ea4e5 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -1106,9 +1106,9 @@ unexpand-abbrev
           (unless (stringp val)
             (error "Value of abbrev-symbol must be a string"))
           ;; Don't inherit properties here; just copy from old contents.
-          (insert last-abbrev-text)
-          ;; Delete after inserting, to better preserve markers.
-          (delete-region (point) (+ (point) (length val)))
+          (forward-char (length val))
+          (replace-region-contents last-abbrev-location (point)
+                                   last-abbrev-text 0)
           (setq last-abbrev-text nil))))))
 
 (defun abbrev--write (sym)
diff --git a/lisp/mail/rmailedit.el b/lisp/mail/rmailedit.el
index 60916818caf..32b5e0d3da3 100644
--- a/lisp/mail/rmailedit.el
+++ b/lisp/mail/rmailedit.el
@@ -350,8 +350,7 @@ rmail-abort-edit
   "Abort edit of current message; restore original contents."
   (interactive)
   (widen)
-  (delete-region (point-min) (point-max))
-  (insert rmail-old-text)
+  (replace-region-contents (point-min) (point-max) rmail-old-text)
   (rmail-cease-edit t)
   (rmail-highlight-headers))
 
diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
index 7052ac75817..d2379ecac78 100644
--- a/lisp/progmodes/cperl-mode.el
+++ b/lisp/progmodes/cperl-mode.el
@@ -5829,9 +5829,10 @@ cperl-fix-line-spacing
 				  cperl-extra-newline-before-brace)))
 		      (setq pp (point))
 		      (skip-chars-forward " \t\n")
-		      (delete-region pp (point))
-		      (insert
-		       (make-string cperl-indent-region-fix-constructs ?\ )))
+		      (replace-region-contents
+		       pp (point)
+		       (make-string cperl-indent-region-fix-constructs ?\ )
+		       0))
 		     ((and (looking-at "[\t ]*{")
 			   (if ml cperl-extra-newline-before-brace-multiline
 			     cperl-extra-newline-before-brace))
diff --git a/lisp/simple.el b/lisp/simple.el
index 7037158df8d..26513d52430 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8834,18 +8834,19 @@ transpose-subr-1
   (atomic-change-group
     ;; This sequence of insertions attempts to preserve marker
     ;; positions at the start and end of the transposed objects.
-    (let* ((word (buffer-substring (car pos2) (cdr pos2)))
+    ;; FIXME: Why don't we use `transpose-regions', which has extra code
+    ;; to "transpose" the markers together with the text!?
+    (let* ((word2 (buffer-substring (car pos2) (cdr pos2)))
 	   (len1 (- (cdr pos1) (car pos1)))
-	   (len2 (length word))
-	   (boundary (make-marker)))
-      (set-marker boundary (car pos2))
+	   (len2 (length word2))
+	   (boundary (copy-marker (car pos2))))
       (goto-char (cdr pos1))
-      (insert-before-markers word)
-      (setq word (delete-and-extract-region (car pos1) (+ (car pos1) len1)))
-      (goto-char boundary)
-      (insert word)
-      (goto-char (+ boundary len1))
-      (delete-region (point) (+ (point) len2))
+      ;; FIXME: We could use `replace-region-contents' but at the cost of
+      ;; not benefiting from `delete-and-extract-region'.
+      (insert-before-markers word2)
+      (let ((word1 (delete-and-extract-region (car pos1) (+ (car pos1) len1))))
+        (goto-char (+ boundary len2))
+        (replace-region-contents boundary (+ boundary len2) word1 0))
       (set-marker boundary nil))))
 
 (defun backward-word (&optional arg)
diff --git a/lisp/tar-mode.el b/lisp/tar-mode.el
index 3b9898bd2f4..a235dd4ab83 100644
--- a/lisp/tar-mode.el
+++ b/lisp/tar-mode.el
@@ -1425,31 +1425,30 @@ tar-alter-one-field
   ;;
   ;; update the header-line.
   (let ((col (current-column)))
-    (delete-region (line-beginning-position)
-                   (prog2 (forward-line 1)
-                       (point)
-                     ;; Insert the new text after the old, before deleting,
-                     ;; to preserve markers such as the window start.
-                     (insert (tar-header-block-summarize descriptor) "\n")))
-    (forward-line -1) (move-to-column col))
+    (replace-region-contents
+     (line-beginning-position) (line-end-position)
+     (tar-header-block-summarize descriptor) 0)
+    (move-to-column col))
 
   (cl-assert (tar-data-swapped-p))
   (with-current-buffer tar-data-buffer
     (let* ((start (- (tar-header-data-start descriptor) 512)))
-        ;;
-        ;; delete the old field and insert a new one.
-        (goto-char (+ start data-position))
-        (delete-region (point) (+ (point) (length new-data-string))) ; <--
-        (cl-assert (not (or enable-multibyte-characters
-                            (multibyte-string-p new-data-string))))
-        (insert new-data-string)
-        ;;
-        ;; compute a new checksum and insert it.
-        (let ((chk (tar-header-block-checksum
+      ;;
+      (cl-assert (not (or enable-multibyte-characters
+                          (multibyte-string-p new-data-string))))
+      ;; delete the old field and insert a new one.
+      (replace-region-contents
+       (+ start data-position)
+       (+ start data-position (length new-data-string))
+       new-data-string 0)
+      ;;
+      ;; compute a new checksum and insert it.
+      (let ((chk (tar-header-block-checksum
 		  (buffer-substring start (+ start 512)))))
-	(goto-char (+ start tar-chk-offset))
-	(delete-region (point) (+ (point) 8))
-	(insert (format "%6o\0 " chk))
+	(replace-region-contents
+	 (+ start tar-chk-offset)
+	 (+ start tar-chk-offset 8)
+	 (format "%6o\0 " chk) 0)
 	(setf (tar-header-checksum descriptor) chk)
 	;;
 	;; ok, make sure we didn't botch it.
diff --git a/lisp/term.el b/lisp/term.el
index 862103d88e6..05001de098c 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -4054,7 +4054,6 @@ term-erase-in-line
       (unless (and (eq saved-point (1- (point)))
                    (eq (char-before) ?\n)
                    (not wrapped))
-        ;; Insert before deletion to preserve markers.
         ;; wrapped is true if we're at the beginning of screen line,
         ;; but not a buffer line.  If we delete the current screen line
         ;; that will make the previous line no longer wrap, and (because
@@ -4064,10 +4063,8 @@ term-erase-in-line
         ;; contain a space, to force the previous line to continue to wrap.
         ;; We could do this always, but it seems preferable to not add the
         ;; extra space when wrapped is false.
-        (when wrapped
-	  (insert-before-markers ? ))
-        (insert-before-markers ?\n)
-        (delete-region saved-point (point)))
+        (replace-region-contents saved-point (point)
+	                         (if wrapped " \n" "\n") 0))
       (put-text-property saved-point (point) 'font-lock-face 'default)
       (goto-char saved-point))))
 
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 459154f534b..84c735502be 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2139,9 +2139,8 @@ diff-apply-hunk
      (t
       ;; Apply the hunk
       (with-current-buffer buf
-	(goto-char (car pos))
-	(delete-region (car pos) (cdr pos))
-	(insert (car new)))
+	(goto-char (cdr pos))
+	(replace-region-contents (car pos) (cdr pos) (car new)))
       ;; Display BUF in a window
       (set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
       (diff-hunk-status-msg line-offset (xor switched reverse) nil)

Message #150 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76313 <at> debbugs.gnu.org, stefankangas <at> gmail.com, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sun, 30 Mar 2025 08:12:13 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: stefankangas <at> gmail.com,  phst <at> google.com,  76313 <at> debbugs.gnu.org,
>   tsdh <at> gnu.org
> Date: Sat, 29 Mar 2025 18:20:15 -0400
> 
> > I'd prefer a completely backward-compatible change of passing a
> > special value of MAX-SECS to indicate that the potentially costly
> > algorithm is to be bypassed.
> 
> We already have that: pass a 0 for MAX-SECS.

So we should probably mention this in the documentation?

> But I've changed my mind: I don't think we should unify
> `replace-region-contents` and my proposed `replace-region`.
> 
> When I sent the patches there were still some tests failing, and
> I assumed these were just the result of idiotic errors on my part which
> I'd fix the next day (it was late).
> 
> While that was partly true, some of the problems were a lot more
> delicate: while `replace-region` is "easy" to use to do what I would
> describe as "edits", `replace-region-contents` is a good bit more
> delicate because its effect on point is less predictable.
> 
> When point is "in the middle" of the modified text, all is fine callers
> usually don't expect any specific position for point after wards, in
> that case.  But if point is at the beginning (resp. at the end) of the text
> being replaced, most callers expect it to still be at the beginning
> (resp. the end) of the new text afterwards.
> 
> This does hold when point is at the beginning of the text but *not* when
> it's at the end.  So in a typical loop scanning a buffer with
> `re-search-forward` and then calling `replace-region-contents`, it's
> easy to get bitten by it when occasionally we'll end up finding a match
> in the text we just inserted because the "careful" version of
> `replace-region-contents` moved point back somewhat.
> 
> IOW, if you have code that uses
> 
>     (replace-region-contents BEG END SOURCE 0)
> 
> and you change it to:
> 
>     (replace-region-contents BEG END SOURCE 0.1)
> 
> thinking "oh, I can afford to spend a bit more time here, let's
> enable the fancy diff algorithm", you may end up breaking your code.

So maybe these subtleties should be described in the manual?  And if
the caller doesn't care, or has point in a place that won't bump into
these issues, then replace-region-contents can still be used as a Lisp
API to replace_region?  For example, a loop like you describe above
could carefully place point before each call to re-search-forward, to
avoid the problems, no?

Message #153 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76313 <at> debbugs.gnu.org, stefankangas <at> gmail.com, tsdh <at> gnu.org
Subject: Re: bug#76313: New function `replace-region`
Date: Sun, 30 Mar 2025 11:30:18 -0400

>> > I'd prefer a completely backward-compatible change of passing a
>> > special value of MAX-SECS to indicate that the potentially costly
>> > algorithm is to be bypassed.
>> We already have that: pass a 0 for MAX-SECS.
> So we should probably mention this in the documentation?

I think that's already implied by the doc of MAX-SECS:

    The MAX-SECS argument, if given, defines a hard limit on the time used
    for comparing the buffers.  If it takes longer than MAX-SECS, the
    function falls back to a plain ‘delete-region’ and
    ‘insert-buffer-substring’.  (Note that the checks are not performed
    too evenly over time, so in some cases it may run a bit longer than
    allowed).

>> IOW, if you have code that uses
>> 
>>     (replace-region-contents BEG END SOURCE 0)
>> 
>> and you change it to:
>> 
>>     (replace-region-contents BEG END SOURCE 0.1)
>> 
>> thinking "oh, I can afford to spend a bit more time here, let's
>> enable the fancy diff algorithm", you may end up breaking your code.
>
> So maybe these subtleties should be described in the manual?  And if
> the caller doesn't care, or has point in a place that won't bump into
> these issues, then replace-region-contents can still be used as a Lisp
> API to replace_region?

I feel like this is setting up a trap for the average coder.

> For example, a loop like you describe above could carefully place
> point before each call to re-search-forward, to avoid the
> problems, no?

Of course, the careful coder will find a solution.

I still can't understand why you're so opposed to having
2 different functions.

In comparison, we have:

On the insertion side:
  - insert
  - insert-and-inherit
  - insert-before-markers
  - insert-buffer-substring
  - insert-buffer
  - insert-into-buffer
On the deletion side:
  - delete-region
  - delete-char
  - delete-and-extract-region
  - erase-buffer

I mean, I understand that the above functions all cover somewhat
different calling cases [side note: `replace-region-contents` tries to
unify them], whereas `replace-region` and `replace-region-carefully`
would be called with the same arguments to "do the same".

But I don't think coders would have any difficulty understanding that
one of them is the low-level, deterministic, O(N) primitive while the
other is a "smart" O(almost N²) alternative, with a much larger constant
factor, which decomposes the replacement into smaller calls to
`replace-region`, using heuristics, with the uncertainty this entails.


        Stefan

Message #156 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 stefankangas <at> gmail.com
Subject: Re: bug#76313: New function `replace-region`
Date: Mon, 31 Mar 2025 08:21:56 +0200

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> But I've changed my mind: I don't think we should unify
> `replace-region-contents` and my proposed `replace-region`.
>
> [...]
>
> While that was partly true, some of the problems were a lot more
> delicate: while `replace-region` is "easy" to use to do what I would
> describe as "edits", `replace-region-contents` is a good bit more
> delicate because its effect on point is less predictable.

I think that's a very good point.  Stefan's `replace-region` can be use
to replace text with some arbitrary other text with predictable
positioning of point.  `replace-region-contents` has always had the
assumption that the replacing text is very similar to the text it
replaces, e.g., a differently formatted version it.  Only in this
context it's sensible and intended that point doesn't move "out of the
middle" because only here we can assume the thing point is on is also
there in the replacing text.  (And, yes, the name
`replace-region-contents` doesn't transport that intention well.)

Bye,
Tassilo

Message #159 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 stefankangas <at> gmail.com
Subject: Re: bug#76313: New function `replace-region`
Date: Mon, 31 Mar 2025 16:10:29 +0300

> From: Tassilo Horn <tsdh <at> gnu.org>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,   stefankangas <at> gmail.com,
>    phst <at> google.com,   76313 <at> debbugs.gnu.org
> Date: Mon, 31 Mar 2025 08:21:56 +0200
> 
> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
> 
> > But I've changed my mind: I don't think we should unify
> > `replace-region-contents` and my proposed `replace-region`.
> >
> > [...]
> >
> > While that was partly true, some of the problems were a lot more
> > delicate: while `replace-region` is "easy" to use to do what I would
> > describe as "edits", `replace-region-contents` is a good bit more
> > delicate because its effect on point is less predictable.
> 
> I think that's a very good point.  Stefan's `replace-region` can be use
> to replace text with some arbitrary other text with predictable
> positioning of point.

And the same cannot be done by calling replace-region-contents with
MAX-SECS of zero?  If not, why not?

Message #162 received at 76313 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 stefankangas <at> gmail.com
Subject: Re: bug#76313: New function `replace-region`
Date: Mon, 31 Mar 2025 21:48:54 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> I think that's a very good point.  Stefan's `replace-region` can be
>> use to replace text with some arbitrary other text with predictable
>> positioning of point.
>
> And the same cannot be done by calling replace-region-contents with
> MAX-SECS of zero?  If not, why not?

It certainly can.  I just think the difference in intended use-case and
behavior justify separate functions.

A replace-region function can be used in many places and is easy to
document.  The complexities of replace-region-contents which leak into
its signature and docstring are justified only in very few situations.

If replace-region-contents should do both jobs, quick and careful
replacements, it's hard to document.  Either you start with the quick
variant and have to say that MAX-SECS should be 0 (why? and why that
strange name?), or you start with the careful variant and append that a
MAX-SECS of 0 makes everything you've read until now meaningless.  And
that the frequently useful quick variant requires one explicit argument
more than the seldomly needed careful variant is a bit odd, too.

Bye,
Tassilo

Previous Next

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