Re: [PATCH] Fix bug #11580

[Tassilo Horn]

Sergio Durigan Junior <address@hidden> writes:

Hi Sergio,

> The attached patch fixes the bug listed on $SUBJECT.  Maybe there are
> better ways to fix it, but a quick hack did the trick so I am sending
> it for review.

I don't use bbdb, so I've Cc-ed Roland who is the current bbdb
maintainer.  Roland, the complete bug thread is here:

  http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11580

In general, I think the patch isn't wrong, but somehow the whole
function looks awkward to me.  COMMENTs inside:

--8<---------------cut here---------------start------------->8---
(defun eudc-bbdb-format-record-as-result (record)
  "Format the BBDB RECORD as a EUDC query result record.
The record is filtered according to `eudc-bbdb-current-return-attributes'"
  (let ((attrs (or eudc-bbdb-current-return-attributes
                   '(firstname lastname aka company phones addresses net 
notes)))
        attr
        eudc-rec
        val)
    (while (prog1
               (setq attr (car attrs))
             (setq attrs (cdr attrs)))
      (cond
       ((eq attr 'phones)
        (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))
        (setq val (eval
                   (list (intern
                          (concat "bbdb-record-"
                                  (symbol-name attr)))
                         'record))))
       (t
        ;; COMMENT1: Shouldn't that be (error "Unknown BBDB attribute")?
        ;; Now, we'll enter the case of COMMENT3 with that val.
        (setq val "Unknown BBDB attribute")))
      ;; COMMENT2: Your change, basically ok.  Before it was just (if val ...
      (if (and val (or (listp val) (not (string= val ""))))
        (cond
         ((memq attr '(phones addresses))
          (setq eudc-rec (append val eudc-rec)))
         ((and (listp val)
               (= 1 (length val)))
          (setq eudc-rec (cons (cons attr (car val)) eudc-rec)))
         ;; COMMENT3: Don't we need to distinguish between a list of
         ;; length > 0 and a string of length > 0? Or can't that happen?
         ((> (length val) 0)
          (setq eudc-rec (cons (cons attr val) eudc-rec)))
         (t
          (error "Unexpected attribute value")))))
    (nreverse eudc-rec)))
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo