bug#46033: up arrow in query replace should not step into the prompt

bug#46033: up arrow in query replace should not step into the prompt

[laszlomail--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Up/down arrow is useful in the replace prompt, because sometimes
the replace text is long and spans multiple lines, so up/down is
handy to navigate it.

Up arrow can also retrieve previous history lines when you are
already at the top line of the replacement.

However, if there is a multiline replacement default in the
prompt then up arrow steps into the prompt first and only further
presses retrieve history:

https://i.imgur.com/7G3FcPX.png

Up arrow should not lead the user uselessly into the prompt text.

bug#46033: up arrow in query replace should not step into the prompt

[Eli Zaretskii]

> Date: Fri, 22 Jan 2021 13:21:50 +0000
> From: laszlomail--- via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Up/down arrow is useful in the replace prompt, because sometimes
> the replace text is long and spans multiple lines, so up/down is
> handy to navigate it.
> 
> Up arrow can also retrieve previous history lines when you are
> already at the top line of the replacement.
> 
> However, if there is a multiline replacement default in the
> prompt then up arrow steps into the prompt first and only further
> presses retrieve history:
> 
> https://i.imgur.com/7G3FcPX.png
> 
> Up arrow should not lead the user uselessly into the prompt text.

This is a feature: it allows you to scroll through the text shown in
the minibuffer, in case you want to edit it.  Use M-p to go directly
to the previous history.

bug#46033: up arrow in query replace should not step into the prompt

[laszlomail--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> This is a feature: it allows you to scroll through the text shown in
> the minibuffer, in case you want to edit it. Use M-p to go directly
> to the previous history.

I understand it's a feature, but as you see in the picture the prompt is
empty, and the full default replacement is visible.

In this case arrow should retrieve the previous history, instead of going
into the prompt, because there is nothing to scroll.

bug#46033: up arrow in query replace should not step into the prompt

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> This is a feature: it allows you to scroll through the text shown in
> the minibuffer, in case you want to edit it.  Use M-p to go directly
> to the previous history.

I think the complaint is that this:

M-: (read-string "Foo: ")
<up>

and this:

M-: (read-string "This is a very long prompt indeed and it goes on and on and never seems to stop oh yes now it did: ")
<up>

work very differently.  In the first case, the <up> works the same as
`M-p',  In the latter case, <up> puts point into the prompt.

It does seem inconsistent.

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

bug#46033: up arrow in query replace should not step into the prompt

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: reporter <laszlomail@protonmail.com>,  46033@debbugs.gnu.org
> Date: Fri, 22 Jan 2021 19:44:52 +0100
> 
> I think the complaint is that this:
> 
> M-: (read-string "Foo: ")
> <up>
> 
> and this:
> 
> M-: (read-string "This is a very long prompt indeed and it goes on and on and never seems to stop oh yes now it did: ")
> <up>
> 
> work very differently.  In the first case, the <up> works the same as
> `M-p',  In the latter case, <up> puts point into the prompt.
> 
> It does seem inconsistent.

That's how it's supposed to work, though.

bug#46033: up arrow in query replace should not step into the prompt

[Juri Linkov]

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

>> This is a feature: it allows you to scroll through the text shown in
>> the minibuffer, in case you want to edit it. Use M-p to go directly
>> to the previous history.
>
> I understand it's a feature, but as you see in the picture the prompt is
> empty, and the full default replacement is visible.
>
> In this case arrow should retrieve the previous history, instead of going
> into the prompt, because there is nothing to scroll.

Thanks for the well justified request.  This patch avoids moving point
to the prompt on the rightful assumption that most of the time the users
would not want to edit read-only text:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: avoid-minibuffer-prompt.patch --]
[-- Type: text/x-diff, Size: 627 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index 0355ac863a..f5fe366f69 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2476,7 +2476,10 @@ previous-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (previous-line arg))
+	  (previous-line arg)
+          ;; Avoid moving point to the prompt
+          (when (< (point) (minibuffer-prompt-end))
+            (signal 'beginning-of-buffer nil)))
       (beginning-of-buffer
        ;; Restore old position since `line-move-visual' moves point to
        ;; the beginning of the line when it fails to go to the previous line.

bug#46033: up arrow in query replace should not step into the prompt

[laszlomail--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> That's how it's supposed to work, though.

It is not useful at all when the prompt is empty and the full default
replacement is visible, because then there is nothing to scroll.

This is the case just after the user invoked query replace. If
the user does not type anything for a new replacement then he
wants to retrieve a previous replacement.

In this specific case up arrow should also retrieve the previous
history, instead of going into the prompt which is useless in
this case.

https://i.imgur.com/7G3FcPX.png

bug#46033: up arrow in query replace should not step into the prompt

[laszlomail--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

>
> Thanks for the well justified request. This patch avoids moving point
> to the prompt on the rightful assumption that most of the time the users
> would not want to edit read-only text:

This breaks the history retrieval feature with up arrow.

With the current implementation up arrow takes you into the prompt, but if
you are in the top line of the prompt and press up arrow again then up
arrow retrieves history.

In the case I described going into the prompt is useless, so up arrow
should retrieve history immediately, without having to go to the top
line of the prompt first.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: avoid-minibuffer-prompt.patch --]
[-- Type: text/x-diff; name=avoid-minibuffer-prompt.patch, Size: 627 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index 0355ac863a..f5fe366f69 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2476,7 +2476,10 @@ previous-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (previous-line arg))
+	  (previous-line arg)
+          ;; Avoid moving point to the prompt
+          (when (< (point) (minibuffer-prompt-end))
+            (signal 'beginning-of-buffer nil)))
       (beginning-of-buffer
        ;; Restore old position since `line-move-visual' moves point to
        ;; the beginning of the line when it fails to go to the previous line.

bug#46033: up arrow in query replace should not step into the prompt

[laszlomail--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

>
> In this specific case up arrow should also retrieve the previous
> history, instead of going into the prompt which is useless in
> this case.
>

Attached a patch doing what I propose.

[-- Attachment #2: prompt.patch --]
[-- Type: application/octet-stream, Size: 777 bytes --]

c:/Progra~2/GnuWin32/bin/diff.exe -u "c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/simple.el" "d:/download/simple.el"
--- c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/simple.el	2020-08-12 08:36:54.000000000 +0200
+++ d:/download/simple.el	2021-01-23 19:15:00.428631300 +0100
@@ -2303,6 +2303,9 @@
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
+          (if (eq (point-min) ;; prompt is entirely visible
+                  (- (minibuffer-prompt-end) (length (minibuffer-prompt))))
+              (signal 'beginning-of-buffer nil)) ;; get previous history item
 	  (previous-line arg))
       (beginning-of-buffer
        ;; Restore old position since `line-move-visual' moves point to

Diff finished.  Sat Jan 23 19:15:02 2021

bug#46033: up arrow in query replace should not step into the prompt

[laszlomail--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

>
> Attached a patch doing what I propose.


Fixed typo (using window-start).


[-- Attachment #2: prompt.patch --]
[-- Type: application/octet-stream, Size: 780 bytes --]

c:/Progra~2/GnuWin32/bin/diff.exe -u "c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/simple.el" "d:/download/simple.el"
--- c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/simple.el	2020-08-12 08:36:54.000000000 +0200
+++ d:/download/simple.el	2021-01-23 19:28:19.752834800 +0100
@@ -2303,6 +2303,9 @@
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
+          (if (eq (window-start) ;; prompt is entirely visible
+                  (- (minibuffer-prompt-end) (length (minibuffer-prompt))))
+              (signal 'beginning-of-buffer nil)) ;; get previous history item
 	  (previous-line arg))
       (beginning-of-buffer
        ;; Restore old position since `line-move-visual' moves point to

Diff finished.  Sat Jan 23 19:28:42 2021

bug#46033: up arrow in query replace should not step into the prompt

[laszlomail--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> > Attached a patch doing what I propose.
>
> Fixed typo (using window-start).


Third time's a charm.

[-- Attachment #2: prompt.patch --]
[-- Type: application/octet-stream, Size: 716 bytes --]

c:/Progra~2/GnuWin32/bin/diff.exe -u "c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/simple.el" "d:/download/simple.el"
--- c:/Program Files/emacs-27.1/share/emacs/27.1/lisp/simple.el	2020-08-12 08:36:54.000000000 +0200
+++ d:/download/simple.el	2021-01-23 19:31:50.824461200 +0100
@@ -2303,6 +2303,8 @@
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
+          (if (eq (window-start) (point-min))  ;; prompt is entirely visible
+              (signal 'beginning-of-buffer nil)) ;; get previous history item
 	  (previous-line arg))
       (beginning-of-buffer
        ;; Restore old position since `line-move-visual' moves point to

Diff finished.  Sat Jan 23 19:32:06 2021

bug#46033: up arrow in query replace should not step into the prompt

[Juri Linkov]

>> This patch avoids moving point to the prompt on the rightful
>> assumption that most of the time the users would not want to edit
>> read-only text:
>
> This breaks the history retrieval feature with up arrow.
>
> With the current implementation up arrow takes you into the prompt, but if
> you are in the top line of the prompt and press up arrow again then up
> arrow retrieves history.
>
> In the case I described going into the prompt is useless, so up arrow
> should retrieve history immediately, without having to go to the top
> line of the prompt first.

This is exactly what the patch does.  Have you really tried it?

bug#46033: up arrow in query replace should not step into the prompt

[laszlomail--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> This is exactly what the patch does. Have you really tried it?

Yes, I don't know what happened (I applied it manually), but I
must have made an error, because it didn't work.

Then I checked the original code and I came up with the same
solution you did.

So, yes, it does work. Thanks, and sorry for the confusion.

bug#46033: up arrow in query replace should not step into the prompt

[Juri Linkov]

tags 46033 fixed
close 46033 28.0.50
thanks

>> This is exactly what the patch does. Have you really tried it?
>
> Yes, I don't know what happened (I applied it manually), but I
> must have made an error, because it didn't work.
>
> Then I checked the original code and I came up with the same
> solution you did.
>
> So, yes, it does work. Thanks, and sorry for the confusion.

Thanks for confirming, now this is fixed.

bug#46033: up arrow in query replace should not step into the prompt

[Juri Linkov]

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

>>> This is exactly what the patch does. Have you really tried it?
>>
>> Yes, I don't know what happened (I applied it manually), but I
>> must have made an error, because it didn't work.
>>
>> Then I checked the original code and I came up with the same
>> solution you did.
>>
>> So, yes, it does work. Thanks, and sorry for the confusion.
>
> Thanks for confirming, now this is fixed.

Actually, this is not so simple.  Currently this doesn't work with
multi-line minibuffer.  When there is non-prompt text on the previous line,
it should move point to the editable portion of that line.  So I pushed
this patch that supports not only multi-line minibuffer contents,
but also multi-line prompts.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: previous-line-or-history-element.patch --]
[-- Type: text/x-diff, Size: 1282 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index c878fdab92..680bfd7f43 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2472,14 +2472,24 @@ previous-line-or-history-element
 				   (save-excursion
 				     (goto-char (1- prompt-end))
 				     (current-column)))
-				0)
+				1)
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
 	  (previous-line arg)
           ;; Avoid moving point to the prompt
           (when (< (point) (minibuffer-prompt-end))
-            (signal 'beginning-of-buffer nil)))
+            ;; If there is minibuffer contents on the same line
+            (if (<= (minibuffer-prompt-end)
+                    (save-excursion
+                      (if (or truncate-lines (not line-move-visual))
+                          (end-of-line)
+                        (end-of-visual-line))
+                      (point)))
+                ;; Move to the beginning of minibuffer contents
+                (goto-char (minibuffer-prompt-end))
+              ;; Otherwise, go to the previous history element
+              (signal 'beginning-of-buffer nil))))
       (beginning-of-buffer
        ;; Restore old position since `line-move-visual' moves point to
        ;; the beginning of the line when it fails to go to the previous line.