1 Return-Path: <m.walters@qmul.ac.uk>
\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 56372431FB6
\r
6 for <notmuch@notmuchmail.org>; Sun, 2 Sep 2012 00:52:25 -0700 (PDT)
\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
\r
11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5
\r
12 tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,
\r
13 NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] 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 SPCrILXDRfq2 for <notmuch@notmuchmail.org>;
\r
17 Sun, 2 Sep 2012 00:52:24 -0700 (PDT)
\r
18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])
\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 DA000431FAF
\r
22 for <notmuch@notmuchmail.org>; Sun, 2 Sep 2012 00:52:23 -0700 (PDT)
\r
23 Received: from smtp.qmul.ac.uk ([138.37.6.40])
\r
24 by mail2.qmul.ac.uk with esmtp (Exim 4.71)
\r
25 (envelope-from <m.walters@qmul.ac.uk>)
\r
26 id 1T84yf-0002nP-7D; Sun, 02 Sep 2012 08:52:19 +0100
\r
27 Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost)
\r
28 by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)
\r
29 (envelope-from <m.walters@qmul.ac.uk>)
\r
30 id 1T84ye-0006fi-IO; Sun, 02 Sep 2012 08:52:17 +0100
\r
31 From: Mark Walters <markwalters1009@gmail.com>
\r
32 To: Jameson Graef Rollins <jrollins@finestructure.net>,
\r
33 Notmuch Mail <notmuch@notmuchmail.org>
\r
34 Subject: Re: [PATCH 06/11] lib: store thread recipients in thread structure
\r
35 In-Reply-To: <1345427570-26518-7-git-send-email-jrollins@finestructure.net>
\r
36 References: <1345427570-26518-1-git-send-email-jrollins@finestructure.net>
\r
37 <1345427570-26518-2-git-send-email-jrollins@finestructure.net>
\r
38 <1345427570-26518-3-git-send-email-jrollins@finestructure.net>
\r
39 <1345427570-26518-4-git-send-email-jrollins@finestructure.net>
\r
40 <1345427570-26518-5-git-send-email-jrollins@finestructure.net>
\r
41 <1345427570-26518-6-git-send-email-jrollins@finestructure.net>
\r
42 <1345427570-26518-7-git-send-email-jrollins@finestructure.net>
\r
43 User-Agent: Notmuch/0.14+28~ge689a44 (http://notmuchmail.org) Emacs/23.4.1
\r
44 (x86_64-pc-linux-gnu)
\r
45 Date: Sun, 02 Sep 2012 08:52:15 +0100
\r
46 Message-ID: <87ligsx3o0.fsf@qmul.ac.uk>
\r
48 Content-Type: text/plain; charset=us-ascii
\r
49 X-Sender-Host-Address: 93.97.24.31
\r
50 X-QM-SPAM-Info: Sender has good ham record. :)
\r
51 X-QM-Body-MD5: d7b69fec47cff9dcbc3c3c3e010f9da1 (of first 20000 bytes)
\r
52 X-SpamAssassin-Score: -2.3
\r
53 X-SpamAssassin-SpamBar: --
\r
54 X-SpamAssassin-Report: The QM spam filters have analysed this message to
\r
56 spam. We require at least 5.0 points to mark a message as spam.
\r
57 This message scored -2.3 points.
\r
58 Summary of the scoring:
\r
59 * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,
\r
61 * [138.37.6.40 listed in list.dnswl.org]
\r
62 * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail
\r
63 provider * (markwalters1009[at]gmail.com)
\r
64 X-QM-Scan-Virus: ClamAV says the message is clean
\r
65 X-BeenThere: notmuch@notmuchmail.org
\r
66 X-Mailman-Version: 2.1.13
\r
68 List-Id: "Use and development of the notmuch mail system."
\r
69 <notmuch.notmuchmail.org>
\r
70 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
\r
71 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
72 List-Archive: <http://notmuchmail.org/pipermail/notmuch>
\r
73 List-Post: <mailto:notmuch@notmuchmail.org>
\r
74 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
75 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
\r
76 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
77 X-List-Received-Date: Sun, 02 Sep 2012 07:52:25 -0000
\r
80 On Mon, 20 Aug 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
\r
81 > This utilizes the new thread addresses struct to store thread
\r
82 > recipients, again in parallel to authors.
\r
84 > Since message recipients are not stored in the database, including
\r
85 > recipients in the thread structure exacts a significant overhead as
\r
86 > the recipients are retrieved from the original message files. Because
\r
87 > of this, a new boolean argument, include_recipients, is added to the
\r
88 > necessary functions (_notmuch_thread_create, _thread_add_message and
\r
89 > _thread_add_matched_message) that controls whether the recipients are
\r
90 > fetched and included. If message recipients are ever stored in the
\r
91 > database this new argument could probably be removed.
\r
96 I have briefly looked through the series and overall it looks good.
\r
98 > lib/notmuch-private.h | 3 +-
\r
99 > lib/notmuch.h | 14 +++++++++
\r
100 > lib/query.cc | 3 +-
\r
101 > lib/thread.cc | 77 +++++++++++++++++++++++++++++++++++++------------
\r
102 > 4 files changed, 76 insertions(+), 21 deletions(-)
\r
104 > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
\r
105 > index 27a41b6..32d1523 100644
\r
106 > --- a/lib/notmuch-private.h
\r
107 > +++ b/lib/notmuch-private.h
\r
108 > @@ -232,7 +232,8 @@ _notmuch_thread_create (void *ctx,
\r
109 > unsigned int seed_doc_id,
\r
110 > notmuch_doc_id_set_t *match_set,
\r
111 > notmuch_string_list_t *excluded_terms,
\r
112 > - notmuch_sort_t sort);
\r
113 > + notmuch_sort_t sort,
\r
114 > + notmuch_bool_t include_recipients);
\r
118 > diff --git a/lib/notmuch.h b/lib/notmuch.h
\r
119 > index 6acd38d..f9e71c1 100644
\r
120 > --- a/lib/notmuch.h
\r
121 > +++ b/lib/notmuch.h
\r
122 > @@ -759,6 +759,20 @@ notmuch_thread_get_matched_messages (notmuch_thread_t *thread);
\r
124 > notmuch_thread_get_authors (notmuch_thread_t *thread);
\r
126 > +/* Get the recipients of 'thread'
\r
128 > + * The returned string is a comma-separated list of the names of the
\r
129 > + * recipients of mail messages in the query results that belong to this
\r
132 > + * The returned string belongs to 'thread' and as such, should not be
\r
133 > + * modified by the caller and will only be valid for as long as the
\r
134 > + * thread is valid, (which is until notmuch_thread_destroy or until
\r
135 > + * the query from which it derived is destroyed).
\r
138 > +notmuch_thread_get_recipients (notmuch_thread_t *thread);
\r
140 > /* Get the subject of 'thread'
\r
142 > * The subject is taken from the first message (according to the query
\r
143 > diff --git a/lib/query.cc b/lib/query.cc
\r
144 > index e9c1a2d..54833a7 100644
\r
145 > --- a/lib/query.cc
\r
146 > +++ b/lib/query.cc
\r
147 > @@ -486,7 +486,8 @@ notmuch_threads_get (notmuch_threads_t *threads)
\r
149 > &threads->match_set,
\r
150 > threads->query->exclude_terms,
\r
151 > - threads->query->sort);
\r
152 > + threads->query->sort,
\r
157 > diff --git a/lib/thread.cc b/lib/thread.cc
\r
158 > index 757e143..baf07c2 100644
\r
159 > --- a/lib/thread.cc
\r
160 > +++ b/lib/thread.cc
\r
161 > @@ -37,6 +37,7 @@ struct visible _notmuch_thread {
\r
164 > notmuch_thread_addresses_t *authors;
\r
165 > + notmuch_thread_addresses_t *recipients;
\r
166 > GHashTable *tags;
\r
168 > notmuch_message_list_t *message_list;
\r
169 > @@ -63,6 +64,7 @@ static int
\r
170 > _notmuch_thread_destructor (notmuch_thread_t *thread)
\r
172 > _notmuch_thread_addresses_destructor (thread->authors);
\r
173 > + _notmuch_thread_addresses_destructor (thread->recipients);
\r
174 > g_hash_table_unref (thread->tags);
\r
175 > g_hash_table_unref (thread->message_hash);
\r
177 > @@ -204,14 +206,17 @@ _thread_cleanup_address (notmuch_thread_t *thread,
\r
179 > _thread_add_message (notmuch_thread_t *thread,
\r
180 > notmuch_message_t *message,
\r
181 > - notmuch_string_list_t *exclude_terms)
\r
182 > + notmuch_string_list_t *exclude_terms,
\r
183 > + notmuch_bool_t include_recipients)
\r
185 > notmuch_tags_t *tags;
\r
187 > - InternetAddressList *list = NULL;
\r
188 > + InternetAddressList *from_list = NULL;
\r
189 > + InternetAddressList *to_list = NULL;
\r
190 > InternetAddress *address;
\r
191 > const char *from, *author;
\r
192 > - char *clean_author;
\r
193 > + const char *to, *recipient;
\r
194 > + char *clean_address;
\r
196 > _notmuch_message_list_add_message (thread->message_list,
\r
197 > talloc_steal (thread, message));
\r
198 > @@ -223,10 +228,9 @@ _thread_add_message (notmuch_thread_t *thread,
\r
200 > from = notmuch_message_get_header (message, "from");
\r
202 > - list = internet_address_list_parse_string (from);
\r
205 > - address = internet_address_list_get_address (list, 0);
\r
206 > + from_list = internet_address_list_parse_string (from);
\r
207 > + if (from_list) {
\r
208 > + address = internet_address_list_get_address (from_list, 0);
\r
210 > author = internet_address_get_name (address);
\r
211 > if (author == NULL) {
\r
212 > @@ -234,11 +238,32 @@ _thread_add_message (notmuch_thread_t *thread,
\r
213 > mailbox = INTERNET_ADDRESS_MAILBOX (address);
\r
214 > author = internet_address_mailbox_get_addr (mailbox);
\r
216 > - clean_author = _thread_cleanup_author (thread, author, from);
\r
217 > - _thread_add_address (thread->authors, clean_author, FALSE);
\r
218 > - notmuch_message_set_author (message, clean_author);
\r
219 > + clean_address = _thread_cleanup_address (thread, author, from);
\r
220 > + _thread_add_address (thread->authors, clean_address, FALSE);
\r
221 > + notmuch_message_set_author (message, clean_address);
\r
223 > + g_object_unref (G_OBJECT (from_list));
\r
226 > + if (include_recipients) {
\r
227 > + to = notmuch_message_get_header (message, "to");
\r
229 > + to_list = internet_address_list_parse_string (to);
\r
231 > + address = internet_address_list_get_address (to_list, 0);
\r
233 I think you are just adding the first address on the to: line. Is that
\r
234 deliberate? The comment before notmuch_thread_get_recipients (above)
\r
235 suggests all recipients are being added, but doing so may make the
\r
236 string too large to be useful.
\r
245 > + recipient = internet_address_get_name (address);
\r
246 > + if (recipient == NULL) {
\r
247 > + InternetAddressMailbox *mailbox;
\r
248 > + mailbox = INTERNET_ADDRESS_MAILBOX (address);
\r
249 > + recipient = internet_address_mailbox_get_addr (mailbox);
\r
251 > + clean_address = _thread_cleanup_address (thread, recipient, to);
\r
252 > + _thread_add_address (thread->recipients, clean_address, FALSE);
\r
253 > + notmuch_message_set_recipients (message, clean_address);
\r
255 > - g_object_unref (G_OBJECT (list));
\r
256 > + g_object_unref (G_OBJECT (to_list));
\r
260 > if (! thread->subject) {
\r
261 > @@ -301,7 +326,8 @@ _thread_set_subject_from_message (notmuch_thread_t *thread,
\r
263 > _thread_add_matched_message (notmuch_thread_t *thread,
\r
264 > notmuch_message_t *message,
\r
265 > - notmuch_sort_t sort)
\r
266 > + notmuch_sort_t sort,
\r
267 > + notmuch_bool_t include_recipients)
\r
270 > notmuch_message_t *hashed_message;
\r
271 > @@ -331,6 +357,8 @@ _thread_add_matched_message (notmuch_thread_t *thread,
\r
274 > _thread_add_address (thread->authors, notmuch_message_get_author (hashed_message), TRUE);
\r
275 > + if (include_recipients)
\r
276 > + _thread_add_address (thread->recipients, notmuch_message_get_recipients (hashed_message), TRUE);
\r
280 > @@ -399,10 +427,10 @@ _thread_addresses_init (const void *ctx)
\r
282 > * Creating the thread will perform a database search to get all
\r
283 > * messages belonging to the thread and will get the first subject
\r
284 > - * line, the total count of messages, and all authors in the thread.
\r
285 > - * Each message in the thread is checked against match_set to allow
\r
286 > - * for a separate count of matched messages, and to allow a viewer to
\r
287 > - * display these messages differently.
\r
288 > + * line, the total count of messages, and all authors and recipients
\r
289 > + * of the thread. Each message in the thread is checked against
\r
290 > + * match_set to allow for a separate count of matched messages, and to
\r
291 > + * allow a viewer to display these messages differently.
\r
293 > * Here, 'ctx' is talloc context for the resulting thread object.
\r
295 > @@ -414,7 +442,8 @@ _notmuch_thread_create (void *ctx,
\r
296 > unsigned int seed_doc_id,
\r
297 > notmuch_doc_id_set_t *match_set,
\r
298 > notmuch_string_list_t *exclude_terms,
\r
299 > - notmuch_sort_t sort)
\r
300 > + notmuch_sort_t sort,
\r
301 > + notmuch_bool_t include_recipients)
\r
303 > notmuch_thread_t *thread;
\r
304 > notmuch_message_t *seed_message;
\r
305 > @@ -453,6 +482,9 @@ _notmuch_thread_create (void *ctx,
\r
306 > thread->authors = _thread_addresses_init (thread);
\r
307 > if (unlikely (thread->authors == NULL))
\r
309 > + thread->recipients = _thread_addresses_init (thread);
\r
310 > + if (unlikely (thread->recipients == NULL))
\r
313 > thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,
\r
315 > @@ -486,11 +518,11 @@ _notmuch_thread_create (void *ctx,
\r
316 > if (doc_id == seed_doc_id)
\r
317 > message = seed_message;
\r
319 > - _thread_add_message (thread, message, exclude_terms);
\r
320 > + _thread_add_message (thread, message, exclude_terms, include_recipients);
\r
322 > if ( _notmuch_doc_id_set_contains (match_set, doc_id)) {
\r
323 > _notmuch_doc_id_set_remove (match_set, doc_id);
\r
324 > - _thread_add_matched_message (thread, message, sort);
\r
325 > + _thread_add_matched_message (thread, message, sort, include_recipients);
\r
328 > _notmuch_message_close (message);
\r
329 > @@ -499,6 +531,7 @@ _notmuch_thread_create (void *ctx,
\r
330 > notmuch_query_destroy (thread_id_query);
\r
332 > _resolve_thread_addresses_string (thread->authors);
\r
333 > + _resolve_thread_addresses_string (thread->recipients);
\r
335 > _resolve_thread_relationships (thread);
\r
337 > @@ -536,6 +569,12 @@ notmuch_thread_get_authors (notmuch_thread_t *thread)
\r
341 > +notmuch_thread_get_recipients (notmuch_thread_t *thread)
\r
343 > + return thread->recipients->string;
\r
347 > notmuch_thread_get_subject (notmuch_thread_t *thread)
\r
349 > return thread->subject;
\r
353 > _______________________________________________
\r
354 > notmuch mailing list
\r
355 > notmuch@notmuchmail.org
\r
356 > http://notmuchmail.org/mailman/listinfo/notmuch
\r