Package: emacs;
Reported by: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Sat, 15 Feb 2025 22:19:02 UTC
Severity: normal
Tags: patch
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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.