Re: [PATCH] EUDC email addresses via completion-at-point in message-mode

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

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

Hello Filipp,

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> I've only skimmed through previous discussion, but will point out a few
> things in the patch.

Thanks for your helpful review! Updated patch at the very bottom of this
message.

> [...]
>> +;;; Usage:
>> +
>> +;;    (require 'eudc-capf)
>> +;;    (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
>
> As you're showing add-hook with LOCAL t, perhaps you should also show
> some context, to avoid users pasting this directly into their .emacs.

Good point. I've added a few more words of comment.

>> +(defconst eudc-capf-modes '(message-mode) "List of modes in which email \
>> +address completion is to be attempted.")
>
> Put docstring on its own line.

Done.

>> +  (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)
>
> You don't need to specify else-branch for an if.  Just omit it.  (same
> below)

Ah, well spotted. Removed.

>> +`completion-at-point'."
>> +  (if (or (and (boundp 'eudc-server) eudc-server)
>> +          (and (boundp 'eudc-server-hotlist) eudc-server-hotlist))
>
> You're requiring eudc, why should these be unbound?

Another well spotted one. I borrowed this fragment from some other code
I wrote, where eudc is not being required. Boundp removed.

>> +      (progn
>> +        (setq-local completion-styles '(substring partial-completion))
>> +        (let* ((beg (save-excursion
>> +                      (if (re-search-backward "\\([:,]\\|^\\)[ \t]*"
>> +                                              (point-at-bol) 'move)
>
> t instead of 'move?

Hm. Quoting from the docstring (re-search-forward in this case):
---------------------------- Begin Quote -----------------------------
The optional third argument NOERROR indicates how errors are handled
  when the search fails. If it is nil or omitted, emit an error; if it
  is t, simply return nil and do nothing; if it is neither nil nor t,
  move to the limit of search and return nil.
----------------------------- End Quote ------------------------------

This reads as if t ("do nothing") vs. 'move ("move to the limit of
search") should make a difference? A few quick experiments seem to
indicate that in practice both seem to behave the same though. In this
light, I'm fine with changing 'move to t.

> I also don't like the side-effect of setting completion-styles for the
> user, although only locally. 

Me neither.

> Can it be done in some other way?

Short answer: I don't think so.

Longer answer:

The motivation for putting this is that completion-at-point filters the
completion tables based on completion-styles. Thus, if completion-styles
is set too restrictive, useful results may not be offered to the user.
The default completion-styles in message-mode is '(basic
partial-completion emacs22). I wanted 'substring to be the primary as it
matches the search term anywhere within the candidate, as opposed to
'basic which matches it at the beginning only. In this respect, you
might view 'substring as a super-set of 'basic.

That said, it would probably be desirable for message mode to have
different values for both, completion-at-point-functions and
completion-styles, depending on where point is (email header, newsgroup
header, message body, etc.). But this seems like a wider discussion
about the architecture of message.el rather than this patch.


Many thanks and looking forward to your thoughts,

  --alexander



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-EUDC-email-addresses-via-completion-at-point-in-mess.patch --]
[-- Type: text/x-patch, Size: 10468 bytes --]

From ad3199033f0e317d91098a80dbc5ee7583d83443 Mon Sep 17 00:00:00 2001
From: Alexander Adolf <alexander.adolf@condition-alpha.com>
Date: Thu, 28 Apr 2022 19:12:10 +0200
Subject: [PATCH] EUDC email addresses via completion-at-point in message-mode

* lisp/net/eudc-capf.el (new file): Add new 'eudc-capf-complete'
function.
* lisp/gnus/message.el (message-mode): Add 'eudc-capf-complete' to
'completion-at-point-functions' when a 'message-mode' buffer is
created, removing the FIXME.
* doc/misc/eudc.texi (Inline Query Expansion): Add a new subsection,
describing the new 'completion-at-point' mechanism in 'message-mode'.
* etc/NEWS (EUDC): Describe the new 'completion-at-point' method.
---
 doc/misc/eudc.texi    |  23 +++++++
 etc/NEWS              |   6 ++
 lisp/gnus/message.el  |   4 +-
 lisp/net/eudc-capf.el | 136 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 167 insertions(+), 2 deletions(-)
 create mode 100644 lisp/net/eudc-capf.el

diff --git a/doc/misc/eudc.texi b/doc/misc/eudc.texi
index d2850282fe..d2a1efeed0 100644
--- a/doc/misc/eudc.texi
+++ b/doc/misc/eudc.texi
@@ -713,6 +713,7 @@ be passed to the program.
 
 @node Inline Query Expansion
 @section Inline Query Expansion
+@subsection Inline Query Expansion Using a Key Binding
 
 Inline query expansion is a powerful method to get completion from
 your directory servers.  The most common usage is for expanding names
@@ -885,6 +886,28 @@ An error is signaled.  The expansion aborts.
 Default is @code{select}
 @end defvar
 
+@subsection Inline Query Expansion Using completion-at-point
+
+In addition to providing a dedicated EUDC function for binding to a
+key shortcut (@pxref{Inline Query Expansion Using a Key Binding}),
+EUDC also provides a function to contribute search results to the
+Emacs in-buffer completion system available via the function
+@code{completion-at-point} (@pxref{Identifier
+Inquiries,,,maintaining}) in @code{message-mode} buffers
+(@pxref{Message}).  When using this mechanism, queries are made in the
+multi-server query mode of operation (@pxref{Multi-server Queries}).
+
+When a buffer in @code{message-mode} is created, EUDC's inline
+expansion function is automatically added to the variable
+@code{completion-at-point-functions}.  As a result, whenever
+@code{completion-at-point} is invoked in a @code{message-mode} buffer,
+EUDC will be queried for email addresses matching the words before
+point.  Since this will be useful only when editing specific message
+header fields that require specifying one or more email addresses, an
+additional check is performed whether point is actually in one of
+those header fields.  Thus, any matching email addresses will be
+offered for completion in suitable message header fields only, and not
+in other places, like for example the body of the message.
 
 
 @node The Server Hotlist
diff --git a/etc/NEWS b/etc/NEWS
index 70087f2629..1b3b70a46f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -889,6 +889,12 @@ is called, and the returned values are used to populate the phrase and
 comment parts (see RFC 5322 for definitions).  In both cases, the
 phrase part will be automatically quoted if necessary.
 
++++
+*** New function 'eudc-capf-complete' with message-mode integration
+EUDC can now contribute email addresses to 'completion-at-point' by
+adding the new function 'eudc-capf-complete' to
+'completion-at-point-functions' in message-mode.
+
 ** eww/shr
 
 +++
diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index cc994d3ba5..b59ab2c31d 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -51,6 +51,7 @@
 (require 'yank-media)
 (require 'mailcap)
 (require 'sendmail)
+(require 'eudc-capf)
 
 (autoload 'mailclient-send-it "mailclient")
 
@@ -3180,8 +3181,7 @@ Like `text-mode', but with these additional commands:
     (mail-abbrevs-setup))
    ((message-mail-alias-type-p 'ecomplete)
     (ecomplete-setup)))
-  ;; FIXME: merge the completion tables from ecomplete/bbdb/...?
-  ;;(add-hook 'completion-at-point-functions #'message-ecomplete-capf nil t)
+  (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
new file mode 100644
index 0000000000..0c4449fd46
--- /dev/null
+++ b/lisp/net/eudc-capf.el
@@ -0,0 +1,136 @@
+;;; eudc-capf.el --- EUDC - completion-at-point bindings  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+;;
+;; Author: Alexander Adolf
+;;
+;; This file is part of GNU Emacs.
+;;
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;    This library provides functions to deliver email addresses from
+;;    EUDC search results to `completion-at-point'.
+;;
+;;    Email address completion will likely be desirable only in
+;;    situations where designating email recipients plays a role, such
+;;    as when composing or replying to email messages, or when posting
+;;    to newsgroups, possibly with copies of the post being emailed.
+;;    Hence, modes relevant in such contexts, such as for example
+;;    `message-mode' and `mail-mode', often at least to some extent
+;;    provide infrastructure for different functions to be called when
+;;    completing in certain message header fields, or in the body of
+;;    the message.  In other modes for editing email messages or
+;;    newsgroup posts, which do not provide such infrastructure, any
+;;    completion function providing email addresses will need to check
+;;    whether the completion attempt occurs in an appropriate context
+;;    (that is, in a relevant message header field) before providing
+;;    completion candidates.  Two mechanisms are thus provided by this
+;;    library.
+;;
+;;    The first mechanism is intended for use by the modes listed in
+;;    `eudc-capf-modes', and relies on these modes adding
+;;    `eudc-capf-complete' to `completion-at-point-functions', as
+;;    would be usually done for any general-purpose completion
+;;    function.  In this mode of operation, and in order to offer
+;;    email addresses only in contexts where the user would expect
+;;    them, a check is performed whether point is on a line that is a
+;;    message header field suitable for email addresses, such as for
+;;    example "To:", "Cc:", etc.
+;;
+;;    The second mechanism is intended for when the user modifies
+;;    `message-completion-alist' to replace `message-expand-name' with
+;;    the function `eudc-capf-message-expand-name'.  As a result,
+;;    minibuffer completion (`completing-read') for email addresses
+;;    would no longer enabled in `message-mode', but
+;;    `completion-at-point' (in-buffer completion) only.
+
+;;; Usage:
+
+;;    In a major mode, or context where you want email address
+;;    completion, you would do something along the lines of:
+;;
+;;    (require 'eudc-capf)
+;;    (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
+;;
+;;    The minus one argument puts it at the front of the list so it is
+;;    called first, and the t value for the LOCAL parameter causes the
+;;    setting to be buffer local, so as to avoid modifying any global
+;;    setting.
+;;
+;;    The value of the variable `eudc-capf-modes' indicates which
+;;    major modes do such a setup as part of their initialisation
+;;    code.
+
+;;; Code:
+
+(require 'eudc)
+
+(defvar message-email-recipient-header-regexp)
+(defvar mail-abbrev-mode-regexp)
+(declare-function mail-abbrev-in-expansion-header-p "mailabbrev" ())
+
+(defconst eudc-capf-modes '(message-mode)
+  "List of modes in which email address completion is to be attempted.")
+
+;; completion functions
+
+;;;###autoload
+(defun eudc-capf-complete ()
+  "Email address completion function for `completion-at-point-functions'.
+
+This function checks whether the current major mode is one of the
+modes listed in `eudc-capf-modes', and whether point is on a line
+with a message header listing email recipients, that is, a line
+whose beginning matches `message-email-recipient-header-regexp',
+and, if the check succeeds, searches for records matching the
+words before point.
+
+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)))
+
+;;;###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
+        (setq-local completion-styles '(substring partial-completion))
+        (let* ((beg (save-excursion
+                      (if (re-search-backward "\\([:,]\\|^\\)[ \t]*"
+                                              (point-at-bol) t)
+                          (goto-char (match-end 0)))
+                      (point)))
+               (end (point))
+               (prefix (save-excursion (buffer-substring-no-properties beg end))))
+          (list beg end
+                (completion-table-with-cache
+                 (lambda (_)
+                   (eudc-query-with-words (split-string prefix "[ \t]+") t))
+                 t))))))
+
+(provide 'eudc-capf)
+;;; eudc-capf.el ends here
-- 
2.36.0