bug#27268: 26.0.50; Undo comma replacements in query-replace

[Tino Calancha]

X-Debbugs-CC: Juri Linkov <juri@linkov.net>
Severity: minor
Tags: patch

During a `query-replace' we can input a ?, character.
>From `query-replace-help':
Comma to replace but not move point immediately.

Currently, those replacements are ignored.
Following patch undo the comma replacements as well.

--8<-----------------------------cut here---------------start------------->8---
>From 6ff176755eb3472d86c718ba67457ba88aa95fac Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 7 Jun 2017 10:59:49 +0900
Subject: [PATCH] query-replace: Undo replacements performed with 'comma

During a `query-replace', character ?, replaces the character at point
but not move point.  A character ?u must undo such replacement
(Bug#27268).
* lisp/replace.el (replace--push-stack):
New macro extracted from `perform-replace'.
(perform-replace): Use it.
---
 lisp/replace.el | 70 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 42 insertions(+), 28 deletions(-)

diff --git a/lisp/replace.el b/lisp/replace.el
index 64dfe7da22..ba8ea40162 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2216,6 +2216,26 @@ replace-dehighlight
   ;; Close overlays opened by `isearch-range-invisible' in `perform-replace'.
   (isearch-clean-overlays))
 
+;; A macro because we push STACK, i.e. a local var in `perform-replace'.
+(defmacro replace--push-stack (replaced search-str next-replace stack)
+  (declare (indent 0) (debug (form form form gv-place)))
+  `(push (list (point) ,replaced
+;;;  If the replacement has already happened, all we need is the
+;;;  current match start and end.  We could get this with a trivial
+;;;  match like
+;;;  (save-excursion (goto-char (match-beginning 0))
+;;;                 (search-forward (match-string 0))
+;;;                  (match-data t))
+;;;  if we really wanted to avoid manually constructing match data.
+;;;  Adding current-buffer is necessary so that match-data calls can
+;;;  return markers which are appropriate for editing.
+              (if ,replaced
+                  (list
+                   (match-beginning 0) (match-end 0) (current-buffer))
+                (match-data t))
+              ,search-str ,next-replace)
+         ,stack))
+
 (defun perform-replace (from-string replacements
                        query-flag regexp-flag delimited-flag
                        &optional repeat-count map start end backward 
region-noncontiguous-p)
@@ -2259,6 +2279,8 @@ perform-replace
          (next-replacement-replaced nil) ; replacement string
                                          ; (substituted regexp)
          (last-was-undo)
+         (last-was-act-and-show)
+         (update-stack t)
          (replace-count 0)
          (skip-read-only-count 0)
          (skip-filtered-count 0)
@@ -2542,7 +2564,7 @@ perform-replace
                                  next-replacement)
                              (while (and (< stack-idx stack-len)
                                          stack
-                                         (null replaced))
+                                         (or (null replaced) 
last-was-act-and-show))
                                (let* ((elt (nth stack-idx stack)))
                                  (setq
                                   stack-idx (1+ stack-idx)
@@ -2552,10 +2574,11 @@ perform-replace
                                   search-string (nth (if replaced 4 3) elt)
                                   next-replacement (nth (if replaced 3 4) elt)
                                   search-string-replaced search-string
-                                  next-replacement-replaced next-replacement)
+                                  next-replacement-replaced next-replacement
+                                  last-was-act-and-show nil)
 
                                  (when (and (= stack-idx stack-len)
-                                            (null replaced)
+                                            (and (null replaced) (not 
last-was-act-and-show))
                                             (zerop num-replacements))
                                           (message "Nothing to undo")
                                           (ding 'no-terminate)
@@ -2595,7 +2618,7 @@ perform-replace
                                           "replacements"))
                                (ding 'no-terminate)
                                (sit-for 1)))
-                          (setq replaced nil last-was-undo t)))
+                          (setq replaced nil last-was-undo t 
last-was-act-and-show nil)))
                        ((eq def 'act)
                         (or replaced
                             (setq noedit
@@ -2603,7 +2626,7 @@ perform-replace
                                    next-replacement nocasify literal
                                    noedit real-match-data backward)
                                   replace-count (1+ replace-count)))
-                        (setq done t replaced t))
+                        (setq done t replaced t update-stack (not 
last-was-act-and-show)))
                        ((eq def 'act-and-exit)
                         (or replaced
                             (setq noedit
@@ -2614,7 +2637,7 @@ perform-replace
                         (setq keep-going nil)
                         (setq done t replaced t))
                        ((eq def 'act-and-show)
-                        (if (not replaced)
+                        (unless replaced
                             (setq noedit
                                   (replace-match-maybe-edit
                                    next-replacement nocasify literal
@@ -2622,7 +2645,11 @@ perform-replace
                                   replace-count (1+ replace-count)
                                   real-match-data (replace-match-data
                                                    t real-match-data)
-                                  replaced t)))
+                                  replaced t last-was-act-and-show t)
+                             (replace--push-stack
+                              replaced
+                              search-string-replaced
+                              next-replacement-replaced stack)))
                        ((or (eq def 'automatic) (eq def 'automatic-all))
                         (or replaced
                             (setq noedit
@@ -2633,7 +2660,7 @@ perform-replace
                         (setq done t query-flag nil replaced t)
                         (if (eq def 'automatic-all) (setq multi-buffer t)))
                        ((eq def 'skip)
-                        (setq done t))
+                        (setq done t update-stack (not last-was-act-and-show)))
                        ((eq def 'recenter)
                         ;; `this-command' has the value `query-replace',
                         ;; so we need to bind it to `recenter-top-bottom'
@@ -2703,27 +2730,14 @@ perform-replace
                ;; Record previous position for ^ when we move on.
                ;; Change markers to numbers in the match data
                ;; since lots of markers slow down editing.
-               (push (list (point) replaced
-;;;  If the replacement has already happened, all we need is the
-;;;  current match start and end.  We could get this with a trivial
-;;;  match like
-;;;  (save-excursion (goto-char (match-beginning 0))
-;;;                 (search-forward (match-string 0))
-;;;                  (match-data t))
-;;;  if we really wanted to avoid manually constructing match data.
-;;;  Adding current-buffer is necessary so that match-data calls can
-;;;  return markers which are appropriate for editing.
-                           (if replaced
-                               (list
-                                (match-beginning 0)
-                                (match-end 0)
-                                (current-buffer))
-                             (match-data t))
-                               search-string-replaced
-                               next-replacement-replaced)
-                     stack)
+                (when update-stack
+                  (replace--push-stack
+                   replaced
+                   search-string-replaced
+                   next-replacement-replaced stack))
                 (setq next-replacement-replaced nil
-                      search-string-replaced    nil))))))
+                      search-string-replaced    nil
+                      last-was-act-and-show     nil))))))
       (replace-dehighlight))
     (or unread-command-events
        (message "Replaced %d occurrence%s%s"
-- 
2.11.0

>From 4a9297fde76b0a6bebba994f99b9e3bcd9af67dc Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 7 Jun 2017 11:00:02 +0900
Subject: [PATCH] * test/lisp/replace-tests.el (query-replace--undo): Add test.

---
 test/lisp/replace-tests.el | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/test/lisp/replace-tests.el b/test/lisp/replace-tests.el
index adef5a3f3d..6c4cbc45b8 100644
--- a/test/lisp/replace-tests.el
+++ b/test/lisp/replace-tests.el
@@ -358,4 +358,25 @@ replace-occur-test-create
 (dotimes (i (length replace-occur-tests))
   (replace-occur-test-create i))
 
+(defun replace-tests--query-replace-undo (&optional comma)
+  (with-temp-buffer
+    (insert "111")
+    (goto-char 1)
+    (let ((count 0))
+      (cl-letf (((symbol-function 'read-event)
+                 (lambda (&rest args)
+                   (cl-incf count)
+                   (let ((val (pcase count
+                                ('2 (if comma ?, ?\s))
+                                ('3 ?u)
+                                ('4 ?q)
+                                (_ ?\s))))
+                     val))))
+        (perform-replace "1" "2" t nil nil)))
+    (buffer-string)))
+
+(ert-deftest query-replace--undo ()
+  (should (string= "211" (replace-tests--query-replace-undo)))
+  (should (string= "211" (replace-tests--query-replace-undo 'comma))))
+
 ;;; replace-tests.el ends here
-- 
2.11.0

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-06-07
Repository revision: 43885eac09d7c69ecbac08c033d33381e21f48a2