Re: query-replace-interactive

[Juri Linkov <juri@jurta.org>]

Stefan <monnier@iro.umontreal.ca> writes:
> Is the added complexity of (eq query-replace-interactive 'initial) really
> worth it ?
> For one, it has an unusual behavior (the Emacs way would be not
> to put it as INITIAL but as DEFAULT and to place it in the prompt as
> "Replace (default foo): ").

It is right to put it as DEFAULT instead as INITIAL, but I think
we should not place it in the prompt, because displaying the last
search string in the M-% prompt every time M-% is invoked might be too
confusing.  If M-% is typed not immediately after exiting the search,
the last search string might look completely inappropriate.

But the last search string still could be used as the `default'
argument of `read-from-minibuffer'.  This is not quite a "default"
value but rather what the Emacs manual names as "future history" value,
i.e. the value available with M-n.

> For two, it's only used for isearch-query-replace and in my experience this
> additional prompt is just an annoyance: if I hit M-% in isearch I really
> want to replace the currently matched text.

I implemented it having in mind users' requests for the ability to
copy words from the buffer into the from-string.  Now this is possible
by typing:

C-s C-w C-w ... M-%

But with using the last search string as a "default" value there is
no need for an additional prompt, because to edit the from-string
before making replacements, the user can type:

C-s C-w C-w ... RET M-% M-n

> So I suggest we back out this introduction of an `initial' value of
> query-replace-interactive.

I agree.  But I also think we should revert its definition from
defcustom back to defvar, since as a user option it is useless now:
users have no reason to set it permanently to t if they can now invoke
M-% from isearch, and also the last search string/regexp is easily
accessible in the minibuffer with M-n.

> I also incidentally suggest that
> isearch-query-replace don't do (call-interactively 'query-replace) but use
> (perform-replace isearch-string nil t isearch-regexp isearch-word) instead.
                                  ===
If the argument `replacements' is nil, this function signals an error.
Moreover, it's impossible to guess the to-string, so asking for it from
the user is still inevitable.

> After all, we already agreed that M-% in isearch should obey the
> isearch-regexp.

Yes, but I think C-M-% should be available in isearch too
to make it compatible with typing C-M-% not in isearch.

Below is a new patch to isearch.el and replace.el.
I could update the Emacs manual later if this change is ok.

Index: lisp/replace.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/replace.el,v
retrieving revision 1.180
diff -u -r1.180 replace.el
--- lisp/replace.el	3 Jul 2004 05:18:38 -0000	1.180
+++ lisp/replace.el	4 Jul 2004 09:44:13 -0000
@@ -36,15 +36,9 @@
 
 (defvar query-replace-history nil)
 
-(defcustom query-replace-interactive nil
+(defvar query-replace-interactive nil
   "Non-nil means `query-replace' uses the last search string.
-That becomes the \"string to replace\".
-If value is `initial', the last search string is inserted into
-the minibuffer as an initial value for \"string to replace\"."
-  :type '(choice (const :tag "Off" nil)
-                 (const :tag "Initial content" initial)
-                 (other :tag "Use default value" t))
-  :group 'matching)
+That becomes the \"string to replace\".")
 
 (defcustom query-replace-from-history-variable 'query-replace-history
   "History list to use for the FROM argument of `query-replace' commands.
@@ -74,8 +68,7 @@
   (unless noerror
     (barf-if-buffer-read-only))
   (let (from to)
-    (if (and query-replace-interactive
-             (not (eq query-replace-interactive 'initial)))
+    (if query-replace-interactive
         (setq from (car (if regexp-flag regexp-search-ring search-ring)))
       ;; The save-excursion here is in case the user marks and copies
       ;; a region in order to specify the minibuffer input.
@@ -83,11 +76,10 @@
       (save-excursion
         (setq from (read-from-minibuffer
                     (format "%s: " string)
-                    (if (eq query-replace-interactive 'initial)
-                        (car (if regexp-flag regexp-search-ring search-ring)))
-                    nil nil
+                    nil nil nil
                     query-replace-from-history-variable
-                    nil t)))
+                    (car (if regexp-flag regexp-search-ring search-ring))
+                    t)))
       ;; Warn if user types \n or \t, but don't reject the input.
       (and regexp-flag
 	   (string-match "\\(\\`\\|[^\\]\\)\\(\\\\\\\\\\)*\\(\\\\[nt]\\)" from)
@@ -148,9 +140,12 @@
 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.
+You can access the last incremental search string in the minibuffer
+with \\<minibuffer-local-map>\\[next-history-element].  \
+Typing \\<isearch-mode-map>\\[isearch-query-replace] \
+in incremental search invokes this command
+with the last incremental search string used as FROM-STRING--
+you don't have to specify it with the minibuffer.
 
 Matching is independent of case if `case-fold-search' is non-nil and
 FROM-STRING has no uppercase letters.  Replacement transfers the case
@@ -187,9 +182,12 @@
 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.
+You can access the last incremental search regexp in the minibuffer
+with \\<minibuffer-local-map>\\[next-history-element].  \
+Typing \\<isearch-mode-map>\\[isearch-query-replace-regexp] in incremental search \
+invokes this command
+with the last incremental search string used as FROM-STRING--
+you don't have to specify it with the minibuffer.
 
 Matching is independent of case if `case-fold-search' is non-nil and
 REGEXP has no uppercase letters.  Replacement transfers the case
@@ -258,9 +256,8 @@
 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.
+You can access the last incremental search regexp in the minibuffer
+by typing \\<minibuffer-local-map>\\[next-history-element].
 
 Preserves case in each replacement if `case-replace' and `case-fold-search'
 are non-nil and REGEXP has no uppercase letters.
@@ -275,7 +272,7 @@
        (setq from (read-from-minibuffer "Query replace regexp: "
                                         nil nil nil
                                         query-replace-from-history-variable
-                                        nil t)))
+                                        (car regexp-search-ring) t)))
      (setq to (list (read-from-minibuffer
                      (format "Query replace regexp %s with eval: " from)
                      nil nil t query-replace-to-history-variable from t)))
@@ -304,8 +301,8 @@
 
 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.
+You can access the last incremental search regexp in the minibuffer
+by typing \\<minibuffer-local-map>\\[next-history-element].
 
 A prefix argument N says to use each replacement string N times
 before rotating to the next.
@@ -316,7 +313,8 @@
 		    (car regexp-search-ring)
 		  (read-from-minibuffer "Map query replace (regexp): "
 					nil nil nil
-					'query-replace-history nil t)))
+					'query-replace-history
+					(car regexp-search-ring) t)))
      (setq to (read-from-minibuffer
 	       (format "Query replace %s with (space-separated strings): "
 		       from)
@@ -358,9 +356,8 @@
 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.
+You can access the last incremental search string in the minibuffer
+by typing \\<minibuffer-local-map>\\[next-history-element].
 
 This function is usually the wrong thing to use in a Lisp program.
 What you probably want is a loop like this:
@@ -415,8 +412,8 @@
 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.
+You can access the last incremental search regexp in the minibuffer
+by typing \\<minibuffer-local-map>\\[next-history-element].
 
 This function is usually the wrong thing to use in a Lisp program.
 What you probably want is a loop like this:

Index: lisp/isearch.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/isearch.el,v
retrieving revision 1.230
diff -c -r1.230 isearch.el
*** lisp/isearch.el	1 Jul 2004 09:54:51 -0000	1.230
--- lisp/isearch.el	4 Jul 2004 09:45:09 -0000
***************
*** 1059,1087 ****
    (sit-for 1)
    (isearch-update))
  
! (defun isearch-query-replace ()
    "Start query-replace with string to replace from last search string."
    (interactive)
    (let ((query-replace-interactive 'initial)
          (case-fold-search isearch-case-fold-search))
-     ;; Put search string into the right ring
-     (setq isearch-regexp nil)
      (isearch-done)
      (isearch-clean-overlays)
!     (and isearch-forward isearch-other-end (goto-char isearch-other-end))
!     (call-interactively 'query-replace)))
  
  (defun isearch-query-replace-regexp ()
    "Start query-replace-regexp with string to replace from last search string."
    (interactive)
!   (let ((query-replace-interactive 'initial)
!         (case-fold-search isearch-case-fold-search))
!     ;; Put search string into the right ring
!     (setq isearch-regexp t)
!     (isearch-done)
!     (isearch-clean-overlays)
!     (and isearch-forward isearch-other-end (goto-char isearch-other-end))
!     (call-interactively 'query-replace-regexp)))
  
  \f
  (defun isearch-delete-char ()
--- 1059,1081 ----
    (sit-for 1)
    (isearch-update))
  
! (defun isearch-query-replace (&optional regexp-flag)
    "Start query-replace with string to replace from last search string."
    (interactive)
+   (if regexp-flag (setq isearch-regexp t))
    (let ((query-replace-interactive 'initial)
          (case-fold-search isearch-case-fold-search))
      (isearch-done)
      (isearch-clean-overlays)
!     (if (< isearch-other-end (point)) (goto-char isearch-other-end))
!     (if isearch-regexp
!         (call-interactively 'query-replace-regexp)
!       (call-interactively 'query-replace))))
  
  (defun isearch-query-replace-regexp ()
    "Start query-replace-regexp with string to replace from last search string."
    (interactive)
!   (isearch-query-replace t))
  
  \f
  (defun isearch-delete-char ()

-- 
Juri Linkov
http://www.jurta.org/emacs/