b2/55f6768f769bf1d08ac12e795e9079314d9075

   1 Return-Path: <amdragon@mit.edu>\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 C004D429E26\r
   6         for <notmuch@notmuchmail.org>; Thu, 12 Jan 2012 13:59:06 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id sJgCh8RfuvtR for <notmuch@notmuchmail.org>;\r
  16         Thu, 12 Jan 2012 13:59:06 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-5.mit.edu (DMZ-MAILSEC-SCANNER-5.MIT.EDU\r
  18         [18.7.68.34])\r
  19         by olra.theworths.org (Postfix) with ESMTP id E0B69431FB6\r
  20         for <notmuch@notmuchmail.org>; Thu, 12 Jan 2012 13:59:05 -0800 (PST)\r
  21 X-AuditID: 12074422-b7fd66d0000008f9-2d-4f0f57a9ebbc\r
  22 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  23         by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 8E.2D.02297.9A75F0F4; Thu, 12 Jan 2012 16:59:05 -0500 (EST)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id q0CLx4jD026441; \r
  27         Thu, 12 Jan 2012 16:59:04 -0500\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q0CLx2Ih022355\r
  32         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
  33         Thu, 12 Jan 2012 16:59:03 -0500 (EST)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
  35         (envelope-from <amdragon@mit.edu>)\r
  36         id 1RlSfp-0006X8-PV; Thu, 12 Jan 2012 16:59:05 -0500\r
  37 Date: Thu, 12 Jan 2012 16:59:05 -0500\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: Jani Nikula <jani@nikula.org>\r
  40 Subject: Re: [PATCH v4 1/5] cli: slightly refactor "notmuch reply" address\r
  41         scanning functions\r
  42 Message-ID: <20120112215905.GF18625@mit.edu>\r
  43 References: <cover.1325794371.git.jani@nikula.org>\r
  44         <cover.1326403905.git.jani@nikula.org>\r
  45         <9935c31d8727331b442ce266ae22469243b85f36.1326403905.git.jani@nikula.org>\r
  46 MIME-Version: 1.0\r
  47 Content-Type: text/plain; charset=us-ascii\r
  48 Content-Disposition: inline\r
  49 In-Reply-To:\r
  50  <9935c31d8727331b442ce266ae22469243b85f36.1326403905.git.jani@nikula.org>\r
  51 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  52 X-Brightmail-Tracker:\r
  53  H4sIAAAAAAAAA+NgFuplleLIzCtJLcpLzFFi42IR4hTV1l0Zzu9v8OKZhEXTdGeL1XN5LK7f\r
  54         nMnswOyxc9Zddo9b91+zezxbdYs5gDmKyyYlNSezLLVI3y6BK+PeiUb2gpt6FY+v3WVsYLyi\r
  55         3MXIySEhYCKx9PYtRghbTOLCvfVsXYxcHEIC+xglFv5ewwjhbGCU6Niwkh3COckk0dO8lBXC\r
  56         WcIocW3KeWaQfhYBVYndvy8xgdhsAhoS2/YvB5srIqAosfnkfjCbWcBMYuXU72A1wgLxEi3f\r
  57         DoHFeQV0JO5/7IEaupxR4snZ7awQCUGJkzOfsEA0a0nc+PcSqJkDyJaWWP6PAyTMKRAmsXL9\r
  58         AXYQW1RARWLKyW1sExiFZiHpnoWkexZC9wJG5lWMsim5Vbq5iZk5xanJusXJiXl5qUW6pnq5\r
  59         mSV6qSmlmxhBoc7uorSD8edBpUOMAhyMSjy8L4X5/YVYE8uKK3MPMUpyMCmJ8ooCI0WILyk/\r
  60         pTIjsTgjvqg0J7X4EKMEB7OSCG+MLlCONyWxsiq1KB8mJc3BoiTOq671zk9IID2xJDU7NbUg\r
  61         tQgmK8PBoSTBuyAMqFGwKDU9tSItM6cEIc3EwQkynAdo+E2QGt7igsTc4sx0iPwpRkUpcd6X\r
  62         IAkBkERGaR5cLywVvWIUB3pFmPcGSBUPMI3Bdb8CGswENLgshQ9kcEkiQkqqgdEl8x+/XuSh\r
  63         DYfPti7hzij/0mienrlst+xhk+Uhb9csNNi6qv7ra+8uzVK5dv4MB+fPp8pSbnIr6ngvXe9t\r
  64         eUK2eFqX3qEXt7nK/e6+tOjvlRP/q528wOzOI+MQHTXOCcYrbMItqg+UBn1umWXk+OzLeyap\r
  65         Pc6BKTJu2wUEfm2ev6Pu8sOueUosxRmJhlrMRcWJAALDZKEgAwAA\r
  66 Cc: notmuch@notmuchmail.org\r
  67 X-BeenThere: notmuch@notmuchmail.org\r
  68 X-Mailman-Version: 2.1.13\r
  69 Precedence: list\r
  70 List-Id: "Use and development of the notmuch mail system."\r
  71         <notmuch.notmuchmail.org>\r
  72 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  73         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  74 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  75 List-Post: <mailto:notmuch@notmuchmail.org>\r
  76 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  77 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  78         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  79 X-List-Received-Date: Thu, 12 Jan 2012 21:59:06 -0000\r
  80 \r
  81 LGTM.  One thing you could fix below (and a few comments), but not\r
  82 enough alone to warrant a new version.\r
  83 \r
  84 Quoth Jani Nikula on Jan 12 at 11:40 pm:\r
  85 > Slightly refactor "notmuch reply" recipient and user from address scanning\r
  86 > functions in preparation for reply-to-sender feature.\r
  87 > \r
  88 > Add support for not adding messages at all (just scan for user from\r
  89 > address), and returning the number of messages added.\r
  90 > \r
  91 > No externally visible functional changes.\r
  92 > \r
  93 > Signed-off-by: Jani Nikula <jani@nikula.org>\r
  94 > ---\r
  95 >  notmuch-reply.c |   74 ++++++++++++++++++++++++++++--------------------------\r
  96 >  1 files changed, 38 insertions(+), 36 deletions(-)\r
  97 > \r
  98 > diff --git a/notmuch-reply.c b/notmuch-reply.c\r
  99 > index 000f6da..4fae66f 100644\r
 100 > --- a/notmuch-reply.c\r
 101 > +++ b/notmuch-reply.c\r
 102 > @@ -168,22 +168,28 @@ address_is_users (const char *address, notmuch_config_t *config)\r
 103 >      return 0;\r
 104 >  }\r
 105 >  \r
 106 > -/* For each address in 'list' that is not configured as one of the\r
 107 > - * user's addresses in 'config', add that address to 'message' as an\r
 108 > - * address of 'type'.\r
 109 > +/* Scan addresses in 'list'.\r
 110 >   *\r
 111 > - * The first address encountered that *is* the user's address will be\r
 112 > - * returned, (otherwise NULL is returned).\r
 113 > + * If 'message' is non-NULL, then for each address in 'list' that is not\r
 114 > + * configured as one of the user's addresses in 'config', add that address to\r
 115 > + * 'message' as an address of 'type'.\r
 116 > + *\r
 117 > + * If 'user_from' is non-NULL and *user_from is NULL, the first address\r
 118 > + * encountered in 'list' that *is* the user's address will be set to *user_from.\r
 119 > + *\r
 120 > + * Return the number of addresses added to 'message'. (If 'message' is NULL, the\r
 121 > + * function returns 0 by definition.)\r
 122 \r
 123 Ah, I like the return value.  Better than adding an umpteenth argument\r
 124 like I was suggesting.\r
 125 \r
 126 >   */\r
 127 > -static const char *\r
 128 > -add_recipients_for_address_list (GMimeMessage *message,\r
 129 > -                              notmuch_config_t *config,\r
 130 > -                              GMimeRecipientType type,\r
 131 > -                              InternetAddressList *list)\r
 132 > +static unsigned int\r
 133 > +scan_address_list (InternetAddressList *list,\r
 134 > +                notmuch_config_t *config,\r
 135 > +                GMimeMessage *message,\r
 136 > +                GMimeRecipientType type,\r
 137 > +                const char **user_from)\r
 138 >  {\r
 139 >      InternetAddress *address;\r
 140 >      int i;\r
 141 > -    const char *ret = NULL;\r
 142 > +    unsigned int n = 0;\r
 143 >  \r
 144 >      for (i = 0; i < internet_address_list_length (list); i++) {\r
 145 >       address = internet_address_list_get_address (list, i);\r
 146 > @@ -196,8 +202,7 @@ add_recipients_for_address_list (GMimeMessage *message,\r
 147 >           if (group_list == NULL)\r
 148 >               continue;\r
 149 >  \r
 150 > -         add_recipients_for_address_list (message, config,\r
 151 > -                                          type, group_list);\r
 152 > +         n += scan_address_list (group_list, config, message, type, NULL);\r
 153 \r
 154 Should the NULL above be user_from?  You're being compatible with the\r
 155 original code, which is the right thing to do in this patch, but the\r
 156 new-found explicitness made me wonder if this is actually a bug.\r
 157 \r
 158 >       } else {\r
 159 >           InternetAddressMailbox *mailbox;\r
 160 >           const char *name;\r
 161 > @@ -209,40 +214,40 @@ add_recipients_for_address_list (GMimeMessage *message,\r
 162 >           addr = internet_address_mailbox_get_addr (mailbox);\r
 163 >  \r
 164 >           if (address_is_users (addr, config)) {\r
 165 > -             if (ret == NULL)\r
 166 > -                 ret = addr;\r
 167 > -         } else {\r
 168 > +             if (user_from && *user_from == NULL)\r
 169 > +                 *user_from = addr;\r
 170 > +         } else if (message) {\r
 171 >               g_mime_message_add_recipient (message, type, name, addr);\r
 172 > +             n++;\r
 173 >           }\r
 174 >       }\r
 175 >      }\r
 176 >  \r
 177 > -    return ret;\r
 178 > +    return n;\r
 179 >  }\r
 180 >  \r
 181 > -/* For each address in 'recipients' that is not configured as one of\r
 182 > - * the user's addresses in 'config', add that address to 'message' as\r
 183 > - * an address of 'type'.\r
 184 > +/* Scan addresses in 'recipients'.\r
 185 >   *\r
 186 > - * The first address encountered that *is* the user's address will be\r
 187 > - * returned, (otherwise NULL is returned).\r
 188 > + * See the documentation of scan_address_list() above. This function does\r
 189 > + * exactly the same, but converts 'recipients' to an InternetAddressList first.\r
 190 \r
 191 Just bikeshedding, but comments in the notmuch codebase almost\r
 192 universally wrap at 72 columns, not 80 (based on \r
 193 egrep '[ /]\* .{70}' *.[ch]*)\r
 194 \r
 195 >   */\r
 196 > -static const char *\r
 197 > -add_recipients_for_string (GMimeMessage *message,\r
 198 > -                        notmuch_config_t *config,\r
 199 > -                        GMimeRecipientType type,\r
 200 > -                        const char *recipients)\r
 201 > +static unsigned int\r
 202 > +scan_address_string (const char *recipients,\r
 203 > +                  notmuch_config_t *config,\r
 204 > +                  GMimeMessage *message,\r
 205 > +                  GMimeRecipientType type,\r
 206 > +                  const char **user_from)\r
 207 >  {\r
 208 >      InternetAddressList *list;\r
 209 >  \r
 210 >      if (recipients == NULL)\r
 211 > -     return NULL;\r
 212 > +     return 0;\r
 213 >  \r
 214 >      list = internet_address_list_parse_string (recipients);\r
 215 >      if (list == NULL)\r
 216 > -     return NULL;\r
 217 > +     return 0;\r
 218 >  \r
 219 > -    return add_recipients_for_address_list (message, config, type, list);\r
 220 > +    return scan_address_list (list, config, message, type, user_from);\r
 221 >  }\r
 222 >  \r
 223 >  /* Does the address in the Reply-To header of 'message' already appear\r
 224 > @@ -324,7 +329,7 @@ add_recipients_from_message (GMimeMessage *reply,\r
 225 >      }\r
 226 >  \r
 227 >      for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {\r
 228 > -     const char *addr, *recipients;\r
 229 > +     const char *recipients;\r
 230 >  \r
 231 >       recipients = notmuch_message_get_header (message,\r
 232 >                                                reply_to_map[i].header);\r
 233 > @@ -332,11 +337,8 @@ add_recipients_from_message (GMimeMessage *reply,\r
 234 >           recipients = notmuch_message_get_header (message,\r
 235 >                                                    reply_to_map[i].fallback);\r
 236 >  \r
 237 > -     addr = add_recipients_for_string (reply, config,\r
 238 > -                                       reply_to_map[i].recipient_type,\r
 239 > -                                       recipients);\r
 240 > -     if (from_addr == NULL)\r
 241 > -         from_addr = addr;\r
 242 > +     scan_address_string (recipients, config, reply,\r
 243 > +                          reply_to_map[i].recipient_type, &from_addr);\r
 244 >      }\r
 245 >  \r
 246 >      return from_addr;\r