unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#27191: 26.0.50; Long history items in minibuffer (again)
@ 2017-06-01 20:46 Stephen Berman
  2017-06-02  8:01 ` Stephen Berman
  2020-08-24 14:38 ` Lars Ingebrigtsen
  0 siblings, 2 replies; 6+ messages in thread
From: Stephen Berman @ 2017-06-01 20:46 UTC (permalink / raw)
  To: 27191

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

0. emacs -Q
1. C-x C-f ~/foo/bar/very/long/file/name/that/overflows/minibuffer/window/line/when/displayed <RET>
2. C-x C-f <up>
=> The file name entered in step 1 appears in the minibuffer, with point
   on the "w" of "when" (i.e., column 80, the end of the visual line).

If at step 2 instead of <up> you type `M-p', then point is at the end of
the file name in the minibuffer.  This is what I expected for <up> too.

The result with <up> is due to the fix for bug#22544.  In the bug thread
(http://lists.gnu.org/archive/html/bug-gnu-emacs/2016-02/msg00357.html),
the above problem was noted:

> > Can't we special-case a line that isn't broken into several visual
> > lines, and put the cursor at the end of such lines only?  That'd be
> > the best.
> 
> The problem here is that like bash and other shells with histories do,
> we need to put the cursor at the end of the previous history element
> so the user can start editing it immediately (usually deleting the chars
> from the end of the logical line).  OTOH, a subsequent <UP> should
> continue navigating the history and put the next previous element to the
> minibuffer.  But then <UP> can't be used to move between visual lines.
> This is a lose-lose situation, unless we'll find some clever DWIM.

The attached patch isn't particularly clever, but (unless I've
overlooked something) DWIM: it puts point at the end of a history
element longer than window-width, and if such an element is a single
line, the next <up> displays the previous history element.  Otherwise,
<up> moves by visual lines (specifically also in multi-line history
elements, including those with lines longer than window-width).

(I wanted to add tests for this, but I haven't been able to figure out
how to emulate minibuffer history navigation non-interactively.)

In GNU Emacs 26.0.50 (build 30, x86_64-pc-linux-gnu, GTK+ Version 3.22.8)
 of 2017-05-29 built on rosalinde
Repository revision: c503188f8079ae73d95abd0bce0f53d104b03205
Windowing system distributor 'The X.Org Foundation', version 11.0.11901000


2017-06-01  Stephen Berman  <stephen.berman@gmx.net>

	Improve navigation through minibuffer history

	* lisp/simple.el (previous-line-or-history-element): If the
	element extends beyond window-width, go to its logical end,
	since that is the natural target for editing history elements.
	If it consists of a single line, move by logical lines to hit
	beginning-of-buffer immediately and get the previous history
	element.  Otherwise, move by visual lines.


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

diff --git a/lisp/simple.el b/lisp/simple.el
index ea3a495fbc..1b96bce773 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2140,7 +2140,16 @@ previous-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (previous-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; beginning-of-buffer immediately and get the previous
+	  ;; history element.  Otherwise, move by visual lines.
+	  (if (and (save-excursion
+	  	     (end-of-line)
+	  	     (> (current-column) (window-width)))
+		   (= (line-number-at-pos) 1))
+	      (previous-logical-line arg)
+	    (previous-line arg)))
       (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.
@@ -2162,10 +2171,9 @@ previous-line-or-history-element
 	 ;; would move to the previous history element, not to a possible upper
 	 ;; visual line from the end of logical line in `line-move-visual' mode.
 	 (end-of-visual-line)
-	 ;; Since `end-of-visual-line' puts the cursor at the beginning
-	 ;; of the next visual line, move it one char back to the end
-	 ;; of the first visual line (bug#22544).
-	 (unless (eolp) (backward-char 1)))))))
+	 ;; If the line extends beyond window-width, go to its logical end,
+	 ;; since that is the natural target for editing history elements.
+	 (unless (eolp) (end-of-line)))))))
 
 (defun next-complete-history-element (n)
   "Get next history element which completes the minibuffer before the point.

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

* bug#27191: 26.0.50; Long history items in minibuffer (again)
  2017-06-01 20:46 bug#27191: 26.0.50; Long history items in minibuffer (again) Stephen Berman
@ 2017-06-02  8:01 ` Stephen Berman
  2017-06-02 21:17   ` Stephen Berman
  2020-08-24 14:38 ` Lars Ingebrigtsen
  1 sibling, 1 reply; 6+ messages in thread
From: Stephen Berman @ 2017-06-02  8:01 UTC (permalink / raw)
  To: 27191

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

On Thu, 01 Jun 2017 22:46:15 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:

> 0. emacs -Q
> 1. C-x C-f
> ~/foo/bar/very/long/file/name/that/overflows/minibuffer/window/line/when/displayed
> <RET>
> 2. C-x C-f <up>
> => The file name entered in step 1 appears in the minibuffer, with point
>    on the "w" of "when" (i.e., column 80, the end of the visual line).
>
> If at step 2 instead of <up> you type `M-p', then point is at the end of
> the file name in the minibuffer.  This is what I expected for <up> too.
>
> The result with <up> is due to the fix for bug#22544.  In the bug thread
> (http://lists.gnu.org/archive/html/bug-gnu-emacs/2016-02/msg00357.html),
> the above problem was noted:
>
>> > Can't we special-case a line that isn't broken into several visual
>> > lines, and put the cursor at the end of such lines only?  That'd be
>> > the best.
>> 
>> The problem here is that like bash and other shells with histories do,
>> we need to put the cursor at the end of the previous history element
>> so the user can start editing it immediately (usually deleting the chars
>> from the end of the logical line).  OTOH, a subsequent <UP> should
>> continue navigating the history and put the next previous element to the
>> minibuffer.  But then <UP> can't be used to move between visual lines.
>> This is a lose-lose situation, unless we'll find some clever DWIM.
>
> The attached patch isn't particularly clever, but (unless I've
> overlooked something)

Oops, I did.  Here's the corrected patch:


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

diff --git a/lisp/simple.el b/lisp/simple.el
index ea3a495fbc..5c7dab8f74 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2140,7 +2140,16 @@ previous-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (previous-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; beginning-of-buffer immediately and get the previous
+	  ;; history element.  Otherwise, move by visual lines.
+	  (if (and (save-excursion
+	  	     (end-of-line)
+	  	     (> (current-column) (window-width)))
+		   (= (line-number-at-pos) 1))
+	      (previous-logical-line arg)
+	    (previous-line arg)))
       (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.
@@ -2157,15 +2166,12 @@ previous-line-or-history-element
 	   (if (= (line-number-at-pos) 1)
 	       (move-to-column (+ old-column (1- (minibuffer-prompt-end))))
 	     (move-to-column old-column))
-	 ;; Put the cursor at the end of the visual line instead of the
-	 ;; logical line, so the next `previous-line-or-history-element'
-	 ;; would move to the previous history element, not to a possible upper
-	 ;; visual line from the end of logical line in `line-move-visual' mode.
-	 (end-of-visual-line)
-	 ;; Since `end-of-visual-line' puts the cursor at the beginning
-	 ;; of the next visual line, move it one char back to the end
-	 ;; of the first visual line (bug#22544).
-	 (unless (eolp) (backward-char 1)))))))
+	 ;; Put the cursor at the end of the logical line, even if it extends
+         ;; beyond window-width, since that is the natural target for editing
+         ;; history elements (bug#27191).  The condition above makes sure the
+         ;; next `previous-line-or-history-element' will move to the previous
+         ;; history element in either case.
+         (end-of-line))))))
 
 (defun next-complete-history-element (n)
   "Get next history element which completes the minibuffer before the point.

[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


Steve Berman

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

* bug#27191: 26.0.50; Long history items in minibuffer (again)
  2017-06-02  8:01 ` Stephen Berman
@ 2017-06-02 21:17   ` Stephen Berman
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Berman @ 2017-06-02 21:17 UTC (permalink / raw)
  To: 27191

On Fri, 02 Jun 2017 10:01:04 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Thu, 01 Jun 2017 22:46:15 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:
>
>> 0. emacs -Q
>> 1. C-x C-f
>> ~/foo/bar/very/long/file/name/that/overflows/minibuffer/window/line/when/displayed
>> <RET>
>> 2. C-x C-f <up>
>> => The file name entered in step 1 appears in the minibuffer, with point
>>    on the "w" of "when" (i.e., column 80, the end of the visual line).
>>
>> If at step 2 instead of <up> you type `M-p', then point is at the end of
>> the file name in the minibuffer.  This is what I expected for <up> too.
>>
>> The result with <up> is due to the fix for bug#22544.  In the bug thread
>> (http://lists.gnu.org/archive/html/bug-gnu-emacs/2016-02/msg00357.html),
>> the above problem was noted:
>>
>>> > Can't we special-case a line that isn't broken into several visual
>>> > lines, and put the cursor at the end of such lines only?  That'd be
>>> > the best.
>>> 
>>> The problem here is that like bash and other shells with histories do,
>>> we need to put the cursor at the end of the previous history element
>>> so the user can start editing it immediately (usually deleting the chars
>>> from the end of the logical line).  OTOH, a subsequent <UP> should
>>> continue navigating the history and put the next previous element to the
>>> minibuffer.  But then <UP> can't be used to move between visual lines.
>>> This is a lose-lose situation, unless we'll find some clever DWIM.
>>
>> The attached patch isn't particularly clever, but (unless I've
>> overlooked something)
>
> Oops, I did.

I also overlooked that <down> on such a long line in the minibuffer
moves visually to the overflowing part of the element, rather than
immediately going to the next history element; this is likewise fixed by
the corresponding patch to next-line-or-history-element:

diff --git a/lisp/simple.el b/lisp/simple.el
index ea3a495fbc..7914fc1fae 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2106,7 +2106,16 @@ next-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (next-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; end-of-buffer immediately and get the next history
+	  ;; element.  Otherwise, move by visual lines.
+	  (if (and (save-excursion
+	  	     (end-of-line)
+	  	     (> (current-column) (window-width)))
+		   (= (line-number-at-pos) 1))
+	      (next-logical-line arg)
+	    (next-line arg)))
       (end-of-buffer
        ;; Restore old position since `line-move-visual' moves point to
        ;; the end of the line when it fails to go to the next line.

(Unlike with previous-line-or-history-element, with without
next-line-or-history-element I don't see any difference with or without
an explicit call to end-of-line at the end.)

Steve Berman





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

* bug#27191: 26.0.50; Long history items in minibuffer (again)
  2017-06-01 20:46 bug#27191: 26.0.50; Long history items in minibuffer (again) Stephen Berman
  2017-06-02  8:01 ` Stephen Berman
@ 2020-08-24 14:38 ` Lars Ingebrigtsen
  2020-08-25 18:01   ` Stephen Berman
  1 sibling, 1 reply; 6+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-24 14:38 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 27191

Stephen Berman <stephen.berman@gmx.net> writes:

> 0. emacs -Q
> 1. C-x C-f
> ~/foo/bar/very/long/file/name/that/overflows/minibuffer/window/line/when/displayed
> <RET>
> 2. C-x C-f <up>
> => The file name entered in step 1 appears in the minibuffer, with point
>    on the "w" of "when" (i.e., column 80, the end of the visual line).
>
> If at step 2 instead of <up> you type `M-p', then point is at the end of
> the file name in the minibuffer.  This is what I expected for <up> too.

Yes, that's really confusing.  I do get a different result than you,
though -- point is placed inside the "Find file" prompt when I hit
<up>.  :-/  That's even worse.

> 2017-06-01  Stephen Berman  <stephen.berman@gmx.net>
>
> 	Improve navigation through minibuffer history
>
> 	* lisp/simple.el (previous-line-or-history-element): If the
> 	element extends beyond window-width, go to its logical end,
> 	since that is the natural target for editing history elements.
> 	If it consists of a single line, move by logical lines to hit
> 	beginning-of-buffer immediately and get the previous history
> 	element.  Otherwise, move by visual lines.
>

Your change makes sense, I think, but when I tried applying the two
patches, they no longer applied to Emacs 28, so I couldn't give it a
try.  Could you possibly respin the patches?

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





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

* bug#27191: 26.0.50; Long history items in minibuffer (again)
  2020-08-24 14:38 ` Lars Ingebrigtsen
@ 2020-08-25 18:01   ` Stephen Berman
  2020-08-25 19:17     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Berman @ 2020-08-25 18:01 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 27191

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

On Mon, 24 Aug 2020 16:38:02 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> 0. emacs -Q
>> 1. C-x C-f
>> ~/foo/bar/very/long/file/name/that/overflows/minibuffer/window/line/when/displayed
>> <RET>
>> 2. C-x C-f <up>
>> => The file name entered in step 1 appears in the minibuffer, with point
>>    on the "w" of "when" (i.e., column 80, the end of the visual line).
>>
>> If at step 2 instead of <up> you type `M-p', then point is at the end of
>> the file name in the minibuffer.  This is what I expected for <up> too.
>
> Yes, that's really confusing.  I do get a different result than you,
> though -- point is placed inside the "Find file" prompt when I hit
> <up>.  :-/  That's even worse.

I see that too, now.  That's because at step 2 the current buffer is the
one made in step one, so after <up> the prompt includes all parent
directory names.  When, after step 1, I switch to another buffer and
then do step 2, I see what I reported.  I don't know if I accidentally
left out that intermediate step in my OP or if the behavior has changed
since then.  As an alternative, replacing all the slashes by dashes, so
it's just a long file name in ~/, and doing step 2 also puts point at
the end of the visual line.

>> 2017-06-01  Stephen Berman  <stephen.berman@gmx.net>
>>
>> 	Improve navigation through minibuffer history
>>
>> 	* lisp/simple.el (previous-line-or-history-element): If the
>> 	element extends beyond window-width, go to its logical end,
>> 	since that is the natural target for editing history elements.
>> 	If it consists of a single line, move by logical lines to hit
>> 	beginning-of-buffer immediately and get the previous history
>> 	element.  Otherwise, move by visual lines.
>>
>
> Your change makes sense, I think, but when I tried applying the two
> patches, they no longer applied to Emacs 28, so I couldn't give it a
> try.  Could you possibly respin the patches?

Thanks for taking a look at this.  Here are the changes adjusted to
apply to current master:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch 1 --]
[-- Type: text/x-patch, Size: 3042 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index fa6e154004..96b56815cd 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2355,7 +2355,16 @@ next-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (next-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; end-of-buffer immediately and get the next history
+	  ;; element.  Otherwise, move by visual lines.
+	  (if (and (save-excursion
+		     (end-of-line)
+		     (> (current-column) (window-width)))
+		   (= (line-number-at-pos) 1))
+	      (next-logical-line arg)
+	    (next-line arg))
       (end-of-buffer
        ;; Restore old position since `line-move-visual' moves point to
        ;; the end of the line when it fails to go to the next line.
@@ -2396,7 +2405,16 @@ previous-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (previous-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; beginning-of-buffer immediately and get the previous
+	  ;; history element.  Otherwise, move by visual lines.
+	  (if (and (save-excursion
+		     (end-of-line)
+		     (> (current-column) (window-width)))
+		   (= (line-number-at-pos) 1))
+	      (previous-logical-line arg)
+	    (previous-line arg))
       (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.
@@ -2416,17 +2434,14 @@ previous-line-or-history-element
 				    (goto-char (1- (minibuffer-prompt-end)))
 				    (current-column))))
 	     (move-to-column old-column))
-	 (if (not line-move-visual) ; Handle logical lines (bug#42862)
-	     (end-of-line)
-	   ;; Put the cursor at the end of the visual line instead of the
-	   ;; logical line, so the next `previous-line-or-history-element'
-	   ;; would move to the previous history element, not to a possible upper
-	   ;; visual line from the end of logical line in `line-move-visual' mode.
-	   (end-of-visual-line)
-	   ;; Since `end-of-visual-line' puts the cursor at the beginning
-	   ;; of the next visual line, move it one char back to the end
-	   ;; of the first visual line (bug#22544).
-	   (unless (eolp) (backward-char 1))))))))
+	   ;; Put the cursor at the end of the logical line, even if
+           ;; it extends beyond window-width, since that is the
+           ;; natural target for editing history elements (bug#27191).
+           ;; The condition above makes sure the next
+           ;; `previous-line-or-history-element' will move to the
+           ;; previous history element regardless of the value of
+           ;; `line-move-visual'.
+           (end-of-line)))))))

 (defun next-complete-history-element (n)
   "Get next history element that completes the minibuffer before the point.

[-- Attachment #3: Type: text/plain, Size: 890 bytes --]


This patch also subsumes the recent change to fix bug#42862.

While this seems to work well for file-name-history and
buffer-name-history, whose elements typically don't contain a line feed
character, that's not the case for e.g. extended-command-history: the
above changes don't actually test that the history element consists of a
*single* line longer than window-width, but only whether the *first*
line of the element was longer than window-width.  So if the element
contains two or more such lines, as in:

M-: (setq bla "A long long long long long long long long long long long long long long long value"
          bli "Another long long long long long long long long long long long long long long long value")

<up> moves by visual lines through the lower ones but by logical lines
through the top one, which seems wrong.  I tried to accommodate such
cases, and came up with this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: patch 2 --]
[-- Type: text/x-patch, Size: 3452 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index fa6e154004..c2b55997b4 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2355,7 +2355,21 @@ next-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (next-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; end-of-buffer immediately and get the next history
+	  ;; element.  Otherwise, move by visual lines.
+	  (let ((beg (save-excursion
+		       (goto-char (point-min))
+		       (line-number-at-pos)))
+		(end (save-excursion
+		       (goto-char (point-max))
+                       (line-number-at-pos))))
+            (if (or (= beg end)
+		    (and (= (line-end-position) (buffer-end 1))
+			 (> (current-column) (window-width))))
+		(next-logical-line arg)
+	      (next-line arg))))
       (end-of-buffer
        ;; Restore old position since `line-move-visual' moves point to
        ;; the end of the line when it fails to go to the next line.
@@ -2396,7 +2410,21 @@ previous-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (previous-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; beginning-of-buffer immediately and get the previous
+	  ;; history element.  Otherwise, move by visual lines.
+	  (let ((beg (save-excursion
+		       (goto-char (point-min))
+		       (line-number-at-pos)))
+		(end (save-excursion
+		       (goto-char (point-max))
+                       (line-number-at-pos))))
+	    (if (or (= beg end)
+		    (and (= (line-beginning-position) (buffer-end -1))
+			 (> (current-column) (window-width))))
+		(previous-logical-line arg)
+	      (previous-line arg))))
       (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.
@@ -2416,17 +2444,14 @@ previous-line-or-history-element
 				    (goto-char (1- (minibuffer-prompt-end)))
 				    (current-column))))
 	     (move-to-column old-column))
-	 (if (not line-move-visual) ; Handle logical lines (bug#42862)
-	     (end-of-line)
-	   ;; Put the cursor at the end of the visual line instead of the
-	   ;; logical line, so the next `previous-line-or-history-element'
-	   ;; would move to the previous history element, not to a possible upper
-	   ;; visual line from the end of logical line in `line-move-visual' mode.
-	   (end-of-visual-line)
-	   ;; Since `end-of-visual-line' puts the cursor at the beginning
-	   ;; of the next visual line, move it one char back to the end
-	   ;; of the first visual line (bug#22544).
-	   (unless (eolp) (backward-char 1))))))))
+	   ;; Put the cursor at the end of the logical line, even if
+           ;; it extends beyond window-width, since that is the
+           ;; natural target for editing history elements (bug#27191).
+           ;; The condition above makes sure the next
+           ;; `previous-line-or-history-element' will move to the
+           ;; previous history element regardless of the value of
+           ;; `line-move-visual'.
+           (end-of-line)))))))

 (defun next-complete-history-element (n)
   "Get next history element that completes the minibuffer before the point.

[-- Attachment #5: Type: text/plain, Size: 492 bytes --]


With this, <up> and <down> move by visual lines through multline history
elements like the above example, and then <up> on the first line or
<down> on the last brings up the next history element.  But going back
and forth between history elements changes the goal-column when the
elements have different lengths.  In fact, the current implementations
of next-line-or-history-element and previous-line-or-history-element
have the same problem.  This can be prevented by setting goal-column:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: patch 2 --]
[-- Type: text/x-patch, Size: 3714 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index fa6e154004..228924f65a 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2355,7 +2355,24 @@ next-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (next-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; end-of-buffer immediately and get the next history
+	  ;; element.  Otherwise, move by visual lines.
+	  (let ((beg (save-excursion
+		       (goto-char (point-min))
+		       (line-number-at-pos)))
+		(end (save-excursion
+		       (goto-char (point-max))
+                       (line-number-at-pos)))
+		(col (save-excursion
+		       (goto-char (minibuffer-prompt-end))
+		       (current-column))))
+            (if (or (= beg end)
+		    (and (= (line-end-position) (buffer-end 1))
+			 (> (current-column) (window-width))))
+		(progn (setq goal-column col) (next-logical-line arg))
+	    (next-line arg))))
       (end-of-buffer
        ;; Restore old position since `line-move-visual' moves point to
        ;; the end of the line when it fails to go to the next line.
@@ -2396,7 +2413,24 @@ previous-line-or-history-element
 			 (current-column)))))
     (condition-case nil
 	(with-no-warnings
-	  (previous-line arg))
+	  ;; If the history element consists of a single line longer
+	  ;; than window-width, move by logical lines to hit
+	  ;; beginning-of-buffer immediately and get the previous
+	  ;; history element.  Otherwise, move by visual lines.
+	  (let ((beg (save-excursion
+		       (goto-char (point-min))
+		       (line-number-at-pos)))
+		(end (save-excursion
+		       (goto-char (point-max))
+                       (line-number-at-pos)))
+		(col (save-excursion
+		       (goto-char (minibuffer-prompt-end))
+		       (current-column))))
+	    (if (or (= beg end)
+		    (and (= (line-beginning-position) (buffer-end -1))
+			 (> (current-column) (window-width))))
+		(progn (setq goal-column col) (previous-logical-line arg))
+	    (previous-line arg))))
       (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.
@@ -2416,17 +2450,14 @@ previous-line-or-history-element
 				    (goto-char (1- (minibuffer-prompt-end)))
 				    (current-column))))
 	     (move-to-column old-column))
-	 (if (not line-move-visual) ; Handle logical lines (bug#42862)
-	     (end-of-line)
-	   ;; Put the cursor at the end of the visual line instead of the
-	   ;; logical line, so the next `previous-line-or-history-element'
-	   ;; would move to the previous history element, not to a possible upper
-	   ;; visual line from the end of logical line in `line-move-visual' mode.
-	   (end-of-visual-line)
-	   ;; Since `end-of-visual-line' puts the cursor at the beginning
-	   ;; of the next visual line, move it one char back to the end
-	   ;; of the first visual line (bug#22544).
-	   (unless (eolp) (backward-char 1))))))))
+	   ;; Put the cursor at the end of the logical line, even if
+           ;; it extends beyond window-width, since that is the
+           ;; natural target for editing history elements (bug#27191).
+           ;; The condition above makes sure the next
+           ;; `previous-line-or-history-element' will move to the
+           ;; previous history element regardless of the value of
+           ;; `line-move-visual'.
+           (end-of-line)))))))

 (defun next-complete-history-element (n)
   "Get next history element that completes the minibuffer before the point.

[-- Attachment #7: Type: text/plain, Size: 836 bytes --]


However, setting goal-column disables line-move-visual, so <up> and
<down> only move by logical lines through the multiline elements.  I
also tried let-binding goal-column instead of setting it, but that had
no effect, i.e., same as the second patch above.

One difference between navigating through file-name-history or
buffer-name-history and navigating through extended-command-history is
that with the former, points is always at the end of the history element
(with my proposed change, otherwise at least at the end of a visual
line), while with the latter, point starts out at the beginning of the
history element.  The goal-column problem seems likely to be related to
this difference, but I currently don't have a solution for it.  So until
someone comes up with one, it may be best to stick with the status quo.

Steve Berman

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

* bug#27191: 26.0.50; Long history items in minibuffer (again)
  2020-08-25 18:01   ` Stephen Berman
@ 2020-08-25 19:17     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 6+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-25 19:17 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 27191

Stephen Berman <stephen.berman@gmx.net> writes:

> So until someone comes up with one, it may be best to stick with the
> status quo.

Hm, yes, this looks like a difficult problem to get just right...

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





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

end of thread, other threads:[~2020-08-25 19:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-01 20:46 bug#27191: 26.0.50; Long history items in minibuffer (again) Stephen Berman
2017-06-02  8:01 ` Stephen Berman
2017-06-02 21:17   ` Stephen Berman
2020-08-24 14:38 ` Lars Ingebrigtsen
2020-08-25 18:01   ` Stephen Berman
2020-08-25 19:17     ` Lars Ingebrigtsen

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

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

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