af/75cb738f34fd3763495a410275453ce9540f37

   1 Return-Path: <bacuh@riseup.net>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id AE333431FAF\r
   6         for <notmuch@notmuchmail.org>; Fri,  4 Jul 2014 02:25:45 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0.001\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0.001 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_NONE=-0.0001, UNPARSEABLE_RELAY=0.001]\r
  13         autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id 8T3r01Yrb0KL for <notmuch@notmuchmail.org>;\r
  17         Fri,  4 Jul 2014 02:25:38 -0700 (PDT)\r
  18 Received: from mx1.riseup.net (mx1.riseup.net [198.252.153.129])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id C9DDA431FAE\r
  22         for <notmuch@notmuchmail.org>; Fri,  4 Jul 2014 02:25:38 -0700 (PDT)\r
  23 Received: from fruiteater.riseup.net (fruiteater-pn.riseup.net [10.0.1.74])\r
  24         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  25         (Client CN "*.riseup.net",\r
  26         Issuer "Gandi Standard SSL CA" (not verified))\r
  27         by mx1.riseup.net (Postfix) with ESMTPS id 5FCBB50E6C;\r
  28         Fri,  4 Jul 2014 02:25:37 -0700 (PDT)\r
  29 Received: from [127.0.0.1] (localhost [127.0.0.1])\r
  30         (Authenticated sender: bacuh@fruiteater.riseup.net)\r
  31         with ESMTPSA id 2E27E10BF\r
  32 From: Sebastian Lipp <bacuh@riseup.net>\r
  33 To: Karl Fogel <kfogel@red-bean.com>\r
  34 Subject: Re: [PATCH] emacs: add missing paren to fix defun in\r
  35         notmuch-address.el.\r
  36 In-Reply-To: <87oax6vt4u.fsf@ktab.red-bean.com>\r
  37 References: <87wqsfik5l.fsf@floss.red-bean.com>\r
  38         <87mwtah2hd.fsf@zancas.localnet>\r
  39         <877gkeau42.fsf@floss.red-bean.com>\r
  40         <m2ppy5v3mc.fsf@guru.guru-group.fi> <8738v1jtzc.fsf@nikula.org>\r
  41         <87k3odgutd.fsf@zancas.localnet> <87ip3wbj7g.fsf@mcs.anl.gov>\r
  42         <m2hajgxli5.fsf@guru.guru-group.fi>\r
  43         <87vc7vgbym.fsf@zancas.localnet>\r
  44         <m2ehejykb1.fsf@guru.guru-group.fi>\r
  45         <87pphmc604.fsf@verb.i-did-not-set--mail-host-address--so-tickle-me>\r
  46         <87oax6vt4u.fsf@ktab.red-bean.com>\r
  47 User-Agent: Notmuch/0.18 (http://notmuchmail.org) Emacs/24.3.1\r
  48         (i686-pc-linux-gnu)\r
  49 Date: Fri, 04 Jul 2014 11:25:38 +0200\r
  50 Message-ID:\r
  51  <87r4215vcd.fsf@verb.i-did-not-set--mail-host-address--so-tickle-me>\r
  52 MIME-Version: 1.0\r
  53 Content-Type: text/plain\r
  54 X-Virus-Scanned: clamav-milter 0.98.1 at mx1\r
  55 X-Virus-Status: Clean\r
  56 Cc: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org\r
  57 X-BeenThere: notmuch@notmuchmail.org\r
  58 X-Mailman-Version: 2.1.13\r
  59 Precedence: list\r
  60 List-Id: "Use and development of the notmuch mail system."\r
  61         <notmuch.notmuchmail.org>\r
  62 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  63         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  64 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  65 List-Post: <mailto:notmuch@notmuchmail.org>\r
  66 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  67 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  68         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  69 X-List-Received-Date: Fri, 04 Jul 2014 09:25:45 -0000\r
  70 \r
  71 Karl Fogel <kfogel@red-bean.com> writes:\r
  72 > Sebastian Lipp <bacuh@riseup.net> writes:\r
  73 >>As I'd like to see this in notmuch I made the change. The patch is\r
  74 >>attached. As it is my first contribution to notmuch at all: Just tell me\r
  75 >>if I'm supposed to do it in any other way.\r
  76 >\r
  77 > I think your patch includes much more than just the above, though.\r
  78 \r
  79 True. See below on that.\r
  80 \r
  81 >>diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el\r
  82 >>index fa65cd5..ee7b169 100644\r
  83 >>--- a/emacs/notmuch-address.el\r
  84 >>+++ b/emacs/notmuch-address.el\r
  85 >>@@ -113,6 +113,59 @@ to know how address selection is made by default."\r
  86 >> (when (notmuch-address-locate-command notmuch-address-command)\r
  87 >>   (notmuch-address-message-insinuate))\r
  88 >> \r
  89 >>+;; functions to add sender / recipients to BBDB\r
  90 >>+\r
  91 >>+(defvar bbdb-get-addresses-headers)\r
  92 >\r
  93 > I think it's good to include an initial value (even an invalid\r
  94 > placeholder one, if the real initialization has not happened yet), and a\r
  95 > doc string.  C-h f defvar RET will say more about how to do that.\r
  96 \r
  97 I will read into this and get back to it.\r
  98 \r
  99 >>+(declare-function notmuch-show-get-header "notmuch-show" (header &optional props))\r
 100 >>+\r
 101 >>+(declare-function bbdb-get-addresses "bbdb-com" \r
 102 >>+  (only-first-address\r
 103 >>+   uninteresting-senders\r
 104 >>+   get-header-content-function\r
 105 >>+   &rest get-header-content-function-args))\r
 106 >>+\r
 107 >>+(declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p offer-to-create))\r
 108 >\r
 109 > At this point, your patch has accomplished what Tomi originally\r
 110 > suggested.  But then the patch continues with what looks like\r
 111 > substantive new code:\r
 112 >\r
 113 >>+(defun notmuch-bbdb/snarf-headers (headers)\r
 114 >>+  ;; Helper function to avoid code duplication in the two below\r
 115 >>+  ;; headers should have the same format as bbdb-get-addresses-headers\r
 116 >>+\r
 117 >>+  ;; bbdb-get-addresses reads these\r
 118 >>+  ;; Ugh, pass-by-global\r
 119 >>+  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))\r
 120 >>+     (bbdb-get-addresses-headers headers) ; headers to read\r
 121 >>+     (bbdb-gag-messages t)) ; suppress m/n processed message)\r
 122 >>+    (bbdb-update-records addrs t t)))\r
 123 >>+\r
 124 >>+(defun notmuch-bbdb/snarf-from ()\r
 125 >>+  "Import the sender of the current message into BBDB"\r
 126 >>+  (interactive)\r
 127 >>+  (notmuch-bbdb/snarf-headers\r
 128 >>+   (list (assoc 'authors bbdb-get-addresses-headers))))\r
 129 >>+\r
 130 >>+(defun notmuch-bbdb/snarf-to ()\r
 131 >>+  "Import all recipients of the current message into BBDB"\r
 132 >>+  (interactive)\r
 133 >>+  (notmuch-bbdb/snarf-headers\r
 134 >>+   (list (assoc 'recipients bbdb-get-addresses-headers))))\r
 135 >>+\r
 136 >>+(defvar notmuch-bbdb/header-by-name\r
 137 >>+  ;; both are case sensitive\r
 138 >>+  '( ("From" . :From)\r
 139 >>+     ("To" . :To)\r
 140 >>+     ("CC" . :Cc)\r
 141 >>+     ("BCC" . :Bcc)\r
 142 >>+     ("Resent-From" . nil)\r
 143 >>+     ("Reply-To" . nil)\r
 144 >>+     ("Resent-To" . nil)\r
 145 >>+     ("Resent-CC" . nil))\r
 146 >>+  "Alist for dispatching header symbols as used by notmuch-show-get-header\r
 147 >>+from strings as used by bbdb-get-addresses")\r
 148 >>+\r
 149 >>+(defun notmuch-bbdb/get-header-content (name)\r
 150 >>+  (notmuch-show-get-header (cdr (assoc name notmuch-bbdb/header-by-name))))\r
 151 >\r
 152 > It looks like new code, but I suspect what actually happened is that\r
 153 > this is just the original code, somehow mis-expressed as "+" lines in\r
 154 > the patch.  Is that what happened?\r
 155 \r
 156 No, it happened by intention because what is "original" to you is not\r
 157 part of my freshly cloned notmuch. So I thought good practise is to\r
 158 construct *one* patch that brings the already fixed feature to the\r
 159 notmuch codebase to keep it clean.\r
 160 \r
 161 If you like better, my next patch will only base Tomis and your\r
 162 suggestions on top of the "original" patch.\r
 163 \r
 164 basti\r