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

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

[Eric Abrahamsen]

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.

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.

Eric

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

[Thomas Fitzsimmons]

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.)

Thanks,
Thomas

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

[Eric Abrahamsen]

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.

So I need to clobber `message-expand-name' altogether.

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.

Eric

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

[Thomas Fitzsimmons]

Hi Eric,

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.

Ah, OK, that's what happened then.  The most recent patch I pushed made
`eudc-server-hotlist' non-nil by default, which makes
`eudc-capf-message-expand-name' do something other than return nil.

Can you try just (setq eudc-server-hotlist nil) and confirm that avoids
the breakage?

If it does, I'll revert that part of the patch for now.

Thanks,
Thomas

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

[Eric Abrahamsen]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> Hi Eric,
>
> 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.
>
> Ah, OK, that's what happened then.  The most recent patch I pushed made
> `eudc-server-hotlist' non-nil by default, which makes
> `eudc-capf-message-expand-name' do something other than return nil.
>
> Can you try just (setq eudc-server-hotlist nil) and confirm that avoids
> the breakage?
>
> If it does, I'll revert that part of the patch for now.

It does! Thanks.

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

[Eric Abrahamsen]

On 11/16/22 15:54 PM, Thomas Fitzsimmons wrote:
> Hi Eric,
>
> 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.
>
> Ah, OK, that's what happened then.  The most recent patch I pushed made
> `eudc-server-hotlist' non-nil by default, which makes
> `eudc-capf-message-expand-name' do something other than return nil.
>
> Can you try just (setq eudc-server-hotlist nil) and confirm that avoids
> the breakage?
>
> If it does, I'll revert that part of the patch for now.

Also, I didn't mean that rant to be directed at you! I realized it might
have come off that way.

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

[Thomas Fitzsimmons]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>> Hi Eric,
>>
>> 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.
>>
>> Ah, OK, that's what happened then.  The most recent patch I pushed made
>> `eudc-server-hotlist' non-nil by default, which makes
>> `eudc-capf-message-expand-name' do something other than return nil.
>>
>> Can you try just (setq eudc-server-hotlist nil) and confirm that avoids
>> the breakage?
>>
>> If it does, I'll revert that part of the patch for now.
>
> It does! Thanks.

As I was considering reverting the default change, I figured that this
is likely a bug in `eudc-capf-message-expand-name'; it should return nil
if it gets no results from any EUDC backend, right?  I pushed a fix for
that.  Can you apply it to your tree and see if your completion setup
works again?

Thanks,
Thomas

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

[Eric Abrahamsen]

On 11/16/22 20:34 PM, Thomas Fitzsimmons wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>>
>>> Hi Eric,
>>>
>>> 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.
>>>
>>> Ah, OK, that's what happened then.  The most recent patch I pushed made
>>> `eudc-server-hotlist' non-nil by default, which makes
>>> `eudc-capf-message-expand-name' do something other than return nil.
>>>
>>> Can you try just (setq eudc-server-hotlist nil) and confirm that avoids
>>> the breakage?
>>>
>>> If it does, I'll revert that part of the patch for now.
>>
>> It does! Thanks.
>
> As I was considering reverting the default change, I figured that this
> is likely a bug in `eudc-capf-message-expand-name'; it should return nil
> if it gets no results from any EUDC backend, right?  I pushed a fix for
> that.  Can you apply it to your tree and see if your completion setup
> works again?

Yes that worked, rebuilt Emacs with no additional changes.

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

[Thomas Fitzsimmons]

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).

> 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.

> 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.

Thanks,
Thomas

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

[Thomas Fitzsimmons]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 11/16/22 15:54 PM, Thomas Fitzsimmons wrote:
>> Hi Eric,
>>
>> 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.
>>
>> Ah, OK, that's what happened then.  The most recent patch I pushed made
>> `eudc-server-hotlist' non-nil by default, which makes
>> `eudc-capf-message-expand-name' do something other than return nil.
>>
>> Can you try just (setq eudc-server-hotlist nil) and confirm that avoids
>> the breakage?
>>
>> If it does, I'll revert that part of the patch for now.
>
> Also, I didn't mean that rant to be directed at you! I realized it might
> have come off that way.

No, I didn't interpret it that way; I appreciated you digging into the
complexity.  I still have to figure out how to hook in
`eudc-expand-try-all' so that all configured EUDC backends are attempted
by default.  (See the ERT idea in my other response).

Thanks,
Thomas

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

[Eric Abrahamsen]

[-- 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

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

[Thomas Fitzsimmons]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> 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,

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

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

[Eric Abrahamsen]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:


[...]

> 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

No love!

Running 11 tests (2022-11-21 16:04:40-0800, selector `(not (or (tag :unstable) (tag :nativecomp)))')
   passed   1/11  eudc--plist-member (0.000396 sec)
   passed   2/11  eudc-lax-plist-get (0.000433 sec)
   passed   3/11  eudc-plist-get (0.000417 sec)
   passed   4/11  eudc-plist-member (0.000390 sec)
   passed   5/11  eudc-test-make-address (0.000151 sec)
   passed   6/11  eudc-test-rfc5322-quote-phrase (0.000067 sec)
   passed   7/11  eudc-test-rfc5322-valid-comment-p (0.000760 sec)
  skipped   8/11  eudcb-bbdb (0.000102 sec)
   passed   9/11  eudcb-ecomplete (0.007451 sec)
Loading eudcb-ldap...
Parsing results...
Parsing results... done
Parsing results...
Parsing results... done
Test eudcb-ldap backtrace:
  signal(error ("No match"))
  apply(signal (error ("No match")))
  (setq value-640 (apply fn-638 args-639))
  (unwind-protect (setq value-640 (apply fn-638 args-639)) (setq form-
  (if (unwind-protect (setq value-640 (apply fn-638 args-639)) (setq f
  (let (form-description-642) (if (unwind-protect (setq value-640 (app
  (let ((value-640 'ert-form-evaluation-aborted-641)) (let (form-descr
  (let* ((fn-638 #'equal) (args-639 (condition-case err (let ((signal-
  (let ((ldap-process (start-process "slapd" "*slapd*" "/usr/sbin/slap
  (closure (t) nil (let ((value-636 (gensym "ert-form-evaluation-abort
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name eudcb-ldap :documentation "Test the L
  ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
  ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
  ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
  ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
  eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
  command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/net/eudc-tests.el" "
  command-line()
  normal-top-level()
Test eudcb-ldap condition:
    (error "No match")
   FAILED  10/11  eudcb-ldap (1.024022 sec) at lisp/net/eudc-tests.el:271
Parsing /home/eric/dev/emacs/test/lisp/net/eudc-resources/mailrc...
Parsing /home/eric/dev/emacs/test/lisp/net/eudc-resources/mailrc... done
   passed  11/11  eudcb-mailabbrev (0.002174 sec)

Ran 11 tests, 9 results as expected, 1 unexpected, 1 skipped (2022-11-21 16:04:41-0800, 1.151547 sec)

1 unexpected results:
   FAILED  eudcb-ldap

1 skipped results:
  SKIPPED  eudcb-bbdb


> 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.

I've already said this, but I don't think we should be pushing things in
a direction that relies any external package or its particular behavior.
The capf machinery should be enough, and that's very clearly defined.

Eric

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

[Thomas Fitzsimmons]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
> [...]
>
>> 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
>
> No love!
>
> Running 11 tests (2022-11-21 16:04:40-0800, selector `(not (or (tag :unstable) (tag :nativecomp)))')
>    passed   1/11  eudc--plist-member (0.000396 sec)
>    passed   2/11  eudc-lax-plist-get (0.000433 sec)
>    passed   3/11  eudc-plist-get (0.000417 sec)
>    passed   4/11  eudc-plist-member (0.000390 sec)
>    passed   5/11  eudc-test-make-address (0.000151 sec)
>    passed   6/11  eudc-test-rfc5322-quote-phrase (0.000067 sec)
>    passed   7/11  eudc-test-rfc5322-valid-comment-p (0.000760 sec)
>   skipped   8/11  eudcb-bbdb (0.000102 sec)
>    passed   9/11  eudcb-ecomplete (0.007451 sec)
> Loading eudcb-ldap...
> Parsing results...
> Parsing results... done
> Parsing results...
> Parsing results... done
> Test eudcb-ldap backtrace:
>   signal(error ("No match"))
>   apply(signal (error ("No match")))
>   (setq value-640 (apply fn-638 args-639))
>   (unwind-protect (setq value-640 (apply fn-638 args-639)) (setq form-
>   (if (unwind-protect (setq value-640 (apply fn-638 args-639)) (setq f
>   (let (form-description-642) (if (unwind-protect (setq value-640 (app
>   (let ((value-640 'ert-form-evaluation-aborted-641)) (let (form-descr
>   (let* ((fn-638 #'equal) (args-639 (condition-case err (let ((signal-
>   (let ((ldap-process (start-process "slapd" "*slapd*" "/usr/sbin/slap
>   (closure (t) nil (let ((value-636 (gensym "ert-form-evaluation-abort
>   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>   ert-run-test(#s(ert-test :name eudcb-ldap :documentation "Test the L
>   ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
>   ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
>   ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
>   ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
>   eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
>   command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/net/eudc-tests.el" "
>   command-line()
>   normal-top-level()
> Test eudcb-ldap condition:
>     (error "No match")
>    FAILED  10/11  eudcb-ldap (1.024022 sec) at lisp/net/eudc-tests.el:271
> Parsing /home/eric/dev/emacs/test/lisp/net/eudc-resources/mailrc...
> Parsing /home/eric/dev/emacs/test/lisp/net/eudc-resources/mailrc... done
>    passed  11/11  eudcb-mailabbrev (0.002174 sec)
>
> Ran 11 tests, 9 results as expected, 1 unexpected, 1 skipped (2022-11-21 16:04:41-0800, 1.151547 sec)
>
> 1 unexpected results:
>    FAILED  eudcb-ldap
>
> 1 skipped results:
>   SKIPPED  eudcb-bbdb

Thanks for trying.

Can you try changing "(sleep-for 1)" to "(sleep-for 5)"?  (In a
subsequent patch I'll replace the sleep with a retry loop to make this
more reliable.)

If that doesn't work, then below is roughly what the test is doing.  Can
you see if this works for you on the command line?  You can change "-d
0" => "-d 255" for lots of debugging output.

The test first changes to the "test/lisp/net" directory, which is
important, because the slapd.conf line "directory eudc-resources"
specifies a relative directory.

cd test/lisp/net
/usr/sbin/slapd -h "ldap://127.0.0.1:3899" \
    -d 0 -4 \
    -f eudc-resources/slapd.conf & \
sleep 5 && \
ldapsearch -x -LLL -h127.0.0.1:3899 \
    -b "dc=gnu,dc=org" "cn=emacs-ert-test-1"
=>
dn: cn=emacs-ert-test-1,dc=gnu,dc=org
objectClass: OpenLDAPperson
cn: emacs-ert-test-1
description:: RW1hY3Mg
uid: 1
sn: ERT1
givenName: Emacs
mail: emacs-ert-test-1@ldap.gnu.org

Thanks,
Thomas

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

[Thomas Fitzsimmons]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>>
>> [...]
>>
>>> 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
>>
>> No love!
>>
>> Running 11 tests (2022-11-21 16:04:40-0800, selector `(not (or (tag :unstable) (tag :nativecomp)))')
>>    passed   1/11  eudc--plist-member (0.000396 sec)
>>    passed   2/11  eudc-lax-plist-get (0.000433 sec)
>>    passed   3/11  eudc-plist-get (0.000417 sec)
>>    passed   4/11  eudc-plist-member (0.000390 sec)
>>    passed   5/11  eudc-test-make-address (0.000151 sec)
>>    passed   6/11  eudc-test-rfc5322-quote-phrase (0.000067 sec)
>>    passed   7/11  eudc-test-rfc5322-valid-comment-p (0.000760 sec)
>>   skipped   8/11  eudcb-bbdb (0.000102 sec)
>>    passed   9/11  eudcb-ecomplete (0.007451 sec)
>> Loading eudcb-ldap...
>> Parsing results...
>> Parsing results... done
>> Parsing results...
>> Parsing results... done
>> Test eudcb-ldap backtrace:
>>   signal(error ("No match"))
>>   apply(signal (error ("No match")))
>>   (setq value-640 (apply fn-638 args-639))
>>   (unwind-protect (setq value-640 (apply fn-638 args-639)) (setq form-
>>   (if (unwind-protect (setq value-640 (apply fn-638 args-639)) (setq f
>>   (let (form-description-642) (if (unwind-protect (setq value-640 (app
>>   (let ((value-640 'ert-form-evaluation-aborted-641)) (let (form-descr
>>   (let* ((fn-638 #'equal) (args-639 (condition-case err (let ((signal-
>>   (let ((ldap-process (start-process "slapd" "*slapd*" "/usr/sbin/slap
>>   (closure (t) nil (let ((value-636 (gensym "ert-form-evaluation-abort
>>   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>>   ert-run-test(#s(ert-test :name eudcb-ldap :documentation "Test the L
>>   ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
>>   ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
>>   ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
>>   ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
>>   eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
>>   command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/net/eudc-tests.el" "
>>   command-line()
>>   normal-top-level()
>> Test eudcb-ldap condition:
>>     (error "No match")
>>    FAILED  10/11  eudcb-ldap (1.024022 sec) at lisp/net/eudc-tests.el:271
>> Parsing /home/eric/dev/emacs/test/lisp/net/eudc-resources/mailrc...
>> Parsing /home/eric/dev/emacs/test/lisp/net/eudc-resources/mailrc... done
>>    passed  11/11  eudcb-mailabbrev (0.002174 sec)
>>
>> Ran 11 tests, 9 results as expected, 1 unexpected, 1 skipped (2022-11-21 16:04:41-0800, 1.151547 sec)
>>
>> 1 unexpected results:
>>    FAILED  eudcb-ldap
>>
>> 1 skipped results:
>>   SKIPPED  eudcb-bbdb
>
> Thanks for trying.
>
> 
[...]

>  (In a subsequent patch I'll replace the sleep with a retry loop to
> make this more reliable.)

I pushed this patch to the master branch.  When you get a chance, can
you retry:

make -C test lisp/net/eudc-tests.log

and see if it succeeds for you now?

Thanks,
Thomas

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

[Thomas Fitzsimmons]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

[...]

>> 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.
>
> I've already said this, but I don't think we should be pushing things in
> a direction that relies any external package or its particular behavior.
> The capf machinery should be enough, and that's very clearly defined.

I haven't figured out how the defaults are supposed to work for EUDC,
despite a bunch of attempts.  I'll keep at it, but wanted to post
status.

Here's a recipe to replicate what I'm trying:

cd test/lisp/net
/usr/sbin/slapd -h "ldap://127.0.0.1:3899" -d 0 -4 \
    -f eudc-resources/slapd.conf &

mkdir test-home
cat >test-home/.emacs <<EOF
(setq ldap-host-parameters-alist '(("ldap://localhost:3899"
                                    base "dc=gnu,dc=org" auth simple)))
(setq eudc-server-hotlist '(("ldap://localhost:3899" . ldap)))
(setq eudc-ignore-options-file t)
EOF
HOME=test-home ../../../src/emacs

C-x m emacs-ertTAB

There seem to have been many attempts in the code to get that to work by
default, but currently all the various approaches fail.  The result is
that three spaces are added to the buffer after "emacs-ert".

If instead of pressing TAB, one invokes M-x eudc-expand-try-all RET, the
EUDC multiple results UI is presented, and the completion one selects is
correctly inserted.  (This proves that message machinery is failing even
though EUDC and LDAP work.)

I've been hacking around the various EUDC code paths in message.el
(including the (setq message-expand-name-standard-ui t) one) trying to
get something to work by default but I haven't succeeded yet.

Thomas

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

[Eric Abrahamsen]

On 11/24/22 02:24 AM, Thomas Fitzsimmons wrote:
> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>>>
>>> [...]
>>>
>>>> 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
>>>
>>> No love!
>>>
>>> Running 11 tests (2022-11-21 16:04:40-0800, selector `(not (or (tag :unstable) (tag :nativecomp)))')
>>>    passed   1/11  eudc--plist-member (0.000396 sec)
>>>    passed   2/11  eudc-lax-plist-get (0.000433 sec)
>>>    passed   3/11  eudc-plist-get (0.000417 sec)
>>>    passed   4/11  eudc-plist-member (0.000390 sec)
>>>    passed   5/11  eudc-test-make-address (0.000151 sec)
>>>    passed   6/11  eudc-test-rfc5322-quote-phrase (0.000067 sec)
>>>    passed   7/11  eudc-test-rfc5322-valid-comment-p (0.000760 sec)
>>>   skipped   8/11  eudcb-bbdb (0.000102 sec)
>>>    passed   9/11  eudcb-ecomplete (0.007451 sec)
>>> Loading eudcb-ldap...
>>> Parsing results...
>>> Parsing results... done
>>> Parsing results...
>>> Parsing results... done
>>> Test eudcb-ldap backtrace:
>>>   signal(error ("No match"))
>>>   apply(signal (error ("No match")))
>>>   (setq value-640 (apply fn-638 args-639))
>>>   (unwind-protect (setq value-640 (apply fn-638 args-639)) (setq form-
>>>   (if (unwind-protect (setq value-640 (apply fn-638 args-639)) (setq f
>>>   (let (form-description-642) (if (unwind-protect (setq value-640 (app
>>>   (let ((value-640 'ert-form-evaluation-aborted-641)) (let (form-descr
>>>   (let* ((fn-638 #'equal) (args-639 (condition-case err (let ((signal-
>>>   (let ((ldap-process (start-process "slapd" "*slapd*" "/usr/sbin/slap
>>>   (closure (t) nil (let ((value-636 (gensym "ert-form-evaluation-abort
>>>   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>>>   ert-run-test(#s(ert-test :name eudcb-ldap :documentation "Test the L
>>>   ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
>>>   ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
>>>   ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
>>>   ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
>>>   eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
>>>   command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/net/eudc-tests.el" "
>>>   command-line()
>>>   normal-top-level()
>>> Test eudcb-ldap condition:
>>>     (error "No match")
>>>    FAILED  10/11  eudcb-ldap (1.024022 sec) at lisp/net/eudc-tests.el:271
>>> Parsing /home/eric/dev/emacs/test/lisp/net/eudc-resources/mailrc...
>>> Parsing /home/eric/dev/emacs/test/lisp/net/eudc-resources/mailrc... done
>>>    passed  11/11  eudcb-mailabbrev (0.002174 sec)
>>>
>>> Ran 11 tests, 9 results as expected, 1 unexpected, 1 skipped (2022-11-21 16:04:41-0800, 1.151547 sec)
>>>
>>> 1 unexpected results:
>>>    FAILED  eudcb-ldap
>>>
>>> 1 skipped results:
>>>   SKIPPED  eudcb-bbdb
>>
>> Thanks for trying.
>>
>> 
> [...]
>
>>  (In a subsequent patch I'll replace the sleep with a retry loop to
>> make this more reliable.)
>
> I pushed this patch to the master branch.  When you get a chance, can
> you retry:
>
> make -C test lisp/net/eudc-tests.log
>
> and see if it succeeds for you now?

slapd refuses to run without root permissions on my laptop for some
reason, so this will have to wait until next Monday when I'm back at my
desktop.

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

[Alexander Adolf]

Hello Eric,

Apologies for chiming late into this.

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> On 11/16/22 14:18 PM, Thomas Fitzsimmons wrote:
>> [...]
>>> 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.
>> [...]
>> (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.

That the behaviour changes depending on the CAPF front-end (corfu in
this case) could be indicative of the completion styles [1] kicking in.
I've seen cases where my completion function did return results, but
nothing was shown in the CAPF UI because the completion styles'
"filtering" wouldn't leave any alternatives to show. Not to suggest that
this is what's actually happening, but it's one possibility.

[1] https://www.gnu.org/software/emacs/manual/html_node/emacs/Completion-Styles.html

Other than that, I would be interested in reproducing this at my end -
if possible - to see whether there's any corner case I missed. I seems
that the change of eudc-server-hotlist from `nil' to `(("localhost" .
ecomplete) ("localhost" . mailabbrev))` triggered this behaviour for
you. With that value, no database files for ecomplete or mailabbrev, and
all Elisp settings for ecomplete and mailabbrev at their default values,
`eudc-capf-complete` does return nil for me. What else should I be doing
to reproduce the issue?


Many thanks and cheers,

  --alexander

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

[Alexander Adolf]

Hello Eric,

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> [...]
> 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.
> [...]
> So I need to clobber `message-expand-name' altogether.
>
> 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.
> [...]

Perhaps we can be slightly more CONstructive that this. ;-)))

I am preparing a patch to message.el which refactors
`message-completion-alist` along the lines of this:

---------------------------- Begin Quote -----------------------------
(defcustom message-completion-alist
  `((,message-newgroups-header-regexp . (:category newsgroup
                                         :fieldsep-re "\\([:,]\\|^\\)[ \t]*"
                                         :completions ,#'message-expand-group))
    (,message-email-recipient-header-regexp . (:category email
                                               :fieldsep-re "\\([:,]\\|^\\)[ \t]*"
                                               :completions ,#'eudc-capf-message-expand-name)))
  "Alist of (RE . RECIPE), defining completion contexts.
This variable controls how `message-completion-function' performs
in-buffer completion.  RECIPE is either a function (deprecated),
or a plist.

When `message-completion-function' is invoked, and point is on a
line matching one of the REs in the alist, the settings in the
corresponding RECIPE are applied.

When RECIPE is a function, it is called for completion.  RECIPE
should be a function that obeys the same rules as those of
`completion-at-point-functions'.

When RECIPE is a plist, the properties control how in-buffer
completion is performed.  The following properties are currently
defined:

:category

    The symbol defining the category in
    `completion-category-defaults' to use for completion.  Also
    see `completion-category-overrides', and `completion-styles'.

:fieldsep-re

    The regular expression to use when scanning backwards in the
    buffer.  All text between point, and any preceding text
    matching this regular expression, will be used as the prefix
    for finding completion candidates.

:completions

    The function that provides completions, and that obeys the
    same rules as those of `completion-at-point-functions'.
    In-buffer completion will be performed as if
    `completion-at-point-functions' had been set to this value."
  :version "29.1"
  :group 'message
  :type '(alist :key-type regexp
                :value-type (choice (plist)
                                    (function
                                     :tag "Completion function (deprecated)"))))
----------------------------- End Quote ------------------------------

As you can see, `eudc-capf-message-expand-name` effectively replaces
`message-expand-name` altogether:

---------------------------- Begin Quote -----------------------------
(make-obsolete 'message-expand-name 'eudc-capf-message-expand-name
               "29.1")
----------------------------- End Quote ------------------------------

The patch goes on to remove everything relating to ecomplete,
mailabbrev, bbdb, and the likes from message.el, too. Get rid of all the
burritos, except one. The one and only source for email addresses in
message.el IMHO should be EUDC, and the one and only completion UI
should be whatever CAPF uses. Any and all sources for email addresses
should implement back-ends for EUDC, so they can be queried via EUDC,
which does the aggregation of results from the different sources, and
delivers it back to message.el as one lump.


EUDC backends:   ldap   ecomplete   mailabbrev   bbdb    you-name-it
               \________________________ ____________________________/
                                        V
                                        |
                                        V
eudc-capf.el:            eudc-capf-message-expand-name
                                        |
                                        V
message.el:               message-completion-function
                                        |
                                        V
minibuffer.el                completion-at-point
                                        |
                                        V
                   [ optional completion UI (for example corfu) ]


As you may imagine, this is a bigger patch, and I am discussing it on
emacs-devel with Stefan Monnier. So it'll still be a little while until
it might hopefully get merged, and the burritos unwrapped.


Many thanks and looking forward to your thoughts,

  --alexander

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

[Eric Abrahamsen]

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Hello Eric,
>
> Apologies for chiming late into this.
>
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> On 11/16/22 14:18 PM, Thomas Fitzsimmons wrote:
>>> [...]
>>>> 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.
>>> [...]
>>> (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.
>
> That the behaviour changes depending on the CAPF front-end (corfu in
> this case) could be indicative of the completion styles [1] kicking in.
> I've seen cases where my completion function did return results, but
> nothing was shown in the CAPF UI because the completion styles'
> "filtering" wouldn't leave any alternatives to show. Not to suggest that
> this is what's actually happening, but it's one possibility.
>
> [1] https://www.gnu.org/software/emacs/manual/html_node/emacs/Completion-Styles.html
>
> Other than that, I would be interested in reproducing this at my end -
> if possible - to see whether there's any corner case I missed. I seems
> that the change of eudc-server-hotlist from `nil' to `(("localhost" .
> ecomplete) ("localhost" . mailabbrev))` triggered this behaviour for
> you. With that value, no database files for ecomplete or mailabbrev, and
> all Elisp settings for ecomplete and mailabbrev at their default values,
> `eudc-capf-complete` does return nil for me. What else should I be doing
> to reproduce the issue?

I think Thomas already fixed it! Shortly after I reported.

Thanks,
Eric

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

[Eric Abrahamsen]

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


On 12/01/22 16:48 PM, Alexander Adolf wrote:
> Hello Eric,
>
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> [...]
>> 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.
>> [...]
>> So I need to clobber `message-expand-name' altogether.
>>
>> 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.
>> [...]
>
> Perhaps we can be slightly more CONstructive that this. ;-)))
>
> I am preparing a patch to message.el which refactors
> `message-completion-alist` along the lines of this:
>
> ---------------------------- Begin Quote -----------------------------
> (defcustom message-completion-alist
>   `((,message-newgroups-header-regexp . (:category newsgroup
>                                          :fieldsep-re "\\([:,]\\|^\\)[ \t]*"
>                                          :completions ,#'message-expand-group))
>     (,message-email-recipient-header-regexp . (:category email
>                                                :fieldsep-re "\\([:,]\\|^\\)[ \t]*"
>                                                :completions ,#'eudc-capf-message-expand-name)))
>   "Alist of (RE . RECIPE), defining completion contexts.
> This variable controls how `message-completion-function' performs
> in-buffer completion.  RECIPE is either a function (deprecated),
> or a plist.
>
> When `message-completion-function' is invoked, and point is on a
> line matching one of the REs in the alist, the settings in the
> corresponding RECIPE are applied.
>
> When RECIPE is a function, it is called for completion.  RECIPE
> should be a function that obeys the same rules as those of
> `completion-at-point-functions'.
>
> When RECIPE is a plist, the properties control how in-buffer
> completion is performed.  The following properties are currently
> defined:
>
> :category
>
>     The symbol defining the category in
>     `completion-category-defaults' to use for completion.  Also
>     see `completion-category-overrides', and `completion-styles'.
>
> :fieldsep-re
>
>     The regular expression to use when scanning backwards in the
>     buffer.  All text between point, and any preceding text
>     matching this regular expression, will be used as the prefix
>     for finding completion candidates.
>
> :completions
>
>     The function that provides completions, and that obeys the
>     same rules as those of `completion-at-point-functions'.
>     In-buffer completion will be performed as if
>     `completion-at-point-functions' had been set to this value."
>   :version "29.1"
>   :group 'message
>   :type '(alist :key-type regexp
>                 :value-type (choice (plist)
>                                     (function
>                                      :tag "Completion function (deprecated)"))))
> ----------------------------- End Quote ------------------------------
>
> As you can see, `eudc-capf-message-expand-name` effectively replaces
> `message-expand-name` altogether:
>
> ---------------------------- Begin Quote -----------------------------
> (make-obsolete 'message-expand-name 'eudc-capf-message-expand-name
>                "29.1")
> ----------------------------- End Quote ------------------------------
>
> The patch goes on to remove everything relating to ecomplete,
> mailabbrev, bbdb, and the likes from message.el, too. Get rid of all the
> burritos, except one. The one and only source for email addresses in
> message.el IMHO should be EUDC, and the one and only completion UI
> should be whatever CAPF uses. Any and all sources for email addresses
> should implement back-ends for EUDC, so they can be queried via EUDC,
> which does the aggregation of results from the different sources, and
> delivers it back to message.el as one lump.
>
>
> EUDC backends:   ldap   ecomplete   mailabbrev   bbdb    you-name-it
>                \________________________ ____________________________/
>                                         V
>                                         |
>                                         V
> eudc-capf.el:            eudc-capf-message-expand-name
>                                         |
>                                         V
> message.el:               message-completion-function
>                                         |
>                                         V
> minibuffer.el                completion-at-point
>                                         |
>                                         V
>                    [ optional completion UI (for example corfu) ]
>
>
> As you may imagine, this is a bigger patch, and I am discussing it on
> emacs-devel with Stefan Monnier. So it'll still be a little while until
> it might hopefully get merged, and the burritos unwrapped.
>
>
> Many thanks and looking forward to your thoughts,

My only thought is that we already have a mechanism for combining
results from multiple contact-management packages, and it's called
`message--name-table'. I don't see why we should be obliged to add EUDC
as an additional and obligatory point of collation. I'm attaching a
patch I floated earlier, showing how I think it could works -- it's very
simple.

Stefan was the one who added `message--name-table', and if you're
talking to him I will obviously defer to whatever you (plural) decide.
But that's my two cents.

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

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

[Thomas Fitzsimmons]

Hi Alexander,

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Hello Eric,
>
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> [...]
>> 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.
>> [...]
>> So I need to clobber `message-expand-name' altogether.
>>
>> 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.
>> [...]
>
> Perhaps we can be slightly more CONstructive that this. ;-)))
>
> I am preparing a patch to message.el which refactors
> `message-completion-alist` along the lines of this:

[...]

The eventual message.el refactoring could work.  But for now, for Emacs
29.1, let's focus on what's already in message.el, which is a lot!

In particular, I'm trying to make this recipe do something useful, with
a minimal patch to message.el.

1. Build Emacs master tip.

2. Install and configure BBDB:

mkdir -p test/.emacs.d
cat >test/.emacs.d/bbdb <<EOF
;; -*- mode: Emacs-Lisp; coding: utf-8; -*-
;;; file-format: 9
["Emacs" "ERT3" nil nil nil nil nil ("emacs-ert-test-3@bbdb.gnu.org") ((notes . " ")) "c8bd3a63-3a83-48a7-a95b-be118a923e00" "2022-11-19 16:36:04 +0000" "2022-11-19 16:36:04 +0000" nil]
["Emacs" "ERT4" nil nil nil nil nil ("emacs-ert-test-4@bbdb.gnu.org") ((notes . " ")) "5a93c3c5-9270-4e10-8b28-d28cfa2562cf" "2022-11-19 16:47:49 +0000" "2022-11-19 16:47:49 +0000" nil]
EOF
HOME=`pwd`/test emacs
M-x list-packages
(install BBDB from GNU ELPA)
exit Emacs

HOME=`pwd`/test emacs
C-: (eudc-set-server "localhost" 'bbdb t)
C-x m
emacs-ertTAB

When I try this, a bunch of stuff happens, then, uselessly, 3 spaces are
inserted after "emacs-ert".  During "bunch of stuff",
eudc-query-with-words is called and gets expansion results.  Why are
they not presented as possible completions?  What code is dropping them?

There is so much EUDC handling code in message.el and apparently none of
it works.  And most of this code predates the addition of
eudc-capf-complete, but its addition didn't fix this basic usage.
That's why the recommendation in the EUDC manual has always been to just
clobber "TAB" in message mode and assign eudc-expand-inline (now
eudc-expand-try-all) to it.

Before rewriting completion in message.el, before Emacs 29.1, can we
please make the logic that was there already actually work?  Please try
the above recipe; maybe I'm missing something.  If you can make it work,
let me know, or send a minimal patch to make it work.

Thanks,
Thomas

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

[Alexander Adolf]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
> The eventual message.el refactoring could work.  But for now, for Emacs
> 29.1, let's focus on what's already in message.el, which is a lot!

Fully agree.

> In particular, I'm trying to make this recipe do something useful, with
> a minimal patch to message.el.
> [...]

Looking at this now.

While I set this up: What's your value of the variable
`completion-styles` during this test? Does changing `completion-styles`
to '(substring partial-completion) for the test change the result and
outcome?


Cheers,

  --alexander

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

[Thomas Fitzsimmons]

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>> [...]
>> The eventual message.el refactoring could work.  But for now, for Emacs
>> 29.1, let's focus on what's already in message.el, which is a lot!
>
> Fully agree.
>
>> In particular, I'm trying to make this recipe do something useful, with
>> a minimal patch to message.el.
>> [...]
>
> Looking at this now.

Great, thank you.

> While I set this up: What's your value of the variable
> `completion-styles` during this test?

I didn't change it, so it should be the default, which looks to be:
(basic partial-completion emacs22)

> Does changing `completion-styles` to '(substring partial-completion)
> for the test change the result and outcome?

I'll try this later today.

Thanks,
Thomas

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

[Alexander Adolf]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

>> [...]
>> Looking at this now.
>
> Great, thank you.
> [...]

With the default value of `completion-styles`, which is

     '(basic partial-completion emacs22)

I get no completion. But when setting it to

     '(substring partial-completion)

as I mentioned, I get this after calling `completion-at-point:`
----------------------------------------------------------------------
To:  <emacs-ert-test-
Subject: 
From: Alexander Adolf <alexanderadolf@laptop.fritz.box>
--text follows this line--
----------------------------------------------------------------------
Which I guess is kind of the expected result, as it inserts the longest
common prefix of the two candidates' email addresses.

I get the same result with `message-tab` (or just pressing TAB), btw.

Actually, I think the leading "<" should not be there? After appending
either "3", or "4" further completion attempts (using either function)
do not provide any results. But when I remove the leading "<", and try
to complete again (with either "3" or "4" added), I get the correct
result (for instance "Emacs ERT4 <emacs-ert-test-4@bbdb.gnu.org>").

Thus, it seems there are two things to further look into :

- set `completion-styles` to '(substring partial-completion) during the
  test

- make sure the leading "<" is not inserted when there is a partial
  match


Looking forward to your thoughts,

  --alexander

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

[Thomas Fitzsimmons]

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>>> [...]
>>> Looking at this now.
>>
>> Great, thank you.
>> [...]
>
> With the default value of `completion-styles`, which is
>
>      '(basic partial-completion emacs22)
>
> I get no completion. But when setting it to
>
>      '(substring partial-completion)
>
> as I mentioned, I get this after calling `completion-at-point:`
> ----------------------------------------------------------------------
> To:  <emacs-ert-test-
> Subject: 
> From: Alexander Adolf <alexanderadolf@laptop.fritz.box>
> --text follows this line--
> ----------------------------------------------------------------------
> Which I guess is kind of the expected result, as it inserts the longest
> common prefix of the two candidates' email addresses.
>
> I get the same result with `message-tab` (or just pressing TAB), btw.
>
> Actually, I think the leading "<" should not be there? After appending
> either "3", or "4" further completion attempts (using either function)
> do not provide any results. But when I remove the leading "<", and try
> to complete again (with either "3" or "4" added), I get the correct
> result (for instance "Emacs ERT4 <emacs-ert-test-4@bbdb.gnu.org>").
>
> Thus, it seems there are two things to further look into :
>
> - set `completion-styles` to '(substring partial-completion) during the
>   test
>
> - make sure the leading "<" is not inserted when there is a partial
>   match

Confirmed, adding substring to completion-styles, anywhere in the list, e.g.:

(setq completion-styles '(basic substring partial-completion emacs22))

results in the behaviour you describe.  We need to make this work with
the default completion-styles setting though.

I wonder where the leading " <" comes from (a space character is also
inserted, before the '<').

Thomas

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

[Thomas Fitzsimmons]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> Alexander Adolf <alexander.adolf@condition-alpha.com> writes:
>
>> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>>
>>>> [...]
>>>> Looking at this now.
>>>
>>> Great, thank you.
>>> [...]
>>
>> With the default value of `completion-styles`, which is
>>
>>      '(basic partial-completion emacs22)
>>
>> I get no completion. But when setting it to
>>
>>      '(substring partial-completion)
>>
>> as I mentioned, I get this after calling `completion-at-point:`
>> ----------------------------------------------------------------------
>> To:  <emacs-ert-test-
>> Subject: 
>> From: Alexander Adolf <alexanderadolf@laptop.fritz.box>
>> --text follows this line--
>> ----------------------------------------------------------------------
>> Which I guess is kind of the expected result, as it inserts the longest
>> common prefix of the two candidates' email addresses.
>>
>> I get the same result with `message-tab` (or just pressing TAB), btw.
>>
>> Actually, I think the leading "<" should not be there? After appending
>> either "3", or "4" further completion attempts (using either function)
>> do not provide any results. But when I remove the leading "<", and try
>> to complete again (with either "3" or "4" added), I get the correct
>> result (for instance "Emacs ERT4 <emacs-ert-test-4@bbdb.gnu.org>").
>>
>> Thus, it seems there are two things to further look into :
>>
>> - set `completion-styles` to '(substring partial-completion) during the
>>   test
>>
>> - make sure the leading "<" is not inserted when there is a partial
>>   match
>
> Confirmed, adding substring to completion-styles, anywhere in the list, e.g.:
>
> (setq completion-styles '(basic substring partial-completion emacs22))
>
> results in the behaviour you describe.  We need to make this work with
> the default completion-styles setting though.
>
> I wonder where the leading " <" comes from (a space character is also
> inserted, before the '<').

If I remove this line from message.el:

(add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)

and rebuild Emacs, then my recipe works, via:

message-tab -...-> message-expand-name -> eudc-expand-inline

The EUDC multi-selector UI is invoked allowing me to choose between
emacs-ert-test-3 and emacs-ert-test-4, and the full name and email
address is inserted correctly.  This is with completion-styles left at
its default value.

So that's one code path validated, phew.

The next one I'd like to get working is a similar recipe (again, with
eudc-capf-complete not present in message's
completion-at-point-functions):

HOME=`pwd`/test emacs
C-: (setq message-expand-name-standard-ui t)
C-: (eudc-set-server "localhost" 'bbdb t)
C-x m
emacs-ertTAB

Then I get " <emacs-ert-test-", the same as your result, in the absence
of eudc-capf-complete.  This seems to be caused by misinterpreting the
results of eudc-query-with-words.  What is 'message--name-table'
supposed to return, what is it returning, and why is its return value
producing " <..." in the output?  I could use your input on how
completion-at-point is supposed to work here.

Thank you,
Thomas

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

[Alexander Adolf]

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

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
> If I remove this line from message.el:
>
> (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
>
> and rebuild Emacs, then my recipe works, via:
>
> message-tab -...-> message-expand-name -> eudc-expand-inline
>
> The EUDC multi-selector UI is invoked allowing me to choose between
> emacs-ert-test-3 and emacs-ert-test-4, and the full name and email
> address is inserted correctly.  This is with completion-styles left at
> its default value.
>
> So that's one code path validated, phew.
> [...]

There are two ends to this, I think.

For one, you are no longer adding `eudc-capf-complete` to
`completion-at-point-functions` in message mode. This seems fine given
that we are not there yet to make `eudc-capf-complete` the default thing
to happen in 29.

OTOH, `eudc-capf-complete` is now just sitting there for people to use
it. The docstring, and the NEWS entry advertise it for adding to
`completion-at-point-functions`. If & when someone does that, it won't
work as expected, as you & I just figured, because `completion-styles`
has a default value aimed at single-word replacements (such as for
programming language keywords, or prose words from a dictionary), but
not for more complex, multi-word replacements (such as email addresses).
The `substring` entry is needed for this use-case.

To make this work, I am hence attaching a patch (relative to the tip of
master as of this writing), which sets `completion-style` from
`eudc-capf-complete`. The modification is done when the user has not
modified its value (assuming that a sensible value will have been set by
the user). If you need a patch on top of the 29 branch, please don't
hesitate to drop me a note.


Hoping to have helped, and looking forward to your thoughts,

  --alexander


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Set-completion-style-for-email-addresses-when-EUDC-c.patch --]
[-- Type: text/x-patch, Size: 1639 bytes --]

From a7c5eed95321c77d4e5069a508b4b16094f4a68d Mon Sep 17 00:00:00 2001
From: Alexander Adolf <alexander.adolf@condition-alpha.com>
Date: Wed, 7 Dec 2022 23:07:11 +0100
Subject: [PATCH] Set completion-style for email addresses when EUDC
 contributes to CAPF

* lisp/net/eudc-capf.el (eudc-capf-complete): when the user has not
modified 'completion-styles', set it to a value better suited for
email address completion than the default setting (bug#59314)
---
 lisp/net/eudc-capf.el | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lisp/net/eudc-capf.el b/lisp/net/eudc-capf.el
index e2bbd5b28b..5bd2c92441 100644
--- a/lisp/net/eudc-capf.el
+++ b/lisp/net/eudc-capf.el
@@ -104,7 +104,16 @@ eudc-capf-complete
   (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)))
+      (progn
+        ;; FIXME: `completion-styles' is set to a value which seems
+        ;; better suited for email address completion if and when it
+        ;; has not been modified from it's default value (which is not
+        ;; well suited for this purpose).  This is needed until
+        ;; `message.el' will be updated to use completion categories.
+        (when (equal completion-styles
+                     (eval (car (get 'completion-styles 'standard-value))))
+          (setq-local completion-styles '(substring partial-completion)))
+        (eudc-capf-message-expand-name))))
 
 ;;;###autoload
 (defun eudc-capf-message-expand-name ()
-- 
2.38.1

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

[Alexander Adolf]

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

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
> The next one I'd like to get working is a similar recipe (again, with
> eudc-capf-complete not present in message's
> completion-at-point-functions):
>
> HOME=`pwd`/test emacs
> C-: (setq message-expand-name-standard-ui t)
> C-: (eudc-set-server "localhost" 'bbdb t)
> C-x m
> emacs-ertTAB
>
> Then I get " <emacs-ert-test-", the same as your result, in the absence
> of eudc-capf-complete.  This seems to be caused by misinterpreting the
> results of eudc-query-with-words.  What is 'message--name-table'
> supposed to return, what is it returning, and why is its return value
> producing " <..." in the output?  I could use your input on how
> completion-at-point is supposed to work here.

I'll look into this tomorrow, and will let you know my findings.

FWIW, I'm attaching a diagram of what I _think_ `completion-at-point`
works like in general. Perhaps it will help understand how message-mode
does completion?

As you see in the diagram (and we learnt), `completion-styles` can cause
some head-scratching, since it is used to filter any candidates lists
returned by completion functions. See also [1].

[1] https://www.gnu.org/software/emacs/manual/html_node/emacs/Completion-Styles.html


Cheers,

  --alexander


[-- Attachment #2: Emacs CAPF.png --]
[-- Type: image/png, Size: 310464 bytes --]

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

[Thomas Fitzsimmons]

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>> [...]
>> If I remove this line from message.el:
>>
>> (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
>>
>> and rebuild Emacs, then my recipe works, via:
>>
>> message-tab -...-> message-expand-name -> eudc-expand-inline
>>
>> The EUDC multi-selector UI is invoked allowing me to choose between
>> emacs-ert-test-3 and emacs-ert-test-4, and the full name and email
>> address is inserted correctly.  This is with completion-styles left at
>> its default value.
>>
>> So that's one code path validated, phew.
>> [...]
>
> There are two ends to this, I think.
>
> For one, you are no longer adding `eudc-capf-complete` to
> `completion-at-point-functions` in message mode. This seems fine given
> that we are not there yet to make `eudc-capf-complete` the default thing
> to happen in 29.

To be clear, I wasn't going to push a patch to remove that, at least not
yet.  It's not making anything worse, so might as well leave it as-is.

> [...]

I'm trying to get message--name-table's EUDC support working.  I only
wanted to remove eudc-capf-complete from the debugging equation, because
message--name-table existed way before eudc-capf-complete was
introduced.

I want to understand why it doesn't work (why it results in the " <..."
expansion).  I haven't delved deep enough into the completion-at-point
to figure that out.  I was hoping with your experience writing
eudc-capf-complete that you'd know what was wrong with
message--name-table's EUDC support.

I don't think completion-styles should have any bearing on this.
message--name-table's EUDC support has to be made to work with the
default completion-styles setting.  Do you know how to do that?

Thanks,
Thomas

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

[Alexander Adolf]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
>> For one, you are no longer adding `eudc-capf-complete` to
>> `completion-at-point-functions` in message mode. This seems fine given
>> that we are not there yet to make `eudc-capf-complete` the default thing
>> to happen in 29.
>
> To be clear, I wasn't going to push a patch to remove that, at least not
> yet.  It's not making anything worse, so might as well leave it as-is.

I see. Please kindly ignore yesterday's path then, please.

>> [...]
> I'm trying to get message--name-table's EUDC support working.  I only
> wanted to remove eudc-capf-complete from the debugging equation, because
> message--name-table existed way before eudc-capf-complete was
> introduced.
>
> I want to understand why it doesn't work (why it results in the " <..."
> expansion).  I haven't delved deep enough into the completion-at-point
> to figure that out.  I was hoping with your experience writing
> eudc-capf-complete that you'd know what was wrong with
> message--name-table's EUDC support.

I'll look into `message--name-table` and will find out what breaks with
the " <" prefix. Please allow until Friday USA time (assuming you're
there?); it's late already over here (Europe).

> I don't think completion-styles should have any bearing on this.
> message--name-table's EUDC support has to be made to work with the
> default completion-styles setting.  Do you know how to do that?

The thing is, they do have a bearing, and there is no way to avoid that.
`completion-at-point` filters the candidates returned by the
`completion-at-point-functions`, and it uses the completion style (set
by either `completion-styles`, or via a completion category signalled in
a completion table) for this.

When the completion table does not signal a completion style,
`completion-at-point` uses the value of `completion-styles` to filter
the candidates. Only matching candidates will be presented in any UI.
The default value of `completion-styles` is '(basic partial-completion
emacs22). Which - according to the manual [1] - effects the following:

---------------------------- Begin Quote -----------------------------
basic

   A matching completion alternative must have the same beginning as the
   text in the minibuffer before point. Furthermore, if there is any
   text in the minibuffer after point, the rest of the completion
   alternative must contain that text as a substring.

partial-completion

   This aggressive completion style divides the minibuffer text into
   words separated by hyphens or spaces, and completes each word
   separately. (For example, when completing command names, ‘em-l-m’
   completes to ‘emacs-lisp-mode’.)

   Furthermore, a ‘*’ in the minibuffer text is treated as a wildcard—it
   matches any string of characters at the corresponding position in the
   completion alternative.

emacs22

   This completion style is similar to basic, except that it ignores the
   text in the minibuffer after point. It is so-named because it
   corresponds to the completion behavior in Emacs 22.
----------------------------- End Quote ------------------------------
[1] https://www.gnu.org/software/emacs/manual/html_node/emacs/Completion-Styles.html

I.e. the default setting of `completion-styles` will match for
candidates that have the search string at their beginning only. Example:
when the text before point is "foo", the candidates "foo", "foox", and
"foobar" will be shown, but not the candidate "barfoo".

Clearly, this is fairly useless for email address completion. Thus, the
function `message--name-table` in message.el begins like this:

---------------------------- Begin Quote -----------------------------
(defun message--name-table (orig-string)
  (let ((orig-words (split-string orig-string "[ \t]+"))
        eudc-responses
        bbdb-responses)
    (lambda (string pred action)
      (pcase action
        ('metadata '(metadata (category . email)))
[...]
----------------------------- End Quote ------------------------------

In the last quoted line, it return the list '(metadata (category .
email)) in response to the 'metadata action.

When message.el is loaded, the init code there does this:

---------------------------- Begin Quote -----------------------------
(add-to-list 'completion-category-defaults '(email (styles substring
                                                           partial-completion)))
----------------------------- End Quote ------------------------------

This defines the 'email completion category to imply the completion
styles '(substring partial-completion). Thus, whenever
`message--name-table` comes into play, these completion styles will be
in effect.

Long story, short conclusion: you can't do meaningful email address
completion with the default value of `completion styles`.


Cheers,

  --alexander

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

[Thomas Fitzsimmons]

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>> [...]
>>> For one, you are no longer adding `eudc-capf-complete` to
>>> `completion-at-point-functions` in message mode. This seems fine given
>>> that we are not there yet to make `eudc-capf-complete` the default thing
>>> to happen in 29.
>>
>> To be clear, I wasn't going to push a patch to remove that, at least not
>> yet.  It's not making anything worse, so might as well leave it as-is.
>
> I see. Please kindly ignore yesterday's path then, please.
>
>>> [...]
>> I'm trying to get message--name-table's EUDC support working.  I only
>> wanted to remove eudc-capf-complete from the debugging equation, because
>> message--name-table existed way before eudc-capf-complete was
>> introduced.
>>
>> I want to understand why it doesn't work (why it results in the " <..."
>> expansion).  I haven't delved deep enough into the completion-at-point
>> to figure that out.  I was hoping with your experience writing
>> eudc-capf-complete that you'd know what was wrong with
>> message--name-table's EUDC support.
>
> I'll look into `message--name-table` and will find out what breaks with
> the " <" prefix. Please allow until Friday USA time (assuming you're
> there?); it's late already over here (Europe).

Sure, thank you, please take your time.  I haven't digested the diagram
you posted yet, but I'll have a look when I get a chance.

>> I don't think completion-styles should have any bearing on this.
>> message--name-table's EUDC support has to be made to work with the
>> default completion-styles setting.  Do you know how to do that?
>
> The thing is, they do have a bearing, and there is no way to avoid that.
> `completion-at-point` filters the candidates returned by the
> `completion-at-point-functions`, and it uses the completion style (set
> by either `completion-styles`, or via a completion category signalled in
> a completion table) for this.
>
> When the completion table does not signal a completion style,
> `completion-at-point` uses the value of `completion-styles` to filter
> the candidates. Only matching candidates will be presented in any UI.
> The default value of `completion-styles` is '(basic partial-completion
> emacs22). Which - according to the manual [1] - effects the following:
>
> ---------------------------- Begin Quote -----------------------------
> basic
>
>    A matching completion alternative must have the same beginning as the
>    text in the minibuffer before point. Furthermore, if there is any
>    text in the minibuffer after point, the rest of the completion
>    alternative must contain that text as a substring.
>
> partial-completion
>
>    This aggressive completion style divides the minibuffer text into
>    words separated by hyphens or spaces, and completes each word
>    separately. (For example, when completing command names, ‘em-l-m’
>    completes to ‘emacs-lisp-mode’.)
>
>    Furthermore, a ‘*’ in the minibuffer text is treated as a wildcard—it
>    matches any string of characters at the corresponding position in the
>    completion alternative.
>
> emacs22
>
>    This completion style is similar to basic, except that it ignores the
>    text in the minibuffer after point. It is so-named because it
>    corresponds to the completion behavior in Emacs 22.
> ----------------------------- End Quote ------------------------------
> [1] https://www.gnu.org/software/emacs/manual/html_node/emacs/Completion-Styles.html
>
> I.e. the default setting of `completion-styles` will match for
> candidates that have the search string at their beginning only. Example:
> when the text before point is "foo", the candidates "foo", "foox", and
> "foobar" will be shown, but not the candidate "barfoo".
>
> Clearly, this is fairly useless for email address completion. Thus, the
> function `message--name-table` in message.el begins like this:
>
> ---------------------------- Begin Quote -----------------------------
> (defun message--name-table (orig-string)
>   (let ((orig-words (split-string orig-string "[ \t]+"))
>         eudc-responses
>         bbdb-responses)
>     (lambda (string pred action)
>       (pcase action
>         ('metadata '(metadata (category . email)))
> [...]
> ----------------------------- End Quote ------------------------------
>
> In the last quoted line, it return the list '(metadata (category .
> email)) in response to the 'metadata action.
>
> When message.el is loaded, the init code there does this:
>
> ---------------------------- Begin Quote -----------------------------
> (add-to-list 'completion-category-defaults '(email (styles substring
>                                                            partial-completion)))
> ----------------------------- End Quote ------------------------------
>
> This defines the 'email completion category to imply the completion
> styles '(substring partial-completion). Thus, whenever
> `message--name-table` comes into play, these completion styles will be
> in effect.

I see, hmm.  That's quite complicated.  Nice analysis, thank you.

> Long story, short conclusion: you can't do meaningful email address
> completion with the default value of `completion styles`.

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?

Thanks,
Thomas

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

[Alexander Adolf]

[-- 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

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

[Thomas Fitzsimmons]

Hi Alexander,

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> another lengthy explanation, for which apologies up front!

OK, trimmed it down.

> 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.

[...]

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?  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?  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.

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.  I want to understand and fix that case.

(I'm rushing responses here, I'll dedicate more time to real testing
maybe tomorrow, but I'm hoping you'll beat me to it with a minimal patch
:-))

Thanks,
Thomas

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

[Alexander Adolf]

I have done some further debugging, and it seems that the behaviour is
independent of EUDC.

With this:

(setq message-expand-name-standard-ui t)
(setq eudc-server nil eudc-server-hotlist nil)
(setq message-expand-name-databases '(bbdb))

and `eudc-capf-complete` removed from `completion-at-point-functions` in
`message-mode`, the result is the same (completion is broke).

It seems that the completion styles are attempting to find the longest
common prefix of all candidates, and somehow take words into account.
Quoting from the Elisp manual [1]:

---------------------------- Begin Quote -----------------------------
The procedure of determining what constitutes a match is quite
intricate. Emacs attempts to offer plausible completions under most
circumstances.
----------------------------- End Quote ------------------------------

Have I just been handed a word of warning? Looks like.

Anyways, I will now try to see whether I can identify a commit that the
issue. I'll start with the one mentioned by Eric.

[puts on helmet, switches on headlamp, and grabs a pickax]

Wish me luck,

  --alexander

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

[Thomas Fitzsimmons]

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> I have done some further debugging, and it seems that the behaviour is
> independent of EUDC.
>
> With this:
>
> (setq message-expand-name-standard-ui t)
> (setq eudc-server nil eudc-server-hotlist nil)
> (setq message-expand-name-databases '(bbdb))
>
> and `eudc-capf-complete` removed from `completion-at-point-functions` in
> `message-mode`, the result is the same (completion is broke).

I confirmed this on my set up, this was a good thing to test, thanks.
(Before, I had tested BBDB-only (no-EUDC) completion with
message-expand-name-standard-ui set to nil, which worked, but not with
it set to t.)

> It seems that the completion styles are attempting to find the longest
> common prefix of all candidates, and somehow take words into account.
> Quoting from the Elisp manual [1]:
>
> ---------------------------- Begin Quote -----------------------------
> The procedure of determining what constitutes a match is quite
> intricate. Emacs attempts to offer plausible completions under most
> circumstances.
> ----------------------------- End Quote ------------------------------
>
> Have I just been handed a word of warning? Looks like.
>
> Anyways, I will now try to see whether I can identify a commit that the
> issue. I'll start with the one mentioned by Eric.
>
> [puts on helmet, switches on headlamp, and grabs a pickax]
>
> Wish me luck,

Good luck, this is good progress.

Thomas

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

[Alexander Adolf]

[-- 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

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

[Thomas Fitzsimmons]

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> 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,

I've applied your patch locally and I'm testing with it, thanks.  I'm
also setting (setq message-expand-name-standard-ui t).

This may be a clue; while I was experimenting, I found that this:

@@ -8432,7 +8438,7 @@ message--name-table
                           (all-completions string
                                            (ecomplete-completion-table 'mail)
                                            pred)))))
-	   (if action candidates (try-completion string candidates))))))))
+	   (if action (cdr candidates) (try-completion string candidates))))))))

results in the expansion working (albeit always expanding to Emacs
ERT4).  So it seems the issue is with a list of more than one candidate,
probably on the return path through the completion functions in
minibuffer.el.  I haven't been able to follow the return path all the
way yet.

Other complications:

1. If I do completion via bbdb-complete-mail first, before I've done
   (setq message-expand-name-standard-ui t), message-expand-name gets
   added to message--old-style-completion-functions dynamically,
   resulting in completion doing absolutely nothing.  If I set
   message-expand-name-standard-ui back to nil, then stock BBDB
   completion works again.  So there is a stateful interaction between
   stock BBDB completion and message-expand-name-standard-ui, where you
   have to set message--old-style-completion-functions back to nil if
   you want to change between the two.

2. When I switch to the *unsent mail* buffer, and press TAB, message-tab
   runs.  But if I then press TAB again, completion-at-point runs.  I
   have to switch out of the buffer then back in for message-tab to be
   run again.  It seems to enter a mode, with a new TAB keybinding,
   where it's trying to use completion-at-point within a region.

Thomas

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

[Alexander Adolf]

Hello Thomas,

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
> This may be a clue; while I was experimenting, I found that this:
>
> @@ -8432,7 +8438,7 @@ message--name-table
>                            (all-completions string
>                                             (ecomplete-completion-table 'mail)
>                                             pred)))))
> -	   (if action candidates (try-completion string candidates))))))))
> +	   (if action (cdr candidates) (try-completion string candidates))))))))
> [...]

Interesting; thanks for sharing this. Effectively, you're dropping the
first entry from the `candidates` list before returning it. What does
your `candidates` list contain at that point (before removal)?


I have some observations to share, too.

When the lambda function returned by `message--name-table` is called
with 't as the action argument, that is when it is expected to return
completion candidates, I have modified the code to return a static list
so I can experiment. It turns out that the breakage is triggered by the
_last_ candidate in that list. If & when the last element contains
white-space in what would be the "common prefix", then things go south.
All other candidates in that list may contain generous amounts of
white-space anywhere, and things will still work as expected (selection
UI is presented upon the second TAB key press).

The second observation is related to completion styles. With our
test-case, 'partial-completion does not produce useful results for my
standards. Perhaps a "collateral conclusion" will be that 'substring is
all that is needed when completing email recipients? Let's see.

I'll now dig further into minibuffer.el and what happens in the
'substring completion style code with the last element of the candidates
list.


Cheers,

  --alexander

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

[Thomas Fitzsimmons]

Hi Alexander,

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>> [...]
>> This may be a clue; while I was experimenting, I found that this:
>>
>> @@ -8432,7 +8438,7 @@ message--name-table
>>                            (all-completions string
>>                                             (ecomplete-completion-table 'mail)
>>                                             pred)))))
>> -	   (if action candidates (try-completion string candidates))))))))
>> +	   (if action (cdr candidates) (try-completion string candidates))))))))
>> [...]
>
> Interesting; thanks for sharing this. Effectively, you're dropping the
> first entry from the `candidates` list before returning it. What does
> your `candidates` list contain at that point (before removal)?

("Emacs ERT3 <emacs-ert-test-3@bbdb.gnu.org>"
 "Emacs ERT4 <emacs-ert-test-4@bbdb.gnu.org>")

> I have some observations to share, too.
>
> When the lambda function returned by `message--name-table` is called
> with 't as the action argument, that is when it is expected to return
> completion candidates, I have modified the code to return a static list
> so I can experiment. It turns out that the breakage is triggered by the
> _last_ candidate in that list. If & when the last element contains
> white-space in what would be the "common prefix", then things go south.
> All other candidates in that list may contain generous amounts of
> white-space anywhere, and things will still work as expected (selection
> UI is presented upon the second TAB key press).

OK, interesting; maybe we've found a bug in the completion engine
itself.

> The second observation is related to completion styles. With our
> test-case, 'partial-completion does not produce useful results for my
> standards. Perhaps a "collateral conclusion" will be that 'substring is
> all that is needed when completing email recipients? Let's see.

Maybe, but it seems like the "completion-category-defaults" logic that
you pointed out earlier is working, such that when completion is
attempted, 'substring is in completion-styles.  Here is the debugging
patch I have to print completion styles in-context:

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5faa3c8d4e8..455135628c8 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1191,7 +1191,7 @@ completion--nth-completion
                                (error "Invalid completion style %s" style))
                            string table pred point)))
                (and probe (cons probe style))))
-           (completion--styles md)))
+           (let ((styles (completion--styles md))) (message "STYLES: %S" styles) styles)))

When I press TAB, it prints:

STYLES: (substring partial-completion basic emacs22)

even though globally, completion-styles is set to its default,
(basic partial-completion emacs22)

> I'll now dig further into minibuffer.el and what happens in the
> 'substring completion style code with the last element of the candidates
> list.

OK, it sounds like you're close to finding the root cause.

Thanks,
Thomas

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

[Alexander Adolf]

Hello Thomas,

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
>>> This may be a clue; while I was experimenting, I found that this:
>>>
>>> @@ -8432,7 +8438,7 @@ message--name-table
>>>                            (all-completions string
>>>                                             (ecomplete-completion-table 'mail)
>>>                                             pred)))))
>>> -	   (if action candidates (try-completion string candidates))))))))
>>> +	   (if action (cdr candidates) (try-completion string candidates))))))))
>>> [...]
>>
>> Interesting; thanks for sharing this. Effectively, you're dropping the
>> first entry from the `candidates` list before returning it. What does
>> your `candidates` list contain at that point (before removal)?
>
> ("Emacs ERT3 <emacs-ert-test-3@bbdb.gnu.org>"
>  "Emacs ERT4 <emacs-ert-test-4@bbdb.gnu.org>")

Looking at the code in `completion-pcm--merge-try` (which is part of the
partial-completion style, and gets called from the basic, substring, and
flex completion styles), reducing it to a single candidate may make a
difference; the function begins like this:

---------------------------- Begin Quote -----------------------------
(defun completion-pcm--merge-try (pattern all prefix suffix)
  (cond
   ((not (consp all)) all)
   ((and (not (consp (cdr all)))        ;Only one completion.
         ;; Ignore completion-ignore-case here.
         (equal (completion-pcm--pattern->string pattern) (car all)))
    t)
   (t
[...]
----------------------------- End Quote ------------------------------

I.e. the behaviour for a single candidate (second condition case) is
different from when there's more than one candidate (third, and default
condition case). This could explain why you change seemed to improve
things.

> [...]
> OK, interesting; maybe we've found a bug in the completion engine
> itself.

That may well be the case.

And as the completion styles generously call each other, any change in
that area will require extra care.

> [...]
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 5faa3c8d4e8..455135628c8 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -1191,7 +1191,7 @@ completion--nth-completion
>                                 (error "Invalid completion style %s" style))
>                             string table pred point)))
>                 (and probe (cons probe style))))
> -           (completion--styles md)))
> +           (let ((styles (completion--styles md))) (message "STYLES: %S" styles) styles)))
>
> When I press TAB, it prints:
>
> STYLES: (substring partial-completion basic emacs22)
>
> even though globally, completion-styles is set to its default,
> (basic partial-completion emacs22)

Yes, the the "completion-category-defaults" logic does work, and what
you're seeing is that `completion-category-defaults` (or *-overrides)
when non-nil gets pre-pended to the buffer-local value of
`completion-styles`.


Cheers,

  --alexander

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

[Thomas Fitzsimmons]

Hi Alexander,

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

[...]

> 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))

I pushed this part of the patch to the emacs-29 branch.  It will soon be
automatically merged back to the master branch.

The other part of the patch got feedback for you, from Stefan.

Thomas

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

[Julien Cubizolles]

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.
>
> 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))))

In my case, also using corfu and BBDB, I'm currently happy with :

--8<---------------cut here---------------start------------->8---
(setq eudc-server-hotlist '(("localhost" . bbdb)))

(add-hook 'message-mode-hook
          (lambda ()
            (add-to-list 'completion-at-point-functions
                         #'eudc-capf-complete)))
--8<---------------cut here---------------end--------------->8---

The removal of message-completion-function was breaking completion for
the Gcc: field and 'eudc-capf-complete seems enough to get completion in
the To: field.

-- 
Julien Cubizolles

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

[Thomas Fitzsimmons]

Hi Julien,

Julien Cubizolles <j.cubizolles@free.fr> writes:

> 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.
>>
>> 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))))
>
> In my case, also using corfu and BBDB, I'm currently happy with :
>
> --8<---------------cut here---------------start------------->8---
> (setq eudc-server-hotlist '(("localhost" . bbdb)))
>
> (add-hook 'message-mode-hook
>           (lambda ()
>             (add-to-list 'completion-at-point-functions
>                          #'eudc-capf-complete)))
> --8<---------------cut here---------------end--------------->8---
>
> The removal of message-completion-function was breaking completion for
> the Gcc: field and 'eudc-capf-complete seems enough to get completion in
> the To: field.

OK, thanks for posting.  Is this with emacs-29 branch tip, i.e., what
will be released as Emacs 29.1?

Thomas

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

[Thomas Fitzsimmons]

Hi Stefan,

We've been trying to make the standard completion UI code in message.el
work with EUDC.

Would you be able to try this recipe and figure out why step 5 doesn't
work?

1. Build Emacs "emacs-29" branch.

2. Create test BBDB configuration:

mkdir -p /tmp/test/.emacs.d
cat >/tmp/test/.emacs.d/bbdb <<EOF
;; -*- mode: Emacs-Lisp; coding: utf-8; -*-
;;; file-format: 9
["Emacs" "ERT3" nil nil nil nil nil ("emacs-ert-test-3@bbdb.gnu.org") ((notes . " ")) "c8bd3a63-3a83-48a7-a95b-be118a923e00" "2022-11-19 16:36:04 +0000" "2022-11-19 16:36:04 +0000" nil]
["Emacs" "ERT4" nil nil nil nil nil ("emacs-ert-test-4@bbdb.gnu.org") ((notes . " ")) "5a93c3c5-9270-4e10-8b28-d28cfa2562cf" "2022-11-19 16:47:49 +0000" "2022-11-19 16:47:49 +0000" nil]
EOF

3. Install BBDB:

HOME=/tmp/test emacs
M-x list-packages
(install BBDB from GNU ELPA)
(exit Emacs)

4. Test EUDC using BBDB backend, and EUDC completion UI:

HOME=/tmp/test emacs
C-: (eudc-set-server "localhost" 'bbdb t)
C-x m
emacs-ertTAB

The EUDC UI pops up, and the completed email address is correctly added
to the To: line, e.g. "Emacs ERT3 <emacs-ert-test-3@bbdb.gnu.org>".

5. Test EUDC using BBDB backend, and the standard completion UI:

HOME=/tmp/test emacs
C-: (setq message-expand-name-standard-ui t)
C-: (eudc-set-server "localhost" 'bbdb t)
C-x m
emacs-ertTAB

This produces the string " <emacs-ert-test-" in the To: line.  Pressing
TAB again inserts three spaces.  Can you figure out why the standard UI
isn't prompting with multiple results?  We've made some attempts to
debug it but without deep knowledge of completion-at-point, we're
getting lost in the callbacks.

Thank you,
Thomas