bug#81537: 32.0.50; Fixing icomplete-in-buffer requiring users to advice-add

Hi,

Sean Whitton <[email protected]> writes:

> Eshel Yaron [04/Aug  6:30pm +02] wrote:
>>> I have a command which finishes by adding :after advice to
>>> icomplete-post-command-hook.  In that advice it does something on the
>>> condition that (length= completion-all-sorted-completions 1).
>>
>> Got it.  I wouldn't worry about retaining compatibility with something
>> like that anyway, since completion-all-sorted-completions is very much
>> an implementation detail.  But as it happens, I think it should work
>> also with my patch.
>
> Okay, I guess that I should have noticed the absence of a docstring
> before using it, back whenever it was I first did.
>
> Is there some other way to do what I want that you might suggest?

Hard to say: you didn't quite tell what it is that you want, although
I understand it's trying to see if there's just one possible completion
under some conditions.
But more importantly, whatever it is exactly, there's nothing wrong with
relying on implementation details in you private config; it just comes
with the understanding that you may need to adapt when the
implementation changes.  Right?

>>>> diff --git a/lisp/icomplete.el b/lisp/icomplete.el
>>>> index c8da0e9bf9b..7b7d10f96fc 100644
>>>> --- a/lisp/icomplete.el
>>>> +++ b/lisp/icomplete.el
>>>> @@ -537,6 +537,39 @@ fido-mode
>>>>      (add-hook 'minibuffer-setup-hook #'icomplete-minibuffer-setup)
>>>>      (add-hook 'minibuffer-setup-hook #'icomplete--fido-mode-setup)))
>>>>
>>>> +(defun icomplete-in-buffer-force-complete-and-exit ()
>>>> +  (interactive "" icomplete-in-buffer-mode)
>>>> +  (minibuffer-force-complete
>>>> +   (icomplete--field-beg) (icomplete--field-end) 'dont-cycle)
>>>> +  (icomplete-in-buffer-mode -1))
>>>> +
>>>> +(defun icomplete-in-buffer-abort ()
>>>> +  (interactive "" icomplete-in-buffer-mode)
>>>> +  (icomplete-in-buffer-mode -1)
>>>> +  (keyboard-quit))
>>>
>>> This part of your patch is what worries me the most.  We have had very
>>> long discussions about getting the several different exiting functions
>>> *just* right, over the years.  I'm loathe to add more subtly different
>>> ones.
>>
>> Is there some documentation of that "just right" behavior?
>
> Unfortunately not, though, there are the old bugs that you could
> re-read.  I wouldn't recommend it!
>
>> Or specific things you notice that aren't just right?
>
> Not yet.

Alright.
So I'd say it currently seems that the completion-in-region-function API
can/should be enough for icomplete-in-buffer.

>>> I'd like to see your proposal for adding a hook but I may have similar
>>> trepidation about installing it ...
>>
>> It's just a mechanical change: rename completion--icomplete-in-buffer-p
>> to completion--FOO (where FOO doesn't include "icomplete", or any other
>> frontend name), and replace its Icomplete-specific body with:
>>
>>   (run-hook-with-args-until-success completion-FOO-functions)
>>
>> Then, in icomplete-mode, add the current
>> completion--icomplete-in-buffer-p to completion-FOO-functions.
>> That should preserve the exact behavior of the existing code, and so
>> hopefully avoid that trepidation. :)
>
> That sounds fine to me, though I'd like to see the patch before
> installing.  Thanks for figuring out this proposal.

To be clear, I don't advocate this approach (yet): it only treats the
most painful symptom of the current code (Icomplete-specific logic in
the completion infrastructure), while a proper remedy would be using the
pre-existing API with no changes in minibuffer.el.
Also, I don't know what you'd like FOO to be, or if/how the hook should
be documented...  If this is the approach you prefer, I'll have to leave
these final details to you.


Best,

Eshel