[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
master 47b497b4dac: (update_search_regs): Install better fix for bug#671
From: |
Stefan Monnier |
Subject: |
master 47b497b4dac: (update_search_regs): Install better fix for bug#67124 |
Date: |
Sat, 18 Nov 2023 16:34:49 -0500 (EST) |
branch: master
commit 47b497b4dac91e5ea56102018223bdeb5e21a93b
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>
(update_search_regs): Install better fix for bug#67124
The recent fix for the bug in `replace-match-maybe-edit`
was basically a refinement of a previously installed workaround,
whereas the bug was really in `update_search_regs`.
* src/search.c (update_search_regs): Improve handling of `start` positions.
* lisp/replace.el (replace-match-maybe-edit): Remove workaround.
* test/src/search-tests.el (search-test--replace-match-update-data): New
test.
---
lisp/replace.el | 7 -------
src/search.c | 18 ++++++++++++++++--
test/src/search-tests.el | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/lisp/replace.el b/lisp/replace.el
index 7fec54ecb27..ac677db2feb 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2642,13 +2642,6 @@ passed in. If LITERAL is set, no checking is done,
anyway."
noedit nil)))
(set-match-data match-data)
(replace-match newtext fixedcase literal)
- ;; `query-replace' undo feature needs the beginning of the match position,
- ;; but `replace-match' may change it, for instance, with a regexp like "^".
- ;; Ensure that this function preserves the beginning of the match position
- ;; (bug#31492). But we need to avoid clobbering the end of the match with
- ;; the original match-end position, since `replace-match' could have made
- ;; that incorrect or even invalid (bug#67124).
- (set-match-data (list (car match-data) (nth 1 (match-data))))
;; `replace-match' leaves point at the end of the replacement text,
;; so move point to the beginning when replacing backward.
(when backward (goto-char (nth 0 match-data)))
diff --git a/src/search.c b/src/search.c
index 692d8488049..2996d32fca1 100644
--- a/src/search.c
+++ b/src/search.c
@@ -3140,11 +3140,25 @@ update_search_regs (ptrdiff_t oldstart, ptrdiff_t
oldend, ptrdiff_t newend)
ptrdiff_t change = newend - oldend;
ptrdiff_t i;
+ /* When replacing subgroup 3 in a match for regexp '\(\)\(\(\)\)\(\)'
+ start[i] should ideally stay unchanged for all but i=4 and end[i]
+ should move for all but i=1.
+ We don't have enough info here to distinguish those different subgroups
+ (except for subgroup 0), so instead we lean towards leaving the start[i]s
+ unchanged and towards moving the end[i]s. */
+
for (i = 0; i < search_regs.num_regs; i++)
{
- if (search_regs.start[i] >= oldend)
+ if (search_regs.start[i] <= oldstart)
+ /* If the subgroup that 'replace-match' is modifying encloses the
+ subgroup 'i', then its 'start' position should stay unchanged.
+ That's always true for subgroup 0.
+ For other subgroups it depends on details we don't have, so
+ we optimistically assume that it also holds for them. */
+ ;
+ else if (search_regs.start[i] >= oldend)
search_regs.start[i] += change;
- else if (search_regs.start[i] > oldstart)
+ else
search_regs.start[i] = oldstart;
if (search_regs.end[i] >= oldend)
search_regs.end[i] += change;
diff --git a/test/src/search-tests.el b/test/src/search-tests.el
index 293a715f5dc..32dc8a72a86 100644
--- a/test/src/search-tests.el
+++ b/test/src/search-tests.el
@@ -39,4 +39,42 @@
(replace-match "bcd"))
(should (= (point) 10)))))
+(ert-deftest search-test--replace-match-update-data ()
+ (with-temp-buffer
+ (pcase-dolist (`(,pre ,post) '(("" "")
+ ("a" "")
+ ("" "b")
+ ("a" "b")))
+ (erase-buffer)
+ (insert "hello ")
+ (save-excursion (insert pre post " world"))
+ (should (looking-at
+ (concat "\\(\\)" pre "\\(\\)\\(\\(\\)\\)\\(\\)" post "\\(\\)")))
+ (let* ((beg0 (match-beginning 0))
+ (beg4 (+ beg0 (length pre)))
+ (end4 (+ beg4 (length "BOO")))
+ (end0 (+ end4 (length post))))
+ (replace-match "BOO" t t nil 4)
+ (should (equal (match-beginning 0) beg0))
+ (should (equal (match-beginning 1) beg0))
+ (should (equal (match-beginning 2) beg4))
+ (should (equal (match-beginning 3) beg4))
+ (should (equal (match-beginning 4) beg4))
+ (should (equal (match-end 6) end0))
+ (should (equal (match-end 5) end4))
+ (should (equal (match-end 4) end4))
+ (should (equal (match-end 3) end4))
+ (should (equal (match-end 0) end0))
+ ;; `update_search_regs' doesn't have enough information to get
+ ;; the ones below correctly in all cases.
+ (when (> (length post) 0)
+ (should (equal (match-beginning 6) end0)))
+ (when (> (length pre) 0)
+ (should (equal (match-end 1) beg0)))
+ ;; `update_search_regs' doesn't have enough information to get
+ ;; the ones below correctly at all.
+ ;;(should (equal (match-beginning 5) end4))
+ ;;(should (equal (match-end 2) beg4))
+ ))))
+
;;; search-tests.el ends here
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- master 47b497b4dac: (update_search_regs): Install better fix for bug#67124,
Stefan Monnier <=