[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] EUDC email addresses via completion-at-point in message-mode
From: |
Thomas Fitzsimmons |
Subject: |
Re: [PATCH] EUDC email addresses via completion-at-point in message-mode |
Date: |
Thu, 05 May 2022 12:57:28 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) |
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