bug#21684: 25.0.50; undo in query-replace w/o exit

bug#21684: 25.0.50; undo in query-replace w/o exit

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]



In GNU Emacs 25.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.23)
  of 2015-10-14
Repository revision: 38f99a02b83e9e408970cb7abdb685725b2004f9


Quite often, performing query-replace, i just hit the [space] quicky
with no much attention. If i replace one string by mistake, i can
comeback to it with "^", but if i edit the string with "E" to reset it 
to its original value, the interactive query-replace session ends.

I found useful if we can undo such replacements without exiting the 
query-replace.

The attached patch implement such behaviour for `query-replace'.
Having this also working for `query-replace-regexp' would be very nice,
but harder to implement.


[-- Attachment #2: Type: text/plain, Size: 2767 bytes --]

diff --git a/lisp/replace.el b/lisp/replace.el
index 3a908ac..e3c906b 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1802,6 +1802,7 @@ query-replace-help
 C-l to clear the screen, redisplay, and offer same replacement again,
 ! to replace all remaining matches in this buffer with no more questions,
 ^ to move point back to previous match,
+U to undo previous replacement (queries w/ regexp not supported),
 E to edit the replacement string.
 In multi-buffer replacements type `Y' to replace all remaining
 matches in all remaining buffers with no more questions,
@@ -1831,6 +1832,8 @@ query-replace-map
     (define-key map "\C-l" 'recenter)
     (define-key map "!" 'automatic)
     (define-key map "^" 'backup)
+    (define-key map "u" 'undo)
+    (define-key map "U" 'undo)
     (define-key map "\C-h" 'help)
     (define-key map [f1] 'help)
     (define-key map [help] 'help)
@@ -1856,7 +1859,7 @@ query-replace-map
 `act-and-exit', `exit', `exit-prefix', `recenter', `scroll-up',
 `scroll-down', `scroll-other-window', `scroll-other-window-down',
 `edit', `edit-replacement', `delete-and-edit', `automatic',
-`backup', `quit', and `help'.
+`backup', `undo', `quit', and `help'.
 
 This keymap is used by `y-or-n-p' as well as `query-replace'.")
 
@@ -2332,6 +2335,32 @@ perform-replace
 			   (message "No previous match")
 			   (ding 'no-terminate)
 			   (sit-for 1)))
+			((eq def 'undo)
+			 (cond (regexp-flag
+					(message "Undo not supported in queries with regexp")
+					(ding 'no-terminate)
+					(sit-for 1))
+				   (t
+					 ;; bind locally swapped values (search-string/replacement)
+					(let* ((search-string-1   next-replacement)
+					       (next-replacement  search-string)
+                                               (search-string     search-string-1)
+                                               (elt               (if stack (pop stack) (match-data))))
+					  (goto-char (nth 0 elt))
+					  (setq replaced (nth 1 elt))
+					  (and replaced (goto-char (- (point) (length next-replacement))))
+
+					  (replace-match-maybe-edit
+					   next-replacement nocasify literal
+					   noedit real-match-data backward)
+
+					  (setq noedit nil
+                                                       replace-count (1- replace-count)
+                                                       replaced nil
+                                                       real-match-data
+                                                       (replace-match-data
+                                                        t real-match-data
+                                                        (nth 2 elt)))))))
 			((eq def 'act)
 			 (or replaced
 			     (setq noedit

bug#21684: 25.0.50; undo in query-replace w/o exit

[Juri Linkov]

> Quite often, performing query-replace, i just hit the [space] quicky
> with no much attention. If i replace one string by mistake, i can
> comeback to it with "^", but if i edit the string with "E" to reset it to
> its original value, the interactive query-replace session ends.
>
> I found useful if we can undo such replacements without exiting the
> query-replace.
>
> The attached patch implement such behaviour for `query-replace'.
> Having this also working for `query-replace-regexp' would be very nice,
> but harder to implement.

This would be a useful feature.  But I wonder why it's harder to
implement for regexps?  If you need, you could add all previous
replacements strings to the same stack at the end of data elements in
the stack like we did when necessary by adding new data at the end of the
similar isearch stack elements in isearch-cmds.  This would simplify
restoring the previous replaced strings from the stack.

PS: an alternative solution would be to call ordinary (undo) since
perform-replace places undo boundaries with (undo-boundary), but
it seems this would be a less reliable approach?

bug#21684: 25.0.50; undo in query-replace w/o exit

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]



> I wonder why it's harder to implement for regexps?
Basically for my lack of expertise in this code: i was trying
to extend my patch to support regexp without success.

> you could add all previous replacements strings
> to the same stack at the end of data elements
> like we did when necessary by adding new data at
> the end of the similar isearch stack elements in isearch-cmds.
Thank you, it sounds a good idea!!

See new patch (replace-2.patch): AFAICS is working as i wanted.
Please try it and feel free to give me any comments if you
find some issues to fix/improve.

[-- Attachment #2: Type: text/plain, Size: 6384 bytes --]

diff --git a/lisp/replace.el b/lisp/replace.el
index 3a908ac..7ada519 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1802,6 +1802,7 @@ query-replace-help
 C-l to clear the screen, redisplay, and offer same replacement again,
 ! to replace all remaining matches in this buffer with no more questions,
 ^ to move point back to previous match,
+U to undo previous replacement,
 E to edit the replacement string.
 In multi-buffer replacements type `Y' to replace all remaining
 matches in all remaining buffers with no more questions,
@@ -1831,6 +1832,8 @@ query-replace-map
     (define-key map "\C-l" 'recenter)
     (define-key map "!" 'automatic)
     (define-key map "^" 'backup)
+    (define-key map "u" 'undo)
+    (define-key map "U" 'undo)
     (define-key map "\C-h" 'help)
     (define-key map [f1] 'help)
     (define-key map [help] 'help)
@@ -1856,7 +1859,7 @@ query-replace-map
 `act-and-exit', `exit', `exit-prefix', `recenter', `scroll-up',
 `scroll-down', `scroll-other-window', `scroll-other-window-down',
 `edit', `edit-replacement', `delete-and-edit', `automatic',
-`backup', `quit', and `help'.
+`backup', `undo', `quit', and `help'.
 
 This keymap is used by `y-or-n-p' as well as `query-replace'.")
 
@@ -2105,6 +2108,9 @@ perform-replace
          (noedit nil)
          (keep-going t)
          (stack nil)
+         (search-string-solved-regexp nil)    ; last string matching `from-string'
+         (next-replacement-solved-regexp nil) ; replacement string (substituted regexp)
+         (last-was-undo)
          (replace-count 0)
          (skip-read-only-count 0)
          (skip-filtered-count 0)
@@ -2279,6 +2285,22 @@ perform-replace
 		   (match-beginning 0) (match-end 0)
 		   start end search-string
 		   regexp-flag delimited-flag case-fold-search backward)
+                  ;; Obtain the matched groups: needed only when regexp-flag non nil
+                  (when (and last-was-undo regexp-flag)
+                    (setq last-was-undo nil
+                          real-match-data
+                          (save-excursion
+                            (goto-char (match-beginning 0))
+                            (looking-at search-string)
+                            (match-data t real-match-data))))
+                  ;; Matched string and next-replacement (subtituted matched groups) stored in stack.
+                  (setq search-string-solved-regexp (buffer-substring-no-properties (match-beginning 0)
+                                                                                    (match-end 0))
+                        next-replacement-solved-regexp
+                        (query-replace-descr
+                         (save-match-data
+                           (set-match-data real-match-data)
+                           (match-substitute-replacement next-replacement nocasify literal))))
 		  ;; Bind message-log-max so we don't fill up the message log
 		  ;; with a bunch of identical messages.
 		  (let ((message-log-max nil)
@@ -2332,6 +2354,45 @@ perform-replace
 			   (message "No previous match")
 			   (ding 'no-terminate)
 			   (sit-for 1)))
+                        ((eq def 'undo)
+                         (if (null stack)
+                             (progn
+                               (message "No previous match")
+                               (ding 'no-terminate)
+                               (sit-for 1))
+                           (let* (search-string next-replacement (elt (pop stack)))
+                             (goto-char (nth 0 elt))
+                             (setq replaced                       (nth 1 elt)
+                                   ;; Bind locally swapped values (search-string <---> replacement).
+                                   search-string                  (nth (if replaced 1 0) (nth 3 elt))
+                                   next-replacement               (nth (if replaced 0 1) (nth 3 elt))
+                                   search-string-solved-regexp    search-string
+                                   next-replacement-solved-regexp next-replacement)
+
+                             (cond ((null replaced) ; nothing to undo
+                                    (setq real-match-data
+                                          (replace-match-data
+                                           t real-match-data
+                                           (nth 2 elt))))
+                                   (t ; undo replacement
+                                    (set-match-data (nth 2 elt))
+                                    (setq real-match-data
+                                          (save-excursion
+                                            (goto-char (match-beginning 0))
+                                            (looking-at search-string)
+                                            (match-data t (nth 2 elt)))
+                                          noedit
+                                          (replace-match-maybe-edit
+                                           next-replacement nocasify literal
+                                           noedit real-match-data backward)
+                                          replace-count (1- replace-count)
+                                          real-match-data
+                                          (save-excursion
+                                            (goto-char (match-beginning 0))
+                                            (looking-at next-replacement)
+                                            (match-data t (nth 2 elt))))))
+                             (setq replaced nil
+                                   last-was-undo t))))
 			((eq def 'act)
 			 (or replaced
 			     (setq noedit
@@ -2454,8 +2515,12 @@ perform-replace
 				 (match-beginning 0)
 				 (match-end 0)
 				 (current-buffer))
-			      (match-data t)))
-		      stack))))))
+			      (match-data t))
+                            (list search-string-solved-regexp
+                                  next-replacement-solved-regexp))
+		      stack)
+                (setq next-replacement-solved-regexp nil
+                      search-string-solved-regexp    nil))))))
 
       (replace-dehighlight))
     (or unread-command-events

bug#21684: 25.0.50; undo in query-replace w/o exit

[Juri Linkov]

> See new patch (replace-2.patch): AFAICS is working as i wanted.
> Please try it and feel free to give me any comments if you
> find some issues to fix/improve.

Thank you, your patch is working well.  One issue to improve I noticed that
when a previous replacement is skipped with "n" then undo "U" acts exactly
as backup "^" by going to the previous place, and doing no undo actually.
Maybe better would be for undo "U" to only undo and skip non-replacement
entries in the stack?  (Implementation-wise this means looping until
finding a previous non-replacement entry in the stack).

A minor comment is that to save cons cells you could just put both new
elements shallow as elements 3 and 4 in the stack instead of adding
a list of them.

Another minor suggestions is for better names you could rename
the prefix -solved-regexp to -replaced, i.e. replace
search-string-solved-regexp, next-replacement-solved-regexp with
search-string-replaced, next-replacement-replaced.

Also we need this feature to be documented in etc/NEWS.

Have you already signed FSF copyright assignment papers?

Your patch for bug#21663 is short enough to be installed without the
copyright paperwork, but your changes for bug#21684 require it.

bug#21684: 25.0.50; undo in query-replace w/o exit

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]


>Maybe better would be for undo "U" to only undo and skip non-replacement
>entries in the stack?  (Implementation-wise this means looping until
>finding a previous non-replacement entry in the stack).
I agree, its better undo 'just' undo. Fixed in the new patch
(replace-3.patch).

>A minor comment is that to save cons cells you could just put both new
>elements shallow as elements 3 and 4 in the stack instead of adding
>a list of them.
Sure, thank you for remind me this. Applied.

>Another minor suggestions is for better names you could rename
>the prefix -solved-regexp to -replaced
Done. Thank you.

>Also we need this feature to be documented in etc/NEWS.
>Have you already signed FSF copyright assignment papers?
I didn't. I would like to sign. Maybe someone could guide me how to
complete such process.

[-- Attachment #2: Type: text/plain, Size: 6556 bytes --]

diff --git a/lisp/replace.el b/lisp/replace.el
index 3a908ac..43a5a78 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1802,6 +1802,7 @@ query-replace-help
 C-l to clear the screen, redisplay, and offer same replacement again,
 ! to replace all remaining matches in this buffer with no more questions,
 ^ to move point back to previous match,
+U to undo previous replacement,
 E to edit the replacement string.
 In multi-buffer replacements type `Y' to replace all remaining
 matches in all remaining buffers with no more questions,
@@ -1831,6 +1832,8 @@ query-replace-map
     (define-key map "\C-l" 'recenter)
     (define-key map "!" 'automatic)
     (define-key map "^" 'backup)
+    (define-key map "u" 'undo)
+    (define-key map "U" 'undo)
     (define-key map "\C-h" 'help)
     (define-key map [f1] 'help)
     (define-key map [help] 'help)
@@ -1856,7 +1859,7 @@ query-replace-map
 `act-and-exit', `exit', `exit-prefix', `recenter', `scroll-up',
 `scroll-down', `scroll-other-window', `scroll-other-window-down',
 `edit', `edit-replacement', `delete-and-edit', `automatic',
-`backup', `quit', and `help'.
+`backup', `undo', `quit', and `help'.
 
 This keymap is used by `y-or-n-p' as well as `query-replace'.")
 
@@ -2105,6 +2108,9 @@ perform-replace
          (noedit nil)
          (keep-going t)
          (stack nil)
+         (search-string-replaced nil)    ; last string matching `from-string'
+         (next-replacement-replaced nil) ; replacement string (substituted regexp)
+         (last-was-undo)
          (replace-count 0)
          (skip-read-only-count 0)
          (skip-filtered-count 0)
@@ -2279,6 +2285,22 @@ perform-replace
 		   (match-beginning 0) (match-end 0)
 		   start end search-string
 		   regexp-flag delimited-flag case-fold-search backward)
+                  ;; Obtain the matched groups: needed only when regexp-flag non nil
+                  (when (and last-was-undo regexp-flag)
+                    (setq last-was-undo nil
+                          real-match-data
+                          (save-excursion
+                            (goto-char (match-beginning 0))
+                            (looking-at search-string)
+                            (match-data t real-match-data))))
+                  ;; Matched string and next-replacement (subtituted matched groups) stored in stack.
+                  (setq search-string-replaced (buffer-substring-no-properties (match-beginning 0)
+                                                                                    (match-end 0))
+                        next-replacement-replaced
+                        (query-replace-descr
+                         (save-match-data
+                           (set-match-data real-match-data)
+                           (match-substitute-replacement next-replacement nocasify literal))))
 		  ;; Bind message-log-max so we don't fill up the message log
 		  ;; with a bunch of identical messages.
 		  (let ((message-log-max nil)
@@ -2332,6 +2354,50 @@ perform-replace
 			   (message "No previous match")
 			   (ding 'no-terminate)
 			   (sit-for 1)))
+			((eq def 'undo)
+			 (if (null stack)
+                             (progn
+                               (message "Nothing to undo")
+                               (ding 'no-terminate)
+                               (sit-for 1))
+			   (let ((stack-idx 0)
+                                 (stack-len (length stack)))
+                             (while (and (< stack-idx stack-len) stack (null replaced))
+                               (let* (search-string next-replacement (elt (nth stack-idx stack)))
+                                 (setq stack-idx                      (1+ stack-idx)
+                                       replaced                       (nth 1 elt)
+                                       ;; Bind locally swapped values (search-string <---> replacement).
+                                       search-string                  (if replaced (nth 4 elt) (nth 3 elt))
+                                       next-replacement               (if replaced (nth 3 elt) (nth 4 elt))
+                                       search-string-replaced    search-string
+                                       next-replacement-replaced next-replacement)
+
+                                 (when (and (null replaced) (= stack-idx stack-len))
+                                   (message "Nothing to undo")
+                                   (ding 'no-terminate)
+                                   (sit-for 1))
+
+                                 (when replaced
+                                   (setq stack (nthcdr stack-idx stack))
+                                   (goto-char (nth 0 elt))
+                                   (set-match-data (nth 2 elt))
+                                   (setq real-match-data
+                                         (save-excursion
+                                           (goto-char (match-beginning 0))
+                                           (looking-at search-string)
+                                           (match-data t (nth 2 elt)))
+                                         noedit
+                                         (replace-match-maybe-edit
+                                          next-replacement nocasify literal
+                                          noedit real-match-data backward)
+                                         replace-count (1- replace-count)
+                                         real-match-data
+                                         (save-excursion
+                                           (goto-char (match-beginning 0))
+                                           (looking-at next-replacement)
+                                           (match-data t (nth 2 elt))))))))
+			   (setq replaced nil
+                                 last-was-undo t)))
 			((eq def 'act)
 			 (or replaced
 			     (setq noedit
@@ -2454,9 +2520,12 @@ perform-replace
 				 (match-beginning 0)
 				 (match-end 0)
 				 (current-buffer))
-			      (match-data t)))
-		      stack))))))
-
+			      (match-data t))
+				search-string-replaced
+				next-replacement-replaced)
+		      stack)
+                (setq next-replacement-replaced nil
+                      search-string-replaced    nil))))))
       (replace-dehighlight))
     (or unread-command-events
 	(message "Replaced %d occurrence%s%s"

bug#21684: 25.0.50; undo in query-replace w/o exit

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >Maybe better would be for undo "U" to only undo and skip non-replacement
  > >entries in the stack?  (Implementation-wise this means looping until
  > >finding a previous non-replacement entry in the stack).
  > I agree, its better undo 'just' undo. Fixed in the new patch
  > (replace-3.patch).

What change in functionality is being considered here?

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

bug#21684: 25.0.50; undo in query-replace w/o exit

[Tino Calancha]

> What change in functionality is being considered here?

Hi Richard,

the purpose is provide 'undo' into `query-replace'/`query-replace-regexp'.
Original code do not support such functionality in a convenient way.

For instance, in the scratch buffer:
M-x query-replace RET f RET @ RET [space] RET [space] RET C-/
(C-/ undo last replacement but end session: [space] would insert one space)

Indeed, you may correct last replacement with existing code without exit current
search replace, as follows:

I)
M-x query-replace RET f RET @ RET [space] RET [space] "^" C-r
(now you can edit last replacement; comeback to query-replace session with C-M-c)
C-M-c [space] [space] ...

This seems too complicated/slow to just undo last 1-2 replacements.

In my experience as an user 99% of the times, the situation is that i hit [space]
many times and fast, and then i realize i just want to undo last 1-2 
replacements and keep going. It would be useful if that can be done just 
with one key.

The patch replace-3.patch in this thread implements such interactive undo and binds
the new funtionality to keys "u" and "U".

Every time you hit "u" the latest replacement is undo: now you can hit "n" to skip
it (tipically) or [space] to replace it again, or maybe "u" to undo 
another previous replacement (if any). The query-replace session is not 
exit.

Tino

bug#21684: 25.0.50; undo in query-replace w/o exit

[Juri Linkov]

> The patch replace-3.patch in this thread implements such interactive undo
> and binds the new funtionality to keys "u" and "U".

Please note that usually the upper-case "U" applies to all
elements in the set such as e.g. in Dired the lower-case "u"
is bound to a single-mark dired-unmark whereas the upper-case "U"
is bound to dired-unmark-all-marks, etc.

For query-replace this means that better would be to bind the
upper-case "U" to a new operation undo-all that could be easy to
implement in the same branch with (or (eq def 'undo) (eq def 'undo-all))
and looping until the top of the stack when (eq def 'undo-all).

bug#21684: 25.0.50; undo in query-replace w/o exit

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]



> Please note that usually the upper-case "U" applies to all
> elements in the set.
> For query-replace this means that better would be to bind the
> upper-case "U" to a new operation undo-all

Good point. Thank you very much.
Included in new patch (replace-4.patch).

[-- Attachment #2: Type: text/plain, Size: 7554 bytes --]

diff --git a/lisp/replace.el b/lisp/replace.el
index 3a908ac..ef8c5e4 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1802,6 +1802,8 @@ query-replace-help
 C-l to clear the screen, redisplay, and offer same replacement again,
 ! to replace all remaining matches in this buffer with no more questions,
 ^ to move point back to previous match,
+u to undo previous replacement,
+U to undo all replacements,
 E to edit the replacement string.
 In multi-buffer replacements type `Y' to replace all remaining
 matches in all remaining buffers with no more questions,
@@ -1831,6 +1833,8 @@ query-replace-map
     (define-key map "\C-l" 'recenter)
     (define-key map "!" 'automatic)
     (define-key map "^" 'backup)
+    (define-key map "u" 'undo)
+    (define-key map "U" 'undo-all)
     (define-key map "\C-h" 'help)
     (define-key map [f1] 'help)
     (define-key map [help] 'help)
@@ -1856,7 +1860,7 @@ query-replace-map
 `act-and-exit', `exit', `exit-prefix', `recenter', `scroll-up',
 `scroll-down', `scroll-other-window', `scroll-other-window-down',
 `edit', `edit-replacement', `delete-and-edit', `automatic',
-`backup', `quit', and `help'.
+`backup', `undo', `undo-all', `quit', and `help'.
 
 This keymap is used by `y-or-n-p' as well as `query-replace'.")
 
@@ -2105,6 +2109,9 @@ perform-replace
          (noedit nil)
          (keep-going t)
          (stack nil)
+         (search-string-replaced nil)    ; last string matching `from-string'
+         (next-replacement-replaced nil) ; replacement string (substituted regexp)
+         (last-was-undo)
          (replace-count 0)
          (skip-read-only-count 0)
          (skip-filtered-count 0)
@@ -2279,6 +2286,23 @@ perform-replace
 		   (match-beginning 0) (match-end 0)
 		   start end search-string
 		   regexp-flag delimited-flag case-fold-search backward)
+                  ;; Obtain the matched groups: needed only when regexp-flag non nil.
+                  (when (and last-was-undo regexp-flag)
+                    (setq last-was-undo nil
+                          real-match-data
+                          (save-excursion
+                            (goto-char (match-beginning 0))
+                            (looking-at search-string)
+                            (match-data t real-match-data))))
+                  ;; Matched string and next-replacement-replaced stored in stack.
+                  (setq search-string-replaced (buffer-substring-no-properties
+                                                (match-beginning 0)
+                                                (match-end 0))
+                        next-replacement-replaced
+                        (query-replace-descr
+                         (save-match-data
+                           (set-match-data real-match-data)
+                           (match-substitute-replacement next-replacement nocasify literal))))
 		  ;; Bind message-log-max so we don't fill up the message log
 		  ;; with a bunch of identical messages.
 		  (let ((message-log-max nil)
@@ -2332,6 +2356,64 @@ perform-replace
 			   (message "No previous match")
 			   (ding 'no-terminate)
 			   (sit-for 1)))
+			((or (eq def 'undo) (eq def 'undo-all))
+			 (if (null stack)
+                             (progn
+                               (message "Nothing to undo")
+                               (ding 'no-terminate)
+                               (sit-for 1))
+			   (let ((stack-idx         0)
+                                 (stack-len         (length stack))
+                                 (num-replacements  0)
+                                 search-string
+                                 next-replacement)
+                             (while (and (< stack-idx stack-len) stack (null replaced))
+                               (let* ((elt (nth stack-idx stack)))
+                                 (setq stack-idx                  (1+ stack-idx)
+                                       replaced                   (nth 1 elt)
+                                       ;; Bind swapped values (search-string <--> replacement)
+                                       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)
+
+                                 (when (and (null replaced) (= stack-idx stack-len))
+                                   (message "Nothing to undo")
+                                   (ding 'no-terminate)
+                                   (sit-for 1))
+
+                                 (when replaced
+                                   (setq stack (nthcdr stack-idx stack))
+                                   (goto-char (nth 0 elt))
+                                   (set-match-data (nth 2 elt))
+                                   (setq real-match-data
+                                         (save-excursion
+                                           (goto-char (match-beginning 0))
+                                           (looking-at search-string)
+                                           (match-data t (nth 2 elt)))
+                                         noedit
+                                         (replace-match-maybe-edit
+                                          next-replacement nocasify literal
+                                          noedit real-match-data backward)
+                                         replace-count (1- replace-count)
+                                         real-match-data
+                                         (save-excursion
+                                           (goto-char (match-beginning 0))
+                                           (looking-at next-replacement)
+                                           (match-data t (nth 2 elt))))
+                                   (when (eq def 'undo-all) ; Set replaced nil to keep in loop
+                                     (setq replaced          nil
+                                           stack-len         (- stack-len stack-idx)
+                                           stack-idx         0 
+                                           num-replacements  (1+ num-replacements))
+                                     (unless stack
+                                       (message "Undid %d %s" num-replacements
+                                                (if (= num-replacements 1)
+                                                    "replacement"
+                                                  "replacements"))
+                                       (ding 'no-terminate)
+                                       (sit-for 1)))))))
+			   (setq replaced nil last-was-undo t)))
 			((eq def 'act)
 			 (or replaced
 			     (setq noedit
@@ -2454,9 +2536,12 @@ perform-replace
 				 (match-beginning 0)
 				 (match-end 0)
 				 (current-buffer))
-			      (match-data t)))
-		      stack))))))
-
+			      (match-data t))
+				search-string-replaced
+				next-replacement-replaced)
+		      stack)
+                (setq next-replacement-replaced nil
+                      search-string-replaced    nil))))))
       (replace-dehighlight))
     (or unread-command-events
 	(message "Replaced %d occurrence%s%s"

bug#21684: Acknowledgement (25.0.50; undo in query-replace w/o exit)

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 218 bytes --]


In previous patch 'undo-all just show the total number of undid 
replacements if the first element was replaced.

New patch (replace-5.patch) fix that: undo-all always show the accumulated
number of undone performed.

[-- Attachment #2: Type: text/plain, Size: 7907 bytes --]

diff --git a/lisp/replace.el b/lisp/replace.el
index 3a908ac..16a9942 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1802,6 +1802,8 @@ query-replace-help
 C-l to clear the screen, redisplay, and offer same replacement again,
 ! to replace all remaining matches in this buffer with no more questions,
 ^ to move point back to previous match,
+u to undo previous replacement,
+U to undo all replacements,
 E to edit the replacement string.
 In multi-buffer replacements type `Y' to replace all remaining
 matches in all remaining buffers with no more questions,
@@ -1831,6 +1833,8 @@ query-replace-map
     (define-key map "\C-l" 'recenter)
     (define-key map "!" 'automatic)
     (define-key map "^" 'backup)
+    (define-key map "u" 'undo)
+    (define-key map "U" 'undo-all)
     (define-key map "\C-h" 'help)
     (define-key map [f1] 'help)
     (define-key map [help] 'help)
@@ -1856,7 +1860,7 @@ query-replace-map
 `act-and-exit', `exit', `exit-prefix', `recenter', `scroll-up',
 `scroll-down', `scroll-other-window', `scroll-other-window-down',
 `edit', `edit-replacement', `delete-and-edit', `automatic',
-`backup', `quit', and `help'.
+`backup', `undo', `undo-all', `quit', and `help'.
 
 This keymap is used by `y-or-n-p' as well as `query-replace'.")
 
@@ -2105,6 +2109,9 @@ perform-replace
          (noedit nil)
          (keep-going t)
          (stack nil)
+         (search-string-replaced nil)    ; last string matching `from-string'
+         (next-replacement-replaced nil) ; replacement string (substituted regexp)
+         (last-was-undo)
          (replace-count 0)
          (skip-read-only-count 0)
          (skip-filtered-count 0)
@@ -2279,6 +2286,23 @@ perform-replace
 		   (match-beginning 0) (match-end 0)
 		   start end search-string
 		   regexp-flag delimited-flag case-fold-search backward)
+                  ;; Obtain the matched groups: needed only when regexp-flag non nil.
+                  (when (and last-was-undo regexp-flag)
+                    (setq last-was-undo nil
+                          real-match-data
+                          (save-excursion
+                            (goto-char (match-beginning 0))
+                            (looking-at search-string)
+                            (match-data t real-match-data))))
+                  ;; Matched string and next-replacement-replaced stored in stack.
+                  (setq search-string-replaced (buffer-substring-no-properties
+                                                (match-beginning 0)
+                                                (match-end 0))
+                        next-replacement-replaced
+                        (query-replace-descr
+                         (save-match-data
+                           (set-match-data real-match-data)
+                           (match-substitute-replacement next-replacement nocasify literal))))
 		  ;; Bind message-log-max so we don't fill up the message log
 		  ;; with a bunch of identical messages.
 		  (let ((message-log-max nil)
@@ -2332,6 +2356,67 @@ perform-replace
 			   (message "No previous match")
 			   (ding 'no-terminate)
 			   (sit-for 1)))
+			((or (eq def 'undo) (eq def 'undo-all))
+			 (if (null stack)
+                             (progn
+                               (message "Nothing to undo")
+                               (ding 'no-terminate)
+                               (sit-for 1))
+			   (let ((stack-idx         0)
+                                 (stack-len         (length stack))
+                                 (num-replacements  0)
+                                 search-string
+                                 next-replacement)
+                             (while (and (< stack-idx stack-len) stack (null replaced))
+                               (let* ((elt (nth stack-idx stack)))
+                                 (setq stack-idx                  (1+ stack-idx)
+                                       replaced                   (nth 1 elt)
+                                       ;; Bind swapped values (search-string <--> replacement)
+                                       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)
+
+                                 (when (= stack-idx stack-len)
+                                   (cond ((and (null replaced) (zerop num-replacements))
+                                          (message "Nothing to undo")
+                                          (ding 'no-terminate)
+                                          (sit-for 1))
+                                         ((or replaced (null (zerop num-replacements)))
+                                          (let ((num-replacements (if replaced (1+ num-replacements)
+                                                                    num-replacements)))
+                                            (message "Undid %d %s" num-replacements
+                                                     (if (= num-replacements 1)
+                                                         "replacement"
+                                                       "replacements"))
+                                            (ding 'no-terminate)
+                                            (sit-for 1)))))
+
+                                 (when replaced
+                                   (setq stack (nthcdr stack-idx stack))
+                                   (goto-char (nth 0 elt))
+                                   (set-match-data (nth 2 elt))
+                                   (setq real-match-data
+                                         (save-excursion
+                                           (goto-char (match-beginning 0))
+                                           (looking-at search-string)
+                                           (match-data t (nth 2 elt)))
+                                         noedit
+                                         (replace-match-maybe-edit
+                                          next-replacement nocasify literal
+                                          noedit real-match-data backward)
+                                         replace-count (1- replace-count)
+                                         real-match-data
+                                         (save-excursion
+                                           (goto-char (match-beginning 0))
+                                           (looking-at next-replacement)
+                                           (match-data t (nth 2 elt))))
+                                   (when (eq def 'undo-all) ; Set replaced nil to keep in loop
+                                     (setq replaced          nil
+                                           stack-len         (- stack-len stack-idx)
+                                           stack-idx         0 
+                                           num-replacements  (1+ num-replacements)))))))
+			   (setq replaced nil last-was-undo t)))
 			((eq def 'act)
 			 (or replaced
 			     (setq noedit
@@ -2454,9 +2539,12 @@ perform-replace
 				 (match-beginning 0)
 				 (match-end 0)
 				 (current-buffer))
-			      (match-data t)))
-		      stack))))))
-
+			      (match-data t))
+				search-string-replaced
+				next-replacement-replaced)
+		      stack)
+                (setq next-replacement-replaced nil
+                      search-string-replaced    nil))))))
       (replace-dehighlight))
     (or unread-command-events
 	(message "Replaced %d occurrence%s%s"

bug#21684: 25.0.50; undo in query-replace w/o exit

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Thanks.  It seems useful.

(Please forgive my delay.)

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

bug#21684: 25.0.50; undo in query-replace w/o exit

[Lars Ingebrigtsen]

Tino Calancha <f92capac@gmail.com> writes:

> Good point. Thank you very much.
> Included in new patch (replace-4.patch).

Looks good.  Could you also add documentation for this (to the manual)
and a NEWS entry?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#21684: 25.0.50; undo in query-replace w/o exit

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 142 bytes --]


> Looks good.  Could you also add documentation for this (to the manual)
> and a NEWS entry?
Sure. Added NEWS entry and updated emacs manual.

[-- Attachment #2: Type: text/plain, Size: 8265 bytes --]

From 8c9f54de418166036df0da0c81d3c2b42da39430 Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Wed, 24 Feb 2016 01:30:52 +0900
Subject: [PATCH 1/2] Undo in query-replace without exit

* lisp/replace.el (perform-replace): 'undo', undo last replacement
and move back to that place; 'undo-all', undo all replacements
and move back to the place where the first replacement was performed.
(Bug#21684)
---
 lisp/replace.el | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 4 deletions(-)

diff --git a/lisp/replace.el b/lisp/replace.el
index 488eff7..ab839ce 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1824,6 +1824,8 @@ query-replace-help
 C-l to clear the screen, redisplay, and offer same replacement again,
 ! to replace all remaining matches in this buffer with no more questions,
 ^ to move point back to previous match,
+u to undo previous replacement,
+U to undo all replacements,
 E to edit the replacement string.
 In multi-buffer replacements type `Y' to replace all remaining
 matches in all remaining buffers with no more questions,
@@ -1853,6 +1855,8 @@ query-replace-map
     (define-key map "\C-l" 'recenter)
     (define-key map "!" 'automatic)
     (define-key map "^" 'backup)
+    (define-key map "u" 'undo)
+    (define-key map "U" 'undo-all)
     (define-key map "\C-h" 'help)
     (define-key map [f1] 'help)
     (define-key map [help] 'help)
@@ -1878,7 +1882,7 @@ query-replace-map
 `act-and-exit', `exit', `exit-prefix', `recenter', `scroll-up',
 `scroll-down', `scroll-other-window', `scroll-other-window-down',
 `edit', `edit-replacement', `delete-and-edit', `automatic',
-`backup', `quit', and `help'.
+`backup', `undo', `undo-all', `quit', and `help'.
 
 This keymap is used by `y-or-n-p' as well as `query-replace'.")
 
@@ -2132,6 +2136,9 @@ perform-replace
          (noedit nil)
          (keep-going t)
          (stack nil)
+         (search-string-replaced nil)    ; last string matching `from-string'
+         (next-replacement-replaced nil) ; replacement string (substituted regexp)
+         (last-was-undo)
          (replace-count 0)
          (skip-read-only-count 0)
          (skip-filtered-count 0)
@@ -2328,6 +2335,23 @@ perform-replace
 		   (match-beginning 0) (match-end 0)
 		   start end search-string
 		   regexp-flag delimited-flag case-fold-search backward)
+                  ;; Obtain the matched groups: needed only when regexp-flag non nil.
+                  (when (and last-was-undo regexp-flag)
+                    (setq last-was-undo nil
+                          real-match-data
+                          (save-excursion
+                            (goto-char (match-beginning 0))
+                            (looking-at search-string)
+                            (match-data t real-match-data))))
+                  ;; Matched string and next-replacement-replaced stored in stack.
+                  (setq search-string-replaced (buffer-substring-no-properties
+                                                (match-beginning 0)
+                                                (match-end 0))
+                        next-replacement-replaced
+                        (query-replace-descr
+                         (save-match-data
+                           (set-match-data real-match-data)
+                           (match-substitute-replacement next-replacement nocasify literal))))
 		  ;; Bind message-log-max so we don't fill up the message log
 		  ;; with a bunch of identical messages.
 		  (let ((message-log-max nil)
@@ -2381,6 +2405,66 @@ perform-replace
 			   (message "No previous match")
 			   (ding 'no-terminate)
 			   (sit-for 1)))
+			((or (eq def 'undo) (eq def 'undo-all))
+			 (if (null stack)
+                             (progn
+                               (message "Nothing to undo")
+                               (ding 'no-terminate)
+                               (sit-for 1))
+			   (let ((stack-idx         0)
+                                 (stack-len         (length stack))
+                                 (num-replacements  0)
+                                 search-string
+                                 next-replacement)
+                             (while (and (< stack-idx stack-len) stack (null replaced))
+                               (let* ((elt (nth stack-idx stack)))
+                                 (setq stack-idx                  (1+ stack-idx)
+                                       replaced                   (nth 1 elt)
+                                       ;; Bind swapped values (search-string <--> replacement)
+                                       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)
+
+                                 (when (and (= stack-idx stack-len)
+                                            (null replaced)
+                                            (zerop num-replacements))
+                                          (message "Nothing to undo")
+                                          (ding 'no-terminate)
+                                          (sit-for 1))
+
+                                 (when replaced
+                                   (setq stack (nthcdr stack-idx stack))
+                                   (goto-char (nth 0 elt))
+                                   (set-match-data (nth 2 elt))
+                                   (setq real-match-data
+                                         (save-excursion
+                                           (goto-char (match-beginning 0))
+                                           (looking-at search-string)
+                                           (match-data t (nth 2 elt)))
+                                         noedit
+                                         (replace-match-maybe-edit
+                                          next-replacement nocasify literal
+                                          noedit real-match-data backward)
+                                         replace-count (1- replace-count)
+                                         real-match-data
+                                         (save-excursion
+                                           (goto-char (match-beginning 0))
+                                           (looking-at next-replacement)
+                                           (match-data t (nth 2 elt))))
+                                   (when (eq def 'undo-all) ; Set replaced nil to keep in loop
+                                     (setq replaced          nil
+                                           stack-len         (- stack-len stack-idx)
+                                           stack-idx         0
+                                           num-replacements  (1+ num-replacements))))))
+                             (when (and (eq def 'undo-all) (null (zerop num-replacements)))
+                               (message "Undid %d %s" num-replacements
+                                        (if (= num-replacements 1)
+                                            "replacement"
+                                          "replacements"))
+                               (ding 'no-terminate)
+                               (sit-for 1)))
+			   (setq replaced nil last-was-undo t)))
 			((eq def 'act)
 			 (or replaced
 			     (setq noedit
@@ -2503,9 +2587,12 @@ perform-replace
 				 (match-beginning 0)
 				 (match-end 0)
 				 (current-buffer))
-			      (match-data t)))
-		      stack))))))
-
+			      (match-data t))
+				search-string-replaced
+				next-replacement-replaced)
+		      stack)
+                (setq next-replacement-replaced nil
+                      search-string-replaced    nil))))))
       (replace-dehighlight))
     (or unread-command-events
 	(message "Replaced %d occurrence%s%s"
-- 
2.7.0


[-- Attachment #3: Type: text/plain, Size: 2044 bytes --]

From a0bdcedd9aa6eb9e05dde2eae01389ae67f911c3 Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Wed, 24 Feb 2016 01:38:50 +0900
Subject: [PATCH 2/2] ; Add NEWS and documentation for last commit

---
 ChangeLog.2                | 7 +++++++
 doc/lispref/searching.texi | 8 ++++++++
 etc/NEWS                   | 5 +++++
 3 files changed, 20 insertions(+)

diff --git a/ChangeLog.2 b/ChangeLog.2
index 6edc654..0d76762 100644
--- a/ChangeLog.2
+++ b/ChangeLog.2
@@ -1,3 +1,10 @@
+2016-02-23  Tino Calancha  <f92capac@gmail.com>
+
+	* lisp/replace.el (perform-replace): `undo', undo last replacement
+	and move back to that place; `undo-all', undo all replacements
+	and move back to the place where the first replacement was performed.
+	(Bug#21684)
+
 2016-02-15  Glenn Morris  <rgm@gnu.org>
 
 	* lisp/dired-aux.el: Require cl-lib.  (Bug#22613)
diff --git a/doc/lispref/searching.texi b/doc/lispref/searching.texi
index 1243d72..644716a 100644
--- a/doc/lispref/searching.texi
+++ b/doc/lispref/searching.texi
@@ -1805,6 +1805,14 @@ Search and Replace
 @item backup
 Move back to the previous place that a question was asked about.
 
+@item undo
+Undo last replacement and move back to the place where that
+replacement was performed.
+
+@item undo-all
+Undo all replacements and move back to the place where the first
+replacement was performed.
+
 @item edit
 Enter a recursive edit to deal with this question---instead of any
 other action that would normally be taken.
diff --git a/etc/NEWS b/etc/NEWS
index 255afde..8c14e6d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -435,6 +435,11 @@ is intended for adding to 'kill-emacs-query-functions'.
 in favor of the global `M-s h' bindings introduced in Emacs-23.1.
 They'll disappear soon.
 
++++
+** New bindings for 'query-replace-map'.
+`undo', undo the last replacement; bind to `u'.
+`undo-all', undo all replacements; Bind to `U'.
+
 \f
 * Changes in Specialized Modes and Packages in Emacs 25.1
 
-- 
2.7.0

bug#21684: 25.0.50; undo in query-replace w/o exit

[Eli Zaretskii]

> Date: Wed, 24 Feb 2016 01:46:14 +0900 (JST)
> From: Tino Calancha <f92capac@gmail.com>
> Cc: Tino Calancha <f92capac@gmail.com>, 21684@debbugs.gnu.org
> 
> > Looks good.  Could you also add documentation for this (to the manual)
> > and a NEWS entry?
> Sure. Added NEWS entry and updated emacs manual.

Thanks.  A minor nit below:

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -435,6 +435,11 @@ is intended for adding to 'kill-emacs-query-functions'.
>  in favor of the global `M-s h' bindings introduced in Emacs-23.1.
>  They'll disappear soon.
>  
> ++++
> +** New bindings for 'query-replace-map'.
> +`undo', undo the last replacement; bind to `u'.
> +`undo-all', undo all replacements; Bind to `U'.

"Bound", not "bind"; and the capitalization needs to be fixed.

bug#21684: 25.0.50; undo in query-replace w/o exit

[Lars Ingebrigtsen]

Tino Calancha <f92capac@gmail.com> writes:

>> Looks good.  Could you also add documentation for this (to the manual)
>> and a NEWS entry?
> Sure. Added NEWS entry and updated emacs manual.

Thanks; I've now applied this to the trunk.  Seems to be working great,
and it's a feature I've missed for years.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#21684: 25.0.50; undo in query-replace w/o exit

[Tino Calancha]

> Thanks; I've now applied this to the trunk.  Seems to be working great,
> and it's a feature I've missed for years.
Great! It has being my pleasure.

bug#21684: 25.0.50; undo in query-replace w/o exit

[Glenn Morris]

Lars Ingebrigtsen wrote:

> Thanks; I've now applied this to the trunk.  Seems to be working great,
> and it's a feature I've missed for years.

Was a copyright assignment ever completed?
I don't see one on file.

http://debbugs.gnu.org/21684#14

Juri Linkov wrote:

> Have you already signed FSF copyright assignment papers?
>
> Your patch for bug#21663 is short enough to be installed without the
> copyright paperwork, but your changes for bug#21684 require it.

bug#21684: 25.0.50; undo in query-replace w/o exit

[Tino Calancha]

On Tue, 31 May 2016, Glenn Morris wrote:

> Was a copyright assignment ever completed?
> I don't see one on file.
>
> http://debbugs.gnu.org/21684#14

Yes it was, around November 2015: copyright assignment  #1056152

Tino

bug#21684: 25.0.50; undo in query-replace w/o exit

[Glenn Morris]

Tino Calancha wrote:

>> Was a copyright assignment ever completed?
>> I don't see one on file.
>>
>> http://debbugs.gnu.org/21684#14
>
> Yes it was, around November 2015: copyright assignment  #1056152

Thanks. The semi-public file that records these things must not be
up-to-date (not for the first time). Not your problem. I'll ask the
relevant people.

bug#21684: 25.0.50; undo in query-replace w/o exit

[Lars Ingebrigtsen]

Glenn Morris <rgm@gnu.org> writes:

> Tino Calancha wrote:
>
>>> Was a copyright assignment ever completed?
>>> I don't see one on file.
>>>
>>> http://debbugs.gnu.org/21684#14
>>
>> Yes it was, around November 2015: copyright assignment  #1056152
>
> Thanks. The semi-public file that records these things must not be
> up-to-date (not for the first time). Not your problem. I'll ask the
> relevant people.

It was the last time I fetched it, but the name is in a slightly
different format.  :-)

Search for "Calancha".  (I'm not sure I'm allowed to post the full name
from the assignment here.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#21684: 25.0.50; undo in query-replace w/o exit

[Glenn Morris]

Lars Ingebrigtsen wrote:

> It was the last time I fetched it, but the name is in a slightly
> different format.  :-)
>
> Search for "Calancha".  (I'm not sure I'm allowed to post the full name
> from the assignment here.)

I don't think my grep-fu is to blame here. I generally try a broad search.
I've asked assign@gnu. There seem to be several missing entries AFAICS.

bug#21684: 25.0.50; undo in query-replace w/o exit

[Glenn Morris]

Glenn Morris wrote:

> I've asked assign@gnu. There seem to be several missing entries AFAICS.

The file that records the info was indeed out-of-date (malfunctioning
cron job) and has now been fixed.

bug#21663: 25.0.50; isearch-edit-string dont resume multi isearches

[Juri Linkov]

> Lars Ingebrigtsen wrote:
>
>> Thanks; I've now applied this to the trunk.  Seems to be working great,
>> and it's a feature I've missed for years.
>
> Was a copyright assignment ever completed?
> I don't see one on file.
>
> http://debbugs.gnu.org/21684#14
>
> Juri Linkov wrote:
>
>> Have you already signed FSF copyright assignment papers?
>>
>> Your patch for bug#21663 is short enough to be installed without the
>> copyright paperwork, but your changes for bug#21684 require it.

Thanks for the reminder.  This is now installed (required merging
with other changes in the same function).