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

[Alexander Adolf <alexander.adolf@condition-alpha.com>]

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

Hello Thomas,

another lengthy explanation, for which apologies up front!

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
> OK, but then (to lengthen the conclusion) message--name-table ignores
> the default value of 'completion-styles' (or if it doesn't, it should),
> and so the default global value of 'completion-styles' should not have
> any bearing on any of these tests we're doing.  Is that correct?
> [...]

Yes, that's correct.

message--name-table responds to the metadata action by returning a list
containing (category . email), which instructs the completion-at-point
machinery to ignore the global completion-styles variable, and instead
consult the completion-category-defaults and
completion-category-overrides alists for the completion style defined
under the 'email entry (if one exists).


With an eye to fixing this bug, I think there are three plus one
scenarios to look at.

----------------------------------------------------------------------
Scenario 1

- eudc-capf-complete is at the front of completion-at-point-functions

- EUDC is not configured (eudc-server and eudc-server-hotlist are both
  nil)

What happens: eudc-capf-complete will return nil, and completion will
              proceed via message-completion-function as usual. This
              works with the current code on master.

----------------------------------------------------------------------
Scenario 2

- eudc-capf-complete is at the front of completion-at-point-functions

- EUDC is configured, but none of the back-ends has any completion
  candidates for the prefix

What happens: eudc-capf-complete will return nil, and completion will
              proceed via message-completion-function as usual. This
              works with the current code on master.

----------------------------------------------------------------------
Scenario 3

- eudc-capf-complete is at the front of completion-at-point-functions

- EUDC is configured, one or more back-ends have one or more completion
  candidates for the prefix

What happens: eudc-capf-complete is called from completion-at-point, and
              returns the list of candidates. Hence, any subsequent
              functions in the list will NOT be called. No completion
              candidates are offered to the user, however, because the
              default value of completion-styles is not suited for
              completing email addresses. This is what causes the
              behaviour described in this bug.

To fix this, the buffer-local value of completion-styles needs to be set
by eudc-capf-complete to a value suitable for completing email
addresses. Once this will done, completion proceeds as usual. I have a
patch ready that adds such a fix to eudc-capf-complete (attached to the
end of this message).

----------------------------------------------------------------------
Scenario 3+1

- eudc-capf-complete is NOT in completion-at-point-functions

- EUDC is configured, one or more back-ends have one or more completion
  candidates for the prefix

What happens: If and when any of the candidates has more than one word
              in the text preceding the "angled address", the prefix
              matching in the completion-at-point machinery somehow
              breaks, and hitting TAB another time does not bring up the
              multiple candidates minibuffer UI.

(NOTE: This scenario can't occur unless a user re-removes
eudc-capf-complete from completion-at-point-functions in code s:he puts
on message-mode-hook. END NOTE)

Practically, this means that a candidate like

                       "John <john@company.com>"

works, whereas something like

                     "John Doe <j.doe@example.org>"

breaks. Text _after_ the address (e.g. "j.doe@example.org (John Doe the
Third)") does not seem to cause any troubles, however.

I've done a couple of experiments, and it seems things work best when
the candidates are "bare" email addresses (e.g. "j.doe@example.org").

I've peeked and poked around in message--name-table a bit, but couldn't
spot anything suspicious. It seems that function does what it should,
and correctly. My impression is that the code in the completion-at-point
machinery somehow gets confused by too much white-space preceding the
email address. I am still scratching my head as to why this happens, and
whether that is a bug or a feature of completion-at-point. ;-) Will keep
you posted.

----------------------------------------------------------------------


Those were the combinations I could think of. Any further ones we should
be considering?


I think overall, the approach for end users the approach regarding
eudc-capf-complete vs. message-completion-function would be:

- users not using EUDC at all will be able to continue working as before
  (cf. scenario 1);

- users who are using EUDC, will need to "migrate" email address
  completion to eudc-capf-completion, since it gets added to (the
  buffer-local value of) completion-at-point-functions when a new
  message-mode buffer is created (cf. scenario 3); this is readily
  possible, because EUDC provides back-ends for all email address
  completion sources supported by message.el (plus additional ones not
  supported by message.el).

Scenario 2 is some kind of mixture, which could occur if the sets of
databses used by EUDC and message.el are disjoint. This could e.g
happen, if the user has - say - set up EUDC for his:her work LDAP, and
uses bbdb for non-work contacts. It would work though.


Looking forward to your thoughts,

  --alexander



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-bug-59314.patch --]
[-- Type: text/x-patch, Size: 3282 bytes --]

From d02b15f2f9bf9b83641eac4e169ba79ac5026df8 Mon Sep 17 00:00:00 2001
From: Alexander Adolf <alexander.adolf@condition-alpha.com>
Date: Fri, 9 Dec 2022 22:15:42 +0100
Subject: [PATCH] Fix bug#59314

* lisp/net/eudc-capf.el (eudc-capf-complete): set completion-styles
buffer locally to a more generous value, so that more candidates can
pass the filtering
(eudc-capf-message-expand-name): renamed to
eudc-capf--message-expand-name to mark it as an internal use function,
and improved the doc string
---
 lisp/net/eudc-capf.el | 48 ++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/lisp/net/eudc-capf.el b/lisp/net/eudc-capf.el
index e2bbd5b28b..c655c14df6 100644
--- a/lisp/net/eudc-capf.el
+++ b/lisp/net/eudc-capf.el
@@ -101,34 +101,30 @@ eudc-capf-complete
 The return value is either nil when no match is found, or a
 completion table as required for functions listed in
 `completion-at-point-functions'."
-  (if (and (seq-some #'derived-mode-p eudc-capf-modes)
-           (let ((mail-abbrev-mode-regexp message-email-recipient-header-regexp))
-             (mail-abbrev-in-expansion-header-p)))
-      (eudc-capf-message-expand-name)))
+  (when (and (or eudc-server eudc-server-hotlist)
+             (seq-some #'derived-mode-p eudc-capf-modes)
+             (let ((mail-abbrev-mode-regexp message-email-recipient-header-regexp))
+               (mail-abbrev-in-expansion-header-p)))
+    (setq-local completion-styles '(substring partial-completion))
+    (eudc-capf--message-expand-name)))
 
 ;;;###autoload
-(defun eudc-capf-message-expand-name ()
-  "Email address completion function for `message-completion-alist'.
-
-When this function is added to `message-completion-alist',
-replacing any existing entry for `message-expand-name' there,
-with an appropriate regular expression such as for example
-`message-email-recipient-header-regexp', then EUDC will be
-queried for email addresses, and the results delivered to
-`completion-at-point'."
-  (if (or eudc-server eudc-server-hotlist)
-      (progn
-        (let* ((beg (save-excursion
-                      (re-search-backward "\\([:,]\\|^\\)[ \t]*")
-                      (match-end 0)))
-               (end (point))
-               (prefix (save-excursion (buffer-substring-no-properties beg end))))
-          (let ((result
-                 (eudc-query-with-words (split-string prefix "[ \t]+") t)))
-            (when result
-              (list beg end
-                    (completion-table-with-cache
-                     (lambda (_) result) t))))))))
+(defun eudc-capf--message-expand-name ()
+  "Helper for `eudc-capf-complete'.
+
+Computes a completion table as required for functions listed in
+`completion-at-point-functions'."
+  (let* ((beg (save-excursion
+                (re-search-backward "\\([:,]\\|^\\)[ \t]*")
+                (match-end 0)))
+         (end (point))
+         (prefix (save-excursion (buffer-substring-no-properties beg end))))
+    (let ((result
+           (eudc-query-with-words (split-string prefix "[ \t]+") t)))
+      (when result
+        (list beg end
+              (completion-table-with-cache
+               (lambda (_) result) t))))))
 
 (provide 'eudc-capf)
 ;;; eudc-capf.el ends here
-- 
2.38.1