From: "Kévin Le Gouguec" <kevin.legouguec@gmail.com>
To: Lars Ingebrigtsen <larsi@gnus.org>
Cc: 57733@debbugs.gnu.org
Subject: bug#57733: 29.0.50; Fix "Invalid search bound (wrong side of point)" error in replace-*-in-region
Date: Sun, 11 Sep 2022 19:09:04 +0200 [thread overview]
Message-ID: <87illtbzyn.fsf@gmail.com> (raw)
In-Reply-To: <87illtdgxe.fsf@gnus.org> (Lars Ingebrigtsen's message of "Sun, 11 Sep 2022 18:17:17 +0200")
[-- Attachment #1: Type: text/plain, Size: 243 bytes --]
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Perhaps a simpler solution would to just be to use
> `save-restriction'+`narrow-to-region' first and then don't use an END in
> the search?
That makes a lot of sense, thanks; new patch attached.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Restrict-replace-in-region-to-the-bounds-defined-by-.patch --]
[-- Type: text/x-patch, Size: 5015 bytes --]
From b5cc63cfd89f695067e9bb3a0135aded615557f0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Sun, 11 Sep 2022 16:15:38 +0200
Subject: [PATCH] Restrict replace-*-in-region to the bounds defined by caller
And call the corresponding *search-forward functions without giving
them a bound. The previous code can lead to two surprising outcomes:
(1) when REPLACEMENT is much larger than the original text, point
might move beyond END, which causes the search functions to error out;
(2) when REPLACEMENT is much shorter than the original text, the
search functions might find matches that the user did not intend, as
they were beyond END before the replacements happened.
Fixes bug#57733.
* lisp/subr.el (replace-string-in-region, replace-regexp-in-region):
Narrow to region before iterating over matches, instead of giving a
bound to the search functions.
* test/lisp/subr-tests.el (test-replace-string-in-region): Add
regression tests.
---
lisp/subr.el | 38 +++++++++++++++++++++-----------------
test/lisp/subr-tests.el | 32 ++++++++++++++++++++++++++++++--
2 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/lisp/subr.el b/lisp/subr.el
index 686189e69b..8769fec2b9 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4219,15 +4219,17 @@ replace-string-in-region
(error "End after end of buffer"))
(setq end (point-max)))
(save-excursion
- (let ((matches 0)
- (case-fold-search nil))
- (goto-char start)
- (while (search-forward string end t)
- (delete-region (match-beginning 0) (match-end 0))
- (insert replacement)
- (setq matches (1+ matches)))
- (and (not (zerop matches))
- matches))))
+ (goto-char start)
+ (save-restriction
+ (narrow-to-region start end)
+ (let ((matches 0)
+ (case-fold-search nil))
+ (while (search-forward string nil t)
+ (delete-region (match-beginning 0) (match-end 0))
+ (insert replacement)
+ (setq matches (1+ matches)))
+ (and (not (zerop matches))
+ matches)))))
(defun replace-regexp-in-region (regexp replacement &optional start end)
"Replace REGEXP with REPLACEMENT in the region from START to END.
@@ -4254,14 +4256,16 @@ replace-regexp-in-region
(error "End after end of buffer"))
(setq end (point-max)))
(save-excursion
- (let ((matches 0)
- (case-fold-search nil))
- (goto-char start)
- (while (re-search-forward regexp end t)
- (replace-match replacement t)
- (setq matches (1+ matches)))
- (and (not (zerop matches))
- matches))))
+ (goto-char start)
+ (save-restriction
+ (narrow-to-region start end)
+ (let ((matches 0)
+ (case-fold-search nil))
+ (while (re-search-forward regexp nil t)
+ (replace-match replacement t)
+ (setq matches (1+ matches)))
+ (and (not (zerop matches))
+ matches)))))
(defun yank-handle-font-lock-face-property (face start end)
"If `font-lock-defaults' is nil, apply FACE as a `face' property.
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index 4310b7291a..3011713210 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -968,7 +968,21 @@ test-replace-string-in-region
(insert "Foo bar zot foobar")
(should (= (replace-string-in-region "Foo" "new" (point-min))
1))
- (should (equal (buffer-string) "new bar zot foobar"))))
+ (should (equal (buffer-string) "new bar zot foobar")))
+
+ (with-temp-buffer
+ (insert "foo bar baz")
+ (should (= (replace-string-in-region "ba" "quux corge grault" (point-min))
+ 2))
+ (should (equal (buffer-string)
+ "foo quux corge graultr quux corge graultz")))
+
+ (with-temp-buffer
+ (insert "foo bar bar")
+ (should (= (replace-string-in-region " bar" "" (point-min) 8)
+ 1))
+ (should (equal (buffer-string)
+ "foo bar"))))
(ert-deftest test-replace-regexp-in-region ()
(with-temp-buffer
@@ -991,7 +1005,21 @@ test-replace-regexp-in-region
(insert "Foo bar zot foobar")
(should (= (replace-regexp-in-region "Fo+" "new" (point-min))
1))
- (should (equal (buffer-string) "new bar zot foobar"))))
+ (should (equal (buffer-string) "new bar zot foobar")))
+
+ (with-temp-buffer
+ (insert "foo bar baz")
+ (should (= (replace-regexp-in-region "ba." "quux corge grault" (point-min))
+ 2))
+ (should (equal (buffer-string)
+ "foo quux corge grault quux corge grault")))
+
+ (with-temp-buffer
+ (insert "foo bar bar")
+ (should (= (replace-regexp-in-region " bar" "" (point-min) 8)
+ 1))
+ (should (equal (buffer-string)
+ "foo bar"))))
(ert-deftest test-with-existing-directory ()
(let ((dir (make-temp-name "/tmp/not-exist-")))
--
2.37.3
next prev parent reply other threads:[~2022-09-11 17:09 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-11 15:59 bug#57733: 29.0.50; Fix "Invalid search bound (wrong side of point)" error in replace-*-in-region Kévin Le Gouguec
2022-09-11 16:17 ` Lars Ingebrigtsen
2022-09-11 17:09 ` Kévin Le Gouguec [this message]
2022-09-11 17:55 ` Lars Ingebrigtsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87illtbzyn.fsf@gmail.com \
--to=kevin.legouguec@gmail.com \
--cc=57733@debbugs.gnu.org \
--cc=larsi@gnus.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.