GNU bug report logs - #37073
26.2.90; query-replace-regexp undo feature missing quote the regexps

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 37073 in the body.
You can then email your comments to 37073 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.2.90; query-replace-regexp undo feature missing quote the regexps
Date: Sun, 18 Aug 2019 16:04:00 +0200

Tags: patch

That causes the undo feature to fail if the replacement contains special characters.

emacs -Q

C-x b *foo* RET
M-: (insert "theorem 1\ntheorem 2\ntheorem 3") RET
M-<
M-C-%
theorem \([0-9]+\) RET
\,(format "theorem \\ref{theo_%s}" \1) RET
SPC
SPC
u
q

;; Buffer *foo* now contains:
theorem 1ref{theo_1}
theorem 2ref{theo_2}
theorem 3

;; Expected:
theorem 1
theorem 2
theorem 3


--8<-----------------------------cut here---------------start------------->8---
commit 9f9c56cfa99b1e6fe296ec591ab335521e185e55
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Sun Aug 18 15:57:59 2019 +0200

    Fix query-replace-regexp undo feature
    
    * lisp/replace.el (perform-replace): Quote regexp (Bug#37073).
    * test/lisp/replace-tests.el (query-replace-undo-bug37073): Add test

diff --git a/lisp/replace.el b/lisp/replace.el
index 08feb8eae7..fb703abd92 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2614,7 +2614,7 @@ perform-replace
                                    (setq real-match-data
                                          (save-excursion
                                            (goto-char (match-beginning 0))
-                                           (looking-at search-string)
+                                           (looking-at (regexp-quote search-string)) ; Bug#37073
                                            (match-data t (nth 2 elt)))
                                          noedit
                                          (replace-match-maybe-edit
diff --git a/test/lisp/replace-tests.el b/test/lisp/replace-tests.el
index cd30633e37..f96bd9cf05 100644
--- a/test/lisp/replace-tests.el
+++ b/test/lisp/replace-tests.el
@@ -454,5 +454,15 @@ replace-tests--query-replace-undo
       input "a" "B" ((?\s . (1 2 3)) (?E . (4)) (?U . (5))) ?q
       (string= input (buffer-string))))))
 
+(ert-deftest query-replace-undo-bug37073 ()
+  "Test for https://debbugs.gnu.org/37073 ."
+  (let ((input "theorem 1\ntheorem 2\ntheorem 3"))
+    (should
+     (replace-tests-with-undo
+      input "theorem \\([0-9]+\\)"
+      "theorem \\\\ref{theo_\\1}"
+      ((?\s . (1 2)) (?U . (3)))
+      ?q
+      (string= input (buffer-string))))))
 
 ;;; replace-tests.el ends here

--8<-----------------------------cut here---------------end--------------->8---

In GNU Emacs 26.2.90 (build 19, x86_64-pc-linux-gnu, GTK+ Version 3.24.5)
 of 2019-08-18 built
Repository revision: 3f00db7ca6d40312651a302842561e7fb168ee99

Message #8 received at 37073 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 37073 <at> debbugs.gnu.org
Subject: Re: bug#37073: 26.2.90; query-replace-regexp undo feature missing
 quote the regexps
Date: Mon, 19 Aug 2019 00:06:20 +0300

> That causes the undo feature to fail if the replacement contains special characters.

Thanks.  Are there other looking-at/looking-back occurrences
in perform-replace that are looking at non-regexp strings?

> commit 9f9c56cfa99b1e6fe296ec591ab335521e185e55
> Author: Tino Calancha <tino.calancha <at> gmail.com>
> Date:   Sun Aug 18 15:57:59 2019 +0200
>
>     Fix query-replace-regexp undo feature
>
>     * lisp/replace.el (perform-replace): Quote regexp (Bug#37073).
>     * test/lisp/replace-tests.el (query-replace-undo-bug37073): Add test

This looks good, but please ask Eli about pushing it to emacs-26 branch.
Recently we fixed another problem exactly in the same place,
but pushed the fix to the master.

Message #11 received at 37073 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 37073 <at> debbugs.gnu.org
Subject: Re: bug#37073: 26.2.90; query-replace-regexp undo feature missing
 quote the regexps
Date: Mon, 19 Aug 2019 17:16:55 +0200

>Thanks.  Are there other looking-at/looking-back occurrences
>in perform-replace that are looking at non-regexp strings?
Yes, there is another one affecting the same feature
(see updated patch at the end).

>This looks good, but please ask Eli about pushing it to emacs-26 branch.
>Recently we fixed another problem exactly in the same place,
>but pushed the fix to the master.
I see.  Added Eli as CC.

--8<-----------------------------cut here---------------start------------->8---
commit 6e58c32caaf34f914b64a6dbc976bc1e62c12d90
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Mon Aug 19 17:06:02 2019 +0200

    Fix query-replace-regexp undo feature
    
    Ensure that non-regexp strings used with `looking-at' are quoted.
    * lisp/replace.el (perform-replace): Quote regexp (Bug#37073).
    * test/lisp/replace-tests.el (replace-tests-perform-replace-regexp-flag):
    New variable.
    (replace-tests-with-undo): Use it.
    (query-replace-undo-bug37073): Add tests.

diff --git a/lisp/replace.el b/lisp/replace.el
index 08feb8eae7..0ddebb1270 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2614,7 +2614,8 @@ perform-replace
                                    (setq real-match-data
                                          (save-excursion
                                            (goto-char (match-beginning 0))
-                                           (looking-at search-string)
+                                           ;; We must quote the string (Bug#37073)
+                                           (looking-at (regexp-quote search-string))
                                            (match-data t (nth 2 elt)))
                                          noedit
                                          (replace-match-maybe-edit
@@ -2624,7 +2625,9 @@ perform-replace
                                          real-match-data
                                          (save-excursion
                                            (goto-char (match-beginning 0))
-                                           (looking-at next-replacement)
+                                           (if regexp-flag
+                                               (looking-at next-replacement)
+                                             (looking-at (regexp-quote next-replacement)))
                                            (match-data t (nth 2 elt))))
                                    ;; Set replaced nil to keep in loop
                                    (when (eq def 'undo-all)
diff --git a/test/lisp/replace-tests.el b/test/lisp/replace-tests.el
index cd30633e37..cd08a522e3 100644
--- a/test/lisp/replace-tests.el
+++ b/test/lisp/replace-tests.el
@@ -365,6 +365,9 @@ replace-occur-test-create
 (defvar replace-tests-bind-read-string nil
   "A string to bind `read-string' and avoid the prompt.")
 
+(defvar replace-tests-perform-replace-regexp-flag t
+  "Value for regexp-flag argument passed to `perform-replace' in undo tests.")
+
 (defmacro replace-tests-with-undo (input from to char-nums def-chr &rest body)
   "Helper to test `query-replace' undo feature.
 INPUT is a string to insert in a temporary buffer.
@@ -412,7 +415,7 @@ replace-tests-with-undo
                     (if replace-tests-bind-read-string
                         (lambda (&rest args) replace-tests-bind-read-string)
                       (symbol-function 'read-string))))
-           (perform-replace ,from ,to t t nil))
+           (perform-replace ,from ,to t replace-tests-perform-replace-regexp-flag nil))
          ,@body))))
 
 (defun replace-tests--query-replace-undo (&optional comma)
@@ -454,5 +457,26 @@ replace-tests--query-replace-undo
       input "a" "B" ((?\s . (1 2 3)) (?E . (4)) (?U . (5))) ?q
       (string= input (buffer-string))))))
 
+(ert-deftest query-replace-undo-bug37073 ()
+  "Test for https://debbugs.gnu.org/37073 ."
+  (let ((input "theorem 1\ntheorem 2\ntheorem 3"))
+    (should
+     (replace-tests-with-undo
+         input "theorem \\([0-9]+\\)"
+         "theorem \\\\ref{theo_\\1}"
+         ((?\s . (1 2)) (?U . (3)))
+         ?q
+       (string= input (buffer-string)))))
+  ;; Now run a test with regexp-flag arg in `perform-replace' set to nil
+  (let ((input " ^theorem$ 1\n ^theorem$ 2\n ^theorem$ 3")
+        (replace-tests-perform-replace-regexp-flag nil)
+        (expected " theo 1\n ^theorem$ 2\n ^theorem$ 3"))
+    (should
+     (replace-tests-with-undo
+         input "^theorem$"
+         "theo"
+         ((?\s . (1 2 4)) (?U . (3)))
+         ?q
+       (string= expected (buffer-string))))))
 
 ;;; replace-tests.el ends here

--8<-----------------------------cut here---------------end--------------->8---


In GNU Emacs 26.2.90 (build 29, x86_64-pc-linux-gnu, GTK+ Version 3.24.5)
 of 2019-08-19 built
Repository revision: 876a0c65c4f8a28dc2fbd4d2044321d5a8c48b1a

Message #14 received at 37073 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 37073 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#37073: 26.2.90;
 query-replace-regexp undo feature missing quote the regexps
Date: Mon, 19 Aug 2019 18:29:42 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Date: Mon, 19 Aug 2019 00:06:20 +0300
> Cc: 37073 <at> debbugs.gnu.org
> 
> >     Fix query-replace-regexp undo feature
> >
> >     * lisp/replace.el (perform-replace): Quote regexp (Bug#37073).
> >     * test/lisp/replace-tests.el (query-replace-undo-bug37073): Add test
> 
> This looks good, but please ask Eli about pushing it to emacs-26 branch.

I'm okay with pushing this to emacs-26, but please do it soon, because
RC1 of 26.3 is in the works.

Message #19 received at 37073-done <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: 37073-done <at> debbugs.gnu.org
Subject: Re: bug#37073: 26.2.90; query-replace-regexp undo feature missing
 quote the regexps
Date: Mon, 19 Aug 2019 17:36:23 +0200

 
>> This looks good, but please ask Eli about pushing it to emacs-26 branch.

>I'm okay with pushing this to emacs-26, but please do it soon, because
>RC1 of 26.3 is in the works.
Pushed into emacs-26 as commit 'Fix query-replace-regexp undo feature'
(beb1d22260af2e03d80d34fcc1db212785a9d903)

[Hopefully soon enough :-|]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.