To: <notmuch@notmuchmail.org>
MIME-Version: 1.0
Date: Sun, 22 Nov 2009 10:33:53 +0100
From: Michiel Buddingh' <michiel@michielbuddingh.net>
In-Reply-To: <87d43b2oif.fsf@yoom.home.cworth.org>
References: <87fx8bygi7.fsf@linux.vnet.ibm.com>
	<87bpiv4t9h.fsf@yoom.home.cworth.org>
	<87y6lz39nd.fsf@yoom.home.cworth.org>
	<20091121221207.GB17268@jukie.net>
	<9cce5525b093b87fe74d427954ffad89@localhost>
	<87d43b2oif.fsf@yoom.home.cworth.org>
Message-ID: <9bfdedddeab9c58cd45d8d448323d0fc@localhost>
User-Agent: RoundCube Webmail/0.1
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=UTF-8
Subject: Re: [notmuch] [PATCH] notmuch: Add Maildir directory name as tag
 name for messages
Precedence: list


On Sun, 22 Nov 2009 05:04:56 +0100, Carl Worth <cworth@cworth.org> wrote:
> Hi Michel, welcome to Notmuch!

Thanks, and apologies for the accidental base64 encoding.

First things first:

>> In the mean time, I've made a smaller, hopefully more harmless 
>> patch to let 'notmuch new' mark messages stored in a Maildir 'cur'
>> folder as 'read' rather than 'unread'.
> 
> Can others who have more experience weigh in here? Will this do the
> right thing for you? Do mail clients wait to move things into "cur"
> after the user has actually read them, or is that just a place where
> they are moved when the MUA _receives_ them?

You're absolutely right, and I'm a fool (because I _knew_ this, but
forgot).  Maildir stores flags (seen, replied, flagged, trashed,
passed) in file names.

On the positive side, this allows us to map these flags onto tags,
at least for indexing (the patch at the bottom implements this), and,
if I can pry you away from your principles, later for modification
as well.

>> Any attempt to match tags up to directories will eventually have 
>> to deal with with the fact that tags can't be neatly mapped onto 
>> them.  If I remove a directory-tag from a message, does this 
>> mean the message is removed from that directory?  What if a 
>> message has two directory-tags, does it mean it's present in both
>> directories?
> 
> Right. We definitely don't want a strong mapping here.

I propose that the maildir 'storage_type' could make an exception for
standard Maildir flags.  It'll take relatively little effort to
special-case the abovementioned flags, and it'd be a huge boon to
interoperability.

>> At the same time, this kind of interoperability would be highly
>> desirable to those of us who access their mail using other  
>> clients (webmail, mobile phones, etc.) that expect hierarchical
>> ordering.
> 
> That kind of thing is going to be "harder".
> 
> So far we're trying to stick with the principle that notmuch itself
> doesn't mess with the data store.

I respect your desire to stick to that principle.  But I also know 
that purity and simplicity, generally speaking, are unattainable
luxuries for most applications that handle mail.

> But then, we also want notmuch to be
> very scriptable, so someone might write a tool that uses notmuch search
> to export a set of hierarchical maildirs based on the tag names. (These
> could even just be populated with symlinks like mairix does.) So
> something like that could be really useful for integrating.

That is a very interesting idea.  On the other hand, interoperability
with Maildir mail stores is unlikely to be a corner case.  The MTA is
probably going to deliver new mail to a Maildir, procmail understands it,
etc.  I'd feel more comfortable relegating this integration to a 
scripted glue layer if I knew for certain such a glue layer would be
up to the task.
 
> I'm very much of the opinion that the user shouldn't care at all about
> the storage of the actual mail files that notmuch indexes.

The user certainly shouldn't, but I'm not sure that notmuch can remain
as agnostic about the actual storage of messages as planned.

Another thing; notmuch currently indexes by message-id (or SHA-1 hash
if that is not present).  Maildir, on the other hand, uses file names
as stable (when stripped of the parts trailing the colon) unique 
(knock on wood) identifiers.  A Maildir-aware notmuch could incorporate
this to be far more resistant to bulk mail moves done by other clients,
by using filename lookups to avoid accessing and parsing the mail 
files themselves.

I should re-iterate that I'm new to notmuch, and it's obvious that I'm
trying to beat it into becoming something it was never intended to be;
on the other hand, I'd like people to chime in on this.

again via webmail,
Michiel Buddingh'

---
 notmuch-client.h |   10 ++++++
 notmuch-config.c |   33 +++++++++++++++++--
 notmuch-new.c    |   95
+++++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 124 insertions(+), 14 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index ea77686..c39be06 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -69,12 +69,16 @@
 #define STRNCMP_LITERAL(var, literal) \
     strncmp ((var), (literal), sizeof (literal) - 1)
 
+enum storage_type { UNSET, NONE, MAILDIR };
+
 typedef void (*add_files_callback_t) (notmuch_message_t *message);
 
 typedef struct {
     int ignore_read_only_directories;
     int saw_read_only_directory;
 
+    enum storage_type storage_type;
+
     int total_files;
     int processed_files;
     int added_messages;
@@ -179,7 +183,13 @@ notmuch_config_set_user_other_email (notmuch_config_t
*config,
 				     const char *other_email[],
 				     size_t length);
 
+enum storage_type
+notmuch_config_get_storage_type (notmuch_config_t *config);
+
 notmuch_bool_t
 debugger_is_active (void);
 
+
+
+
 #endif
diff --git a/notmuch-config.c b/notmuch-config.c
index aaa0372..13bd341 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -31,11 +31,13 @@ static const char toplevel_config_comment[] =
 static const char database_config_comment[] =
     " Database configuration\n"
     "\n"
-    " The only value supported here is 'path' which should be the
top-level\n"
+    " The most important value here is 'path' which should be the
top-level\n"
     " directory where your mail currently exists and to where mail will
be\n"
     " delivered in the future. Files should be individual email
messages.\n"
     " Notmuch will store its database within a sub-directory of the
path\n"
-    " configured here named \".notmuch\".\n";
+    " configured here named \".notmuch\".\n"
+    " The other value is 'storage_type', which can currently be set to\n"
+    " 'maildir' or 'none'.\n";
 
 static const char user_config_comment[] =
     " User configuration\n"
@@ -62,6 +64,8 @@ struct _notmuch_config {
     char *user_primary_email;
     char **user_other_email;
     size_t user_other_email_length;
+
+    enum storage_type storage_type;
 };
 
 static int
@@ -193,6 +197,7 @@ notmuch_config_open (void *ctx,
     config->user_primary_email = NULL;
     config->user_other_email = NULL;
     config->user_other_email_length = 0;
+    config->storage_type = UNSET;
 
     if (! g_key_file_load_from_file (config->key_file,
 				     config->filename,
@@ -257,7 +262,7 @@ notmuch_config_open (void *ctx,
 	    talloc_free (email);
 	}
     }
-
+    
     /* When we create a new configuration file here, we  add some
      * comments to help the user understand what can be done. */
     if (is_new) {
@@ -334,6 +339,28 @@ notmuch_config_get_database_path (notmuch_config_t
*config)
     return config->database_path;
 }
 
+enum storage_type
+notmuch_config_get_storage_type (notmuch_config_t * config)
+{
+    char * type;
+
+    if (config->storage_type == UNSET) {
+	type = g_key_file_get_string (config->key_file,
+				      "database", "storage_type", NULL);
+	if (type) {
+	    if (strcasecmp(type, "maildir") == 0) {
+		config->storage_type = MAILDIR;
+	    } else if (strcasecmp(type, "none") == 0) {
+		config->storage_type = NONE;
+	    }
+	}
+    }
+
+    return config->storage_type;
+}
+
+
+
 void
 notmuch_config_set_database_path (notmuch_config_t *config,
 				  const char *database_path)
diff --git a/notmuch-new.c b/notmuch-new.c
index 0dd2784..0918774 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -41,12 +41,65 @@ handle_sigint (unused (int sig))
 }
 
 static void
-tag_inbox_and_unread (notmuch_message_t *message)
+tag_as_inbox (notmuch_message_t *message)
 {
     notmuch_message_add_tag (message, "inbox");
+}
+
+static void
+tag_as_unread (notmuch_message_t *message)
+{
     notmuch_message_add_tag (message, "unread");
 }
 
+
+static void
+derive_tags_from_maildir_flags (notmuch_message_t *message, const char *
path) 
+{
+    int seen = FALSE;
+    int end_of_flags = FALSE;
+    size_t l = strlen(path);
+
+    /* Non-experimental message flags start with this */
+    char * i = strstr(path, ":2,");
+    if (i != NULL) {
+
+	i += 3;
+	for (; i < (path + l) && !end_of_flags; i++) {
+	    switch (*i) {
+	    case 'F': /* flagged */
+		notmuch_message_add_tag (message, "flagged");
+		break;
+	    case 'R': /* the message has been replied to */
+		notmuch_message_add_tag (message, "replied");
+		break;
+	    case 'D': 
+		notmuch_message_add_tag (message, "draft");
+		break;
+	    case 'S': /* Indicate a message has been read */
+		notmuch_message_add_tag (message, "seen");
+		seen = TRUE;
+		break;
+	    case 'T': /* Indicates a message has been marked as trash */
+		notmuch_message_add_tag (message, "trashed");
+		break;
+	    case 'P': /* indicates a message has been forwarded */
+		notmuch_message_add_tag (message, "passed");
+		break;
+	    default:
+		end_of_flags = TRUE;
+		break;
+	    }
+	}
+    }
+    
+    if (i == NULL || !seen) { 
+	/* mark messages without any flags, or the seen flag as 'unseen' */
+	notmuch_message_add_tag (message, "unseen");
+    }
+}
+
+
 static void
 add_files_print_progress (add_files_state_t *state)
 {
@@ -104,6 +157,7 @@ static notmuch_status_t
 add_files_recursive (notmuch_database_t *notmuch,
 		     const char *path,
 		     struct stat *st,
+		     int scan_files,
 		     add_files_state_t *state)
 {
     DIR *dir = NULL;
@@ -119,11 +173,9 @@ add_files_recursive (notmuch_database_t *notmuch,
      * directory, (with this being a clear clue from the user to
      * Notmuch that new mail won't be arriving there and we need not
      * look. */
-    if (state->ignore_read_only_directories &&
-	(st->st_mode & S_IWUSR) == 0)
-    {
+    if (((st->st_mode & S_IWUSR) == 0) &&
(state->ignore_read_only_directories)) {
 	state->saw_read_only_directory = TRUE;
-	goto DONE;
+	goto DONE;	    
     }
 
     path_mtime = st->st_mtime;
@@ -173,7 +225,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 	    continue;
 	}
 
-	if (S_ISREG (st->st_mode)) {
+	if (S_ISREG (st->st_mode) && scan_files) {
 	    /* If the file hasn't been modified since the last
 	     * add_files, then we need not look at it. */
 	    if (path_dbtime == 0 || st->st_mtime > path_dbtime) {
@@ -184,7 +236,11 @@ add_files_recursive (notmuch_database_t *notmuch,
 		    /* success */
 		    case NOTMUCH_STATUS_SUCCESS:
 			state->added_messages++;
-			tag_inbox_and_unread (message);
+			tag_as_inbox (message);
+			if (state->storage_type == MAILDIR)
+			    derive_tags_from_maildir_flags (message, next);
+			else 
+			    tag_as_unread (message);
 			break;
 		    /* Non-fatal issues (go on to next file) */
 		    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
@@ -223,9 +279,25 @@ add_files_recursive (notmuch_database_t *notmuch,
 		}
 	    }
 	} else if (S_ISDIR (st->st_mode)) {
-	    status = add_files_recursive (notmuch, next, st, state);
-	    if (status && ret == NOTMUCH_STATUS_SUCCESS)
-		ret = status;
+	    if (state->storage_type == MAILDIR) {
+		char * leaf = basename(next);
+		
+		if (strcmp(leaf, "cur") == 0) {
+		    status = add_files_recursive (notmuch, next, st, TRUE, state);
+		} else if (strcmp(leaf, "tmp") == 0) {
+		    /* respect Maildir specification and don't touch files in tmp */
+		    continue;
+		} else if (strcmp(leaf, "new") == 0) {
+		    status = add_files_recursive (notmuch, next, st, TRUE, state);
+		} else {
+		    /* Perhaps add in tags? */
+		    status = add_files_recursive (notmuch, next, st, FALSE, state);
+		}
+	    } else {
+		status = add_files_recursive (notmuch, next, st, TRUE, state);
+		if (status && ret == NOTMUCH_STATUS_SUCCESS)
+		    ret = status;
+	    }
 	}
 
 	talloc_free (next);
@@ -292,7 +364,7 @@ add_files (notmuch_database_t *notmuch,
 	timer_is_active = TRUE;
     }
 
-    status = add_files_recursive (notmuch, path, &st, state);
+    status = add_files_recursive (notmuch, path, &st, TRUE, state);
 
     /* Now stop the timer. */
     if (timer_is_active) {
@@ -437,6 +509,7 @@ notmuch_new_command (void *ctx,
     add_files_state.saw_read_only_directory = FALSE;
     add_files_state.processed_files = 0;
     add_files_state.added_messages = 0;
+    add_files_state.storage_type = notmuch_config_get_storage_type
(config);
     gettimeofday (&add_files_state.tv_start, NULL);
 
     ret = add_files (notmuch, db_path, &add_files_state);
-- 
1.6.5.3