unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* query-replace-interactive
@ 2004-07-03 22:59 Stefan
  2004-07-04  9:54 ` query-replace-interactive Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan @ 2004-07-03 22:59 UTC (permalink / raw)



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): ").
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.

So I suggest we back out this introduction of an `initial' value of
query-replace-interactive.  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 really someone is in isearch and wants to do a query-replace on
something else than what he's currently searching, he can do RET M-%.

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


        Stefan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-03 22:59 query-replace-interactive Stefan
@ 2004-07-04  9:54 ` Juri Linkov
  2004-07-04 16:13   ` query-replace-interactive Stefan
  0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2004-07-04  9:54 UTC (permalink / raw)
  Cc: emacs-devel

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/

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-04  9:54 ` query-replace-interactive Juri Linkov
@ 2004-07-04 16:13   ` Stefan
  2004-07-05  6:08     ` query-replace-interactive Juri Linkov
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan @ 2004-07-04 16:13 UTC (permalink / raw)
  Cc: emacs-devel

> 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.

I think we've already agreed that M-% should offer

   Query replace (default foo -> bar): 

i.e. default to redo the same replace as last time, rather than use the
last search string.

> 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-%

C-SPC M-f M-f ... M-w M-% C-y

with the advantage that it works not just for "take words and pass them to
M-%" but for non-words as well and for other commands than M-%.
Other cases could be

M-C-SPC M-C-SPC ... M-w M-% C-y

> 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

Nowadays, I never use M-% directly  other than to redo the last replace.
I.e. instead of

   M-% foo RET bar RET

I always do

   C-s foo M-% bar RET

that also allows me to easily choose regexp-or-no, delimited-or-no, and
case-fold-or-no without having to remember whether it's C-M-% or C-u M-%
or ...

> I agree.  But I also think we should revert its definition from
> defcustom back to defvar,

Sure.

>> 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.

Oh, right, I forgot that part of my local hacks

    --- orig/lisp/replace.el
    +++ mod/lisp/replace.el
    @@ -1336,6 +1366,10 @@
     		    (funcall (car replacements) (cdr replacements)
     			     replace-count)
     		    noedit nil))
    +	    (unless next-replacement
    +	      (setq next-replacement
    +		    (read-string "Replace with: " nil nil
    +				 from-string)))
     	    (if (not query-flag)
     		(let ((inhibit-read-only
     		       query-replace-skip-read-only))

Obviously, it's not good enough.  But we can easily take out the "read
`to'" part of query-replace-read-args.  The point about using
perform-replace is that it allows us to obey both isearch-regexp and
isearch-word without doing any if-gymnastic.


        Stefan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-04 16:13   ` query-replace-interactive Stefan
@ 2004-07-05  6:08     ` Juri Linkov
  2004-07-05 12:44       ` query-replace-interactive Stefan
                         ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Juri Linkov @ 2004-07-05  6:08 UTC (permalink / raw)
  Cc: emacs-devel

Stefan <monnier@iro.umontreal.ca> writes:
>> 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.
>
> I think we've already agreed that M-% should offer
>
>    Query replace (default foo -> bar): 
>
> i.e. default to redo the same replace as last time, rather than use the
> last search string.

Even if using the last replacement as default doesn't hurt, such use
of the default value is non-standard in Emacs.  Usually, the default
value is available for inserting into the minibuffer by typing M-n,
but not in this case, since inserting "foo -> bar" into the minibuffer
is useless.  Moreover, the last replacement actually consists of the
last elements of the replacement history.  The standard Emacs way to
access the history values is M-p.  But this is not possible for two
separate replacement values (from and to) either.

One possibility to follow Emacs conventions is to use a user-defined
value `query-replace-args-separator' to allow to specify both
from-string and to-string in the same input string.  In this case
there is no need in interpreting an empty input string as repeating
the last replacement since the last replacement (as well as all
previous replacements) are easily accessible by M-p.  However, I am
not sure this option should have some non-nil value by default.

But even in case with the default to repeat the last replacement it is
possible to use the last search string/regexp as a default value only
for the sake of making it available by M-n.  But the real default value
(such as used when the user types RET with empty input) could be
interpreted as repeating the last replacement, since if the user
enters empty input, `read-from-minibuffer' returns the empty string,
not the argument `default-value'.

>> 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-%
>
> C-SPC M-f M-f ... M-w M-% C-y

Yes, currently this is the most convenient method to do that but it
requires more keystrokes and wastes the kill ring.

There are other cases where making the last search string accessible
by M-n is useful.  For example, users might want to start replacing
the current search string from the beginning of the buffer.  To not
type C-r several times to reach the beginning of the buffer the user
can type:

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

So, in conclusion, I agree that M-% in isearch should not ask for
the from-string with initial content of the last search string,
but M-% invoked not from isearch could provide the last search string
by M-n.

>>> (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.
>
> Oh, right, I forgot that part of my local hacks
> [...]
> Obviously, it's not good enough.  But we can easily take out the "read
> `to'" part of query-replace-read-args.

Calling `perform-replace' directly is not good because it skips many
useful things implemented in interactive commands which call it.
Most useful of them is recently added handling of \, and \# in
`query-replace-read-args'.

> The point about using perform-replace is that it allows us to obey
> both isearch-regexp and isearch-word without doing any if-gymnastic.

Using `isearch-regexp' to call either `query-replace' or
`query-replace-regexp' is easy.  Using the value of `isearch-word' for
replacement might be tricky but currently this is possible with the
prefix argument C-u M-% or C-u C-M-% even in isearch.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-05  6:08     ` query-replace-interactive Juri Linkov
@ 2004-07-05 12:44       ` Stefan
  2004-07-06  9:56         ` query-replace-interactive Juri Linkov
  2004-07-06 11:53       ` query-replace-interactive Richard Stallman
  2004-07-06 11:53       ` query-replace-interactive Richard Stallman
  2 siblings, 1 reply; 21+ messages in thread
From: Stefan @ 2004-07-05 12:44 UTC (permalink / raw)
  Cc: emacs-devel

> But even in case with the default to repeat the last replacement it is
> possible to use the last search string/regexp as a default value only
> for the sake of making it available by M-n.  But the real default value
> (such as used when the user types RET with empty input) could be
> interpreted as repeating the last replacement, since if the user
> enters empty input, `read-from-minibuffer' returns the empty string,
> not the argument `default-value'.

But as you said regarding the "a -> b" default, "such use of the default
value is non-standard in Emacs".  And more importantly, it's unimportant.
If you want to reuse a previous search string for replacement, just do

        C-s C-s M-%

instead of

        M-% M-n RET

this also has the advantage of being more general, so you can also do
a replacement on the penultimate search string if you want:

        C-s M-p M-p C-s M-%

>>> C-s C-w C-w ... M-%
>> 
>> C-SPC M-f M-f ... M-w M-% C-y

> Yes, currently this is the most convenient method to do that but it
> requires more keystrokes and wastes the kill ring.

For what it's worth, I can also do

   M-% C-w C-w C-w RET

since I've added a C-w binding in minibuffer-local-map that mimics
isearch's C-w.

> There are other cases where making the last search string accessible
> by M-n is useful.  For example, users might want to start replacing
> the current search string from the beginning of the buffer.  To not
> type C-r several times to reach the beginning of the buffer the user
> can type:

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

Why is that better than

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

> So, in conclusion, I agree that M-% in isearch should not ask for
> the from-string with initial content of the last search string,
> but M-% invoked not from isearch could provide the last search string
> by M-n.

I don't think it's worth the trouble.

>> Obviously, it's not good enough.  But we can easily take out the "read
>> `to'" part of query-replace-read-args.

> Calling `perform-replace' directly is not good because it skips many
> useful things implemented in interactive commands which call it.
> Most useful of them is recently added handling of \, and \# in
> `query-replace-read-args'.

That's exactly what I meant by the "read `to'" part.  We can take it out of
query-replace-read-args and call it explicitly.

>> The point about using perform-replace is that it allows us to obey
>> both isearch-regexp and isearch-word without doing any if-gymnastic.

> Using `isearch-regexp' to call either `query-replace' or
> `query-replace-regexp' is easy.  Using the value of `isearch-word' for
> replacement might be tricky but currently this is possible with the
> prefix argument C-u M-% or C-u C-M-% even in isearch.

And if you call perform-replace it's neither tricky nor requires
remembering any particular C-u combination.


        Stefan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-05 12:44       ` query-replace-interactive Stefan
@ 2004-07-06  9:56         ` Juri Linkov
  2004-07-06 22:00           ` query-replace-interactive Richard Stallman
  2004-07-06 22:00           ` query-replace-interactive Richard Stallman
  0 siblings, 2 replies; 21+ messages in thread
From: Juri Linkov @ 2004-07-06  9:56 UTC (permalink / raw)
  Cc: emacs-devel

Stefan <monnier@iro.umontreal.ca> writes:
> But as you said regarding the "a -> b" default, "such use of the default
> value is non-standard in Emacs".

Using the "a -> b" default would be a good improvement, but it seems
still unfinished in its current implementation.  There are several
problems with it:

1. When users see "(default a -> b)" text in the prompt and want
to change the last replacement slightly, the first reaction is to type
M-n to insert it into the minibuffer for editing, since this is the
standard Emacs behavior.  Instead of that, they will get an error
"End of history; no default available".  Then they might try M-p,
but this doesn't insert the last replacement into the minibuffer either.

2. It is limited only to the last replacement.  As such, it is not
a big improvement, since to repeat other replacements, users still
need to use old and inconvenient methods: C-x ESC ESC or messing
in the minibuffer history with intermixed `from' and `to' history elements.

3. The last replacement `from' and `to' strings might be too long
so that displaying them in the prompt becomes undesirable.

To cope with these problems we could maintain a separate history list
for *all* replacements in the form '("a -> b" "x -> y") and
to make it available for history commands in `read-from-minibuffer'.
This requires choosing such a separator which will have low probability
to appear in text to replace.  But maybe even " -> " is a good default
separator.

> For what it's worth, I can also do
>
>    M-% C-w C-w C-w RET
>
> since I've added a C-w binding in minibuffer-local-map that mimics
> isearch's C-w.

Why not add something like this to Emacs?  Or maybe more general
character-based command like typing C-f in the end of the minibuffer
to pull text from the source buffer character by character to the minibuffer.

>>> Obviously, it's not good enough.  But we can easily take out the "read
>>> `to'" part of query-replace-read-args.
>
>> Calling `perform-replace' directly is not good because it skips many
>> useful things implemented in interactive commands which call it.
>> Most useful of them is recently added handling of \, and \# in
>> `query-replace-read-args'.
>
> That's exactly what I meant by the "read `to'" part.  We can take it out of
> query-replace-read-args and call it explicitly.

Using `perform-replace' directly still has at least three problems:

1. It doesn't take into account the region boundaries in transient-mark
mode.  But this is useful even when M-% is called from isearch: to put
the upper bound, then to search for the first occurrence of a string
and to start the replacement in the region, i.e. with transient-mark
mode to type:

C-SPC M-< C-s C-s M-%

Though, this can be fixed by adding to (region-beginning) and (region-end)
explicitly as last two arguments of the `perform-replace' call.

2. It doesn't put the last replacement invoked from isearch to the
command history available by C-x ESC ESC.  This might be unnecessary
if all replacements were available by the history command M-n in the
minibuffer as proposed above.

3. It doesn't put the last search string into the history.  In its
current implementation it provides it as a default value available
by M-n in the to-string minibuffer, but it's not where users expect
to find it.  The standard method to access the last value of from-string
in the to-string minibuffer is M-p.

This can be fixed by pushing the last search string into from-history
before calling `query-replace-read-to'.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-05  6:08     ` query-replace-interactive Juri Linkov
  2004-07-05 12:44       ` query-replace-interactive Stefan
@ 2004-07-06 11:53       ` Richard Stallman
  2004-07-06 12:24         ` query-replace-interactive Stefan
  2004-07-06 11:53       ` query-replace-interactive Richard Stallman
  2 siblings, 1 reply; 21+ messages in thread
From: Richard Stallman @ 2004-07-06 11:53 UTC (permalink / raw)
  Cc: monnier, emacs-devel

    but M-% invoked not from isearch could provide the last search string
    by M-n.

That sounds like a good and unproblematical idea.  Right now there is
no default for the from-string, so it can't cause any trouble to offer
the last search string as a default.

We could then perhaps make M-% no longer special in a search.
Instead, the default value would do the whole job.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-05  6:08     ` query-replace-interactive Juri Linkov
  2004-07-05 12:44       ` query-replace-interactive Stefan
  2004-07-06 11:53       ` query-replace-interactive Richard Stallman
@ 2004-07-06 11:53       ` Richard Stallman
  2004-07-06 12:09         ` query-replace-interactive David Kastrup
  2 siblings, 1 reply; 21+ messages in thread
From: Richard Stallman @ 2004-07-06 11:53 UTC (permalink / raw)
  Cc: monnier, emacs-devel

    Calling `perform-replace' directly is not good because it skips many
    useful things implemented in interactive commands which call it.
    Most useful of them is recently added handling of \, and \# in
    `query-replace-read-args'.

Why are these implemented in the interactive commands
andnot in perform-replace directly?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-06 11:53       ` query-replace-interactive Richard Stallman
@ 2004-07-06 12:09         ` David Kastrup
  2004-07-07  6:41           ` query-replace-interactive Richard Stallman
  0 siblings, 1 reply; 21+ messages in thread
From: David Kastrup @ 2004-07-06 12:09 UTC (permalink / raw)
  Cc: Juri Linkov, monnier, emacs-devel

Richard Stallman <rms@gnu.org> writes:

>     Calling `perform-replace' directly is not good because it skips many
>     useful things implemented in interactive commands which call it.
>     Most useful of them is recently added handling of \, and \# in
>     `query-replace-read-args'.
> 
> Why are these implemented in the interactive commands
> andnot in perform-replace directly?

Basically because they are actually more like a user-friendly frontend
to query-replace-regexp-eval functionality than a command of their
own.  Since arbitrary code can be executed by `\,', existing uses of
all replacement commands in Lisp programs would have different
implications for security.  I did not want to introduce security
relevant changes in the middle of the release consolidation.

As long as the automatic evaluation of stuff is restricted to
interactive usage, and as long as you can easily cut&paste the
respective generated Lisp calls with C-x ESC ESC, I think that there
are no security concerns in connection with the new functionality.

Now `\#' is harmless in this regard compared to `\,', but I don't
think that there is much of a point in splitting just `\#' off to a
different place.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-06 11:53       ` query-replace-interactive Richard Stallman
@ 2004-07-06 12:24         ` Stefan
  2004-07-07  6:41           ` query-replace-interactive Richard Stallman
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan @ 2004-07-06 12:24 UTC (permalink / raw)
  Cc: Juri Linkov, emacs-devel

>     but M-% invoked not from isearch could provide the last search string
>     by M-n.

> That sounds like a good and unproblematical idea.  Right now there is
> no default for the from-string, so it can't cause any trouble to offer
> the last search string as a default.

> We could then perhaps make M-% no longer special in a search.
> Instead, the default value would do the whole job.

But then

    C-s foo M-% bar RET

becomes

    C-s foo M-% M-n RET bar RET

and you lose the fact that M-% obeys the regexp/delimited/case-fold
attributes of the isearch.


        Stefan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-06  9:56         ` query-replace-interactive Juri Linkov
@ 2004-07-06 22:00           ` Richard Stallman
  2004-07-07  5:11             ` query-replace-interactive Juri Linkov
  2004-07-06 22:00           ` query-replace-interactive Richard Stallman
  1 sibling, 1 reply; 21+ messages in thread
From: Richard Stallman @ 2004-07-06 22:00 UTC (permalink / raw)
  Cc: monnier, emacs-devel

    1. When users see "(default a -> b)" text in the prompt and want
    to change the last replacement slightly, the first reaction is to type
    M-n to insert it into the minibuffer for editing, since this is the
    standard Emacs behavior.  Instead of that, they will get an error
    "End of history; no default available".

I misunderstood the idea of "(default a->b)".  Now that I see what
it really does, I agree with you that it has lots of problems.

Here's what I THOUGHT the idea was.

The idea is that M-x query-replace would read just one minibuffer argument.
That argument would have the form FROM->TO.  After the arg is read,
the interactive spec would split it apart at the -> to get the FROM
and TO strings, and they would become the arguments in the call
to query-replace.

Further, the default for this one argument would be the previous
argument to query-replace.  The default would be used if you type RET
and available to edit with M-n.

This is radical, perhaps too radical, but it is clean and coherent.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-06  9:56         ` query-replace-interactive Juri Linkov
  2004-07-06 22:00           ` query-replace-interactive Richard Stallman
@ 2004-07-06 22:00           ` Richard Stallman
  2004-07-06 22:11             ` query-replace-interactive David Kastrup
  1 sibling, 1 reply; 21+ messages in thread
From: Richard Stallman @ 2004-07-06 22:00 UTC (permalink / raw)
  Cc: monnier, emacs-devel

    > For what it's worth, I can also do
    >
    >    M-% C-w C-w C-w RET
    >
    > since I've added a C-w binding in minibuffer-local-map that mimics
    > isearch's C-w.

    Why not add something like this to Emacs?  Or maybe more general
    character-based command like typing C-f in the end of the minibuffer
    to pull text from the source buffer character by character to the minibuffer.

No, no, no!  Let's not have more special casing.  People should be
able to rely on standard Emacs commands to do the standard thing.

You can switch windows from the minibuffer to copy text into it.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-06 22:00           ` query-replace-interactive Richard Stallman
@ 2004-07-06 22:11             ` David Kastrup
  0 siblings, 0 replies; 21+ messages in thread
From: David Kastrup @ 2004-07-06 22:11 UTC (permalink / raw)
  Cc: Juri Linkov, monnier, emacs-devel

Richard Stallman <rms@gnu.org> writes:

>     > For what it's worth, I can also do
>     >
>     >    M-% C-w C-w C-w RET
>     >
>     > since I've added a C-w binding in minibuffer-local-map that mimics
>     > isearch's C-w.
> 
>     Why not add something like this to Emacs?  Or maybe more general
>     character-based command like typing C-f in the end of the
>     minibuffer to pull text from the source buffer character by
>     character to the minibuffer.
> 
> No, no, no!  Let's not have more special casing.  People should be
> able to rely on standard Emacs commands to do the standard thing.

Which is to beep and flash at least with regard to C-f at the end of
line in the minibuffer.  That is not particularly useful.  However, I
doubt that people will be overenthusiastic if using C-f and autorepeat
will not just get them to the end of the line, but yank random stuff
while fouling up window-point of the buffer in question.

> You can switch windows from the minibuffer to copy text into it.

And you can copy it before starting your minibuffer command.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-06 22:00           ` query-replace-interactive Richard Stallman
@ 2004-07-07  5:11             ` Juri Linkov
  2004-07-07  5:42               ` query-replace-interactive Miles Bader
                                 ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Juri Linkov @ 2004-07-07  5:11 UTC (permalink / raw)
  Cc: monnier, emacs-devel

Richard Stallman <rms@gnu.org> writes:
> I misunderstood the idea of "(default a->b)".  Now that I see what
> it really does, I agree with you that it has lots of problems.
>
> Here's what I THOUGHT the idea was.
>
> The idea is that M-x query-replace would read just one minibuffer argument.
> That argument would have the form FROM->TO.  After the arg is read,
> the interactive spec would split it apart at the -> to get the FROM
> and TO strings, and they would become the arguments in the call
> to query-replace.
>
> Further, the default for this one argument would be the previous
> argument to query-replace.  The default would be used if you type RET
> and available to edit with M-n.
>
> This is radical, perhaps too radical, but it is clean and coherent.

The original Stefan's idea was to display "(default a->b)" in the
prompt for the from-string and to interpret empty input of the
from-string as a request for repeating the last replacement.

I extended this idea to what you described above, i.e. splitting the
from-string argument in the form "FROM -> TO".  This would be more
consistent with standard Emacs handling of default values, but it has
one drawback: the separator should not appear in the text to replace.
But if the string " -> " is rare enough to appear in replacement strings
we could use it as a default separator and to allow to quote it (e.g.
" \-\> ") if someone really wants to use it in the from-string.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-07  5:11             ` query-replace-interactive Juri Linkov
@ 2004-07-07  5:42               ` Miles Bader
  2004-07-07  9:35               ` query-replace-interactive David Kastrup
  2004-07-07 20:58               ` query-replace-interactive Richard Stallman
  2 siblings, 0 replies; 21+ messages in thread
From: Miles Bader @ 2004-07-07  5:42 UTC (permalink / raw)
  Cc: emacs-devel, rms, monnier

Juri Linkov <juri@jurta.org> writes:
> I extended this idea to what you described above, i.e. splitting the
> from-string argument in the form "FROM -> TO".  This would be more
> consistent with standard Emacs handling of default values, but it has
> one drawback: the separator should not appear in the text to replace.
> But if the string " -> " is rare enough to appear in replacement strings

I think that it could be extremely confusing for the user when problems
_did_ crop up (which they will).

Another problem is that what may be convenient for small strings, might
actually be pretty annoying for large ones -- I often use replace on
very large strings, e.g., multi-line C expressions [and why not, it
works fine], and finding and editing around (without accidentally
deleting necessary whitespace, etc.) a little 2-4 character string in
the midst of all that could be quite bad.

Perhaps a more robust method would be to use some sort of "separator"
string like you suggest, but make it not a simple string, e.g., how
about:

   (defvar query-replace-separator
      (propertize "\nwith: "
                  'read-only t
                  'field 'prompt
                  'face 'minibuffer-prompt
                  'rear-nonsticky t))
   (read-string "Query replace: " query-replace-separator)

[That won't quite work, but...]

That'd make the initial query-replace prompt look something like:

   Query replace: 
   with: 

and allow the user to freely edit both fields at once, with M-p and M-n
operating on both!

Yes, slightly :-), but it would be a very nice interface.

-Miles
-- 
Fast, small, soon; pick any 2.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-06 12:09         ` query-replace-interactive David Kastrup
@ 2004-07-07  6:41           ` Richard Stallman
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Stallman @ 2004-07-07  6:41 UTC (permalink / raw)
  Cc: juri, monnier, emacs-devel

    Basically because they are actually more like a user-friendly frontend
    to query-replace-regexp-eval functionality than a command of their
    own.  Since arbitrary code can be executed by `\,', existing uses of
    all replacement commands in Lisp programs would have different
    implications for security.  I did not want to introduce security
    relevant changes in the middle of the release consolidation.

I see.  That is a good reason in the short run.

However, in the long run it would be better to make perform-replace
itself do these things, even though it means checking the callers.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-06 12:24         ` query-replace-interactive Stefan
@ 2004-07-07  6:41           ` Richard Stallman
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Stallman @ 2004-07-07  6:41 UTC (permalink / raw)
  Cc: juri, emacs-devel

    But then

	C-s foo M-% bar RET

    becomes

	C-s foo M-% M-n RET bar RET

That is no disaster.

    and you lose the fact that M-% obeys the regexp/delimited/case-fold
    attributes of the isearch.

That is more of a complication.  So I guess we should
continue with M-% special in the search.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-07  5:11             ` query-replace-interactive Juri Linkov
  2004-07-07  5:42               ` query-replace-interactive Miles Bader
@ 2004-07-07  9:35               ` David Kastrup
  2004-07-07 20:58               ` query-replace-interactive Richard Stallman
  2 siblings, 0 replies; 21+ messages in thread
From: David Kastrup @ 2004-07-07  9:35 UTC (permalink / raw)
  Cc: emacs-devel, rms, monnier

Juri Linkov <juri@jurta.org> writes:

> Richard Stallman <rms@gnu.org> writes:
> > I misunderstood the idea of "(default a->b)".  Now that I see what
> > it really does, I agree with you that it has lots of problems.
> >
> > Here's what I THOUGHT the idea was.
> >
> > The idea is that M-x query-replace would read just one minibuffer argument.
> > That argument would have the form FROM->TO.  After the arg is read,
> > the interactive spec would split it apart at the -> to get the FROM
> > and TO strings, and they would become the arguments in the call
> > to query-replace.
> >
> > Further, the default for this one argument would be the previous
> > argument to query-replace.  The default would be used if you type RET
> > and available to edit with M-n.
> >
> > This is radical, perhaps too radical, but it is clean and coherent.
> 
> The original Stefan's idea was to display "(default a->b)" in the
> prompt for the from-string and to interpret empty input of the
> from-string as a request for repeating the last replacement.
> 
> I extended this idea to what you described above, i.e. splitting the
> from-string argument in the form "FROM -> TO".  This would be more
> consistent with standard Emacs handling of default values, but it has
> one drawback: the separator should not appear in the text to replace.
> But if the string " -> " is rare enough to appear in replacement strings
> we could use it as a default separator and to allow to quote it (e.g.
> " \-\> ") if someone really wants to use it in the from-string.

You can't quote within a non-regexp replacement.  One could place the
"->" as an empty overlay in between and check its position after the
entry has been done, that way you would not be able to edit it away
and it would not be confused with -> in the text.

But this is all academic for now.  It does not belong in 21.4.  And I
also don't like getting possibly large and unrelated old replacements
displayed in the prompt unless I explicitly ask for them.  That's
what the history is for.

And there is no need for repeating a search after editing operations,
anyway, since one can then just use the "edit" keybinding in
query-replace-map for the editing.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-07-07  5:11             ` query-replace-interactive Juri Linkov
  2004-07-07  5:42               ` query-replace-interactive Miles Bader
  2004-07-07  9:35               ` query-replace-interactive David Kastrup
@ 2004-07-07 20:58               ` Richard Stallman
  2 siblings, 0 replies; 21+ messages in thread
From: Richard Stallman @ 2004-07-07 20:58 UTC (permalink / raw)
  Cc: monnier, emacs-devel

    I extended this idea to what you described above, i.e. splitting the
    from-string argument in the form "FROM -> TO".  This would be more
    consistent with standard Emacs handling of default values, but it has
    one drawback: the separator should not appear in the text to replace.

You mean "cannot appear", I think.

Yes, but we could have some escape mechanism to handle that case.

For instance, if you enter just the separator, so that the from and to
strings would be empty, it could call the minibuffer twice (the same
thing it does now) to read the from and to strings.

    Perhaps a more robust method would be to use some sort of "separator"
    string like you suggest, but make it not a simple string, e.g., how
    about:

       (defvar query-replace-separator
	  (propertize "\nwith: "
		      'read-only t
		      'field 'prompt
		      'face 'minibuffer-prompt
		      'rear-nonsticky t))

That is an interesting idea.  Would you like to try it?

One case where this could cause some annoyance is that it
may get in the way of certain kinds of editing.  We'd have to see.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* query-replace-interactive
@ 2004-10-11  1:10 Luc Teirlinck
  2004-10-11  1:18 ` query-replace-interactive Luc Teirlinck
  0 siblings, 1 reply; 21+ messages in thread
From: Luc Teirlinck @ 2004-10-11  1:10 UTC (permalink / raw)


A while ago, I converted `query-replace-interactive' into a defcustom,
after it was decided to do so after discussion on emacs-devel.  I just
noticed that this change was reverted in version 1.181.  The Change
Log does not mention the reversion.  It only says that the `initial'
special value was removed.  Therefore, I assume that the reversion was
accidental.

Unless there are objections, I will make `query-replace-interactive'
into a defcustom again, since we agreed on that.

Sincerely,

Luc.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: query-replace-interactive
  2004-10-11  1:10 query-replace-interactive Luc Teirlinck
@ 2004-10-11  1:18 ` Luc Teirlinck
  0 siblings, 0 replies; 21+ messages in thread
From: Luc Teirlinck @ 2004-10-11  1:18 UTC (permalink / raw)
  Cc: emacs-devel

>From my earlier message:

   Unless there are objections, I will make `query-replace-interactive'
   into a defcustom again, since we agreed on that.

I now realize, after reading an old message from Juri on the subject,
that it was intentional, so I will _not_ revert it.  I just thought it
was accidental because of the way things were formulated in the Change Log.

Sincerely,

Luc.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2004-10-11  1:18 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-07-03 22:59 query-replace-interactive Stefan
2004-07-04  9:54 ` query-replace-interactive Juri Linkov
2004-07-04 16:13   ` query-replace-interactive Stefan
2004-07-05  6:08     ` query-replace-interactive Juri Linkov
2004-07-05 12:44       ` query-replace-interactive Stefan
2004-07-06  9:56         ` query-replace-interactive Juri Linkov
2004-07-06 22:00           ` query-replace-interactive Richard Stallman
2004-07-07  5:11             ` query-replace-interactive Juri Linkov
2004-07-07  5:42               ` query-replace-interactive Miles Bader
2004-07-07  9:35               ` query-replace-interactive David Kastrup
2004-07-07 20:58               ` query-replace-interactive Richard Stallman
2004-07-06 22:00           ` query-replace-interactive Richard Stallman
2004-07-06 22:11             ` query-replace-interactive David Kastrup
2004-07-06 11:53       ` query-replace-interactive Richard Stallman
2004-07-06 12:24         ` query-replace-interactive Stefan
2004-07-07  6:41           ` query-replace-interactive Richard Stallman
2004-07-06 11:53       ` query-replace-interactive Richard Stallman
2004-07-06 12:09         ` query-replace-interactive David Kastrup
2004-07-07  6:41           ` query-replace-interactive Richard Stallman
  -- strict thread matches above, loose matches on Subject: below --
2004-10-11  1:10 query-replace-interactive Luc Teirlinck
2004-10-11  1:18 ` query-replace-interactive Luc Teirlinck

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).