[PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

[Sebastian Lipp]

Karl Fogel 
> In general, supplying a log message with the patch with avoid such
> confusion.  If there is some prose expressing what the change is
> supposed to to, and giving any historical context (such as the mailing
> list thread starting from last year), then it will be easy for any
> reviewer to understand what the patch is intended to do, and check if it
> actually does that.

Okay, next time I will provide that information inline. Thought keeping
References and In-Reply-To headers would be sufficient reference.

[PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

[Sebastian Lipp]

Karl Fogel  writes:
> Sebastian Lipp  writes:
>>diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
>>index fa65cd5..ee7b169 100644
>>--- a/emacs/notmuch-address.el
>>+++ b/emacs/notmuch-address.el
>>@@ -113,6 +113,59 @@ to know how address selection is made by default."
>> (when (notmuch-address-locate-command notmuch-address-command)
>>   (notmuch-address-message-insinuate))
>> 
>>+;; functions to add sender / recipients to BBDB
>>+
>>+(defvar bbdb-get-addresses-headers)
>
> I think it's good to include an initial value (even an invalid
> placeholder one, if the real initialization has not happened yet), and a
> doc string.  C-h f defvar RET will say more about how to do that.

I hope I got that right now. (I've got no real clue of Lisp yet because
I just recently switched to emacs partly because of notmuch. :)

How about

(defvar bbdb-get-addresses-headers nil
  "List of Addresses to import into bbdb")

?

> (By the way, this isn't a user-customizeable variable, right?  If it
> were, then `defcustom' would be better than `defvar'.)

As far as I understand it: It's not.

Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

[Sebastian Lipp]

Karl Fogel kfo...@red-bean.com writes:
 Sebastian Lipp ba...@riseup.net writes:
diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index fa65cd5..ee7b169 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -113,6 +113,59 @@ to know how address selection is made by default.
 (when (notmuch-address-locate-command notmuch-address-command)
   (notmuch-address-message-insinuate))
 
+;; functions to add sender / recipients to BBDB
+
+(defvar bbdb-get-addresses-headers)

 I think it's good to include an initial value (even an invalid
 placeholder one, if the real initialization has not happened yet), and a
 doc string.  C-h f defvar RET will say more about how to do that.

I hope I got that right now. (I've got no real clue of Lisp yet because
I just recently switched to emacs partly because of notmuch. :)

How about

(defvar bbdb-get-addresses-headers nil
  List of Addresses to import into bbdb)

?

 (By the way, this isn't a user-customizeable variable, right?  If it
 were, then `defcustom' would be better than `defvar'.)

As far as I understand it: It's not.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

[Sebastian Lipp]

Karl Fogel kfo...@red-bean.com
 In general, supplying a log message with the patch with avoid such
 confusion.  If there is some prose expressing what the change is
 supposed to to, and giving any historical context (such as the mailing
 list thread starting from last year), then it will be easy for any
 reviewer to understand what the patch is intended to do, and check if it
 actually does that.

Okay, next time I will provide that information inline. Thought keeping
References and In-Reply-To headers would be sufficient reference.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

[Sebastian Lipp]

Karl Fogel  writes:
> Sebastian Lipp  writes:
>>As I'd like to see this in notmuch I made the change. The patch is
>>attached. As it is my first contribution to notmuch at all: Just tell me
>>if I'm supposed to do it in any other way.
>
> I think your patch includes much more than just the above, though.

True. See below on that.

>>diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
>>index fa65cd5..ee7b169 100644
>>--- a/emacs/notmuch-address.el
>>+++ b/emacs/notmuch-address.el
>>@@ -113,6 +113,59 @@ to know how address selection is made by default."
>> (when (notmuch-address-locate-command notmuch-address-command)
>>   (notmuch-address-message-insinuate))
>> 
>>+;; functions to add sender / recipients to BBDB
>>+
>>+(defvar bbdb-get-addresses-headers)
>
> I think it's good to include an initial value (even an invalid
> placeholder one, if the real initialization has not happened yet), and a
> doc string.  C-h f defvar RET will say more about how to do that.

I will read into this and get back to it.

>>+(declare-function notmuch-show-get-header "notmuch-show" (header  
>>props))
>>+
>>+(declare-function bbdb-get-addresses "bbdb-com" 
>>+  (only-first-address
>>+   uninteresting-senders
>>+   get-header-content-function
>>+get-header-content-function-args))
>>+
>>+(declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
>>offer-to-create))
>
> At this point, your patch has accomplished what Tomi originally
> suggested.  But then the patch continues with what looks like
> substantive new code:
>
>>+(defun notmuch-bbdb/snarf-headers (headers)
>>+  ;; Helper function to avoid code duplication in the two below
>>+  ;; headers should have the same format as bbdb-get-addresses-headers
>>+
>>+  ;; bbdb-get-addresses reads these
>>+  ;; Ugh, pass-by-global
>>+  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))
>>+ (bbdb-get-addresses-headers headers) ; headers to read
>>+ (bbdb-gag-messages t)) ; suppress m/n processed message)
>>+(bbdb-update-records addrs t t)))
>>+
>>+(defun notmuch-bbdb/snarf-from ()
>>+  "Import the sender of the current message into BBDB"
>>+  (interactive)
>>+  (notmuch-bbdb/snarf-headers
>>+   (list (assoc 'authors bbdb-get-addresses-headers
>>+
>>+(defun notmuch-bbdb/snarf-to ()
>>+  "Import all recipients of the current message into BBDB"
>>+  (interactive)
>>+  (notmuch-bbdb/snarf-headers
>>+   (list (assoc 'recipients bbdb-get-addresses-headers
>>+
>>+(defvar notmuch-bbdb/header-by-name
>>+  ;; both are case sensitive
>>+  '( ("From" . :From)
>>+ ("To" . :To)
>>+ ("CC" . :Cc)
>>+ ("BCC" . :Bcc)
>>+ ("Resent-From" . nil)
>>+ ("Reply-To" . nil)
>>+ ("Resent-To" . nil)
>>+ ("Resent-CC" . nil))
>>+  "Alist for dispatching header symbols as used by notmuch-show-get-header
>>+from strings as used by bbdb-get-addresses")
>>+
>>+(defun notmuch-bbdb/get-header-content (name)
>>+  (notmuch-show-get-header (cdr (assoc name notmuch-bbdb/header-by-name
>
> It looks like new code, but I suspect what actually happened is that
> this is just the original code, somehow mis-expressed as "+" lines in
> the patch.  Is that what happened?

No, it happened by intention because what is "original" to you is not
part of my freshly cloned notmuch. So I thought good practise is to
construct *one* patch that brings the already fixed feature to the
notmuch codebase to keep it clean.

If you like better, my next patch will only base Tomis and your
suggestions on top of the "original" patch.

basti

[PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

[Sebastian Lipp]

Tomi Ollila  writes:
> On Tue, Apr 09 2013, David Bremner wrote:
>> There seems to be a few warnings:
>>
>> In notmuch-bbdb/snarf-from:
>> notmuch-address.el:116:26:Warning: reference to free variable
>> `bbdb-get-addresses-headers'
>>
>> In notmuch-bbdb/snarf-to:
>> notmuch-address.el:122:29:Warning: reference to free variable
>> `bbdb-get-addresses-headers'
>>
>> In end of data:
>> notmuch-address.el:143:1:Warning: the following functions are not known to be
>> defined: bbdb-get-addresses, bbdb-update-records, notmuch-show-get-header
>>
>> Do we need a few defvars?
>
> For the above set, something like:
>
> (defvar bbdb-get-addresses-headers)
>
> (declare-function notmuch-show-get-header "notmuch-show" (header  
> props))
>
> (declare-function bbdb-get-addresses "bbdb-com" 
>   (only-first-address
>uninteresting-senders
>get-header-content-function
> get-header-content-function-args))
>
> (declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
> offer-to-create))

As I'd like to see this in notmuch I made the change. The patch is
attached. As it is my first contribution to notmuch at all: Just tell me
if I'm supposed to do it in any other way.

LG
basti

-- next part --
A non-text attachment was scrubbed...
Name: 0001-emacs-functions-to-import-sender-or-recipient-into-B.patch
Type: text/x-diff
Size: 3030 bytes
Desc: not available
URL: 

Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

[Sebastian Lipp]

Karl Fogel kfo...@red-bean.com writes:
 Sebastian Lipp ba...@riseup.net writes:
As I'd like to see this in notmuch I made the change. The patch is
attached. As it is my first contribution to notmuch at all: Just tell me
if I'm supposed to do it in any other way.

 I think your patch includes much more than just the above, though.

True. See below on that.

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index fa65cd5..ee7b169 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -113,6 +113,59 @@ to know how address selection is made by default.
 (when (notmuch-address-locate-command notmuch-address-command)
   (notmuch-address-message-insinuate))
 
+;; functions to add sender / recipients to BBDB
+
+(defvar bbdb-get-addresses-headers)

 I think it's good to include an initial value (even an invalid
 placeholder one, if the real initialization has not happened yet), and a
 doc string.  C-h f defvar RET will say more about how to do that.

I will read into this and get back to it.

+(declare-function notmuch-show-get-header notmuch-show (header optional 
props))
+
+(declare-function bbdb-get-addresses bbdb-com 
+  (only-first-address
+   uninteresting-senders
+   get-header-content-function
+   rest get-header-content-function-args))
+
+(declare-function bbdb-update-records bbdb-com (addrs auto-create-p 
offer-to-create))

 At this point, your patch has accomplished what Tomi originally
 suggested.  But then the patch continues with what looks like
 substantive new code:

+(defun notmuch-bbdb/snarf-headers (headers)
+  ;; Helper function to avoid code duplication in the two below
+  ;; headers should have the same format as bbdb-get-addresses-headers
+
+  ;; bbdb-get-addresses reads these
+  ;; Ugh, pass-by-global
+  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))
+ (bbdb-get-addresses-headers headers) ; headers to read
+ (bbdb-gag-messages t)) ; suppress m/n processed message)
+(bbdb-update-records addrs t t)))
+
+(defun notmuch-bbdb/snarf-from ()
+  Import the sender of the current message into BBDB
+  (interactive)
+  (notmuch-bbdb/snarf-headers
+   (list (assoc 'authors bbdb-get-addresses-headers
+
+(defun notmuch-bbdb/snarf-to ()
+  Import all recipients of the current message into BBDB
+  (interactive)
+  (notmuch-bbdb/snarf-headers
+   (list (assoc 'recipients bbdb-get-addresses-headers
+
+(defvar notmuch-bbdb/header-by-name
+  ;; both are case sensitive
+  '( (From . :From)
+ (To . :To)
+ (CC . :Cc)
+ (BCC . :Bcc)
+ (Resent-From . nil)
+ (Reply-To . nil)
+ (Resent-To . nil)
+ (Resent-CC . nil))
+  Alist for dispatching header symbols as used by notmuch-show-get-header
+from strings as used by bbdb-get-addresses)
+
+(defun notmuch-bbdb/get-header-content (name)
+  (notmuch-show-get-header (cdr (assoc name notmuch-bbdb/header-by-name

 It looks like new code, but I suspect what actually happened is that
 this is just the original code, somehow mis-expressed as + lines in
 the patch.  Is that what happened?

No, it happened by intention because what is original to you is not
part of my freshly cloned notmuch. So I thought good practise is to
construct *one* patch that brings the already fixed feature to the
notmuch codebase to keep it clean.

If you like better, my next patch will only base Tomis and your
suggestions on top of the original patch.

basti
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

[Sebastian Lipp]

Tomi Ollila tomi.oll...@iki.fi writes:
 On Tue, Apr 09 2013, David Bremner wrote:
 There seems to be a few warnings:

 In notmuch-bbdb/snarf-from:
 notmuch-address.el:116:26:Warning: reference to free variable
 `bbdb-get-addresses-headers'

 In notmuch-bbdb/snarf-to:
 notmuch-address.el:122:29:Warning: reference to free variable
 `bbdb-get-addresses-headers'

 In end of data:
 notmuch-address.el:143:1:Warning: the following functions are not known to be
 defined: bbdb-get-addresses, bbdb-update-records, notmuch-show-get-header

 Do we need a few defvars?

 For the above set, something like:

 (defvar bbdb-get-addresses-headers)

 (declare-function notmuch-show-get-header notmuch-show (header optional 
 props))

 (declare-function bbdb-get-addresses bbdb-com 
   (only-first-address
uninteresting-senders
get-header-content-function
rest get-header-content-function-args))

 (declare-function bbdb-update-records bbdb-com (addrs auto-create-p 
 offer-to-create))

As I'd like to see this in notmuch I made the change. The patch is
attached. As it is my first contribution to notmuch at all: Just tell me
if I'm supposed to do it in any other way.

LG
basti

From 522e4294258e6392a02c923b4b7e78a898986fca Mon Sep 17 00:00:00 2001
From: Daniel Bergey ber...@alum.mit.edu
Date: Mon, 8 Apr 2013 19:55:04 -0500
Subject: [PATCH] emacs: functions to import sender or recipient into BBDB [v3]

From a show buffer, notmuch-bbdb/snarf-from imports the sender into
bbdb.  notmuch-bbdb/snarf-to imports all recipients.  Newly imported
contacts are reported in the minibuffer / Messages buffer.

Both functions use the BBDB parser to recognize email address formats.

[v3] Fixes a few warnings as suggested by Tomi Ollila in
 id:87vc7vgbym.fsf@zancas.localnet
[v2] Fixes missing close parenthesis in original.
 Spotted by Karl Fogel kfo...@red-bean.com.
---
 emacs/notmuch-address.el | 53 
 1 file changed, 53 insertions(+)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index fa65cd5..ee7b169 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -113,6 +113,59 @@ to know how address selection is made by default.
 (when (notmuch-address-locate-command notmuch-address-command)
   (notmuch-address-message-insinuate))
 
+;; functions to add sender / recipients to BBDB
+
+(defvar bbdb-get-addresses-headers)
+
+(declare-function notmuch-show-get-header notmuch-show (header optional props))
+
+(declare-function bbdb-get-addresses bbdb-com 
+  (only-first-address
+   uninteresting-senders
+   get-header-content-function
+   rest get-header-content-function-args))
+
+(declare-function bbdb-update-records bbdb-com (addrs auto-create-p offer-to-create))
+
+(defun notmuch-bbdb/snarf-headers (headers)
+  ;; Helper function to avoid code duplication in the two below
+  ;; headers should have the same format as bbdb-get-addresses-headers
+
+  ;; bbdb-get-addresses reads these
+  ;; Ugh, pass-by-global
+  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))
+	(bbdb-get-addresses-headers headers) ; headers to read
+	(bbdb-gag-messages t)) ; suppress m/n processed message)
+(bbdb-update-records addrs t t)))
+
+(defun notmuch-bbdb/snarf-from ()
+  Import the sender of the current message into BBDB
+  (interactive)
+  (notmuch-bbdb/snarf-headers
+   (list (assoc 'authors bbdb-get-addresses-headers
+
+(defun notmuch-bbdb/snarf-to ()
+  Import all recipients of the current message into BBDB
+  (interactive)
+  (notmuch-bbdb/snarf-headers
+   (list (assoc 'recipients bbdb-get-addresses-headers
+
+(defvar notmuch-bbdb/header-by-name
+  ;; both are case sensitive
+  '( (From . :From)
+ (To . :To)
+ (CC . :Cc)
+ (BCC . :Bcc)
+ (Resent-From . nil)
+ (Reply-To . nil)
+ (Resent-To . nil)
+ (Resent-CC . nil))
+  Alist for dispatching header symbols as used by notmuch-show-get-header
+from strings as used by bbdb-get-addresses)
+
+(defun notmuch-bbdb/get-header-content (name)
+  (notmuch-show-get-header (cdr (assoc name notmuch-bbdb/header-by-name
+
 ;;
 
 (provide 'notmuch-address)
-- 
2.0.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch