Subject: bug#19678: [PATCH] EUDC does not support BBDB 3.x

From: Sergio Durigan Junior <[email-masked]>
Date: Fri, 06 Feb 2015 20:43:32 -0500

On Monday, January 26 2015, Thomas Fitzsimmons wrote:

> Sergio Durigan Junior <[email-masked]> writes:
>
>> On Monday, January 26 2015, Thomas Fitzsimmons wrote:
>>
>>>> Almost there...  The patch doesn't work as-is because
>>>> 'eudc-bbdb-current-return-attributes' is set to '(firstname lastname
>>>> mail) for me when eudc-bbdb-format-record-as-result is called.  It means
>>>> that, in the while loop, it will try to call eq/memq against 'mail, and
>>>> it fails.
>>>
>>> Hmm, I thought those get converted; in any case, I wasn't seeing that
>>> problem.  Do you have any customizations of the relevant variables?  Do
>>> you have my latest EUDC/LDAP changes from master tip?
>>
>> I am using git HEAD to test and develop, so I think I do have your
>> changes.
>>
>> As for my customizations, I'm almost sure they're the reason for me
>> seeing the errors.  What I have here is:
>>
>>   (eudc-protocol-set 'eudc-inline-expansion-format '("%s %s <%s>" firstname name mail)
>>                      'bbdb)
>
> OK, yes, that explains why you're seeing an error and I'm not.
>
> Though I can understand why you're doing it this way (see below), I
> think calling eudc-protocol-set directly in this case is a
> misconfiguration; it bypasses the "defcustom" way of doing things.
> Prior to my patches, the way this was supposed to work is that the user
> customized eudc-inline-expansion-format using generic EUDC symbols for
> the fields.  In your case, this would have been:
>
> (customize-set-variable 'eudc-inline-expansion-format
>                         '("%s %s <%s>" firstname name email))
>
> i.e., email not mail, and with no protocol specified.  Then EUDC would
> translate those to the fields used by the specific backend.  Then if you
> were using an LDAP server and a BBDB "server", you'd get the same
> results from both without having to figure out the "email" field for
> each of them.
>
> All that said, my patches fixed the default to be exactly what you're
> configuring.  Now you can just remove that line and it'll do exactly
> what you want.

[ I'm back from the trip.  Sorry about the long delay.  I confess I had
some things to mention/discuss about your reply before leaving, but now
I forgot almost all of them.  Oh, well...  ]

Right, thanks for explaining.  It indeed makes sense to use EUDC's
specific names, instead of BBDB/LDAP/whatever.  I will change my
configuration to reflect that.

>>   (eudc-protocol-set 'eudc-inline-query-format '((mail)
>>                                                  (firstname)
>>                                                  (lastname)
>>                                                  (firstname lastname)
>>                                                  (aka))
>>                      'bbdb)
>
> I also changed this default to be:
>
> '((email)
>   (firstname)
>   (firstname name))
>
> The aka is where eudc-protocol-set seems to make most sense; after all,
> aka is BBDB-specific.  But EUDC backends will just return no results for
> fields they don't know about, so it's actually safe to just put aka in
> eudc-inline-query-format via customization, without resorting to
> eudc-protocol-set.  The LDAP backend will ignore it, the BBDB backend
> will interpret and attempt it:
>
> (customize-set-variable 'eudc-inline-query-format '((email)
>                                                     (firstname)
>                                                     (name)
>                                                     (firstname name)
>                                                     (aka))
>
> The manual should maybe explicitly mention this, but then we'd have to
> expose all fields from all backends directly to the user for
> configuration, which may confuse things even more.
>
> In general I don't like this aspect of EUDC: how confusing it is to
> configure.  It's made even worse by the fact that even when configured
> properly it can still fail because of bugs.  I've tried to simplify the
> configuration by providing better defaults, and I'm also trying to fix
> the bugs.

Thanks a lot for working on EUDC.  I agree that it is frustrating to
stumble upon bugs every time.

>> Also, IMHO, the final patch makes sense to me, even if there are still
>> other issues to be fix on EUDC.
>
> I'd like to keep the attribute symbols "private" in the backends, and
> encourage people to use the generic EUDC symbol names instead (plus
> hidden extras like aka, if they want) using the standard customization
> procedures.  In which case the 'net stuff can stay as is in the BBDB
> backend.  Can you try the configuration changes I've outlined above,
> along with 0001-EUDC-Support-BBDB-3.patch, and confirm that they work
> for you?
>
> I'll credit you in the ChangeLog too since we both worked on the patch.
> Let me know when your copyright assignment goes through, and I'll push
> it after that.

All right, thanks a lot.

I still haven't received feedback from the FSF, but I believe things are
mostly OK since this is just an extension of my existing copyright
assignment for GDB/binutils.  That being said, I think that, if you want
to go ahead and push the patch, it's fine.  Otherwise, I will get back
to you when I receive FSF's reply.

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/