[PATCH] implement a capf for address completion

[PATCH] implement a capf for address completion

[Antoine Beaupré]

I recently enabled corfu-mode everywhere, and was disappointed to find
out that I lost tab-completion on my message buffers. At most, corfu
would pop suggestions about existing words in the buffer, but no
address completion, even when I would hit TAB. I believe this is
because `message-tab' won't attempt completion if something else
already did it, and also because, somehow,
`notmuch-address-expand-name' ends up in
`message--old-style-completion-functions'.

Now, it seems to me a simple fix is to implement a proper
capf (`completion-at-point-function') for notmuch. And that, in turn,
is actually pretty simple compared to the code hidden underneath
`notmuch-address-expand-name', which not only finds completion
candidates, but also does the whole trouble of editing the buffer.

So this patch turns `notmuch-address-expand-name' into a wrapper
around the capf, and hooks the capf instead of the old function in the
message-mode completion hooks.

This ... works. Now I have popup completion, automatically (even
before hitting TAB), in my message-mode buffers. It's a bit jarring
because I'm so used to having completion in the minibuffer, but I
think I'll get used to it.

I haven't figured out how to make an escape hatch for this, to get
autocompletion in the minibuffer the same wauy way we did
before. Maybe we'd need something like the
`notmuch-address-from-minibuffer', but interactive somehow. Not
sure. But for now, this allows me to keep corfu globally active, and I
suspect will make things easier and faster going forward.
---
 emacs/notmuch-address.el | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index f756254c..0c57add8 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -171,7 +171,7 @@ matching `notmuch-address-completion-headers-regexp'."
 	       (require 'company nil t))
       (notmuch-company-setup))
     (cl-pushnew (cons notmuch-address-completion-headers-regexp
-		      #'notmuch-address-expand-name)
+		      #'notmuch-address-complete-at-point)
 		message-completion-alist :test #'equal)))
 
 (defun notmuch-address-toggle-internal-completion ()
@@ -225,15 +225,10 @@ requiring external commands."
 	 (bound-and-true-p company-mode))
     (company-manual-begin))
    (notmuch-address-command
-    (let* ((end (point))
-	   (beg (save-excursion
-		  (re-search-backward "\\(\\`\\|[\n:,]\\)[ \t]*")
-		  (goto-char (match-end 0))
-		  (point)))
-	   (orig (buffer-substring-no-properties beg end))
-	   (completion-ignore-case t)
-	   (options (with-temp-message "Looking for completion candidates..."
-		      (notmuch-address-options orig)))
+    (let* ((capf (notmuch-address-complete-at-point))
+	   (beg (pop capf))
+	   (end (pop capf))
+	   (options (pop capf))
 	   (num-options (length options))
 	   (chosen (cond
 		    ((eq num-options 0)
@@ -256,6 +251,24 @@ requiring external commands."
 	(ding))))
    (t nil)))
 
+(defun notmuch-address-complete-at-point ()
+  "Complete the address using `notmuch-address-command'.
+
+This replaces the old `notmuch-address-expand-name' with the new
+`completion-at-point-functions' (capf) system that's compatible
+with corfu, company and friends."
+  (when notmuch-address-command
+    (let* ((end (point))
+	   (beg (save-excursion
+		  (re-search-backward "\\(\\`\\|[\n:,]\\)[ \t]*")
+		  (goto-char (match-end 0))
+		  (point)))
+	   (orig (buffer-substring-no-properties beg end))
+	   (completion-ignore-case t)
+	   (options (with-temp-message "Looking for completion candidates..."
+		      (notmuch-address-options orig))))
+      (list beg end options))))
+
 ;;; Harvest
 
 (defun notmuch-address-harvest-addr (result)
-- 
2.39.2

Re: [PATCH] implement a capf for address completion

[Antoine Beaupré]

I have *just* realized, after writing all this code and sending the
patch, that someone else might have discussed this here. Oops.

So, turns out, it *has* been discussed, but it doesn't look like anyone
tackled that problem fully just yet.

The closest we have in the archive is (author in CC):

<71a3382b-3f1b-4b81-883c-b4a8bd710888%40picnicpark.org>

... which writes a new `kea/notmuch-address-message-capf' function from
scratch. It might, however, do a better job than i do at taking into
account the "harvest" process, which I only discovered after writing the
patch and decided I would pretend it doesn't exist (and is probably
inaccurate). So that might be a flaw with my patch already, but in my
defense, WTF is that harvest thing!? :)

Another discussion about capf was about integration with EUDC:

<8a437e3f646f7972c86c4aae57ae7452%40condition-alpha.com>

But I don't quite understand what EUDC is or why it matters here. Might
be *some* overlap, not sure.

Finally, there's this whole other thread from 2022 about capf, but it's
quite diffuse and I couldn't make heads or tails of it. The head of the
thread according to the online archives is:

<m2v8xnn1l3.fsf%40guru.guru-group.fi>

... but that's a reply to... something else, so I'm not sure.

As usual, your mileage may vary. :) To test my patch, you apply it,
restart Emacs (or rewire `message-completion-alist' like Keith Amidon
did), fire off a new email, start entering an address, and hit TAB (or,
if you have corfu, just wait for the timeout).

a.
-- 
We don't need any more heroes.
We just need someone to take out recycling.
                        - Banksy

Re: [PATCH] implement a capf for address completion

[Antoine Beaupré]

On 2024-03-19 16:12:04, Antoine Beaupré wrote:

[...]

> The closest we have in the archive is (author in CC):
>
> <71a3382b-3f1b-4b81-883c-b4a8bd710888%40picnicpark.org>
>
> ... which writes a new `kea/notmuch-address-message-capf' function from
> scratch. It might, however, do a better job than i do at taking into
> account the "harvest" process, which I only discovered after writing the
> patch and decided I would pretend it doesn't exist (and is probably
> inaccurate). So that might be a flaw with my patch already, but in my
> defense, WTF is that harvest thing!? :)

Thinking about this more, I *think* the capf *can* return a *function*
instead of a list of strings, so we might be able to refactor this a bit
better and send a function that waits for the harvesting to complete, or
stream the results or something.

Above my pay grade, at this point.

-- 
We have no friends but the mountains.
                        - Kurdish saying\r

Re: [PATCH] implement a capf for address completion

[David Bremner]

Antoine Beaupré <anarcat@debian.org> writes:

>
> Now, it seems to me a simple fix is to implement a proper
> capf (`completion-at-point-function') for notmuch. And that, in turn,
> is actually pretty simple compared to the code hidden underneath
> `notmuch-address-expand-name', which not only finds completion
> candidates, but also does the whole trouble of editing the buffer.

Disclaimer: I have not looked at 'capf' before today.

Did you see the note at the end of docstring for completion-at-point
functions

    NOTE: These functions should be cheap to run since they’re sometimes
    run from ‘post-command-hook’; and they should ideally only choose
    which kind of completion table to use, and not pre-filter it based
    on the current text between START and END (e.g., they should not
    obey ‘completion-styles’).

I'm not sure how expensive #notmuch-address-options is with the
defaults, but calling it from post-command-hook does sound like it could
be problematic.

A second question I have about this change is how can we test
completion? It is quite complex already, and I'm nervous about breakage.\r

Re: [PATCH] implement a capf for address completion

[Antoine Beaupré]

On 2024-08-06 07:30:30, David Bremner wrote:
> Antoine Beaupré <anarcat@debian.org> writes:
>
>>
>> Now, it seems to me a simple fix is to implement a proper
>> capf (`completion-at-point-function') for notmuch. And that, in turn,
>> is actually pretty simple compared to the code hidden underneath
>> `notmuch-address-expand-name', which not only finds completion
>> candidates, but also does the whole trouble of editing the buffer.
>
> Disclaimer: I have not looked at 'capf' before today.
>
> Did you see the note at the end of docstring for completion-at-point
> functions
>
>     NOTE: These functions should be cheap to run since they’re sometimes
>     run from ‘post-command-hook’; and they should ideally only choose
>     which kind of completion table to use, and not pre-filter it based
>     on the current text between START and END (e.g., they should not
>     obey ‘completion-styles’).
>
> I'm not sure how expensive #notmuch-address-options is with the
> defaults, but calling it from post-command-hook does sound like it could
> be problematic.
>
> A second question I have about this change is how can we test
> completion? It is quite complex already, and I'm nervous about breakage.

I should have posted a followup to this patch already, and must say it's
really not ready at all.

I have found myself in the situation where my address completion is now
completely broken. I don't even *know* how to restore the previous
behavior.

So clearly, this patch should be ignored for now... But I do feel we
should somehow figure out how to hook ourselves in there. There's
probably a way to do some asynchronous thing here, and indeed that's how
even the normal tab completion works, from what I gathered.

But this is getting way too deep for me.

I will definitely need to look at this again soon though because my
stuff is broken, I will try to keep you updated.

a.
-- 
It is better to sit alone than in company with the bad; and it is better
still to sit with the good than alone. It better to speak to a seeker of
knowledge than to remain silent; but silence is better than idle words.
                        - Imam Bukhari\r

Re: [PATCH] implement a capf for address completion

[David Bremner]

Antoine Beaupré <anarcat@debian.org> writes:

>
> I should have posted a followup to this patch already, and must say it's
> really not ready at all.
>
> I have found myself in the situation where my address completion is now
> completely broken. I don't even *know* how to restore the previous
> behavior.
>
> So clearly, this patch should be ignored for now... But I do feel we
> should somehow figure out how to hook ourselves in there. There's
> probably a way to do some asynchronous thing here, and indeed that's how
> even the normal tab completion works, from what I gathered.

Thanks for the update / explanation. Indeed, the existing address completion
stuff is pretty complex.

d
\r

Re: [PATCH] implement a capf for address completion

[Liam Hupfer]

[-- Attachment #1: Type: text/plain, Size: 1710 bytes --]

Antoine Beaupré <anarcat@debian.org> writes:

> I should have posted a followup to this patch already, and must say it’s
> really not ready at all.
>
> I have found myself in the situation where my address completion is now
> completely broken. I don’t even *know* how to restore the previous
> behavior.
>
> So clearly, this patch should be ignored for now… But I do feel we
> should somehow figure out how to hook ourselves in there. There’s
> probably a way to do some asynchronous thing here, and indeed that’s how
> even the normal tab completion works, from what I gathered.
>
> But this is getting way too deep for me.
>
> I will definitely need to look at this again soon though because my
> stuff is broken, I will try to keep you updated.

As you mentioned upthread, there have been [previous attempts] at this,
the first of which was by (Emacs household name!) Jonas Bernoulli in
[[PATCH 0/3] emacs: allow opting out of notmuch’s address completion]. He
implemented a separate package, available from MELPA and [GitHub -
tarsius/notmuch-addr: An alternative to notmuch-address.el].

notmuch-addr works great for me with Corfu. Perhaps it can someday ship
with Notmuch as an alternative to notmuch-address or even replace it
outright when notmuch moves the minimum supported Emacs version to 27.1.

—Liam


[previous attempts] <https://nmbug.notmuchmail.org/nmweb/search/capf>

[[PATCH 0/3] emacs: allow opting out of notmuch’s address completion] <https://nmbug.notmuchmail.org/nmweb/show/20201108231150.5419-1-jonas%40bernoul.li>

[GitHub -
tarsius/notmuch-addr: An alternative to notmuch-address.el] <https://github.com/tarsius/notmuch-addr>

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] implement a capf for address completion

[Antoine Beaupré]

On 2024-08-11 10:36:07, Liam Hupfer wrote:
> Antoine Beaupré <anarcat@debian.org> writes:
>
>> I should have posted a followup to this patch already, and must say it’s
>> really not ready at all.
>>
>> I have found myself in the situation where my address completion is now
>> completely broken. I don’t even *know* how to restore the previous
>> behavior.
>>
>> So clearly, this patch should be ignored for now… But I do feel we
>> should somehow figure out how to hook ourselves in there. There’s
>> probably a way to do some asynchronous thing here, and indeed that’s how
>> even the normal tab completion works, from what I gathered.
>>
>> But this is getting way too deep for me.
>>
>> I will definitely need to look at this again soon though because my
>> stuff is broken, I will try to keep you updated.
>
> As you mentioned upthread, there have been [previous attempts] at this,
> the first of which was by (Emacs household name!) Jonas Bernoulli in
> [[PATCH 0/3] emacs: allow opting out of notmuch’s address completion]. He
> implemented a separate package, available from MELPA and [GitHub -
> tarsius/notmuch-addr: An alternative to notmuch-address.el].
>
> notmuch-addr works great for me with Corfu. Perhaps it can someday ship
> with Notmuch as an alternative to notmuch-address or even replace it
> outright when notmuch moves the minimum supported Emacs version to 27.1.

Oh! This is actually pretty interesting, i didn't realize tarsius made a
standalone package for this...

I have just tested this briefly and it *seems* to work well. For some
reason, the *first* call to it was very slow, with Emacs hanging along
with the message "Collecting email addresses..." in the message
bar... Further calls were near instantaneous even after spawning a new
buffer...

So I'm not sure what to do with this. This certainly seems really
precious stuff! Could we add this to notmuch-emacs, perhaps gated on the
Emacs version?

Running Emacs 29.4 from Debian bookworm-backports here, in case that
matters.

a.
-- 
Dr. King’s major assumption was that if you are nonviolent, if you
suffer, your opponent will see your suffering and will be moved to
change his heart. He only made one fallacious assumption: In order for
nonviolence to work, your opponent must have a conscience. The United
States has none.        - Stokely Carmichael\r

Re: [PATCH] implement a capf for address completion

[David Bremner]

Antoine Beaupré <anarcat@debian.org> writes:

>
> So I'm not sure what to do with this. This certainly seems really
> precious stuff! Could we add this to notmuch-emacs, perhaps gated on the
> Emacs version?
>
> Running Emacs 29.4 from Debian bookworm-backports here, in case that
> matters.
>

I'm more interested in changing the baseline requirement for
notmuch-emacs to emacs 27 than in shipping two address completion
frameworks.  Emacs 27 is about 4 years old now, so should be available
for most people (but not all).

From a debian point of view, the last version that shipped with emacs
pre-27 has fallen out of long term support a month ago, and even that
version has emacs 27 in backports.  People on "enterprise" systems will
probably be worse off, per usual.

d\r

Re: [PATCH] implement a capf for address completion

[Antoine Beaupré]

On 2024-08-13 10:05:40, David Bremner wrote:
> Antoine Beaupré <anarcat@debian.org> writes:
>
>>
>> So I'm not sure what to do with this. This certainly seems really
>> precious stuff! Could we add this to notmuch-emacs, perhaps gated on the
>> Emacs version?
>>
>> Running Emacs 29.4 from Debian bookworm-backports here, in case that
>> matters.
>>
>
> I'm more interested in changing the baseline requirement for
> notmuch-emacs to emacs 27 than in shipping two address completion
> frameworks.  Emacs 27 is about 4 years old now, so should be available
> for most people (but not all).
>
> From a debian point of view, the last version that shipped with emacs
> pre-27 has fallen out of long term support a month ago, and even that
> version has emacs 27 in backports.  People on "enterprise" systems will
> probably be worse off, per usual.

I'm fine with that too, what's the next step? A patch that actually
replaces the notmuch-address.el stuff with tarsis's? A reply to tarsius'
original patch? :)

I can try either, but don't have the latter in my archives, i think.

-- 
The United States is a nation of laws:
badly written and randomly enforced.
                        - Frank Zappa\r

Re: [PATCH] implement a capf for address completion

[David Bremner]

Antoine Beaupré <anarcat@debian.org> writes:

>
> I'm fine with that too, what's the next step? A patch that actually
> replaces the notmuch-address.el stuff with tarsis's? A reply to tarsius'
> original patch? :)
>
> I can try either, but don't have the latter in my archives, i think.


My plan is as follows.

1) release 0.39 soonish, after applying some fixes currently under review.
(particularly the refresh fixs in emacs).

2) Deprecate emacs older than 27.1 in 0.39.

3) wait for address completion patches to test

I'll try to test the functionality [1] at some point in that interval, but
if we are going to replace the existing address completion code we'll
need updates to the tests and documentation. Other people testing their
use cases would be welcome as well.

d

[1]: https://github.com/tarsius/notmuch-addr\r

Re: [PATCH] implement a capf for address completion

[Alexander Adolf]

[-- Attachment #1: Type: text/plain, Size: 2172 bytes --]

First of all, apologies for being late to the discussion, and for having
missed its initial part.

I have been tinkering with email address completion in message mode for
a while, and would like to add a few comments with the hope of providing
slightly bigger picture, and with the hope that it may help you in
deciding on possible ways forward.

The email completion in message.el is in a state of - let's say "layers
of history". Having code specific to several email address databases in
there doesn't seem to provide a clear road-map how future extensions
(for instance adding notmuch's address database) will work.
Notmuch-address's fiddling with message-completion-alist can get in the
way, and message-tab is a pain of its own.

I have hence experimented with removing all email address database
specific code from message.el, and making it use a single mechanism
only, EUDC (lisp/net/eudc.el). EUDC already has backends for querying
ldap, bbdb, mailabbrev, and ecomplete. Queries to EUDC deliver
aggregated results from all configured backends. I have thus written a
new EUDC backend so it can additionally query notmuch-address (code
attached for reference). Adapting that to any potential new notmuch
email address lookup would be no big deal, of course. Now, when I need
completion for an email address in (notmuch) message mode, it queries
EUDC, and I am presented a combined list of results from all EUDC
backends I have configured. Nice.

While this may be cool as an exercise, and for keeping in a personal
branch (which is what I'll do for the time being), it does not provide
any immediate benefit for the notmuch project, however, because there is
no indication as to whether and when in the future message.el may indeed
be updated to only use EUDC for email addresses.

In that light, tarsius's proposed notmuch-addr code, and without having
tried it, does seem like it could be a worthwhile improvement over the
current state of things. I'd therefore tend to welcome Antoine's
proposed steps. I'd also offer to test drive tarsius's code (or
variations of it) in my setup.


Hoping to have helped, and looking forward to your thoughts,

  --alexander



[-- Attachment #2: eudcb-notmuch-address.el --]
[-- Type: application/emacs-lisp, Size: 3378 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]