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

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

[Sergio Durigan Junior]

This bug was opened due to a request made at:

  <https://lists.gnu.org/archive/html/emacs-devel/2015-01/msg00800.html>

As explained at:

  <https://lists.gnu.org/archive/html/emacs-devel/2015-01/msg00542.html>

With the release of BBDB 3.x, EUDC needs adjustments to properly work
with it.  Actually, after some investigation, I noticed that the only
adjustment was to replace the occurrences of 'net' by 'mail' in the
code.  The second link above contains a patch that does that, but Thomas
Fitzsimmons correctly noted (in the first link) that the patch breaks
compatibility with BBDB 2.x.

The following patch is a first attempt to implement the support for both
BBDB 2.x and 3.x.

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

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 15518a7..0ed1d67 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,15 @@
+2015-01-24  Sergio Durigan Junior  <sergiodj@sergiodj.net>
+
+	Adapt EUDC to BBDB 3.x
+	* net/eudcb-bbdb.el (eudc-bbdb-get-major-version-number): New
+	function.
+	(eudc-bbdb-attributes-translation-alist): Use
+	eudc-bbdb-get-major-version-number to decide whether to use 'net'
+	(BBDB 2.x) or 'mail' (BBDB 3.x).
+	(eudc-bbdb-format-query): Likewise.
+	(eudc-bbdb-filter-non-matching-record): Likewise.
+	(eudc-bbdb-format-record-as-result): Likewise.
+
 2015-01-23  Thomas Fitzsimmons  <fitzsim@fitzsim.org>
 
 	* net/ldap.el (ldap-search-internal): Mention binddn in invalid
diff --git a/lisp/net/eudcb-bbdb.el b/lisp/net/eudcb-bbdb.el
index 0400e5b..d400871 100644
--- a/lisp/net/eudcb-bbdb.el
+++ b/lisp/net/eudcb-bbdb.el
@@ -41,9 +41,18 @@
 (defvar eudc-bbdb-current-query nil)
 (defvar eudc-bbdb-current-return-attributes nil)
 
+(defun eudc-bbdb-get-major-version-number ()
+  "Return the major version number of the BBDB version we are
+  currently using.  Return nil if we could not load BBDB."
+  (if (require 'bbdb nil t)
+      (string-to-number (substring bbdb-version 0 1))
+    0))
+
 (defvar eudc-bbdb-attributes-translation-alist
   '((name . lastname)
-    (email . net)
+    (email . (if (= (eudc-bbdb-get-major-version-number) 3)
+		 mail
+	       net))
     (phone . phones))
   "Alist mapping EUDC attribute names to BBDB names.")
 
@@ -63,10 +72,13 @@
 		   firstname
 		   lastname))
 	(company (cdr (assq 'company query)))
-	(net (cdr (assq 'net query)))
+	(mail (cdr (assq (if (= (eudc-bbdb-get-major-version-number) 3)
+			     'mail
+			   'net)
+			 query)))
 	(notes (cdr (assq 'notes query)))
 	(phone (cdr (assq 'phone query))))
-    (list name company net notes phone)))
+    (list name company mail notes phone)))
 
 
 (defun eudc-bbdb-filter-non-matching-record (record)
@@ -80,7 +92,10 @@
               (case-fold-search t)
               bbdb-val)
           (or (and (memq attr '(firstname lastname aka company phones
-                                addresses net))
+				addresses
+			        (if (= (eudc-bbdb-get-major-version-number) 3)
+				    mail
+				  net)))
                    (progn
                      (setq bbdb-val
                            (eval (list (intern (concat "bbdb-record-"
@@ -151,7 +166,11 @@
 The record is filtered according to `eudc-bbdb-current-return-attributes'"
   (require 'bbdb)
   (let ((attrs (or eudc-bbdb-current-return-attributes
-		   '(firstname lastname aka company phones addresses net notes)))
+		   '(firstname lastname aka company phones addresses
+		     (if (= (eudc-bbdb-get-major-version-number) 3)
+			 mail
+		       net)
+		     notes)))
 	attr
 	eudc-rec
 	val)
@@ -163,7 +182,11 @@ The record is filtered according to `eudc-bbdb-current-return-attributes'"
 	(setq val (eudc-bbdb-extract-phones record)))
        ((eq attr 'addresses)
 	(setq val (eudc-bbdb-extract-addresses record)))
-       ((memq attr '(firstname lastname aka company net notes))
+       ((memq attr '(firstname lastname aka company
+		     (if (= (eudc-bbdb-get-major-version-number) 3)
+			 mail
+		       net)
+		     notes))
 	(setq val (eval
 		   (list (intern
 			  (concat "bbdb-record-"

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

[Thomas Fitzsimmons]

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

Sergio Durigan Junior <sergiodj@sergiodj.net> writes:

> This bug was opened due to a request made at:
>
>   <https://lists.gnu.org/archive/html/emacs-devel/2015-01/msg00800.html>
>
> As explained at:
>
>   <https://lists.gnu.org/archive/html/emacs-devel/2015-01/msg00542.html>
>
> With the release of BBDB 3.x, EUDC needs adjustments to properly work
> with it.  Actually, after some investigation, I noticed that the only
> adjustment was to replace the occurrences of 'net' by 'mail' in the
> code.  The second link above contains a patch that does that, but Thomas
> Fitzsimmons correctly noted (in the first link) that the patch breaks
> compatibility with BBDB 2.x.
>
> The following patch is a first attempt to implement the support for both
> BBDB 2.x and 3.x.

Can you see if the attached patch works for you?  It works for me,
introduces minimal changes and allows for upgrading BBDB 2 to BBDB 3
without restarting Emacs.  The defvar change in your patch concerns me
because it will miss if the BBDB version changes after the variable is
first assigned.

I'm also going to experiment with eudc-export which also may need to be
updated for BBDB 3 handling.

Longer term I hope to work toward bundling BBDB in Emacs which would
eliminate the need to support multiple versions.

Thomas

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-EUDC-Support-BBDB-3.patch --]
[-- Type: text/x-patch, Size: 2635 bytes --]

From 34dbaf966d2875c1f129f35c9866122f88e0ac16 Mon Sep 17 00:00:00 2001
From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
Date: Sun, 25 Jan 2015 01:32:52 -0500
Subject: [PATCH 1/2] EUDC: Support BBDB >= 3

* net/eudcb-bbdb.el (eudc-bbdb-field): New function.
(eudc-bbdb-filter-non-matching-record): Call eudc-bbdb-field.
(eudc-bbdb-format-record-as-result): Likewise.
---
 lisp/ChangeLog         |  6 ++++++
 lisp/net/eudcb-bbdb.el | 16 ++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 70293af..e26e028 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,9 @@
+2015-01-25  Thomas Fitzsimmons  <fitzsim@fitzsim.org>
+
+	* net/eudcb-bbdb.el (eudc-bbdb-field): New function.
+	(eudc-bbdb-filter-non-matching-record): Call eudc-bbdb-field.
+	(eudc-bbdb-format-record-as-result): Likewise.
+
 2015-01-25  Stefan Monnier  <monnier@iro.umontreal.ca>
 
 	* emacs-lisp/cl-generic.el (cl--generic-no-next-method-function): New fun.
diff --git a/lisp/net/eudcb-bbdb.el b/lisp/net/eudcb-bbdb.el
index 0400e5b..7aed542 100644
--- a/lisp/net/eudcb-bbdb.el
+++ b/lisp/net/eudcb-bbdb.el
@@ -41,6 +41,16 @@
 (defvar eudc-bbdb-current-query nil)
 (defvar eudc-bbdb-current-return-attributes nil)
 
+(defun eudc-bbdb-field (field-symbol)
+  "Convert FIELD-SYMBOL so that it is recognized by the current BBDB version.
+BBDB < 3 used `net'; BBDB >= 3 uses `mail'."
+  ;; This just-in-time translation permits upgrading from BBDB 2 to
+  ;; BBDB 3 without restarting Emacs.
+  (if (and (eq field-symbol 'net)
+	   (version<= "3" bbdb-version))
+      'mail
+    field-symbol))
+
 (defvar eudc-bbdb-attributes-translation-alist
   '((name . lastname)
     (email . net)
@@ -84,7 +94,9 @@
                    (progn
                      (setq bbdb-val
                            (eval (list (intern (concat "bbdb-record-"
-                                                       (symbol-name attr)))
+                                                       (symbol-name
+                                                        (eudc-bbdb-field
+                                                         attr))))
                                        'record)))
                      (if (listp bbdb-val)
                          (if eudc-bbdb-enable-substring-matches
@@ -167,7 +179,7 @@ The record is filtered according to `eudc-bbdb-current-return-attributes'"
 	(setq val (eval
 		   (list (intern
 			  (concat "bbdb-record-"
-				  (symbol-name attr)))
+				  (symbol-name (eudc-bbdb-field attr))))
 			 'record))))
        (t
 	(error "Unknown BBDB attribute")))
-- 
1.8.1.4

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

[Sergio Durigan Junior]

On Sunday, January 25 2015, Thomas Fitzsimmons wrote:

> Can you see if the attached patch works for you?  It works for me,
> introduces minimal changes and allows for upgrading BBDB 2 to BBDB 3
> without restarting Emacs.  The defvar change in your patch concerns me
> because it will miss if the BBDB version changes after the variable is
> first assigned.

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.

What I've done now is to call eudc-bbdb-field when we are composing the
variables that will hold the BBDB attributes to match against.

> I'm also going to experiment with eudc-export which also may need to be
> updated for BBDB 3 handling.

Nice.  Let me know if you need some help.

> Longer term I hope to work toward bundling BBDB in Emacs which would
> eliminate the need to support multiple versions.

That would be awesome.  Again, I could help with the task.

So, the following patch is the last version of what works for my case.

I hope it is OK to apply.

Cheers,

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

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index ff352a2..4bee79b 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,11 @@
+2015-01-25  Thomas Fitzsimmons  <fitzsim@fitzsim.org>
+	    Sergio Durigan Junior  <sergiodj@sergiodj.net>
+
+	* net/eudcb-bbdb.el (eudc-bbdb-field): New function.
+	(eudc-bbdb-format-query): Call eudc-bbdb-field.
+	(eudc-bbdb-filter-non-matching-record): Likewise.
+	(eudc-bbdb-format-record-as-result): Likewise.
+
 2015-01-26  Stefan Monnier  <monnier@iro.umontreal.ca>
 
 
diff --git a/lisp/net/eudcb-bbdb.el b/lisp/net/eudcb-bbdb.el
index 0400e5b..10c2c86 100644
--- a/lisp/net/eudcb-bbdb.el
+++ b/lisp/net/eudcb-bbdb.el
@@ -41,6 +41,16 @@
 (defvar eudc-bbdb-current-query nil)
 (defvar eudc-bbdb-current-return-attributes nil)
 
+(defun eudc-bbdb-field (field-symbol)
+  "Convert FIELD-SYMBOL so that it is recognized by the current BBDB version.
+BBDB < 3 used `net'; BBDB >= 3 uses `mail'."
+  ;; This just-in-time translation permits upgrading from BBDB 2 to
+  ;; BBDB 3 without restarting Emacs.
+  (if (and (eq field-symbol 'net)
+	   (version<= "3" bbdb-version))
+      'mail
+    field-symbol))
+
 (defvar eudc-bbdb-attributes-translation-alist
   '((name . lastname)
     (email . net)
@@ -63,7 +73,7 @@
 		   firstname
 		   lastname))
 	(company (cdr (assq 'company query)))
-	(net (cdr (assq 'net query)))
+	(net (cdr (assq (eudc-bbdb-field 'net) query)))
 	(notes (cdr (assq 'notes query)))
 	(phone (cdr (assq 'phone query))))
     (list name company net notes phone)))
@@ -79,12 +89,14 @@
               (val (cdr condition))
               (case-fold-search t)
               bbdb-val)
-          (or (and (memq attr '(firstname lastname aka company phones
-                                addresses net))
+          (or (and (memq attr `(firstname lastname aka company phones
+                                addresses ,(eudc-bbdb-field 'net)))
                    (progn
                      (setq bbdb-val
                            (eval (list (intern (concat "bbdb-record-"
-                                                       (symbol-name attr)))
+                                                       (symbol-name
+                                                        (eudc-bbdb-field
+                                                         attr))))
                                        'record)))
                      (if (listp bbdb-val)
                          (if eudc-bbdb-enable-substring-matches
@@ -151,7 +163,8 @@
 The record is filtered according to `eudc-bbdb-current-return-attributes'"
   (require 'bbdb)
   (let ((attrs (or eudc-bbdb-current-return-attributes
-		   '(firstname lastname aka company phones addresses net notes)))
+		   `(firstname lastname aka company phones addresses
+		     ,(eudc-bbdb-field 'net) notes)))
 	attr
 	eudc-rec
 	val)
@@ -163,11 +176,12 @@ The record is filtered according to `eudc-bbdb-current-return-attributes'"
 	(setq val (eudc-bbdb-extract-phones record)))
        ((eq attr 'addresses)
 	(setq val (eudc-bbdb-extract-addresses record)))
-       ((memq attr '(firstname lastname aka company net notes))
+       ((memq attr `(firstname lastname aka company
+		     ,(eudc-bbdb-field 'net) notes))
 	(setq val (eval
 		   (list (intern
 			  (concat "bbdb-record-"
-				  (symbol-name attr)))
+				  (symbol-name (eudc-bbdb-field attr))))
 			 'record))))
        (t
 	(error "Unknown BBDB attribute")))

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

[Thomas Fitzsimmons]

Sergio Durigan Junior <sergiodj@sergiodj.net> writes:

> On Sunday, January 25 2015, Thomas Fitzsimmons wrote:
>
>> Can you see if the attached patch works for you?  It works for me,
>> introduces minimal changes and allows for upgrading BBDB 2 to BBDB 3
>> without restarting Emacs.  The defvar change in your patch concerns me
>> because it will miss if the BBDB version changes after the variable is
>> first assigned.
>
> 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?

Can we step back a bit and make sure we're doing the same tests?  BBDB
2.x is tricky because it is provided by the distro, in my case Fedora.
Let's focus on testing BBDB 3.x so that I can replicate the exact same
issue that you're seeing.

Can you revert our patch, then:

1) Checkout and build Emacs revision
   03a20dc9519616359bfa1b77fd4b31e1963c8bd4 from
   git://git.savannah.gnu.org/emacs.git

   This revision has a bunch of my EUDC/LDAP updates in it.

2) Download
   http://download.savannah.gnu.org/releases/bbdb/bbdb-3.1.2.tar.gz

3) Untar and build the ELPA package:
   export EMACS=<your-emacs-build-dir>/src/emacs
   ./configure && make elpa
   unset EMACS

4) In the emacs src directory:
   mkdir test-home

5) HOME=`pwd`/test-home ./emacs -Q

6) M-x package-install-file

   bbdb-3.1.2.tar (the one built in step 3)

7) M-x bbdb-create

   Name: Test User
   Network Address: test@gnu.org

8) C-x s

   to save .bbdb

9) M-: (eval-after-load "message"
        '(define-key message-mode-map (kbd "TAB") 'eudc-expand-inline))

10) M-: (setq debug-on-error 't)

11) C-x m

12) Tes[TAB]

    (no server, bbdb protocol)

Without my patch, I get:

Debugger entered--Lisp error: (void-function bbdb-record-net)
  (bbdb-record-net record)
  eval((bbdb-record-net record))

  eudc-bbdb-format-record-as-result(["Test" "User" nil nil nil nil nil
  ("test@gnu.org") ((creation-date . "2015-01-26 21:56:26 +0000")
  (timestamp . "2015-01-26 21:56:26 +0000")) ["Test User" "User, Test"
  nil ("test@gnu.org") nil #<marker at 64 in bbdb>]])
  
  mapcar(eudc-bbdb-format-record-as-result (["Test" "User" nil nil nil
  nil nil ("test@gnu.org") ((creation-date . "2015-01-26 21:56:26
  +0000") (timestamp . "2015-01-26 21:56:26 +0000")) ["Test User" "User,
  Test" nil ("test@gnu.org") nil #<marker at 64 in bbdb>]]))
  
  eudc-bbdb-query-internal(((firstname . "Tes")) (firstname lastname
  net))
  
  eudc-query(((firstname . "Tes")) (firstname lastname net))
  eudc-expand-inline()
  funcall-interactively(eudc-expand-inline)
  call-interactively(eudc-expand-inline nil nil)
  command-execute(eudc-expand-inline)

With my patch (0001-EUDC-Support-BBDB-3.patch), it works.  eudc-query
gets called with 'net, not 'mail.

If that works for you, can you try to replicate the other error you're
seeing when my patch is applied, in this same minimal environment, and
paste the testing steps and the backtrace you get?

Thanks,
Thomas

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

[Sergio Durigan Junior]

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)
  (eudc-protocol-set 'eudc-inline-query-format '((mail)
                                                 (firstname)
                                                 (lastname)
                                                 (firstname lastname)
                                                 (aka))
                     'bbdb)

> Can we step back a bit and make sure we're doing the same tests?  BBDB
> 2.x is tricky because it is provided by the distro, in my case Fedora.
> Let's focus on testing BBDB 3.x so that I can replicate the exact same
> issue that you're seeing.
>
> Can you revert our patch, then:
>
> 1) Checkout and build Emacs revision
>    03a20dc9519616359bfa1b77fd4b31e1963c8bd4 from
>    git://git.savannah.gnu.org/emacs.git
>
>    This revision has a bunch of my EUDC/LDAP updates in it.

I'm buying an even newer revision from git, FWIW.

> 2) Download
>    http://download.savannah.gnu.org/releases/bbdb/bbdb-3.1.2.tar.gz
>
> 3) Untar and build the ELPA package:
>    export EMACS=<your-emacs-build-dir>/src/emacs
>    ./configure && make elpa
>    unset EMACS
>
> 4) In the emacs src directory:
>    mkdir test-home
>
> 5) HOME=`pwd`/test-home ./emacs -Q
>
> 6) M-x package-install-file
>
>    bbdb-3.1.2.tar (the one built in step 3)
>
> 7) M-x bbdb-create
>
>    Name: Test User
>    Network Address: test@gnu.org
>
> 8) C-x s
>
>    to save .bbdb
>
> 9) M-: (eval-after-load "message"
>         '(define-key message-mode-map (kbd "TAB") 'eudc-expand-inline))
>
> 10) M-: (setq debug-on-error 't)
>
> 11) C-x m
>
> 12) Tes[TAB]
>
>     (no server, bbdb protocol)
>
> Without my patch, I get:
>
> Debugger entered--Lisp error: (void-function bbdb-record-net)
>   (bbdb-record-net record)
>   eval((bbdb-record-net record))
>
>   eudc-bbdb-format-record-as-result(["Test" "User" nil nil nil nil nil
>   ("test@gnu.org") ((creation-date . "2015-01-26 21:56:26 +0000")
>   (timestamp . "2015-01-26 21:56:26 +0000")) ["Test User" "User, Test"
>   nil ("test@gnu.org") nil #<marker at 64 in bbdb>]])
>   
>   mapcar(eudc-bbdb-format-record-as-result (["Test" "User" nil nil nil
>   nil nil ("test@gnu.org") ((creation-date . "2015-01-26 21:56:26
>   +0000") (timestamp . "2015-01-26 21:56:26 +0000")) ["Test User" "User,
>   Test" nil ("test@gnu.org") nil #<marker at 64 in bbdb>]]))
>   
>   eudc-bbdb-query-internal(((firstname . "Tes")) (firstname lastname
>   net))
>   
>   eudc-query(((firstname . "Tes")) (firstname lastname net))
>   eudc-expand-inline()
>   funcall-interactively(eudc-expand-inline)
>   call-interactively(eudc-expand-inline nil nil)
>   command-execute(eudc-expand-inline)
>
> With my patch (0001-EUDC-Support-BBDB-3.patch), it works.  eudc-query
> gets called with 'net, not 'mail.
>
> If that works for you, can you try to replicate the other error you're
> seeing when my patch is applied, in this same minimal environment, and
> paste the testing steps and the backtrace you get?

If you insist, I can do this test later (unfortunately I will be very
busy these next days).  Meanwhile, if you could check that my
configuration is what triggers the failure, I'd appreciate.  Also, IMHO,
the final patch makes sense to me, even if there are still other issues
to be fix on EUDC.

Thank you,

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

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

[Thomas Fitzsimmons]

Sergio Durigan Junior <sergiodj@sergiodj.net> 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.

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

>> Can we step back a bit and make sure we're doing the same tests?  BBDB
>> 2.x is tricky because it is provided by the distro, in my case Fedora.
>> Let's focus on testing BBDB 3.x so that I can replicate the exact same
>> issue that you're seeing.
>>
>> Can you revert our patch, then:
>>
>> 1) Checkout and build Emacs revision
>>    03a20dc9519616359bfa1b77fd4b31e1963c8bd4 from
>>    git://git.savannah.gnu.org/emacs.git
>>
>>    This revision has a bunch of my EUDC/LDAP updates in it.
>
> I'm buying an even newer revision from git, FWIW.
>
>> 2) Download
>>    http://download.savannah.gnu.org/releases/bbdb/bbdb-3.1.2.tar.gz
>>
>> 3) Untar and build the ELPA package:
>>    export EMACS=<your-emacs-build-dir>/src/emacs
>>    ./configure && make elpa
>>    unset EMACS
>>
>> 4) In the emacs src directory:
>>    mkdir test-home
>>
>> 5) HOME=`pwd`/test-home ./emacs -Q
>>
>> 6) M-x package-install-file
>>
>>    bbdb-3.1.2.tar (the one built in step 3)
>>
>> 7) M-x bbdb-create
>>
>>    Name: Test User
>>    Network Address: test@gnu.org
>>
>> 8) C-x s
>>
>>    to save .bbdb
>>
>> 9) M-: (eval-after-load "message"
>>         '(define-key message-mode-map (kbd "TAB") 'eudc-expand-inline))
>>
>> 10) M-: (setq debug-on-error 't)
>>
>> 11) C-x m
>>
>> 12) Tes[TAB]
>>
>>     (no server, bbdb protocol)
>>
>> Without my patch, I get:
>>
>> Debugger entered--Lisp error: (void-function bbdb-record-net)
>>   (bbdb-record-net record)
>>   eval((bbdb-record-net record))
>>
>>   eudc-bbdb-format-record-as-result(["Test" "User" nil nil nil nil nil
>>   ("test@gnu.org") ((creation-date . "2015-01-26 21:56:26 +0000")
>>   (timestamp . "2015-01-26 21:56:26 +0000")) ["Test User" "User, Test"
>>   nil ("test@gnu.org") nil #<marker at 64 in bbdb>]])
>>   
>>   mapcar(eudc-bbdb-format-record-as-result (["Test" "User" nil nil nil
>>   nil nil ("test@gnu.org") ((creation-date . "2015-01-26 21:56:26
>>   +0000") (timestamp . "2015-01-26 21:56:26 +0000")) ["Test User" "User,
>>   Test" nil ("test@gnu.org") nil #<marker at 64 in bbdb>]]))
>>   
>>   eudc-bbdb-query-internal(((firstname . "Tes")) (firstname lastname
>>   net))
>>   
>>   eudc-query(((firstname . "Tes")) (firstname lastname net))
>>   eudc-expand-inline()
>>   funcall-interactively(eudc-expand-inline)
>>   call-interactively(eudc-expand-inline nil nil)
>>   command-execute(eudc-expand-inline)
>>
>> With my patch (0001-EUDC-Support-BBDB-3.patch), it works.  eudc-query
>> gets called with 'net, not 'mail.
>>
>> If that works for you, can you try to replicate the other error you're
>> seeing when my patch is applied, in this same minimal environment, and
>> paste the testing steps and the backtrace you get?
>
> If you insist, I can do this test later (unfortunately I will be very
> busy these next days). 

No need.

> Meanwhile, if you could check that my configuration is what triggers
> the failure, I'd appreciate.

Done.

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

Thanks,
Thomas

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

[Sergio Durigan Junior]

On Monday, January 26 2015, Thomas Fitzsimmons wrote:

> Sergio Durigan Junior <sergiodj@sergiodj.net> 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/

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

[Thomas Fitzsimmons]

Hi,

I pushed our patch, with some extra version checks I discovered were
needed during testing.

Thanks for reporting and providing a patch,
Thomas