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

From: Stefan Monnier <[email-masked]>
Date: Sat, 15 Feb 2025 17:18:15 -0500

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
diff --git a/etc/NEWS b/etc/NEWS
index 6d934b2029c..189ee92dac8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1265,6 +1265,9 @@ restore the old behavior, you can set 'eshell-pwd-convert-function' to
 
 * Lisp Changes in Emacs 31.1
 
+** New function 'replace-region'.
+Acts like delete-region+insert, but preserves markers more carefully.
+
 ** Time & Date
 
 +++
diff --git a/lisp/language/japan-util.el b/lisp/language/japan-util.el
index c61e98c8fca..2be202ce5fe 100644
--- a/lisp/language/japan-util.el
+++ b/lisp/language/japan-util.el
@@ -215,11 +215,7 @@ japanese-zenkaku
     (or (get-char-code-property obj 'jisx0208)
 	obj)))
 
-(defun japanese-replace-region (from to string)
-  "Replace the region specified by FROM and TO to STRING."
-  (goto-char from)
-  (insert string)
-  (delete-char (- to from)))
+(define-obsolete-function-alias 'japanese-replace-region #'replace-region "31")
 
 ;;;###autoload
 (defun japanese-katakana-region (from to &optional hankaku)
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 19b78173792..dc436bf65fa 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1421,6 +1421,7 @@ completion--replace
       (setq end (- end suffix-len))
       (setq newtext (substring newtext 0 (- suffix-len))))
     (goto-char beg)
+    ;; FIXME: Use `replace-region'?
     (let ((length (- end beg)))         ;Read `end' before we insert the text.
       (insert-and-inherit newtext)
       (delete-region (point) (+ (point) length)))
diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index df6571311e4..0da85e1c053 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -331,7 +331,7 @@ flymake-proc--find-possible-master-files
       (setq dirs (cdr dirs)))
     (when files
       (let ((flymake-proc--included-file-name (file-name-nondirectory file-name)))
-	(setq files (sort files 'flymake-proc--master-file-compare))))
+	(setq files (sort files #'flymake-proc--master-file-compare))))
     (flymake-log 3 "found %d possible master file(s)" (length files))
     files))
 
@@ -407,9 +407,10 @@ flymake-proc--check-patch-master-file-buffer
                   ;;  replace-match is not used here as it fails in
                   ;; XEmacs with 'last match not a buffer' error as
                   ;; check-includes calls replace-in-string
-                  (flymake-proc--replace-region
-                   match-beg match-end
-                   (file-name-nondirectory patched-source-file-name))))
+                  (save-excursion
+                    (replace-region
+                     match-beg match-end
+                     (file-name-nondirectory patched-source-file-name)))))
               (forward-line 1)))
           (when found
             (flymake-proc--save-buffer-in-file patched-master-file-name)))
@@ -424,11 +425,9 @@ flymake-proc--check-patch-master-file-buffer
 ;;; XXX: remove
 (defun flymake-proc--replace-region (beg end rep)
   "Replace text in BUFFER in region (BEG END) with REP."
+  (declare (obsolete replace-region "31"))
   (save-excursion
-    (goto-char end)
-    ;; Insert before deleting, so as to better preserve markers's positions.
-    (insert rep)
-    (delete-region beg end)))
+    (replace-region beg end rep)))
 
 (defun flymake-proc--read-file-to-temp-buffer (file-name)
   "Insert contents of FILE-NAME into newly created temp buffer."
diff --git a/lisp/subr.el b/lisp/subr.el
index 77e909d1bf6..267ae741052 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4680,6 +4680,24 @@ insert-into-buffer
     (with-current-buffer buffer
       (insert-buffer-substring current start end))))
 
+(defun replace-region (from to string)
+  "Replace text in buffer between FROM and TO with STRING.
+Moves point to the end of STRING."
+  (let ((to (+ to 0)))             ;Convert to integer if it's a marker.
+    (goto-char to)
+    (if (= from to)
+        (insert string)
+      ;; FIXME: Arguably, move-after-insertion markers found inside the
+      ;; FROM..TO region should be moved to the new end of the region
+      ;; rather than to the beginning.
+      ;; FIXME: We should call the before/after-change-functions only once
+      ;; (the best way to do that is probably to move it to `insdel.c').
+      ;; FIXME: Should we be careful to notice when a prefix/suffix of STRING
+      ;; is already a prefix/suffix of the FROM..TO region and refrain
+      ;; from touching it?
+      (insert-before-markers string)
+      (delete-region from to))))
+
 (defun replace-string-in-region (string replacement &optional start end)
   "Replace STRING with REPLACEMENT in the region from START to END.
 The number of replaced occurrences are returned, or nil if STRING
@@ -4703,8 +4721,7 @@ replace-string-in-region
       (let ((matches 0)
             (case-fold-search nil))
         (while (search-forward string nil t)
-          (delete-region (match-beginning 0) (match-end 0))
-          (insert replacement)
+          (replace-region (match-beginning 0) (match-end 0) replacement)
           (setq matches (1+ matches)))
         (and (not (zerop matches))
              matches)))))
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index 5b17c598efa..642071f825b 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -996,6 +996,29 @@ test-buffer-local-boundp
     (should-not (buffer-local-boundp 'test-not-boundp buf))
     (should (buffer-local-boundp 'test-global-boundp buf))))
 
+(ert-deftest subr-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-min) 8)))
+      ;; 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 test-replace-string-in-region ()
   (with-temp-buffer
     (insert "foo bar zot foobar")