Re: [patch] alist option for add-log-mailing-address

[Thien-Thi Nguyen]

() "Bradley M. Kuhn" <bkuhn@softwarefreedom.org>
() Wed, 28 Mar 2007 13:47:31 -0400

   Finally, let me know if you like the patch or would like me to rewrite it
   a bit differently!

   +    * add-log.el (add-log-mailing-address): Added alist option.  Made
   +    tags more specific.
   +    (add-change-log-entry): Added code to handle alist setting on
   +    add-log-mailing-address.

s/Added/Add;s/Made/Make;

context diff preferred.

   +This defaults to the value of `user-mail-address'.  This value
   +can be a simple string; setting it as such will always use that
   +string in ChangeLog headers.  If the value is instead a list of
   +strings, when creating a new ChangeLog entry, one element will be
   +chosen at random.  If the value is an alist, the keys will be
   +considered regular expressions compared against the
   +`buffer-file-name' of the ChangeLog file, and the values will be
   +email address used when the expression matches.  If you set the
   +value to an alist, but no regular expressions match the
   +`buffer-file-name' of the ChangeLog, `user-mail-address' is used
   +instead."

i would move this documentation into the `add-change-log-entry'
docstring, including suitable cross references.  also, i would
restructure "foo will do bar" to be "foo bars" and factor "list of
elements where each element can be" idiom towards the front.  also, i
would not use "you", prefering to take `add-change-log-entry' pov.

                       (if (consp mailing-address)
   -                       mailing-address
   +                         (if (consp (car mailing-address)) ; it's an alist
   +                           (let ( (changelog-buffer (buffer-file-name))
   +                                  (preferred-addr user-mail-address) )
   +                             (mapc (lambda (regex-email-pair)
   +                                   (if (string-match
   +                                        (car regex-email-pair) 
changelog-buffer)
   +                                    (setq preferred-addr (cdr 
regex-email-pair))))
   +                                   add-log-mailing-address)
   +                             (list preferred-addr))
   +                           mailing-address)

IMHO, a `cond' expression is clearer than a nested-`if' here.  also,
extra space between parens is disturbing...

thi