bug#59314: 29.0.50; EUDC and message-mode header completion

[Eric Abrahamsen <eric@ericabrahamsen.net>]

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


On 12/01/22 16:48 PM, Alexander Adolf wrote:
> Hello Eric,
>
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> [...]
>> In fact this whole message-mode thing is an impossible tangle, burritos
>> within burritos, and it would be great to get rid of it altogether.
>> [...]
>> So I need to clobber `message-expand-name' altogether.
>>
>> And EUDC having a function on `completion-at-point-functions' is
>> wrapping yet another burrito outside the existing burritos, because now
>> EUDC has a completion function both inside and outside message-mode's
>> own completion machinery.
>>
>> In fact the docstring of `eudc-capf-message-expand-name' makes it sound
>> like it thinks it's being called as part of `message-expand-name', but
>> now it isn't -- it's being called as part of the capf machinery. Or
>> rather, it could potentially be called in both places.
>>
>> I think a half-stick of dynamite is the only real solution.
>> [...]
>
> Perhaps we can be slightly more CONstructive that this. ;-)))
>
> I am preparing a patch to message.el which refactors
> `message-completion-alist` along the lines of this:
>
> ---------------------------- Begin Quote -----------------------------
> (defcustom message-completion-alist
>   `((,message-newgroups-header-regexp . (:category newsgroup
>                                          :fieldsep-re "\\([:,]\\|^\\)[ \t]*"
>                                          :completions ,#'message-expand-group))
>     (,message-email-recipient-header-regexp . (:category email
>                                                :fieldsep-re "\\([:,]\\|^\\)[ \t]*"
>                                                :completions ,#'eudc-capf-message-expand-name)))
>   "Alist of (RE . RECIPE), defining completion contexts.
> This variable controls how `message-completion-function' performs
> in-buffer completion.  RECIPE is either a function (deprecated),
> or a plist.
>
> When `message-completion-function' is invoked, and point is on a
> line matching one of the REs in the alist, the settings in the
> corresponding RECIPE are applied.
>
> When RECIPE is a function, it is called for completion.  RECIPE
> should be a function that obeys the same rules as those of
> `completion-at-point-functions'.
>
> When RECIPE is a plist, the properties control how in-buffer
> completion is performed.  The following properties are currently
> defined:
>
> :category
>
>     The symbol defining the category in
>     `completion-category-defaults' to use for completion.  Also
>     see `completion-category-overrides', and `completion-styles'.
>
> :fieldsep-re
>
>     The regular expression to use when scanning backwards in the
>     buffer.  All text between point, and any preceding text
>     matching this regular expression, will be used as the prefix
>     for finding completion candidates.
>
> :completions
>
>     The function that provides completions, and that obeys the
>     same rules as those of `completion-at-point-functions'.
>     In-buffer completion will be performed as if
>     `completion-at-point-functions' had been set to this value."
>   :version "29.1"
>   :group 'message
>   :type '(alist :key-type regexp
>                 :value-type (choice (plist)
>                                     (function
>                                      :tag "Completion function (deprecated)"))))
> ----------------------------- End Quote ------------------------------
>
> As you can see, `eudc-capf-message-expand-name` effectively replaces
> `message-expand-name` altogether:
>
> ---------------------------- Begin Quote -----------------------------
> (make-obsolete 'message-expand-name 'eudc-capf-message-expand-name
>                "29.1")
> ----------------------------- End Quote ------------------------------
>
> The patch goes on to remove everything relating to ecomplete,
> mailabbrev, bbdb, and the likes from message.el, too. Get rid of all the
> burritos, except one. The one and only source for email addresses in
> message.el IMHO should be EUDC, and the one and only completion UI
> should be whatever CAPF uses. Any and all sources for email addresses
> should implement back-ends for EUDC, so they can be queried via EUDC,
> which does the aggregation of results from the different sources, and
> delivers it back to message.el as one lump.
>
>
> EUDC backends:   ldap   ecomplete   mailabbrev   bbdb    you-name-it
>                \________________________ ____________________________/
>                                         V
>                                         |
>                                         V
> eudc-capf.el:            eudc-capf-message-expand-name
>                                         |
>                                         V
> message.el:               message-completion-function
>                                         |
>                                         V
> minibuffer.el                completion-at-point
>                                         |
>                                         V
>                    [ optional completion UI (for example corfu) ]
>
>
> As you may imagine, this is a bigger patch, and I am discussing it on
> emacs-devel with Stefan Monnier. So it'll still be a little while until
> it might hopefully get merged, and the burritos unwrapped.
>
>
> Many thanks and looking forward to your thoughts,

My only thought is that we already have a mechanism for combining
results from multiple contact-management packages, and it's called
`message--name-table'. I don't see why we should be obliged to add EUDC
as an additional and obligatory point of collation. I'm attaching a
patch I floated earlier, showing how I think it could works -- it's very
simple.

Stefan was the one who added `message--name-table', and if you're
talking to him I will obviously defer to whatever you (plural) decide.
But that's my two cents.

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: message-name-databases.diff --]
[-- Type: text/x-patch, Size: 2816 bytes --]

diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index 3bbd68bdcd..e609aa7405 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -8266,9 +8266,11 @@ message-completion-alist
 (defcustom message-expand-name-databases
   '(bbdb eudc)
   "List of databases to try for name completion (`message-expand-name').
-Each element is a symbol and can be `bbdb' or `eudc'."
+Each element can be the symbol `bbdb', the symbol `eudc', or a function."
   :group 'message
-  :type '(set (const bbdb) (const eudc)))
+  :version "29.1"
+  :type '(repeat
+          (choice (const bbdb) (const eudc) function)))
 
 (defcustom message-tab-body-function nil
   "Function to execute when `message-tab' (TAB) is executed in the body.
@@ -8379,6 +8381,8 @@ message-expand-name
                ;; completion took place.  So let's double check the buffer was
                ;; not modified.
                (/= starttick (buffer-modified-tick)))))
+        ((and (functionp (car message-expand-name-databases))
+              (funcall (car message-expand-name-databases))))
 	(t
 	 (expand-abbrev))))
 
@@ -8408,26 +8412,28 @@ message--bbdb-query-with-words
 
 (defun message--name-table (orig-string)
   (let ((orig-words (split-string orig-string "[ \t]+"))
-        eudc-responses
-        bbdb-responses)
+        database-responses)
     (lambda (string pred action)
       (pcase action
         ('metadata '(metadata (category . email)))
         ('lambda t)
         ((or 'nil 't)
          (when orig-words
-           (when (and (memq 'eudc message-expand-name-databases)
-		      (boundp 'eudc-protocol)
-		      eudc-protocol)
-	     (setq eudc-responses (eudc-query-with-words orig-words)))
-	   (when (memq 'bbdb message-expand-name-databases)
-	     (setq bbdb-responses (message--bbdb-query-with-words orig-words)))
+           (dolist (db message-expand-name-databases)
+             (push
+              (pcase db
+                ((and `eudc (guard (bound-and-true-p eudc-protocol)))
+                 (eudc-query-with-words orig-words))
+                (`bbdb (message--bbdb-query-with-words orig-words))
+                ((pred functionp) (funcall db orig-words)))
+              database-responses))
 	   (ecomplete-setup)
 	   (setq orig-words nil))
          (let ((candidates
 	        ;; FIXME: Add `expand-abbrev'!
-	        (append (all-completions string eudc-responses pred)
-	                (all-completions string bbdb-responses pred)
+	        (append (mapcan (lambda (resp)
+                                  (all-completions string resp pred))
+                                database-responses)
 	                (when (and (bound-and-true-p ecomplete-database)
 	                           (fboundp 'ecomplete-completion-table))
                           (all-completions string