Re: [PATCH] EUDC email addresses via completion-at-point in message-mode

[Thomas Fitzsimmons]

Hi Alexander,

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>> [...]
>>> What is your value of the variable completion-auto-help?
>>
>> It is t.
>
> Ok, so that's not the reason for you having to TAB through the
> alternatives, instead of being offered the "*Completions*" buffer.
>
>>> Dies changing the value of the variable message-expand-name-standard-ui
>>> change anything for you?
>>
>> It is currently nil.
>
> Did you try with a non-nil value?
>
>> [...]
>> Can you reproduce the scenario in your screenshot, then subsequently
>> type '-' then TAB, to complete on "emacs-" instead of "emacs"; I think
>> the completion backends will be queried again.  That's what I want to
>> avoid (if that doesn't happen for you, then I'll have to recreate my
>> testing environment and test more carefully with all the combinations of
>> the above variables, I guess).
>>
>> Did you confirm completion-table-with-cache is doing something?  If so,
>> do you mind adding a comment within eudc-capf-message-expand-name's
>> body, describing what the caching is doing, when the cache is
>> invalidated, etc.?
>
> I have done a few experiments with this version of
> completion-table-with-cache(), sprinkled with some message printing:
>
> ---------------------------- Begin Quote -----------------------------
>     (defun completion-table-with-cache (fun &optional ignore-case)
>       (let* (last-arg last-result
>              (new-fun
>               (lambda (arg)
>                 (if (and last-arg (string-prefix-p last-arg arg ignore-case))
>                     (prog1
>                         last-result
>                       (message "[cache HIT] arg = [%s]" arg))
>                   (prog1
>                       (setq last-result (funcall fun arg))
>                     (message "[cache miss] arg = [%s]" arg)
>                     (setq last-arg arg))))))
>         (completion-table-dynamic new-fun)))
> ----------------------------- End Quote ------------------------------
>
>
> Experiment #1: 3rd Party UI Package
> ===================================
>
> [1] https://github.com/minad/corfu
>
> Using the corfu package [1], I get the following output in the messages
> buffer for your use-case:
>
> --------------------------- Begin Transcript -------------------------
>         <<< typing "emacs"
>
>     eudc-capf-complete --------------------------------------
>     eudc-capf-message-expand-name
>     eudc-query-with-words
>     [cache miss] arg = []
>     [cache HIT] arg = [emacs]
>
>         <<< typing "-"
>
>     eudc-capf-complete --------------------------------------
>     eudc-capf-message-expand-name
>     [cache HIT] arg = []
>     [cache HIT] arg = [emacs-]
> ---------------------------- End Transcript --------------------------
>
> On the first message "eudc-capf-complete", I type "emacs", and on the
> second, I type "-" as you describe.
>
> I.e. eudc-query-with-words gets called once only, whereas the capf
> function gets called every time. The caching works as expected for me.
>
>
> Experiment #2: message-tab
> ==========================
>
> Similar workflow as before, but instead of the 3rd party package,
> invoking message-tab:
>
> --------------------------- Begin Transcript -------------------------
>         <<< typing "emacs"
>         <<< invoking message-tab
>
>     eudc-capf-complete --------------------------------------
>     eudc-capf-message-expand-name
>     eudc-query-with-words
>     [cache miss] arg = []
>     [cache HIT] arg = [emacs]
>     Making completion list...
>     [cache HIT] arg = []
>     nil
>     eudc-capf-complete --------------------------------------
>     eudc-capf-message-expand-name
>
>         <<< typing "-"
>
>     eudc-capf-complete --------------------------------------
>     eudc-capf-message-expand-name
>
>         <<< invoking message-tab
>
>     eudc-capf-complete --------------------------------------
>     eudc-capf-message-expand-name
>     eudc-query-with-words
>     [cache miss] arg = []
>     [cache HIT] arg = [emacs-]
>     Making completion list...
>     [cache HIT] arg = []
>     nil
>     eudc-capf-complete --------------------------------------
>     eudc-capf-message-expand-name
> ---------------------------- End Transcript --------------------------
>
> As you observed, too, eudc-query-with-words is invoked every time.
>
>
> Experiment #3: completion-at-point
> ==================================
>
> Same as #2 above, but instead of message-tab, this time invoking
> completion-at-point.
>
> The result is almost identical to #2, too, with the difference that
> instead of the nil near the end of the end of the bigger message blocks,
> it now shows a t. Not sure where this nil/t output comes from anyway?
>
>
> Conclusions
> ===========
>
> How completion-at-point is wrapped makes a significant difference. With
> the vanilla front-end(s) in message mode, the CAPF function gets invoked
> many more times than with the 3rd party package. This also seems to
> affect to what extent the cache can be taken advantage of by
> completion-at-point.

This is good analysis, thanks for writing it up.  Interesting that corfu
seems to be more efficient than the built-in front-ends.  For this use
case, it seems to me that the built-in front-ends should continue
narrowing down from what's in the cache, since no new completions will
come from the backends if the search string only gets more specific.

> Given that the behaviour appears linked to message.el, what could seem a
> useful way forward? Perhaps this is something for the maintainer of
> message.el to look into? A view from someone familiar with the details
> of completion-at-point might also be helpful?

Yes; in particular, it would be nice to have an overview node in the
manual about how everything CAPF-related is supposed to fit together,
for mode authors who want to "complete this type of thing", for
completion UI authors who want to show a list of choices in some way,
and for completion backend authors who want to provide lists to
requesters.

In any case, I think your patch is an improvement to the default
message.el behaviour, and I don't necessarily want to hold it up waiting
for built-in CAPF front-end redesigns.  Let's give it another day and if
we haven't figured out this design/optimization issue by then, I'll push
what you've provided so far, as a work-in-progress.

Thomas