Subject: bug#23484: 25.1.50; undo doesn't work properly in xref-query-replace-in-results

From: Dmitry Gutov <[email-masked]>
Date: Sun, 08 May 2016 22:06:37 +0300

1. Do a search that has several matches in one buffer, e.g. using
dired-do-find-regexp.
2. Press `r' in the *xref* buffer, to initiate replacement.  Input `.*'
and `abcd', for instance.  This value of FROM is important.
3. Replace a couple then undo that (press y, y, u, u).
4. Try agreeing with the subsequent prompts.  The replacements performed
then will be wrong.

I've tried to come up with a patch but stopped short of really delving
into the code of perform-replace. Which seems really necessary at this
point. Attaching what I already have.

The big problem is that perform-replace does not consistently use
replace-re-search-function. With the new undo code, it's became worse.
After you press `u', it seems to switch to moving around the saved match
data stack and using a plain looking-at, without checking with
isearch-filter-predicate.

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index f651dc9..5686cc9 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -552,6 +552,7 @@ xref--buf-pairs-iterator
                       (end (move-marker (make-marker)
                                         (+ beg (xref-match-length item))
                                         (marker-buffer beg))))
+                 (set-marker-insertion-type end t)
                  (let ((pair (cons beg end)))
                    (push pair all-pairs)
                    ;; Perform sanity check first.
@@ -593,7 +594,7 @@ xref--outdated-p
 (defun xref--query-replace-1 (from to iter)
   (let* ((query-replace-lazy-highlight nil)
          (continue t)
-         did-it-once buf-pairs pairs
+         did-it-once buf-pairs
          current-beg current-end
          ;; Counteract the "do the next match now" hack in
          ;; `perform-replace'.  And still, it'll report that those
@@ -605,23 +606,24 @@ xref--query-replace-1
                  (<= end current-end))))
          (replace-re-search-function
           (lambda (from &optional _bound noerror)
-            (let (found pair)
-              (while (and (not found) pairs)
-                (setq pair (pop pairs)
-                      current-beg (car pair)
-                      current-end (cdr pair))
-                (goto-char current-beg)
-                (when (re-search-forward from current-end noerror)
-                  (setq found t)))
-              found))))
+            (let ((tmp-pairs (cdr buf-pairs)) pair)
+              (while (and (not pair) tmp-pairs)
+                (setq current-beg (caar tmp-pairs)
+                      current-end (cdar tmp-pairs))
+                (unless (< current-beg (point))
+                  (goto-char current-beg)
+                  (when (re-search-forward from current-end noerror)
+                    (setq pair (car tmp-pairs))))
+                (setq tmp-pairs (cdr tmp-pairs)))
+              pair))))
     (while (and continue (setq buf-pairs (funcall iter :next)))
       (if did-it-once
           ;; Reuse the same window for subsequent buffers.
           (switch-to-buffer (car buf-pairs))
         (xref--with-dedicated-window
-         (pop-to-buffer (car buf-pairs)))
+         (pop-to-buffer (car buf-pairs))
+         (goto-char (point-min)))
         (setq did-it-once t))
-      (setq pairs (cdr buf-pairs))
       (setq continue
             (perform-replace from to t t nil nil multi-query-replace-map)))
     (unless did-it-once (user-error "No suitable matches here"))))

In GNU Emacs 25.1.50.5 (x86_64-unknown-linux-gnu, GTK+ Version 3.18.9)
 of 2016-05-08 built on axl
Repository revision: 2eb6817ba971184cc109f8530f4b3b38f65650ea
Windowing system distributor 'The X.Org Foundation', version 11.0.11803000
System Description:	Ubuntu 16.04 LTS