Date: Wed, 13 Nov 2013 21:08:52 +0100
From: Peter Zijlstra <peterz@infradead.org>
To: Austin Clements <aclements@csail.mit.edu>
Subject: Re: [PATCH] notmuch: Add "maildir:" search option
Message-ID: <20131113200852.GG16796@laptop.programming.kicks-ass.net>
References: <20131112155637.GA16796@laptop.programming.kicks-ass.net>
	<87mwl94dte.fsf@awakening.csail.mit.edu>
	<87k3gd4dfb.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87k3gd4dfb.fsf@awakening.csail.mit.edu>
User-Agent: Mutt/1.5.21 (2012-12-30)
Cc: notmuch@notmuchmail.org
Precedence: list

On Tue, Nov 12, 2013 at 02:39:52PM -0500, Austin Clements wrote:
> On Tue, 12 Nov 2013, Austin Clements <aclements@csail.mit.edu> wrote:
> > I think this is a great idea.  Personally I think this is how folder:
> > should work.  I find the semantics of folder: to be useless except where
> > they happen to coincide with the boolean semantics used here.
> > Unfortunately, changing folder: would require versioning the database,
> > which we have only primordial support for right now.
> >
> > Various comments below, though nothing major.  Of course, we'd also need
> > some tests and man page updates for this.
> 
> Sorry, one important thing I missed: this doesn't correctly handle when
> file names are removed from a message
> (_notmuch_message_remove_filename).  Probably the simplest thing would
> be to follow the template for how folder: works by first removing *all*
> folder terms and then adding back the still-valid ones.  (Unfortunately,
> just removing the term for the removed filename's directory won't work
> because the message could have other filenames in the same directory,
> though maybe you could just scan for that possibility?)

Oh, right you are. A little something like the below? Its compile tested
only and I've not yet had time to look at how the test infrastructure
works.


---
 lib/database.cc |   3 +-
 lib/message.cc  | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 104 insertions(+), 10 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index a021bf17253c..e43e17dffcd0 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -208,7 +208,8 @@ static prefix_t BOOLEAN_PREFIX_EXTERNAL[] = {
     { "thread",			"G" },
     { "tag",			"K" },
     { "is",			"K" },
-    { "id",			"Q" }
+    { "id",			"Q" },
+    { "maildir",		"XMAILDIR:" },
 };
 
 static prefix_t PROBABILISTIC_PREFIX[]= {
diff --git a/lib/message.cc b/lib/message.cc
index 1b4637950f8e..73d3bb65ab67 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -22,6 +22,7 @@
 #include "database-private.h"
 
 #include <stdint.h>
+#include <string.h>
 
 #include <gmime/gmime.h>
 
@@ -473,6 +474,71 @@ notmuch_message_get_replies (notmuch_message_t *message)
     return _notmuch_messages_create (message->replies);
 }
 
+/* Construct a proper 'maildir' from 'directory'
+ *
+ * Takes the relative directory component inside the maildir pathname and
+ * construct a maildir path from it.
+ *
+ * For filesystem layout Maildir we use the regular filesystem path except the
+ * trailing "cur"/"new" component.
+ *
+ * For Maildir++ we strip the leading '.' and replace subsequent '.'s with '/'s
+ */
+static char *
+_notmuch_message_maildir (void *ctx, const char *directory)
+{
+    char *maildir;
+    int i;
+
+    maildir = talloc_strdup (ctx, directory);
+    i = strlen (maildir);
+
+    /* Strip trailing '/' */
+    while (i && maildir[i - 1] == '/') {
+	maildir[i - 1] = '\0';
+	i--;
+    }
+
+    /* Strip leading '/' */
+    while (maildir[0] == '/') {
+	maildir++;
+	i--;
+    }
+
+    if (i >= 3) {
+	/* Consume trailing maildir directory entries */
+	if (STRNCMP_LITERAL (maildir, "cur") == 0 ||
+	    STRNCMP_LITERAL (maildir, "new") == 0)
+	{
+	    maildir[i - 3] = '\0';
+	    i -= 3;
+	}
+
+	/* Strip trailing '/' */
+	while (i && maildir[i - 1] == '/') {
+	    maildir[i-1] = '\0';
+	    i--;
+	}
+    }
+
+    /* Maildir++ */
+    if (maildir[0] == '.') {
+	maildir++;
+
+	/* Replace all remaining '.' with '/' */
+	for (i = 0; maildir[i]; i++) {
+	    if (maildir[i] == '.')
+		maildir[i] = '/';
+	}
+    }
+
+    /* If there's no string left, we're the "INBOX" */
+    if (maildir[0] == '\0')
+	    maildir = talloc_strdup (ctx, "INBOX");
+
+    return maildir;
+}
+
 /* Add an additional 'filename' for 'message'.
  *
  * This change will not be reflected in the database until the next
@@ -485,6 +551,7 @@ _notmuch_message_add_filename (notmuch_message_t *message,
     notmuch_status_t status;
     void *local = talloc_new (message);
     char *direntry;
+    char *maildir;
 
     if (filename == NULL)
 	INTERNAL_ERROR ("Message filename cannot be NULL.");
@@ -507,6 +574,10 @@ _notmuch_message_add_filename (notmuch_message_t *message,
     /* New terms allow user to search with folder: specification. */
     _notmuch_message_gen_terms (message, "folder", directory);
 
+    /* New terms allow user to serarch with maildir: specification. */
+    maildir = _notmuch_message_maildir (local, directory);
+    _notmuch_message_add_term (message, "maildir", maildir);
+
     talloc_free (local);
 
     return NOTMUCH_STATUS_SUCCESS;
@@ -535,11 +606,18 @@ _notmuch_message_remove_filename (notmuch_message_t *message,
     void *local = talloc_new (message);
     char *zfolder_prefix = talloc_asprintf(local, "Z%s", folder_prefix);
     int zfolder_prefix_len = strlen (zfolder_prefix);
-    char *direntry;
+    const char *relative, *directory;
+    char *direntry, *maildir;
     notmuch_private_status_t private_status;
     notmuch_status_t status;
     Xapian::TermIterator i, last;
 
+    relative = _notmuch_database_relative_path (message->notmuch, filename);
+
+    status = _notmuch_database_split_path (local, relative, &directory, NULL);
+    if (status)
+	return status;
+
     status = _notmuch_database_filename_to_direntry (
 	local, message->notmuch, filename, NOTMUCH_FIND_LOOKUP, &direntry);
     if (status || !direntry)
@@ -553,12 +631,21 @@ _notmuch_message_remove_filename (notmuch_message_t *message,
     if (status)
 	return status;
 
-    /* Re-synchronize "folder:" terms for this message. This requires:
-     *  1. removing all "folder:" terms
-     *  2. removing all "folder:" stemmed terms
-     *  3. adding back terms for all remaining filenames of the message. */
-
-    /* 1. removing all "folder:" terms */
+    /* Re-synchronize "folder:" and "maildir:" terms for this message. This
+     * requires:
+     *  1. removing "maildir:" for this filename
+     *  2. removing all "folder:" terms
+     *  3. removing all "folder:" stemmed terms
+     *
+     * For all remaining filenames of the message:
+     *  4. adding back "folder:" terms
+     *  5. adding back "maildir:" */
+
+    /* 1. remove "maildir:" for this message */
+    maildir = _notmuch_message_maildir (local, directory);
+    _notmuch_message_remove_term (message, "maildir", maildir);
+
+    /* 2. removing all "folder:" terms */
     while (1) {
 	i = message->doc.termlist_begin ();
 	i.skip_to (folder_prefix);
@@ -577,7 +664,7 @@ _notmuch_message_remove_filename (notmuch_message_t *message,
 	}
     }
 
-    /* 2. removing all "folder:" stemmed terms */
+    /* 3. removing all "folder:" stemmed terms */
     while (1) {
 	i = message->doc.termlist_begin ();
 	i.skip_to (zfolder_prefix);
@@ -596,7 +683,7 @@ _notmuch_message_remove_filename (notmuch_message_t *message,
 	}
     }
 
-    /* 3. adding back terms for all remaining filenames of the message. */
+    /* for all remaining filenames of the message */
     i = message->doc.termlist_begin ();
     i.skip_to (direntry_prefix);
 
@@ -623,8 +710,14 @@ _notmuch_message_remove_filename (notmuch_message_t *message,
 	directory = _notmuch_database_get_directory_path (local,
 							  message->notmuch,
 							  directory_id);
+
+	/* 4. adding back "folder:" terms */
 	if (strlen (directory))
 	    _notmuch_message_gen_terms (message, "folder", directory);
+
+	/* 5. adding back "maildir:" */
+	maildir = _notmuch_message_maildir (local, directory);
+	_notmuch_message_add_term (message, "maildir", maildir);
     }
 
     talloc_free (local);