GNU bug report logs - #31073
27.0.50; query-replace undo might change the word case

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 31073 in the body.
You can then email your comments to 31073 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: 27.0.50; query-replace undo might change the word case
Date: Fri, 06 Apr 2018 14:02:31 +0900

X-Debbugs-CC: Juri Linkov <juri <at> jurta.org>

The undo feature shouldn't change the case; it must return
to the original words.

emacs -Q
<
M-% t RET FOO RET SPC SPC SPC SPC U
;; The first line shows upcase 'T' at several positions, for instance
;; it shows 'TexT', originally shown as 'text'.


--8<-----------------------------cut here---------------start------------->8---
commit 6fcb746c4efcbc2afce6a520f4a8b67a8d40cdd1
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Fri Apr 6 13:40:06 2018 +0900

    Preserve case in query-replace undo
    
    If the user query and replaces 'foo' with 'BAR', then
    undo must comeback to 'foo', not to 'FOO'.
    * lisp/replace.el (perform-replace): Bind nocasify to non-nil
    value during undo/undo-all actions.
    * test/lisp/replace-tests.el (query-replace-undo-bug31073): Add test.

diff --git a/lisp/replace.el b/lisp/replace.el
index c28c9b36f0..a147c8dd86 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2619,6 +2619,7 @@ perform-replace
 			   (let ((stack-idx         0)
                                  (stack-len         (length stack))
                                  (num-replacements  0)
+                                 (nocasify t) ; Bug#31073.
                                  search-string
                                  next-replacement)
                              (while (and (< stack-idx stack-len)
diff --git a/test/lisp/replace-tests.el b/test/lisp/replace-tests.el
index 66c6842660..d6043e297c 100644
--- a/test/lisp/replace-tests.el
+++ b/test/lisp/replace-tests.el
@@ -380,4 +380,24 @@ replace-tests--query-replace-undo
   (should (string= "211" (replace-tests--query-replace-undo)))
   (should (string= "211" (replace-tests--query-replace-undo 'comma))))
 
+(ert-deftest query-replace-undo-bug31073 ()
+  "Test for https://debbugs.gnu.org/31073 ."
+  (let ((text "The teeth must be cleaned after every meal.")
+        (count 0))
+    (with-temp-buffer
+      (insert text)
+      (goto-char 1)
+      (cl-letf (((symbol-function 'read-event)
+                 (lambda (&rest args)
+                   (cl-incf count)
+                   (let ((val (pcase count
+                                ('4 ?U) ; undo-all
+                                ('5 ?q) ; exit
+                                (_ ?\s)))) ; replace current and go next
+                     val))))
+        (perform-replace "t" "FOO" t nil nil))
+      ;; After undo text must be the same.
+      (should (string= text (buffer-string))))))
+
+
 ;;; replace-tests.el ends here
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 25, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2018-04-03 built on calancha-pc
Repository revision: 10ef466a9eb621a3752de69930fcb14bf1af4887

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

From: Juri Linkov <juri <at> jurta.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 31073 <at> debbugs.gnu.org
Subject: Re: bug#31073: 27.0.50; query-replace undo might change the word case
Date: Sat, 07 Apr 2018 23:42:48 +0300

> The undo feature shouldn't change the case; it must return
> to the original words.
>
> emacs -Q
> <
> M-% t RET FOO RET SPC SPC SPC SPC U
> ;; The first line shows upcase 'T' at several positions, for instance
> ;; it shows 'TexT', originally shown as 'text'.

Good catch!  This means we should have more test coverage.  Thanks.

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Juri Linkov <juri <at> jurta.org>, 31073 <at> debbugs.gnu.org
Subject: Re: bug#31073: 27.0.50; query-replace undo might change the word case
Date: Sun, 08 Apr 2018 10:14:01 +0900

Juri Linkov <juri <at> jurta.org> writes:

>> The undo feature shouldn't change the case; it must return
>> to the original words.
>>
>> emacs -Q
>> <
>> M-% t RET FOO RET SPC SPC SPC SPC U
>> ;; The first line shows upcase 'T' at several positions, for instance
>> ;; it shows 'TexT', originally shown as 'text'.
>
> Good catch!  This means we should have more test coverage.  Thanks.
Eli,

this feature was introduced for Emacs-26 release.  Should the fix go
into that branch (Emacs-26)?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: juri <at> jurta.org, 31073 <at> debbugs.gnu.org
Subject: Re: bug#31073: 27.0.50; query-replace undo might change the word case
Date: Sun, 08 Apr 2018 05:33:34 +0300

> From: Tino Calancha <tino.calancha <at> gmail.com>
> Cc: 31073 <at> debbugs.gnu.org, Juri Linkov <juri <at> jurta.org>
> Date: Sun, 08 Apr 2018 10:14:01 +0900
> 
> Juri Linkov <juri <at> jurta.org> writes:
> 
> >> The undo feature shouldn't change the case; it must return
> >> to the original words.
> >>
> >> emacs -Q
> >> <
> >> M-% t RET FOO RET SPC SPC SPC SPC U
> >> ;; The first line shows upcase 'T' at several positions, for instance
> >> ;; it shows 'TexT', originally shown as 'text'.
> >
> > Good catch!  This means we should have more test coverage.  Thanks.
> Eli,
> 
> this feature was introduced for Emacs-26 release.  Should the fix go
> into that branch (Emacs-26)?

It's too late for Emacs 26.1.  For the other 26.x, I'd need to see the
patch first.

Thanks.

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> jurta.org, 31073 <at> debbugs.gnu.org
Subject: Re: bug#31073: 27.0.50; query-replace undo might change the word case
Date: Sun, 08 Apr 2018 11:46:15 +0900

Eli Zaretskii <eliz <at> gnu.org> writes:

>> this feature was introduced for Emacs-26 release.  Should the fix go
>> into that branch (Emacs-26)?
>
> It's too late for Emacs 26.1.  For the other 26.x, I'd need to see the
> patch first.
I see.
The patch is pretty obvious: just 1 line:

--8<-----------------------------cut here---------------start------------->8---
commit a70414e82048f425cdb00e3cd30c15fcb9fbab86
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Fri Apr 6 14:03:55 2018 +0900

    Preserve case in query-replace undo
    
    If the user query and replaces 'foo' with 'BAR', then
    undo must comeback to 'foo', not to 'FOO' (Bug#31073).
    * lisp/replace.el (perform-replace): Bind nocasify to non-nil
    value during undo/undo-all actions.

diff --git a/lisp/replace.el b/lisp/replace.el
index c28c9b36f0..2b0555d580 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2619,6 +2619,7 @@ perform-replace
 			   (let ((stack-idx         0)
                                  (stack-len         (length stack))
                                  (num-replacements  0)
+                                 (nocasify t) ; Bug#31073.
                                  search-string
                                  next-replacement)
                              (while (and (< stack-idx stack-len)

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 25, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
Repository revision: 8df23a82042fa7dbaaa4377bc376d705595b073f

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: juri <at> jurta.org, 31073 <at> debbugs.gnu.org
Subject: Re: bug#31073: 27.0.50; query-replace undo might change the word case
Date: Sun, 08 Apr 2018 15:53:44 +0300

> From: Tino Calancha <tino.calancha <at> gmail.com>
> Cc: juri <at> jurta.org,  31073 <at> debbugs.gnu.org
> Date: Sun, 08 Apr 2018 11:46:15 +0900
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> this feature was introduced for Emacs-26 release.  Should the fix go
> >> into that branch (Emacs-26)?
> >
> > It's too late for Emacs 26.1.  For the other 26.x, I'd need to see the
> > patch first.
> I see.
> The patch is pretty obvious: just 1 line:

Thanks.  This is indeed OK for the emacs-26 branch, but please wait
with pushing this until Emacs 26.1 is released, hopefully soon enough.

Alternatively, feel free to push to master and cherry-pick to emacs-26
later, after the 26.1 release.

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: 31073-done <at> debbugs.gnu.org
Subject: Re: bug#31073: 27.0.50; query-replace undo might change the word case
Date: Mon, 09 Apr 2018 11:55:00 +0900

Eli Zaretskii <eliz <at> gnu.org> writes:

>> The patch is pretty obvious: just 1 line:
>
> Thanks.  This is indeed OK for the emacs-26 branch, but please wait
> with pushing this until Emacs 26.1 is released, hopefully soon enough.
>
> Alternatively, feel free to push to master and cherry-pick to emacs-26
> later, after the 26.1 release.
Pushed fix into master branch as commit
"Preserve case in query-replace undo"
(32dc0cb1b5ae895d237c7118ccaeb084715934fd).

I will backport the fix to emacs-26 after 26.1 release.

Previous Next

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