History for query replace pairs

History for query replace pairs

[Tom]

query-replace has a nice feature offering the previous
search/replacement pair as a default, to quickly repeat
the previous search/replace. I find it quite useful.

Often it would also be useful to quickly retrieve
other previous search/replacement pairs. I know I can
select them separately by selecting first the search string 
from the history and then the replacement, but I find
that in most cases I want to use the same replacement which
I used previously for that search.

So a search/replacement pairs history would be quite useful,
it may even be more useful (more frequently used) and 
more convenient than the current history if it were available.

The question is: how could it be integrated into the current
query-replace interface, that is how could the user choose
from a history of defaults (if he does not want to choose the 
latest default which he can access with RET).

RE: History for query replace pairs

[Drew Adams]

> query-replace has a nice feature offering the previous
> search/replacement pair as a default, to quickly repeat
> the previous search/replace. I find it quite useful.
> 
> Often it would also be useful to quickly retrieve
> other previous search/replacement pairs. I know I can
> select them separately by selecting first the search string
> from the history and then the replacement, but I find
> that in most cases I want to use the same replacement which
> I used previously for that search.
> 
> So a search/replacement pairs history would be quite useful,
> it may even be more useful (more frequently used) and
> more convenient than the current history if it were available.
> 
> The question is: how could it be integrated into the current
> query-replace interface, that is how could the user choose
> from a history of defaults (if he does not want to choose the
> latest default which he can access with RET).

You can access the history of those pairs using `C-x ESC ESC'
(`repeat-complex-command').  But that is the history of all
complex commands, so you might to cycle to the pair you want,
or fish it out using, say, `M-r'.

Re: History for query replace pairs

[Juri Linkov]

> The question is: how could it be integrated into the current
> query-replace interface, that is how could the user choose
> from a history of defaults (if he does not want to choose the
> latest default which he can access with RET).

Since the default values are displayed in a special format
using an arrow

  Query replace (default from1 -> to1): 

a history navigation key should bring the pair into the minibuffer
in the same format like

  Query replace (default from1 -> to1): from2 -> to2

Then the user has to be careful to not remove the arrow
while editing the inserted values in the minibuffer.

Another problem is how to parse such format
when a from/to value contains the arrow "->"

  Query replace (default from1 -> to1): from2 -> from2 -> to2 -> to2

RE: History for query replace pairs

[Herring, Davis]

> Then the user has to be careful to not remove the arrow
> while editing the inserted values in the minibuffer.
> 
> Another problem is how to parse such format
> when a from/to value contains the arrow "->"
> 
>   Query replace (default from1 -> to1): from2 -> from2 -> to2 -> to2

This part, at least, is easy: give the -> a text property to make it recognizable (and, while you're at it, a display property so that it's really one character if the user does decide to delete it).

Davis

Re: History for query replace pairs

[Josh]

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

On Fri, Aug 8, 2014 at 4:38 PM, Juri Linkov <juri@jurta.org> wrote:

> > The question is: how could it be integrated into the current
> > query-replace interface, that is how could the user choose
> > from a history of defaults (if he does not want to choose the
> > latest default which he can access with RET).
>
> Since the default values are displayed in a special format
> using an arrow
>
>   Query replace (default from1 -> to1):
>
> a history navigation key should bring the pair into the minibuffer
> in the same format like
>
>   Query replace (default from1 -> to1): from2 -> to2
>

I've often wished for the sort of pair-wise history Tom described
myself, and thought a bit about possible UIs.  So far, the idea I
find most appealing is a history ring whose elements are
single-line yasnippet-style snippets with `from' and `to' fields
that defaulted to the values for that history element.  So after
the first M-p the minibuffer could look something like
  Query replace (TAB for next field, RET to accept): [|from1] -> [to1]
with the point at the start of the from1 field.  Entering any text
would replace from1, while hitting TAB would advance the point
to the to1 field, which would behave similarly.  Entering RET
at any time would commit the current from and to values in
whatever state they were in.  Subsequent M-p's would cycle
through previous pairs as you would expect.  Hitting RET
immediately after any M-p would accept the current default
pair and replacing or editing one or both fields would be
convenient as well.

I'm afraid I have no clue about how to go about implementing
such behavior though :)

Josh

[-- Attachment #2: Type: text/html, Size: 2259 bytes --]

Re: History for query replace pairs

[Tom]

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

On Sun, Aug 10, 2014 at 3:18 AM, Josh <josh@foxtail.org> wrote:

>
>> So far, the idea I
> find most appealing is a history ring whose elements are
> single-line yasnippet-style snippets with `from' and `to' fields
> that defaulted to the values for that history element.
>

Yes, that would be the full realization of the idea.
Being able to select from FROM -> TO history pairs and quickly edit
them in-place if  needed.

It could be much more convenient and efficient in everyday practice
than the current history interface, though it should be implemented
parallel to that, so the current interface is also awailable if needed.

So the current FROM input field should provide access to  previous
FROM -> TO pairs and handle the case when such a pair is
inputted instead of a simple FROM string.

[-- Attachment #2: Type: text/html, Size: 1532 bytes --]

Re: History for query replace pairs

[Tom]

[-- Attachment #1.1: Type: text/plain, Size: 1072 bytes --]

On Sun, Aug 10, 2014 at 7:59 AM, Tom <adatgyujto@gmail.com> wrote:

>
>
> Yes, that would be the full realization of the idea.
> Being able to select from FROM -> TO history pairs and quickly edit
> them in-place if  needed.
>
> So the current FROM input field should provide access to  previous
> FROM -> TO pairs and handle the case when such a pair is
> inputted instead of a simple FROM string.
>
>
I created a quick proof of concept implementation. It changes the behavior
of the arrow keys for the query-replace(-regexp) FROM prompt. You can stll
access the original behavior with M-p/n.

So when you start a query-replace and press the up arrow then
search/replace history is rendered as FROM -> TO pairs.  The -> marker is
highlighted and intangible, the cursor jumps over it and you are free to
edit the search/replace string. If you submit a FROM->TO pair from the FROM
input field with RET then the replacement is started immediately. If you
submit a simple string (without the marker) then it behaves as usual and
asks for a TO string.

See the attached file.

[-- Attachment #1.2: Type: text/html, Size: 1798 bytes --]

[-- Attachment #2: hist.el --]
[-- Type: application/octet-stream, Size: 3620 bytes --]

(defvar histpairs-history nil)

(defvar histpairs-history-current nil)


(defun histpairs-query-replace-p ()
  (string-match "Query replace\\( regexp\\)?\\( (\\|:\\)" (minibuffer-prompt)))


(defun histpairs-update-history-if-necessary ()
  (unless (eq (cdr histpairs-history) command-history)
    (setq histpairs-history
          (cons 
           (delete-if 'null 
                      (mapcar
                       (lambda (item)
                         (if (or (eq (car item) 'query-replace)
                                 (eq (car item) 'query-replace-regexp))
                             (cons (second item) (third item))))
                       command-history))
           command-history))))


(defun histpairs-update-minibuffer ()
  (delete-minibuffer-contents)
  (insert (caar histpairs-history-current)
          (propertize " -> " 'face 'highlight 
                      'intangible t 'rear-nonsticky t)

          (if (stringp (cdar histpairs-history-current))
              (cdar histpairs-history-current)
            (concat "can't handle this history element yet, "
                    "probably a regexp lisp expression"))))
  

(defun histpairs-previous-history-element (n)
  (interactive "p")
  (if (not (histpairs-query-replace-p))
      (previous-history-element n)

    (histpairs-update-history-if-necessary)

    (if (not histpairs-history-current)
        (setq histpairs-history-current (car histpairs-history))

      (setq histpairs-history-current (cdr histpairs-history-current)))
      

    (if (not histpairs-history-current)
        (message "No previous element.")

      (histpairs-update-minibuffer))))


(defun histpairs-next-history-element (n)
  (interactive "p")
  (if (not (histpairs-query-replace-p))
      (next-history-element n)

    (histpairs-update-history-if-necessary)

    (if histpairs-history-current
        (let ((items (car histpairs-history)))
          (while (and items
                      (not (eq (cdr items) histpairs-history-current)))
            (setq items (cdr items)))
          (setq histpairs-history-current items)))

    (if (not histpairs-history-current)
        (message "No next element.")

      (histpairs-update-minibuffer))))


(defun histpairs-finish ()
  (interactive)
  (if (not (histpairs-query-replace-p))
      (exit-minibuffer)

    (setq histpairs-history-current nil)

    (let ((pos (next-single-property-change 0 'intangible
                                            (minibuffer-contents))))
      (if (not pos)
          (exit-minibuffer)  ;; normal execution
        
        (if (get-text-property 0 'intangible (minibuffer-contents))
            (message "FROM is empty.")

          (let* ((end (next-single-property-change pos 'intangible
                                                   (minibuffer-contents)))
                 (from (substring (minibuffer-contents) 0 pos))
                 (to (if end
                         (substring (minibuffer-contents) end)
                       "")))
            (delete-minibuffer-contents)
            (insert from)

            (push 'return unread-command-events)
            (dolist (c (nreverse (string-to-list to)))
              (push c unread-command-events))

            (exit-minibuffer)))))))



(define-key minibuffer-local-map
  (kbd "<up>") 'histpairs-previous-history-element)
(define-key minibuffer-local-map
  (kbd "<down>") 'histpairs-next-history-element)
(define-key minibuffer-local-map
  (kbd "<return>") 'histpairs-finish)

Re: History for query replace pairs

[Ted Zlatanov]

On Mon, 11 Aug 2014 20:34:33 +0200 Tom <adatgyujto@gmail.com> wrote: 

T> I created a quick proof of concept implementation. It changes the behavior
T> of the arrow keys for the query-replace(-regexp) FROM prompt. You can stll
T> access the original behavior with M-p/n.

T> So when you start a query-replace and press the up arrow then
T> search/replace history is rendered as FROM -> TO pairs.  The -> marker is
T> highlighted and intangible, the cursor jumps over it and you are free to
T> edit the search/replace string. If you submit a FROM->TO pair from the FROM
T> input field with RET then the replacement is started immediately. If you
T> submit a simple string (without the marker) then it behaves as usual and
T> asks for a TO string.

I'd like to make this part of the Emacs core, disabled by default. Any
objections?  I think it's nicer-looking than the current UI.

If not, maybe Tom would like to make it a GNU ELPA package?

Ted

Re: History for query replace pairs

[Juri Linkov]

> T> I created a quick proof of concept implementation. It changes the behavior
> T> of the arrow keys for the query-replace(-regexp) FROM prompt. You can stll
> T> access the original behavior with M-p/n.
>
> T> So when you start a query-replace and press the up arrow then
> T> search/replace history is rendered as FROM -> TO pairs.  The -> marker is
> T> highlighted and intangible, the cursor jumps over it and you are free to
> T> edit the search/replace string. If you submit a FROM->TO pair from the FROM
> T> input field with RET then the replacement is started immediately. If you
> T> submit a simple string (without the marker) then it behaves as usual and
> T> asks for a TO string.
>
> I'd like to make this part of the Emacs core, disabled by default. Any
> objections?  I think it's nicer-looking than the current UI.
>
> If not, maybe Tom would like to make it a GNU ELPA package?

I realized right now that the most intuitive and convenient way is
to implement a new general function to read two values in the minibuffer
with two entry fields.  These fields could be like in Widget/Customization UI
where the user can switch the entry point between two fields.
Anything between them would be read-only like the current prompt is, e.g.

  Query replace: [from] with: [to]

where [from] and [to] are entry fields.  `M-p' will update these two fields
with the previous values from the history.

Re: History for query replace pairs

[Stefan Monnier]

T> So when you start a query-replace and press the up arrow then
T> search/replace history is rendered as FROM -> TO pairs.  The -> marker is
T> highlighted and intangible, the cursor jumps over it and you are free to
T> edit the search/replace string.  If you submit a FROM->TO pair from the FROM
T> input field with RET then the replacement is started immediately.  If you
T> submit a simple string (without the marker) then it behaves as usual and
T> asks for a TO string.
> I'd like to make this part of the Emacs core, disabled by default.

I haven't found the time to look at the code, but the functionality
described sounds good (a good way to provide a new feature while
preserving the old behavior at the same time).

So if someone can check that the code (and copyright) is clean, I'm OK
with installing it into trunk.


        Stefan

Re: History for query replace pairs

[Yuri Khan]

On Sun, Oct 5, 2014 at 6:36 AM, Juri Linkov <juri@jurta.org> wrote:

> I realized right now that the most intuitive and convenient way is
> to implement a new general function to read two values in the minibuffer
> with two entry fields.

Where there is two, there is +infinity.

In other words, why stop at two? Let’s have a general function that
accepts a description of zero or more parameters (I don’t know, call
it a “dialog template” or “interactive specification”) and allows the
user to enter all of them using Customize-like widgets in the
minibuffer. (Though at this point it probably makes sense to maintain
both composite histories for each dialog and separate histories for
each field.)

Re: History for query replace pairs

[Tom]

Ted Zlatanov <tzz <at> lifelogs.com> writes:
> 
> I'd like to make this part of the Emacs core, disabled by default. Any
> objections?  I think it's nicer-looking than the current UI.
> 
> If not, maybe Tom would like to make it a GNU ELPA package?
> 

If you put it into the core then it won't be available in this
release for users (I guess, because it's new functionality), only
in the next one which may be years away.

For this reason it may be better if you make an elpa (or melpa)
package, because then every user can use it immediately, it can
be polished there and it can be moved into the core later when
it's matured.

Re: History for query replace pairs

[Andreas Schwab]

Juri Linkov <juri@jurta.org> writes:

> Anything between them would be read-only like the current prompt is, e.g.
>
>   Query replace: [from] with: [to]
>
> where [from] and [to] are entry fields.  `M-p' will update these two fields
> with the previous values from the history.

If you are in [to] you definitely don't want M-p to update both fields.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: History for query replace pairs

[Juri Linkov]

>> I realized right now that the most intuitive and convenient way is
>> to implement a new general function to read two values in the minibuffer
>> with two entry fields.
>
> Where there is two, there is +infinity.

Or more accurately: Where there is one, there is +infinity :)

In case of the minibuffer, this means that we already have
one entry field in the minibuffer, so it makes sense to extend it
to multiple entry fields.

> In other words, why stop at two? Let’s have a general function that
> accepts a description of zero or more parameters (I don’t know, call
> it a “dialog template” or “interactive specification”) and allows the
> user to enter all of them using Customize-like widgets in the
> minibuffer. (Though at this point it probably makes sense to maintain
> both composite histories for each dialog and separate histories for
> each field.)

There are some problems with reading multiple values in the
minibuffer like proposed by Josh.  One problem is what a key to use
to switch to another input field? In Customize-like widgets this is
accomplished by TAB, but in the minibuffer TAB performs completion
(not in query-replace but in completing-read generally).
Customize provides another key M-TAB (M-C-i) for completion.
We could use the same key for multi-value completing-read.

Re: History for query replace pairs

[Ted Zlatanov]

On Sun, 5 Oct 2014 05:59:34 +0000 (UTC) Tom <adatgyujto@gmail.com> wrote: 

T> Ted Zlatanov <tzz <at> lifelogs.com> writes:
>> 
>> I'd like to make this part of the Emacs core, disabled by default. Any
>> objections?  I think it's nicer-looking than the current UI.
>> 
>> If not, maybe Tom would like to make it a GNU ELPA package?

T> If you put it into the core then it won't be available in this
T> release for users (I guess, because it's new functionality), only
T> in the next one which may be years away.

T> For this reason it may be better if you make an elpa (or melpa)
T> package, because then every user can use it immediately, it can
T> be polished there and it can be moved into the core later when
T> it's matured.

That's true for code that adds functionality, but IMHO code that
improves usability (such as yours) should go into the core for two
reasons: one, it should be usable by everyone; two, it can take
advantage of core improvements without keeping backward compatibility.

This is just my opinion, though.  It's your code and you're free to
submit it to the GNU ELPA or any other archive :)  Just let us know what
you prefer soon so we can move on with it.

On Sun, 05 Oct 2014 02:36:57 +0300 Juri Linkov <juri@jurta.org> wrote: 

JL> I realized right now that the most intuitive and convenient way is
JL> to implement a new general function to read two values in the minibuffer
JL> with two entry fields.  These fields could be like in Widget/Customization UI
JL> where the user can switch the entry point between two fields.
JL> Anything between them would be read-only like the current prompt is, e.g.

JL>   Query replace: [from] with: [to]

JL> where [from] and [to] are entry fields.  `M-p' will update these two fields
JL> with the previous values from the history.

That sounds fine, especially if it can be implemented quickly. Otherwise
maybe we should adopt Tom's code now and improve it later. What do you
think? I'll hold off until you and Tom have given your opinions.

Either way, I am excited about this. It will definitely improve the UI
experience!

Thanks
Ted

Re: History for query replace pairs

[Tom]

Ted Zlatanov <tzz <at> lifelogs.com> writes:
> 
> This is just my opinion, though.  It's your code and you're free to
> submit it to the GNU ELPA or any other archive :)  Just let us know what
> you prefer soon so we can move on with it.

I don't really plan to do anything more with it. It was an idea
and I implemented just a quick proof of concept to test the idea, so
this code served its purpose.

The important part is the concept and I think in practice it's better
than the current replace history interface.

It should be reimplemented properly. I created it in a quick and dirty
way using shortcuts (e.g it matches the prompt with a regexp to test
if it's in query-replace which surely is not the proper way).

The code is quite short, so redoing it properly should not be hard.
(And it must be done anyway if you want to put it into the core
or ELPA, because I haven't signed copyright stuff.)

I don't plan to do it, so it's up for grabs.

Re: History for query replace pairs

[Juri Linkov]

> That sounds fine, especially if it can be implemented quickly. Otherwise
> maybe we should adopt Tom's code now and improve it later. What do you
> think? I'll hold off until you and Tom have given your opinions.
>
> Either way, I am excited about this. It will definitely improve the UI
> experience!

Since it's important to keep backward compatibility we have
not much options.  Presenting the two-field minibuffer initially
or after typing M-p might break some existing code that relies
on two functions query-replace-read-from and query-replace-read-to
used to read FROM and TO args separately.

This leaves us with the remaining option to keep current UI
mostly unchanged but improve it to make it easier to repeat
previous replacements.  It's possible to do this with just
two RET keystrokes, e.g. `M-% M-p M-p M-p ... RET RET'
where any number of M-p will browse FROM parts (paired with the
TO part), after the first RET query-replace-read-to will be called
where the TO part (corresponding to the selected previous FROM)
will be inserted into the minibuffer as initial input.
Then second RET will accept it by default.

So the first RET will actually do almost the same as TAB would do
in the two-field minibuffer to switch to editing the TO part.

Re: History for query replace pairs

[Juri Linkov]

> So the first RET will actually do almost the same as TAB would do
> in the two-field minibuffer to switch to editing the TO part.

This is the minimal patch to implement this.  Opinions?

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2014-08-25 02:36:45 +0000
+++ lisp/replace.el	2014-10-07 22:07:18 +0000
@@ -56,8 +56,8 @@ (defvar query-replace-history nil
 
 (defvar query-replace-defaults nil
   "Default values of FROM-STRING and TO-STRING for `query-replace'.
-This is a cons cell (FROM-STRING . TO-STRING), or nil if there is
-no default value.")
+This is a list of cons cells (FROM-STRING . TO-STRING),
+or nil if there is no default values.")
 
 (defvar query-replace-interactive nil
   "Non-nil means `query-replace' uses the last search string.
@@ -135,8 +135,8 @@ (defun query-replace-read-from (prompt r
 	   (prompt
 	    (if query-replace-defaults
 		(format "%s (default %s -> %s): " prompt
-			(query-replace-descr (car query-replace-defaults))
-			(query-replace-descr (cdr query-replace-defaults)))
+			(query-replace-descr (car (car query-replace-defaults)))
+			(query-replace-descr (cdr (car query-replace-defaults))))
 	      (format "%s: " prompt)))
 	   (from
 	    ;; The save-excursion here is in case the user marks and copies
@@ -149,9 +149,9 @@ (defun query-replace-read-from (prompt r
 		 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)
+	  (cons (car (car query-replace-defaults))
 		(query-replace-compile-replacement
-		 (cdr query-replace-defaults) regexp-flag))
+		 (cdr (car query-replace-defaults)) regexp-flag))
 	(add-to-history query-replace-from-history-variable from nil t)
 	;; Warn if user types \n or \t, but don't reject the input.
 	(and regexp-flag
@@ -213,10 +213,10 @@ (defun query-replace-read-to (from promp
      (let* ((history-add-new-input nil)
 	    (to (read-from-minibuffer
 		 (format "%s %s with: " prompt (query-replace-descr from))
-		 nil nil nil
+		 (cdr (assoc from query-replace-defaults)) nil nil
 		 query-replace-to-history-variable from t)))
        (add-to-history query-replace-to-history-variable to nil t)
-       (setq query-replace-defaults (cons from to))
+       (push (cons from to) query-replace-defaults)
        to))
    regexp-flag))

Re: History for query replace pairs

[Stefan Monnier]

>> So the first RET will actually do almost the same as TAB would do
>> in the two-field minibuffer to switch to editing the TO part.
> This is the minimal patch to implement this.  Opinions?

I think this is "too minimal".  I much prefer the UI where you get both
FROM and TO at the same time in the minibuffer.


        Stefan

Re: History for query replace pairs

[Juri Linkov]

>>> So the first RET will actually do almost the same as TAB would do
>>> in the two-field minibuffer to switch to editing the TO part.
>> This is the minimal patch to implement this.  Opinions?
>
> I think this is "too minimal".  I much prefer the UI where you get both
> FROM and TO at the same time in the minibuffer.

There are two ways to get both FROM and TO in the minibuffer:

1. On typing some key in the FROM reading minibuffer, insert the second
   field TO with a read-only separator between FROM and TO.

2. Display both FROM and TO initially.  Then to use RET or TAB to switch
   from FROM to TO, and S-TAB to switch back from TO to FROM.

But the main question in both cases which key to use to browse FROM-TO pairs
from the minibuffer's history?  It seems both M-p and <up> should still get
either FROM or TO from the history one by one, so it would be easy to reuse
a previous TO for the FROM part (to invert replacements).  What about <M-up>?

Re: History for query replace pairs

[Alan Mackenzie]

Hello, Juri.

On Tue, Oct 14, 2014 at 10:02:52PM +0300, Juri Linkov wrote:
> >>> So the first RET will actually do almost the same as TAB would do
> >>> in the two-field minibuffer to switch to editing the TO part.
> >> This is the minimal patch to implement this.  Opinions?

> > I think this is "too minimal".  I much prefer the UI where you get both
> > FROM and TO at the same time in the minibuffer.

> There are two ways to get both FROM and TO in the minibuffer:

> 1. On typing some key in the FROM reading minibuffer, insert the second
>    field TO with a read-only separator between FROM and TO.

> 2. Display both FROM and TO initially.  Then to use RET or TAB to switch
>    from FROM to TO, and S-TAB to switch back from TO to FROM.

How about, simply, TAB to switch to the other field?  S-TAB doesn't exist
on a tty.  This would create a slight inconsistency with C-s, where TAB
is a self-inserting character.

> But the main question in both cases which key to use to browse FROM-TO pairs
> from the minibuffer's history?  It seems both M-p and <up> should still get
> either FROM or TO from the history one by one, so it would be easy to reuse
> a previous TO for the FROM part (to invert replacements).  What about <M-up>?

M-up doesn't exist on a tty either.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: History for query replace pairs

[Juri Linkov]

> How about, simply, TAB to switch to the other field?  S-TAB doesn't exist
> on a tty.

I guess we could use the same key M-TAB or M-C-i as is used
to switch to the other field in the Customization UI on a tty.

> This would create a slight inconsistency with C-s, where TAB
> is a self-inserting character.

C-s uses M-TAB or M-C-i as well for completion on a tty.

>> But the main question in both cases which key to use to browse FROM-TO pairs
>> from the minibuffer's history?  It seems both M-p and <up> should still get
>> either FROM or TO from the history one by one, so it would be easy to reuse
>> a previous TO for the FROM part (to invert replacements).  What about <M-up>?
>
> M-up doesn't exist on a tty either.

Then what about `<ESC> <up>'?

Re: History for query replace pairs

[Andreas Schwab]

Alan Mackenzie <acm@muc.de> writes:

> M-up doesn't exist on a tty either.

xterm can do it, as does konsole.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: History for query replace pairs

[Alan Mackenzie]

On Tue, Oct 14, 2014 at 10:05:34PM +0200, Andreas Schwab wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > M-up doesn't exist on a tty either.

> xterm can do it, as does konsole.

A Linux virtual terminal (at least, with the factory keyboard layout)
cannot.

> Andreas.

> -- 
> Andreas Schwab, schwab@linux-m68k.org

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: History for query replace pairs

[Alan Mackenzie]

Hello, Juri.

On Tue, Oct 14, 2014 at 10:44:01PM +0300, Juri Linkov wrote:
> > How about, simply, TAB to switch to the other field?  S-TAB doesn't exist
> > on a tty.

> I guess we could use the same key M-TAB or M-C-i as is used
> to switch to the other field in the Customization UI on a tty.

> > This would create a slight inconsistency with C-s, where TAB
> > is a self-inserting character.

> C-s uses M-TAB or M-C-i as well for completion on a tty.

Yes.  M-TAB does exist on ttys.

> >> But the main question in both cases which key to use to browse
> >> FROM-TO pairs from the minibuffer's history?  It seems both M-p and
> >> <up> should still get either FROM or TO from the history one by one,
> >> so it would be easy to reuse a previous TO for the FROM part (to
> >> invert replacements).  What about <M-up>?

> > M-up doesn't exist on a tty either.

> Then what about `<ESC> <up>'?

Bad.  It's far too cumbersome.  It's two distinct keystrokes with the two
keys being at opposite sides of the keyboard, so far apart that they
cannot even be seen at the same time.

-- 
Alan Mackenzie (Nuremberg, Germany).

RE: History for query replace pairs

[Drew Adams]

FWIW, it sounds to me like you are complicating things for no
real gain.  When you are all done fiddling with this, count
the keystrokes needed for a user to take advantage of your
new dream behavior...

Keep it simple.

And just what is the problem that this is trying to solve?
Are you trying to find a way to give users easy access to
previous replacement pairs?  We already have that, no?

If you want to reuse a previous old/new replacement pair,
just use `C-x ESC ESC'.  You have `M-p' or `M-r' to choose
which previous pair to reuse.  And you can edit such a choice.

What's the problem?

---

With a reasonable extension to allow completion on demand
against the minibuffer history (with Icicles you can use `M-o'
to do that, for example), you could also complete-match against
past inputs.  (And yes, it should be on-demand - past inputs
as completion candidates should not just be mixed in with the
current set of completion candidates.)

It might be better to spend a little time to provide such a
past-inputs completion feature than to monkey around adding
keys to maneuver among past input pairs they way you seem to
be doing.  Just a suggestion.

Re: History for query replace pairs

[Andreas Schwab]

Alan Mackenzie <acm@muc.de> writes:

> On Tue, Oct 14, 2014 at 10:05:34PM +0200, Andreas Schwab wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > M-up doesn't exist on a tty either.
>
>> xterm can do it, as does konsole.
>
> A Linux virtual terminal (at least, with the factory keyboard layout)
> cannot.

Still, it exists.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: History for query replace pairs

[Juri Linkov]

> If you want to reuse a previous old/new replacement pair,
> just use `C-x ESC ESC'.  You have `M-p' or `M-r' to choose
> which previous pair to reuse.  And you can edit such a choice.
>
> What's the problem?

There are no previous replacement pairs in `C-x ESC ESC'
when M-% is invoked from isearch.

RE: History for query replace pairs

[Drew Adams]

> > If you want to reuse a previous old/new replacement pair,
> > just use `C-x ESC ESC'.  You have `M-p' or `M-r' to choose
> > which previous pair to reuse.  And you can edit such a choice.
> >
> > What's the problem?
> 
> There are no previous replacement pairs in `C-x ESC ESC'
> when M-% is invoked from isearch.

Really?  That's what this all is about - `M-%' from Isearch?

I just searched this 3-month-long thread for `M-%' and found
_one_ occurrence of `M-%', and that occurrence had nothing to
do with being invoked during Isearch.

And I searched the entire thread separately for "isearch",
and found _no_ occurrences of that.  Sure, it's still
possible that I missed some indirect mention of the problem
of invoking `M-%' from Isearch.

But it's truly hard to imagine that the discussion has been
focused for 3 months on finding a solution to the
`M-%'-in-isearch-default-pairs problem.

Why would a discussion of this wander all over the map for
3 months, and never once mention `M-%' during Isearch, if
that is in fact the only real use case that is problematic?

Why wait until I ask about `C-x ESC ESC' to focus on the
problem?  (I mentioned `C-x ESC ESC' the first day of this
thread, BTW, but no one responded.)

This is much ado about nothing, AFAICT.  Misplaced zeal,
perhaps.

If you want to find a way to pick up previous `M-%' pairs
for `M-%' when it is invoked from Isearch, then just do that.
Find a reasonable solution for Isearch, if it is an
Isearch-specific problem (as it seems to be).

If `C-x ESC ESC' is otherwise adequate (with `M-p' & `M-r')
for retrieving query-replace pairs, then maybe just use
`command-history', filtered for query-replace commands, to
supplement `M-%' during Isearch.

Re: History for query replace pairs

[Stefan Monnier]

> But the main question in both cases which key to use to browse FROM-TO pairs
> from the minibuffer's history?  It seems both M-p and <up> should still get
> either FROM or TO from the history one by one, so it would be easy to reuse
> a previous TO for the FROM part (to invert replacements).  What about <M-up>?

The only thing that makes sense is to get the pairs via M-p/M-n and
up/down.  Of course, that clashes with the current behavior and hence
prevents access to "TO-history" in the FROM case and to "FROM-history"
in the TO case.

But if we only do that for the FROM prompt (which is already a "FROM or
FROM/TO pair" because of the behavior in case you just hit RET, and
would turn even more clearly into a "FROM or FROM/TO pair"), I don't
think it's that bad:
- You still have access to the "TO-history" in the FROM case, indirectly
  (by browsing the corresponding FROM/TO pair and erasing the FROM and
  the separator).
- You still have access to the FROM-history in the TO case, just as before.


        Stefan

Re: History for query replace pairs

[Juri Linkov]

> - You still have access to the "TO-history" in the FROM case, indirectly
>   (by browsing the corresponding FROM/TO pair and erasing the FROM and
>   the separator).

The only problematic part is that if the user will be able
to erase the separator, then the user rightfully might want
to insert it back, but it's not straightforward to type a string
with text properties.

OTOH, it should be easy for the user to kill/yank the TO-string
to the FROM-field without erasing the separator that could be
marked as read-only exactly like the minibuffer prompt.

Perhaps using the Widget library is too heavy to implement this,
so it should be possible to have two fields in the minibuffer
by putting the properties `field' and `read-only' on the separator.

Re: History for query replace pairs

[Stefan Monnier]

>> - You still have access to the "TO-history" in the FROM case, indirectly
>> (by browsing the corresponding FROM/TO pair and erasing the FROM and
>> the separator).
> The only problematic part is that if the user will be able
> to erase the separator, then the user rightfully might want
> to insert it back, but it's not straightforward to type a string
> with text properties.

`undo' should do it.

> Perhaps using the Widget library is too heavy to implement this,
> so it should be possible to have two fields in the minibuffer
> by putting the properties `field' and `read-only' on the separator.

The widget library is definitely too heavy.  And I don't like the idea
of making it read-only either.  Just use a special character/string, and
give it a special appearance via text-properties (make sure the user
can't delete just a part of it, so a single char is best, probably with
a display property), so that the user can tell it's not just text and so
that Elisp can reliably recognize it as well.
E.g. (propertize "\0" 'display (propertize " -> " 'face 'my-face))


        Stefan

Re: History for query replace pairs

[Artur Malabarba]

>>> - You still have access to the "TO-history" in the FROM case, indirectly
>>> (by browsing the corresponding FROM/TO pair and erasing the FROM and
>>> the separator).
>> The only problematic part is that if the user will be able
>> to erase the separator, then the user rightfully might want
>> to insert it back, but it's not straightforward to type a string
>> with text properties.
>
> `undo' should do it.
>

If this line is followed, I'd recommend rebinding one of the transpose
commands (transpose-lines, perhaps) to swap the TO and FROM. Having to
manually kill and yank would be a very cumbersome way of reversing the
replacement.

Re: History for query replace pairs

[Juri Linkov]

> The widget library is definitely too heavy.  And I don't like the idea
> of making it read-only either.  Just use a special character/string, and
> give it a special appearance via text-properties (make sure the user
> can't delete just a part of it, so a single char is best, probably with
> a display property), so that the user can tell it's not just text and so
> that Elisp can reliably recognize it as well.
> E.g. (propertize "\0" 'display (propertize " -> " 'face 'my-face))

Currently there are two user variables `query-replace-from-history-variable'
and `query-replace-to-history-variable' by default both pointing
to the same history variable `query-replace-history'.  The simplest
solution would be to add another history variable named
`query-replace-from-to-history', so the users could easily customize
`query-replace-from-history-variable' either to the old value
`query-replace-history' to restore the current functionality,
or customize it to the new from-to pair history.

Below is a preliminary implementation.  However, there is a problem
that after saving it to the desktop or savehist, and restoring back
FROM-TO string pairs in the history lose text-properties.
If there is no way to save text properties then perhaps we should have
two history variables: one with cons pairs (FROM . TO) to save
in the desktop, and another with a list of strings "FROM<separator>TO"
to use in `read-from-minibuffer', and keep these two variables in sync.

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2014-08-25 02:36:45 +0000
+++ lisp/replace.el	2014-10-25 20:48:10 +0000
@@ -54,10 +54,11 @@ (defvar query-replace-history nil
 See `query-replace-from-history-variable' and
 `query-replace-to-history-variable'.")
 
-(defvar query-replace-defaults nil
-  "Default values of FROM-STRING and TO-STRING for `query-replace'.
-This is a cons cell (FROM-STRING . TO-STRING), or nil if there is
-no default value.")
+(defvar query-replace-from-to-history nil
+  "Default history list of FROM-TO strings for `query-replace'.
+This is a list of string pairs of FROM-STRING and TO-STRING
+separated by `query-replace-from-to-separator', or nil
+if there is no history.  See `query-replace-from-history-variable'.")
 
 (defvar query-replace-interactive nil
   "Non-nil means `query-replace' uses the last search string.
@@ -67,14 +68,24 @@ (make-obsolete-variable 'query-replace-i
 to the minibuffer that reads the string to replace, or invoke replacements
 from Isearch by using a key sequence like `C-s C-s M-%'." "24.3")
 
-(defcustom query-replace-from-history-variable 'query-replace-history
+(defvar query-replace-from-to-separator
+  (propertize
+   "\0" 'display (propertize
+		  (if (char-displayable-p ?\u2192) " \u2192 " " -> ")
+		  'face 'minibuffer-prompt))
+  "String that separates FROM and TO in `query-replace-from-to-history'.")
+
+(defcustom query-replace-from-history-variable 'query-replace-from-to-history
   "History list to use for the FROM argument of `query-replace' commands.
 The value of this variable should be a symbol; that symbol
 is used as a variable to hold a history list for the strings
 or patterns to be replaced."
   :group 'matching
-  :type 'symbol
-  :version "20.3")
+  :type '(choice
+	  (const :tag "History of FROM-TO pairs" query-replace-from-to-history)
+	  (const :tag "History of FROM strings" query-replace-history)
+	  (symbol :tag "Your choice of history list"))
+  :version "24.5")
 
 (defcustom query-replace-to-history-variable 'query-replace-history
   "History list to use for the TO argument of `query-replace' commands.
@@ -132,11 +143,10 @@ (defun query-replace-read-from (prompt r
   (if query-replace-interactive
       (car (if regexp-flag regexp-search-ring search-ring))
     (let* ((history-add-new-input nil)
+	   (default (car query-replace-from-to-history))
 	   (prompt
-	    (if query-replace-defaults
-		(format "%s (default %s -> %s): " prompt
-			(query-replace-descr (car query-replace-defaults))
-			(query-replace-descr (cdr query-replace-defaults)))
+	    (if query-replace-from-to-history
+		(format "%s (default %s): " prompt default)
 	      (format "%s: " prompt)))
 	   (from
 	    ;; The save-excursion here is in case the user marks and copies
@@ -148,11 +158,25 @@ (defun query-replace-read-from (prompt r
 		(read-from-minibuffer
 		 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)
+      (if (or
+	   ;; Default value
+	   (and (zerop (length from)) query-replace-from-to-history
+		(string-match query-replace-from-to-separator default))
+	   ;; History of FROM-TO pairs
+	   (and (eq query-replace-from-history-variable
+		    'query-replace-from-to-history)
+		(string-match query-replace-from-to-separator from)))
+	  (let ((from-string (substring (if (zerop (length from)) default from) 0 (match-beginning 0)))
+		(to-string (substring (if (zerop (length from)) default from) (match-end 0))))
+	    (unless (zerop (length from))
+	      (add-to-history query-replace-from-history-variable from nil t))
+	    (cons from-string
 		(query-replace-compile-replacement
-		 (cdr query-replace-defaults) regexp-flag))
-	(add-to-history query-replace-from-history-variable from nil t)
+		   to-string regexp-flag)))
+	;; Just FROM string
+	(unless (eq query-replace-from-history-variable
+		    'query-replace-from-to-history)
+	  (add-to-history query-replace-from-history-variable from nil t))
 	;; Warn if user types \n or \t, but don't reject the input.
 	(and regexp-flag
 	     (string-match "\\(\\`\\|[^\\]\\)\\(\\\\\\\\\\)*\\(\\\\[nt]\\)" from)
@@ -216,7 +240,9 @@ (defun query-replace-read-to (from promp
 		 nil nil nil
 		 query-replace-to-history-variable from t)))
        (add-to-history query-replace-to-history-variable to nil t)
-       (setq query-replace-defaults (cons from to))
+       (add-to-history 'query-replace-from-to-history
+		       (concat from query-replace-from-to-separator to)
+		       nil t)
        to))
    regexp-flag))
 

Re: History for query replace pairs

[Juri Linkov]

> If this line is followed, I'd recommend rebinding one of the transpose
> commands (transpose-lines, perhaps) to swap the TO and FROM. Having to
> manually kill and yank would be a very cumbersome way of reversing the
> replacement.

Good idea.  However, `transpose-lines' doesn't seem to fit semantically.
But perhaps you proposed it because other transpose-* commands might be
useful in the query-replace minibuffer inside the FROM or TO part.

Re: History for query replace pairs

[Artur Malabarba]

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

> > If this line is followed, I'd recommend rebinding one of the transpose
> > commands (transpose-lines, perhaps) to swap the TO and FROM. Having to
> > manually kill and yank would be a very cumbersome way of reversing the
> > replacement.
>
> Good idea.  However, `transpose-lines' doesn't seem to fit semantically.
> But perhaps you proposed it because other transpose-* commands might be
> useful in the query-replace minibuffer inside the FROM or TO part.

Yes, I figured transpose-lines is the least useful of the transpose
commands when it comes to query-replace input. But transpose sexps is
probably almost as useless and might feel more intuitive.

[-- Attachment #2: Type: text/html, Size: 779 bytes --]

Re: History for query replace pairs

[Stefan Monnier]

> Below is a preliminary implementation.  However, there is a problem
> that after saving it to the desktop or savehist, and restoring back
> FROM-TO string pairs in the history lose text-properties.
> If there is no way to save text properties then perhaps we should have
> two history variables: one with cons pairs (FROM . TO) to save
> in the desktop, and another with a list of strings "FROM<separator>TO"
> to use in `read-from-minibuffer', and keep these two variables in sync.

We definitely shouldn't store the pairs as a single string.
But do we really need to change the history vars at all?
I mean, we currently manage to extract "the last pair" from the current
history vars, so can't we extend this hack?


        Stefan

Re: History for query replace pairs

[Stefan Monnier]

> Yes, I figured transpose-lines is the least useful of the transpose
> commands when it comes to query-replace input.  But transpose sexps is
> probably almost as useless and might feel more intuitive.

Transpose sexps actually is very useful when editing the TO field.


        Stefan

Re: History for query replace pairs

[Andreas Schwab]

Artur Malabarba <bruce.connor.am@gmail.com> writes:

> Yes, I figured transpose-lines is the least useful of the transpose
> commands when it comes to query-replace input.

Minibuffer input can be multi-line.  Any editing command can be useful
there.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: History for query replace pairs

[Juri Linkov]

> We definitely shouldn't store the pairs as a single string.
> But do we really need to change the history vars at all?
> I mean, we currently manage to extract "the last pair" from the current
> history vars, so can't we extend this hack?

Since currently the default is displayed as "FROM -> TO" for
the last pair, and usually M-n inserts it into the minibuffer,
it makes sense to extend it so more M-n will insert more
previous pairs.  Then we don't need to change the history vars.

Re: History for query replace pairs

[Ted Zlatanov]

On Sat, 25 Oct 2014 23:52:18 +0300 Juri Linkov <juri@jurta.org> wrote: 

JL> If there is no way to save text properties then perhaps we should have
JL> two history variables: one with cons pairs (FROM . TO) to save
JL> in the desktop, and another with a list of strings "FROM<separator>TO"
JL> to use in `read-from-minibuffer', and keep these two variables in sync.

I like your POC.

This feature seems stalled, is this issue the only thing blocking it?

You could use a Unicode character (RIGHT ARROW → for instance), it's
unlikely to occur in a normal search and replace, and could be specially
encoded to "→→" in those cases. So you'd store "X→→Y→A→→→→B" to record
FROM="X→Y" and TO="A→→B".  It's an unambiguous encoding.  If that's too
hacky, let's find another way, and let me know how I can help.

Thanks!
Ted

Re: History for query replace pairs

[Juri Linkov]

> This feature seems stalled, is this issue the only thing blocking it?

The only blocking thing is to decide whether to move through the history
of replacements using M-p or M-n.  This patch implements a sequence of M-n
to insert replacement pairs, but it's easy to change just one line from

(read-from-minibuffer
 prompt nil nil nil query-replace-from-history-variable defaults t)))))

to

(read-from-minibuffer
 prompt nil nil nil 'defaults nil t)

then M-p will move through replacement pairs instead of previous
single history strings `from' or `to'.

Maybe this choice should be customizable?

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2014-08-25 02:36:45 +0000
+++ lisp/replace.el	2014-11-03 23:27:28 +0000
@@ -56,8 +56,8 @@ (defvar query-replace-history nil
 
 (defvar query-replace-defaults nil
   "Default values of FROM-STRING and TO-STRING for `query-replace'.
-This is a cons cell (FROM-STRING . TO-STRING), or nil if there is
-no default value.")
+This is a list of cons cells (FROM-STRING . TO-STRING),
+or nil if there is no default values.")
 
 (defvar query-replace-interactive nil
   "Non-nil means `query-replace' uses the last search string.
@@ -67,6 +67,13 @@ (make-obsolete-variable 'query-replace-i
 to the minibuffer that reads the string to replace, or invoke replacements
 from Isearch by using a key sequence like `C-s C-s M-%'." "24.3")
 
+(defvar query-replace-from-to-separator
+  (propertize
+   "\0" 'display (propertize
+		  (if (char-displayable-p ?\u2192) " \u2192 " " -> ")
+		  'face 'minibuffer-prompt))
+  "String that separates FROM and TO in the default replacements.")
+
 (defcustom query-replace-from-history-variable 'query-replace-history
   "History list to use for the FROM argument of `query-replace' commands.
 The value of this variable should be a symbol; that symbol
@@ -132,11 +139,15 @@ (defun query-replace-read-from (prompt r
   (if query-replace-interactive
       (car (if regexp-flag regexp-search-ring search-ring))
     (let* ((history-add-new-input nil)
+	   (defaults (mapcar (lambda (from-to)
+	   		       (concat (query-replace-descr (car from-to))
+				       query-replace-from-to-separator
+				       (query-replace-descr (cdr from-to))))
+	   		     query-replace-defaults))
 	   (prompt
-	    (if query-replace-defaults
-		(format "%s (default %s -> %s): " prompt
-			(query-replace-descr (car query-replace-defaults))
-			(query-replace-descr (cdr query-replace-defaults)))
+	    (if defaults
+		(format "%s (default %s): " prompt
+		        (car defaults))
 	      (format "%s: " prompt)))
 	   (from
 	    ;; The save-excursion here is in case the user marks and copies
@@ -144,14 +155,18 @@ (defun query-replace-read-from (prompt r
 	    ;; That should not clobber the region for the query-replace itself.
 	    (save-excursion
 	      (if regexp-flag
-		  (read-regexp prompt nil query-replace-from-history-variable)
+		  (read-regexp prompt defaults query-replace-from-history-variable)
 		(read-from-minibuffer
-		 prompt nil nil nil query-replace-from-history-variable
-		 (car (if regexp-flag regexp-search-ring search-ring)) t)))))
+		 prompt nil nil nil query-replace-from-history-variable
+		 defaults t)))))
       (if (and (zerop (length from)) query-replace-defaults)
-	  (cons (car query-replace-defaults)
+	  (cons (caar query-replace-defaults)
+		(query-replace-compile-replacement
+		 (cdar query-replace-defaults) regexp-flag))
+	(let* ((to (if (string-match query-replace-from-to-separator from)
 		(query-replace-compile-replacement
-		 (cdr query-replace-defaults) regexp-flag))
+			(substring from (match-end 0)) regexp-flag)))
+	       (from (if to (substring from 0 (match-beginning 0)) from)))
 	(add-to-history query-replace-from-history-variable from nil t)
 	;; Warn if user types \n or \t, but don't reject the input.
 	(and regexp-flag
@@ -163,7 +178,11 @@ (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))))
+	  (if (not to)
+	      from
+	    (add-to-history query-replace-to-history-variable to nil t)
+	    (push (cons from to) query-replace-defaults)
+	    (cons from to)))))))
 
 (defun query-replace-compile-replacement (to regexp-flag)
   "Maybe convert a regexp replacement TO to Lisp.
@@ -216,7 +235,7 @@ (defun query-replace-read-to (from promp
 		 nil nil nil
 		 query-replace-to-history-variable from t)))
        (add-to-history query-replace-to-history-variable to nil t)
-       (setq query-replace-defaults (cons from to))
+       (push (cons from to) query-replace-defaults)
        to))
    regexp-flag))
 


> You could use a Unicode character (RIGHT ARROW → for instance), it's
> unlikely to occur in a normal search and replace, and could be specially
> encoded to "→→" in those cases. So you'd store "X→→Y→A→→→→B" to record
> FROM="X→Y" and TO="A→→B".  It's an unambiguous encoding.  If that's too
> hacky, let's find another way, and let me know how I can help.

The patch uses "\0" as a separator that is highly unlikely to occur in
the replacement strings.

Also for better displaying of special characters in the minibuffer,
the following patch modifies isearch-text-char-description used by
query-replace-descr to put a formatted display property on special characters
instead of changing them.  This patch also works better for isearch,
because it keep the faces on the special characters in the failed string.

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2014-09-04 16:14:26 +0000
+++ lisp/isearch.el	2014-11-03 23:22:06 +0000
@@ -2873,8 +2873,12 @@ (defun isearch-no-upper-case-p (string r
 
 (defun isearch-text-char-description (c)
   (cond
-   ((< c ?\s) (propertize (format "^%c" (+ c 64)) 'face 'escape-glyph))
-   ((= c ?\^?) (propertize "^?" 'face 'escape-glyph))
+   ((< c ?\s) (propertize
+	       (char-to-string c)
+	       'display (propertize (format "^%c" (+ c 64)) 'face 'escape-glyph)))
+   ((= c ?\^?) (propertize
+		(char-to-string c)
+		'display (propertize "^?" 'face 'escape-glyph)))
    (t (char-to-string c))))
 
 ;; General function to unread characters or events.

Re: History for query replace pairs

[Ted Zlatanov]

On Tue, 04 Nov 2014 01:46:47 +0200 Juri Linkov <juri@jurta.org> wrote: 

>> This feature seems stalled, is this issue the only thing blocking it?
JL> The only blocking thing is to decide whether to move through the history
JL> of replacements using M-p or M-n.  This patch implements a sequence of M-n
JL> to insert replacement pairs, but it's easy to change just one line from

JL> (read-from-minibuffer
JL>  prompt nil nil nil query-replace-from-history-variable defaults t)))))

JL> to

JL> (read-from-minibuffer
JL>  prompt nil nil nil 'defaults nil t)

JL> then M-p will move through replacement pairs instead of previous
JL> single history strings `from' or `to'.

I'd rather use M-p as the default because the new behavior is IMO much
better.  But it's really no big deal, we can switch it later.

JL> Maybe this choice should be customizable?

OK with me, and probably most Emacs-like :)

JL> The patch uses "\0" as a separator that is highly unlikely to occur in
JL> the replacement strings.

Ah, much better than my suggestion.

JL> Also for better displaying of special characters in the minibuffer,
JL> the following patch modifies isearch-text-char-description used by
JL> query-replace-descr to put a formatted display property on special characters
JL> instead of changing them.  This patch also works better for isearch,
JL> because it keep the faces on the special characters in the failed string.

Nice, I like it.

Ted

Re: History for query replace pairs

[Juri Linkov]

> I'd rather use M-p as the default because the new behavior is IMO much
> better.  But it's really no big deal, we can switch it later.
>
> JL> Maybe this choice should be customizable?
>
> OK with me, and probably most Emacs-like :)

Then a new customizable variable `query-replace-from-to-defaults'
could define whether to show previous replacement pairs as defaults
available via M-n, or via M-p if nil:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2014-08-25 02:36:45 +0000
+++ lisp/replace.el	2014-11-04 23:04:55 +0000
@@ -54,10 +54,10 @@ (defvar query-replace-history nil
 See `query-replace-from-history-variable' and
 `query-replace-to-history-variable'.")
 
-(defvar query-replace-defaults nil
+(defvar query-replace-from-to-history nil
   "Default values of FROM-STRING and TO-STRING for `query-replace'.
-This is a cons cell (FROM-STRING . TO-STRING), or nil if there is
-no default value.")
+This is a list of cons cells (FROM-STRING . TO-STRING),
+or nil if there is no default values.")
 
 (defvar query-replace-interactive nil
   "Non-nil means `query-replace' uses the last search string.
@@ -67,6 +67,19 @@ (make-obsolete-variable 'query-replace-i
 to the minibuffer that reads the string to replace, or invoke replacements
 from Isearch by using a key sequence like `C-s C-s M-%'." "24.3")
 
+(defcustom query-replace-from-to-defaults nil
+  "Non-nil means a list of replacement pairs is available by M-n instead of M-p."
+  :group 'matching
+  :type 'boolean
+  :version "24.5")
+
+(defvar query-replace-from-to-separator
+  (propertize
+   "\0" 'display (propertize
+		  (if (char-displayable-p ?\u2192) " \u2192 " " -> ")
+		  'face 'minibuffer-prompt))
+  "String that separates FROM and TO in the default replacements.")
+
 (defcustom query-replace-from-history-variable 'query-replace-history
   "History list to use for the FROM argument of `query-replace' commands.
 The value of this variable should be a symbol; that symbol
@@ -132,11 +145,14 @@ (defun query-replace-read-from (prompt r
   (if query-replace-interactive
       (car (if regexp-flag regexp-search-ring search-ring))
     (let* ((history-add-new-input nil)
+	   (from-to-history (mapcar (lambda (from-to)
+	   		       (concat (query-replace-descr (car from-to))
+				       query-replace-from-to-separator
+				       (query-replace-descr (cdr from-to))))
+	   		     query-replace-from-to-history))
 	   (prompt
-	    (if query-replace-defaults
-		(format "%s (default %s -> %s): " prompt
-			(query-replace-descr (car query-replace-defaults))
-			(query-replace-descr (cdr query-replace-defaults)))
+	    (if from-to-history
+		(format "%s (default %s): " prompt (car from-to-history))
 	      (format "%s: " prompt)))
 	   (from
 	    ;; The save-excursion here is in case the user marks and copies
@@ -144,14 +160,29 @@ (defun query-replace-read-from (prompt r
 	    ;; That should not clobber the region for the query-replace itself.
 	    (save-excursion
 	      (if regexp-flag
-		  (read-regexp prompt nil query-replace-from-history-variable)
+		  (read-regexp prompt
+			       (if query-replace-from-to-defaults
+				   from-to-history)
+			       (if query-replace-from-to-defaults
+				   query-replace-from-history-variable
+				 'from-to-history))
 		(read-from-minibuffer
-		 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)
+		 prompt nil nil nil
+		 (if query-replace-from-to-defaults
+		     query-replace-from-history-variable
+		   'from-to-history)
+		 (if query-replace-from-to-defaults
+		     from-to-history
+		   (car (if regexp-flag regexp-search-ring search-ring)))
+		 t)))))
+      (if (and (zerop (length from)) query-replace-from-to-history)
+	  (cons (caar query-replace-from-to-history)
+		(query-replace-compile-replacement
+		 (cdar query-replace-from-to-history) regexp-flag))
+	(let* ((to (if (string-match query-replace-from-to-separator from)
 		(query-replace-compile-replacement
-		 (cdr query-replace-defaults) regexp-flag))
+			(substring from (match-end 0)) regexp-flag)))
+	       (from (if to (substring from 0 (match-beginning 0)) from)))
 	(add-to-history query-replace-from-history-variable from nil t)
 	;; Warn if user types \n or \t, but don't reject the input.
 	(and regexp-flag
@@ -163,7 +194,11 @@ (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))))
+	  (if (not to)
+	      from
+	    (add-to-history query-replace-to-history-variable to nil t)
+	    (push (cons from to) query-replace-from-to-history)
+	    (cons from to)))))))
 
 (defun query-replace-compile-replacement (to regexp-flag)
   "Maybe convert a regexp replacement TO to Lisp.
@@ -216,7 +251,7 @@ (defun query-replace-read-to (from promp
 		 nil nil nil
 		 query-replace-to-history-variable from t)))
        (add-to-history query-replace-to-history-variable to nil t)
-       (setq query-replace-defaults (cons from to))
+       (push (cons from to) query-replace-from-to-history)
        to))
    regexp-flag))
 

Re: History for query replace pairs

[Stefan Monnier]

> Then a new customizable variable `query-replace-from-to-defaults'
> could define whether to show previous replacement pairs as defaults
> available via M-n, or via M-p if nil:

Don't bother, please.  If/when the need becomes clear, we can add such
an option, but for now it seems completely unneeded.


        Stefan

Re: History for query replace pairs

[Juri Linkov]

>> Then a new customizable variable `query-replace-from-to-defaults'
>> could define whether to show previous replacement pairs as defaults
>> available via M-n, or via M-p if nil:
>
> Don't bother, please.  If/when the need becomes clear, we can add such
> an option, but for now it seems completely unneeded.

Do I understand you right that you want to put the replacement pairs
by default to M-p, not M-n?

Re: History for query replace pairs

[Stefan Monnier]

> Do I understand you right that you want to put the replacement pairs
> by default to M-p, not M-n?

Of course.


        Stefan

Re: History for query replace pairs

[Juri Linkov]

>> Do I understand you right that you want to put the replacement pairs
>> by default to M-p, not M-n?
>
> Of course.

The initial implementation is installed.  Pretty sure more changes
might be necessary, such as e.g. when some users unwilling to use
this feature can set `query-replace-from-to-separator' to nil,
but maybe this should be more customizable.

Re: History for query replace pairs

[Ted Zlatanov]

On Sat, 08 Nov 2014 01:34:27 +0200 Juri Linkov <juri@jurta.org> wrote: 

>>> Do I understand you right that you want to put the replacement pairs
>>> by default to M-p, not M-n?
>> 
>> Of course.

JL> The initial implementation is installed.  Pretty sure more changes
JL> might be necessary, such as e.g. when some users unwilling to use
JL> this feature can set `query-replace-from-to-separator' to nil,
JL> but maybe this should be more customizable.

I really like it, thank you for implementing it.  It behaves so
intuitively that I don't have to think about it.

I was able to delete the separator character.  I wonder if it can be
marked read-only.  But the code behaved well in that case, simply
assuming I had entered the "from" text, so it's not a big deal.

Ted

Re: History for query replace pairs

[Eli Zaretskii]

> From: Juri Linkov <juri@jurta.org>
> Date: Sat, 08 Nov 2014 01:34:27 +0200
> Cc: emacs-devel@gnu.org
> 
> The initial implementation is installed.

Shouldn't there be something about that in NEWS?

Re: History for query replace pairs

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Date: Fri, 07 Nov 2014 19:59:37 -0500
> 
> On Sat, 08 Nov 2014 01:34:27 +0200 Juri Linkov <juri@jurta.org> wrote: 
> 
> JL> The initial implementation is installed.  Pretty sure more changes
> JL> might be necessary, such as e.g. when some users unwilling to use
> JL> this feature can set `query-replace-from-to-separator' to nil,
> JL> but maybe this should be more customizable.
> 
> I really like it, thank you for implementing it.  It behaves so
> intuitively that I don't have to think about it.

I briefly tried it (will do more testing once NEWS or the manual
describes the feature in detail), and saw one problem: the Unicode
character → (u+2192) is a problem on TTYs that can't encode it or on
GUI frames that use font without a glyph for it.  So I think we need a
fallback for those cases (e.g., using char-displayable-p to detect them).

Thanks.

Re: History for query replace pairs

[Juri Linkov]

> I was able to delete the separator character.  I wonder if it can be
> marked read-only.  But the code behaved well in that case, simply
> assuming I had entered the "from" text, so it's not a big deal.

Yes, it's better to allow deleting the separator,
and undo should bring it back.

Re: History for query replace pairs

[Juri Linkov]

> I briefly tried it (will do more testing once NEWS or the manual

It's too early to describe it in the manual because it might require
more changes after testing.  And to announce it in NEWS we need
something more substantial such as to add an option to enable/disable
this feature.

> describes the feature in detail), and saw one problem: the Unicode
> character → (u+2192) is a problem on TTYs that can't encode it or on
> GUI frames that use font without a glyph for it.  So I think we need a
> fallback for those cases (e.g., using char-displayable-p to detect them).

When I tried to use char-displayable-p in defcustom, then
bootstraping failed with the error

  Attempt to autoload char-displayable-p while preparing to dump

because replace.el is preloaded, and we don't know whether the character
will be displayable on the current display.

I wonder is it possible to detect undisplayable characters in the
display engine and perform the normalization using the rules
for confusable characters?

In lisp/international/latin1-disp.el in latin1-display-ucs-per-lynx I see
the mapping (?\→ "->").  Could it be used for an automatic fallback?

Re: History for query replace pairs

[Eli Zaretskii]

> From: Juri Linkov <juri@jurta.org>
> Cc: emacs-devel@gnu.org
> Date: Sat, 08 Nov 2014 12:29:41 +0200
> 
> > I briefly tried it (will do more testing once NEWS or the manual
> 
> It's too early to describe it in the manual because it might require
> more changes after testing.  And to announce it in NEWS we need
> something more substantial such as to add an option to enable/disable
> this feature.

I'm okay with delaying the changes in the manual, but as for NEWS, you
cannot expect people who track the trunk to start using something
about which they have no hints at all.  At the very least, post some
initial information here.  Also, NEWS can explain features that have
no expression at all in user-level options; as long as there are
user-visible changes in behavior or keybindings or their meaning, this
is definitely NEWS-worthy stuff.

> > describes the feature in detail), and saw one problem: the Unicode
> > character → (u+2192) is a problem on TTYs that can't encode it or on
> > GUI frames that use font without a glyph for it.  So I think we need a
> > fallback for those cases (e.g., using char-displayable-p to detect them).
> 
> When I tried to use char-displayable-p in defcustom, then
> bootstraping failed with the error
> 
>   Attempt to autoload char-displayable-p while preparing to dump
> 
> because replace.el is preloaded, and we don't know whether the character
> will be displayable on the current display.

You could do that test only in interactive calls, or check
purify-flag, to avoid calling the function while dumping.

> I wonder is it possible to detect undisplayable characters in the
> display engine and perform the normalization using the rules
> for confusable characters?

It's possible, but I don't think we have such a feature now.

> In lisp/international/latin1-disp.el in latin1-display-ucs-per-lynx I see
> the mapping (?\→ "->").  Could it be used for an automatic fallback?

Used how?  Applying those large display tables sounds like a
misfeature to me: Emacs shouldn't do that without user's say-so, IMO.
If you mean something else, please elaborate.

Re: History for query replace pairs

[Stefan Monnier]

>> In lisp/international/latin1-disp.el in latin1-display-ucs-per-lynx I see
>> the mapping (?\→ "->").  Could it be used for an automatic fallback?
> Used how?  Applying those large display tables sounds like a
> misfeature to me: Emacs shouldn't do that without user's say-so, IMO.
> If you mean something else, please elaborate.

Why don't we just use " -> " even on terminals where → is available?
That's what we've been using in the prompt (when showing the default
replacement) and nobody complained about it.


        Stefan

Re: History for query replace pairs

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Juri Linkov <juri@jurta.org>,  emacs-devel@gnu.org
> Date: Sat, 08 Nov 2014 10:28:56 -0500
> 
> >> In lisp/international/latin1-disp.el in latin1-display-ucs-per-lynx I see
> >> the mapping (?\→ "->").  Could it be used for an automatic fallback?
> > Used how?  Applying those large display tables sounds like a
> > misfeature to me: Emacs shouldn't do that without user's say-so, IMO.
> > If you mean something else, please elaborate.
> 
> Why don't we just use " -> " even on terminals where → is available?

Fine with me.

Re: History for query replace pairs

[Juri Linkov]

> I'm okay with delaying the changes in the manual, but as for NEWS, you
> cannot expect people who track the trunk to start using something
> about which they have no hints at all.  At the very least, post some
> initial information here.  Also, NEWS can explain features that have
> no expression at all in user-level options; as long as there are
> user-visible changes in behavior or keybindings or their meaning, this
> is definitely NEWS-worthy stuff.

I just added a few lines about this feature to NEWS.

> You could do that test only in interactive calls

It seems the only remaining place to use char-displayable-p is
in the interactive call, and like isearch-text-char-description
replaces every character for its display representation, this code
could do something similar to convert the characters to be displayable:

(setq query-replace-from-to-separator
      (propertize
       query-replace-from-to-separator
       'display
       (mapconcat
        (lambda (c)
          (if (or (char-displayable-p c)
                  (not (eq c ?\u2192)))
              (char-to-string c)
            "->"))
        (get-text-property 0 'display query-replace-from-to-separator)
        "")))

Or maybe better would be to create two variables
with different versions of the separator?

Re: History for query replace pairs

[Juri Linkov]

> Why don't we just use " -> " even on terminals where → is available?
> That's what we've been using in the prompt (when showing the default
> replacement) and nobody complained about it.

The same reason why `prettify-symbols-alist' with ("->"  . ?→) exists -
because it's prettier than ASCII art :)

Actually nowadays I can't find anymore a terminal that doesn't
support Unicode and doesn't display a shorter and prettier → .
Then the most reliable detection would be to use char-displayable-p.

Re: History for query replace pairs

[Stefan Monnier]

> The same reason why `prettify-symbols-alist' with ("->"  . ?→) exists -
> because it's prettier than ASCII art :)

I know, but 

> Then the most reliable detection would be to use char-displayable-p.

char-displayable-p doesn't work reliably for text-terminals.
And of course we can't use the same separator globally, because
char-displayable-p can return different results in different terminals
(or even different frames).


        Stefan

Re: History for query replace pairs

[Paul Eggert]

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

Juri Linkov wrote:
> I can't find anymore a terminal that doesn't
> support Unicode and doesn't display a shorter and prettier → .

I can: the Terminal program on Ubuntu 14.10, the latest stable version of 
Ubuntu.  Attached is an image containing "→x" and "->x" as displayed on the 
terminal with the default font in my environment (Monospace 12); the "→x" is 
unreadable.




[-- Attachment #2: arrows.png --]
[-- Type: image/png, Size: 842 bytes --]

Re: History for query replace pairs

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Sat, 08 Nov 2014 21:01:54 -0500
> 
> I know, but 
> 
> > Then the most reliable detection would be to use char-displayable-p.
> 
> char-displayable-p doesn't work reliably for text-terminals.

I think it's the other way around: it's not 100% reliable on GUI
frames, due to half-hearted way we test whether the available fonts
support the character.

> And of course we can't use the same separator globally, because
> char-displayable-p can return different results in different terminals
> (or even different frames).

Indeed, this test will need to be done each time the history element
is about to be displayed.

Re: History for query replace pairs

[Juri Linkov]

>> And of course we can't use the same separator globally, because
>> char-displayable-p can return different results in different terminals
>> (or even different frames).
>
> Indeed, this test will need to be done each time the history element
> is about to be displayed.

This problem is solved by the following patch:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2014-11-07 23:33:41 +0000
+++ lisp/replace.el	2014-11-09 17:11:01 +0000
@@ -67,11 +67,20 @@ (make-obsolete-variable 'query-replace-i
 to the minibuffer that reads the string to replace, or invoke replacements
 from Isearch by using a key sequence like `C-s C-s M-%'." "24.3")
 
-(defvar query-replace-from-to-separator
-  (propertize "\0"
-	      'display (propertize " \u2192 " 'face 'minibuffer-prompt)
-	      'separator t)
-  "String that separates FROM and TO in the history of replacement pairs.")
+(defcustom query-replace-from-to-separator
+  (propertize
+   "\0"
+   'display (propertize
+	     (if (and
+		  ;; Don't call char-displayable-p while pre-loading replace.
+		  (not (equal (car preloaded-file-list) "replace"))
+		  (char-displayable-p ?\u2192)) " \u2192 " " -> ")
+	     'face 'minibuffer-prompt)
+   'separator t)
+  "String that separates FROM and TO in the history of replacement pairs."
+  :group 'matching
+  :type 'sexp
+  :version "25.1")
 
 (defcustom query-replace-from-history-variable 'query-replace-history
   "History list to use for the FROM argument of `query-replace' commands.
@@ -137,6 +146,7 @@ (defun query-replace-read-from (prompt r
 wants to replace FROM with TO."
   (if query-replace-interactive
       (car (if regexp-flag regexp-search-ring search-ring))
+    (custom-reevaluate-setting 'query-replace-from-to-separator)
     (let* ((history-add-new-input nil)
 	   (query-replace-from-to-history
 	    (append

Re: History for query replace pairs

[Juri Linkov]

> I can: the Terminal program on Ubuntu 14.10, the latest stable version of
> Ubuntu.  Attached is an image containing "→x" and "->x" as displayed on the
> terminal with the default font in my environment (Monospace 12); the "→x"
> is unreadable.

I confirm this problem with the Monospace 12 font.  Fortunately, there
is a space character after default separator, so "→ x" is readable,
because → is combined with the space character.

Re: History for query replace pairs

[Eli Zaretskii]

> From: Juri Linkov <juri@jurta.org>
> Cc: emacs-devel@gnu.org
> Date: Sun, 09 Nov 2014 00:51:38 +0200
> 
> > I'm okay with delaying the changes in the manual, but as for NEWS, you
> > cannot expect people who track the trunk to start using something
> > about which they have no hints at all.  At the very least, post some
> > initial information here.  Also, NEWS can explain features that have
> > no expression at all in user-level options; as long as there are
> > user-visible changes in behavior or keybindings or their meaning, this
> > is definitely NEWS-worthy stuff.
> 
> I just added a few lines about this feature to NEWS.

Thanks.  I took the liberty of making minor improvements there.

Re: History for query replace pairs

[Stefan Monnier]

>> char-displayable-p doesn't work reliably for text-terminals.
> I think it's the other way around: it's not 100% reliable on GUI
> frames, due to half-hearted way we test whether the available fonts
> support the character.

It's not 100% reliable for GUI frames (especially if the test doesn't
pay attention to the `face' that's added), but for tty frames it's even
worse since the only thing we (can) test is whether the char can be
encoded in the terminal's coding-system.  With the typical utf-8
encoding used under GNU/Linux and OSX nowadays this basically means that 
char-displayable-p under ttys will almost always return t, regardless of
whether the text-terminal's font can actually display such a char (and
since text-terminals are usually restricted to monospaced fonts, they
have much less liberty to "look for alternative fonts" when faced with
an unusual char, so it's much more frequent that they just can't
display a character).


        Stefan

Re: History for query replace pairs

[Stefan Monnier]

> +  (propertize
> +   "\0"
> +   'display (propertize
> +	     (if (and
> +		  ;; Don't call char-displayable-p while pre-loading replace.
> +		  (not (equal (car preloaded-file-list) "replace"))
> +		  (char-displayable-p ?\u2192)) " \u2192 " " -> ")
> +	     'face 'minibuffer-prompt)
> +   'separator t)

I think the (propertize "\0" ... 'separator t) wrapper should *not*
be customizable.

And making the arrow cutomizable seems like a bad idea anyway, just
pushing the problem onto the user.

The (not (equal (car preloaded-file-list) "replace")) test makes
no sense.  Why don't we want to call char-displayable-p while
pre-loading replace?  Shouldn't we instead test (fboundp 'char-displayable-p)
or simply wrap the code in ignore-errors (if the problem is that
char-displayable-p fails when run without a real frame at hand, which
might also happen in daemon mode)?


        Stefan

Re: History for query replace pairs

[Juri Linkov]

> The (not (equal (car preloaded-file-list) "replace")) test makes
> no sense.  Why don't we want to call char-displayable-p while
> pre-loading replace?  Shouldn't we instead test (fboundp 'char-displayable-p)
> or simply wrap the code in ignore-errors (if the problem is that
> char-displayable-p fails when run without a real frame at hand, which
> might also happen in daemon mode)?

I already tried (fboundp 'char-displayable-p) earlier but it
returns t while pre-loading because the problem was caused
by an attempt to autoload char-displayable-p.

Now I tried ignore-errors, and there are no problems with it:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2014-11-07 23:33:41 +0000
+++ lisp/replace.el	2014-11-09 22:58:25 +0000
@@ -67,11 +67,16 @@ (make-obsolete-variable 'query-replace-i
 to the minibuffer that reads the string to replace, or invoke replacements
 from Isearch by using a key sequence like `C-s C-s M-%'." "24.3")
 
-(defvar query-replace-from-to-separator
-  (propertize "\0"
-	      'display (propertize " \u2192 " 'face 'minibuffer-prompt)
-	      'separator t)
-  "String that separates FROM and TO in the history of replacement pairs.")
+(defcustom query-replace-from-to-separator
+  (propertize
+   (or (ignore-errors
+	 (if (char-displayable-p ?\u2192) " \u2192 " " -> "))
+       " -> ")
+   'face 'minibuffer-prompt)
+  "String that separates FROM and TO in the history of replacement pairs."
+  :group 'matching
+  :type 'sexp
+  :version "25.1")
 
 (defcustom query-replace-from-history-variable 'query-replace-history
   "History list to use for the FROM argument of `query-replace' commands.
@@ -137,19 +142,25 @@ (defun query-replace-read-from (prompt r
 wants to replace FROM with TO."
   (if query-replace-interactive
       (car (if regexp-flag regexp-search-ring search-ring))
+    (custom-reevaluate-setting 'query-replace-from-to-separator)
     (let* ((history-add-new-input nil)
+	   (separator
+	    (when query-replace-from-to-separator
+	      (propertize "\0"
+			  'display query-replace-from-to-separator
+			  'separator t)))
 	   (query-replace-from-to-history
 	    (append
-	     (when query-replace-from-to-separator
+	     (when separator
 	       (mapcar (lambda (from-to)
 			 (concat (query-replace-descr (car from-to))
-				 query-replace-from-to-separator
+				 separator
 				 (query-replace-descr (cdr from-to))))
 		       query-replace-defaults))
 	     (symbol-value query-replace-from-history-variable)))
 	   (minibuffer-allow-text-properties t) ; separator uses text-properties
 	   (prompt
-	    (if (and query-replace-defaults query-replace-from-to-separator)
+	    (if (and query-replace-defaults separator)
 		(format "%s (default %s): " prompt (car query-replace-from-to-history))
 	      (format "%s: " prompt)))
 	   (from
@@ -166,7 +177,7 @@ (defun query-replace-read-from (prompt r
 	  (cons (caar query-replace-defaults)
 		(query-replace-compile-replacement
 		 (cdar query-replace-defaults) regexp-flag))
-	(let* ((to (if (and (string-match query-replace-from-to-separator from)
+	(let* ((to (if (and (string-match separator from)
 			    (get-text-property (match-beginning 0) 'separator from))
 		       (query-replace-compile-replacement
 			(substring-no-properties from (match-end 0)) regexp-flag)))