Re: Navigating completions from minibuffer

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Spencer Baugh <sbaugh@catern.com>
>> Date: Sat, 25 Nov 2023 15:19:37 +0000 (UTC)
>> Cc: emacs-devel@gnu.org
>> 
>> +(defcustom completions-auto-update 'deselect
>> +  "If non-nil, change the *Completions* buffer as you type.
>                   ^^^^^^
> "Update", not "change".

Fixed.

>> +If `deselect', if a completion candidate in *Completions* is
>> +selected (point is on it), it will be deselected (point will be
>> +moved just before it) when the minibuffer point or contents
>> +change.
>
> This has several issues.
>
> First, "If `deselect'" is sub-optimal; it is better to say
>
>   The value of `deselect' means that ...
>
> The double "if" ("If `deselect', if a completion candidate...") is
> also sub-optimal.  At the very least, if you need to say something
> like that, say
>
>   If this, then if that, ...

Fixed by rewriting the docstring.

> Finally, I'm guessing that the value `deselect' means to deslect the
> candidate _in_addition_to_ update "*Completions*" as the user types.
> If my guess is correct, please say
>
>   The value of `deselect' means that in addition to updating
>   *Completions*, the selected candidate will be deselected...
>
>> +This only affects the *Completions* buffer if it is already
>> +displayed."
>
> Not sure what this sentence adds.  What will happen if you drop it?

Dropped.

>> +  :type '(choice (const :tag "*Completions* doesn't change as you type" nil)
>> +                 (const :tag "Typing deselects any completion candidate in 
>> *Completions*" deselect))
>
> And now I see that my guess above perhaps was wrong: there's no value
> t in the choice?  Then why not make this a simple boolean?

I want to add another value later to support actually updating
*Completions* as the user types.  That value will include the current
'deselect behavior, but do even more, using the same machinery.  By just
having one option we avoid proliferation of completion-related options.

Actually, though, I think updating *Completions* doesn't have to mean
deselecting the currently selected candidate.  It might make sense to
support completions-auto-deselect=nil and completions-auto-update=t.  So
a separate option makes sense.

So I just change this option to be completions-auto-deselect, which is
specific to deselecting on typing.

>> +(defun completions--deselect ()
>> +  "If in a completion candidate, move to just after the end of it.
>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> "If point is at a completion candidate, ..."
>
>> +(defun completions--post-command ()
>> +  "Update displayed *Completions* buffer after change in minibuffer point."
>> +  (when (and (minibufferp) (not (eq minibuffer--old-point (point))))
>> +    (setq minibuffer--old-point (point))
>> +    (unless (and completions-auto-update
>> +                 (memq this-command '(minibuffer-next-completion 
>> minibuffer-previous-completion)))
>> +      (completions--update-if-displayed))))
>
> Ugh, another post-command hook...  It is small wonder users complain
> that Emacs is slow to respond and feels sluggish.  What will happen if
> the user types fast?  Could there be a design that doesn't use
> post-command-hook?

Yes, actually, this can be dropped for now.  We only really need the
after-change-based updating.

The post-command-hook only serves to deselect the completion candidate
when the user moves point.  But that's probably not necessary and maybe
even undesirable.

The core of all of this is that we need to update what's displayed in a
different buffer (*Completions*) based on what the user is doing and
typing in one buffer (the minibuffer).  So I think a hook-based design
is our only option?  (Maybe redisplay of *Completions* could check if
the first buffer has changed and modify it?)

Anyway, dropped the post-command-hook.

Fixed patch:

>From ece9fefeb267a24a7a09b1828104b619a62ea2a7 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Thu, 23 Nov 2023 13:37:29 +0000
Subject: [PATCH] Deselect the selected completion candidate when typing

minibuffer-choose-completion-or-exit submits the selected completion
candidate, if any, ignoring the contents of the minibuffer.  But a
user might select a completion candidate and then want to type
something else in the minibuffer and submit what they typed.

Now typing will automatically deselect the selected completion
candidate so that minibuffer-choose-completion-or-exit will not choose
it.

minibuffer-choose-completion has the same behavior as before, and is
not affected by the deselection.

* lisp/minibuffer.el (completions-auto-deselect, completions--deselect)
(completions--after-change): Add.
(minibuffer-completion-help): Add completions--after-change hook.
(minibuffer-next-completion): Bind completions-auto-deselect to nil to
avoid immediately deselecting the completion.
(minibuffer-choose-completion-or-exit): Bind
choose-completion-deselect-if-after so deselection takes effect.
* lisp/simple.el (choose-completion-deselect-if-after): Add.
(choose-completion): Check choose-completion-deselect-if-after.
---
 lisp/minibuffer.el | 36 ++++++++++++++++++++++++++++++++++--
 lisp/simple.el     | 10 +++++++++-
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5c12d9fc914..69483ad2aa4 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2378,6 +2378,33 @@ completions--fit-window-to-buffer
         (resize-temp-buffer-window win))
     (fit-window-to-buffer win completions-max-height)))
 
+(defcustom completions-auto-deselect t
+  "If non-nil, deselect the selected completion candidate when you type.
+
+A non-nil value means that after typing, point in *Completions*
+will be moved off any completion candidates.  This means
+`minibuffer-choose-completion-or-exit' will exit with the
+minibuffer's current contents, instead of a completion candidate."
+  :type '(choice (const :tag "Candidates in *Completions* stay selected as you 
type" nil)
+                 (const :tag "Typing deselects any completion candidate in 
*Completions*" t))
+  :version "30.1")
+
+(defun completions--deselect ()
+  "If point is in a completion candidate, move to just after the end of it.
+
+The candidate will still be chosen by `choose-completion' unless
+`choose-completion-deselect-if-after' is non-nil."
+  (when (get-text-property (point) 'mouse-face)
+    (goto-char (or (next-single-property-change (point) 'mouse-face)
+                   (point-max)))))
+
+(defun completions--after-change (_start _end _old-len)
+  "Update displayed *Completions* buffer after change in buffer contents."
+  (when completions-auto-deselect
+    (when-let (window (get-buffer-window "*Completions*" 0))
+      (with-selected-window window
+        (completions--deselect)))))
+
 (defun minibuffer-completion-help (&optional start end)
   "Display a list of possible completions of the current minibuffer contents."
   (interactive)
@@ -2400,6 +2427,7 @@ minibuffer-completion-help
           ;; If there are no completions, or if the current input is already
           ;; the sole completion, then hide (previous&stale) completions.
           (minibuffer-hide-completions)
+          (remove-hook 'after-change-functions #'completions--after-change t)
           (if completions
               (completion--message "Sole completion")
             (unless completion-fail-discreetly
@@ -2460,6 +2488,8 @@ minibuffer-completion-help
             (body-function
              . ,#'(lambda (_window)
                     (with-current-buffer mainbuf
+                      (when completions-auto-deselect
+                        (add-hook 'after-change-functions 
#'completions--after-change t))
                       ;; Remove the base-size tail because `sort' requires a 
properly
                       ;; nil-terminated list.
                       (when last (setcdr last nil))
@@ -4673,7 +4703,8 @@ minibuffer-next-completion
           (next-line-completion (or n 1))
         (next-completion (or n 1)))
       (when auto-choose
-        (let ((completion-use-base-affixes t))
+        (let ((completion-use-base-affixes t)
+              (completions-auto-deselect nil))
           (choose-completion nil t t))))))
 
 (defun minibuffer-previous-completion (&optional n)
@@ -4721,7 +4752,8 @@ minibuffer-choose-completion-or-exit
 contents."
   (interactive "P")
   (condition-case nil
-      (minibuffer-choose-completion no-exit no-quit)
+      (let ((choose-completion-deselect-if-after t))
+        (minibuffer-choose-completion no-exit no-quit))
     (error (minibuffer-complete-and-exit))))
 
 (defun minibuffer-complete-history ()
diff --git a/lisp/simple.el b/lisp/simple.el
index 02c68912dba..791b7dddedc 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10094,6 +10094,11 @@ next-line-completion
           (if pos (goto-char pos))))
       (setq n (1+ n)))))
 
+(defvar choose-completion-deselect-if-after nil
+  "If non-nil, don't choose a completion candidate if point is right after it.
+
+This makes `completions--deselect' effective.")
+
 (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.
@@ -10114,6 +10119,9 @@ choose-completion
           (insert-function completion-list-insert-choice-function)
           (completion-no-auto-exit (if no-exit t completion-no-auto-exit))
           (choice
+           (if choose-completion-deselect-if-after
+               (substring-no-properties
+                (get-text-property (posn-point (event-start event)) 
'completion--string))
            (save-excursion
              (goto-char (posn-point (event-start event)))
              (let (beg)
@@ -10129,7 +10137,7 @@ choose-completion
                               beg 'completion--string)
                              beg))
                (substring-no-properties
-                (get-text-property beg 'completion--string))))))
+                (get-text-property beg 'completion--string)))))))
 
       (unless (buffer-live-p buffer)
         (error "Destination buffer is dead"))
-- 
2.42.1