bug#12526: 24.2.50; `query-replace-interactive' not always respected

[Juri Linkov]

>>> Is that feature documented somewhere?
>>
>> Yes, it is documented in the Info node
>> (info "(emacs) Special Isearch")
>
> Ok, but definitely the docstring of query-replace should be updated,
> because currently it is misleading:
>   If `query-replace-interactive' is non-nil, the last incremental search
>   string is used as FROM-STRING--you don't have to specify it with the
>   minibuffer.

Thanks for the suggestion.  I fixed this docstring in the patch below.

> Also, the docstring of `query-replace-interactive' should be update
> for the same reason.

Since it's easy to do `C-s C-s M-%' to replace the last search string,
the option `query-replace-interactive' is useless now and can be
declared obsolete.  Another reason to remove it is your suggestion below
to use `M-% M-n' as its replacement.

>> (define-key isearch-mode-map [?\M-%]
>>   (lambda ()
>>     (interactive)
>>     (isearch-exit)  ;; I'm too lazy to terminate search with RET.
>>     (call-interactively 'query-replace)))
>
> This is an ugly solution, IMO.  I don't want to define a new command
> and bind M-% to it;

If you don't want to define a new command,
then you can add just one line to your init file:

(define-key isearch-mode-map [?\M-%] 'isearch-other-meta-char)

> I'd like to set some user-option in my init file so that query-replace
> never assumes nothing about the "from" string.

If you don't want to use `define-key' for configuration purposes,
we might consider using an user option, but not `query-replace-interactive'
that is irrelevant for isearch.

Such an option has been discussed but not yet implemented.
To get an idea how this option could work, please see bug#9706
and its related thread on emacs-devel.  With this new option,
you will be able to define any command (not only `query-replace')
to bypass its isearch keybinding to its original command.

> BTW: I think that a much better approach would be to add the last
> search string to the list of default arguments, so that, after an
> Isearch, I could do `M-% M-n' if I wanted to retrieve that last search
> string.

Two weeks ago I added the last search string to `C-M-% M-n'.
But it's still missing for `M-% M-n'.  Thank you for noticing this omission.
This is fixed in the patch below.  This is another reason
to declare `query-replace-interactive' obsolete because now
there are two functional replacements for it as is documented
in the fixed docstrings.

Also I verified that "Query replace (default foo -> bar): RET"
still works correctly and empty input redoes the last replace.

=== modified file 'lisp/replace.el'
--- lisp/replace.el     2012-09-25 04:13:02 +0000
+++ lisp/replace.el     2012-10-03 23:38:00 +0000
@@ -62,6 +62,10 @@ (defvar query-replace-defaults nil
 (defvar query-replace-interactive nil
   "Non-nil means `query-replace' uses the last search string.
 That becomes the \"string to replace\".")
+(make-obsolete-variable 'query-replace-interactive
+                       "use `M-n' in the minibuffer
+that asks for the string to replace, or run `query-replace'
+from Isearch by using a key sequence like `C-s C-s M-%'." "24.3")
 
 (defcustom query-replace-from-history-variable 'query-replace-history
   "History list to use for the FROM argument of `query-replace' commands.
@@ -125,8 +129,6 @@ (defun query-replace-read-from (prompt r
   "Query and return the `from' argument of a query-replace operation.
 The return value can also be a pair (FROM . TO) indicating that the user
 wants to replace FROM with TO."
-  (if query-replace-interactive
-      (car (if regexp-flag regexp-search-ring search-ring))
     (let* ((history-add-new-input nil)
           (prompt
            (if query-replace-defaults
@@ -142,7 +144,8 @@ (defun query-replace-read-from (prompt r
              (if regexp-flag
                  (read-regexp prompt nil query-replace-from-history-variable)
                (read-from-minibuffer
-                prompt nil nil nil query-replace-from-history-variable nil 
t)))))
+              prompt nil nil nil query-replace-from-history-variable
+              (car (if regexp-flag regexp-search-ring search-ring)) t)))))
       (if (and (zerop (length from)) query-replace-defaults)
          (cons (car query-replace-defaults)
                (query-replace-compile-replacement
@@ -158,7 +161,7 @@ (defun query-replace-read-from (prompt r
                ((string= match "\\t")
                 (message "Note: `\\t' here doesn't match a tab; to do that, 
just type TAB")))
               (sit-for 2)))
-       from))))
+      from)))
 
 (defun query-replace-compile-replacement (to regexp-flag)
   "Maybe convert a regexp replacement TO to Lisp.
@@ -231,9 +234,11 @@ (defun query-replace (from-string to-str
 In Transient Mark mode, if the mark is active, operate on the contents
 of the region.  Otherwise, operate from point to the end of the buffer.
 
-If `query-replace-interactive' is non-nil, the last incremental search
-string is used as FROM-STRING--you don't have to specify it with the
-minibuffer.
+Use \\<minibuffer-local-map>\\[next-history-element] \
+to pull the last incremental search string to the minibuffer
+that reads FROM-STRING, or start `query-replace' from
+incremental search with a key sequence like `C-s C-s M-%'
+to use its current search string as the string to replace.
 
 Matching is independent of case if `case-fold-search' is non-nil and
 FROM-STRING has no uppercase letters.  Replacement transfers the case
@@ -279,9 +284,11 @@ (defun query-replace-regexp (regexp to-s
 In Transient Mark mode, if the mark is active, operate on the contents
 of the region.  Otherwise, operate from point to the end of the buffer.
 
-If `query-replace-interactive' is non-nil, the last incremental search
-regexp is used as REGEXP--you don't have to specify it with the
-minibuffer.
+Use \\<minibuffer-local-map>\\[next-history-element] \
+to pull the last incremental search regexp to the minibuffer
+that reads REGEXP, or start `query-replace-regexp' from
+incremental search with a key sequence like `C-M-s C-M-s C-M-%'
+to use its current search regexp as the regexp to replace.
 
 Matching is independent of case if `case-fold-search' is non-nil and
 REGEXP has no uppercase letters.  Replacement transfers the case
@@ -364,9 +371,9 @@ (defun query-replace-regexp-eval (regexp
 In Transient Mark mode, if the mark is active, operate on the contents
 of the region.  Otherwise, operate from point to the end of the buffer.
 
-If `query-replace-interactive' is non-nil, the last incremental search
-regexp is used as REGEXP--you don't have to specify it with the
-minibuffer.
+Use \\<minibuffer-local-map>\\[next-history-element] \
+to pull the last incremental search regexp to the minibuffer
+that reads REGEXP.
 
 Preserves case in each replacement if `case-replace' and `case-fold-search'
 are non-nil and REGEXP has no uppercase letters.
@@ -417,19 +424,16 @@ (defun map-query-replace-regexp (regexp
 
 Non-interactively, TO-STRINGS may be a list of replacement strings.
 
-If `query-replace-interactive' is non-nil, the last incremental search
-regexp is used as REGEXP--you don't have to specify it with the minibuffer.
+Use \\<minibuffer-local-map>\\[next-history-element] \
+to pull the last incremental search regexp to the minibuffer
+that reads REGEXP.
 
 A prefix argument N says to use each replacement string N times
 before rotating to the next.
 Fourth and fifth arg START and END specify the region to operate on."
   (interactive
-   (let* ((from (if query-replace-interactive
-                   (car regexp-search-ring)
-                 (read-from-minibuffer "Map query replace (regexp): "
-                                       nil nil nil
-                                       query-replace-from-history-variable
-                                       nil t)))
+   (let* ((from (read-regexp "Map query replace (regexp): " nil
+                            query-replace-from-history-variable))
          (to (read-from-minibuffer
               (format "Query replace %s with (space-separated strings): "
                       (query-replace-descr from))
@@ -475,9 +479,9 @@ (defun replace-string (from-string to-st
 only matches surrounded by word boundaries.
 Fourth and fifth arg START and END specify the region to operate on.
 
-If `query-replace-interactive' is non-nil, the last incremental search
-string is used as FROM-STRING--you don't have to specify it with the
-minibuffer.
+Use \\<minibuffer-local-map>\\[next-history-element] \
+to pull the last incremental search string to the minibuffer
+that reads FROM-STRING.
 
 This function is usually the wrong thing to use in a Lisp program.
 What you probably want is a loop like this:
@@ -540,8 +544,9 @@ (defun replace-regexp (regexp to-string
 text, TO-STRING is actually made a list instead of a string.
 Use \\[repeat-complex-command] after this command for details.
 
-If `query-replace-interactive' is non-nil, the last incremental search
-regexp is used as REGEXP--you don't have to specify it with the minibuffer.
+Use \\<minibuffer-local-map>\\[next-history-element] \
+to pull the last incremental search regexp to the minibuffer
+that reads REGEXP.
 
 This function is usually the wrong thing to use in a Lisp program.
 What you probably want is a loop like this: