From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Thomas Fitzsimmons Newsgroups: gmane.emacs.bugs Subject: bug#59314: 29.0.50; EUDC and message-mode header completion Date: Sat, 19 Nov 2022 02:42:27 -0500 Message-ID: References: <87a64q7p25.fsf@ericabrahamsen.net> <878rka1y4n.fsf@ericabrahamsen.net> <87sfigx58k.fsf@ericabrahamsen.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="26304"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Alexander Adolf , 59314@debbugs.gnu.org To: Eric Abrahamsen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Nov 19 08:43:24 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1owIVX-0006f8-L5 for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 19 Nov 2022 08:43:24 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1owIVM-0000N0-FJ; Sat, 19 Nov 2022 02:43:13 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1owIVC-0000J7-Ea for bug-gnu-emacs@gnu.org; Sat, 19 Nov 2022 02:43:04 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1owIVC-0007Bt-65 for bug-gnu-emacs@gnu.org; Sat, 19 Nov 2022 02:43:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1owIVB-00070P-U3 for bug-gnu-emacs@gnu.org; Sat, 19 Nov 2022 02:43:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Thomas Fitzsimmons Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 19 Nov 2022 07:43:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 59314 X-GNU-PR-Package: emacs Original-Received: via spool by 59314-submit@debbugs.gnu.org id=B59314.166884375826900 (code B ref 59314); Sat, 19 Nov 2022 07:43:01 +0000 Original-Received: (at 59314) by debbugs.gnu.org; 19 Nov 2022 07:42:38 +0000 Original-Received: from localhost ([127.0.0.1]:38900 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1owIUn-0006zm-Tc for submit@debbugs.gnu.org; Sat, 19 Nov 2022 02:42:38 -0500 Original-Received: from mail.fitzsim.org ([69.165.165.189]:33904) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1owIUk-0006zZ-Lc for 59314@debbugs.gnu.org; Sat, 19 Nov 2022 02:42:36 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=fitzsim.org ; s=20220430; h=Content-Type:MIME-Version:Message-ID:Date:References: In-Reply-To:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=xsMeUomzlZFziXDquEodEt+y/uX/oJl0zoB/rXlivcw=; b=LW3ZsXstNhatUAfpWjJ9mCdQer E+t33Xj2ln3FXybTiUsAvl4dG6Gy1Jeknh1EBOJl5gv/aLvsEMXvTqsHi/pi813Ewgld0D5s7S1LA 5gaEt7Iqx+Vyl46vepgD5GxAaV8nCbCfqDKEdq/2Feqp+eq5PxDx5nzVZZVhuRGCu2qqeoEKtETEY yCV+MqvoDxF37Otffp+WgdGcBRLeXQsb/pZiyaKmebbjVHTJ3bINmqw4GPU7wJ7XKdU1PxeBrH5iT fs5eJnq33MP/TpfrtBH2+hc6mGM94rFWJAckwPQ5tknAbup5T8A+c2bh0QJ4JkkVfLTFEYehPy0Uu kFS088mg==; Original-Received: from [192.168.1.1] (helo=localhost.localdomain) by mail.fitzsim.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1owIUd-000EfK-NR; Sat, 19 Nov 2022 02:42:28 -0500 In-Reply-To: <87sfigx58k.fsf@ericabrahamsen.net> (Eric Abrahamsen's message of "Thu, 17 Nov 2022 20:21:15 -0800") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:248316 Archived-At: Eric Abrahamsen writes: > On 11/16/22 22:28 PM, Thomas Fitzsimmons wrote: >> Eric Abrahamsen writes: >> >>> On 11/16/22 14:18 PM, Thomas Fitzsimmons wrote: >>>> Hi Eric, >>>> >>>> Thanks for filing this. >>>> >>>> Eric Abrahamsen 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 . >>>>> >>>>> `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 )' >>> 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, I'm experimenting with this area; thanks for the patch. I'm first trying to get sensible default behaviour just from EUDC and its backends. (eudcb-mailabbrev doesn't work for me, in particular in the case of an empty .mailrc file. Alexander seems to be offline nowadays, unfortunately. I have a patch that simplifies eudc-mailabbrev-query-internal considerably (and makes it work how I think it should), but I'd probably want Alexander to review it. Likewise with the capf functionality he added. I'll give him a little while longer to reply, but I may just proceed, since I'd like to make this more solid before Emacs 29.1 is released). I wouldn't actually propose doing the following, but to give you a sense of my perspective I'll say: my inclination would be to replace the binding: TAB 'message-tab with TAB 'eudc-expand-try-all That's what I actually do, with eudc-server-list containing a BBDB entry followed by an LDAP entry. I press "TAB" to complete entries that I'm pretty sure are locally stored in my BBDB database. If there is no entry in BBDB, that will fall back to looking in LDAP. To force getting all results from both BBDB and LDAP, I do: C-u TAB That's the extent of my email completion setup. I think the only reason this setup doesn't generalize (assuming an EUDC EBDB backend in your case) is that other people like different UIs, e.g., when the same prefix expands to multiple possible addresses (as you alluded to), what UI should one use to select? I use the UI provided by EUDC. Anyway, tonight I did manage to add ERT tests for the EUDC LDAP backend. Can you try: make -C test lisp/net/eudc-tests.log on your machine to confirm they work for you? You need /usr/sbin/slapd installed. I'll work on adding EUDC BBDB backend tests, and I may write an EBDB EUDC backend. Once I have that, I'll be able to extract all the configuration resources (like .mailrc, ecompleterc, etc.) into a tarball. Then we can use that tarball to debug/redesign out-of-the-box completion scenarios, starting from "emacs -Q + tarball", "emacs -Q + EBDB + tarball", "emacs -Q + BBDB + tarball", etc. Without starting from "emacs -Q", it's impossible for me to know what I might break in other people's configurations, when making changes to message-mode's default completion code. Thomas