From: Stephen Berman <stephen.berman@gmx.net>
To: Lars Ingebrigtsen <larsi@gnus.org>
Cc: 27191@debbugs.gnu.org
Subject: bug#27191: 26.0.50; Long history items in minibuffer (again)
Date: Tue, 25 Aug 2020 20:01:47 +0200 [thread overview]
Message-ID: <87bliyli44.fsf@gmx.net> (raw)
In-Reply-To: <874lvzh1wo.fsf@rosalinde>
[-- 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
next prev parent reply other threads:[~2020-08-25 18:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2020-08-25 19:17 ` Lars Ingebrigtsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bliyli44.fsf@gmx.net \
--to=stephen.berman@gmx.net \
--cc=27191@debbugs.gnu.org \
--cc=larsi@gnus.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).