bug#74019: [PATCH] Optionally preserve selected candidate across *Completions* update

[Spencer Baugh]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Or maybe instead of a buffer-local, we could have
>> minibuffer-hide-completions do (goto-char (point-min)) so no
>> completion is selected anymore.  Then it becomes harmless to reuse
>> that *Completions* buffer.
>
> Either way works for me.

OK, updated patch with this and other feedback:

>From c9b1336692c095e4f8423797e58aac9fe55325fb Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Tue, 29 Oct 2024 12:16:31 -0400
Subject: [PATCH] Preserve selected candidate across *Completions* update

When *Completions* is updated and point was on some completion
candidate, move point to the same candidate after the update.

Also, a selected completion in *Completions* is now always
highlighted, even if it was selected by the user or other code
moving point rather than by minibuffer-next-completion, because
cursor-face-highlight-nonselected-window is now set in
completion-setup-function.

Other completion UIs (e.g. ido, vertico, etc) effectively have
this behavior: whenever they update the list of completions,
they preserve whatever candidate is selected.  This matters a
lot when completions are auto-updated, but is still useful
without auto-updating.  Including this behavior is a step
towards supporting auto-updating in the default completion UI.

* lisp/minibuffer.el (minibuffer-completion-help): Preserve the
selected completion candidate across updates. (bug#74019)
(minibuffer-hide-completions): Move point to BOB.
(minibuffer-next-completion): Don't set
cursor-face-highlight-nonselected-window.
* lisp/simple.el (completions--start-of-candidate-at)
(choose-completion): Extract the current-completion-finding code
into a separate function.
(completion-setup-function): Set
cursor-face-highlight-nonselected-window.
* etc/NEWS: Announce new behavior.
---
 etc/NEWS           |  8 ++++++++
 lisp/minibuffer.el | 29 +++++++++++++++++++++++------
 lisp/simple.el     | 39 ++++++++++++++++++++++++---------------
 3 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 18b6678dce9..ad85bbf116e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -86,6 +86,14 @@ different values for completion-affecting variables like
 applies for the styles configuration in 'completion-category-overrides'
 and 'completion-category-defaults'.
 
+---
+*** Selected completion candidate is preserved across *Completions* updates.
+When point is on a completion candidate in the *Completions* buffer
+(because of 'minibuffer-next-completion' or for any other reason), point
+will still be on that candidate after *Completions* is updated with a
+new list of completions.  The candidate is automatically deselected when
+the *Completions* buffer is hidden.
+
 ** Windows
 
 +++
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 44d07557f48..9b498615926 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2624,6 +2624,12 @@ minibuffer-completion-help
              (sort-fun (completion-metadata-get all-md 'display-sort-function))
              (group-fun (completion-metadata-get all-md 'group-function))
              (mainbuf (current-buffer))
+             (current-candidate-and-offset
+              (when-let* ((buffer (get-buffer "*Completions*"))
+                          (window (get-buffer-window buffer 0)))
+                (with-current-buffer buffer
+                  (when-let* ((beg (completions--start-of-candidate-at 
(window-point window))))
+                    (cons (get-text-property beg 'completion--string) (- 
(point) beg))))))
              ;; If the *Completions* buffer is shown in a new
              ;; window, mark it as softly-dedicated, so bury-buffer in
              ;; minibuffer-hide-completions will know whether to
@@ -2647,7 +2653,7 @@ minibuffer-completion-help
             ,(when temp-buffer-resize-mode
                '(preserve-size . (nil . t)))
             (body-function
-             . ,#'(lambda (_window)
+             . ,#'(lambda (window)
                     (with-current-buffer mainbuf
                       (when completion-auto-deselect
                         (add-hook 'after-change-functions 
#'completions--after-change nil t))
@@ -2737,7 +2743,16 @@ minibuffer-completion-help
                                                      (if (eq (car bounds) 
(length result))
                                                          'exact 'finished))))))
 
-                      (display-completion-list completions nil group-fun)))))
+                      (display-completion-list completions nil group-fun)
+                      (when current-candidate-and-offset
+                        (with-current-buffer standard-output
+                          (when-let* ((match (text-property-search-forward
+                                              'completion--string (car 
current-candidate-and-offset) t)))
+                            (goto-char (prop-match-beginning match))
+                            ;; Preserve the exact offset for the sake of
+                            ;; `choose-completion-deselect-if-after'.
+                            (forward-char (cdr current-candidate-and-offset))
+                            (set-window-point window (point)))))))))
           nil)))
     nil))
 
@@ -2746,8 +2761,12 @@ minibuffer-hide-completions
   ;; FIXME: We could/should use minibuffer-scroll-window here, but it
   ;; can also point to the minibuffer-parent-window, so it's a bit tricky.
   (interactive)
-  (let ((win (get-buffer-window "*Completions*" 0)))
-    (if win (with-selected-window win (bury-buffer)))))
+  (when-let* ((win (get-buffer-window "*Completions*" 0)))
+    (with-selected-window win
+      ;; Move point off any completions, so we don't move point there
+      ;; again the next time `minibuffer-completion-help' is called.
+      (goto-char (point-min))
+      (bury-buffer))))
 
 (defun exit-minibuffer ()
   "Terminate this minibuffer argument."
@@ -4905,8 +4924,6 @@ minibuffer-next-completion
   (interactive "p")
   (let ((auto-choose minibuffer-completion-auto-choose))
     (with-minibuffer-completions-window
-      (when completions-highlight-face
-        (setq-local cursor-face-highlight-nonselected-window t))
       (if vertical
           (next-line-completion (or n 1))
         (next-completion (or n 1)))
diff --git a/lisp/simple.el b/lisp/simple.el
index 2ffd6e86e56..3a142ef14b3 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10246,6 +10246,23 @@ choose-completion-deselect-if-after
 
 This makes `completions--deselect' effective.")
 
+(defun completions--start-of-candidate-at (position)
+  "Return the start position of the completion candidate at POSITION."
+  (save-excursion
+    (goto-char position)
+    (let (beg)
+      (cond
+       ((and (not (eobp))
+             (get-text-property (point) 'completion--string))
+        (setq beg (1+ (point))))
+       ((and (not (bobp))
+             (get-text-property (1- (point)) 'completion--string))
+        (setq beg (point))))
+      (when beg
+        (or (previous-single-property-change
+             beg 'completion--string)
+            beg)))))
+
 (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.
@@ -10269,21 +10286,11 @@ choose-completion
                (or (get-text-property (posn-point (event-start event))
                                       'completion--string)
                    (error "No completion here"))
-           (save-excursion
-             (goto-char (posn-point (event-start event)))
-             (let (beg)
-               (cond
-                ((and (not (eobp))
-                      (get-text-property (point) 'completion--string))
-                 (setq beg (1+ (point))))
-                ((and (not (bobp))
-                      (get-text-property (1- (point)) 'completion--string))
-                 (setq beg (point)))
-                (t (error "No completion here")))
-               (setq beg (or (previous-single-property-change
-                              beg 'completion--string)
-                             beg))
-               (get-text-property beg 'completion--string))))))
+             (if-let* ((candidate-start
+                        (completions--start-of-candidate-at
+                         (posn-point (event-start event)))))
+                 (get-text-property candidate-start 'completion--string)
+               (error "No completion here")))))
 
       (unless (buffer-live-p buffer)
         (error "Destination buffer is dead"))
@@ -10451,6 +10458,8 @@ completion-setup-function
       (let ((base-position completion-base-position)
             (insert-fun completion-list-insert-choice-function))
         (completion-list-mode)
+        (when completions-highlight-face
+          (setq-local cursor-face-highlight-nonselected-window t))
         (setq-local completion-base-position base-position)
         (setq-local completion-list-insert-choice-function insert-fun))
       (setq-local completion-reference-buffer mainbuf)
-- 
2.39.3