Select completions from the minibuffer

Select completions from the minibuffer

[Juri Linkov]

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

Here is a non-intrusive patch that doesn't change the default behavior
while provides navigation of completions from the minibuffer
like web browsers do on their address bar.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: minibuffer-completion-next-previous.patch --]
[-- Type: text/x-diff, Size: 2313 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 36b8d80841..e92bb0f885 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2681,7 +2766,10 @@ minibuffer-local-completion-map
   "?"         #'minibuffer-completion-help
   "<prior>"   #'switch-to-completions
   "M-v"       #'switch-to-completions
-  "M-g M-c"   #'switch-to-completions)
+  "M-g M-c"   #'switch-to-completions
+  "M-<up>"    #'minibuffer-completion-previous
+  "M-<down>"  #'minibuffer-completion-next
+  "M-RET"     #'minibuffer-completion-choose)
 
 (defvar-keymap minibuffer-local-must-match-map
   :doc "Local keymap for minibuffer input with completion, for exact match."
@@ -4271,6 +4359,39 @@ minibuffer-scroll-other-window-down
   (with-minibuffer-selected-window
     (scroll-other-window-down arg)))
 
+(defmacro with-minibuffer-scroll-window (&rest body)
+  "Execute the forms in BODY from the minibuffer in its scrolling window.
+When used in a minibuffer window, select the window where scrolling command
+should scroll, and execute the forms."
+  (declare (indent 0) (debug t))
+  `(let ((window (or (get-buffer-window "*Completions*" 0) ; remove?
+		     ;; Make sure we have a completions window.
+                     (progn (minibuffer-completion-help)
+                            (get-buffer-window "*Completions*" 0)))))
+     (when window
+       (with-selected-window window
+         ,@body))))
+
+(defun minibuffer-completion-previous (&optional arg)
+  "Run `previous-completion' from the minibuffer in its scrolling window."
+  (interactive "P")
+  (with-minibuffer-scroll-window
+    (previous-completion (or arg 1))
+    (choose-completion nil t t)))
+
+(defun minibuffer-completion-next (&optional arg)
+  "Run `next-completion' from the minibuffer in its scrolling window."
+  (interactive "P")
+  (with-minibuffer-scroll-window
+    (next-completion (or arg 1))
+    (choose-completion nil t t)))
+
+(defun minibuffer-completion-choose (&optional arg)
+  "Run `choose-completion' from the minibuffer in its scrolling window."
+  (interactive "P")
+  (with-minibuffer-scroll-window
+    (choose-completion nil arg)))
+
 (defcustom minibuffer-default-prompt-format " (default %s)"
   "Format string used to output \"default\" values.
 When prompting for input, there will often be a default value,

Re: Select completions from the minibuffer

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Here is a non-intrusive patch that doesn't change the default behavior
> while provides navigation of completions from the minibuffer
> like web browsers do on their address bar.

[...]

> +  "M-<up>"    #'minibuffer-completion-previous
> +  "M-<down>"  #'minibuffer-completion-next
> +  "M-RET"     #'minibuffer-completion-choose)

If I'm reading this patch correctly, then this looks like just what I've
wanted for decades.  😀

[...]

> +    (choose-completion nil t t)))

But this doesn't seem quite correct?  The function only takes a single
parameter...  Or is that part of the patch missing?

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

Re: Select completions from the minibuffer

[Juri Linkov]

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

>> Here is a non-intrusive patch that doesn't change the default behavior
>> while provides navigation of completions from the minibuffer
>> like web browsers do on their address bar.
>
> [...]
>
>> +  "M-<up>"    #'minibuffer-completion-previous
>> +  "M-<down>"  #'minibuffer-completion-next
>> +  "M-RET"     #'minibuffer-completion-choose)
>
> If I'm reading this patch correctly, then this looks like just what I've
> wanted for decades.  😀
>
> [...]
>
>> +    (choose-completion nil t t)))
>
> But this doesn't seem quite correct?  The function only takes a single
> parameter...  Or is that part of the patch missing?

Oops, it depends on another patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: choose-completion-no-auto-exit.patch --]
[-- Type: text/x-diff, Size: 2702 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index accc119e2b..65fc2c94a3 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9039,6 +9039,7 @@ completion-list-mode-map
     (define-key map [follow-link] 'mouse-face)
     (define-key map [down-mouse-2] nil)
     (define-key map "\C-m" 'choose-completion)
+    (define-key map [C-return] 'choose-completion-no-auto-exit)
     (define-key map "\e\e\e" 'delete-completion-window)
     (define-key map [remap keyboard-quit] #'delete-completion-window)
     (define-key map [left] 'previous-completion)
@@ -9151,10 +9152,12 @@ next-completion
     (when (/= 0 n)
       (switch-to-minibuffer))))
 
-(defun choose-completion (&optional event)
+(defun choose-completion (&optional event no-exit no-quit)
   "Choose the completion at point.
-If EVENT, use EVENT's position to determine the starting position."
-  (interactive (list last-nonmenu-event))
+If EVENT, use EVENT's position to determine the starting position.
+With the prefix NO-EXIT, insert the completion at point to the minibuffer
+and quit the completion window without exiting the minibuffer."
+  (interactive (list last-nonmenu-event current-prefix-arg))
   ;; In case this is run via the mouse, give temporary modes such as
   ;; isearch a chance to turn off.
   (run-hooks 'mouse-leave-buffer-hook)
@@ -9162,6 +9165,7 @@ choose-completion
     (let ((buffer completion-reference-buffer)
           (base-position completion-base-position)
           (insert-function completion-list-insert-choice-function)
+          (completion-no-auto-exit (if no-exit t completion-no-auto-exit))
           (choice
            (save-excursion
              (goto-char (posn-point (event-start event)))
@@ -9179,7 +9184,8 @@ choose-completion
 
       (unless (buffer-live-p buffer)
         (error "Destination buffer is dead"))
-      (quit-window nil (posn-window (event-start event)))
+      (unless no-quit
+        (quit-window nil (posn-window (event-start event))))
 
       (with-current-buffer buffer
         (choose-completion-string
@@ -9189,6 +9195,14 @@ choose-completion
              (list (choose-completion-guess-base-position choice)))
          insert-function)))))
 
+(defun choose-completion-no-auto-exit (&optional event)
+  "Insert the completion at point to the minibuffer without exiting it.
+Like `choose-completion', it chooses the completion at point,
+inserts it to the minibuffer, but doesn't exit the minibuffer."
+  (interactive (list last-nonmenu-event))
+  (let ((completion-no-auto-exit t))
+    (choose-completion event)))
+
 ;; Delete the longest partial match for STRING
 ;; that can be found before POINT.
 (defun choose-completion-guess-base-position (string)

RE: [External] : Re: Select completions from the minibuffer

[Drew Adams]

> > +  "M-<up>"    #'minibuffer-completion-previous
> > +  "M-<down>"  #'minibuffer-completion-next
> > +  "M-RET"     #'minibuffer-completion-choose)
> 
> If I'm reading this patch correctly, then this looks 
> like just what I've wanted for decades.  😀

In Icicles for over 2 decades.

<up>, <down>, RET, by default - no need for `M-'.

(And keys customizable with just a simple option.)
___

Suggested for vanilla Emacs long ago - no secret.
It's the most basic thing Icicles ever offered:
cycling, with the current candidate highlighted in
*Completions* and inserted as minibuffer content.

Re: Select completions from the minibuffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 10 Mar 2022 20:58:39 +0200
> 
> +  "M-RET"     #'minibuffer-completion-choose)

This one is a bad idea, IMO, as some WMs use this for their own
purposes, and on Windows it will cause an even worse effects.

Re: [External] : Re: Select completions from the minibuffer

[Juri Linkov]

>> > +  "M-<up>"    #'minibuffer-completion-previous
>> > +  "M-<down>"  #'minibuffer-completion-next
>> > +  "M-RET"     #'minibuffer-completion-choose)
>> 
>> If I'm reading this patch correctly, then this looks 
>> like just what I've wanted for decades.  😀
>
> In Icicles for over 2 decades.
>
> <up>, <down>, RET, by default - no need for `M-'.
>
> (And keys customizable with just a simple option.)

Obviously <up>, <down> can't by used without modifiers by default.
So in the next patch I intended to add an option to use these keys
without modifiers.

Re: Select completions from the minibuffer

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> +  "M-RET"     #'minibuffer-completion-choose)
>
> This one is a bad idea, IMO, as some WMs use this for their own
> purposes, and on Windows it will cause an even worse effects.

M-RET is bound in a whole bunch of modes already, though.  So it
apparently works well enough that people don't complain.  If it's an
issue on Windows, then perhaps it should be translated to something else
there?

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

Re: Select completions from the minibuffer

[Lars Ingebrigtsen]

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

Juri Linkov <juri@linkov.net> writes:

> Oops, it depends on another patch:

Thanks, with that it compiles, but doesn't quite seem to do the right
thing.

emacs -Q
f TAB M-<down> M-<down>

gives me


[-- Attachment #2: Type: image/png, Size: 9836 bytes --]

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


I.e., it prepends the choice instead of replacing it.

But it's otherwise what I want.  😃

Perhaps it should also highlight the current choice in the
*Completions* buffer?  It moves point in the buffer, but that's a
too-subtle hint, I think.

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

Re: Select completions from the minibuffer

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Juri Linkov <juri@linkov.net>,  emacs-devel@gnu.org
> Date: Thu, 10 Mar 2022 21:23:31 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> +  "M-RET"     #'minibuffer-completion-choose)
> >
> > This one is a bad idea, IMO, as some WMs use this for their own
> > purposes, and on Windows it will cause an even worse effects.
> 
> M-RET is bound in a whole bunch of modes already, though.  So it
> apparently works well enough that people don't complain.  If it's an
> issue on Windows, then perhaps it should be translated to something else
> there?

I don't think it's possible, at least not easily.

RE: [External] : Re: Select completions from the minibuffer

[Drew Adams]

> > <up>, <down>, RET, by default - no need for `M-'.
> 
> Obviously <up>, <down> can't by used without modifiers by default.

Maybe you say that because (inheriting from
`minibuffer-local-map') they default to
`(next|previous)-line-or-history-element'?

`M-n' and `M-p' are sufficient for that, IMO.
`C-n' and `C-p' are enough for moving down/up
lines of multiline input. 

Re: Select completions from the minibuffer

[Juri Linkov]

> Thanks, with that it compiles,

Actually, it's an improved version of the patch from bug#47417.

> but doesn't quite seem to do the right thing.
>
> emacs -Q
> f TAB M-<down> M-<down>

> I.e., it prepends the choice instead of replacing it.
>
> But it's otherwise what I want.  😃

Oh, this is bug#34517 and bug#49931.  So we need to finish fixing this now.

> Perhaps it should also highlight the current choice in the
> *Completions* buffer?  It moves point in the buffer, but that's a
> too-subtle hint, I think.

Ergus already added completions-highlight-mode
in the feature/completions-customs branch.
But maybe it could enabled by default?

Re: Select completions from the minibuffer

[Ergus]

On Thu, Mar 10, 2022 at 09:32:31PM +0100, Lars Ingebrigtsen wrote:
>Juri Linkov <juri@linkov.net> writes:
>
>> Oops, it depends on another patch:
>
>Thanks, with that it compiles, but doesn't quite seem to do the right
>thing.
>
>emacs -Q
>f TAB M-<down> M-<down>
>
>gives me
>


>
>I.e., it prepends the choice instead of replacing it.
>
>But it's otherwise what I want.  😃
>
>Perhaps it should also highlight the current choice in the
>*Completions* buffer?  It moves point in the buffer, but that's a
>too-subtle hint, I think.
>
Hi Lars:

I added a highlight mode for completions in my branch using hooks and
overlays, but I am wondering since then: why we don't have a text
property that highlight the region when the cursor is over the region,
like the `mouse-face` but for the cursor (point)... Is it possible to
implement such thing more or less easily?

if so, then it will be easier and simpler (and probably more efficient
without requiring hooks and overlays).

WDYT?
Best,
Ergus

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

Re: Select completions from the minibuffer

[Lars Ingebrigtsen]

Ergus <spacibba@aol.com> writes:

> I added a highlight mode for completions in my branch using hooks and
> overlays, but I am wondering since then: why we don't have a text
> property that highlight the region when the cursor is over the region,
> like the `mouse-face` but for the cursor (point)... Is it possible to
> implement such thing more or less easily?

Hm, that does sound kind of attractive.  I don't know whether it'd be a
lot of work to implement -- Eli?

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

Re: Select completions from the minibuffer

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Oh, this is bug#34517 and bug#49931.  So we need to finish fixing this now.

Ah -- I've also seen that weird behaviour sometimes, but never pinned
down why it's happening.

I think clicking something in *Completions* should select that, and
ignore what the user has (probably accidentally) typed into the
minibuffer after TAB-ing.  (Perhaps with a user option?)

>> Perhaps it should also highlight the current choice in the
>> *Completions* buffer?  It moves point in the buffer, but that's a
>> too-subtle hint, I think.
>
> Ergus already added completions-highlight-mode
> in the feature/completions-customs branch.
> But maybe it could enabled by default?

Or perhaps `M-<up>' etc should enable it?

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

Re: Select completions from the minibuffer

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Juri Linkov <juri@linkov.net>,  Eli Zaretskii <eliz@gnu.org>,
>   emacs-devel@gnu.org
> Date: Sat, 12 Mar 2022 18:04:54 +0100
> 
> Ergus <spacibba@aol.com> writes:
> 
> > I added a highlight mode for completions in my branch using hooks and
> > overlays, but I am wondering since then: why we don't have a text
> > property that highlight the region when the cursor is over the region,
> > like the `mouse-face` but for the cursor (point)... Is it possible to
> > implement such thing more or less easily?
> 
> Hm, that does sound kind of attractive.  I don't know whether it'd be a
> lot of work to implement -- Eli?

Isn't it very similar to the region?

Re: Select completions from the minibuffer

[Ergus]

On March 12, 2022 6:29:57 PM GMT+01:00, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: Juri Linkov <juri@linkov.net>,  Eli Zaretskii <eliz@gnu.org>,
>>   emacs-devel@gnu.org
>> Date: Sat, 12 Mar 2022 18:04:54 +0100
>> 
>> Ergus <spacibba@aol.com> writes:
>> 
>> > I added a highlight mode for completions in my branch using hooks and
>> > overlays, but I am wondering since then: why we don't have a text
>> > property that highlight the region when the cursor is over the region,
>> > like the `mouse-face` but for the cursor (point)... Is it possible to
>> > implement such thing more or less easily?
>> 
>> Hm, that does sound kind of attractive.  I don't know whether it'd be a
>> lot of work to implement -- Eli?
>
>Isn't it very similar to the region?
>

I don't think so... I think it is more similar to the mouse-face feature / text property.

BTW completions already have mouse-face but it doesn't work in terminal+xterm mouse mode... But that's a different issue I guess.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Sat, 12 Mar 2022 18:37:14 +0100
> From: Ergus <spacibba@aol.com>
> CC: juri@linkov.net
> 
> 
> 
> On March 12, 2022 6:29:57 PM GMT+01:00, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Lars Ingebrigtsen <larsi@gnus.org>
> >> Cc: Juri Linkov <juri@linkov.net>,  Eli Zaretskii <eliz@gnu.org>,
> >>   emacs-devel@gnu.org
> >> Date: Sat, 12 Mar 2022 18:04:54 +0100
> >> 
> >> Ergus <spacibba@aol.com> writes:
> >> 
> >> > I added a highlight mode for completions in my branch using hooks and
> >> > overlays, but I am wondering since then: why we don't have a text
> >> > property that highlight the region when the cursor is over the region,
> >> > like the `mouse-face` but for the cursor (point)... Is it possible to
> >> > implement such thing more or less easily?
> >> 
> >> Hm, that does sound kind of attractive.  I don't know whether it'd be a
> >> lot of work to implement -- Eli?
> >
> >Isn't it very similar to the region?
> >
> 
> I don't think so... I think it is more similar to the mouse-face feature / text property.

Why do you think so?  The region is redrawn when the cursor moves, and
the cursor moves as result of Emacs commands, so how is this very
different from what you want?  By contrast, the mouse highlight is
triggered by mouse movements that are completely independent of Emacs
commands, and the implementation, as part of the display engine, is
therefore quite complicated.  Whereas the region display is nowadays
implemented completely in Lisp.

Re: Select completions from the minibuffer

[Juri Linkov]

>> >> +  "M-RET"     #'minibuffer-completion-choose)
>> >
>> > This one is a bad idea, IMO, as some WMs use this for their own
>> > purposes, and on Windows it will cause an even worse effects.
>>
>> M-RET is bound in a whole bunch of modes already, though.  So it
>> apparently works well enough that people don't complain.  If it's an
>> issue on Windows, then perhaps it should be translated to something else
>> there?
>
> I don't think it's possible, at least not easily.

On Microsoft Windows another set of keys could be used:
`C-up', `C-down' and `C-return'.

Re: Select completions from the minibuffer

[Juri Linkov]

>> Oh, this is bug#34517 and bug#49931.  So we need to finish fixing this now.
>
> Ah -- I've also seen that weird behaviour sometimes, but never pinned
> down why it's happening.
>
> I think clicking something in *Completions* should select that, and
> ignore what the user has (probably accidentally) typed into the
> minibuffer after TAB-ing.  (Perhaps with a user option?)

I posted a patch to bug#49931 that adds a new user option.
Maybe it could be enabled by default?

>>> Perhaps it should also highlight the current choice in the
>>> *Completions* buffer?  It moves point in the buffer, but that's a
>>> too-subtle hint, I think.
>>
>> Ergus already added completions-highlight-mode
>> in the feature/completions-customs branch.
>> But maybe it could enabled by default?
>
> Or perhaps `M-<up>' etc should enable it?

`M-<up>' in bug#49931 let-binds the new user option,
so it could let-bind completions-highlight-mode as well.

RE: [External] : Re: Select completions from the minibuffer

[Drew Adams]

> why we don't have a text property that highlight the
> region when the cursor is over the region, like the
> `mouse-face` but for the cursor (point)

The cursor is _always_ "over" the region (as much
as it ever can be "over" the region - point is a
region limit).

Re: Select completions from the minibuffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  emacs-devel@gnu.org
> Date: Sat, 12 Mar 2022 20:52:56 +0200
> 
> >> >> +  "M-RET"     #'minibuffer-completion-choose)
> >> >
> >> > This one is a bad idea, IMO, as some WMs use this for their own
> >> > purposes, and on Windows it will cause an even worse effects.
> >>
> >> M-RET is bound in a whole bunch of modes already, though.  So it
> >> apparently works well enough that people don't complain.  If it's an
> >> issue on Windows, then perhaps it should be translated to something else
> >> there?
> >
> > I don't think it's possible, at least not easily.
> 
> On Microsoft Windows another set of keys could be used:
> `C-up', `C-down' and `C-return'.

That is even worse.

Re: Select completions from the minibuffer

[Ergus]

On March 12, 2022 7:14:10 PM GMT+01:00, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Sat, 12 Mar 2022 18:37:14 +0100
>> From: Ergus <spacibba@aol.com>
>> CC: juri@linkov.net
>> 
>> 
>> 
>> On March 12, 2022 6:29:57 PM GMT+01:00, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> From: Lars Ingebrigtsen <larsi@gnus.org>
>> >> Cc: Juri Linkov <juri@linkov.net>,  Eli Zaretskii <eliz@gnu.org>,
>> >>   emacs-devel@gnu.org
>> >> Date: Sat, 12 Mar 2022 18:04:54 +0100
>> >> 
>> >> Ergus <spacibba@aol.com> writes:
>> >> 
>> >> > I added a highlight mode for completions in my branch using hooks and
>> >> > overlays, but I am wondering since then: why we don't have a text
>> >> > property that highlight the region when the cursor is over the region,
>> >> > like the `mouse-face` but for the cursor (point)... Is it possible to
>> >> > implement such thing more or less easily?
>> >> 
>> >> Hm, that does sound kind of attractive.  I don't know whether it'd be a
>> >> lot of work to implement -- Eli?
>> >
>> >Isn't it very similar to the region?
>> >
>> 
>> I don't think so... I think it is more similar to the mouse-face feature / text property.
>
>Why do you think so?  The region is redrawn when the cursor moves, and
>the cursor moves as result of Emacs commands, so how is this very
>different from what you want?  By contrast, the mouse highlight is
>triggered by mouse movements that are completely independent of Emacs
>commands, and the implementation, as part of the display engine, is
>therefore quite complicated.  Whereas the region display is nowadays
>implemented completely in Lisp.

Ok, then it will be similar to the code I already have in completions-highlight-mode. Essentially add/move an overlay on post command hook based on some conditions.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Sat, 12 Mar 2022 20:30:35 +0100
> From: Ergus <spacibba@aol.com>
> CC: emacs-devel@gnu.org, larsi@gnus.org, juri@linkov.net
> 
> >> I don't think so... I think it is more similar to the mouse-face feature / text property.
> >
> >Why do you think so?  The region is redrawn when the cursor moves, and
> >the cursor moves as result of Emacs commands, so how is this very
> >different from what you want?  By contrast, the mouse highlight is
> >triggered by mouse movements that are completely independent of Emacs
> >commands, and the implementation, as part of the display engine, is
> >therefore quite complicated.  Whereas the region display is nowadays
> >implemented completely in Lisp.
> 
> Ok, then it will be similar to the code I already have in completions-highlight-mode. Essentially add/move an overlay on post command hook based on some conditions.

Actually, I think it should be like the implementation of the region
highlight, which uses other hooks.  See simple.el.

Re: Select completions from the minibuffer

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> I posted a patch to bug#49931 that adds a new user option.
> Maybe it could be enabled by default?

Skimming the patch, I think that might make sense.  But perhaps push it
disabled first, and then we can gain some experience with it, and then
enable it by default if there's no unforeseen side effects?

> `M-<up>' in bug#49931 let-binds the new user option,
> so it could let-bind completions-highlight-mode as well.

Yes.

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

Re: Select completions from the minibuffer

[Juri Linkov]

>>> >Isn't it very similar to the region?
>>>
>>> I don't think so... I think it is more similar to the mouse-face
>>> feature / text property.
>>
>>Why do you think so?  The region is redrawn when the cursor moves, and
>>the cursor moves as result of Emacs commands, so how is this very
>>different from what you want?  By contrast, the mouse highlight is
>>triggered by mouse movements that are completely independent of Emacs
>>commands, and the implementation, as part of the display engine, is
>>therefore quite complicated.  Whereas the region display is nowadays
>>implemented completely in Lisp.
>
> Ok, then it will be similar to the code I already have in
> completions-highlight-mode.  Essentially add/move an overlay on post
> command hook based on some conditions.

You could just add your function to the default value of
pre-redisplay-functions, e.g.

  (defvar pre-redisplay-functions (list #'redisplay--update-region-highlight
                                        #'redisplay--update-mouse-face)

where the new function 'redisplay--update-mouse-face'
will activate the mouse-face when the cursor moves
over the mouse-face property (only when an additional user option
will be enabled).

Re: Select completions from the minibuffer

[Juri Linkov]

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

>> I posted a patch to bug#49931 that adds a new user option.
>> Maybe it could be enabled by default?
>
> Skimming the patch, I think that might make sense.  But perhaps push it
> disabled first, and then we can gain some experience with it, and then
> enable it by default if there's no unforeseen side effects?

Yep, this would be the best course of action.

The new variable completion-use-base-affixes will be also enabled
for the M-up/down feature.  Here is an updated patch that also requires
other two patches from bug#47417 (merged with bug#49931) and bug#48356:


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

diff --git a/lisp/simple.el b/lisp/simple.el
index accc119e2b..52cf54c563 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -3891,6 +3891,9 @@ minibuffer-local-shell-command-map
   (let ((map (make-sparse-keymap)))
     (set-keymap-parent map minibuffer-local-map)
     (define-key map "\t" 'completion-at-point)
+    (define-key map (kbd "M-<up>") 'minibuffer-previous-completion)
+    (define-key map (kbd "M-<down>") 'minibuffer-next-completion)
+    (define-key map (kbd "M-RET") 'minibuffer-choose-completion)
     map)
   "Keymap used for completing shell commands in minibuffer.")
 
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 36b8d80841..5685f078ad 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2681,7 +2775,12 @@ minibuffer-local-completion-map
   "?"         #'minibuffer-completion-help
   "<prior>"   #'switch-to-completions
   "M-v"       #'switch-to-completions
-  "M-g M-c"   #'switch-to-completions)
+  "M-g M-c"   #'switch-to-completions
+  "M-S-<up>"    #'minibuffer-previous-completion
+  "M-S-<down>"  #'minibuffer-next-completion
+  "M-<up>"    #'minibuffer-choose-previous-completion
+  "M-<down>"  #'minibuffer-choose-next-completion
+  "M-RET"     #'minibuffer-choose-completion)
 
 (defvar-keymap minibuffer-local-must-match-map
   :doc "Local keymap for minibuffer input with completion, for exact match."
@@ -4271,6 +4370,52 @@ minibuffer-scroll-other-window-down
   (with-minibuffer-selected-window
     (scroll-other-window-down arg)))
 
+(defmacro with-minibuffer-completions-window (&rest body)
+  "Execute the forms in BODY from the minibuffer in its completions window.
+When used in a minibuffer window, select the window with completions,
+and execute the forms."
+  (declare (indent 0) (debug t))
+  `(let ((window (or (get-buffer-window "*Completions*" 0)
+		     ;; Make sure we have a completions window.
+                     (progn (minibuffer-completion-help)
+                            (get-buffer-window "*Completions*" 0)))))
+     (when window
+       (with-selected-window window
+         ,@body))))
+
+(defun minibuffer-previous-completion (&optional n)
+  "Run `previous-completion' from the minibuffer in its completions window."
+  (interactive "p")
+  (with-minibuffer-completions-window
+    (previous-completion n)))
+
+(defun minibuffer-next-completion (&optional n)
+  "Run `next-completion' from the minibuffer in its completions window."
+  (interactive "p")
+  (with-minibuffer-completions-window
+    (next-completion n)))
+
+(defun minibuffer-choose-previous-completion (&optional n)
+  "Run `previous-completion' from the minibuffer in its completions window.
+Also insert the selected completion to the minibuffer."
+  (interactive "p")
+  (minibuffer-previous-completion n)
+  (minibuffer-choose-completion t t))
+
+(defun minibuffer-choose-next-completion (&optional n)
+  "Run `next-completion' from the minibuffer in its completions window.
+Also insert the selected completion to the minibuffer."
+  (interactive "p")
+  (minibuffer-next-completion n)
+  (minibuffer-choose-completion t t))
+
+(defun minibuffer-choose-completion (&optional no-exit no-quit)
+  "Run `choose-completion' from the minibuffer in its completions window."
+  (interactive "p")
+  (with-minibuffer-completions-window
+    (let ((completion-use-base-affixes t))
+      (choose-completion nil no-exit no-quit))))
+
 (defcustom minibuffer-default-prompt-format " (default %s)"
   "Format string used to output \"default\" values.
 When prompting for input, there will often be a default value,

Re: Select completions from the minibuffer

[Ergus]

On Sun, Mar 13, 2022 at 07:47:11PM +0200, Juri Linkov wrote:
>>>> >Isn't it very similar to the region?
>>>>
>>>> I don't think so... I think it is more similar to the mouse-face
>>>> feature / text property.
>>>
>>>Why do you think so?  The region is redrawn when the cursor moves, and
>>>the cursor moves as result of Emacs commands, so how is this very
>>>different from what you want?  By contrast, the mouse highlight is
>>>triggered by mouse movements that are completely independent of Emacs
>>>commands, and the implementation, as part of the display engine, is
>>>therefore quite complicated.  Whereas the region display is nowadays
>>>implemented completely in Lisp.
>>
>> Ok, then it will be similar to the code I already have in
>> completions-highlight-mode.  Essentially add/move an overlay on post
>> command hook based on some conditions.
>
>You could just add your function to the default value of
>pre-redisplay-functions, e.g.
>
>  (defvar pre-redisplay-functions (list #'redisplay--update-region-highlight
>                                        #'redisplay--update-mouse-face)
>
>where the new function 'redisplay--update-mouse-face'
>will activate the mouse-face when the cursor moves
>over the mouse-face property (only when an additional user option
>will be enabled).

Sone, please check it... I will add the extra option in a moment.

Re: Select completions from the minibuffer

[Ergus]

On Sat, Mar 12, 2022 at 09:39:34PM +0200, Eli Zaretskii wrote:
>
>Actually, I think it should be like the implementation of the region
>highlight, which uses other hooks.  See simple.el.
>
Hi Eli:

Please check the commit in fd7bde6 in the feature/completions-option
branch.

Best,
Ergus

Re: Select completions from the minibuffer

[Juri Linkov]

>> `M-<up>' in bug#49931 let-binds the new user option,
>> so it could let-bind completions-highlight-mode as well.
>
> Yes.

Alas, after the last commit c1ea52f4ad in the feature/completions-customs branch
this is not possible anymore.

Before this commit we were able to let-bind
`cursor-face-highlight-mode' in the new command:

```
(defun minibuffer-previous-completion (&optional n)
  "Run `previous-completion' from the minibuffer in its completions window."
  (interactive "p")
  (with-minibuffer-completions-window
    (let ((cursor-face-highlight-mode t))
      (previous-completion n))))
```

because 'redisplay--update-cursor-face-highlight' was always
presented in `pre-redisplay-functions', and the mode was checked with

  (when cursor-face-highlight-mode

But now the hook is added only when mode is enabled.

Re: Select completions from the minibuffer

[Ergus]

Hi:

Now there is a variable completions-highlight-face. When nil highlight
is disabled, otherwise it must be a the face to use, by default
completions-highlight.

I had the mode before because I needed some initialization, but with the
cursor-face feature that is not needed and the code got simpler.

If anyone knows how to modify redisplay--update-cursor-face-highlight to
enable cursor-face to accept cons and plists (as the documentation of
mouse-face indicates it should) please just add it there.

On Mon, Mar 14, 2022 at 10:41:56AM +0200, Juri Linkov wrote:
>>> `M-<up>' in bug#49931 let-binds the new user option,
>>> so it could let-bind completions-highlight-mode as well.
>>
>> Yes.
>
>Alas, after the last commit c1ea52f4ad in the feature/completions-customs branch
>this is not possible anymore.
>
>Before this commit we were able to let-bind
>`cursor-face-highlight-mode' in the new command:
>
>```
>(defun minibuffer-previous-completion (&optional n)
>  "Run `previous-completion' from the minibuffer in its completions window."
>  (interactive "p")
>  (with-minibuffer-completions-window
>    (let ((cursor-face-highlight-mode t))
>      (previous-completion n))))
>```
>
>because 'redisplay--update-cursor-face-highlight' was always
>presented in `pre-redisplay-functions', and the mode was checked with
>
>  (when cursor-face-highlight-mode
>
>But now the hook is added only when mode is enabled.

And it is always added to completions during the setup when
completions-highlight-face is non-nil.

Re: Select completions from the minibuffer

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> The new variable completion-use-base-affixes will be also enabled
> for the M-up/down feature.  Here is an updated patch that also requires
> other two patches from bug#47417 (merged with bug#49931) and bug#48356:

It's a bit difficult to test this stuff when it's spread out over so
many patches -- perhaps you should start pushing the patches to the
trunk (in whatever order makes sense)?

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

Re: Select completions from the minibuffer

[Juri Linkov]

> Now there is a variable completions-highlight-face. When nil highlight
> is disabled, otherwise it must be a the face to use, by default
> completions-highlight.
>
> I had the mode before because I needed some initialization, but with the
> cursor-face feature that is not needed and the code got simpler.
>
> If anyone knows how to modify redisplay--update-cursor-face-highlight to
> enable cursor-face to accept cons and plists (as the documentation of
> mouse-face indicates it should) please just add it there.

Please clarify what cons and plists.  Where the documentation says
about cons and plists for mouse-face?

Re: Select completions from the minibuffer

[Juri Linkov]

> Now there is a variable completions-highlight-face. When nil highlight
> is disabled, otherwise it must be a the face to use, by default
> completions-highlight.
>
> I had the mode before because I needed some initialization, but with the
> cursor-face feature that is not needed and the code got simpler.

Thanks, highlighting works correctly when the current buffer is *Completions*.
But cursor-face is not highlighted in *Completions* when the current buffer
is the minibuffer.  Such highlighting is necessary when completions
are navigated from the minibuffer.

Re: Select completions from the minibuffer

[Ergus]

On Mon, Mar 14, 2022 at 08:19:47PM +0200, Juri Linkov wrote:
>> Now there is a variable completions-highlight-face. When nil highlight
>> is disabled, otherwise it must be a the face to use, by default
>> completions-highlight.
>>
>> I had the mode before because I needed some initialization, but with the
>> cursor-face feature that is not needed and the code got simpler.
>>
>> If anyone knows how to modify redisplay--update-cursor-face-highlight to
>> enable cursor-face to accept cons and plists (as the documentation of
>> mouse-face indicates it should) please just add it there.
>
>Please clarify what cons and plists.  Where the documentation says
>about cons and plists for mouse-face?
>

Hi Juri:

In: 33.19.4 Properties with Special Meanings

Look at the `face` property values; there are 4 more apart from face
name (symbol or string).

In principle mouse-face accepts the same formats right? So I suppose
that cursor-face should also support them.

Currently in my code only the "symbol" alternative is supported. Adding
string is simple to implement, but I suppose there is some API I am not
aware of to support the other cases and I don't want to reinvent the
wheel.

Best,
Ergus

Re: Select completions from the minibuffer

[Stefan Monnier]

> Currently in my code only the "symbol" alternative is supported. Adding
> string is simple to implement, but I suppose there is some API I am not
> aware of to support the other cases and I don't want to reinvent the
> wheel.

There's no real API for it, AFAIK.  Just put that value on the
`face` property and that's that.


        Stefan

Re: Select completions from the minibuffer

[Ergus]

On Mon, Mar 14, 2022 at 04:58:27PM -0400, Stefan Monnier wrote:
>> Currently in my code only the "symbol" alternative is supported. Adding
>> string is simple to implement, but I suppose there is some API I am not
>> aware of to support the other cases and I don't want to reinvent the
>> wheel.
>
>There's no real API for it, AFAIK.  Just put that value on the
>`face` property and that's that.
>
>
>        Stefan
>
>
Hi Stefan:

I tried some of those approached and that didn't work for me... I'm
pretty sure I did something wrong, but any way I don't have plenty time
to do that now... Do we have at least some function to validate the
value?  I mean like facep but that checks it is a valid face name, a
correct plist or a cons with a valid car? How the other functions around
validate that the values received are valid??

Re: Select completions from the minibuffer

[Stefan Monnier]

> value?  I mean like facep but that checks it is a valid face name, a
> correct plist or a cons with a valid car? How the other functions around
> validate that the values received are valid??

Usually they don't, they just put the value in the `face` property and
if it doesn't work it's the caller's fault.


        Stefan

Re: Select completions from the minibuffer

[Stefan Monnier]

> I tried some of those approached and that didn't work for me... I'm
> pretty sure I did something wrong, but any way I don't have plenty time
> to do that now... Do we have at least some function to validate the
> value?  I mean like facep but that checks it is a valid face name, a
> correct plist or a cons with a valid car? How the other functions around
> validate that the values received are valid??

Can you point to the code where you tried it, and tell us what you tried
and in which sense it "didn't work"?


        Stefan

Re: Select completions from the minibuffer

[Ergus]

On Mon, Mar 14, 2022 at 09:34:17PM +0200, Juri Linkov wrote:
>> Now there is a variable completions-highlight-face. When nil highlight
>> is disabled, otherwise it must be a the face to use, by default
>> completions-highlight.
>>
>> I had the mode before because I needed some initialization, but with the
>> cursor-face feature that is not needed and the code got simpler.
>
>Thanks, highlighting works correctly when the current buffer is *Completions*.
>But cursor-face is not highlighted in *Completions* when the current buffer
>is the minibuffer.  Such highlighting is necessary when completions
>are navigated from the minibuffer.
>

When the window is not selected or the active window of the minibuffer
it is not highlighted (because conceptually the cursor is not on that
window)

I can rely on cursor-in-non-selected-windows to have a consistent
behavior or add a new local variable for internal use only. What do you
prefer?

Re: Select completions from the minibuffer

[Ergus]

On Mon, Mar 14, 2022 at 09:34:17PM +0200, Juri Linkov wrote:
>> Now there is a variable completions-highlight-face. When nil highlight
>> is disabled, otherwise it must be a the face to use, by default
>> completions-highlight.
>>
>> I had the mode before because I needed some initialization, but with the
>> cursor-face feature that is not needed and the code got simpler.
>
>Thanks, highlighting works correctly when the current buffer is *Completions*.
>But cursor-face is not highlighted in *Completions* when the current buffer
>is the minibuffer.  Such highlighting is necessary when completions
>are navigated from the minibuffer.
>
Please check the change and try to set
cursor-face-highlight-nonselected-window unconditionally in
*Completions* to t when using your mode.

If you could improve the initialization in order to use
cursor-in-non-selected-windows unless the user sets another value
manually it may be nicer (because in the general scenario the user could
set cursor-in-non-selected-windows latter so this variable may have an
outdated value) otherwise consider if this may be simply t or nil by
default. But at the moment it works fine for the current use cases.

Best, Ergus

Re: Select completions from the minibuffer

[Juri Linkov]

>> But cursor-face is not highlighted in *Completions* when the current buffer
>> is the minibuffer.  Such highlighting is necessary when completions
>> are navigated from the minibuffer.

> Please check the change and try to set
> cursor-face-highlight-nonselected-window unconditionally in
> *Completions* to t when using your mode.

Now it keeps highlighting in *Completions* after switching to the minibuffer,
but still doesn't move the highlighting in *Completions* when
completions are navigated from the minibuffer with

  (defmacro with-minibuffer-completions-window (&rest body)
    "Execute the forms in BODY from the minibuffer in its completions window.
  When used in a minibuffer window, select the window with completions,
  and execute the forms."
    (declare (indent 0) (debug t))
    `(let ((window (or (get-buffer-window "*Completions*" 0)
                       ;; Make sure we have a completions window.
                       (progn (minibuffer-completion-help)
                              (get-buffer-window "*Completions*" 0)))))
       (when window
         (with-selected-window window
           ,@body))))

  (defun minibuffer-next-completion (&optional n)
    "Run `next-completion' from the minibuffer in its completions window."
    (interactive "p")
    (with-minibuffer-completions-window
      (next-completion n)))

> If you could improve the initialization in order to use
> cursor-in-non-selected-windows unless the user sets another value
> manually it may be nicer (because in the general scenario the user could
> set cursor-in-non-selected-windows latter so this variable may have an
> outdated value) otherwise consider if this may be simply t or nil by
> default. But at the moment it works fine for the current use cases.

I think it's unrelated to cursor-in-non-selected-windows
because the cursor in non-selected windows is displayed as
a hollow box, but cursor-face stays the same.
So you can set cursor-face-highlight-nonselected-window
to nil by default.  Then commands could let-bind it like

  (defun minibuffer-next-completion (&optional n)
    "Run `next-completion' from the minibuffer in its completions window."
    (interactive "p")
    (let ((cursor-face-highlight-nonselected-window t))
      (with-minibuffer-completions-window
        (next-completion n))))

(but this currently doesn't work for the same reason as above)

Re: Select completions from the minibuffer

[Stefan Monnier]

> Now it keeps highlighting in *Completions* after switching to the minibuffer,
> but still doesn't move the highlighting in *Completions* when
> completions are navigated from the minibuffer with

Shooting from the side lines: sounds like maybe it's due to the
difference between `point` and `window-point`.  IOW maybe somewhere the
new code should use `window-point` instead of `point`.


        Stefan

Re: Select completions from the minibuffer

[Ergus]

On Thu, Mar 17, 2022 at 05:26:09PM -0400, Stefan Monnier wrote:
>> Now it keeps highlighting in *Completions* after switching to the minibuffer,
>> but still doesn't move the highlighting in *Completions* when
>> completions are navigated from the minibuffer with
>
>Shooting from the side lines: sounds like maybe it's due to the
>difference between `point` and `window-point`.  IOW maybe somewhere the
>new code should use `window-point` instead of `point`.
>
>
>        Stefan
>

Hi Stefan:

redisplay--update-cursor-face-highlight already uses
window-point... What do you mean?

Re: Select completions from the minibuffer

[Stefan Monnier]

>>> Now it keeps highlighting in *Completions* after switching to the minibuffer,
>>> but still doesn't move the highlighting in *Completions* when
>>> completions are navigated from the minibuffer with
>>Shooting from the side lines: sounds like maybe it's due to the
>>difference between `point` and `window-point`.  IOW maybe somewhere the
>>new code should use `window-point` instead of `point`.
> redisplay--update-cursor-face-highlight already uses
> window-point... What do you mean?

Then I guess my crystal ball needs an update.


        Stefan

Re: Select completions from the minibuffer

[Ergus]

Looking around it seems like with your function the *Completions* window
is not passed to redisplay--pre-redisplay-functions, but only the
minibuffer. So basically the window is not updated and the overlay not
moved. Probably there is some optimization that is affecting us... Eli?



On Thu, Mar 17, 2022 at 10:56:01PM +0200, Juri Linkov wrote:
>>> But cursor-face is not highlighted in *Completions* when the current buffer
>>> is the minibuffer.  Such highlighting is necessary when completions
>>> are navigated from the minibuffer.
>
>> Please check the change and try to set
>> cursor-face-highlight-nonselected-window unconditionally in
>> *Completions* to t when using your mode.
>
>Now it keeps highlighting in *Completions* after switching to the minibuffer,
>but still doesn't move the highlighting in *Completions* when
>completions are navigated from the minibuffer with
>
>  (defmacro with-minibuffer-completions-window (&rest body)
>    "Execute the forms in BODY from the minibuffer in its completions window.
>  When used in a minibuffer window, select the window with completions,
>  and execute the forms."
>    (declare (indent 0) (debug t))
>    `(let ((window (or (get-buffer-window "*Completions*" 0)
>                       ;; Make sure we have a completions window.
>                       (progn (minibuffer-completion-help)
>                              (get-buffer-window "*Completions*" 0)))))
>       (when window
>         (with-selected-window window
>           ,@body))))
>
>  (defun minibuffer-next-completion (&optional n)
>    "Run `next-completion' from the minibuffer in its completions window."
>    (interactive "p")
>    (with-minibuffer-completions-window
>      (next-completion n)))
>
>> If you could improve the initialization in order to use
>> cursor-in-non-selected-windows unless the user sets another value
>> manually it may be nicer (because in the general scenario the user could
>> set cursor-in-non-selected-windows latter so this variable may have an
>> outdated value) otherwise consider if this may be simply t or nil by
>> default. But at the moment it works fine for the current use cases.
>
>I think it's unrelated to cursor-in-non-selected-windows
>because the cursor in non-selected windows is displayed as
>a hollow box, but cursor-face stays the same.
>So you can set cursor-face-highlight-nonselected-window
>to nil by default.  Then commands could let-bind it like
>
>  (defun minibuffer-next-completion (&optional n)
>    "Run `next-completion' from the minibuffer in its completions window."
>    (interactive "p")
>    (let ((cursor-face-highlight-nonselected-window t))
>      (with-minibuffer-completions-window
>        (next-completion n))))
>
>(but this currently doesn't work for the same reason as above)
>

Re: Select completions from the minibuffer

[Ergus]

Hi Juri:

Yes I just confirmed, actually the pre-redisplay-function is not called
for simple commands like self-insert or moving the point and in your
case neither.

Looking at the last line in `deactivate-mark` it seems like it is a
known issue and changing such optimization in the display engine may be
probably undesirable (Eli will confirm soon hopefully), so in your case
maybe is better to force the update on demand like in deactivate-mark:

```
(defmacro with-minibuffer-completions-window (&rest body)
   "Execute the forms in BODY from the minibuffer in its completions window.
When used in a minibuffer window, select the window with completions,
and execute the forms."
   (declare (indent 0) (debug t))
   `(let ((window (or (get-buffer-window "*Completions*" 0)
                      ;; Make sure we have a completions window.
                      (progn (minibuffer-completion-help)
                             (get-buffer-window "*Completions*" 0)))))
      (when window
        (with-selected-window window
          ,@body
          (redisplay--update-cursor-face-highlight window)))))
```

This seems to work.

Is it enough?


On Thu, Mar 17, 2022 at 10:56:01PM +0200, Juri Linkov wrote:
>>> But cursor-face is not highlighted in *Completions* when the current buffer
>>> is the minibuffer.  Such highlighting is necessary when completions
>>> are navigated from the minibuffer.
>
>> Please check the change and try to set
>> cursor-face-highlight-nonselected-window unconditionally in
>> *Completions* to t when using your mode.
>
>Now it keeps highlighting in *Completions* after switching to the minibuffer,
>but still doesn't move the highlighting in *Completions* when
>completions are navigated from the minibuffer with
>
>  (defmacro with-minibuffer-completions-window (&rest body)
>    "Execute the forms in BODY from the minibuffer in its completions window.
>  When used in a minibuffer window, select the window with completions,
>  and execute the forms."
>    (declare (indent 0) (debug t))
>    `(let ((window (or (get-buffer-window "*Completions*" 0)
>                       ;; Make sure we have a completions window.
>                       (progn (minibuffer-completion-help)
>                              (get-buffer-window "*Completions*" 0)))))
>       (when window
>         (with-selected-window window
>           ,@body))))
>
>  (defun minibuffer-next-completion (&optional n)
>    "Run `next-completion' from the minibuffer in its completions window."
>    (interactive "p")
>    (with-minibuffer-completions-window
>      (next-completion n)))
>
>> If you could improve the initialization in order to use
>> cursor-in-non-selected-windows unless the user sets another value
>> manually it may be nicer (because in the general scenario the user could
>> set cursor-in-non-selected-windows latter so this variable may have an
>> outdated value) otherwise consider if this may be simply t or nil by
>> default. But at the moment it works fine for the current use cases.
>
>I think it's unrelated to cursor-in-non-selected-windows
>because the cursor in non-selected windows is displayed as
>a hollow box, but cursor-face stays the same.
>So you can set cursor-face-highlight-nonselected-window
>to nil by default.  Then commands could let-bind it like
>
>  (defun minibuffer-next-completion (&optional n)
>    "Run `next-completion' from the minibuffer in its completions window."
>    (interactive "p")
>    (let ((cursor-face-highlight-nonselected-window t))
>      (with-minibuffer-completions-window
>        (next-completion n))))
>
>(but this currently doesn't work for the same reason as above)
>

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Thu, 17 Mar 2022 23:44:33 +0100
> From: Ergus <spacibba@aol.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
> 
> Looking around it seems like with your function the *Completions* window
> is not passed to redisplay--pre-redisplay-functions, but only the
> minibuffer. So basically the window is not updated and the overlay not
> moved. Probably there is some optimization that is affecting us... Eli?

redisplay--pre-redisplay-functions don't know anything about the
differences between windows, so if those functions aren't called, it
is not related to the window or the buffer.  Every window that gets
redisplayed gets the same treatment.

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Fri, 18 Mar 2022 00:10:27 +0100
> From: Ergus <spacibba@aol.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
> 
> Yes I just confirmed, actually the pre-redisplay-function is not called
> for simple commands like self-insert or moving the point and in your
> case neither.

If that's the case, how come the region's display is updated when you
insert a character or move point (with Shift pressed)?

Re: Select completions from the minibuffer

[Juri Linkov]

> Looking at the last line in `deactivate-mark` it seems like it is a
> known issue and changing such optimization in the display engine may be
> probably undesirable (Eli will confirm soon hopefully), so in your case
> maybe is better to force the update on demand like in deactivate-mark:
>
> ```
> (defmacro with-minibuffer-completions-window (&rest body)
>    "Execute the forms in BODY from the minibuffer in its completions window.
> When used in a minibuffer window, select the window with completions,
> and execute the forms."
>    (declare (indent 0) (debug t))
>    `(let ((window (or (get-buffer-window "*Completions*" 0)
>                       ;; Make sure we have a completions window.
>                       (progn (minibuffer-completion-help)
>                              (get-buffer-window "*Completions*" 0)))))
>       (when window
>         (with-selected-window window
>           ,@body
>           (redisplay--update-cursor-face-highlight window)))))
> ```
>
> This seems to work.
>
> Is it enough?

Thanks, I confirm such forcing the update fixes highlighting.

But still when cursor-face-highlight-nonselected-window is nil by default,
let-binding it to t has no effect:

```
(defun minibuffer-next-completion (&optional n)
  "Run `next-completion' from the minibuffer in its completions window."
  (interactive "p")
  (with-minibuffer-completions-window
    (let ((cursor-face-highlight-nonselected-window t))
      (next-completion n))))
```

Re: Select completions from the minibuffer

[Ergus]

On Fri, Mar 18, 2022 at 08:28:34AM +0200, Eli Zaretskii wrote:
>> Date: Fri, 18 Mar 2022 00:10:27 +0100
>> From: Ergus <spacibba@aol.com>
>> Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
>>
>> Yes I just confirmed, actually the pre-redisplay-function is not called
>> for simple commands like self-insert or moving the point and in your
>> case neither.
>
>If that's the case, how come the region's display is updated when you
>insert a character or move point (with Shift pressed)?
>
Hi Eli:

Maybe I expressed a bit incorrectly, sorry.

When moving point redisplay--pre-redisplay-functions is called BUT
receives nil in the list of windows to update, so it goes in the first
part of the if and runs the hooks for the selected-window
only. (actually this code looks like a workaround for that situation)

(if (null windows)
         (with-current-buffer (window-buffer (selected-window))
           (run-hook-with-args 'pre-redisplay-functions (selected-window)))
       (dolist (win (if (listp windows) windows (window-list-1 nil nil t)))
         (with-current-buffer (window-buffer win)
           (run-hook-with-args 'pre-redisplay-functions win))))


In Juri's code it is moving the point in completions while in the
minibuffer so *Completions* never becomes the active window and it is
never passed to redisplay--pre-redisplay-functions.

When we force (redisplay--update-cursor-face-highlight) then
*Completions* is passed to redisplay--pre-redisplay-functions, maybe
because an overlay moved or because of the set-window-parameter
call... I am not really sure about how the windows list is filled to
call pre-redisplay-function.

Does it makes sense?

Best,
Ergus

Re: Select completions from the minibuffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Fri, 18 Mar 2022 11:27:52 +0200
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
> 
> But still when cursor-face-highlight-nonselected-window is nil by default,
> let-binding it to t has no effect:

If this variable is used in a hook run by redisplay, it's expected:
the let-binding of variables done by a command is undone by the time
redisplay runs.

(Apologies if what I said makes no sense: I wasn't closely following
this discussion.)

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Fri, 18 Mar 2022 12:16:08 +0100
> From: Ergus <spacibba@aol.com>
> Cc: larsi@gnus.org, emacs-devel@gnu.org, juri@linkov.net
> 
> When moving point redisplay--pre-redisplay-functions is called BUT
> receives nil in the list of windows to update, so it goes in the first
> part of the if and runs the hooks for the selected-window
> only.

And this is incorrect, i.e. the actual window to be redisplayed is NOT
the selected window?

> (actually this code looks like a workaround for that situation)
> 
> (if (null windows)
>          (with-current-buffer (window-buffer (selected-window))
>            (run-hook-with-args 'pre-redisplay-functions (selected-window)))
>        (dolist (win (if (listp windows) windows (window-list-1 nil nil t)))
>          (with-current-buffer (window-buffer win)
>            (run-hook-with-args 'pre-redisplay-functions win))))

Why is it a workaround?  The doc string of pre-redisplay-function says
that if the argument is nil, it stands for the selected-window.

> In Juri's code it is moving the point in completions while in the
> minibuffer so *Completions* never becomes the active window and it is
> never passed to redisplay--pre-redisplay-functions.

I'd appreciate a test case for this, preferably independent of the
completion framework, so that I could look at what happens there.  It
sounds like you are saying that a window gets redrawn by the display
engine (because its point moved), but pre-redisplay-functions aren't
called for that window, which would be a bug, I think.

Re: Select completions from the minibuffer

[Ergus]

On Fri, Mar 18, 2022 at 11:27:52AM +0200, Juri Linkov wrote:
>> Looking at the last line in `deactivate-mark` it seems like it is a
>> known issue and changing such optimization in the display engine may be
>> probably undesirable (Eli will confirm soon hopefully), so in your case
>> maybe is better to force the update on demand like in deactivate-mark:
>>
>> ```
>> (defmacro with-minibuffer-completions-window (&rest body)
>>    "Execute the forms in BODY from the minibuffer in its completions window.
>> When used in a minibuffer window, select the window with completions,
>> and execute the forms."
>>    (declare (indent 0) (debug t))
>>    `(let ((window (or (get-buffer-window "*Completions*" 0)
>>                       ;; Make sure we have a completions window.
>>                       (progn (minibuffer-completion-help)
>>                              (get-buffer-window "*Completions*" 0)))))
>>       (when window
>>         (with-selected-window window
>>           ,@body
>>           (redisplay--update-cursor-face-highlight window)))))
>> ```
>>
>> This seems to work.
>>
>> Is it enough?
>
>Thanks, I confirm such forcing the update fixes highlighting.
>
>But still when cursor-face-highlight-nonselected-window is nil by default,
>let-binding it to t has no effect:
>
>```
>(defun minibuffer-next-completion (&optional n)
>  "Run `next-completion' from the minibuffer in its completions window."
>  (interactive "p")
>  (with-minibuffer-completions-window
>    (let ((cursor-face-highlight-nonselected-window t))
>      (next-completion n))))
>```
>
Hi Juri:

This should be because when you force the
redisplay--update-cursor-face-highlight; then the display engine detects
that something more important than a point move changed (an overlay
moved) and it passes *Completions* to
redisplay--pre-redisplay-functions.

Then it calls redisplay--update-cursor-face-highlight once again (yes
now it calls redisplay--update-cursor-face-highlight 2 times instead of
none) but the second call is from outside of your function; so it gets
the buffer local value of cursor-face-highlight-nonselected-window.

In general (setq-local cursor-face-highlight-nonselected-window t)
should go into the completion-setup-function because you don't have a
full control for when the display engine calls
redisplay--pre-redisplay-functions for *Completions* window. For
example, in a resize the highlight may disappear too.

So you need:

(when completions-highlight-face
         (setq cursor-face-highlight-nonselected-window t)
         (cursor-face-highlight-mode 1))

in completion-setup-function OR add another function to
completion-setup-hook only to do the setq-local.

Eli:

If there is a way to force the display engine to detect a change in a
window and use it to pass *Completions* in the next call to
redisplay--pre-redisplay-functions? I mean to avoid the need to do a
redundant explicit call to redisplay--update-cursor-face-highlight which
doesn't really take effect?

Best,
Ergus

Re: Select completions from the minibuffer

[Stefan Monnier]

>> Yes I just confirmed, actually the pre-redisplay-function is not called
>> for simple commands like self-insert or moving the point and in your
>> case neither.

I'm pretty sure it's called for "simple commands like self-insert".

But as for moving point, maybe you're right that we have a bug there,
when point is moved in a window other than the selected window.

> If that's the case, how come the region's display is updated when you
> insert a character or move point (with Shift pressed)?

The region is only highlighted in the selected-window, so that doesn't
give us evidence to conclude that the `pre-redisplay-functions` are called
when point moves in a window other than the selected window.


        Stefan

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Fri, 18 Mar 2022 12:53:36 +0100
> From: Ergus <spacibba@aol.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
> 
> Eli:
> 
> If there is a way to force the display engine to detect a change in a
> window and use it to pass *Completions* in the next call to
> redisplay--pre-redisplay-functions? I mean to avoid the need to do a
> redundant explicit call to redisplay--update-cursor-face-highlight which
> doesn't really take effect?

The display engine is supposed to figure out by itself that a window
needs to be redisplayed, due to some changes in the window or its
buffer or anything else that affects the window display.  Anything
else is a bug, because it means a window will not be redrawn when some
change affecting it happens.

So I don't think I understand your question.  It almost sounds like
"can we force redisplay do its job correctly?"

I guess some details are missing from your description, which is why I
asked for a test case.

Re: Select completions from the minibuffer

[Stefan Monnier]

> Looking at the last line in `deactivate-mark` it seems like it is a
> known issue and changing such optimization in the display engine may be

Yuck!
Maybe that code should use `force-mode-line-update` instead.


        Stefan

Re: Select completions from the minibuffer

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Ergus <spacibba@aol.com>,  larsi@gnus.org,  emacs-devel@gnu.org,
>   juri@linkov.net
> Date: Fri, 18 Mar 2022 08:04:12 -0400
> 
> But as for moving point, maybe you're right that we have a bug there,
> when point is moved in a window other than the selected window.
> 
> > If that's the case, how come the region's display is updated when you
> > insert a character or move point (with Shift pressed)?
> 
> The region is only highlighted in the selected-window

Except when highlight-nonselected-windows is non-nil, right?

Re: Select completions from the minibuffer

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Juri Linkov <juri@linkov.net>,  Lars Ingebrigtsen <larsi@gnus.org>,
>  emacs-devel@gnu.org
> Date: Fri, 18 Mar 2022 08:08:13 -0400
> 
> > Looking at the last line in `deactivate-mark` it seems like it is a
> > known issue and changing such optimization in the display engine may be
> 
> Yuck!
> Maybe that code should use `force-mode-line-update` instead.

How much cruft and our own bugs and misunderstanding are we prepared
to hang on that misnomer of a function??

Re: Select completions from the minibuffer

[Ergus]

On Fri, Mar 18, 2022 at 02:11:09PM +0200, Eli Zaretskii wrote:
>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Ergus <spacibba@aol.com>,  larsi@gnus.org,  emacs-devel@gnu.org,
>>   juri@linkov.net
>> Date: Fri, 18 Mar 2022 08:04:12 -0400
>>
>> But as for moving point, maybe you're right that we have a bug there,
>> when point is moved in a window other than the selected window.
>>
>> > If that's the case, how come the region's display is updated when you
>> > insert a character or move point (with Shift pressed)?
>>
>> The region is only highlighted in the selected-window
>
>Except when highlight-nonselected-windows is non-nil, right?

Actually this is the simplest way to test:

(setq highlight-nonselected-windows t)

Create a buffer: test

write some text there and select a part of that text (so you see the region)

Then from scratch try commands like (just an example):

(with-current-buffer "test"
   (right-char)
   (insert "X"))

Or similes several times...

You will see that the region is not updated.

Re: Select completions from the minibuffer

[Stefan Monnier]

> Then from scratch try commands like (just an example):
>
> (with-current-buffer "test"
>   (right-char)
>   (insert "X"))
>
> Or similes several times...
> You will see that the region is not updated.

With your above test, I do see the region being updated.

When it fails to do its job is for:

    (with-selected-window (next-window) (forward-line -1))

I.e. when the only change in that other window is that point was moved.

The redisplay does update the cursor but it fails to run
`pre-redisplay-functions` in that other window :-(


        Stefan

Re: Select completions from the minibuffer

[Ergus]

On Fri, Mar 18, 2022 at 08:35:04AM -0400, Stefan Monnier wrote:
>> Then from scratch try commands like (just an example):
>>
>> (with-current-buffer "test"
>>   (right-char)
>>   (insert "X"))
>>
>> Or similes several times...
>> You will see that the region is not updated.
>
>With your above test, I do see the region being updated.
>
>When it fails to do its job is for:
>
>    (with-selected-window (next-window) (forward-line -1))
>
>I.e. when the only change in that other window is that point was moved.
>
>The redisplay does update the cursor but it fails to run
>`pre-redisplay-functions` in that other window :-(
>
>
>        Stefan
>
You are right sorry... the example code should has been:

(with-selected-window (get-buffer-window "test" 0)
   (right-char))

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Fri, 18 Mar 2022 13:38:59 +0100
> From: Ergus <spacibba@aol.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org, emacs-devel@gnu.org,
> 	juri@linkov.net
> 
> On Fri, Mar 18, 2022 at 08:35:04AM -0400, Stefan Monnier wrote:
> >> Then from scratch try commands like (just an example):
> >>
> >> (with-current-buffer "test"
> >>   (right-char)
> >>   (insert "X"))
> >>
> >> Or similes several times...
> >> You will see that the region is not updated.
> >
> >With your above test, I do see the region being updated.
> >
> >When it fails to do its job is for:
> >
> >    (with-selected-window (next-window) (forward-line -1))
> >
> >I.e. when the only change in that other window is that point was moved.
> >
> >The redisplay does update the cursor but it fails to run
> >`pre-redisplay-functions` in that other window :-(
> >
> >
> >        Stefan
> >
> You are right sorry... the example code should has been:
> 
> (with-selected-window (get-buffer-window "test" 0)
>    (right-char))

Thanks, please submit a bug report with the relevant reproducer, and I
will take a look.

Re: Select completions from the minibuffer

[Ergus]

On Fri, Mar 18, 2022 at 11:27:52AM +0200, Juri Linkov wrote:
>> Looking at the last line in `deactivate-mark` it seems like it is a
>> known issue and changing such optimization in the display engine may be
>> probably undesirable (Eli will confirm soon hopefully), so in your case
>> maybe is better to force the update on demand like in deactivate-mark:
>>
>> ```
>> (defmacro with-minibuffer-completions-window (&rest body)
>>    "Execute the forms in BODY from the minibuffer in its completions window.
>> When used in a minibuffer window, select the window with completions,
>> and execute the forms."
>>    (declare (indent 0) (debug t))
>>    `(let ((window (or (get-buffer-window "*Completions*" 0)
>>                       ;; Make sure we have a completions window.
>>                       (progn (minibuffer-completion-help)
>>                              (get-buffer-window "*Completions*" 0)))))
>>       (when window
>>         (with-selected-window window
>>           ,@body
>>           (redisplay--update-cursor-face-highlight window)))))
>> ```
>>
>> This seems to work.
>>
>> Is it enough?
>
>Thanks, I confirm such forcing the update fixes highlighting.
>
>But still when cursor-face-highlight-nonselected-window is nil by default,
>let-binding it to t has no effect:
>
>```
>(defun minibuffer-next-completion (&optional n)
>  "Run `next-completion' from the minibuffer in its completions window."
>  (interactive "p")
>  (with-minibuffer-completions-window
>    (let ((cursor-face-highlight-nonselected-window t))
>      (next-completion n))))
>```
>

Hi Juri:

Eli just fixed the real issue in the display engine. So now your code
does not need to force the update. BUT it still needs to set the

(setq-local cursor-face-highlight-nonselected-window t)

in completion-setup-function... I can do it for you unless you prefer to
add your own function to completion-setup-hook.

Re: Select completions from the minibuffer

[Juri Linkov]

> Eli just fixed the real issue in the display engine. So now your code
> does not need to force the update. BUT it still needs to set the
>
> (setq-local cursor-face-highlight-nonselected-window t)
>
> in completion-setup-function... I can do it for you unless you prefer to
> add your own function to completion-setup-hook.

Maybe better to set a new window parameter in the command that navigates
completions from the minibuffer.  Then such a window parameter could be
checked in redisplay--update-cursor-face-highlight.

Re: Select completions from the minibuffer

[Ergus]

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

For a general case I am not sure this is correct or desired because the same window may change buffer.  And we want the highlight mode to be buffer local not window local right?... Even the new function in the predisplay hook is buffer local... I will find confusing such a mix...

Any way I am not the best lisper to have a strong opinion on this. But IMHO the code is actually pretty simple to deserve a more complex approach.




On March 19, 2022 8:03:34 PM GMT+01:00, Juri Linkov <juri@linkov.net> wrote:
>> Eli just fixed the real issue in the display engine. So now your code
>> does not need to force the update. BUT it still needs to set the
>>
>> (setq-local cursor-face-highlight-nonselected-window t)
>>
>> in completion-setup-function... I can do it for you unless you prefer to
>> add your own function to completion-setup-hook.
>
>Maybe better to set a new window parameter in the command that navigates
>completions from the minibuffer.  Then such a window parameter could be
>checked in redisplay--update-cursor-face-highlight.
>

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

Re: Select completions from the minibuffer

[Ergus]

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

Actually thinking a bit more on this... You can do the same just doing a setq-local in the same command instead of a let or a window parameter... Right???



On March 19, 2022 8:03:34 PM GMT+01:00, Juri Linkov <juri@linkov.net> wrote:
>> Eli just fixed the real issue in the display engine. So now your code
>> does not need to force the update. BUT it still needs to set the
>>
>> (setq-local cursor-face-highlight-nonselected-window t)
>>
>> in completion-setup-function... I can do it for you unless you prefer to
>> add your own function to completion-setup-hook.
>
>Maybe better to set a new window parameter in the command that navigates
>completions from the minibuffer.  Then such a window parameter could be
>checked in redisplay--update-cursor-face-highlight.
>

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

Re: Select completions from the minibuffer

[Juri Linkov]

> Actually thinking a bit more on this... You can do the same just doing a
> setq-local in the same command instead of a let or a window parameter...

Thanks, good idea, I confirm this works nice:

```
(defun minibuffer-next-completion (&optional n)
  "Run `next-completion' from the minibuffer in its completions window."
  (interactive "p")
  (with-minibuffer-completions-window
    (setq-local cursor-face-highlight-nonselected-window t)
    (next-completion n)))
```

Re: Select completions from the minibuffer

[Ergus]

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

Any way, I think that it may not be necessary to  setq-local in your functions because I plan to set it to t in completions by default... it makes sense at least for completions to stay highlighted if the user changes window. Right?




On March 20, 2022 7:42:44 PM GMT+01:00, Juri Linkov <juri@linkov.net> wrote:
>> Actually thinking a bit more on this... You can do the same just doing a
>> setq-local in the same command instead of a let or a window parameter...
>
>Thanks, good idea, I confirm this works nice:
>
>```
>(defun minibuffer-next-completion (&optional n)
>  "Run `next-completion' from the minibuffer in its completions window."
>  (interactive "p")
>  (with-minibuffer-completions-window
>    (setq-local cursor-face-highlight-nonselected-window t)
>    (next-completion n)))
>```
>

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

Re: Select completions from the minibuffer

[Juri Linkov]

> Any way, I think that it may not be necessary to setq-local in your
> functions because I plan to set it to t in completions by default... it
> makes sense at least for completions to stay highlighted if the user
> changes window. Right?

This looks right.

Re: Select completions from the minibuffer

[Juri Linkov]

>> The new variable completion-use-base-affixes will be also enabled
>> for the M-up/down feature.  Here is an updated patch that also requires
>> other two patches from bug#47417 (merged with bug#49931) and bug#48356:
>
> It's a bit difficult to test this stuff when it's spread out over so
> many patches -- perhaps you should start pushing the patches to the
> trunk (in whatever order makes sense)?

Since Ergus finished implementing new features in the
feature/completions-customs branch (and I fixed small things
like renaming completion-header-format to completions-header-format),
the branch is ready for merging to master now.  After that
we could add more patches based on new features.

Re: Select completions from the minibuffer

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Since Ergus finished implementing new features in the
> feature/completions-customs branch (and I fixed small things
> like renaming completion-header-format to completions-header-format),
> the branch is ready for merging to master now.  After that
> we could add more patches based on new features.

Sounds good to me.

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

Re: Select completions from the minibuffer

[Juri Linkov]

>> Since Ergus finished implementing new features in the
>> feature/completions-customs branch (and I fixed small things
>> like renaming completion-header-format to completions-header-format),
>> the branch is ready for merging to master now.  After that
>> we could add more patches based on new features.
>
> Sounds good to me.

So now the branch is merged to master.  Thanks Ergus for implementing
these features.

Re: Select completions from the minibuffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Tue, 22 Mar 2022 10:24:00 +0200
> Cc: Ergus <spacibba@aol.com>, emacs-devel@gnu.org
> 
> >> Since Ergus finished implementing new features in the
> >> feature/completions-customs branch (and I fixed small things
> >> like renaming completion-header-format to completions-header-format),
> >> the branch is ready for merging to master now.  After that
> >> we could add more patches based on new features.
> >
> > Sounds good to me.
> 
> So now the branch is merged to master.  Thanks Ergus for implementing
> these features.

Thanks, but please see some questions/comments below, and either fix
them or explain what you meant so that I could fix them:

     If @code{completion-auto-help} is set to @code{nil}, the completion
   commands never display the completion list buffer; you must type
  -@kbd{?}  to display the list.  If the value is @code{lazy}, Emacs only
  +@kbd{?} to display the list.  If the value is @code{lazy}, Emacs only
   shows the completion list buffer on the second attempt to complete.
   In other words, if there is nothing to complete, the first @key{TAB}
   echoes @samp{Next char not unique}; the second @key{TAB} shows the
  -completion list buffer.
  +completion list buffer.  With the previous values and the default

What are those "previous values" which you mention here?

  +@code{t} the completions are hidden when some unique completion is
  +executed.

What do you mean by "unique completion is executed"?  Both the
"unique" and the "executed" parts need clarification.

             If @code{completion-auto-help} is set to @code{always}, the
  +completion commands are always shown after a completion attempt, or

"Commands"? didn't you mean "candidates" or "alternatives"?  (The same
problem exists in the doc string of this variable, btw.)

  +updated if they are already visible.  If the value is @code{visible},
  +then completions are not hidden, but updated if they are already
  +visible while the current behavior stays the same as default if they
  +are not.

I think the last sentence should be reworded as follows:

  If the value is @code{visible}, Emacs displays completion the
  completion alternatives when it is unable to complete for the first
  time; thereafter the completion list buffer remains visible and is
  updated as you type.

Is this accurate and correct?

  +@vindex completions-header-format
  +The variable @code{completions-header-format} is a formatted string to
  +control the informative line shown before the completions list of
  +candidates.  It may contain a @code{%s} to show the total number of
  +completions.  When it is @code{nil}, the message is totally suppressed.
  +Text properties may be added to change the appearance, some useful
  +ones are @code{face} or @code{cursor-intangible} (@pxref{Special
  +Properties,,Properties with Special Meanings, elisp, The Emacs Lisp
  +Reference Manual}).

Shouldn't this have a cross-reference to where header-line-format is
described?  This variable determines the format of the header-line
shown in the window that displays the *Completions* buffer, right?  So
knowing what can be used in header-line-format should be useful.

  +@vindex completions-highlight-face
  +When @code{completions-highlight-face} is a face name, then the

Saying "face name" might mislead people to think this is a string.  It
is better to say "If the value of this variable is a face, ..."

Also, I'd lose "then": it's redundant here.

  +current completion candidate will be highlighted with that face.  The
  +default value is @code{completions-highlight}.  When the value is
  +@code{nil}, no highlighting is performed.  This feature sets the text
  +property @code{cursor-face}.

This should explain what is "the current completion candidate".  It
isn't trivial: trying the current defaults with completion commands, I
sometimes see a candidate highlighted, and sometimes don't.  I
couldn't figure out why.

  +@item cursor-face
  +@kindex cursor-face @r{(text property)}
  +This property is similar to @code{mouse-face}, but the face is used if
  +the cursor (instead of mouse) is on or near the character.
                                                   ^^^^^^^^^
Which character?  There's no reference to "characters" in the
preceding text, so this is confusing.  I suggest something like

  when point is inside text which has this face

Re: Select completions from the minibuffer

[Eric S Fraga]

Minor English suggestion:

On Tuesday, 22 Mar 2022 at 15:38, Eli Zaretskii wrote:
>   +@vindex completions-header-format
>   +The variable @code{completions-header-format} is a formatted string to
>   +control the informative line shown before the completions list of
>   +candidates.  It may contain a @code{%s} to show the total number of
>   +completions.  When it is @code{nil}, the message is totally suppressed.

Remove "totally" as suppressed already implies this.  Otherwise, it
would be minimised or reduced...

-- 
Eric S Fraga with org 9.5.2 in Emacs 29.0.50 on Debian 11.2

Re: Select completions from the minibuffer

[Ergus]

On Tue, Mar 22, 2022 at 03:38:20PM +0200, Eli Zaretskii wrote:
>> From: Juri Linkov <juri@linkov.net>
>> Date: Tue, 22 Mar 2022 10:24:00 +0200
>> Cc: Ergus <spacibba@aol.com>, emacs-devel@gnu.org
>>
>> >> Since Ergus finished implementing new features in the
>> >> feature/completions-customs branch (and I fixed small things
>> >> like renaming completion-header-format to completions-header-format),
>> >> the branch is ready for merging to master now.  After that
>> >> we could add more patches based on new features.
>> >
>> > Sounds good to me.
>>
>> So now the branch is merged to master.  Thanks Ergus for implementing
>> these features.
>
>Thanks, but please see some questions/comments below, and either fix
>them or explain what you meant so that I could fix them:
>
Thanks to you

>     If @code{completion-auto-help} is set to @code{nil}, the completion
>   commands never display the completion list buffer; you must type
>  -@kbd{?}  to display the list.  If the value is @code{lazy}, Emacs only
>  +@kbd{?} to display the list.  If the value is @code{lazy}, Emacs only
>   shows the completion list buffer on the second attempt to complete.
>   In other words, if there is nothing to complete, the first @key{TAB}
>   echoes @samp{Next char not unique}; the second @key{TAB} shows the
>  -completion list buffer.
>  +completion list buffer.  With the previous values and the default
>
>What are those "previous values" which you mention here?

t, nil, lazy

>
>  +@code{t} the completions are hidden when some unique completion is
>  +executed.
>
>What do you mean by "unique completion is executed"?  Both the
>"unique" and the "executed" parts need clarification.
>
unique means that, there is a single candidate o prefix so the <tab>
"added" some letter. example: com<tab>pi<tab> -> compil

The first tab shows completions but the second hides it... it is a bit
confusing compared to any other completion engine (bash, zsh,
fish)... that's why I started this changes...

> If @code{completion-auto-help} is set to @code{always}, the
>  +completion commands are always shown after a completion attempt, or
>
>"Commands"? didn't you mean "candidates" or "alternatives"?  (The same
>problem exists in the doc string of this variable, btw.)
>
Maybe candidates is better,

>  +updated if they are already visible.  If the value is @code{visible},
>  +then completions are not hidden, but updated if they are already
>  +visible while the current behavior stays the same as default if they
>  +are not.
>
>I think the last sentence should be reworded as follows:
>
>  If the value is @code{visible}, Emacs displays completion the
>  completion alternatives when it is unable to complete for the first
>  time; thereafter the completion list buffer remains visible and is
>  updated as you type.
>
>Is this accurate and correct?
>
"and is updated as you type."

I am not sure that last part is accurate. updates need <tab> to update
in all cases. "as you type" suggests more an icomplete-like behavior I
thinks.

>  +@vindex completions-header-format
>  +The variable @code{completions-header-format} is a formatted string to
>  +control the informative line shown before the completions list of
>  +candidates.  It may contain a @code{%s} to show the total number of
>  +completions.  When it is @code{nil}, the message is totally suppressed.
>  +Text properties may be added to change the appearance, some useful
>  +ones are @code{face} or @code{cursor-intangible} (@pxref{Special
>  +Properties,,Properties with Special Meanings, elisp, The Emacs Lisp
>  +Reference Manual}).
>
>Shouldn't this have a cross-reference to where header-line-format is
>described?  This variable determines the format of the header-line
>shown in the window that displays the *Completions* buffer, right?  So
>knowing what can be used in header-line-format should be useful.
>
You are right... I just didn't even know there was a header-line-format ;p

>  +@vindex completions-highlight-face
>  +When @code{completions-highlight-face} is a face name, then the
>
>Saying "face name" might mislead people to think this is a string.  It
>is better to say "If the value of this variable is a face, ..."
>
>Also, I'd lose "then": it's redundant here.
>
>  +current completion candidate will be highlighted with that face.  The
>  +default value is @code{completions-highlight}.  When the value is
>  +@code{nil}, no highlighting is performed.  This feature sets the text
>  +property @code{cursor-face}.
>
>This should explain what is "the current completion candidate".  It
>isn't trivial: trying the current defaults with completion commands, I
>sometimes see a candidate highlighted, and sometimes don't.  I
>couldn't figure out why.
>

when it is not highlighting? So far the only way to not have a candidate
highlighted is when there is not candidate effectively selected (like
when completions are just updated (with no completion-auto-select) and
the cursor is before the help line; I that case there is not "current
candidate"); a return in that moment won't execute anything because
there is nothing to select, so "current completion candidate" means
that... Maybe you have a better way to explain it?

>  +@item cursor-face
>  +@kindex cursor-face @r{(text property)}
>  +This property is similar to @code{mouse-face}, but the face is used if
>  +the cursor (instead of mouse) is on or near the character.
>                                                   ^^^^^^^^^
>Which character?  There's no reference to "characters" in the
>preceding text, so this is confusing.
>
The previous text for mouse-face says:

‘mouse-face’
      This property is used instead of ‘face’ when the mouse is on or
      near the character.

>  I suggest something like
>  when point is inside text which has this face

Maybe: when point is inside text which has cursor-face property

Please if you have the time change it... Otherwise I will do in some
days.

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Tue, 22 Mar 2022 15:18:15 +0100
> From: Ergus <spacibba@aol.com>
> Cc: Juri Linkov <juri@linkov.net>, larsi@gnus.org, emacs-devel@gnu.org
> 
> >  +@code{t} the completions are hidden when some unique completion is
> >  +executed.
> >
> >What do you mean by "unique completion is executed"?  Both the
> >"unique" and the "executed" parts need clarification.
> >
> unique means that, there is a single candidate o prefix so the <tab>
> "added" some letter. example: com<tab>pi<tab> -> compil
> 
> The first tab shows completions but the second hides it... it is a bit
> confusing compared to any other completion engine (bash, zsh,
> fish)... that's why I started this changes...

So we could simply say

  when Emacs is able to complete some characters

is that right?

> > If @code{completion-auto-help} is set to @code{always}, the
> >  +completion commands are always shown after a completion attempt, or
> >
> >"Commands"? didn't you mean "candidates" or "alternatives"?  (The same
> >problem exists in the doc string of this variable, btw.)
> >
> Maybe candidates is better,

If it's just "better", then please explain what you mean by
"commands".  The text I quoted above says "the completion commands are
always shown".  What are those "commands" that are shown after a
completion attempt?

> >  If the value is @code{visible}, Emacs displays completion the
> >  completion alternatives when it is unable to complete for the first
> >  time; thereafter the completion list buffer remains visible and is
> >  updated as you type.
> >
> >Is this accurate and correct?
> >
> "and is updated as you type."
> 
> I am not sure that last part is accurate. updates need <tab> to update
> in all cases. "as you type" suggests more an icomplete-like behavior I
> thinks.

Ok, so "the completion list buffer remains visible for all the
subsequent completion attempts".  Better?

> >  +current completion candidate will be highlighted with that face.  The
> >  +default value is @code{completions-highlight}.  When the value is
> >  +@code{nil}, no highlighting is performed.  This feature sets the text
> >  +property @code{cursor-face}.
> >
> >This should explain what is "the current completion candidate".  It
> >isn't trivial: trying the current defaults with completion commands, I
> >sometimes see a candidate highlighted, and sometimes don't.  I
> >couldn't figure out why.
> >
> 
> when it is not highlighting? So far the only way to not have a candidate
> highlighted is when there is not candidate effectively selected (like
> when completions are just updated (with no completion-auto-select) and
> the cursor is before the help line; I that case there is not "current
> candidate"); a return in that moment won't execute anything because
> there is nothing to select, so "current completion candidate" means
> that... Maybe you have a better way to explain it?

Wouldn't it make sense to move point to the first candidate, so that
some candidate is always highlighted?

Btw, when a candidate is highlighted, it is always the first one in
the list, which kinda makes me wonder why this highlighting is useful,
if it always shows the candidate in a fixed location on display?

> Please if you have the time change it... Otherwise I will do in some
> days.

Will do.

Re: Select completions from the minibuffer

[Ergus]

On Tue, Mar 22, 2022 at 04:49:59PM +0200, Eli Zaretskii wrote:
>> Date: Tue, 22 Mar 2022 15:18:15 +0100
>> From: Ergus <spacibba@aol.com>
>> Cc: Juri Linkov <juri@linkov.net>, larsi@gnus.org, emacs-devel@gnu.org
>>
>> >  +@code{t} the completions are hidden when some unique completion is
>> >  +executed.
>> >
>> >What do you mean by "unique completion is executed"?  Both the
>> >"unique" and the "executed" parts need clarification.
>> >
>> unique means that, there is a single candidate o prefix so the <tab>
>> "added" some letter. example: com<tab>pi<tab> -> compil
>>
>> The first tab shows completions but the second hides it... it is a bit
>> confusing compared to any other completion engine (bash, zsh,
>> fish)... that's why I started this changes...
>
>So we could simply say
>
>  when Emacs is able to complete some characters
>
>is that right?
>
I think so,

>> > If @code{completion-auto-help} is set to @code{always}, the
>> >  +completion commands are always shown after a completion attempt, or
>> >
>> >"Commands"? didn't you mean "candidates" or "alternatives"?  (The same
>> >problem exists in the doc string of this variable, btw.)
>> >
>> Maybe candidates is better,
>
>If it's just "better", then please explain what you mean by
>"commands".  The text I quoted above says "the completion commands are
>always shown".  What are those "commands" that are shown after a
>completion attempt?
>
We are just in violent agreement here I thing.. I am saying that as you
suggested "candidates" is a better word, more precise.

>> >  If the value is @code{visible}, Emacs displays completion the
>> >  completion alternatives when it is unable to complete for the first
>> >  time; thereafter the completion list buffer remains visible and is
>> >  updated as you type.
>> >
>> >Is this accurate and correct?
>> >
>> "and is updated as you type."
>>
>> I am not sure that last part is accurate. updates need <tab> to update
>> in all cases. "as you type" suggests more an icomplete-like behavior I
>> thinks.
>
>Ok, so "the completion list buffer remains visible for all the
>subsequent completion attempts".  Better?
>
yes,

>> >  +current completion candidate will be highlighted with that face.  The
>> >  +default value is @code{completions-highlight}.  When the value is
>> >  +@code{nil}, no highlighting is performed.  This feature sets the text
>> >  +property @code{cursor-face}.
>> >
>> >This should explain what is "the current completion candidate".  It
>> >isn't trivial: trying the current defaults with completion commands, I
>> >sometimes see a candidate highlighted, and sometimes don't.  I
>> >couldn't figure out why.
>> >
>>
>> when it is not highlighting? So far the only way to not have a candidate
>> highlighted is when there is not candidate effectively selected (like
>> when completions are just updated (with no completion-auto-select) and
>> the cursor is before the help line; I that case there is not "current
>> candidate"); a return in that moment won't execute anything because
>> there is nothing to select, so "current completion candidate" means
>> that... Maybe you have a better way to explain it?
>
>Wouldn't it make sense to move point to the first candidate, so that
>some candidate is always highlighted?
>
That may change the current behavior in a more intrusive way and I
didn't want to affect current behavior.

What you suggest is something I already considered:

completion-auto-select t makes 3 things I think should have been spitted
into 2:

1) it selects *Completions*
2) Select completions window
3) Go to the first candidate.

These options may be probably independent because the user may want to:

1) show completion and highlight (or put cursor) in the first candidate
but without selecting the *Completions* Window

2) Select completions window without putting cursor in the first
candidate.

I thought on this because in zsh the usual approach for this is:

First tab: attempts to complete common and shows completions
Second tab: go to completions and select first candidate.

Following tabs: move to next candidate

Right now the combination to get this behavior is not possible, but at
least we are closer than before and we give a more familiar interaction
with more options...

>Btw, when a candidate is highlighted, it is always the first one in
>the list, which kinda makes me wonder why this highlighting is useful,
>if it always shows the candidate in a fixed location on display?
>
I don't understand what you mean... The highlight is just the same that
used to happen before when the point selected a candidate, then arrow
navigation selects a candidate... We just added a more visible hint; not
just the pointer...

>> Please if you have the time change it... Otherwise I will do in some
>> days.
>
>Will do.

Re: Select completions from the minibuffer

[Ergus]

BTW Eli:

Some of the new and old completions customs have some name miss-matches:
some of them are called completion-something and some others are
completions-something in plural.

There are some of them with `:group 'completion` while others don't have
that... could you please check if the names could be standardized and
may be added to the right group??

Specially the options I added I am not sure if they are well named and
for sure they don't have a group name...



On Tue, Mar 22, 2022 at 04:49:59PM +0200, Eli Zaretskii wrote:
>> Date: Tue, 22 Mar 2022 15:18:15 +0100
>> From: Ergus <spacibba@aol.com>
>> Cc: Juri Linkov <juri@linkov.net>, larsi@gnus.org, emacs-devel@gnu.org
>>
>> >  +@code{t} the completions are hidden when some unique completion is
>> >  +executed.
>> >
>> >What do you mean by "unique completion is executed"?  Both the
>> >"unique" and the "executed" parts need clarification.
>> >
>> unique means that, there is a single candidate o prefix so the <tab>
>> "added" some letter. example: com<tab>pi<tab> -> compil
>>
>> The first tab shows completions but the second hides it... it is a bit
>> confusing compared to any other completion engine (bash, zsh,
>> fish)... that's why I started this changes...
>
>So we could simply say
>
>  when Emacs is able to complete some characters
>
>is that right?
>
>> > If @code{completion-auto-help} is set to @code{always}, the
>> >  +completion commands are always shown after a completion attempt, or
>> >
>> >"Commands"? didn't you mean "candidates" or "alternatives"?  (The same
>> >problem exists in the doc string of this variable, btw.)
>> >
>> Maybe candidates is better,
>
>If it's just "better", then please explain what you mean by
>"commands".  The text I quoted above says "the completion commands are
>always shown".  What are those "commands" that are shown after a
>completion attempt?
>
>> >  If the value is @code{visible}, Emacs displays completion the
>> >  completion alternatives when it is unable to complete for the first
>> >  time; thereafter the completion list buffer remains visible and is
>> >  updated as you type.
>> >
>> >Is this accurate and correct?
>> >
>> "and is updated as you type."
>>
>> I am not sure that last part is accurate. updates need <tab> to update
>> in all cases. "as you type" suggests more an icomplete-like behavior I
>> thinks.
>
>Ok, so "the completion list buffer remains visible for all the
>subsequent completion attempts".  Better?
>
>> >  +current completion candidate will be highlighted with that face.  The
>> >  +default value is @code{completions-highlight}.  When the value is
>> >  +@code{nil}, no highlighting is performed.  This feature sets the text
>> >  +property @code{cursor-face}.
>> >
>> >This should explain what is "the current completion candidate".  It
>> >isn't trivial: trying the current defaults with completion commands, I
>> >sometimes see a candidate highlighted, and sometimes don't.  I
>> >couldn't figure out why.
>> >
>>
>> when it is not highlighting? So far the only way to not have a candidate
>> highlighted is when there is not candidate effectively selected (like
>> when completions are just updated (with no completion-auto-select) and
>> the cursor is before the help line; I that case there is not "current
>> candidate"); a return in that moment won't execute anything because
>> there is nothing to select, so "current completion candidate" means
>> that... Maybe you have a better way to explain it?
>
>Wouldn't it make sense to move point to the first candidate, so that
>some candidate is always highlighted?
>
>Btw, when a candidate is highlighted, it is always the first one in
>the list, which kinda makes me wonder why this highlighting is useful,
>if it always shows the candidate in a fixed location on display?
>
>> Please if you have the time change it... Otherwise I will do in some
>> days.
>
>Will do.
>

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Tue, 22 Mar 2022 16:46:37 +0100
> From: Ergus <spacibba@aol.com>
> Cc: larsi@gnus.org, emacs-devel@gnu.org, juri@linkov.net
> 
> BTW Eli:
> 
> Some of the new and old completions customs have some name miss-matches:
> some of them are called completion-something and some others are
> completions-something in plural.
> 
> There are some of them with `:group 'completion` while others don't have
> that... could you please check if the names could be standardized and
> may be added to the right group??
> 
> Specially the options I added I am not sure if they are well named and
> for sure they don't have a group name...

For options just added it might be possible, but for options that
exist for a while, I'm not sure renaming them is a good idea, or is
even practical.

For the new ones, feel free to suggest renaming.

Re: Select completions from the minibuffer

[Ergus]

On Tue, Mar 22, 2022 at 06:59:20PM +0200, Eli Zaretskii wrote:
>> Date: Tue, 22 Mar 2022 16:46:37 +0100
>> From: Ergus <spacibba@aol.com>
>> Cc: larsi@gnus.org, emacs-devel@gnu.org, juri@linkov.net
>>
>> BTW Eli:
>>
>> Some of the new and old completions customs have some name miss-matches:
>> some of them are called completion-something and some others are
>> completions-something in plural.
>>
>> There are some of them with `:group 'completion` while others don't have
>> that... could you please check if the names could be standardized and
>> may be added to the right group??
>>
>> Specially the options I added I am not sure if they are well named and
>> for sure they don't have a group name...
>
>For options just added it might be possible, but for options that
>exist for a while, I'm not sure renaming them is a good idea, or is
>even practical.
>
>For the new ones, feel free to suggest renaming.

I suggest 2 things:

1) add the group field for all of them

2) use completionS in plural and declare the old ones that are in
singular deprecated alias for the new ones... and keep them there for
the next 10 years... the intention is to make things uniform and easier
to find (either throw completions or the custom interface...)

Re: Select completions from the minibuffer

[Ergus]

On Tue, Mar 22, 2022 at 10:24:00AM +0200, Juri Linkov wrote:
>>> Since Ergus finished implementing new features in the
>>> feature/completions-customs branch (and I fixed small things
>>> like renaming completion-header-format to completions-header-format),
>>> the branch is ready for merging to master now.  After that
>>> we could add more patches based on new features.
>>
>> Sounds good to me.
>
>So now the branch is merged to master.  Thanks Ergus for implementing
>these features.
>
Hi Juri:

I see that setting cursor-face-highlight-nonselected-window to t by
default produces a highlight a bit annoying for some users...

So I think that we may disable it and problem solved.

For your selection from minibuffer I am not sure how you plan to enable
it, but I suppose you will make a minor mode, so instead of setq-local
on every call probably you will prefer to do something like:

(defun minibuffer-select-completion-setup-function ()
    (with-current-buffer standard-output
       (when completions-highlight-face
         (setq-local cursor-face-highlight-nonselected-window t))))

(add-hook 'completion-setup-hook #'minibuffer-select-completion-setup-function)

Is it fine for you?

Re: Select completions from the minibuffer

[Juri Linkov]

> I see that setting cursor-face-highlight-nonselected-window to t by
> default produces a highlight a bit annoying for some users...
>
> So I think that we may disable it and problem solved.
>
> For your selection from minibuffer I am not sure how you plan to enable
> it, but I suppose you will make a minor mode, so instead of setq-local
> on every call probably you will prefer to do something like:
>
> (defun minibuffer-select-completion-setup-function ()
>    (with-current-buffer standard-output
>       (when completions-highlight-face
>         (setq-local cursor-face-highlight-nonselected-window t))))
>
> (add-hook 'completion-setup-hook #'minibuffer-select-completion-setup-function)
>
> Is it fine for you?

Actually, such highlighting in non-selected completions window is needed
only for new commands that use the new macro with-minibuffer-completions-window,
because this macro navigates completions in the non-selected window,
so this works fine:

```
(defun minibuffer-previous-completion (&optional n)
  "Run `previous-completion' from the minibuffer in its completions window."
  (interactive "p")
  (with-minibuffer-completions-window
    (let ((completion-wrap-movement nil))
      (setq-local cursor-face-highlight-nonselected-window t)
      (previous-completion n))))
```

Re: Select completions from the minibuffer

[Juri Linkov]

>>  +@vindex completions-header-format
>>
>>Shouldn't this have a cross-reference to where header-line-format is
>>described?  This variable determines the format of the header-line
>>shown in the window that displays the *Completions* buffer, right?  So
>>knowing what can be used in header-line-format should be useful.
>
> You are right... I just didn't even know there was a header-line-format ;p

This can't be right, header-line-format is a completely different thing.

Or maybe this is a new idea for a new value of completion-show-help
to show the completions header on the header line instead of the mode line?

Re: Select completions from the minibuffer

[Eli Zaretskii]

> Date: Tue, 22 Mar 2022 16:49:59 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: larsi@gnus.org, emacs-devel@gnu.org, juri@linkov.net
> 
> > Please if you have the time change it... Otherwise I will do in some
> > days.
> 
> Will do.

Done.

Re: Select completions from the minibuffer

[Juri Linkov]

>>So now the branch is merged to master.

Sorry, forgot to delete the branch.  Will do.

Re: Select completions from the minibuffer

[Juri Linkov]

> First tab: attempts to complete common and shows completions
> Second tab: go to completions and select first candidate.
> Following tabs: move to next candidate

This is exactly what happens when completion-auto-select is t.

Re: Select completions from the minibuffer

[Ergus]

On March 22, 2022 7:38:45 PM GMT+01:00, Juri Linkov <juri@linkov.net> wrote:
>> First tab: attempts to complete common and shows completions
>> Second tab: go to completions and select first candidate.
>> Following tabs: move to next candidate
>
>This is exactly what happens when completion-auto-select is t.


No exactly. When completion-auto-select is t the steps 1 and 2 are merged.


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: Select completions from the minibuffer

[Ergus]

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

On Tue, Mar 22, 2022 at 08:38:45PM +0200, Juri Linkov wrote:
>> First tab: attempts to complete common and shows completions
>> Second tab: go to completions and select first candidate.
>> Following tabs: move to next candidate
>
>This is exactly what happens when completion-auto-select is t.
>

Look at the attached patch that produces this behavior... I am
not sure is we want to add it, but it is useful in my case to mix

completion-auto-help 'always/'visible
completion-auto-select 'second-tab

If you think this is useful I may add it to a branch and add some
documentation.

Best,
Ergus

[-- Attachment #2: second-tab.patch --]
[-- Type: text/plain, Size: 3484 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 00d4560865..f38db6d278 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1407,18 +1407,26 @@ completion--in-region-1
    ;; and this command is repeated, scroll that window.
    ((and (window-live-p minibuffer-scroll-window)
          (eq t (frame-visible-p (window-frame minibuffer-scroll-window))))
-    (let ((window minibuffer-scroll-window)
-          (reverse (equal (this-command-keys) [backtab])))
+    (let ((window minibuffer-scroll-window))
       (with-current-buffer (window-buffer window)
-        (if (pos-visible-in-window-p (if reverse (point-min) (point-max)) window)
-            ;; If end or beginning is in view, scroll up to the
-            ;; beginning or end respectively.
-            (if reverse
-                (set-window-point window (point-max))
-              (set-window-start window (point-min) nil))
-          ;; Else scroll down one screen.
-          (with-selected-window window
-            (if reverse (scroll-down) (scroll-up))))
+        (cond
+         ;; here this is possible only when second-tab, so jump now.
+         (completion-auto-select
+          (switch-to-completions))
+         ;; reverse tab
+         ((equal (this-command-keys) [backtab])
+          (if (pos-visible-in-window-p (point-min) window)
+              ;; If beginning is in view, scroll up to the end
+              (set-window-point window (point-max))
+            ;; Else scroll down one screen.
+            (with-selected-window window (scroll-down))))
+         ;; normal tab
+         (t
+          (if (pos-visible-in-window-p (point-max) window)
+              ;; If end is in view, scroll up to the end
+              (set-window-start window (point-min) nil)
+            ;; Else scroll down one screen.
+            (with-selected-window window (scroll-up)))))
         nil)))
    ;; If we're cycling, keep on cycling.
    ((and completion-cycling completion-all-sorted-completions)
diff --git a/lisp/simple.el b/lisp/simple.el
index 9a8ed0bb75..f229608690 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9144,6 +9144,14 @@ completion-wrap-movement
   :version "29.1"
   :group 'completion)
 
+(defcustom completion-auto-select nil
+  "Non-nil means to automatically select the *Completions* buffer."
+  :type '(choice (const :tag "Select window" t)
+                 (const :tag "Disabled" nil)
+                 (const :tag "Select window on second-tab" second-tab))
+  :version "29.1"
+  :group 'completion)
+
 (defun previous-completion (n)
   "Move to the previous item in the completion list.
 With prefix argument N, move back N items (negative N means move
@@ -9365,12 +9373,6 @@ completion-show-help
   :version "22.1"
   :group 'completion)
 
-(defcustom completion-auto-select nil
-  "Non-nil means to automatically select the *Completions* buffer."
-  :type 'boolean
-  :version "29.1"
-  :group 'completion)
-
 ;; This function goes in completion-setup-hook, so that it is called
 ;; after the text of the completion list buffer is written.
 (defun completion-setup-function ()
@@ -9411,8 +9413,8 @@ completion-setup-function
 	(insert (substitute-command-keys
 		 "In this buffer, type \\[choose-completion] to \
 select the completion near point.\n\n")))))
-  (when completion-auto-select
-    (switch-to-completions)))
+  (if (eq completion-auto-select t)
+      (switch-to-completions)))
 
 (add-hook 'completion-setup-hook #'completion-setup-function)
 

Re: Select completions from the minibuffer

[Juri Linkov]

>>> First tab: attempts to complete common and shows completions
>>> Second tab: go to completions and select first candidate.
>>> Following tabs: move to next candidate
>
> Look at the attached patch that produces this behavior... I am
> not sure is we want to add it, but it is useful in my case to mix
>
> completion-auto-help 'always/'visible
> completion-auto-select 'second-tab

Thanks, I think it's useful, and it works nicely when I tested it.
So +1 for installing your patch.

> If you think this is useful I may add it to a branch and add some
> documentation.

Do you want to reuse your branch feature/completions-customs?
It's not deleted yet, although there is a rule to delete the branch
after merging.

Re: Select completions from the minibuffer

[Ergus]

On Wed, Mar 23, 2022 at 10:47:29AM +0200, Juri Linkov wrote:
>>>> First tab: attempts to complete common and shows completions
>>>> Second tab: go to completions and select first candidate.
>>>> Following tabs: move to next candidate
>>
>> Look at the attached patch that produces this behavior... I am
>> not sure is we want to add it, but it is useful in my case to mix
>>
>> completion-auto-help 'always/'visible
>> completion-auto-select 'second-tab
>
>Thanks, I think it's useful, and it works nicely when I tested it.
>So +1 for installing your patch.
>
>> If you think this is useful I may add it to a branch and add some
>> documentation.
>
>Do you want to reuse your branch feature/completions-customs?
>It's not deleted yet, although there is a rule to delete the branch
>after merging.
>
I already deleted it on yesterday... So I will recreate it.

Re: Select completions from the minibuffer

[Juri Linkov]

>>>>> First tab: attempts to complete common and shows completions
>>>>> Second tab: go to completions and select first candidate.
>>>>> Following tabs: move to next candidate
>>>
>>> Look at the attached patch that produces this behavior... I am
>>> not sure is we want to add it, but it is useful in my case to mix
>>>
>>> completion-auto-help 'always/'visible
>>> completion-auto-select 'second-tab
>>
>>Thanks, I think it's useful, and it works nicely when I tested it.
>>So +1 for installing your patch.
>>
>>> If you think this is useful I may add it to a branch and add some
>>> documentation.
>>
>>Do you want to reuse your branch feature/completions-customs?
>>It's not deleted yet, although there is a rule to delete the branch
>>after merging.
>
> I already deleted it on yesterday... So I will recreate it.

There is no need to create a branch for a single commit.
You can push it directly to master (with a NEWS entry).

Re: Select completions from the minibuffer

[Ergus]

On Wed, Mar 23, 2022 at 08:28:49PM +0200, Juri Linkov wrote:
>>>>>> First tab: attempts to complete common and shows completions
>>>>>> Second tab: go to completions and select first candidate.
>>>>>> Following tabs: move to next candidate
>>>>
>>>> Look at the attached patch that produces this behavior... I am
>>>> not sure is we want to add it, but it is useful in my case to mix
>>>>
>>>> completion-auto-help 'always/'visible
>>>> completion-auto-select 'second-tab
>>>
>>>Thanks, I think it's useful, and it works nicely when I tested it.
>>>So +1 for installing your patch.
>>>
>>>> If you think this is useful I may add it to a branch and add some
>>>> documentation.
>>>
>>>Do you want to reuse your branch feature/completions-customs?
>>>It's not deleted yet, although there is a rule to delete the branch
>>>after merging.
>>
>> I already deleted it on yesterday... So I will recreate it.
>
>There is no need to create a branch for a single commit.
>You can push it directly to master (with a NEWS entry).
>
I prefer to add changes that can be reviewed before merging ;p... I am
not a good lisper