From f69158a344e5c95b0dc788bcaafcf87eda21a421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= Date: Sun, 11 Sep 2022 16:15:38 +0200 Subject: [PATCH] Update search bound while replacing matches in replace-*-in-region Not doing so can lead to two surprising results: (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. * lisp/subr.el (replace-string-in-region, replace-regexp-in-region): Update END while replacing matches. * test/lisp/subr-tests.el (test-replace-string-in-region): Add regression tests. --- lisp/subr.el | 18 ++++++++++++------ test/lisp/subr-tests.el | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/lisp/subr.el b/lisp/subr.el index 686189e69b..10b3bdcd8d 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -4220,12 +4220,15 @@ replace-string-in-region (setq end (point-max))) (save-excursion (let ((matches 0) - (case-fold-search nil)) + (case-fold-search nil) + end-of-match) (goto-char start) - (while (search-forward string end t) + (while (setq end-of-match (search-forward string end t)) (delete-region (match-beginning 0) (match-end 0)) (insert replacement) - (setq matches (1+ matches))) + (setq matches (1+ matches) + end (+ end + (- (point) end-of-match)))) (and (not (zerop matches)) matches)))) @@ -4255,11 +4258,14 @@ replace-regexp-in-region (setq end (point-max))) (save-excursion (let ((matches 0) - (case-fold-search nil)) + (case-fold-search nil) + end-of-match) (goto-char start) - (while (re-search-forward regexp end t) + (while (setq end-of-match (re-search-forward regexp end t)) (replace-match replacement t) - (setq matches (1+ matches))) + (setq matches (1+ matches) + end (+ end + (- (point) end-of-match)))) (and (not (zerop matches)) matches)))) 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