f1/ede4bafcaf7b27284cfcf8bb3f5e8de6aed2cd

   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 11380431FD7\r
   6         for <notmuch@notmuchmail.org>; Tue, 12 Nov 2013 11:31:42 -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 jWYG7YjnaAk7 for <notmuch@notmuchmail.org>;\r
  16         Tue, 12 Nov 2013 11:31:32 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-1.mit.edu (dmz-mailsec-scanner-1.mit.edu\r
  18         [18.9.25.12])\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 BFAFE431FD2\r
  22         for <notmuch@notmuchmail.org>; Tue, 12 Nov 2013 11:31:31 -0800 (PST)\r
  23 X-AuditID: 1209190c-b7f7f6d000000bbd-d5-5282821241d3\r
  24 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
  25         (using TLS with cipher AES256-SHA (256/256 bits))\r
  26         (Client did not present a certificate)\r
  27         by dmz-mailsec-scanner-1.mit.edu (Symantec Messaging Gateway) with SMTP\r
  28         id 6A.27.03005.21282825; Tue, 12 Nov 2013 14:31:30 -0500 (EST)\r
  29 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  30         by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id rACJVS2o009792; \r
  31         Tue, 12 Nov 2013 14:31:29 -0500\r
  32 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  33         (authenticated bits=0)\r
  34         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  35         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id rACJVP2a000312\r
  36         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  37         Tue, 12 Nov 2013 14:31:27 -0500\r
  38 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  39         (envelope-from <amdragon@mit.edu>)\r
  40         id 1VgJgL-0005wV-Ap; Tue, 12 Nov 2013 14:31:25 -0500\r
  41 From: Austin Clements <aclements@csail.mit.edu>\r
  42 To: Peter Zijlstra <peterz@infradead.org>, notmuch@notmuchmail.org\r
  43 Subject: Re: [PATCH] notmuch: Add "maildir:" search option\r
  44 In-Reply-To: <20131112155637.GA16796@laptop.programming.kicks-ass.net>\r
  45 References: <20131112155637.GA16796@laptop.programming.kicks-ass.net>\r
  46 User-Agent: Notmuch/0.16+154~g96c0ce2 (http://notmuchmail.org) Emacs/23.4.1\r
  47         (i486-pc-linux-gnu)\r
  48 Date: Tue, 12 Nov 2013 14:31:25 -0500\r
  49 Message-ID: <87mwl94dte.fsf@awakening.csail.mit.edu>\r
  50 MIME-Version: 1.0\r
  51 Content-Type: text/plain; charset=us-ascii\r
  52 X-Brightmail-Tracker:\r
  53  H4sIAAAAAAAAA+NgFrrPIsWRmVeSWpSXmKPExsUixG6nrivU1BRksPWmhcX1mzOZLY73HmBy\r
  54         YPLYvELL49mqW8wBTFFcNimpOZllqUX6dglcGXv7P7MXbLCo2DZnAVMD43vtLkZODgkBE4m2\r
  55         l1dZIWwxiQv31rN1MXJxCAnMZpJoWdfPBOFsZJT48eovI4RzmkmiZ9kMJpAWIYEljBItF3xB\r
  56         bDYBfYkVayeBjRIRcJK4+2kqkM3BISxgJbF/VxJImFPATeLLsZPsEK2uEtuX3mcBsUUF4iWm\r
  57         LdzJDGKzCKhKHH3RCzaeF+i6vSdPs0HYghInZz4Bq2cW0JK48e8l0wRGgVlIUrOQpBYwMq1i\r
  58         lE3JrdLNTczMKU5N1i1OTszLSy3SNdTLzSzRS00p3cQICkdOSZ4djG8OKh1iFOBgVOLhfRDZ\r
  59         FCTEmlhWXJl7iFGSg0lJlFe8ACjEl5SfUpmRWJwRX1Sak1p8iFGCg1lJhNctBijHm5JYWZVa\r
  60         lA+TkuZgURLnvclhHyQkkJ5YkpqdmlqQWgSTleHgUJLgPVkE1ChYlJqeWpGWmVOCkGbi4AQZ\r
  61         zgM0/BlIDW9xQWJucWY6RP4Uo6KUOO8lkIQASCKjNA+uF5YuXjGKA70izHsApIoHmGrgul8B\r
  62         DWYCGrzkbiPI4JJEhJRUA2N3Rt2Df5/uKX8PdZYQ5pLzWn22aLJyTX7oD47p7x5vn+12OfRP\r
  63         bYFs17Ho3rVdzze/tBJ7zbpkXYBdKPePnsjvQV7mR4ruzN22XOiq3g6hd82d8ns3mUsU7PC7\r
  64         ZZruGL1He8dUjS8OqT9/zDoh+bi6xPfSAl+pwp+a1uVmcu4ienE3+ZIXhiixFGckGmoxFxUn\r
  65         AgB957TZ8gIAAA==\r
  66 X-BeenThere: notmuch@notmuchmail.org\r
  67 X-Mailman-Version: 2.1.13\r
  68 Precedence: list\r
  69 List-Id: "Use and development of the notmuch mail system."\r
  70         <notmuch.notmuchmail.org>\r
  71 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  72         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  73 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  74 List-Post: <mailto:notmuch@notmuchmail.org>\r
  75 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  76 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  77         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  78 X-List-Received-Date: Tue, 12 Nov 2013 19:31:42 -0000\r
  79 \r
  80 I think this is a great idea.  Personally I think this is how folder:\r
  81 should work.  I find the semantics of folder: to be useless except where\r
  82 they happen to coincide with the boolean semantics used here.\r
  83 Unfortunately, changing folder: would require versioning the database,\r
  84 which we have only primordial support for right now.\r
  85 \r
  86 Various comments below, though nothing major.  Of course, we'd also need\r
  87 some tests and man page updates for this.\r
  88 \r
  89 On Tue, 12 Nov 2013, Peter Zijlstra <peterz@infradead.org> wrote:\r
  90 > Subject: notmuch: Add "maildir:" search option\r
  91 >\r
  92 > The current "folder:" search terms are near useless when you have\r
  93 > recursive folders, introduce a boolean "maildir:" search term to\r
  94 > exactly match the maildir folder.\r
  95 >\r
  96 > Given a Maildir++ layout like:\r
  97 >\r
  98 >       ~/Maildir/\r
  99 >       ~/Maildir/cur\r
 100 >       ~/Maildir/new\r
 101 >       ~/Maildir/tmp\r
 102 >       ~/Maildir/.Sent\r
 103 >       ~/Maildir/.Sent/cur\r
 104 >       ~/Maildir/.Sent/new\r
 105 >       ~/Maildir/.Sent/tmp\r
 106 >       ~/Maildir/.INBOX.LKML\r
 107 >       ~/Maildir/.INBOX.LKML/cur\r
 108 >       ~/Maildir/.INBOX.LKML/new\r
 109 >       ~/Maildir/.INBOX.LKML/tmp\r
 110 >       ~/Maildir/.INBOX.LKML.netdev\r
 111 >       ~/Maildir/.INBOX.LKML.netdev/cur\r
 112 >       ~/Maildir/.INBOX.LKML.netdev/new\r
 113 >       ~/Maildir/.INBOX.LKML.netdev/tmp\r
 114 >       ~/Maildir/.INBOX.LKML.arch\r
 115 >       ~/Maildir/.INBOX.LKML.arch/cur\r
 116 >       ~/Maildir/.INBOX.LKML.arch/new\r
 117 >       ~/Maildir/.INBOX.LKML.arch/tmp\r
 118 >\r
 119 > This patch generates the following search index:\r
 120 >\r
 121 >       $ delve -a Maildir/.notmuch/xapian/ | ~/s XMAILDIR\r
 122 >       XMAILDIR:INBOX\r
 123 >       XMAILDIR:INBOX/LKML\r
 124 >       XMAILDIR:INBOX/LKML/arch\r
 125 >       XMAILDIR:INBOX/LKML/netdev\r
 126 >       XMAILDIR:Sent\r
 127 >\r
 128 > Which allows one (me!!1) to pose queries like:\r
 129 >\r
 130 >       maildir:INBOX and not tag:list\r
 131 >\r
 132 > to more easily find offlist mail (from people like my family who don't\r
 133 > actually send their stuff over LKML :-).\r
 134 >\r
 135 > Signed-off-by: Peter Zijlstra <peterz@infradead.org>\r
 136 > ---\r
 137 >\r
 138 > XXX: now I need to go figure out how to do searches like:\r
 139 >\r
 140 >   subject:PATCH/0\r
 141 >\r
 142 > which would mandate that PATCH is the first word occurring in the\r
 143 > subject. I think the position index holds enough information but I\r
 144 > need to look into that and obviously the query parser needs work for\r
 145 > this.\r
 146 \r
 147 This is a frequently requested feature.  Unfortunately, there are two\r
 148 technical hurdles.  One is that the position information actually\r
 149 *isn't* enough as it is both because the subject will start at some\r
 150 arbitrary term offset that depends on the from and to (and maybe other\r
 151 things) and because the Xapian Query structure doesn't expose a way to\r
 152 search for a specific term offset (only relative offsets).  The other is\r
 153 that we currently use Xapian's query parser, which isn't extensible,\r
 154 though I took a stab at a custom query parser long ago and swear that\r
 155 one of these days I'll return to it.\r
 156 \r
 157 >\r
 158 >\r
 159 >  lib/database.cc |  7 ++++---\r
 160 >  lib/message.cc  | 40 ++++++++++++++++++++++++++++++++++++++++\r
 161 >  2 files changed, 44 insertions(+), 3 deletions(-)\r
 162 >\r
 163 > diff --git a/lib/database.cc b/lib/database.cc\r
 164 > index a021bf17253c..53aeaa68954d 100644\r
 165 > --- a/lib/database.cc\r
 166 > +++ b/lib/database.cc\r
 167 > @@ -208,15 +208,16 @@ static prefix_t BOOLEAN_PREFIX_EXTERNAL[] = {\r
 168 >      { "thread",                      "G" },\r
 169 >      { "tag",                 "K" },\r
 170 >      { "is",                  "K" },\r
 171 > -    { "id",                  "Q" }\r
 172 > +    { "id",                  "Q" },\r
 173 > +    { "maildir",             "XMAILDIR:" },\r
 174 \r
 175 No colon.  That is, the term prefix should just be "XMAILDIR".\r
 176 \r
 177 >  };\r
 178 >  \r
 179 >  static prefix_t PROBABILISTIC_PREFIX[]= {\r
 180 >      { "from",                        "XFROM" },\r
 181 >      { "to",                  "XTO" },\r
 182 >      { "attachment",          "XATTACHMENT" },\r
 183 > -    { "subject",             "XSUBJECT"},\r
 184 > -    { "folder",                      "XFOLDER"}\r
 185 > +    { "subject",             "XSUBJECT" },\r
 186 > +    { "folder",                      "XFOLDER" },\r
 187 \r
 188 Unintentional whitespace change?\r
 189 \r
 190 >  };\r
 191 >  \r
 192 >  const char *\r
 193 > diff --git a/lib/message.cc b/lib/message.cc\r
 194 > index 1b4637950f8e..45a727a6208f 100644\r
 195 > --- a/lib/message.cc\r
 196 > +++ b/lib/message.cc\r
 197 > @@ -22,6 +22,7 @@\r
 198 >  #include "database-private.h"\r
 199 >  \r
 200 >  #include <stdint.h>\r
 201 > +#include <string.h>\r
 202 >  \r
 203 >  #include <gmime/gmime.h>\r
 204 >  \r
 205 > @@ -485,6 +486,8 @@ _notmuch_message_add_filename (notmuch_message_t *message,\r
 206 >      notmuch_status_t status;\r
 207 >      void *local = talloc_new (message);\r
 208 >      char *direntry;\r
 209 > +    char *maildir;\r
 210 > +    int i;\r
 211 >  \r
 212 >      if (filename == NULL)\r
 213 >       INTERNAL_ERROR ("Message filename cannot be NULL.");\r
 214 > @@ -507,6 +510,43 @@ _notmuch_message_add_filename (notmuch_message_t *message,\r
 215 >      /* New terms allow user to search with folder: specification. */\r
 216 >      _notmuch_message_gen_terms (message, "folder", directory);\r
 217 >  \r
 218 > +    /* Convert the directory into a maildir path */\r
 219 > +    maildir = talloc_strdup(local, directory);\r
 220 \r
 221 Add a space before the "(".  (Same thing throughout the rest of the\r
 222 patch.)\r
 223 \r
 224 > +\r
 225 > +    /* Strip the maildir "cur", "new" directory entries. */\r
 226 > +    i = strlen(maildir);\r
 227 > +    if (strncmp(maildir + i - 3, "cur", 3) == 0 ||\r
 228 > +     strncmp(maildir + i - 3, "new", 3) == 0) {\r
 229 \r
 230 This is unsafe if directory is less than three characters, which I\r
 231 believe could happen if the message is in the root mail directory (which\r
 232 shouldn't happen with a well-formed maildir, but notmuch doesn't require\r
 233 maildir, and, regardless, we should be defensive).\r
 234 \r
 235 Also, we have a STRNCMP_LITERAL macro that we often use for comparisons\r
 236 with string literals, but I'm good with this, too.\r
 237 \r
 238 > +         maildir[i - 3] = '\0';\r
 239 > +         i -= 3;\r
 240 > +    }\r
 241 > +\r
 242 > +    /* Strip trailing '/' */\r
 243 > +    while (maildir[i-1] == '/') {\r
 244 \r
 245 This is also unsafe if maildir is the empty string.\r
 246 \r
 247 Also, add spaces around the "-" (likewise on the next line).\r
 248 \r
 249 > +         maildir[i-1] = '\0';\r
 250 > +         i--;\r
 251 > +    }\r
 252 > +\r
 253 > +    /* Strip leading '/' */\r
 254 > +    while (maildir[0] == '/')\r
 255 > +         maildir++;\r
 256 > +\r
 257 > +    /* Strip leading '.' */\r
 258 > +    while (maildir[0] == '.')\r
 259 \r
 260 Why strip multiple dots?  (I'm not saying you shouldn't, I'm just\r
 261 curious; and it may be worth explaining in the comment.)\r
 262 \r
 263 > +         maildir++;\r
 264 > +\r
 265 > +    /* Replace all remaining '.' with '/' */\r
 266 \r
 267 I think this should only happen if there was a leading '.', indicating a\r
 268 Maildir++ folder hierarchy.  A lot of people use the "file system"\r
 269 Maildir layout, which just consists of nested directories where the\r
 270 leaves are Maildirs (e.g., Dovecot's LAYOUT=fs,\r
 271 http://wiki2.dovecot.org/MailboxFormat/Maildir#Directory_Structure).  In\r
 272 this case, it's perfectly legitimate to have '.'s in folder names and it\r
 273 would be confusing and unexpected to translate them to '/'s.  This would\r
 274 also make this compatible with MH folders (which notmuch supports,\r
 275 though admittedly it's unclear if anyone actually uses them).\r
 276 \r
 277 > +    for (i = 0; maildir[i]; i++) {\r
 278 > +         if (maildir[i] == '.')\r
 279 > +                 maildir[i] = '/';\r
 280 > +    }\r
 281 > +\r
 282 > +    /* If there's no string left, we're the "INBOX" */\r
 283 > +    if (maildir[0] == '\0')\r
 284 > +         maildir = talloc_strdup(local, "INBOX");\r
 285 > +\r
 286 > +    _notmuch_message_add_term (message, "maildir", maildir);\r
 287 > +\r
 288 >      talloc_free (local);\r
 289 >  \r
 290 >      return NOTMUCH_STATUS_SUCCESS;\r
 291 > _______________________________________________\r
 292 > notmuch mailing list\r
 293 > notmuch@notmuchmail.org\r
 294 > http://notmuchmail.org/mailman/listinfo/notmuch\r