On 18/04/2024 17:25, Spencer Baugh wrote:

>>> But then the user could change the mind
>>> and still select a candidate.  This would interfere with the
>>> contents of the minibuffer.
>>
>> Suppose they do. But the list of completions they're shown is for an
>> outdated input. Does it really make more sense to erase the current
>> input than to insert the completion where it was supposed to go?
> 
> Yeah, this patch changes the behavior in the case of an outdated
> *Completions* buffer, and the new behavior is buggy, but I think the old
> behavior was also buggy and the new behavior is better.
> 
> Example: In emacs -Q, if I type
> 
> C-x C-f ~/src/emacs/emacs-2 <TAB>
> 
> I get completions "emacs-28" and "emacs-29"
> 
> Suppose I then type /src (without hitting tab or updating the
> *Completions* buffer) so that the minibuffer contains:
> 
> ~/src/emacs/emacs-2/src
> 
> Then I click on one of the completions to choose it.
> 
> Before this patch the minibuffer will contain:
> 
> ~/src/emacs/emacs-28/
> 
> and after this patch it will contain
> 
> ~/src/emacs/emacs-28//src
> 
> Both of these are wrong IMO, but IMO the second one is closer to
> correct, and it's more consistent with the normal case (when
> *Completions* is up to date) and with in-buffer completion behavior, so
> I think this patch is an improvement and could be installed.
> 
> Still, maybe we can get it to the point where clicking on an outdated
> completion makes the minibuffer contain
> 
> ~/src/emacs/emacs-28/src
> 
> instead?  Which I think is the correct behavior.
> 
> But I don't think we need to do that before landing this patch.

That gave me an idea. With a bit more spaghetti, we can support both 
this scenario and the others mentioned previously that didn't have field 
boundaries in the "new" input:

Whenever POINT > END, we can re-query the completion boundaries again 
inside completion-list-insert-choice-function and adjust END. In theory, 
I guess we'd ideally also recompute the completion entity first (to pass 
the correct prefix and suffix), but the capf function is not in context 
anymore. As long as the field boundaries simply looks for chars in 
passed string, this should work fine.

The attached v4 adds a new step and also fixes an issue apparently 
introduced in 2021 with 0ce2f591ff9 when making 
minibuffer-completion-table and others buffer-local. The bindings for 
CTABLE and other 2 vars currently always set them to nil because the 
variables are looked up right after the current buffer is changed 
(with-current-buffer standard-output ...).

This can affect the completion--done call at the end, reverting it to 
the previous behavior. But nobody noticed the change, so...