* bug#19678: [PATCH] EUDC does not support BBDB 3.x @ 2015-01-24 23:37 Sergio Durigan Junior 2015-01-25 23:49 ` Thomas Fitzsimmons 0 siblings, 1 reply; 8+ messages in thread From: Sergio Durigan Junior @ 2015-01-24 23:37 UTC (permalink / raw) To: 19678 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-" ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#19678: [PATCH] EUDC does not support BBDB 3.x 2015-01-24 23:37 bug#19678: [PATCH] EUDC does not support BBDB 3.x Sergio Durigan Junior @ 2015-01-25 23:49 ` Thomas Fitzsimmons 2015-01-26 19:02 ` Sergio Durigan Junior 0 siblings, 1 reply; 8+ messages in thread From: Thomas Fitzsimmons @ 2015-01-25 23:49 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: 19678 [-- 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#19678: [PATCH] EUDC does not support BBDB 3.x 2015-01-25 23:49 ` Thomas Fitzsimmons @ 2015-01-26 19:02 ` Sergio Durigan Junior 2015-01-26 22:21 ` Thomas Fitzsimmons 0 siblings, 1 reply; 8+ messages in thread From: Sergio Durigan Junior @ 2015-01-26 19:02 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: 19678 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"))) ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#19678: [PATCH] EUDC does not support BBDB 3.x 2015-01-26 19:02 ` Sergio Durigan Junior @ 2015-01-26 22:21 ` Thomas Fitzsimmons 2015-01-26 22:47 ` Sergio Durigan Junior 0 siblings, 1 reply; 8+ messages in thread From: Thomas Fitzsimmons @ 2015-01-26 22:21 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: 19678 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#19678: [PATCH] EUDC does not support BBDB 3.x 2015-01-26 22:21 ` Thomas Fitzsimmons @ 2015-01-26 22:47 ` Sergio Durigan Junior 2015-01-27 2:10 ` Thomas Fitzsimmons 0 siblings, 1 reply; 8+ messages in thread From: Sergio Durigan Junior @ 2015-01-26 22:47 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: 19678 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/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#19678: [PATCH] EUDC does not support BBDB 3.x 2015-01-26 22:47 ` Sergio Durigan Junior @ 2015-01-27 2:10 ` Thomas Fitzsimmons 2015-02-07 1:43 ` Sergio Durigan Junior 0 siblings, 1 reply; 8+ messages in thread From: Thomas Fitzsimmons @ 2015-01-27 2:10 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: 19678 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#19678: [PATCH] EUDC does not support BBDB 3.x 2015-01-27 2:10 ` Thomas Fitzsimmons @ 2015-02-07 1:43 ` Sergio Durigan Junior 2015-03-09 1:22 ` Thomas Fitzsimmons 0 siblings, 1 reply; 8+ messages in thread From: Sergio Durigan Junior @ 2015-02-07 1:43 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: 19678 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/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#19678: [PATCH] EUDC does not support BBDB 3.x 2015-02-07 1:43 ` Sergio Durigan Junior @ 2015-03-09 1:22 ` Thomas Fitzsimmons 0 siblings, 0 replies; 8+ messages in thread From: Thomas Fitzsimmons @ 2015-03-09 1:22 UTC (permalink / raw) To: 19678-done Hi, I pushed our patch, with some extra version checks I discovered were needed during testing. Thanks for reporting and providing a patch, Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-03-09 1:22 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-24 23:37 bug#19678: [PATCH] EUDC does not support BBDB 3.x Sergio Durigan Junior 2015-01-25 23:49 ` Thomas Fitzsimmons 2015-01-26 19:02 ` Sergio Durigan Junior 2015-01-26 22:21 ` Thomas Fitzsimmons 2015-01-26 22:47 ` Sergio Durigan Junior 2015-01-27 2:10 ` Thomas Fitzsimmons 2015-02-07 1:43 ` Sergio Durigan Junior 2015-03-09 1:22 ` Thomas Fitzsimmons
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).