From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stephen Berman Newsgroups: gmane.emacs.bugs Subject: bug#27191: 26.0.50; Long history items in minibuffer (again) Date: Tue, 25 Aug 2020 20:01:47 +0200 Message-ID: <87bliyli44.fsf@gmx.net> References: <874lvzh1wo.fsf@rosalinde> <87ft8c9kj9.fsf@gnus.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="8798"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: 27191@debbugs.gnu.org To: Lars Ingebrigtsen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Aug 25 20:04:14 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kAdIr-0002A1-QJ for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 25 Aug 2020 20:04:14 +0200 Original-Received: from localhost ([::1]:35084 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kAdIq-0006U9-Kq for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 25 Aug 2020 14:04:12 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:47732) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kAdGl-00041x-DN for bug-gnu-emacs@gnu.org; Tue, 25 Aug 2020 14:02:04 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:52653) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kAdGk-00044B-DR for bug-gnu-emacs@gnu.org; Tue, 25 Aug 2020 14:02:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kAdGk-0003L2-Ar for bug-gnu-emacs@gnu.org; Tue, 25 Aug 2020 14:02:02 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <874lvzh1wo.fsf@rosalinde> Resent-From: Stephen Berman Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 25 Aug 2020 18:02:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 27191 X-GNU-PR-Package: emacs Original-Received: via spool by 27191-submit@debbugs.gnu.org id=B27191.159837851812822 (code B ref 27191); Tue, 25 Aug 2020 18:02:02 +0000 Original-Received: (at 27191) by debbugs.gnu.org; 25 Aug 2020 18:01:58 +0000 Original-Received: from localhost ([127.0.0.1]:35966 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kAdGf-0003Kj-MQ for submit@debbugs.gnu.org; Tue, 25 Aug 2020 14:01:58 -0400 Original-Received: from mout.gmx.net ([212.227.15.15]:38985) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kAdGd-0003KU-LT for 27191@debbugs.gnu.org; Tue, 25 Aug 2020 14:01:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1598378509; bh=at5x4znFVpkJPj0kc9AcuxMhi1hnqTAIvtBijFMi988=; h=X-UI-Sender-Class:From:To:Cc:Subject:References:Date; b=Le3vlQfJ98BpzN9fckl+QF00Xei/rikR+X/5coiUzNrFIo27ED9zSqfKyyU1qhBfp Hk0WmdRbPoL15YNO9SoA7r4+/otRoAXU+jtXZYgTYTF4dx11eQTTgt45RZS2Zbkb3N zjE31w1xieKgB464U3Pv2r1X+u4f5noV4RS7FoUA= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Original-Received: from strobe-jhalfs ([188.109.196.40]) by mail.gmx.com (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MV67o-1k1Fd43c7q-00S39f; Tue, 25 Aug 2020 20:01:49 +0200 X-Provags-ID: V03:K1:lNhOZXX8JaHX8V1G+Vp8+gFzDbeB7psrcDhFTGerxtQ3X8XCLWS LAyVjW+XggAenjENgJ/SSug+E75aMji6GzqYZBuqwf/hUEwAf4DlilnRDxBQQ4GxP7sEZxG JXP1tnC2uK05ynQqzyMmgLP7ukPe0MzM3TTI9UI6Lo4POJdAxg5nqxhHBbeUibnsc896irw 4RZssexXtM/KvAMzuy3jA== X-UI-Out-Filterresults: notjunk:1;V03:K0:X4ksLXVRZYY=:47/zhh/1pi1SGYNj7dpBLW joJKWfVTWyXifvodAgj7L68+m7W8xtd39z9oXzZRei43LkwGirm/347svvupAL8PhcsTz+sEQ Uys5SkV5okYLOrmUtSo6OR1kfxtR8bPBzHbxLNAdR7ODOCrWNmiBI/o47IkIy2ZvPIe6gT+QB OycoANsQ8MkD2PCPogicAkSliFrAgqzbA2xmbxb/gWoo4UEHmvufBRdzAEmuhpXVYDuXomQuY 15HC2c3BwaoCPkOyy6HWlsOMhM12GOxCOwLyIdKcnYbg85pi48Xlu9rKqKivhghATvnuabn4R LtETRxRyL70RYtQC4D+OooVQ3Aej+eMfqg4tKGt2sl0ZVtX2IST+K2XF7bPQzbJxupb1hJWce /WW8FuepECp3U8ymZkXguo8eR8dNVSAza2+XERcXMedVIvdBt1BqzCcrBbyfr6B9V98YmNZlV Rzp6x5nW8jY1ROiOh1jWzwu+fKDO8LIuIuL/nVXIRi9dR4aDT7LP52JGZyCbJ1iTXBpO40y4Y 5NRieXDLsFbvIvlI6gV0afE0pnicGrT4dHu1QN4ss5dlUAw1PMG1t4y96y67wpqskMOm9oGKt wB+enZ2jEdtK20lTIIORPzBPT5Y52RHVqyPgwmnM/WzFVA21NW8vlh28T+WnGZwzpyRclv32M ow00tsKlLyCWwf+o2GOvHqMnhJ9m4eoCn3560dmU20yzZuHR5lVsa4kqczJYySkotJObmBcVK bYfo9XCc8pEhYeqXVNQSGWllvyudX6iG548VXXBETOCpMGzk8d2SJ1P7hkwOHBT0hinZ934w X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:186325 Archived-At: --=-=-= Content-Type: text/plain On Mon, 24 Aug 2020 16:38:02 +0200 Lars Ingebrigtsen wrote: > Stephen Berman writes: > >> 0. emacs -Q >> 1. C-x C-f >> ~/foo/bar/very/long/file/name/that/overflows/minibuffer/window/line/when/displayed >> >> 2. C-x C-f >> => 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 you type `M-p', then point is at the end of >> the file name in the minibuffer. This is what I expected for 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 > . :-/ 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 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 >> >> 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: --=-=-= Content-Type: text/x-patch Content-Disposition: attachment Content-Description: patch 1 Content-Transfer-Encoding: quoted-printable diff --git a/lisp/simple.el b/lisp/simple.el index fa6e154004..96b56815cd 100644 =2D-- 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))) + (=3D (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))) + (=3D (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 l= ine. @@ -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 uppe= r - ;; visual line from the end of logical line in `line-move-visual' mod= e. - (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 poin= t. --=-=-= Content-Type: text/plain 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") 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: --=-=-= Content-Type: text/x-patch Content-Disposition: attachment Content-Description: patch 2 Content-Transfer-Encoding: quoted-printable diff --git a/lisp/simple.el b/lisp/simple.el index fa6e154004..c2b55997b4 100644 =2D-- 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 (=3D beg end) + (and (=3D (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 (=3D beg end) + (and (=3D (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 l= ine. @@ -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 uppe= r - ;; visual line from the end of logical line in `line-move-visual' mod= e. - (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 poin= t. --=-=-= Content-Type: text/plain With this, and move by visual lines through multline history elements like the above example, and then on the first line or 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: --=-=-= Content-Type: text/x-patch Content-Disposition: attachment Content-Description: patch 2 Content-Transfer-Encoding: quoted-printable diff --git a/lisp/simple.el b/lisp/simple.el index fa6e154004..228924f65a 100644 =2D-- 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 (=3D beg end) + (and (=3D (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 (=3D beg end) + (and (=3D (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 l= ine. @@ -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 uppe= r - ;; visual line from the end of logical line in `line-move-visual' mod= e. - (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 poin= t. --=-=-= Content-Type: text/plain However, setting goal-column disables line-move-visual, so and 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 --=-=-=--