From 919ac3ae1635bf2b99eb1f3efc7476826359e92a Mon Sep 17 00:00:00 2001 From: Tino Calancha Date: Tue, 8 Aug 2017 10:25:27 +0900 Subject: [PATCH] query-replace: Undo replacements performed with 'comma During a `query-replace', the char ',' replaces the character at point and doesn't move point; right after, the char 'u' must undo such replacement (Bug#27268). * lisp/replace.el (replace--push-stack): New macro extracted from `perform-replace'. (perform-replace): Use it. * test/lisp/replace-tests.el (query-replace--undo): Add test. --- lisp/replace.el | 70 +++++++++++++++++++++++--------------- test/lisp/replace-tests.el | 22 ++++++++++++ 2 files changed, 64 insertions(+), 28 deletions(-) diff --git a/lisp/replace.el b/lisp/replace.el index a5024943e64..09972b40db5 100644 --- a/lisp/replace.el +++ b/lisp/replace.el @@ -2221,6 +2221,26 @@ It is called with three arguments, as if it were ;; 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) @@ -2264,6 +2284,8 @@ It must return a string." (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) @@ -2547,7 +2569,7 @@ It must return a string." 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) @@ -2557,10 +2579,11 @@ It must return a string." 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) @@ -2600,7 +2623,7 @@ It must return a string." "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 @@ -2608,7 +2631,7 @@ It must return a string." 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 @@ -2619,7 +2642,7 @@ It must return a string." (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 @@ -2627,7 +2650,11 @@ It must return a string." 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 @@ -2638,7 +2665,7 @@ It must return a string." (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' @@ -2708,27 +2735,14 @@ It must return a string." ;; 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" diff --git a/test/lisp/replace-tests.el b/test/lisp/replace-tests.el index adef5a3f3dc..a8bc5407f42 100644 --- a/test/lisp/replace-tests.el +++ b/test/lisp/replace-tests.el @@ -358,4 +358,26 @@ Each element has the format: (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)) + ;; Don't wait for user input. + (cl-letf (((symbol-function 'read-event) + (lambda (&rest args) + (cl-incf count) + (let ((val (pcase count + ('2 (if comma ?, ?\s)) ; replace and: ',' no move; '\s' go next + ('3 ?u) ; undo + ('4 ?q) ; exit + (_ ?\s)))) ; replace current and go next + 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.39.5