From: Eric Abrahamsen <eric@ericabrahamsen.net>
To: Thomas Fitzsimmons <fitzsim@fitzsim.org>
Cc: Alexander Adolf <alexander.adolf@condition-alpha.com>,
59314@debbugs.gnu.org
Subject: bug#59314: 29.0.50; EUDC and message-mode header completion
Date: Thu, 17 Nov 2022 20:21:15 -0800 [thread overview]
Message-ID: <87sfigx58k.fsf@ericabrahamsen.net> (raw)
In-Reply-To: <m3leoal0o8.fsf@fitzsim.org> (Thomas Fitzsimmons's message of "Wed, 16 Nov 2022 22:28:23 -0500")
[-- Attachment #1: Type: text/plain, Size: 8637 bytes --]
On 11/16/22 22:28 PM, Thomas Fitzsimmons wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> On 11/16/22 14:18 PM, Thomas Fitzsimmons wrote:
>>> Hi Eric,
>>>
>>> Thanks for filing this.
>>>
>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>
>>>> Address completion in message-mode has stopped working in master,
>>>> possibly as a result of 0e25a39e69acca0324c326ea8e46b1725594bff5. This
>>>> has been reported for several contact-management backends that expect to
>>>> have their completions available with <TAB>.
>>>>
>>>> `completion-at-point-functions' contains '(eudc-capf-complete
>>>> message-completion-function t) at this point -- `eudc-capf-complete'
>>>> returns no matches, and no other functions in the list are consulted.
>>>
>>> I just checked and I didn't think the recent patch I pushed,
>>> 0e25a39e6..., should have affected completion-at-point-functions. It
>>> did change the default of eudc-server-hotlist from `nil' to
>>> `(("localhost" . ecomplete) ("localhost" . mailabbrev))". I thought
>>> that should only affect EUDC users who have not customized
>>> eudc-server-hotlist.
>>>
>>> `eudc-capf-complete' was added to `message-mode' in commit
>>> 620ac6735... I'm pretty sure that commenting out this line in
>>> message.el will restore prior behaviour, but I don't yet know what prior
>>> behaviour should be (see below).
>>>
>>> (add-hook 'completion-at-point-functions #'message-completion-function nil t)
>>>
>>>> On gnus.general, someone using BBDB and corfu reported that this recipe
>>>> fixed the problem:
>>>>
>>>> (setq eudc-server-hotlist '(("localhost" . bbdb)))
>>>>
>>>> (add-hook 'message-mode-hook
>>>> (lambda ()
>>>> (setq-local completion-at-point-functions
>>>> (delq 'message-completion-function
>>>> completion-at-point-functions))))
>>>>
>>>> Someone else *not* using corfu reported that that didn't work for them.
>>>> Dunno.
>>>
>>> I'm not sure what the out-of-the-box behaviour here is meant to be. Can
>>> you make a recipe starting from "emacs -Q" (including adding dummy email
>>> addresses somewhere) that makes completion work how you want it to? For
>>> me:
>>>
>>> emacs -Q
>>> C-x m TAB
>>>
>>> inserts four spaces and prints in *Messages*:
>>>
>>> Loading eudcb-ecomplete...done
>>> Loading eudcb-mailabbrev...done
>>>
>>> (Those are new, due to 0e25a39e6... but I thought should be harmless.)
>>
>> Yuck, it's been a long time since I looked at this...
>>
>> In emacs -Q, message-mode `completion-at-point-functions' is:
>>
>> (eudc-capf-complete message-completion-function t)
>>
>> Actually that's what it is in my regular Emacs, as well. All I'd need
>> for EBDB (and BBDB and everything else) is for
>> `message-completion-function' to get called, which it isn't. I believe
>> you could allow this by having `eudc-capf-complete' return nil, or have
>> `eudc-capf-message-expand-name' return a `(list beg end <table>)'
>> structure that includes the prop `:exclusive 'no' at the end of it. That
>> would allow a fallthrough to the next function.
>>
>> 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.
>>
>> `message-completion-function' already looks at where it is in the
>> message buffer, and calls `message-expand-name' if it's in a "name"
>> header. That function consults `message-expand-name-databases', and
>> *that's* where EBDB should put its completion table, except
>> `message-expand-name-databases' is hardcoded to (or 'eudc 'bbdb) for no
>> reason.
>
> Should we set `message-expand-name-databases' to (or 'eudc 'bbdb 'ebdb)?
> Would that avoid the need to clobber `message-expand-name' for your use
> case? I'd be fine adding "known packages" there, as long as referring
> to non-core packages doesn't break anything (which it doesn't seem to,
> since BBDB is non-core, in GNU ELPA).
I don't think that option should be aware of any contact management
packages at all! I'm attaching a patch that gets message.el about
halfway to where I think it ought to be: any such packages should be
able to push their own function onto `message-expand-name-databases'.
This patch allows that while maintaining some backwards compatibility.
The whole
(and (functionp (car message-expand-name-databases))
(funcall (car message-expand-name-databases)))
part inside `message-expand-name' verges on nonsense, but that
function is very weird anyway, in that it allows multiple values in
`message-expand-name-databases' but only ever consults one of them.
I hope that the behavior hidden behind `message-expand-name-standard-ui'
becomes the new norm at some point.
Right now, if EBDB or some other package pushed a function to
`message-expand-name-databases', that function would have to behave
differently depending on whether it's called by `message-expand-name' or
called as a part of `message--name-table', but it could reliably do that
by checking if `message-expand-name-standard-ui' is non-nil or not.
One thing that might be difficult under the standard ui is the extended
cycling that BBDB/EBDB offer: expanding the initial string to a contact,
and *then* cycle through that contact's multiple mail addresses, any one
of which might not match the initial string at all. But one thing at a
time.
>> So I need to clobber `message-expand-name' altogether.
>
> When I use EUDC, I too clobber `message-mode's completion, by binding
> TAB to `eudc-expand-try-all'. Part of the effort around eudc-capf was
> trying to improve the default so that this clobbering wouldn't be
> necessary. But as you point out, we're not there yet.
I guess I don't know why you need to push `eudc-capf-complete' to
`completion-at-point-functions', when EUDC is already enabled within
`message-complete-name'.
Right now `message-completion-function' does the work of detecting where
in the message buffer point is, and delegating to different functions
depending on the result. That seems reasonable to me, as the structure
of a message buffer is message-mode's business, and other programs
shouldn't need to duplicate the work of parsing text around point. Once
we've called `message-expand-name', though, I think we should be going
back to the built-in completion machinery of merging multiple completion
tables.
If EUDC is called as a part of `message-expand-name', that seems like
enough to me. Take a hypothetical user who for some reason wants to use
*both* BBDB and EBDB. They have the choice of plugging both packages
into EUDC and simply setting `message-expand-name-databases' to '(eudc).
Or they could set it to '(bbdb ebdb-complete-mail). Or heck, they could
use BBDB via EUDC, and then set it to '(eudc ebdb-complete-mail), why
not.
Doesn't that seem like enough?
>> 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.
>
> Agreed it's currently hard to navigate, but I'd prefer to take minimal
> steps from what we have now, since people have configurations that
> depend on the current state.
>
> I think we should probably create a set of core "out-of-the-box"
> `message-mode' completion ERT tests. For example, given:
>
> "emacs -Q" + EBDB + a single EBDB entry "emacs-ert-test@ebdb.gnu.org"
>
> will "C-x m emacs TAB" work? If it won't, will the above-suggested
> `message-expand-name-databases' make it work?
>
> Once we get "emacs-ert-test" examples for @bbdb.gnu.org,
> @ecomplete.gnu.org, @mailabbrev.gnu.org, we'll be able to test how the
> various completion backends interact, and I'm thinking that will help us
> simplify TAB's default behaviour in `message-mode' (while preserving
> backward compatibility).
>
> Do you want to try adding a core ERT test for EBDB completion? Optional
> core tests are allowed to depend on GNU ELPA packages.
If we allow (and eventually expect) `message-expand-name-databases' to
contain a list of functions, I imagine the ERT test will just define its
own dummy function/data, and test that expansion happens correctly.
Hope all this isn't too obnoxious,
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
next prev parent reply other threads:[~2022-11-18 4:21 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-16 18:05 bug#59314: 29.0.50; EUDC and message-mode header completion Eric Abrahamsen
2022-11-16 19:18 ` Thomas Fitzsimmons
2022-11-16 19:46 ` Eric Abrahamsen
2022-11-16 20:54 ` Thomas Fitzsimmons
2022-11-16 22:28 ` Eric Abrahamsen
2022-11-17 1:34 ` Thomas Fitzsimmons
2022-11-17 2:04 ` Eric Abrahamsen
2022-11-17 1:16 ` Eric Abrahamsen
2022-11-17 3:32 ` Thomas Fitzsimmons
2022-11-17 3:28 ` Thomas Fitzsimmons
2022-11-18 4:21 ` Eric Abrahamsen [this message]
2022-11-19 7:42 ` Thomas Fitzsimmons
2022-11-22 0:15 ` Eric Abrahamsen
2022-11-22 15:21 ` Thomas Fitzsimmons
2022-11-24 7:24 ` Thomas Fitzsimmons
2022-11-24 22:09 ` Eric Abrahamsen
2022-11-24 9:53 ` Thomas Fitzsimmons
2022-12-01 15:46 ` Alexander Adolf
2022-12-01 16:02 ` Eric Abrahamsen
2022-12-01 15:48 ` Alexander Adolf
2022-12-01 17:49 ` Eric Abrahamsen
2022-12-02 2:50 ` Thomas Fitzsimmons
2022-12-06 20:40 ` Alexander Adolf
2022-12-06 20:52 ` Thomas Fitzsimmons
2022-12-06 23:29 ` Alexander Adolf
2022-12-07 1:51 ` Thomas Fitzsimmons
2022-12-07 3:14 ` Thomas Fitzsimmons
2022-12-07 22:10 ` Alexander Adolf
2022-12-07 22:21 ` Thomas Fitzsimmons
2022-12-08 22:34 ` Alexander Adolf
2022-12-08 22:58 ` Thomas Fitzsimmons
2022-12-10 1:40 ` Alexander Adolf
2022-12-10 14:27 ` Thomas Fitzsimmons
2022-12-12 22:10 ` Alexander Adolf
2022-12-14 1:34 ` Thomas Fitzsimmons
2022-12-14 18:07 ` Alexander Adolf
2022-12-15 3:32 ` Thomas Fitzsimmons
2022-12-19 16:09 ` Alexander Adolf
2022-12-21 17:39 ` Thomas Fitzsimmons
2022-12-11 16:08 ` Alexander Adolf
2022-12-12 12:31 ` Thomas Fitzsimmons
2022-12-07 22:20 ` Alexander Adolf
2023-02-11 3:30 ` Thomas Fitzsimmons
2023-01-31 13:04 ` Julien Cubizolles
2023-02-05 0:48 ` Thomas Fitzsimmons
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87sfigx58k.fsf@ericabrahamsen.net \
--to=eric@ericabrahamsen.net \
--cc=59314@debbugs.gnu.org \
--cc=alexander.adolf@condition-alpha.com \
--cc=fitzsim@fitzsim.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.