--- /dev/null
+Return-Path: <peterz@infradead.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 3467A431FDA\r
+ for <notmuch@notmuchmail.org>; Tue, 12 Nov 2013 11:47:39 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -2.3\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id zK3Owp4lsx65 for <notmuch@notmuchmail.org>;\r
+ Tue, 12 Nov 2013 11:47:31 -0800 (PST)\r
+Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134])\r
+ (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id 2E633431FD2\r
+ for <notmuch@notmuchmail.org>; Tue, 12 Nov 2013 11:47:31 -0800 (PST)\r
+Received: from dhcp-077-248-225-117.chello.nl ([77.248.225.117] helo=laptop)\r
+ by merlin.infradead.org with esmtpsa (Exim 4.80.1 #2 (Red Hat Linux))\r
+ id 1VgJvt-0000kX-RB; Tue, 12 Nov 2013 19:47:30 +0000\r
+Received: by laptop (Postfix, from userid 1000)\r
+ id 96DD8103BA2B9; Tue, 12 Nov 2013 20:47:27 +0100 (CET)\r
+Date: Tue, 12 Nov 2013 20:47:27 +0100\r
+From: Peter Zijlstra <peterz@infradead.org>\r
+To: Austin Clements <aclements@csail.mit.edu>\r
+Subject: Re: [PATCH] notmuch: Add "maildir:" search option\r
+Message-ID: <20131112194727.GE16796@laptop.programming.kicks-ass.net>\r
+References: <20131112155637.GA16796@laptop.programming.kicks-ass.net>\r
+ <87mwl94dte.fsf@awakening.csail.mit.edu>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Content-Disposition: inline\r
+In-Reply-To: <87mwl94dte.fsf@awakening.csail.mit.edu>\r
+User-Agent: Mutt/1.5.21 (2012-12-30)\r
+X-Mailman-Approved-At: Tue, 12 Nov 2013 12:40:21 -0800\r
+Cc: notmuch@notmuchmail.org\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Tue, 12 Nov 2013 19:47:39 -0000\r
+\r
+On Tue, Nov 12, 2013 at 02:31:25PM -0500, Austin Clements wrote:\r
+> > XXX: now I need to go figure out how to do searches like:\r
+> >\r
+> > subject:PATCH/0\r
+> >\r
+> > which would mandate that PATCH is the first word occurring in the\r
+> > subject. I think the position index holds enough information but I\r
+> > need to look into that and obviously the query parser needs work for\r
+> > this.\r
+> \r
+> This is a frequently requested feature. Unfortunately, there are two\r
+> technical hurdles. One is that the position information actually\r
+> *isn't* enough as it is both because the subject will start at some\r
+> arbitrary term offset that depends on the from and to (and maybe other\r
+> things) and because the Xapian Query structure doesn't expose a way to\r
+> search for a specific term offset (only relative offsets). The other is\r
+> that we currently use Xapian's query parser, which isn't extensible,\r
+> though I took a stab at a custom query parser long ago and swear that\r
+> one of these days I'll return to it.\r
+\r
+Bah, I knew that would end up being more complex :-)\r
+\r
+> > lib/database.cc | 7 ++++---\r
+> > lib/message.cc | 40 ++++++++++++++++++++++++++++++++++++++++\r
+> > 2 files changed, 44 insertions(+), 3 deletions(-)\r
+> >\r
+> > diff --git a/lib/database.cc b/lib/database.cc\r
+> > index a021bf17253c..53aeaa68954d 100644\r
+> > --- a/lib/database.cc\r
+> > +++ b/lib/database.cc\r
+> > @@ -208,15 +208,16 @@ static prefix_t BOOLEAN_PREFIX_EXTERNAL[] = {\r
+> > { "thread", "G" },\r
+> > { "tag", "K" },\r
+> > { "is", "K" },\r
+> > - { "id", "Q" }\r
+> > + { "id", "Q" },\r
+> > + { "maildir", "XMAILDIR:" },\r
+> \r
+> No colon. That is, the term prefix should just be "XMAILDIR".\r
+\r
+No, that colon is crucial; see http://xapian.org/docs/omega/termprefixes.html\r
+\r
+"X starts a multi-capital letter user-defined prefix. If you want a\r
+prefix for something without a standard prefix, you create your own\r
+starting with an X (e.g. XSHOESIZE). The prefix ends with the first\r
+non-capital. If the term you're prefixing starts with a capital, add a\r
+":" between prefix and term to resolve ambiguity about where the prefix\r
+ends and the term begins."\r
+\r
+Since maildir folder names typically start with a capital we need that\r
+':' in between the prefix and value. I tried, my initial versions didn't\r
+have the ':' and reliably didn't work.\r
+\r
+> > };\r
+> > \r
+> > static prefix_t PROBABILISTIC_PREFIX[]= {\r
+> > { "from", "XFROM" },\r
+> > { "to", "XTO" },\r
+> > { "attachment", "XATTACHMENT" },\r
+> > - { "subject", "XSUBJECT"},\r
+> > - { "folder", "XFOLDER"}\r
+> > + { "subject", "XSUBJECT" },\r
+> > + { "folder", "XFOLDER" },\r
+> \r
+> Unintentional whitespace change?\r
+\r
+Oops yes.\r
+\r
+> > + /* Convert the directory into a maildir path */\r
+> > + maildir = talloc_strdup(local, directory);\r
+> \r
+> Add a space before the "(". (Same thing throughout the rest of the\r
+> patch.)\r
+\r
+Urgh, weird coding style but yes you're right, I should've kept it\r
+consistent with the rest of the file. Will fix.\r
+\r
+> > +\r
+> > + /* Strip the maildir "cur", "new" directory entries. */\r
+> > + i = strlen(maildir);\r
+> > + if (strncmp(maildir + i - 3, "cur", 3) == 0 ||\r
+> > + strncmp(maildir + i - 3, "new", 3) == 0) {\r
+> \r
+> This is unsafe if directory is less than three characters, which I\r
+> believe could happen if the message is in the root mail directory (which\r
+> shouldn't happen with a well-formed maildir, but notmuch doesn't require\r
+> maildir, and, regardless, we should be defensive).\r
+> \r
+> Also, we have a STRNCMP_LITERAL macro that we often use for comparisons\r
+> with string literals, but I'm good with this, too.\r
+\r
+Quite so, I haven't actually seen that, but you're quite right.\r
+\r
+> > + maildir[i - 3] = '\0';\r
+> > + i -= 3;\r
+> > + }\r
+> > +\r
+> > + /* Strip trailing '/' */\r
+> > + while (maildir[i-1] == '/') {\r
+> \r
+> This is also unsafe if maildir is the empty string.\r
+> \r
+> Also, add spaces around the "-" (likewise on the next line).\r
+\r
+Will fix.\r
+\r
+> > + maildir[i-1] = '\0';\r
+> > + i--;\r
+> > + }\r
+> > +\r
+> > + /* Strip leading '/' */\r
+> > + while (maildir[0] == '/')\r
+> > + maildir++;\r
+> > +\r
+> > + /* Strip leading '.' */\r
+> > + while (maildir[0] == '.')\r
+> \r
+> Why strip multiple dots? (I'm not saying you shouldn't, I'm just\r
+> curious; and it may be worth explaining in the comment.)\r
+\r
+No reason, copy paste damage from above mostly.\r
+\r
+> > + maildir++;\r
+> > +\r
+> > + /* Replace all remaining '.' with '/' */\r
+> \r
+> I think this should only happen if there was a leading '.', indicating a\r
+> Maildir++ folder hierarchy. A lot of people use the "file system"\r
+> Maildir layout, which just consists of nested directories where the\r
+> leaves are Maildirs (e.g., Dovecot's LAYOUT=fs,\r
+> http://wiki2.dovecot.org/MailboxFormat/Maildir#Directory_Structure). In\r
+> this case, it's perfectly legitimate to have '.'s in folder names and it\r
+> would be confusing and unexpected to translate them to '/'s. This would\r
+> also make this compatible with MH folders (which notmuch supports,\r
+> though admittedly it's unclear if anyone actually uses them).\r
+\r
+Ah, I wasn't aware Dovecot actually supported this Maildir variant --\r
+although I've recently come across this variant someplace and thought it\r
+was odd.\r
+\r
+OK, I can make it conditional on the initial leading dot.\r
+\r
+I'll respin the patch and try to come up with manpage and test additions\r
+to cover this new functionality.\r
projects / notmuch-archives.git / commitdiff