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

From: Stefan Monnier <[email-masked]>
Date: Sat, 26 Oct 2024 10:49:19 -0400

> Add completion-preserve-selection, a defcustom which allows keeping the
> same selected candidate after *Completions* is updated by
> minibuffer-completion-help.

IIUC, this is a change that affects only `minibuffer-completion-help`,
which is part of the standard UI's *Completions*, right, the generic
completion infrastructure, right?

> This works correctly with choose-completion-deselect-if-after: If point
> is after a completion (such that choose-completion-deselect-if-after=t
> means it won't be treated as selected), point will still be after that
> completion after updating the list.

I can't remember what `choose-completion-deselect-if-after` is about,
sorry, but the above reads like "the code doesn't have one of the bugs
I encountered while implemented the code".  Is there more to this paragraph?

> This feature is primarily motivated by the fact that some 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.

Are there cases where the current behavior is preferable?
IOW, why do we need a config var and why does it default to nil?

> * lisp/minibuffer.el (completion-preserve-selection):
> (minibuffer-completion-help):
> (minibuffer-next-completion):
> * lisp/simple.el (choose-completion):
> (completion-setup-function):

I presume you know that this is incomplete.  🙂

[ BTW, I really regret not moving the `completion-list-mode` out of
  `simple.el`.  ]

> +  "If non-nil, `minibuffer-completion-help' preserves the selected completion candidate.
> +
> +If non-nil, and point is on a completion candidate in the displayed
> +*Completions* window, `minibuffer-completion-help' will put point on the
> +same candidate after updating *Completions*."

I think we should be more clear that it *tries* to preserve it.
After all, the selected candidate may simply be absent from the new set
of candidates.

> @@ -2624,6 +2634,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* ((window (get-buffer-window "*Completions*" 0)))
> +                (with-selected-window window
> +                  (when-let* ((beg (completions--start-of-candidate-at (point))))
> +
> +                    (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

Hmm... are we sure here that the `*Completions*`s content is related to
the current completion session?  I don't think we want to preserve the
selection when it came from an unrelated use of completion half an
hour earlier.

> @@ -4905,8 +4928,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)))
[...]
> @@ -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)

Why?

        Stefan