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

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

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

Hello Thomas,

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
> The scenario I'd like you to make work with a minimal patch is:
> Scenario 3+1 + (setq message-expand-name-standard-ui t).
>
> Did you try that? 

Yes.

> I'm pretty sure if you get that working you'll find that we don't need
> eudc-capf-complete in completion-at-point-functions (yet), since what
> was there will already do what eudc-capf-complete was trying to
> achieve.
>
> Basically, this whole part of the discussion I've been thinking in terms
> of "(setq message-expand-name-standard-ui t)".
>
> When message-expand-name-standard-ui is nil, I think we've determined
> that eudc-capf-complete's presence in completion-at-point-functions
> breaks EUDC functionality.  Do you concur?

Yes.

> If that's the case we should probably remove eudc-capf-complete from
> completion-at-point-functions for Emacs 29.1, since that would
> represent a regression.

That's one option of addressing this case. The other option would be to
apply the last patch I sent to add `(setq-local completion-styles...)`
to `eudc-capf-complete` (because this is a defect and needs to be added
in any case). Albeit with an updated message, because the message
pretends to fix the bug, which it apparently doesn't.

My preferred outcome would thus be:

1) Re-remove the `(add-hook 'completion-at-point-functions
#'eudc-capf-complete ...)` line from message.el.

2) Add the `(setq-local completion-styles ...)` fix to
`eudc-capf-complete`.

3) See if we can figure a root cause for the completion styles breakage
within reasonable time and effort.

The attached patch implements 1) and 2).

> I now understand that the intent of the code that was there before we
> added eudc-capf-complete was:
>
> message-expand-name-standard-ui nil => use EUDC multi-selector UI
> message-expand-name-standard-ui t   => use completion-at-point selector UI
>
> But the second case was already broken before we added
> eudc-capf-complete.

Indeed.

And it also broken when not using EUDC at all, as I reported in my
last message ('eudc removed from `message-expand-name-databases`).

> I want to understand and fix that case.
> [...]

My suspicion is that something about the completion styles was changed.
Mu next step would hence be to meditate over the `git blame` of
minibuffer.el.


Looking forward to your thoughts,

  --alexander
  


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Untangle-EUDC-s-completion-at-point-function-from-me.patch --]
[-- Type: text/x-patch, Size: 4554 bytes --]

From bd57cde6c45bf055c900767a4ff4ee817d25810b 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] Untangle EUDC's completion-at-point function from
 message-mode

The default completion-styles turned out to not be generous enough to
allow useful email address completion. Hence, this commit sets the
buffer-local value of completion-styles to a better suited value when
performing email completion via eudc-capf-complete, which is also no
longer added to completion-at-point-functions in message-mode.
message-mode has EUDC support already, and that would have been
duplicated functionality. Users are still free to add
eudc-capf-complete to completion-at-point-functions at their own
discretion (for example in mail-mode).

* 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/gnus/message.el (message-mode): no longer add
eudc-capf-complete to the buffer-local value of
completion-at-point-functions
---
 lisp/gnus/message.el  |  1 -
 lisp/net/eudc-capf.el | 48 ++++++++++++++++++++-----------------------
 2 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index e7d11b597b..6c10a4ae97 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -3191,7 +3191,6 @@ message-mode
     (mail-abbrevs-setup))
    ((message-mail-alias-type-p 'ecomplete)
     (ecomplete-setup)))
-  (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
   (add-hook 'completion-at-point-functions #'message-completion-function nil t)
   (unless buffer-file-name
     (message-set-auto-save-file-name))
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