Re: notmuch release 0.5 now available

2010-11-18 Thread Ethan Glasser-Camp

On 11/12/2010 12:02 AM, Carl Worth wrote:

The major feature in notmuch 0.5 is the ability to automatically
synchronize maildir flags, (so that if a mail file gets marked
externally with the flag 'S' for seen then the unread tag in the
notmuch database will be automatically removed). And of course, there
are various fixes and improvements throughout. See below for details.

Hi guys,

I know I'm late to the party, but I just wanted to say I've been playing 
with notmuch 0.5 and the synchronization stuff works great. Great job guys!


Ethan

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Distributed Notmuch

2012-01-08 Thread Ethan Glasser-Camp

Hi guys,

It's kind of academic for me right now because I'm mostly just using one 
computer, but one reason I've hesitated to switch over entirely to 
notmuch is that it's hard to distribute across many machines. The last 
time I wrote the list about this, David Bremner pointed me to gitmuch in 
the notmuch-scripts package, which uses git to synchronize tags. He 
wrote, No one claims this is a great solution, but it works now.


In brainstorming about the One True Mail Setup, my friend suggested to 
me that Maildir/IMAP are not really the best choices for mail storage. 
Among other flaws: to synchronize mail via IMAP you have to check the 
headers of each message, which means a lot of bandwidth; you can't 
compress Maildir, meaning lots of wasted space; mechanisms to 
synchronize arbitrary tags have to be bolted on top. My friend suggested 
that instead it might be better to dump mail into some kind of database, 
for example CouchDB, and synchronize it that way. Of course, doing 
full-text indexing and tagging using an arbitrary DB would be a ton of 
work, so instead it probably makes the most sense to keep a Xapian 
instance on each node and feed all the mail to that. Tagging operations 
still have to be replicated, probably by an oplog that's also kept in 
Couch, so it's still a lot of work, but keeping things in Couch 
automatically gets you a lot of the replication mechanisms, offline 
access, etc., that would have to be bolted on/hacked up using tools like 
nmbug/gitmuch/rsync. I also see in the wiki that someone proposes to use 
git as the mail store, presumably for similar reasons. Xapian itself has 
the idea of master/slave replication but that doesn't really get you 
full offline access.


So my question for the wizards on this list is what their idea of the 
One True Mail Setup would be in a perfect, or slightly better, world, 
and what needs to be done to get there. I know some people use one 
notmuch install that they access remotely. For myself, I'm on a pretty 
limited Internet connection, so low bandwidth/offline access are big for 
me, and despite Nicolas Sebrecht and Sebastian Spaeth's heroic work on 
OfflineIMAP, it still uses a lot of bandwidth to sync. And obviously the 
whole point of this exercise is tag synchronization..


Ethan

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2] Document external dependencies in the test suite

2012-01-16 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

Add an explicit note to the README explaining what programs are
necessary and the perhaps-surprising behavior of skipping tests if
they aren't present.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---

v2 suggested by Dmitry Kurochkin: document all the dependencies.

 test/README |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/test/README b/test/README
index bde6db0..44ff653 100644
--- a/test/README
+++ b/test/README
@@ -6,6 +6,19 @@ When fixing bugs or enhancing notmuch, you are strongly 
encouraged to
 add tests in this directory to cover what you are trying to fix or
 enhance.
 
+Prerequisites
+-
+Some tests require external dependencies to run. Without them, they
+will be skipped, or (rarely) marked failed. Please install these, so
+that you know if you break anything.
+
+  - dtach(1)
+  - emacs(1)
+  - emacsclient(1)
+  - gdb(1)
+  - gpg(1)
+  - python(1)
+
 Running Tests
 -
 The easiest way to run tests is to say make test, (or simply run the
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] moved _notmuch_get_list () and _notmuch_set_list () up in file

2012-01-27 Thread Ethan Glasser-Camp

On 01/27/2012 05:42 AM, Tomi Ollila wrote:

On Thu, 26 Jan 2012 13:03:46 +, Jani Nikulaj...@nikula.org  wrote:

On Thu, 26 Jan 2012 12:11:57 +0200, Tomi Ollilatomi.oll...@iki.fi  wrote:

Moved _notmuch_get_list () and _notmuch_set_list () to a location
in notmuch-config.c so that new functions that will be located
before the old location of those functions can also use these.

Parse error. ;)

You mean something along the lines of: Move _notmuch_get_list () and
_notmuch_set_list () earlier in the file to avoid forward declarations
in further work. No functional changes.

I'm sure native speakers can bikeshed that further. ;)

Ok, they haven't. I'n resubmit this alone with better commit
message -- I look the other after I have better time.


As a native speaker, your new version is acceptable but I found in 
further work a little odd. (Depending on what you meant, I'd say in 
upcoming patches.) The thing I found most confusing about the comment 
is that the functions aren't called _notmuch_get_list or 
_notmuch_set_list (instead they are _config_get_list and _config_set_list.)


Ethan

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] Free the results of scandir()

2012-02-06 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

scandir() returns strings allocated via malloc(3) which are then
collected in array namelist which is allocated via
malloc(3). Currently we just free the array namelist. Instead, free
all the entries of namelist, and then free namelist.

entry only points to elements of namelist, so we don't free it
separately.
---

This should fix a minor memory leak in notmuch-new. Please confirm I'm
reading the manpage correctly ;)

 notmuch-new.c |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index a569a54..c536873 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -559,12 +559,14 @@ add_files_recursive (notmuch_database_t *notmuch,
   DONE:
 if (next)
talloc_free (next);
-if (entry)
-   free (entry);
 if (dir)
closedir (dir);
-if (fs_entries)
+if (fs_entries){
+   for (i = 0; i  num_fs_entries; i++){
+   free (fs_entries[i]);
+   }
free (fs_entries);
+}
 if (db_subdirs)
notmuch_filenames_destroy (db_subdirs);
 if (db_files)
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] Free the results of scandir()

2012-02-07 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

scandir() returns strings allocated via malloc(3) which are then
collected in array namelist which is allocated via
malloc(3). Currently we just free the array namelist. Instead, free
all the entries of namelist, and then free namelist.

entry only points to elements of namelist, so we don't free it
separately.
---

v3: I'm still learning the house style. Thanks Dmitry.

 notmuch-new.c |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index a569a54..8dbebb3 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -559,12 +559,14 @@ add_files_recursive (notmuch_database_t *notmuch,
   DONE:
 if (next)
talloc_free (next);
-if (entry)
-   free (entry);
 if (dir)
closedir (dir);
-if (fs_entries)
+if (fs_entries) {
+   for (i = 0; i  num_fs_entries; i++)
+   free (fs_entries[i]);
+
free (fs_entries);
+}
 if (db_subdirs)
notmuch_filenames_destroy (db_subdirs);
 if (db_files)
@@ -704,10 +706,12 @@ count_files (const char *path, int *count)
 }
 
   DONE:
-if (entry)
-   free (entry);
-if (fs_entries)
+if (fs_entries) {
+   for (i = 0; i  num_fs_entries; i++)
+   free (fs_entries[i]);
+
 free (fs_entries);
+}
 }
 
 static void
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Free the results of scandir()

2012-02-07 Thread Ethan Glasser-Camp

On 02/07/2012 05:10 AM, Dmitry Kurochkin wrote:

Please use --subject-prefix='PATCH vN' parameter when sending new
versions of patches.  Also, sending new versions as replies to the first
email in the original thread makes it easier to track.


Oops! Thanks again.

Ethan

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 00/13] Modular message store code

2012-02-15 Thread Ethan Glasser-Camp
Hi guys,

I'm submitting as RFC this patch series, which introduces the idea of a 
mailstore, a class that defines how to access mail, instead of currently 
assuming it's always some Maildir-ish hierarchy that contains a bunch of mail.

This was listed as a wishlist item on http://notmuchmail.org/feature-requests/, 
so I went ahead and took a crack at it, learning a lot about the codebase as I 
did. I'm sure there are tons of stylistic concerns so I'd like to get as much 
feedback as possible, starting of course with Does a feature like this have a 
chance of ever making it in and followed by Am I on the right track.

Note that this series breaks the language bundings; the Python bindings have 
very minimal tests so I very minimally fixed them (probably still broken in 
other ways), but the Ruby and Go bindings are probably super broken. Note also 
that the one message = one file approach is pretty thoroughly embedded into 
Notmuch and there are lots of places (again such as the Python bindings) where 
this has yet to be rooted out.

They say an interface isn't trustworthy until you've implemented it three 
times, so while most of the patches define the interface, the last patch adds 
support for an experimental CouchDB backend. It's got at least one known bug 
(it indexes everything, whether or not it's a mail object), sometimes it hangs 
when trying to access a message, and it's definitely leaking memory in 
notmuch-new. I haven't done strict timing comparisons but it seems like 
notmuch-search and notmuch-show are approximately as fast as with maildir and 
notmuch-new takes maybe 25% longer. Nevertheless, it is included as a 
demonstration that the interface is at least plausible.

The implementation of mailstores follows these principles:

- Messages still have filenames, but the mailstore gets to define its own 
semantics for these filenames (document ids, file + byte offset..). 
_notmuch_message_ensure_filename_list converts all filenames coming out of the 
DB to be absolute paths centered at the user's database path, which is 
inconvenient for Couchdb, but workable.

- The user keeps all mail in one mailstore. The alternative, which is that each 
message can be in a different mailstore, seemed like a lot more work. One 
mailstore makes sense when it's cases like maildir vs. couchdb, but if we 
decide to introduce a mbox backend -- directory tree with mbox files -- then 
it might overlap with the maildir mailstore, and then who knows?

Patch 1 introduces the configuration parameter database.type, which will be 
used to select a mailstore type.

Patch 2 introduces the most important API for a mailstore, 
notmuch_mailstore_open, and makes it required when creating a message_file. 
Patch 3 fixes the Python breakage this creates.

Patch 4-6 replace the other places where files are opened directly with calls 
to notmuch_mailstore_open.

Patch 7-8 prepare notmuch-new to be more generic. I couldn't find an elegant 
way to combine add_files logic with other mailstores, so I just decided each 
mailstore should have its own add_files function.

Patches 9-11 add other functions to the mailstore API -- to rename files, to 
close files, and to construct a mailstore. Patch 12 uses the close API to 
close files (where necessary).

Patch 13 proposes the CouchDB mailstore as one block of code.

Points to address:

- Where to put the new notmuch_mailstore_t* parameter in all these functions? I 
applied a decreasing order of importance heuristic, but it's a little weird 
in places like notmuch_database_open and notmuch_database_create.

- Is there a better, more elegant way to pass around mailstore objects without 
adding parameters to each function? Additionally, should I be using some other 
class-like mechanism for mailstores instead of hacks involving structs?

- Should this be configured under [database] or perhaps under some other new 
heading?

- How strict is the rule that braces shouldn't be there if the body of a 
loop/conditional is only one line? This feels really strange to me coming from 
Python.

- If I'm already touching code, should I add other drive-by fixes, as in patch 
05, or should I resolutely refuse to change anything, as in patch 07?

- Should something like the CouchDB backend be optional, and if so, what 
mechanisms do I need to use to make that happen?

Thanks so much for your time!
Ethan

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 01/13] Create configuration paramater database.type

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

This will be used to allow different backends to be developed to allow
access to mail that isn't stored in Maildirs.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 notmuch-client.h |7 ++
 notmuch-config.c |   57 +
 2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 60828aa..4518cb0 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -220,6 +220,13 @@ notmuch_config_set_database_path (notmuch_config_t *config,
  const char *database_path);
 
 const char *
+notmuch_config_get_database_type (notmuch_config_t *config);
+
+void
+notmuch_config_set_database_type (notmuch_config_t *config,
+ const char *database_type);
+
+const char *
 notmuch_config_get_user_name (notmuch_config_t *config);
 
 void
diff --git a/notmuch-config.c b/notmuch-config.c
index a124e34..b8bee69 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -32,11 +32,24 @@ 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
- 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;
+ Here is where you can tell notmuch where your mail currently exists\n
+ and where mail will be delivered in the future.
+\n
+ The following options are supported here:\n
+\n
+\ttypeThe type of mail backend. The only currently supported\n
+\tvalue is \maildir\.\n
+\tpathFor the maildir backend, the top-level maildir directory.\n
+\tFor all backends, the location where notmuch should store its\n
+\tdatabase. Notmuch will store its database within a 
sub-directory\n
+\tof this path named \.notmuch\.\n
+\n
+ Maildir backend\n
+\n
+ This backend reads mail from a directory tree where files are\n
+ individual email messages.\n
+ The only configuration option is 'path' which should be the top-level\n
+ directory.\n;
 
 static const char new_config_comment[] =
  Configuration for \notmuch new\\n
@@ -99,6 +112,7 @@ struct _notmuch_config {
 GKeyFile *key_file;
 
 char *database_path;
+char *database_type;
 char *user_name;
 char *user_primary_email;
 const char **user_other_email;
@@ -258,6 +272,7 @@ notmuch_config_open (void *ctx,
 config-key_file = g_key_file_new ();
 
 config-database_path = NULL;
+config-database_type = NULL;
 config-user_name = NULL;
 config-user_primary_email = NULL;
 config-user_other_email = NULL;
@@ -320,6 +335,10 @@ notmuch_config_open (void *ctx,
talloc_free (path);
 }
 
+if (notmuch_config_get_database_type (config) == NULL) {
+   notmuch_config_set_database_type (config, maildir);
+}
+
 if (notmuch_config_get_user_name (config) == NULL) {
char *name = get_name_from_passwd_file (config);
notmuch_config_set_user_name (config, name);
@@ -538,6 +557,34 @@ notmuch_config_set_database_path (notmuch_config_t *config,
 }
 
 const char *
+notmuch_config_get_database_type (notmuch_config_t *config)
+{
+char *type;
+
+if (config-database_type == NULL) {
+   type = g_key_file_get_string (config-key_file,
+ database, type, NULL);
+   if (type) {
+   config-database_type = talloc_strdup (config, type);
+   free (type);
+   }
+}
+
+return config-database_type;
+}
+
+void
+notmuch_config_set_database_type (notmuch_config_t *config,
+ const char *database_type)
+{
+g_key_file_set_string (config-key_file,
+  database, type, database_type);
+
+talloc_free (config-database_type);
+config-database_type = NULL;
+}
+
+const char *
 notmuch_config_get_user_name (notmuch_config_t *config)
 {
 char *name;
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 02/13] Add the concept of a mailstore in its absolute minimal sense

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

This introduces (and uses) the mailstore parameter to the
notmuch_message_file_open API, and passes this through wherever it
will be needed. This requires touching a lot of places just to change
one API. We end up adding it to the notmuch_database_t struct because
it is needed for notmuch_database_upgrade.

This doesn't touch the Python bindings, which require a certain amount
of effort. (Therefore, the Python tests will be broken until the next commit.)

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/Makefile.local |1 +
 lib/database-private.h |1 +
 lib/database.cc|   10 +---
 lib/mailstore.c|   59 
 lib/message-file.c |   10 +---
 lib/message.cc |   11 +---
 lib/notmuch-private.h  |7 -
 lib/notmuch.h  |   21 ++--
 lib/thread.cc  |   24 +++
 notmuch-client.h   |6 +
 notmuch-config.c   |   12 +
 notmuch-count.c|3 +-
 notmuch-dump.c |3 +-
 notmuch-new.c  |8 +-
 notmuch-reply.c|   45 
 notmuch-restore.c  |3 +-
 notmuch-search.c   |3 +-
 notmuch-show.c |   56 ++---
 notmuch-tag.c  |3 +-
 test/symbol-test.cc|3 +-
 20 files changed, 220 insertions(+), 69 deletions(-)
 create mode 100644 lib/mailstore.c

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 54c4dea..461e5cd 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -51,6 +51,7 @@ libnotmuch_c_srcs =   \
$(dir)/filenames.c  \
$(dir)/string-list.c\
$(dir)/libsha1.c\
+   $(dir)/mailstore.c  \
$(dir)/message-file.c   \
$(dir)/messages.c   \
$(dir)/sha1.c   \
diff --git a/lib/database-private.h b/lib/database-private.h
index 88532d5..1cb8c43 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -39,6 +39,7 @@
 struct _notmuch_database {
 notmuch_bool_t exception_reported;
 
+notmuch_mailstore_t *mailstore;
 char *path;
 
 notmuch_bool_t needs_upgrade;
diff --git a/lib/database.cc b/lib/database.cc
index c928d02..e3c8095 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -521,7 +521,7 @@ parse_references (void *ctx,
 }
 
 notmuch_database_t *
-notmuch_database_create (const char *path)
+notmuch_database_create (notmuch_mailstore_t *mailstore, const char *path)
 {
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path = NULL;
@@ -556,7 +556,7 @@ notmuch_database_create (const char *path)
goto DONE;
 }
 
-notmuch = notmuch_database_open (path,
+notmuch = notmuch_database_open (mailstore, path,
 NOTMUCH_DATABASE_MODE_READ_WRITE);
 notmuch_database_upgrade (notmuch, NULL, NULL);
 
@@ -579,7 +579,8 @@ _notmuch_database_ensure_writable (notmuch_database_t 
*notmuch)
 }
 
 notmuch_database_t *
-notmuch_database_open (const char *path,
+notmuch_database_open (notmuch_mailstore_t *mailstore,
+  const char *path,
   notmuch_database_mode_t mode)
 {
 void *local = talloc_new (NULL);
@@ -619,6 +620,7 @@ notmuch_database_open (const char *path,
 notmuch = talloc_zero (NULL, notmuch_database_t);
 notmuch-exception_reported = FALSE;
 notmuch-path = talloc_strdup (notmuch, path);
+notmuch-mailstore = mailstore;
 
 if (notmuch-path[strlen (notmuch-path) - 1] == '/')
notmuch-path[strlen (notmuch-path) - 1] = '\0';
@@ -1636,7 +1638,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 if (ret)
return ret;
 
-message_file = notmuch_message_file_open (filename);
+message_file = notmuch_message_file_open (notmuch-mailstore, filename);
 if (message_file == NULL)
return NOTMUCH_STATUS_FILE_ERROR;
 
diff --git a/lib/mailstore.c b/lib/mailstore.c
new file mode 100644
index 000..290da70
--- /dev/null
+++ b/lib/mailstore.c
@@ -0,0 +1,59 @@
+/* mailstore.c - mail storage backends
+ *
+ * Copyright © 2009 Carl Worth
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ */
+
+#include stdio.h
+
+#include notmuch-private.h
+
+typedef struct _notmuch_mailstore

[RFC PATCH 03/13] Introduce mailstore in the python bindings

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com


Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 bindings/python/notmuch/database.py  |   31 +-
 bindings/python/notmuch/globals.py   |3 ++
 bindings/python/notmuch/mailstore.py |   38 ++
 3 files changed, 61 insertions(+), 11 deletions(-)
 create mode 100644 bindings/python/notmuch/mailstore.py

diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 36b65ec..638ade3 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -23,7 +23,9 @@ from ctypes import c_char_p, c_void_p, c_uint, c_long, byref, 
POINTER
 from notmuch.globals import (nmlib, STATUS, NotmuchError, NotInitializedError,
  NullPointerError, Enum, _str,
  NotmuchDatabaseP, NotmuchDirectoryP, NotmuchMessageP, NotmuchTagsP,
- NotmuchQueryP, NotmuchMessagesP, NotmuchThreadsP, NotmuchFilenamesP)
+ NotmuchQueryP, NotmuchMessagesP, NotmuchThreadsP, NotmuchFilenamesP,
+ NotmuchMailstoreP)
+from notmuch.mailstore import Mailstore
 from notmuch.thread import Threads
 from notmuch.message import Messages, Message
 from notmuch.tag import Tags
@@ -78,7 +80,7 @@ class Database(object):
 
 notmuch_database_open
 _open = nmlib.notmuch_database_open
-_open.argtypes = [c_char_p, c_uint]
+_open.argtypes = [NotmuchMailstoreP, c_char_p, c_uint]
 _open.restype = NotmuchDatabaseP
 
 notmuch_database_upgrade
@@ -105,11 +107,13 @@ class Database(object):
 
 notmuch_database_create
 _create = nmlib.notmuch_database_create
-_create.argtypes = [c_char_p]
+_create.argtypes = [NotmuchMailstoreP, c_char_p]
 _create.restype = NotmuchDatabaseP
 
-def __init__(self, path=None, create=False, mode=0):
-If *path* is `None`, we will try to read a users notmuch
+def __init__(self, mailstore=None, path=None, create=False, mode=0):
+If *mailstore* is `None`, we will just use 'maildir'.
+
+If *path* is `None`, we will try to read a users notmuch
 configuration and use his configured database. The location of the
 configuration file can be specified through the environment variable
 *NOTMUCH_CONFIG*, falling back to the default `~/.notmuch-config`.
@@ -130,6 +134,9 @@ class Database(object):
 failure.
 
 self._db = None
+if mailstore == None:
+mailstore = Mailstore('maildir')
+
 if path is None:
 # no path specified. use a user's default database
 if Database._std_db_path is None:
@@ -138,16 +145,16 @@ class Database(object):
 path = Database._std_db_path
 
 if create == False:
-self.open(path, mode)
+self.open(mailstore, path, mode)
 else:
-self.create(path)
+self.create(mailstore, path)
 
 def _assert_db_is_initialized(self):
 Raises :exc:`NotInitializedError` if self._db is `None`
 if self._db is None:
 raise NotInitializedError()
 
-def create(self, path):
+def create(self, mailstore, path):
 Creates a new notmuch database
 
 This function is used by __init__() and usually does not need
@@ -167,14 +174,15 @@ class Database(object):
 raise NotmuchError(message=Cannot create db, this Database() 
already has an open one.)
 
-res = Database._create(_str(path), Database.MODE.READ_WRITE)
+mailstore = mailstore._mailstore
+res = Database._create(mailstore, _str(path), Database.MODE.READ_WRITE)
 
 if not res:
 raise NotmuchError(
 message=Could not create the specified database)
 self._db = res
 
-def open(self, path, mode=0):
+def open(self, mailstore, path, mode=0):
 Opens an existing database
 
 This function is used by __init__() and usually does not need
@@ -187,7 +195,8 @@ class Database(object):
 :exception: Raises :exc:`NotmuchError` in case of any failure
 (possibly after printing an error message on stderr).
 
-res = Database._open(_str(path), mode)
+mailstore = mailstore._mailstore # unwrap mailstore
+res = Database._open(mailstore, _str(path), mode)
 
 if not res:
 raise NotmuchError(message=Could not open the specified database)
diff --git a/bindings/python/notmuch/globals.py 
b/bindings/python/notmuch/globals.py
index 4138460..5f01dce 100644
--- a/bindings/python/notmuch/globals.py
+++ b/bindings/python/notmuch/globals.py
@@ -228,6 +228,9 @@ class NotmuchDatabaseS(Structure):
 pass
 NotmuchDatabaseP = POINTER(NotmuchDatabaseS)
 
+class NotmuchMailstoreS(Structure):
+pass
+NotmuchMailstoreP = POINTER(NotmuchMailstoreS)
 
 class NotmuchQueryS(Structure):
 pass
diff --git a/bindings/python/notmuch/mailstore.py 
b/bindings

[RFC PATCH 04/13] Replace remaining places where fopen occurs

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

Because mail might no longer be on disk, other uses of fopen(2) need
to be replaced with calls to notmuch_mailstore_open. This isn't all of
them, but these are the ones that involve touching the API in a lot of
different places.

This commit updates mime_node_open, show_message_body, and a couple
random calls in the commands notmuch show, notmuch reply,

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 mime-node.c  |3 ++-
 notmuch-client.h |2 ++
 notmuch-reply.c  |2 +-
 notmuch-show.c   |9 +
 show-message.c   |3 ++-
 5 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index d6b4506..856fc3b 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -61,6 +61,7 @@ _mime_node_context_free (mime_node_context_t *res)
 
 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
+   notmuch_mailstore_t *mailstore,
 #ifdef GMIME_ATLEAST_26
GMimeCryptoContext *cryptoctx,
 #else
@@ -89,7 +90,7 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
 }
 talloc_set_destructor (mctx, _mime_node_context_free);
 
-mctx-file = fopen (filename, r);
+mctx-file = notmuch_mailstore_open (mailstore, filename);
 if (! mctx-file) {
fprintf (stderr, Error opening %s: %s\n, filename, strerror (errno));
status = NOTMUCH_STATUS_FILE_ERROR;
diff --git a/notmuch-client.h b/notmuch-client.h
index c1c30a2..405aad7 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -188,6 +188,7 @@ query_string_from_args (void *ctx, int argc, char *argv[]);
 
 notmuch_status_t
 show_message_body (notmuch_message_t *message,
+  notmuch_mailstore_t *mailstore,
   const notmuch_show_format_t *format,
   notmuch_show_params_t *params);
 
@@ -372,6 +373,7 @@ typedef struct mime_node {
  */
 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
+   notmuch_mailstore_t *mailstore,
 #ifdef GMIME_ATLEAST_26
GMimeCryptoContext *cryptoctx,
 #else
diff --git a/notmuch-reply.c b/notmuch-reply.c
index cb1dd6e..523e2d0 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -587,7 +587,7 @@ notmuch_reply_format_default(void *ctx,
notmuch_message_get_header (mailstore, message, date),
notmuch_message_get_header (mailstore, message, from));
 
-   show_message_body (message, format, params);
+   show_message_body (message, mailstore, format, params);
 
notmuch_message_destroy (message);
 }
diff --git a/notmuch-show.c b/notmuch-show.c
index 81d4cf0..0d2a246 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -285,7 +285,7 @@ format_message_mbox (const void *ctx,
 ssize_t line_len;
 
 filename = notmuch_message_get_filename (message);
-file = fopen (filename, r);
+file = notmuch_mailstore_open (mailstore, filename);
 if (file == NULL) {
fprintf (stderr, Failed to open %s: %s\n,
 filename, strerror (errno));
@@ -830,7 +830,8 @@ show_message (void *ctx,
void *local = talloc_new (ctx);
mime_node_t *root, *part;
 
-   if (mime_node_open (local, message, params-cryptoctx, params-decrypt,
+   if (mime_node_open (local, message, mailstore,
+   params-cryptoctx, params-decrypt,
root) == NOTMUCH_STATUS_SUCCESS 
(part = mime_node_seek_dfs (root, (params-part  0 ?
   0 : params-part
@@ -853,7 +854,7 @@ show_message (void *ctx,
 }
 
 if (format-part_content)
-   show_message_body (message, format, params);
+   show_message_body (message, mailstore, format, params);
 
 if (params-part = 0) {
fputs (format-body_end, stdout);
@@ -955,7 +956,7 @@ do_show_single (void *ctx,
return 1;
}
 
-   file = fopen (filename, r);
+   file = notmuch_mailstore_open (mailstore, filename);
if (file == NULL) {
fprintf (stderr, Error: Cannot open file %s: %s\n, filename, 
strerror (errno));
return 1;
diff --git a/show-message.c b/show-message.c
index 83ecf81..aed9d3e 100644
--- a/show-message.c
+++ b/show-message.c
@@ -80,6 +80,7 @@ show_message_part (mime_node_t *node,
 
 notmuch_status_t
 show_message_body (notmuch_message_t *message,
+  notmuch_mailstore_t *mailstore,
   const notmuch_show_format_t *format,
   notmuch_show_params_t *params)
 {
@@ -87,7 +88,7 @@ show_message_body (notmuch_message_t *message,
 show_message_state_t state;
 mime_node_t *root, *part;
 
-ret = mime_node_open (NULL, message, params-cryptoctx, params-decrypt,
+ret = mime_node_open (NULL, message, mailstore, params-cryptoctx, 
params-decrypt,
  root);
 if (ret)
return ret;
-- 
1.7.5.4

[RFC PATCH 05/13] notmuch_database_add_message calculates sha1 of messages using mailstore

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

Previously, notmuch_database_add_message used the notmuch_sha1_of_file
function, which accesses a mail file directly. Create a new function
called notmuch_sha1_of_message which uses a mailstore to access the
file, and use that instead.

Also, as a drive-by cleanup, use the named constant BLOCK_SIZE instead
of the integer literal 4096.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/database.cc   |2 +-
 lib/notmuch-private.h |3 ++
 lib/sha1.c|   52 
 3 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index e3c8095..ff44e76 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1700,7 +1700,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
if (message_id == NULL ) {
/* No message-id at all, let's generate one by taking a
 * hash over the file's contents. */
-   char *sha1 = notmuch_sha1_of_file (filename);
+   char *sha1 = notmuch_sha1_of_message (notmuch-mailstore, filename);
 
/* If that failed too, something is really wrong. Give up. */
if (sha1 == NULL) {
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 0f01437..2589928 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -454,6 +454,9 @@ notmuch_sha1_of_string (const char *str);
 char *
 notmuch_sha1_of_file (const char *filename);
 
+char *
+notmuch_sha1_of_message (notmuch_mailstore_t *mailstore, const char *filename);
+
 /* string-list.c */
 
 typedef struct _notmuch_string_node {
diff --git a/lib/sha1.c b/lib/sha1.c
index cc48108..ea25999 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -64,19 +64,11 @@ notmuch_sha1_of_string (const char *str)
 return _hex_of_sha1_digest (digest);
 }
 
-/* Create a hexadecimal string version of the SHA-1 digest of the
- * contents of the named file.
- *
- * This function returns a newly allocated string which the caller
- * should free() when finished.
- *
- * If any error occurs while reading the file, (permission denied,
- * file not found, etc.), this function returns NULL.
+/* Internal function to feed the contents of a FILE * to the sha1 functions.
  */
-char *
-notmuch_sha1_of_file (const char *filename)
+static char *
+_notmuch_sha1_of_filep (FILE *file)
 {
-FILE *file;
 #define BLOCK_SIZE 4096
 unsigned char block[BLOCK_SIZE];
 size_t bytes_read;
@@ -84,14 +76,13 @@ notmuch_sha1_of_file (const char *filename)
 unsigned char digest[SHA1_DIGEST_SIZE];
 char *result;
 
-file = fopen (filename, r);
 if (file == NULL)
return NULL;
 
 sha1_begin (sha1);
 
 while (1) {
-   bytes_read = fread (block, 1, 4096, file);
+   bytes_read = fread (block, 1, BLOCK_SIZE, file);
if (bytes_read == 0) {
if (feof (file)) {
break;
@@ -113,3 +104,38 @@ notmuch_sha1_of_file (const char *filename)
 return result;
 }
 
+/* Create a hexadecimal string version of the SHA-1 digest of the
+ * contents of the named file.
+ *
+ * This function returns a newly allocated string which the caller
+ * should free() when finished.
+ *
+ * If any error occurs while reading the file, (permission denied,
+ * file not found, etc.), this function returns NULL.
+ */
+char *
+notmuch_sha1_of_file (const char *filename)
+{
+FILE *file;
+
+file = fopen (filename, r);
+return _notmuch_sha1_of_filep (file);
+}
+
+/* Create a hexadecimal string version of the SHA-1 digest of the
+ * contents of the named message, which is accessed via a mailstore.
+ *
+ * This function returns a newly allocated string which the caller
+ * should free() when finished.
+ *
+ * If any error occurs while reading the file, (permission denied,
+ * file not found, etc.), this function returns NULL.
+ */
+char *
+notmuch_sha1_of_message (notmuch_mailstore_t *mailstore, const char *filename)
+{
+FILE *file;
+
+file = notmuch_mailstore_open (mailstore, filename);
+return _notmuch_sha1_of_filep (file);
+}
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 06/13] Pass mailstore to _notmuch_message_index_file

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

This is the last place where fopen(2) was used and had to be replaced.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/database.cc   |2 +-
 lib/index.cc  |5 +++--
 lib/notmuch-private.h |3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index ff44e76..0ed4412 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1743,7 +1743,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
date = notmuch_message_file_get_header (message_file, date);
_notmuch_message_set_header_values (message, date, from, subject);
 
-   _notmuch_message_index_file (message, filename);
+   _notmuch_message_index_file (notmuch-mailstore, message, filename);
} else {
ret = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
}
diff --git a/lib/index.cc b/lib/index.cc
index d8f8b2b..54b8f73 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -409,7 +409,8 @@ _index_mime_part (notmuch_message_t *message,
 }
 
 notmuch_status_t
-_notmuch_message_index_file (notmuch_message_t *message,
+_notmuch_message_index_file (notmuch_mailstore_t *mailstore,
+notmuch_message_t *message,
 const char *filename)
 {
 GMimeStream *stream = NULL;
@@ -426,7 +427,7 @@ _notmuch_message_index_file (notmuch_message_t *message,
initialized = 1;
 }
 
-file = fopen (filename, r);
+file = notmuch_mailstore_open (mailstore, filename);
 if (! file) {
fprintf (stderr, Error opening %s: %s\n, filename, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 2589928..d93891e 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -313,7 +313,8 @@ notmuch_message_get_author (notmuch_message_t *message);
 /* index.cc */
 
 notmuch_status_t
-_notmuch_message_index_file (notmuch_message_t *message,
+_notmuch_message_index_file (notmuch_mailstore_t *mailstore,
+notmuch_message_t *message,
 const char *filename);
 
 /* message-file.c */
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 07/13] notmuch-new: pull out useful bits of add_files_recursive

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

This is part of notmuch-new refactor phase 1: make add_files stuff
safe for other backends. add_files_recursive is essentially a
maildir-crawling function that periodically adds files to the database
or adds filenames to remove_files or remove_directory lists. I don't
see an easy way to adapt add_files_recursive for other backends who
might not have concepts of directories with other directories inside
of them, so instead just provide an add_files method for each backend.

This patch pulls some bits out of add_files_recursive which will be
useful for other backends: two reporting functions
_report_before_adding_file and _report_added_file, as well as
_add_message, which actually does the message adding.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 notmuch-new.c |  193 +++--
 1 files changed, 120 insertions(+), 73 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 355dde4..dbdfbb6 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -181,6 +181,123 @@ _entries_resemble_maildir (struct dirent **entries, int 
count)
 return 0;
 }
 
+/* Progress-reporting function.
+ *
+ * Can be used by any mailstore-crawling function that wants to alert
+ * users what message it's about to add. Subsequent errors will be due
+ * to this message ;)
+ */
+static void
+_report_before_adding_file (add_files_state_t *state, const char *filename)
+{
+state-processed_files++;
+
+if (state-verbose) {
+   if (state-output_is_a_tty)
+   printf(\r\033[K);
+
+   printf (%i/%i: %s,
+   state-processed_files,
+   state-total_files,
+   filename);
+
+   putchar((state-output_is_a_tty) ? '\r' : '\n');
+   fflush (stdout);
+}
+}
+
+/* Progress-reporting function.
+ *
+ * Call this to respond to the signal handler for SIGALRM.
+ */
+static void
+_report_added_file (add_files_state_t *state)
+{
+if (do_print_progress) {
+   do_print_progress = 0;
+   generic_print_progress (Processed, files, state-tv_start,
+   state-processed_files, state-total_files);
+}
+}
+
+
+/* Atomically handles adding a message to the database.
+ *
+ * Should be used by any mailstore-crawling function that finds a new
+ * message to add.
+ */
+static notmuch_status_t
+_add_message (add_files_state_t *state, notmuch_database_t *notmuch,
+ const char *filename)
+{
+notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
+notmuch_message_t *message;
+const char **tag;
+
+status = notmuch_database_begin_atomic (notmuch);
+if (status) {
+   ret = status;
+   goto DONE;
+}
+
+status = notmuch_database_add_message (notmuch, filename, message);
+
+switch (status) {
+/* success */
+case NOTMUCH_STATUS_SUCCESS:
+   state-added_messages++;
+   notmuch_message_freeze (message);
+   for (tag=state-new_tags; *tag != NULL; tag++)
+   notmuch_message_add_tag (message, *tag);
+   if (state-synchronize_flags == TRUE)
+   notmuch_message_maildir_flags_to_tags (message);
+   notmuch_message_thaw (message);
+   break;
+/* Non-fatal issues (go on to next file) */
+case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
+   if (state-synchronize_flags == TRUE)
+   notmuch_message_maildir_flags_to_tags (message);
+   break;
+case NOTMUCH_STATUS_FILE_NOT_EMAIL:
+   fprintf (stderr, Note: Ignoring non-mail file: %s\n,
+filename);
+   break;
+/* Fatal issues. Don't process anymore. */
+case NOTMUCH_STATUS_READ_ONLY_DATABASE:
+case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
+case NOTMUCH_STATUS_OUT_OF_MEMORY:
+   fprintf (stderr, Error: %s. Halting processing.\n,
+notmuch_status_to_string (status));
+   ret = status;
+   goto DONE;
+default:
+case NOTMUCH_STATUS_FILE_ERROR:
+case NOTMUCH_STATUS_NULL_POINTER:
+case NOTMUCH_STATUS_TAG_TOO_LONG:
+case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:
+case NOTMUCH_STATUS_UNBALANCED_ATOMIC:
+case NOTMUCH_STATUS_LAST_STATUS:
+   INTERNAL_ERROR (add_message returned unexpected value: %d,  status);
+   ret = status;
+   goto DONE;
+}
+
+status = notmuch_database_end_atomic (notmuch);
+if (status) {
+   ret = status;
+   goto DONE;
+}
+
+  DONE:
+if (message) {
+   notmuch_message_destroy (message);
+   message = NULL;
+}
+
+return ret;
+}
+
+
 /* Examine 'path' recursively as follows:
  *
  *   o Ask the filesystem for the mtime of 'path' (fs_mtime)
@@ -232,7 +349,6 @@ add_files_recursive (notmuch_database_t *notmuch,
 char *next = NULL;
 time_t fs_mtime, db_mtime;
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
-notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
 int i, num_fs_entries;
 notmuch_directory_t

[RFC PATCH 08/13] count_files and add_files shall be generic

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

Rename current count_files and add_files to maildir_count_files and
maildir_add_files. This allows the possibility, at least, of having
other backends.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 notmuch-new.c |   62 +---
 1 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index dbdfbb6..d30fba1 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -340,9 +340,9 @@ _add_message (add_files_state_t *state, notmuch_database_t 
*notmuch,
  * if fs_mtime isn't the current wall-clock time.
  */
 static notmuch_status_t
-add_files_recursive (notmuch_database_t *notmuch,
-const char *path,
-add_files_state_t *state)
+maildir_add_files_recursive (notmuch_database_t *notmuch,
+const char *path,
+add_files_state_t *state)
 {
 DIR *dir = NULL;
 struct dirent *entry = NULL;
@@ -449,7 +449,7 @@ add_files_recursive (notmuch_database_t *notmuch,
}
 
next = talloc_asprintf (notmuch, %s/%s, path, entry-d_name);
-   status = add_files_recursive (notmuch, next, state);
+   status = maildir_add_files_recursive (notmuch, next, state);
if (status  ret == NOTMUCH_STATUS_SUCCESS)
ret = status;
talloc_free (next);
@@ -663,13 +663,34 @@ stop_progress_printing_timer (void)
 sigaction (SIGALRM, action, NULL);
 }
 
+static notmuch_status_t
+maildir_add_files (notmuch_database_t *notmuch,
+  const char *path,
+  add_files_state_t *state);
+
+/* Dispatch function to call the correct mailstore_add_files
+ * function. */
+static notmuch_status_t
+add_files (notmuch_database_t *notmuch, notmuch_config_t *config,
+  add_files_state_t *state)
+{
+const char *path = notmuch_config_get_database_path (config);
+if (strcmp (notmuch_config_get_database_type (config), maildir) == 0)
+   return maildir_add_files (notmuch, path, state);
+
+/* Default case */
+fprintf (stderr, Could not add files for mailstore %s: unknown 
mailstore\n,
+notmuch_config_get_database_type (config));
+/* FIXME: invalid argument error code would be nice */
+return NOTMUCH_STATUS_FILE_ERROR;
+}
 
 /* This is the top-level entry point for add_files. It does a couple
  * of error checks and then calls into the recursive function. */
 static notmuch_status_t
-add_files (notmuch_database_t *notmuch,
-  const char *path,
-  add_files_state_t *state)
+maildir_add_files (notmuch_database_t *notmuch,
+  const char *path,
+  add_files_state_t *state)
 {
 notmuch_status_t status;
 struct stat st;
@@ -685,11 +706,28 @@ add_files (notmuch_database_t *notmuch,
return NOTMUCH_STATUS_FILE_ERROR;
 }
 
-status = add_files_recursive (notmuch, path, state);
+status = maildir_add_files_recursive (notmuch, path, state);
 
 return status;
 }
 
+static void
+maildir_count_files (const char *path, int *count);
+
+/* Dispatch function to call the correct mailstore_count_files
+ * function. N.B. This function may get refactored into add_files! */
+static void
+count_files (unused (notmuch_database_t *notmuch), notmuch_config_t *config, 
int *count)
+{
+if (strcmp (notmuch_config_get_database_type (config), maildir) == 0) {
+   maildir_count_files (notmuch_config_get_database_path (config), count);
+   return;
+}
+
+/* Eh, screw it, this function shouldn't even exist */
+*count = 0;
+}
+
 /* XXX: This should be merged with the add_files function since it
  * shares a lot of logic with it. */
 /* Recursively count all regular files in path and all sub-directories
@@ -697,7 +735,7 @@ add_files (notmuch_database_t *notmuch,
  * initialized to zero by the top-level caller before calling
  * count_files). */
 static void
-count_files (const char *path, int *count)
+maildir_count_files (const char *path, int *count)
 {
 struct dirent *entry = NULL;
 char *next;
@@ -746,7 +784,7 @@ count_files (const char *path, int *count)
fflush (stdout);
}
} else if (S_ISDIR (st.st_mode)) {
-   count_files (next, count);
+   maildir_count_files (next, count);
}
 
free (next);
@@ -905,7 +943,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
int count;
 
count = 0;
-   count_files (db_path, count);
+   count_files (notmuch, config, count);
if (interrupted)
return 1;
 
@@ -962,7 +1000,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
timer_is_active = TRUE;
 }
 
-ret = add_files (notmuch, db_path, add_files_state);
+ret = add_files (notmuch, config, add_files_state);
 
 gettimeofday (tv_start, NULL);
 for (f = add_files_state.removed_files-head; f

[RFC PATCH 09/13] Mailstore also provides a rename function

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

This is used only in notmuch_message_tags_to_maildir_flags, to update
a message's filename when its tags change. For mailstores where this
doesn't make sense, they can of course define rename to be a NOOP.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/mailstore.c   |   21 -
 lib/message.cc|5 +++--
 lib/notmuch.h |   10 +-
 notmuch-restore.c |7 +--
 notmuch-tag.c |7 +--
 5 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/lib/mailstore.c b/lib/mailstore.c
index 290da70..2c6beab 100644
--- a/lib/mailstore.c
+++ b/lib/mailstore.c
@@ -22,6 +22,8 @@
 
 typedef struct _notmuch_mailstore {
 FILE *(*open) (struct _notmuch_mailstore *mailstore, const char *filename);
+int (*rename) (struct _notmuch_mailstore *mailstore, const char 
*old_filename,
+  const char *new_filename);
 } _notmuch_mailstore;
 
 static FILE *
@@ -31,17 +33,27 @@ _maildir_open_function (unused (notmuch_mailstore_t 
*mailstore),
 return fopen (filename, r);
 }
 
+static int
+_maildir_rename_function (unused (notmuch_mailstore_t *mailstore),
+ const char *old_filename, const char *new_filename)
+{
+return rename (old_filename, new_filename);
+}
+
 /* A mailstore is defined as:
  *
  * - A function used to open a mail message. This takes the
  *   filename for the file and should return a FILE *.
  *
+ * - A function to rename a mail message, which is currently only
+ *   used in tags_to_maildir_flags.
+ *
  * - TODO: A way to scan for new messages?
  *
  * - TODO: A constructor?
  */
 _notmuch_mailstore
-notmuch_mailstore_maildir = { _maildir_open_function };
+notmuch_mailstore_maildir = { _maildir_open_function, _maildir_rename_function 
};
 
 _notmuch_mailstore *
 notmuch_mailstore_get_by_name (const char *name)
@@ -57,3 +69,10 @@ notmuch_mailstore_open (notmuch_mailstore_t *mailstore, 
const char *filename)
 {
 return mailstore-open (mailstore, filename);
 }
+
+int
+notmuch_mailstore_rename (notmuch_mailstore_t *mailstore, const char 
*old_filename,
+ const char *new_filename)
+{
+return mailstore-rename (mailstore, old_filename, new_filename);
+}
diff --git a/lib/message.cc b/lib/message.cc
index 762a18f..cd81f6b 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1291,7 +1291,8 @@ _new_maildir_filename (void *ctx,
 }
 
 notmuch_status_t
-notmuch_message_tags_to_maildir_flags (notmuch_message_t *message)
+notmuch_message_tags_to_maildir_flags (notmuch_mailstore_t *mailstore,
+  notmuch_message_t *message)
 {
 notmuch_filenames_t *filenames;
 const char *filename;
@@ -1319,7 +1320,7 @@ notmuch_message_tags_to_maildir_flags (notmuch_message_t 
*message)
int err;
notmuch_status_t new_status;
 
-   err = rename (filename, filename_new);
+   err = notmuch_mailstore_rename (mailstore, filename, filename_new);
if (err)
continue;
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7ebe034..b6e66a9 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -423,6 +423,13 @@ notmuch_mailstore_get_by_name (const char *name);
 FILE *
 notmuch_mailstore_open (notmuch_mailstore_t *mailstore, const char *filename);
 
+/* Rename a file. This is used to update maildir tags and can safely
+ * be a NO-OP for non-filesystem mailstores.
+ */
+int
+notmuch_mailstore_rename (notmuch_mailstore_t *mailstore, const char 
*old_filename,
+ const char *new_filename);
+
 /* Create a new query for 'database'.
  *
  * Here, 'database' should be an open database, (see
@@ -1095,7 +1102,8 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t 
*message);
  * for synchronizing maildir flag changes back to tags.
  */
 notmuch_status_t
-notmuch_message_tags_to_maildir_flags (notmuch_message_t *message);
+notmuch_message_tags_to_maildir_flags (notmuch_mailstore_t *mailstore,
+  notmuch_message_t *message);
 
 /* Freeze the current state of 'message' within the database.
  *
diff --git a/notmuch-restore.c b/notmuch-restore.c
index b382b7b..a0beab4 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -25,6 +25,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 {
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
+notmuch_mailstore_t *mailstore;
 notmuch_bool_t synchronize_flags;
 notmuch_bool_t accumulate = FALSE;
 char *input_file_name = NULL;
@@ -40,7 +41,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 if (config == NULL)
return 1;
 
-notmuch = notmuch_database_open (notmuch_config_get_mailstore (config),
+mailstore = notmuch_config_get_mailstore (config);
+
+notmuch = notmuch_database_open (mailstore,
 notmuch_config_get_database_path (config

[RFC PATCH 10/13] Introduce concept of mailstore constructor

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

Right now this is a fancy no-op because maildir doesn't need any
special data, but getting the API right is good. A constructor can
fail, so return a notmuch_status_t.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/mailstore.c  |   36 +---
 lib/notmuch.h|7 +++
 notmuch-config.c |8 +++-
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/lib/mailstore.c b/lib/mailstore.c
index 2c6beab..b4d512d 100644
--- a/lib/mailstore.c
+++ b/lib/mailstore.c
@@ -17,15 +17,25 @@
  */
 
 #include stdio.h
+#include stdarg.h
 
 #include notmuch-private.h
 
 typedef struct _notmuch_mailstore {
+notmuch_status_t (*constructor) (void **data, va_list args);
 FILE *(*open) (struct _notmuch_mailstore *mailstore, const char *filename);
 int (*rename) (struct _notmuch_mailstore *mailstore, const char 
*old_filename,
   const char *new_filename);
+void *data;
 } _notmuch_mailstore;
 
+static notmuch_status_t
+_maildir_constructor (void **data, unused (va_list ap))
+{
+(*data) = NULL;
+return NOTMUCH_STATUS_SUCCESS;
+}
+
 static FILE *
 _maildir_open_function (unused (notmuch_mailstore_t *mailstore),
const char *filename)
@@ -48,12 +58,18 @@ _maildir_rename_function (unused (notmuch_mailstore_t 
*mailstore),
  * - A function to rename a mail message, which is currently only
  *   used in tags_to_maildir_flags.
  *
- * - TODO: A way to scan for new messages?
+ * - A constructor that creates a mailstore object of the requisite
+ *   type. Arguments are passed via va_args.
  *
- * - TODO: A constructor?
+ * A mailstore also has a data field that can be used to store
+ * instance-specific information about this mailstore -- for example,
+ * a CouchDB URL or a path. FIXME: mailstores are all statically
+ * allocated, so maybe we shouldn't do this.
  */
 _notmuch_mailstore
-notmuch_mailstore_maildir = { _maildir_open_function, _maildir_rename_function 
};
+notmuch_mailstore_maildir = { _maildir_constructor,
+ _maildir_open_function, _maildir_rename_function,
+ NULL };
 
 _notmuch_mailstore *
 notmuch_mailstore_get_by_name (const char *name)
@@ -76,3 +92,17 @@ notmuch_mailstore_rename (notmuch_mailstore_t *mailstore, 
const char *old_filena
 {
 return mailstore-rename (mailstore, old_filename, new_filename);
 }
+
+notmuch_status_t
+notmuch_mailstore_construct (notmuch_mailstore_t *mailstore, ...)
+{
+va_list va_args;
+notmuch_status_t status;
+
+va_start (va_args, mailstore);
+
+status = mailstore-constructor (mailstore-data, va_args);
+
+va_end (va_args);
+return status;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index b6e66a9..7f48507 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -430,6 +430,13 @@ int
 notmuch_mailstore_rename (notmuch_mailstore_t *mailstore, const char 
*old_filename,
  const char *new_filename);
 
+/* Initialize the mailstore.
+ *
+ * Arguments are dependent on the mailstore.
+ */
+notmuch_status_t
+notmuch_mailstore_construct (notmuch_mailstore_t *mailstore, ...);
+
 /* Create a new query for 'database'.
  *
  * Here, 'database' should be an open database, (see
diff --git a/notmuch-config.c b/notmuch-config.c
index f611b26..99f872d 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -592,8 +592,14 @@ notmuch_config_get_mailstore (notmuch_config_t *config)
  * When there are multiple mailstore types and constructors for
  * them, this may have to be much more complicated.
  */
+notmuch_status_t status;
 const char *type = notmuch_config_get_database_type (config);
-return notmuch_mailstore_get_by_name (type);
+notmuch_mailstore_t *mailstore = notmuch_mailstore_get_by_name (type);
+status = notmuch_mailstore_construct (mailstore);
+if (status != NOTMUCH_STATUS_SUCCESS) {
+   /* abort messily? */
+}
+return mailstore;
 }
 
 const char *
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 11/13] Add a close function to mailstore

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

This is a useful way to signal to mailstores that the resources
associated with an existing FILE* are no longer being used and they
can be cleaned up. For maildir, of course, this is just a call to
fclose(), but for other mailstores this might involve freeing memory
or unlinking temporary files...

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/mailstore.c |   17 -
 lib/notmuch.h   |5 +
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/lib/mailstore.c b/lib/mailstore.c
index b4d512d..51c2710 100644
--- a/lib/mailstore.c
+++ b/lib/mailstore.c
@@ -24,6 +24,7 @@
 typedef struct _notmuch_mailstore {
 notmuch_status_t (*constructor) (void **data, va_list args);
 FILE *(*open) (struct _notmuch_mailstore *mailstore, const char *filename);
+int (*close) (struct _notmuch_mailstore *mailstore, FILE *);
 int (*rename) (struct _notmuch_mailstore *mailstore, const char 
*old_filename,
   const char *new_filename);
 void *data;
@@ -44,6 +45,13 @@ _maildir_open_function (unused (notmuch_mailstore_t 
*mailstore),
 }
 
 static int
+_maildir_close_function (unused (notmuch_mailstore_t *mailstore),
+FILE *file)
+{
+return fclose (file);
+}
+
+static int
 _maildir_rename_function (unused (notmuch_mailstore_t *mailstore),
  const char *old_filename, const char *new_filename)
 {
@@ -68,7 +76,8 @@ _maildir_rename_function (unused (notmuch_mailstore_t 
*mailstore),
  */
 _notmuch_mailstore
 notmuch_mailstore_maildir = { _maildir_constructor,
- _maildir_open_function, _maildir_rename_function,
+ _maildir_open_function, _maildir_close_function,
+ _maildir_rename_function,
  NULL };
 
 _notmuch_mailstore *
@@ -93,6 +102,12 @@ notmuch_mailstore_rename (notmuch_mailstore_t *mailstore, 
const char *old_filena
 return mailstore-rename (mailstore, old_filename, new_filename);
 }
 
+int
+notmuch_mailstore_close (notmuch_mailstore_t *mailstore, FILE *file)
+{
+return mailstore-close (mailstore, file);
+}
+
 notmuch_status_t
 notmuch_mailstore_construct (notmuch_mailstore_t *mailstore, ...)
 {
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7f48507..d760d4f 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -423,6 +423,11 @@ notmuch_mailstore_get_by_name (const char *name);
 FILE *
 notmuch_mailstore_open (notmuch_mailstore_t *mailstore, const char *filename);
 
+/* Signal that a filename is no longer being used; get rid of its resources.
+ */
+int
+notmuch_mailstore_close (notmuch_mailstore_t *mailstore, FILE *file);
+
 /* Rename a file. This is used to update maildir tags and can safely
  * be a NO-OP for non-filesystem mailstores.
  */
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 12/13] Close files using notmuch_mailstore_close instead of fclose

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

This requires a little bit of juggling in lib/sha1.c. Wrapper
functions provide the FILE*. Instead of closing the file immediately
ourselves, we let the wrapper functions close it.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/message-file.c |6 --
 lib/sha1.c |   14 --
 mime-node.c|4 +++-
 notmuch-show.c |8 
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/lib/message-file.c b/lib/message-file.c
index 61f4d04..51c77f9 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -34,6 +34,7 @@ typedef struct {
 
 struct _notmuch_message_file {
 /* File object */
+notmuch_mailstore_t *mailstore;
 FILE *file;
 
 /* Header storage */
@@ -86,7 +87,7 @@ _notmuch_message_file_destructor (notmuch_message_file_t 
*message)
g_hash_table_destroy (message-headers);
 
 if (message-file)
-   fclose (message-file);
+   notmuch_mailstore_close (message-mailstore, message-file);
 
 return 0;
 }
@@ -105,6 +106,7 @@ _notmuch_message_file_open_ctx (void *ctx, 
notmuch_mailstore_t *mailstore,
 
 talloc_set_destructor (message, _notmuch_message_file_destructor);
 
+message-mailstore = mailstore;
 message-file = notmuch_mailstore_open (mailstore, filename);
 if (message-file == NULL)
goto FAIL;
@@ -363,7 +365,7 @@ notmuch_message_file_get_header (notmuch_message_file_t 
*message,
 }
 
 if (message-parsing_finished) {
-fclose (message-file);
+   notmuch_mailstore_close (message-mailstore, message-file);
 message-file = NULL;
 }
 
diff --git a/lib/sha1.c b/lib/sha1.c
index ea25999..4266720 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -87,7 +87,6 @@ _notmuch_sha1_of_filep (FILE *file)
if (feof (file)) {
break;
} else if (ferror (file)) {
-   fclose (file);
return NULL;
}
} else {
@@ -99,8 +98,6 @@ _notmuch_sha1_of_filep (FILE *file)
 
 result = _hex_of_sha1_digest (digest);
 
-fclose (file);
-
 return result;
 }
 
@@ -117,9 +114,12 @@ char *
 notmuch_sha1_of_file (const char *filename)
 {
 FILE *file;
+char *ret;
 
 file = fopen (filename, r);
-return _notmuch_sha1_of_filep (file);
+ret = _notmuch_sha1_of_filep (file);
+fclose (file);
+return ret;
 }
 
 /* Create a hexadecimal string version of the SHA-1 digest of the
@@ -135,7 +135,9 @@ char *
 notmuch_sha1_of_message (notmuch_mailstore_t *mailstore, const char *filename)
 {
 FILE *file;
-
+char *ret;
 file = notmuch_mailstore_open (mailstore, filename);
-return _notmuch_sha1_of_filep (file);
+ret = _notmuch_sha1_of_filep (file);
+notmuch_mailstore_close (mailstore, file);
+return ret;
 }
diff --git a/mime-node.c b/mime-node.c
index 856fc3b..db37189 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -27,6 +27,7 @@
 typedef struct mime_node_context {
 /* Per-message resources.  These are allocated internally and must
  * be destroyed. */
+notmuch_mailstore_t *mailstore;
 FILE *file;
 GMimeStream *stream;
 GMimeParser *parser;
@@ -54,7 +55,7 @@ _mime_node_context_free (mime_node_context_t *res)
g_object_unref (res-stream);
 
 if (res-file)
-   fclose (res-file);
+   notmuch_mailstore_close (res-mailstore, res-file);
 
 return 0;
 }
@@ -90,6 +91,7 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
 }
 talloc_set_destructor (mctx, _mime_node_context_free);
 
+mctx-mailstore = mailstore;
 mctx-file = notmuch_mailstore_open (mailstore, filename);
 if (! mctx-file) {
fprintf (stderr, Error opening %s: %s\n, filename, strerror (errno));
diff --git a/notmuch-show.c b/notmuch-show.c
index 0d2a246..2e8e4c9 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -309,7 +309,7 @@ format_message_mbox (const void *ctx,
 
 printf (\n);
 
-fclose (file);
+notmuch_mailstore_close (mailstore, file);
 }
 
 static void
@@ -966,18 +966,18 @@ do_show_single (void *ctx,
size = fread (buf, 1, sizeof (buf), file);
if (ferror (file)) {
fprintf (stderr, Error: Read failed from %s\n, filename);
-   fclose (file);
+   notmuch_mailstore_close (mailstore, file);
return 1;
}
 
if (fwrite (buf, size, 1, stdout) != 1) {
fprintf (stderr, Error: Write failed\n);
-   fclose (file);
+   notmuch_mailstore_close (mailstore, file);
return 1;
}
}
 
-   fclose (file);
+   notmuch_mailstore_close (mailstore, file);
 
 } else {
 
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 13/13] First crack at a CouchDB mailstore

2012-02-15 Thread Ethan Glasser-Camp
From: Ethan Glasser-Camp et...@betacantrips.com

This introduces new parameters to notmuch-config to store the CouchDB
URL and the name of the database.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 Makefile.local   |3 +
 lib/mailstore.c  |  109 
 notmuch-client.h |   14 
 notmuch-config.c |   91 +-
 notmuch-new.c|  184 ++
 5 files changed, 397 insertions(+), 4 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index 1131dea..a105e58 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -27,6 +27,9 @@ endif
 
 UPSTREAM_TAG=$(subst ~,_,$(VERSION))
 DEB_TAG=debian/$(UPSTREAM_TAG)-1
+# FIXME: Where should this really go?
+LDFLAGS += $(shell pkg-config --libs couchdb-glib-1.0 libsoup-2.4)
+extra_cflags += $(shell pkg-config --cflags couchdb-glib-1.0 libsoup-2.4)
 
 RELEASE_HOST=notmuchmail.org
 RELEASE_DIR=/srv/notmuchmail.org/www/releases
diff --git a/lib/mailstore.c b/lib/mailstore.c
index 51c2710..4d7cc79 100644
--- a/lib/mailstore.c
+++ b/lib/mailstore.c
@@ -18,6 +18,10 @@
 
 #include stdio.h
 #include stdarg.h
+#include couchdb-session.h
+#include couchdb-database.h
+#include couchdb-document.h
+#include glib.h
 
 #include notmuch-private.h
 
@@ -58,6 +62,101 @@ _maildir_rename_function (unused (notmuch_mailstore_t 
*mailstore),
 return rename (old_filename, new_filename);
 }
 
+struct _couchdb_data {
+char *db_path;
+CouchdbDatabase *database;
+GHashTable *files_to_documents;
+};
+
+/* CouchDB mailstore */
+static notmuch_status_t
+_couchdb_constructor (void **data, va_list ap)
+{
+CouchdbSession *session = NULL;
+CouchdbDatabase *database = NULL;
+GError *error = NULL;
+char *uri = NULL;
+char *db_name = NULL;
+struct _couchdb_data *my_data = NULL;
+
+uri = va_arg (ap, char*);
+session = couchdb_session_new (uri);
+
+db_name = va_arg (ap, char*);
+database = couchdb_session_get_database (session, db_name, error);
+if (database == NULL) {
+   fprintf (stderr, Couldn't access database %s: %s\n, db_name,
+error-message);
+   return NOTMUCH_STATUS_FILE_ERROR;
+}
+
+my_data = talloc_size (NULL, sizeof (struct _couchdb_data));
+my_data-database = database;
+my_data-db_path  = va_arg (ap, char*);
+my_data-files_to_documents = g_hash_table_new (NULL, NULL);
+(*data) = (void*)my_data;
+
+return NOTMUCH_STATUS_SUCCESS;
+}
+
+static FILE *
+_couchdb_open_function (notmuch_mailstore_t *mailstore,
+   const char *filename)
+{
+CouchdbDatabase *database = NULL;
+CouchdbDocument *document = NULL;
+GError *error = NULL;
+const char *text = NULL;
+const char *relative = NULL;
+struct _couchdb_data *data = (struct _couchdb_data *)mailstore-data;
+FILE *ret = NULL;
+database = data-database;
+/* message assumes all files should be contained within db_path.
+ * This isn't true for us, so remove the db_path.
+ * I'd like to use _notmuch_database_relative_path but I don't have
+ * a notmuch_database_t*.
+ */
+relative = filename;
+if (strncmp (filename, data-db_path, strlen (data-db_path)) == 0) {
+   relative = filename + strlen (data-db_path);
+   while (*relative == '/'  *(relative+1) == '/')
+   relative++;
+}
+
+document = couchdb_database_get_document (database, relative, error);
+if (document == NULL)
+   /* file doesn't exist. Maybe it got deleted? */
+   return NULL;
+
+text = couchdb_document_get_string_field (document, text);
+/* FIXME: null bytes in the mail file? */
+ret = fmemopen ((char *)text, strlen(text), r);
+g_hash_table_insert (data-files_to_documents, ret, document);
+return ret;
+}
+
+static int
+_couchdb_close_function (notmuch_mailstore_t *mailstore, FILE *file)
+{
+struct _couchdb_data *data = (struct _couchdb_data *)mailstore-data;
+GHashTable *hash = data-files_to_documents;
+CouchdbDocument *document;
+document = g_hash_table_lookup (hash, file);
+g_object_unref (document);
+fclose (file); /* just to be polite ;) */
+g_hash_table_remove (hash, file);
+return 0;
+}
+
+static int
+_couchdb_rename_function (unused (notmuch_mailstore_t *mailstore),
+ unused (const char *old_filename),
+ unused (const char *new_filename))
+{
+/* Pass for now. */
+return 0;
+}
+
 /* A mailstore is defined as:
  *
  * - A function used to open a mail message. This takes the
@@ -80,12 +179,22 @@ notmuch_mailstore_maildir = { _maildir_constructor,
  _maildir_rename_function,
  NULL };
 
+_notmuch_mailstore
+notmuch_mailstore_couchdb = { _couchdb_constructor,
+ _couchdb_open_function, _couchdb_close_function,
+ _couchdb_rename_function

Re: [RFC PATCH 00/13] Modular message store code

2012-03-01 Thread Ethan Glasser-Camp

On 02/15/2012 07:56 PM, Mark Walters wrote:

Obviously I have not looked at the patch set in detail yet but I have a
quick question. Since you are allowing more general filenames anyway
couldn't you encode mailstore in filename? Eg
mbox://some-path[:byte-postion], or imap://server...

This would allow lots of different types of mailstore to be used
concurrently, and would push all the mailstore knowledge down into the
file handling functions and away from the callers of file handling
functions.

Of course there may be lots of good reasons why this doesn't work.


Hi, sorry for the delay.

As far as I can tell, currently notmuch stores message filenames in 
Xapian as paths relative to the top-level maildir. I think this is done 
so that the maildir can be moved and, if the .notmuch-config is updated, 
mails are correctly detected and not duplicated. This would be 
especially important when you're talking about changing IMAP servers or 
CouchDB instances.


If I wanted to preserve this feature, the URIs stored as filenames would 
have to be relative to a given mailstore. For example, 
maildir://maildir-1/INBOX/some-filename could mean the file 
INBOX/some-filename in a maildir at /home/user/some-maildir. But then 
this raises the two following issues:


- How does information about mailstores -- for example, that maildir-1 
= /home/user/some-maildir -- enter the library? Do we stick all of that 
information in notmuch_database_t, and then pass a reference to it in 
notmuch_message_file_open? Perhaps a global 
notmuch_mailstore_register(name, parameters..) registry? Or maybe a 
notmuch_mailstore_info type that gets passed around similarly to the 
mailstore type in this patch set?


- Do we mandate that all the filenames in the database be updated or do 
we just assume non-URI-style filenames are relative to some default 
mailstore?


All of which is a fancy way of saying I haven't had the time to write 
the code necessary to explore this idea but think something like it will 
be necessary to support the obviously-valuable feature of multiple 
mailstores. Depending on your answer to the first question, I guess the 
patch series might or might not be a useful starting point.


Thanks for your feedback,

Ethan

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 01/14] All access to mail files goes through the mailstore module

2012-06-26 Thread Ethan Glasser-Camp
This commit introduces the mailstore module which provides two
functions, notmuch_mailstore_open and notmuch_mailstore_close. These
functions are currently just stub calls to fopen and fclose, but later
can be made more complex in order to support mail storage systems
where one message might not be one file.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/Makefile.local|1 +
 lib/database.cc   |2 +-
 lib/index.cc  |2 +-
 lib/mailstore.c   |   34 
 lib/message-file.c|6 ++---
 lib/notmuch-private.h |3 +++
 lib/notmuch.h |   16 +++
 lib/sha1.c|   70 +
 mime-node.c   |4 +--
 notmuch-show.c|   12 -
 10 files changed, 120 insertions(+), 30 deletions(-)
 create mode 100644 lib/mailstore.c

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 8a9aa28..cfc77bb 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -51,6 +51,7 @@ libnotmuch_c_srcs =   \
$(dir)/filenames.c  \
$(dir)/string-list.c\
$(dir)/libsha1.c\
+   $(dir)/mailstore.c  \
$(dir)/message-file.c   \
$(dir)/messages.c   \
$(dir)/sha1.c   \
diff --git a/lib/database.cc b/lib/database.cc
index 761dc1a..c035edc 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1773,7 +1773,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
if (message_id == NULL ) {
/* No message-id at all, let's generate one by taking a
 * hash over the file's contents. */
-   char *sha1 = notmuch_sha1_of_file (filename);
+   char *sha1 = notmuch_sha1_of_message (filename);
 
/* If that failed too, something is really wrong. Give up. */
if (sha1 == NULL) {
diff --git a/lib/index.cc b/lib/index.cc
index e377732..b607e82 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -441,7 +441,7 @@ _notmuch_message_index_file (notmuch_message_t *message,
initialized = 1;
 }
 
-file = fopen (filename, r);
+file = notmuch_mailstore_open (filename);
 if (! file) {
fprintf (stderr, Error opening %s: %s\n, filename, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
diff --git a/lib/mailstore.c b/lib/mailstore.c
new file mode 100644
index 000..48acd47
--- /dev/null
+++ b/lib/mailstore.c
@@ -0,0 +1,34 @@
+/* mailstore.c - code to access individual messages
+ *
+ * Copyright © 2009 Carl Worth
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Carl Worth cwo...@cworth.org
+ */
+#include stdio.h
+
+#include notmuch-private.h
+
+FILE *
+notmuch_mailstore_open (const char *filename)
+{
+return fopen (filename, r);
+}
+
+int
+notmuch_mailstore_close (FILE *file)
+{
+return fclose (file);
+}
diff --git a/lib/message-file.c b/lib/message-file.c
index 915aba8..271389c 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -86,7 +86,7 @@ _notmuch_message_file_destructor (notmuch_message_file_t 
*message)
g_hash_table_destroy (message-headers);
 
 if (message-file)
-   fclose (message-file);
+   notmuch_mailstore_close (message-file);
 
 return 0;
 }
@@ -104,7 +104,7 @@ _notmuch_message_file_open_ctx (void *ctx, const char 
*filename)
 
 talloc_set_destructor (message, _notmuch_message_file_destructor);
 
-message-file = fopen (filename, r);
+message-file = notmuch_mailstore_open (filename);
 if (message-file == NULL)
goto FAIL;
 
@@ -361,7 +361,7 @@ notmuch_message_file_get_header (notmuch_message_file_t 
*message,
 }
 
 if (message-parsing_finished) {
-fclose (message-file);
+notmuch_mailstore_close (message-file);
 message-file = NULL;
 }
 
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index bfb4111..5dbe821 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -468,6 +468,9 @@ notmuch_sha1_of_string (const char *str);
 char *
 notmuch_sha1_of_file (const char *filename);
 
+char *
+notmuch_sha1_of_message (const char *filename);
+
 /* string-list.c */
 
 typedef struct _notmuch_string_node {
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 3633bed..0ca367b 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1233,6 +1233,22 @@ notmuch_message_thaw

[RFC PATCH 02/14] Introduce uriparser

2012-06-26 Thread Ethan Glasser-Camp
Seeing as there is no glib-standard way to parse URIs, an external
library is needed. This commit introduces another program in compat/
and a stanza in ./configure to test if uriparser is there.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 Makefile.local  |2 +-
 compat/have_uriparser.c |   17 +
 configure   |   23 ---
 3 files changed, 38 insertions(+), 4 deletions(-)
 create mode 100644 compat/have_uriparser.c

diff --git a/Makefile.local b/Makefile.local
index a890df2..084f44e 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -41,7 +41,7 @@ PV_FILE=bindings/python/notmuch/version.py
 # Smash together user's values with our extra values
 FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) 
$(CONFIGURE_CFLAGS) $(extra_cflags)
 FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) 
$(extra_cflags) $(extra_cxxflags)
-FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch 
$(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)
+FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch 
$(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) $(URIPARSER_LDFLAGS)
 FINAL_NOTMUCH_LINKER = CC
 ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
 FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS)
diff --git a/compat/have_uriparser.c b/compat/have_uriparser.c
new file mode 100644
index 000..d79e51d
--- /dev/null
+++ b/compat/have_uriparser.c
@@ -0,0 +1,17 @@
+#include uriparser/Uri.h
+
+int
+main (int argc, char *argv[])
+{
+UriParserStateA state;
+UriUriA uri;
+char *uriS = NULL;
+
+state.uri = uri;
+if (uriParseUriA (state, uriS) != URI_SUCCESS) {
+/* Failure */
+uriFreeUriMembersA (uri);
+}
+
+return 0;
+}
diff --git a/configure b/configure
index 3fad424..80aa13c 100755
--- a/configure
+++ b/configure
@@ -313,6 +313,19 @@ else
 errors=$((errors + 1))
 fi
 
+printf Checking for uriparser... 
+if ${CC} -o compat/have_uriparser $srcdir/compat/have_uriparser.c 
-luriparser  /dev/null 21
+then
+printf Yes.\n
+uriparser_ldflags=-luriparser
+have_uriparser=1
+else
+printf No.\n
+have_uriparser=0
+errors=$((errors + 1))
+fi
+rm -f compat/have_uriparser
+
 printf Checking for valgrind development files... 
 if pkg-config --exists valgrind; then
 printf Yes.\n
@@ -431,11 +444,11 @@ case a simple command will install everything you need. 
For example:
 
 On Debian and similar systems:
 
-   sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev
+   sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev 
liburiparser-dev
 
 Or on Fedora and similar systems:
 
-   sudo yum install xapian-core-devel gmime-devel libtalloc-devel
+   sudo yum install xapian-core-devel gmime-devel libtalloc-devel 
uriparser-devel
 
 On other systems, similar commands can be used, but the details of the
 package names may be different.
@@ -669,6 +682,9 @@ GMIME_LDFLAGS = ${gmime_ldflags}
 TALLOC_CFLAGS = ${talloc_cflags}
 TALLOC_LDFLAGS = ${talloc_ldflags}
 
+# Flags needed to link against uriparser
+URIPARSER_LDFLAGS = ${uriparser_ldflags}
+
 # Flags needed to have linker set rpath attribute
 RPATH_LDFLAGS = ${rpath_ldflags}
 
@@ -698,5 +714,6 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) 
\$(GMIME_CFLAGS)\\
 \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
 \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\
  -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR)
-CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
+CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) \\
+ \$(URIPARSER_LDFLAGS)
 EOF
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 03/14] mailstore can read from maildir: URLs

2012-06-26 Thread Ethan Glasser-Camp
No code uses this yet.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/mailstore.c |   37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/lib/mailstore.c b/lib/mailstore.c
index 48acd47..ae02c12 100644
--- a/lib/mailstore.c
+++ b/lib/mailstore.c
@@ -17,14 +17,49 @@
  *
  * Author: Carl Worth cwo...@cworth.org
  */
+#include uriparser/Uri.h
 #include stdio.h
 
 #include notmuch-private.h
 
+static FILE *
+notmuch_mailstore_basic_open (const char *filename)
+{
+return fopen (filename, r);
+}
+
 FILE *
 notmuch_mailstore_open (const char *filename)
 {
-return fopen (filename, r);
+FILE *ret = NULL;
+UriUriA parsed;
+UriParserStateA state;
+state.uri = parsed;
+if (uriParseUriA (state, filename) != URI_SUCCESS) {
+/* Failure. Fall back to fopen and hope for the best. */
+ret = notmuch_mailstore_basic_open (filename);
+goto DONE;
+}
+
+if (parsed.scheme.first == NULL) {
+/* No scheme. Probably not really a URL but just an ordinary filename.
+ * Fall back to fopen for backwards compatibility. */
+ret = notmuch_mailstore_basic_open (filename);
+goto DONE;
+}
+
+if (0 == strncmp (parsed.scheme.first, maildir,
+  parsed.scheme.afterLast-parsed.scheme.first)) {
+/* Maildir URI of the form maildir:///path/to/file.
+ * We want to fopen(/path/to/file).
+ * pathHead starts at path/to/file. */
+ret = notmuch_mailstore_basic_open (parsed.pathHead-text.first - 1);
+goto DONE;
+}
+
+DONE:
+uriFreeUriMembersA (parsed);
+return ret;
 }
 
 int
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 04/14] Not all filenames need to be converted to absolute paths

2012-06-26 Thread Ethan Glasser-Camp
_notmuch_message_ensure_filename_list converts relative paths, such
as those stored in Xapian until now, to absolute paths. However,
URLs are already absolute, and prepending the database path will just
confuse matters.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/message.cc |   14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 978de06..c9857f5 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -700,9 +700,17 @@ _notmuch_message_ensure_filename_list (notmuch_message_t 
*message)
  message-notmuch,
  directory_id);
 
-   if (strlen (directory))
-   filename = talloc_asprintf (message, %s/%s/%s,
-   db_path, directory, basename);
+   if (strlen (directory)) {
+   /* If directory is a URI, we don't need to append the db_path;
+* it is already an absolute path. */
+   /* This is just a quick hack instead of actually parsing the URL. */
+   if (strstr (directory, ://) == NULL)
+   filename = talloc_asprintf (message, %s/%s/%s,
+   db_path, directory, basename);
+   else
+   filename = talloc_asprintf (message, %s/%s,
+   directory, basename);
+   }
else
filename = talloc_asprintf (message, %s/%s,
db_path, basename);
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 05/14] new: use new URL-based filenames for messages

2012-06-26 Thread Ethan Glasser-Camp
This commit breaks a bunch of tests; fixes follow.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 notmuch-new.c |   27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 938ae29..1f11b2c 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -287,7 +287,7 @@ add_files (notmuch_database_t *notmuch,
 {
 DIR *dir = NULL;
 struct dirent *entry = NULL;
-char *next = NULL;
+char *next = NULL, *path_uri = NULL;
 time_t fs_mtime, db_mtime;
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
 notmuch_message_t *message = NULL;
@@ -315,7 +315,16 @@ add_files (notmuch_database_t *notmuch,
 
 fs_mtime = st.st_mtime;
 
-status = notmuch_database_get_directory (notmuch, path, directory);
+/* maildir URIs should never have a hostname component, but
+ * uriparser doesn't parse paths correctly if they start with //,
+ * as in scheme://host//path.
+ */
+if (path[0] == '/')
+   path_uri = talloc_asprintf (notmuch, maildir://%s, path);
+else
+   path_uri = talloc_asprintf (notmuch, maildir:///%s, path);
+
+status = notmuch_database_get_directory (notmuch, path_uri, directory);
 if (status) {
ret = status;
goto DONE;
@@ -423,7 +432,7 @@ add_files (notmuch_database_t *notmuch,
   strcmp (notmuch_filenames_get (db_files), entry-d_name)  0)
{
char *absolute = talloc_asprintf (state-removed_files,
- %s/%s, path,
+ %s/%s, path_uri,
  notmuch_filenames_get (db_files));
 
_filename_list_add (state-removed_files, absolute);
@@ -439,7 +448,7 @@ add_files (notmuch_database_t *notmuch,
if (strcmp (filename, entry-d_name)  0)
{
char *absolute = talloc_asprintf (state-removed_directories,
- %s/%s, path, filename);
+ %s/%s, path_uri, filename);
 
_filename_list_add (state-removed_directories, absolute);
}
@@ -467,7 +476,7 @@ add_files (notmuch_database_t *notmuch,
 
/* We're now looking at a regular file that doesn't yet exist
 * in the database, so add it. */
-   next = talloc_asprintf (notmuch, %s/%s, path, entry-d_name);
+   next = talloc_asprintf (notmuch, %s/%s, path_uri, entry-d_name);
 
state-processed_files++;
 
@@ -559,7 +568,7 @@ add_files (notmuch_database_t *notmuch,
 while (notmuch_filenames_valid (db_files))
 {
char *absolute = talloc_asprintf (state-removed_files,
- %s/%s, path,
+ %s/%s, path_uri,
  notmuch_filenames_get (db_files));
 
_filename_list_add (state-removed_files, absolute);
@@ -570,7 +579,7 @@ add_files (notmuch_database_t *notmuch,
 while (notmuch_filenames_valid (db_subdirs))
 {
char *absolute = talloc_asprintf (state-removed_directories,
- %s/%s, path,
+ %s/%s, path_uri,
  notmuch_filenames_get (db_subdirs));
 
_filename_list_add (state-removed_directories, absolute);
@@ -584,9 +593,11 @@ add_files (notmuch_database_t *notmuch,
  * same second.  This may lead to unnecessary re-scans, but it
  * avoids overlooking messages. */
 if (fs_mtime != stat_time)
-   _filename_list_add (state-directory_mtimes, path)-mtime = fs_mtime;
+   _filename_list_add (state-directory_mtimes, path_uri)-mtime = 
fs_mtime;
 
   DONE:
+if (path_uri)
+   talloc_free (path_uri);
 if (next)
talloc_free (next);
 if (dir)
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 06/14] maildir URIs can be used in tags_to_maildir_flags

2012-06-26 Thread Ethan Glasser-Camp
A better fix would probably be based on scheme.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/message.cc |   51 ++-
 1 file changed, 46 insertions(+), 5 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index c9857f5..8ecec71 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -23,6 +23,7 @@
 
 #include stdint.h
 
+#include uriparser/Uri.h
 #include gmime/gmime.h
 
 struct visible _notmuch_message {
@@ -1093,7 +1094,6 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t 
*message)
 {
filename = notmuch_filenames_get (filenames);
dir = _filename_is_in_maildir (filename);
-
if (! dir)
continue;
 
@@ -1304,12 +1304,46 @@ _new_maildir_filename (void *ctx,
 return filename_new;
 }
 
+/* Parses a maildir URI and returns the filename corresponding to its
+ * path.
+ *
+ * Returns NULL if either the URI couldn't be parsed or if the
+ * scheme isn't maildir:.
+ */
+static char *
+_get_maildir_filename (const char *filename)
+{
+UriParserStateA parser_state;
+UriUriA parsed;
+char *path;
+parser_state.uri = parsed;
+
+if (uriParseUriA (parser_state, filename) != URI_SUCCESS) {
+   uriFreeUriMembersA (parsed);
+   return NULL;
+}
+
+if (parsed.scheme.first != NULL 
+   0 != strncmp(parsed.scheme.first, maildir,
+parsed.scheme.afterLast-parsed.scheme.first)) {
+   /* Full URI with non-maildir scheme. */
+   uriFreeUriMembersA (parsed);
+   return NULL;
+}
+
+path = (char *)parsed.pathHead-text.first - 1;
+uriFreeUriMembersA (parsed);
+return path;
+
+}
+
+
 notmuch_status_t
 notmuch_message_tags_to_maildir_flags (notmuch_message_t *message)
 {
 notmuch_filenames_t *filenames;
 const char *filename;
-char *filename_new;
+char *filename_new, *filename_old, *filename_new_uri;
 char *to_set, *to_clear;
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 
@@ -1324,16 +1358,22 @@ notmuch_message_tags_to_maildir_flags 
(notmuch_message_t *message)
if (! _filename_is_in_maildir (filename))
continue;
 
-   filename_new = _new_maildir_filename (message, filename,
+   filename_old = _get_maildir_filename (filename);
+   if (filename_old == NULL)
+   continue;
+
+   filename_new = _new_maildir_filename (message, filename_old,
  to_set, to_clear);
if (filename_new == NULL)
continue;
 
+   filename_new_uri = talloc_asprintf (message, maildir://%s, 
filename_new);
+
if (strcmp (filename, filename_new)) {
int err;
notmuch_status_t new_status;
 
-   err = rename (filename, filename_new);
+   err = rename (filename_old, filename_new);
if (err)
continue;
 
@@ -1347,7 +1387,7 @@ notmuch_message_tags_to_maildir_flags (notmuch_message_t 
*message)
}
 
new_status = _notmuch_message_add_filename (message,
-   filename_new);
+   filename_new_uri);
/* Hold on to only the first error. */
if (! status  new_status) {
status = new_status;
@@ -1358,6 +1398,7 @@ notmuch_message_tags_to_maildir_flags (notmuch_message_t 
*message)
}
 
talloc_free (filename_new);
+   talloc_free (filename_new_uri);
 }
 
 talloc_free (to_set);
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 07/14] Update tests that need to see filenames to use URIs

2012-06-26 Thread Ethan Glasser-Camp
This fixes all tests except atomicity, which should be next.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 test/emacs   |2 +-
 test/json|4 ++--
 test/maildir-sync|7 ---
 test/multipart   |4 ++--
 test/new |6 +++---
 test/search-folder-coherence |2 +-
 test/search-output   |4 ++--
 test/test-lib.sh |3 +++
 8 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/test/emacs b/test/emacs
index e9f954c..c08791e 100755
--- a/test/emacs
+++ b/test/emacs
@@ -621,7 +621,7 @@ Stash my stashables
 id:bought
 bought
 inbox,stashtest
-${gen_msg_filename}
+${gen_msg_uri}
 http://mid.gmane.org/bought
 http://marc.info/?i=bought
 http://mail-archive.com/search?l=midq=bought
diff --git a/test/json b/test/json
index 6439788..be29fac 100755
--- a/test/json
+++ b/test/json
@@ -5,7 +5,7 @@ test_description=--format=json output
 test_begin_subtest Show message: json
 add_message [subject]=\json-show-subject\ [date]=\Sat, 01 Jan 2000 
12:00:00 -\ [body]=\json-show-message\
 output=$(notmuch show --format=json json-show-message)
-test_expect_equal $output [[[{\id\: \${gen_msg_id}\, \match\: true, 
\excluded\: false, \filename\: \${gen_msg_filename}\, \timestamp\: 
946728000, \date_relative\: \2000-01-01\, \tags\: [\inbox\,\unread\], 
\headers\: {\Subject\: \json-show-subject\, \From\: \Notmuch Test 
Suite test_su...@notmuchmail.org\, \To\: \Notmuch Test Suite 
test_su...@notmuchmail.org\, \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, 
\body\: [{\id\: 1, \content-type\: \text/plain\, \content\: 
\json-show-message\n\}]}, [
+test_expect_equal $output [[[{\id\: \${gen_msg_id}\, \match\: true, 
\excluded\: false, \filename\: \${gen_msg_uri}\, \timestamp\: 
946728000, \date_relative\: \2000-01-01\, \tags\: [\inbox\,\unread\], 
\headers\: {\Subject\: \json-show-subject\, \From\: \Notmuch Test 
Suite test_su...@notmuchmail.org\, \To\: \Notmuch Test Suite 
test_su...@notmuchmail.org\, \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, 
\body\: [{\id\: 1, \content-type\: \text/plain\, \content\: 
\json-show-message\n\}]}, [
 
 test_begin_subtest Search message: json
 add_message [subject]=\json-search-subject\ [date]=\Sat, 01 Jan 2000 
12:00:00 -\ [body]=\json-search-message\
@@ -22,7 +22,7 @@ test_expect_equal $output [{\thread\: \XXX\,
 test_begin_subtest Show message: json, utf-8
 add_message [subject]=\json-show-utf8-body-sübjéct\ [date]=\Sat, 01 Jan 
2000 12:00:00 -\ [body]=\jsön-show-méssage\
 output=$(notmuch show --format=json jsön-show-méssage)
-test_expect_equal $output [[[{\id\: \${gen_msg_id}\, \match\: true, 
\excluded\: false, \filename\: \${gen_msg_filename}\, \timestamp\: 
946728000, \date_relative\: \2000-01-01\, \tags\: [\inbox\,\unread\], 
\headers\: {\Subject\: \json-show-utf8-body-sübjéct\, \From\: \Notmuch 
Test Suite test_su...@notmuchmail.org\, \To\: \Notmuch Test Suite 
test_su...@notmuchmail.org\, \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, 
\body\: [{\id\: 1, \content-type\: \text/plain\, \content\: 
\jsön-show-méssage\n\}]}, [
+test_expect_equal $output [[[{\id\: \${gen_msg_id}\, \match\: true, 
\excluded\: false, \filename\: \${gen_msg_uri}\, \timestamp\: 
946728000, \date_relative\: \2000-01-01\, \tags\: [\inbox\,\unread\], 
\headers\: {\Subject\: \json-show-utf8-body-sübjéct\, \From\: \Notmuch 
Test Suite test_su...@notmuchmail.org\, \To\: \Notmuch Test Suite 
test_su...@notmuchmail.org\, \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, 
\body\: [{\id\: 1, \content-type\: \text/plain\, \content\: 
\jsön-show-méssage\n\}]}, [
 
 test_begin_subtest Show message: json, inline attachment filename
 subject='json-show-inline-attachment-filename'
diff --git a/test/maildir-sync b/test/maildir-sync
index 01348d3..a2e110e 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -8,7 +8,7 @@ test_description=maildir synchronization
 # --format=json output includes some newlines. Also, need to avoid
 # including the local value of MAIL_DIR in the result.
 filter_show_json() {
-sed -e 's/, /,\n/g'  | sed -e s|${MAIL_DIR}/|MAIL_DIR/|
+sed -e 's/, /,\n/g'  | sed -e s|${MAIL_URI}/|MAIL_DIR/|
 echo
 }
 
@@ -102,8 +102,9 @@ No new mail. Detected 1 file rename.
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Removing S flag (inbox 
unread)
 
 test_begin_subtest Removing info from filename leaves tags unchanged
-add_message [subject]='Message to lose maildir info' 
[filename]='message-to-lose-maildir-info' [dir]=cur
-notmuch tag -unread subject:Message to lose maildir info
+generate_message [subject]='Message to lose maildir info' 
[filename]='message-to-lose-maildir-info' [dir]=cur
+notmuch new  hrngh.new
+notmuch tag -unread subject:Message to lose maildir info  hrngh.tag
 mv $MAIL_DIR/cur/message-to-lose-maildir-info:2,S 
$MAIL_DIR/cur/message-without-maildir-info
 output=$(NOTMUCH_NEW)
 output+=
diff --git a/test/multipart b/test

[RFC PATCH 09/14] Fix atomicity test to work without relocatable mailstores

2012-06-26 Thread Ethan Glasser-Camp
Instead of assuming that the mailstore doesn't store its absolute
filenames, we use a symlink that can change back and forth. As long as
filenames contain this symlink, they can work in either the real
database, or the current snapshot.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 test/atomicity |   10 +-
 test/atomicity.gdb |   11 ---
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/test/atomicity b/test/atomicity
index 6df0a00..7b62ec7 100755
--- a/test/atomicity
+++ b/test/atomicity
@@ -49,13 +49,13 @@ if test_require_external_prereq gdb; then
 rm $MAIL_DIR/.remove-dir/remove-directory-duplicate:2,
 rmdir $MAIL_DIR/.remove-dir
 
-# Prepare a snapshot of the updated maildir.  The gdb script will
-# update the database in this snapshot as it goes.
+# Copy the mail database. We will run on this database concurrently.
 cp -ra $MAIL_DIR $MAIL_DIR.snap
-cp ${NOTMUCH_CONFIG} ${NOTMUCH_CONFIG}.snap
-NOTMUCH_CONFIG=${NOTMUCH_CONFIG}.snap notmuch config set database.path 
$MAIL_DIR.snap
-
 
+# Use a symlink instead of the real path. This way, we can change the 
symlink,
+# without filenames having to change.
+mv $MAIL_DIR $MAIL_DIR.real
+ln -s $MAIL_DIR.real $MAIL_DIR
 
 # Execute notmuch new and, at every call to rename, snapshot the
 # database, run notmuch new again on the snapshot, and capture the
diff --git a/test/atomicity.gdb b/test/atomicity.gdb
index fd67525..3d4e210 100644
--- a/test/atomicity.gdb
+++ b/test/atomicity.gdb
@@ -38,12 +38,17 @@ shell mv backtrace backtrace.`cat outcount`
 # Snapshot the database
 shell rm -r $MAIL_DIR.snap/.notmuch
 shell cp -r $MAIL_DIR/.notmuch $MAIL_DIR.snap/.notmuch
+shell rm $MAIL_DIR
+shell ln -s $MAIL_DIR.snap $MAIL_DIR
 # Restore the mtime of $MAIL_DIR.snap, which we just changed
-shell touch -r $MAIL_DIR $MAIL_DIR.snap
+shell touch -r $MAIL_DIR.real $MAIL_DIR.snap
 # Run notmuch new to completion on the snapshot
-shell NOTMUCH_CONFIG=${NOTMUCH_CONFIG}.snap XAPIAN_FLUSH_THRESHOLD=1000 
notmuch new  /dev/null
-shell NOTMUCH_CONFIG=${NOTMUCH_CONFIG}.snap notmuch search '*'  search.`cat 
outcount` 21
+shell NOTMUCH_CONFIG=${NOTMUCH_CONFIG} XAPIAN_FLUSH_THRESHOLD=1000 notmuch new 
 /dev/null
+shell NOTMUCH_CONFIG=${NOTMUCH_CONFIG} notmuch search '*'  search.`cat 
outcount` 21
 shell echo $(expr $(cat outcount) + 1)  outcount
+# restore symlink to correct database before resuming
+shell rm $MAIL_DIR
+shell ln -s $MAIL_DIR.real $MAIL_DIR
 cont
 end
 
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 10/14] new: add scan option

2012-06-26 Thread Ethan Glasser-Camp
This is just a quick hack to get started on adding an mbox backend.

The fact that the default maildir is scanned automagically is a
little weird, but it doesn't do any harm unless you decide to put mail
there that you really don't want indexed.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 notmuch-client.h |9 +
 notmuch-config.c |   30 +-
 notmuch-new.c|   18 ++
 test/config  |1 +
 4 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 9b63eae..9d922fe 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -256,6 +256,15 @@ notmuch_config_set_new_ignore (notmuch_config_t *config,
   const char *new_ignore[],
   size_t length);
 
+const char **
+notmuch_config_get_new_scan (notmuch_config_t *config,
+  size_t *length);
+
+void
+notmuch_config_set_new_scan (notmuch_config_t *config,
+  const char *new_scan[],
+  size_t length);
+
 notmuch_bool_t
 notmuch_config_get_maildir_synchronize_flags (notmuch_config_t *config);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index 3e37a2d..e9d99ea 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -50,7 +50,10 @@ static const char new_config_comment[] =
 \tthat will not be searched for messages by \notmuch new\.\n
 \n
 \tNOTE: *Every* file/directory that goes by one of those names 
will\n
-\tbe ignored, independent of its depth/location in the mail 
store.\n;
+\tbe ignored, independent of its depth/location in the mail 
store.\n
+\n
+\tscanA list (separated by ';') of mail URLs to scan.\n
+\tThe maildir located at database.path, above, will automatically 
be added.\n;
 
 static const char user_config_comment[] =
  User configuration\n
@@ -113,6 +116,8 @@ struct _notmuch_config {
 size_t new_tags_length;
 const char **new_ignore;
 size_t new_ignore_length;
+const char **new_scan;
+size_t new_scan_length;
 notmuch_bool_t maildir_synchronize_flags;
 const char **search_exclude_tags;
 size_t search_exclude_tags_length;
@@ -274,6 +279,8 @@ notmuch_config_open (void *ctx,
 config-new_tags_length = 0;
 config-new_ignore = NULL;
 config-new_ignore_length = 0;
+config-new_scan = NULL;
+config-new_scan_length = 0;
 config-maildir_synchronize_flags = TRUE;
 config-search_exclude_tags = NULL;
 config-search_exclude_tags_length = 0;
@@ -375,6 +382,10 @@ notmuch_config_open (void *ctx,
notmuch_config_set_new_ignore (config, NULL, 0);
 }
 
+if (notmuch_config_get_new_scan (config, tmp) == NULL) {
+   notmuch_config_set_new_scan (config, NULL, 0);
+}
+
 if (notmuch_config_get_search_exclude_tags (config, tmp) == NULL) {
if (is_new) {
const char *tags[] = { deleted, spam };
@@ -631,6 +642,14 @@ notmuch_config_get_new_ignore (notmuch_config_t *config, 
size_t *length)
 (config-new_ignore_length), length);
 }
 
+const char **
+notmuch_config_get_new_scan (notmuch_config_t *config, size_t *length)
+{
+return _config_get_list (config, new, scan,
+(config-new_scan),
+(config-new_scan_length), length);
+}
+
 void
 notmuch_config_set_user_other_email (notmuch_config_t *config,
 const char *list[],
@@ -658,6 +677,15 @@ notmuch_config_set_new_ignore (notmuch_config_t *config,
 (config-new_ignore));
 }
 
+void
+notmuch_config_set_new_scan (notmuch_config_t *config,
+const char *list[],
+size_t length)
+{
+_config_set_list (config, new, scan, list, length,
+(config-new_scan));
+}
+
 const char **
 notmuch_config_get_search_exclude_tags (notmuch_config_t *config, size_t 
*length)
 {
diff --git a/notmuch-new.c b/notmuch-new.c
index 1f11b2c..57b27bf 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -239,6 +239,16 @@ _entry_in_ignore_list (const char *entry, 
add_files_state_t *state)
 return FALSE;
 }
 
+/* Call out to the appropriate add_files function, based on the URI. */
+static notmuch_status_t
+add_files_uri (unused(notmuch_database_t *notmuch),
+  unused(const char *uri),
+  unused(add_files_state_t *state))
+{
+/* Stub for now */
+return NOTMUCH_STATUS_SUCCESS;
+}
+
 /* Examine 'path' recursively as follows:
  *
  *   o Ask the filesystem for the mtime of 'path' (fs_mtime)
@@ -843,6 +853,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 int ret = 0;
 struct stat st;
 const char *db_path;
+const char **new_scan;
+size_t new_scan_length, new_scan_i;
 char *dot_notmuch_path;
 struct sigaction action;
 _filename_node_t *f

[RFC PATCH 11/14] notmuch-new: pull out useful bits of add_files_recursive

2012-06-26 Thread Ethan Glasser-Camp
This is part of notmuch-new refactor phase 1: make add_files stuff
safe for other backends. add_files_recursive is essentially a
maildir-crawling function that periodically adds files to the database
or adds filenames to remove_files or remove_directory lists. I don't
see an easy way to adapt add_files_recursive for other backends who
might not have concepts of directories with other directories inside
of them, so instead just provide an add_files method for each backend.

This patch pulls some bits out of add_files_recursive which will be
useful for other backends: two reporting functions
_report_before_adding_file and _report_added_file, as well as
_add_message, which actually does the message adding.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 notmuch-new.c |  192 +++--
 1 file changed, 119 insertions(+), 73 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 57b27bf..1bf4e25 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -249,6 +249,122 @@ add_files_uri (unused(notmuch_database_t *notmuch),
 return NOTMUCH_STATUS_SUCCESS;
 }
 
+/* Progress-reporting function.
+ *
+ * Can be used by any mailstore-crawling function that wants to alert
+ * users what message it's about to add. Subsequent errors will be due
+ * to this message ;)
+ */
+static void
+_report_before_adding_file (add_files_state_t *state, const char *filename)
+{
+state-processed_files++;
+
+if (state-verbose) {
+   if (state-output_is_a_tty)
+   printf(\r\033[K);
+
+   printf (%i/%i: %s,
+   state-processed_files,
+   state-total_files,
+   filename);
+
+   putchar((state-output_is_a_tty) ? '\r' : '\n');
+   fflush (stdout);
+}
+}
+
+/* Progress-reporting function.
+ *
+ * Call this to respond to the signal handler for SIGALRM.
+ */
+static void
+_report_added_file (add_files_state_t *state)
+{
+if (do_print_progress) {
+   do_print_progress = 0;
+   generic_print_progress (Processed, files, state-tv_start,
+   state-processed_files, state-total_files);
+}
+}
+
+
+/* Atomically handles adding a message to the database.
+ *
+ * Should be used by any mailstore-crawling function that finds a new
+ * message to add.
+ */
+static notmuch_status_t
+_add_message (add_files_state_t *state, notmuch_database_t *notmuch,
+ const char *filename)
+{
+notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
+notmuch_message_t *message;
+const char **tag;
+
+status = notmuch_database_begin_atomic (notmuch);
+if (status) {
+   ret = status;
+   goto DONE;
+}
+
+status = notmuch_database_add_message (notmuch, filename, message);
+
+switch (status) {
+/* success */
+case NOTMUCH_STATUS_SUCCESS:
+   state-added_messages++;
+   notmuch_message_freeze (message);
+   for (tag=state-new_tags; *tag != NULL; tag++)
+   notmuch_message_add_tag (message, *tag);
+   if (state-synchronize_flags == TRUE)
+   notmuch_message_maildir_flags_to_tags (message);
+   notmuch_message_thaw (message);
+   break;
+/* Non-fatal issues (go on to next file) */
+case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
+   if (state-synchronize_flags == TRUE)
+   notmuch_message_maildir_flags_to_tags (message);
+   break;
+case NOTMUCH_STATUS_FILE_NOT_EMAIL:
+   fprintf (stderr, Note: Ignoring non-mail file: %s\n,
+filename);
+   break;
+/* Fatal issues. Don't process anymore. */
+case NOTMUCH_STATUS_READ_ONLY_DATABASE:
+case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
+case NOTMUCH_STATUS_OUT_OF_MEMORY:
+   fprintf (stderr, Error: %s. Halting processing.\n,
+notmuch_status_to_string (status));
+   ret = status;
+   goto DONE;
+default:
+case NOTMUCH_STATUS_FILE_ERROR:
+case NOTMUCH_STATUS_NULL_POINTER:
+case NOTMUCH_STATUS_TAG_TOO_LONG:
+case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:
+case NOTMUCH_STATUS_UNBALANCED_ATOMIC:
+case NOTMUCH_STATUS_LAST_STATUS:
+   INTERNAL_ERROR (add_message returned unexpected value: %d,  status);
+   ret = status;
+   goto DONE;
+}
+
+status = notmuch_database_end_atomic (notmuch);
+if (status) {
+   ret = status;
+   goto DONE;
+}
+
+  DONE:
+if (message) {
+   notmuch_message_destroy (message);
+   message = NULL;
+}
+
+return ret;
+}
+
 /* Examine 'path' recursively as follows:
  *
  *   o Ask the filesystem for the mtime of 'path' (fs_mtime)
@@ -300,7 +416,6 @@ add_files (notmuch_database_t *notmuch,
 char *next = NULL, *path_uri = NULL;
 time_t fs_mtime, db_mtime;
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
-notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
 int i, num_fs_entries = 0, entry_type;
 notmuch_directory_t *directory;
@@ -309,7

[RFC PATCH 12/14] mailstore: support for mbox:// URIs

2012-06-26 Thread Ethan Glasser-Camp

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/mailstore.c |   85 +++
 1 file changed, 85 insertions(+)

diff --git a/lib/mailstore.c b/lib/mailstore.c
index ae02c12..e8d9bc1 100644
--- a/lib/mailstore.c
+++ b/lib/mailstore.c
@@ -19,6 +19,7 @@
  */
 #include uriparser/Uri.h
 #include stdio.h
+#include glib.h
 
 #include notmuch-private.h
 
@@ -28,6 +29,74 @@ notmuch_mailstore_basic_open (const char *filename)
 return fopen (filename, r);
 }
 
+/* Since we have to return a FILE*, we use fmemopen to turn buffers
+ * into FILE* streams. But when we close these streams, we have to
+ * free() the buffers. Use a hash to associate the two.
+ */
+static GHashTable *_mbox_files_to_strings = NULL;
+
+static void
+_ensure_mbox_files_to_strings () {
+if (_mbox_files_to_strings == NULL)
+_mbox_files_to_strings = g_hash_table_new (NULL, NULL);
+}
+
+static FILE *
+notmuch_mailstore_mbox_open (UriUriA *uri)
+{
+FILE *ret = NULL, *mbox = NULL;
+char *filename, *message, *length_s;
+const char *error;
+long int offset, length, this_read;
+_ensure_mbox_files_to_strings ();
+
+offset = strtol (uri-fragment.first, length_s, 10);
+length = strtol (length_s+1, NULL, 10);
+
+filename = talloc_strndup (NULL, uri-pathHead-text.first-1,
+   
uri-pathTail-text.afterLast-uri-pathHead-text.first+1);
+
+if (filename == NULL)
+goto DONE;
+
+mbox = fopen (filename, r);
+if (mbox == NULL) {
+fprintf (stderr, Couldn't open message %s: %s.\n, uri-scheme.first,
+ strerror (errno));
+goto DONE;
+}
+
+message = talloc_array (NULL, char, length);
+fseek (mbox, offset, SEEK_SET);
+
+this_read = fread (message, sizeof(char), length, mbox);
+if (this_read != length) {
+if (feof (mbox))
+error = end of file reached;
+if (ferror (mbox))
+error = strerror (ferror (mbox));
+
+fprintf (stderr, Couldn't read message %s: %s.\n, uri-scheme.first, 
error);
+goto DONE;
+}
+
+ret = fmemopen (message, length, r);
+if (ret == NULL) {
+/* No fclose will ever be called, so let's free message now */
+talloc_free (message);
+goto DONE;
+}
+
+g_hash_table_insert (_mbox_files_to_strings, ret, message);
+DONE:
+if (filename)
+talloc_free (filename);
+if (mbox)
+fclose (mbox);
+
+return ret;
+}
+
 FILE *
 notmuch_mailstore_open (const char *filename)
 {
@@ -57,6 +126,14 @@ notmuch_mailstore_open (const char *filename)
 goto DONE;
 }
 
+if (0 == strncmp (parsed.scheme.first, mbox,
+  parsed.scheme.afterLast-parsed.scheme.first)) {
+/* mbox URI of the form mbox:///path/to/file#offset+length.
+ * Just pass the parsed URI. */
+ret = notmuch_mailstore_mbox_open (parsed);
+goto DONE;
+}
+
 DONE:
 uriFreeUriMembersA (parsed);
 return ret;
@@ -65,5 +142,13 @@ DONE:
 int
 notmuch_mailstore_close (FILE *file)
 {
+char *file_buffer;
+if (_mbox_files_to_strings != NULL) {
+file_buffer = g_hash_table_lookup (_mbox_files_to_strings, file);
+if (file_buffer != NULL) {
+talloc_free (file_buffer);
+}
+g_hash_table_remove (_mbox_files_to_strings, file);
+}
 return fclose (file);
 }
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 13/14] Tests for mbox support

2012-06-26 Thread Ethan Glasser-Camp
These need to be improved, rather than hard-coding byte offsets.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 test/mbox |   59 +
 test/notmuch-test |1 +
 2 files changed, 60 insertions(+)
 create mode 100755 test/mbox

diff --git a/test/mbox b/test/mbox
new file mode 100755
index 000..f03f887
--- /dev/null
+++ b/test/mbox
@@ -0,0 +1,59 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='basic mbox support'
+. ./test-lib.sh
+
+mkdir -p $MAIL_DIR/some-mboxes/subdir $MAIL_DIR/database $MAIL_DIR/corpus
+
+# The Content-Length headers here include the final newline (added later).
+generate_message '[body]=Mbox message 1.' '[header]=Content-Length: 16' 
[dir]=corpus
+generate_message '[body]=Mbox message 2. Longer.' '[header]=Content-Length: 
24' [dir]=corpus
+generate_message '[body]=Mbox message 3.' [dir]=corpus
+generate_message '[body]=Mbox message 4.' [dir]=corpus
+generate_message '[body]=Mbox message 5. Last message.' 
'[header]=Content-Length: 30' [dir]=corpus
+
+MBOX1=$MAIL_DIR/some-mboxes/first.mbox
+for x in $MAIL_DIR/corpus/*; do
+echo From MAILER-DAEMON Sat Jan  3 01:05:34 1996  $MBOX1
+cat $x  $MBOX1
+# Final newline
+echo  $MBOX1
+done
+
+notmuch config set database.path $MAIL_DIR/database
+notmuch config set new.scan mbox://$MAIL_DIR/some-mboxes
+
+test_begin_subtest read a small mbox (5 messages)
+output=$(NOTMUCH_NEW)
+test_expect_equal $output Added 5 new messages to the database.
+
+test_begin_subtest search
+output=$(notmuch search '*' | notmuch_search_sanitize)
+test_expect_equal $output thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; 
Test message #1 (inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Test message #2 (inbox 
unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Test message #3 (inbox 
unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Test message #4 (inbox 
unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Test message #5 (inbox 
unread)
+
+test_begin_subtest show (mboxcl)
+output=$(notmuch show Test message #1 | grep -o filename:[^ ]*)
+test_expect_equal $output 
filename:mbox://$MAIL_DIR/some-mboxes/first.mbox#44+246
+
+test_begin_subtest show doesn't append an extra space at the end (mboxcl)
+output=$(notmuch show --format=raw Test message #1 )
+original=$(cat $MAIL_DIR/corpus/msg-001)
+test_expect_equal $output $original
+
+test_begin_subtest show (non-cl)
+output=$(notmuch show Test message #3 | grep -o filename:[^ ]*)
+test_expect_equal $output 
filename:mbox://$MAIL_DIR/some-mboxes/first.mbox#634+227
+
+test_begin_subtest show doesn't append an extra space at the end (non-cl)
+output=$(notmuch show --format=raw Test message #3 )
+original=$(cat $MAIL_DIR/corpus/msg-003)
+test_expect_equal $output $original
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index bfad5d3..8cbb2cd 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -47,6 +47,7 @@ TESTS=
   emacs-large-search-buffer
   emacs-subject-to-filename
   maildir-sync
+  mbox
   crypto
   symbol-hiding
   search-folder-coherence
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[RFC PATCH 14/14] new: Add scan support for mbox:// URIs

2012-06-26 Thread Ethan Glasser-Camp
A lot of code is duplicated from maildir, I don't think I handled all
those errors correctly, and I didn't report any progress.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 notmuch-new.c |  299 +++--
 1 file changed, 289 insertions(+), 10 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 1bf4e25..36fee34 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -19,6 +19,7 @@
  */
 
 #include notmuch-client.h
+#include uriparser/Uri.h
 
 #include unistd.h
 
@@ -239,16 +240,6 @@ _entry_in_ignore_list (const char *entry, 
add_files_state_t *state)
 return FALSE;
 }
 
-/* Call out to the appropriate add_files function, based on the URI. */
-static notmuch_status_t
-add_files_uri (unused(notmuch_database_t *notmuch),
-  unused(const char *uri),
-  unused(add_files_state_t *state))
-{
-/* Stub for now */
-return NOTMUCH_STATUS_SUCCESS;
-}
-
 /* Progress-reporting function.
  *
  * Can be used by any mailstore-crawling function that wants to alert
@@ -674,6 +665,294 @@ add_files (notmuch_database_t *notmuch,
 return ret;
 }
 
+/* Scan an mbox file for messages.
+ *
+ * We assume that mboxes grow monotonically only.
+ *
+ * The mtime of the mbox file is stored in a directory document in
+ * Xapian.
+ */
+static notmuch_status_t
+add_messages_mbox_file (notmuch_database_t *notmuch,
+   const char *path,
+   add_files_state_t *state)
+{
+notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, status;
+struct stat st;
+time_t fs_mtime, db_mtime, stat_time;
+FILE *mbox;
+char *line, *path_uri = NULL, *message_uri = NULL;
+int line_len;
+size_t offset, end_offset, line_size = 0;
+notmuch_directory_t *directory;
+int content_length = -1, is_headers;
+
+if (stat (path, st)) {
+   fprintf (stderr, Error reading mbox file %s: %s\n,
+path, strerror (errno));
+   return NOTMUCH_STATUS_FILE_ERROR;
+}
+
+stat_time = time (NULL);
+if (! S_ISREG (st.st_mode)) {
+   fprintf (stderr, Error: %s is not a file.\n, path);
+   return NOTMUCH_STATUS_FILE_ERROR;
+}
+
+fs_mtime = st.st_mtime;
+
+path_uri = talloc_asprintf (notmuch, mbox://%s, path);
+status = notmuch_database_get_directory (notmuch, path_uri, directory);
+if (status) {
+   ret = status;
+   goto DONE;
+}
+db_mtime = directory ? notmuch_directory_get_mtime (directory) : 0;
+
+if (directory  db_mtime == fs_mtime) {
+   goto DONE;
+}
+
+mbox = fopen (path, r);
+if (mbox == NULL) {
+   fprintf (stderr, Error: couldn't open %s for reading.\n,
+path);
+   ret = NOTMUCH_STATUS_FILE_ERROR;
+   goto DONE;
+}
+
+line_len = getline (line, line_size, mbox);
+
+if (line_len == -1) {
+   fprintf (stderr, Error: reading from %s failed: %s\n,
+path, strerror (errno));
+   ret = NOTMUCH_STATUS_FILE_ERROR;
+   goto DONE;
+}
+
+if (strncmp (line, From , 5) != 0) {
+   fprintf (stderr, Note: Ignoring non-mbox file: %s\n,
+path);
+   ret = NOTMUCH_STATUS_FILE_ERROR;
+   goto DONE;
+}
+free(line);
+line = NULL;
+
+/* Loop invariant: At the beginning of the loop, we have just read
+ * a From_ line, but haven't yet read any of the headers.
+ */
+while (! feof (mbox)) {
+   is_headers = 1;
+   offset = ftell (mbox);
+   content_length = -1;
+
+   /* Read lines until we either get to the next From_ header, or
+* we find a Content-Length header (mboxcl) and we run out of headers.
+*/
+   do {
+   /* Get the offset before we read, in case we got another From_ 
header. */
+   end_offset = ftell (mbox);
+
+   line_len = getline (line, line_size, mbox);
+
+   /* Check to see if this line is a content-length header,
+* or the end of the headers. */
+   if (is_headers  strncasecmp (line, Content-Length: ,
+  strlen (Content-Length: )) == 0)
+   content_length = strtol (line + strlen (Content-Length: ),
+NULL, 10);
+
+   if (is_headers  strlen (line) == 1  *line == '\n') {
+   is_headers = 0;
+   /* If we got a content_length, skip the message body. */
+   if (content_length != -1) {
+   fseek (mbox, content_length, SEEK_CUR);
+   line_len = getline (line, line_size, mbox);
+
+   /* We should be at the end of the message.  Sanity
+* check: there should be a blank line, and then
+* another From_ header. */
+   if (strlen (line) != 1 || *line != '\n') {
+   fprintf (stderr, Warning: message with Content-Length

[PATCH v2 1/8] All access to mail files goes through the mailstore module

2012-07-01 Thread Ethan Glasser-Camp
This commit introduces the mailstore module which provides two
functions, notmuch_mailstore_open and notmuch_mailstore_close. These
functions are currently just stub calls to fopen and fclose, but later
can be made more complex in order to support mail storage systems
where one message might not be one file.

No functional changes are made, but calls to fopen and fclose are
replaced with notmuch_mailstore_open and notmuch_mailstore_close
wherever they are being used to access messages, but not when talking
about real files like in notmuch-dump or notmuch-restore.

Since the sha1 function notmuch_sha1_of_file is only used on messages,
we convert it to using notmuch_mailstore_open and
notmuch_mailstore_close, and rename it notmuch_sha1_of_message. While
we are there, we also replace a numeric constant with its symbolic
name BLOCK_SIZE.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
This patch does not split notmuch_sha1_of_file the way the previous
patch series did, because there are no other callers of
notmuch_sha1_of_file. Instead it renames the function to
notmuch_sha1_of_message.

 lib/Makefile.local|1 +
 lib/database.cc   |2 +-
 lib/index.cc  |2 +-
 lib/mailstore.c   |   34 ++
 lib/message-file.c|6 +++---
 lib/notmuch-private.h |2 +-
 lib/notmuch.h |   16 
 lib/sha1.c|   11 +--
 mime-node.c   |4 ++--
 notmuch-show.c|   12 ++--
 10 files changed, 70 insertions(+), 20 deletions(-)
 create mode 100644 lib/mailstore.c

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 8a9aa28..cfc77bb 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -51,6 +51,7 @@ libnotmuch_c_srcs =   \
$(dir)/filenames.c  \
$(dir)/string-list.c\
$(dir)/libsha1.c\
+   $(dir)/mailstore.c  \
$(dir)/message-file.c   \
$(dir)/messages.c   \
$(dir)/sha1.c   \
diff --git a/lib/database.cc b/lib/database.cc
index 761dc1a..c035edc 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1773,7 +1773,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
if (message_id == NULL ) {
/* No message-id at all, let's generate one by taking a
 * hash over the file's contents. */
-   char *sha1 = notmuch_sha1_of_file (filename);
+   char *sha1 = notmuch_sha1_of_message (filename);
 
/* If that failed too, something is really wrong. Give up. */
if (sha1 == NULL) {
diff --git a/lib/index.cc b/lib/index.cc
index e377732..b607e82 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -441,7 +441,7 @@ _notmuch_message_index_file (notmuch_message_t *message,
initialized = 1;
 }
 
-file = fopen (filename, r);
+file = notmuch_mailstore_open (filename);
 if (! file) {
fprintf (stderr, Error opening %s: %s\n, filename, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
diff --git a/lib/mailstore.c b/lib/mailstore.c
new file mode 100644
index 000..48acd47
--- /dev/null
+++ b/lib/mailstore.c
@@ -0,0 +1,34 @@
+/* mailstore.c - code to access individual messages
+ *
+ * Copyright © 2009 Carl Worth
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Carl Worth cwo...@cworth.org
+ */
+#include stdio.h
+
+#include notmuch-private.h
+
+FILE *
+notmuch_mailstore_open (const char *filename)
+{
+return fopen (filename, r);
+}
+
+int
+notmuch_mailstore_close (FILE *file)
+{
+return fclose (file);
+}
diff --git a/lib/message-file.c b/lib/message-file.c
index 915aba8..271389c 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -86,7 +86,7 @@ _notmuch_message_file_destructor (notmuch_message_file_t 
*message)
g_hash_table_destroy (message-headers);
 
 if (message-file)
-   fclose (message-file);
+   notmuch_mailstore_close (message-file);
 
 return 0;
 }
@@ -104,7 +104,7 @@ _notmuch_message_file_open_ctx (void *ctx, const char 
*filename)
 
 talloc_set_destructor (message, _notmuch_message_file_destructor);
 
-message-file = fopen (filename, r);
+message-file = notmuch_mailstore_open (filename);
 if (message-file == NULL)
goto FAIL;
 
@@ -361,7 +361,7 @@ notmuch_message_file_get_header (notmuch_message_file_t 
*message

[PATCH v2 2/8] Introduce uriparser

2012-07-01 Thread Ethan Glasser-Camp
Seeing as there is no glib-standard way to parse URIs, an external
library is needed. This commit introduces another program in compat/
and a stanza in ./configure to test if uriparser is there.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 Makefile.local  |2 +-
 compat/have_uriparser.c |   17 +
 configure   |   23 ---
 3 files changed, 38 insertions(+), 4 deletions(-)
 create mode 100644 compat/have_uriparser.c

diff --git a/Makefile.local b/Makefile.local
index a890df2..084f44e 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -41,7 +41,7 @@ PV_FILE=bindings/python/notmuch/version.py
 # Smash together user's values with our extra values
 FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) 
$(CONFIGURE_CFLAGS) $(extra_cflags)
 FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) 
$(extra_cflags) $(extra_cxxflags)
-FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch 
$(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)
+FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch 
$(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) $(URIPARSER_LDFLAGS)
 FINAL_NOTMUCH_LINKER = CC
 ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
 FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS)
diff --git a/compat/have_uriparser.c b/compat/have_uriparser.c
new file mode 100644
index 000..d79e51d
--- /dev/null
+++ b/compat/have_uriparser.c
@@ -0,0 +1,17 @@
+#include uriparser/Uri.h
+
+int
+main (int argc, char *argv[])
+{
+UriParserStateA state;
+UriUriA uri;
+char *uriS = NULL;
+
+state.uri = uri;
+if (uriParseUriA (state, uriS) != URI_SUCCESS) {
+/* Failure */
+uriFreeUriMembersA (uri);
+}
+
+return 0;
+}
diff --git a/configure b/configure
index 3fad424..80aa13c 100755
--- a/configure
+++ b/configure
@@ -313,6 +313,19 @@ else
 errors=$((errors + 1))
 fi
 
+printf Checking for uriparser... 
+if ${CC} -o compat/have_uriparser $srcdir/compat/have_uriparser.c 
-luriparser  /dev/null 21
+then
+printf Yes.\n
+uriparser_ldflags=-luriparser
+have_uriparser=1
+else
+printf No.\n
+have_uriparser=0
+errors=$((errors + 1))
+fi
+rm -f compat/have_uriparser
+
 printf Checking for valgrind development files... 
 if pkg-config --exists valgrind; then
 printf Yes.\n
@@ -431,11 +444,11 @@ case a simple command will install everything you need. 
For example:
 
 On Debian and similar systems:
 
-   sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev
+   sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev 
liburiparser-dev
 
 Or on Fedora and similar systems:
 
-   sudo yum install xapian-core-devel gmime-devel libtalloc-devel
+   sudo yum install xapian-core-devel gmime-devel libtalloc-devel 
uriparser-devel
 
 On other systems, similar commands can be used, but the details of the
 package names may be different.
@@ -669,6 +682,9 @@ GMIME_LDFLAGS = ${gmime_ldflags}
 TALLOC_CFLAGS = ${talloc_cflags}
 TALLOC_LDFLAGS = ${talloc_ldflags}
 
+# Flags needed to link against uriparser
+URIPARSER_LDFLAGS = ${uriparser_ldflags}
+
 # Flags needed to have linker set rpath attribute
 RPATH_LDFLAGS = ${rpath_ldflags}
 
@@ -698,5 +714,6 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) 
\$(GMIME_CFLAGS)\\
 \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
 \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\
  -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR)
-CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
+CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) \\
+ \$(URIPARSER_LDFLAGS)
 EOF
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 3/8] Not all filenames need to be converted to absolute paths

2012-07-01 Thread Ethan Glasser-Camp
_notmuch_message_ensure_filename_list converts relative paths, such
as those stored in Xapian until now, to absolute paths. However,
URLs are already absolute, and prepending the database path will just
confuse matters.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/message.cc |   20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 978de06..235185c 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -21,6 +21,7 @@
 #include notmuch-private.h
 #include database-private.h
 
+#include uriparser/Uri.h
 #include stdint.h
 
 #include gmime/gmime.h
@@ -682,6 +683,8 @@ _notmuch_message_ensure_filename_list (notmuch_message_t 
*message)
const char *db_path, *directory, *basename, *filename;
char *colon, *direntry = NULL;
unsigned int directory_id;
+   UriUriA parsed;
+   UriParserStateA parser;
 
direntry = node-string;
 
@@ -700,9 +703,20 @@ _notmuch_message_ensure_filename_list (notmuch_message_t 
*message)
  message-notmuch,
  directory_id);
 
-   if (strlen (directory))
-   filename = talloc_asprintf (message, %s/%s/%s,
-   db_path, directory, basename);
+   parser.uri = parsed;
+
+   if (strlen (directory)) {
+   /* If directory is a URI, we don't need to append the db_path;
+* it is already an absolute path. */
+   if (uriParseUriA (parser, directory) != URI_SUCCESS ||
+   parsed.scheme.first == NULL)
+   filename = talloc_asprintf (message, %s/%s/%s,
+   db_path, directory, basename);
+   else
+   filename = talloc_asprintf (message, %s/%s,
+   directory, basename);
+   uriFreeUriMembersA (parsed);
+   }
else
filename = talloc_asprintf (message, %s/%s,
db_path, basename);
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 4/8] new: add scan option

2012-07-01 Thread Ethan Glasser-Camp
The new.scan option is a list of URLs that can be scanned.

The fact that the database maildir is scanned automagically is a
little weird, but it doesn't do any harm unless you decide to put mail
there that you really don't want indexed.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
I added text here to respond to Mark Walters's confusion regarding how
I implemented scanning.

 notmuch-client.h |9 +
 notmuch-config.c |   33 -
 notmuch-new.c|   18 ++
 test/config  |1 +
 4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 0c17b79..20349b2 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -257,6 +257,15 @@ notmuch_config_set_new_ignore (notmuch_config_t *config,
   const char *new_ignore[],
   size_t length);
 
+const char **
+notmuch_config_get_new_scan (notmuch_config_t *config,
+  size_t *length);
+
+void
+notmuch_config_set_new_scan (notmuch_config_t *config,
+  const char *new_scan[],
+  size_t length);
+
 notmuch_bool_t
 notmuch_config_get_maildir_synchronize_flags (notmuch_config_t *config);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index 3e37a2d..387f855 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -50,7 +50,13 @@ static const char new_config_comment[] =
 \tthat will not be searched for messages by \notmuch new\.\n
 \n
 \tNOTE: *Every* file/directory that goes by one of those names 
will\n
-\tbe ignored, independent of its depth/location in the mail 
store.\n;
+\tbe ignored, independent of its depth/location in the mail 
store.\n
+\n
+\tscanA list (separated by ';') of mail URLs to scan.\n
+\tEach URL denotes a \root\ which will be searched for mail 
files.\n
+\tHow this search is performed depends on the scheme of the URL 
(the\n
+\tpart before the first colon).\n
+\tThe maildir located at database.path, above, will automatically 
be added.\n;
 
 static const char user_config_comment[] =
  User configuration\n
@@ -113,6 +119,8 @@ struct _notmuch_config {
 size_t new_tags_length;
 const char **new_ignore;
 size_t new_ignore_length;
+const char **new_scan;
+size_t new_scan_length;
 notmuch_bool_t maildir_synchronize_flags;
 const char **search_exclude_tags;
 size_t search_exclude_tags_length;
@@ -274,6 +282,8 @@ notmuch_config_open (void *ctx,
 config-new_tags_length = 0;
 config-new_ignore = NULL;
 config-new_ignore_length = 0;
+config-new_scan = NULL;
+config-new_scan_length = 0;
 config-maildir_synchronize_flags = TRUE;
 config-search_exclude_tags = NULL;
 config-search_exclude_tags_length = 0;
@@ -375,6 +385,10 @@ notmuch_config_open (void *ctx,
notmuch_config_set_new_ignore (config, NULL, 0);
 }
 
+if (notmuch_config_get_new_scan (config, tmp) == NULL) {
+   notmuch_config_set_new_scan (config, NULL, 0);
+}
+
 if (notmuch_config_get_search_exclude_tags (config, tmp) == NULL) {
if (is_new) {
const char *tags[] = { deleted, spam };
@@ -631,6 +645,14 @@ notmuch_config_get_new_ignore (notmuch_config_t *config, 
size_t *length)
 (config-new_ignore_length), length);
 }
 
+const char **
+notmuch_config_get_new_scan (notmuch_config_t *config, size_t *length)
+{
+return _config_get_list (config, new, scan,
+(config-new_scan),
+(config-new_scan_length), length);
+}
+
 void
 notmuch_config_set_user_other_email (notmuch_config_t *config,
 const char *list[],
@@ -658,6 +680,15 @@ notmuch_config_set_new_ignore (notmuch_config_t *config,
 (config-new_ignore));
 }
 
+void
+notmuch_config_set_new_scan (notmuch_config_t *config,
+const char *list[],
+size_t length)
+{
+_config_set_list (config, new, scan, list, length,
+(config-new_scan));
+}
+
 const char **
 notmuch_config_get_search_exclude_tags (notmuch_config_t *config, size_t 
*length)
 {
diff --git a/notmuch-new.c b/notmuch-new.c
index 938ae29..8377750 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -607,6 +607,16 @@ add_files (notmuch_database_t *notmuch,
 return ret;
 }
 
+/* Call out to the appropriate add_files function, based on the URI. */
+static notmuch_status_t
+add_files_uri (unused(notmuch_database_t *notmuch),
+  unused(const char *uri),
+  unused(add_files_state_t *state))
+{
+/* Stub for now */
+return NOTMUCH_STATUS_SUCCESS;
+}
+
 static void
 setup_progress_printing_timer (void)
 {
@@ -832,6 +842,8 @@ notmuch_new_command (void *ctx, int argc, char *argv

[PATCH v2 5/8] notmuch-new: pull out useful bits of add_files_recursive

2012-07-01 Thread Ethan Glasser-Camp
This patch pulls some bits out of add_files_recursive which will be
useful for other backends: two reporting functions
_report_before_adding_file and _report_added_file, as well as
_add_message, which actually does the message adding.

No functional changes.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 notmuch-new.c |  192 +++--
 1 file changed, 119 insertions(+), 73 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 8377750..5250562 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -239,6 +239,122 @@ _entry_in_ignore_list (const char *entry, 
add_files_state_t *state)
 return FALSE;
 }
 
+/* Progress-reporting function.
+ *
+ * Can be used by any mailstore-crawling function that wants to alert
+ * users what message it's about to add. Subsequent errors will be due
+ * to this message ;)
+ */
+static void
+_report_before_adding_file (add_files_state_t *state, const char *filename)
+{
+state-processed_files++;
+
+if (state-verbose) {
+   if (state-output_is_a_tty)
+   printf(\r\033[K);
+
+   printf (%i/%i: %s,
+   state-processed_files,
+   state-total_files,
+   filename);
+
+   putchar((state-output_is_a_tty) ? '\r' : '\n');
+   fflush (stdout);
+}
+}
+
+/* Progress-reporting function.
+ *
+ * Call this to respond to the signal handler for SIGALRM.
+ */
+static void
+_report_added_file (add_files_state_t *state)
+{
+if (do_print_progress) {
+   do_print_progress = 0;
+   generic_print_progress (Processed, files, state-tv_start,
+   state-processed_files, state-total_files);
+}
+}
+
+
+/* Atomically handles adding a message to the database.
+ *
+ * Should be used by any mailstore-crawling function that finds a new
+ * message to add.
+ */
+static notmuch_status_t
+_add_message (add_files_state_t *state, notmuch_database_t *notmuch,
+ const char *filename)
+{
+notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
+notmuch_message_t *message;
+const char **tag;
+
+status = notmuch_database_begin_atomic (notmuch);
+if (status) {
+   ret = status;
+   goto DONE;
+}
+
+status = notmuch_database_add_message (notmuch, filename, message);
+
+switch (status) {
+/* success */
+case NOTMUCH_STATUS_SUCCESS:
+   state-added_messages++;
+   notmuch_message_freeze (message);
+   for (tag=state-new_tags; *tag != NULL; tag++)
+   notmuch_message_add_tag (message, *tag);
+   if (state-synchronize_flags == TRUE)
+   notmuch_message_maildir_flags_to_tags (message);
+   notmuch_message_thaw (message);
+   break;
+/* Non-fatal issues (go on to next file) */
+case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
+   if (state-synchronize_flags == TRUE)
+   notmuch_message_maildir_flags_to_tags (message);
+   break;
+case NOTMUCH_STATUS_FILE_NOT_EMAIL:
+   fprintf (stderr, Note: Ignoring non-mail file: %s\n,
+filename);
+   break;
+/* Fatal issues. Don't process anymore. */
+case NOTMUCH_STATUS_READ_ONLY_DATABASE:
+case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
+case NOTMUCH_STATUS_OUT_OF_MEMORY:
+   fprintf (stderr, Error: %s. Halting processing.\n,
+notmuch_status_to_string (status));
+   ret = status;
+   goto DONE;
+default:
+case NOTMUCH_STATUS_FILE_ERROR:
+case NOTMUCH_STATUS_NULL_POINTER:
+case NOTMUCH_STATUS_TAG_TOO_LONG:
+case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:
+case NOTMUCH_STATUS_UNBALANCED_ATOMIC:
+case NOTMUCH_STATUS_LAST_STATUS:
+   INTERNAL_ERROR (add_message returned unexpected value: %d,  status);
+   ret = status;
+   goto DONE;
+}
+
+status = notmuch_database_end_atomic (notmuch);
+if (status) {
+   ret = status;
+   goto DONE;
+}
+
+  DONE:
+if (message) {
+   notmuch_message_destroy (message);
+   message = NULL;
+}
+
+return ret;
+}
+
 /* Examine 'path' recursively as follows:
  *
  *   o Ask the filesystem for the mtime of 'path' (fs_mtime)
@@ -290,7 +406,6 @@ add_files (notmuch_database_t *notmuch,
 char *next = NULL;
 time_t fs_mtime, db_mtime;
 notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
-notmuch_message_t *message = NULL;
 struct dirent **fs_entries = NULL;
 int i, num_fs_entries = 0, entry_type;
 notmuch_directory_t *directory;
@@ -299,7 +414,6 @@ add_files (notmuch_database_t *notmuch,
 time_t stat_time;
 struct stat st;
 notmuch_bool_t is_maildir;
-const char **tag;
 
 if (stat (path, st)) {
fprintf (stderr, Error reading directory %s: %s\n,
@@ -469,83 +583,15 @@ add_files (notmuch_database_t *notmuch,
 * in the database, so add it. */
next = talloc_asprintf (notmuch, %s/%s, path, entry-d_name);
 
-   state-processed_files++;
-
-   if (state-verbose

[PATCH v2 6/8] mailstore: support for mbox:// URIs

2012-07-01 Thread Ethan Glasser-Camp
No code uses this yet.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 lib/mailstore.c |  117 ++-
 1 file changed, 116 insertions(+), 1 deletion(-)

diff --git a/lib/mailstore.c b/lib/mailstore.c
index 48acd47..a29d734 100644
--- a/lib/mailstore.c
+++ b/lib/mailstore.c
@@ -17,18 +17,133 @@
  *
  * Author: Carl Worth cwo...@cworth.org
  */
+#include uriparser/Uri.h
 #include stdio.h
+#include glib.h
 
 #include notmuch-private.h
 
+static FILE *
+notmuch_mailstore_basic_open (const char *filename)
+{
+return fopen (filename, r);
+}
+
+/* Since we have to return a FILE*, we use fmemopen to turn buffers
+ * into FILE* streams. But when we close these streams, we have to
+ * free() the buffers. Use a hash to associate the two.
+ */
+static GHashTable *_mbox_files_to_strings = NULL;
+
+static void
+_ensure_mbox_files_to_strings () {
+if (_mbox_files_to_strings == NULL)
+_mbox_files_to_strings = g_hash_table_new (NULL, NULL);
+}
+
+static FILE *
+notmuch_mailstore_mbox_open (UriUriA *uri)
+{
+FILE *ret = NULL, *mbox = NULL;
+char *filename, *message, *length_s;
+const char *error;
+long int offset, length, this_read;
+_ensure_mbox_files_to_strings ();
+
+offset = strtol (uri-fragment.first, length_s, 10);
+length = strtol (length_s+1, NULL, 10);
+
+filename = talloc_strndup (NULL, uri-pathHead-text.first-1,
+   
uri-pathTail-text.afterLast-uri-pathHead-text.first+1);
+
+if (filename == NULL)
+goto DONE;
+
+mbox = fopen (filename, r);
+if (mbox == NULL) {
+fprintf (stderr, Couldn't open message %s: %s.\n, uri-scheme.first,
+ strerror (errno));
+goto DONE;
+}
+
+message = talloc_array (NULL, char, length);
+fseek (mbox, offset, SEEK_SET);
+
+this_read = fread (message, sizeof(char), length, mbox);
+if (this_read != length) {
+if (feof (mbox))
+error = end of file reached;
+if (ferror (mbox))
+error = strerror (ferror (mbox));
+
+fprintf (stderr, Couldn't read message %s: %s.\n, uri-scheme.first, 
error);
+goto DONE;
+}
+
+ret = fmemopen (message, length, r);
+if (ret == NULL) {
+/* No fclose will ever be called, so let's free message now */
+talloc_free (message);
+goto DONE;
+}
+
+g_hash_table_insert (_mbox_files_to_strings, ret, message);
+DONE:
+if (filename)
+talloc_free (filename);
+if (mbox)
+fclose (mbox);
+
+return ret;
+}
+
 FILE *
 notmuch_mailstore_open (const char *filename)
 {
-return fopen (filename, r);
+FILE *ret = NULL;
+UriUriA parsed;
+UriParserStateA state;
+state.uri = parsed;
+if (uriParseUriA (state, filename) != URI_SUCCESS) {
+/* Failure. Fall back to fopen and hope for the best. */
+ret = notmuch_mailstore_basic_open (filename);
+goto DONE;
+}
+
+if (parsed.scheme.first == NULL) {
+/* No scheme. Probably not really a URL but just an ordinary
+ * filename, such as a Maildir message. */
+ret = notmuch_mailstore_basic_open (filename);
+goto DONE;
+}
+
+if (0 == strncmp (parsed.scheme.first, mbox,
+  parsed.scheme.afterLast-parsed.scheme.first)) {
+/* mbox URI of the form mbox:///path/to/file#offset+length.
+ * Just pass the parsed URI. */
+ret = notmuch_mailstore_mbox_open (parsed);
+goto DONE;
+}
+
+/* Default case */
+fprintf (stderr, Error: Could not open URI %s: unknown scheme.\n,
+ filename);
+
+DONE:
+uriFreeUriMembersA (parsed);
+return ret;
 }
 
 int
 notmuch_mailstore_close (FILE *file)
 {
+char *file_buffer;
+if (_mbox_files_to_strings != NULL) {
+file_buffer = g_hash_table_lookup (_mbox_files_to_strings, file);
+if (file_buffer != NULL)
+talloc_free (file_buffer);
+
+g_hash_table_remove (_mbox_files_to_strings, file);
+}
 return fclose (file);
 }
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 7/8] Tests for mbox support

2012-07-01 Thread Ethan Glasser-Camp
These are simple tests of one single mbox.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
The test uses bash arrays, which have a slightly odd syntax for
appending.

 test/mbox |   63 +
 test/notmuch-test |1 +
 2 files changed, 64 insertions(+)
 create mode 100755 test/mbox

diff --git a/test/mbox b/test/mbox
new file mode 100755
index 000..a6dab99
--- /dev/null
+++ b/test/mbox
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='basic mbox support'
+. ./test-lib.sh
+
+mkdir -p $MAIL_DIR/some-mboxes/subdir $MAIL_DIR/database $MAIL_DIR/corpus
+
+# The Content-Length headers here include the final newline (added later).
+generate_message '[body]=Mbox message 1.' '[header]=Content-Length: 16' 
[dir]=corpus
+generate_message '[body]=Mbox message 2. Longer.' '[header]=Content-Length: 
24' [dir]=corpus
+generate_message '[body]=Mbox message 3.' [dir]=corpus
+generate_message '[body]=Mbox message 4.' [dir]=corpus
+generate_message '[body]=Mbox message 5. Last message.' 
'[header]=Content-Length: 30' [dir]=corpus
+
+MBOX1=$MAIL_DIR/some-mboxes/first.mbox
+declare -a starts lengths
+
+for x in $MAIL_DIR/corpus/*; do
+echo From MAILER-DAEMON Sat Jan  3 01:05:34 1996  $MBOX1
+starts+=(`wc -c $MBOX1 | cut -f 1 -d ' '`)
+cat $x  $MBOX1
+lengths+=(`wc -c $x | cut -f 1 -d ' '`)
+# Final newline
+echo  $MBOX1
+done
+
+notmuch config set database.path $MAIL_DIR/database
+notmuch config set new.scan mbox://$MAIL_DIR/some-mboxes
+
+test_begin_subtest read a small mbox (5 messages)
+output=$(NOTMUCH_NEW)
+test_expect_equal $output Added 5 new messages to the database.
+
+test_begin_subtest search
+output=$(notmuch search '*' | notmuch_search_sanitize)
+test_expect_equal $output thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; 
Test message #1 (inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Test message #2 (inbox 
unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Test message #3 (inbox 
unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Test message #4 (inbox 
unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Test message #5 (inbox 
unread)
+
+test_begin_subtest show (mboxcl)
+output=$(notmuch show Test message #1 | grep -o filename:[^ ]*)
+test_expect_equal $output 
filename:mbox://$MAIL_DIR/some-mboxes/first.mbox#${starts[0]}+${lengths[0]}
+
+test_begin_subtest show doesn't append an extra space at the end (mboxcl)
+output=$(notmuch show --format=raw Test message #1 )
+original=$(cat $MAIL_DIR/corpus/msg-001)
+test_expect_equal $output $original
+
+test_begin_subtest show (non-cl)
+output=$(notmuch show Test message #3 | grep -o filename:[^ ]*)
+test_expect_equal $output 
filename:mbox://$MAIL_DIR/some-mboxes/first.mbox#${starts[2]}+${lengths[2]}
+
+test_begin_subtest show doesn't append an extra space at the end (non-cl)
+output=$(notmuch show --format=raw Test message #3 )
+original=$(cat $MAIL_DIR/corpus/msg-003)
+test_expect_equal $output $original
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index bfad5d3..8cbb2cd 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -47,6 +47,7 @@ TESTS=
   emacs-large-search-buffer
   emacs-subject-to-filename
   maildir-sync
+  mbox
   crypto
   symbol-hiding
   search-folder-coherence
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 8/8] new: Add scan support for mbox:// URIs

2012-07-01 Thread Ethan Glasser-Camp
This fixes the broken tests introduced by the last commit.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
More text was added to clarify how mbox scanning works.

 notmuch-config.c |4 +
 notmuch-new.c|  304 +-
 2 files changed, 303 insertions(+), 5 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index 387f855..e02b6a9 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -56,6 +56,10 @@ static const char new_config_comment[] =
 \tEach URL denotes a \root\ which will be searched for mail 
files.\n
 \tHow this search is performed depends on the scheme of the URL 
(the\n
 \tpart before the first colon).\n
+\n
+\t\tmbox:///path  scans all subdirectories starting at path for mbox\n
+\t\t  files, and scans all mbox files for all messages.\n
+\n
 \tThe maildir located at database.path, above, will automatically 
be added.\n;
 
 static const char user_config_comment[] =
diff --git a/notmuch-new.c b/notmuch-new.c
index 5250562..061a1a8 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -19,6 +19,7 @@
  */
 
 #include notmuch-client.h
+#include uriparser/Uri.h
 
 #include unistd.h
 
@@ -653,14 +654,307 @@ add_files (notmuch_database_t *notmuch,
 return ret;
 }
 
+/* Scan an mbox file for messages.
+ *
+ * We assume that mboxes are append only -- this function does not
+ * check to see if messages have gone missing.
+ *
+ * The mtime of the mbox file is stored in a directory document in
+ * Xapian.
+ */
+/* FIXME: a certain amount of this code appears in add_files_recursive,
+ * and could be refactored
+ */
+static notmuch_status_t
+add_messages_mbox_file (notmuch_database_t *notmuch,
+   const char *path,
+   add_files_state_t *state)
+{
+notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, status;
+struct stat st;
+time_t fs_mtime, db_mtime, stat_time;
+FILE *mbox;
+char *line, *path_uri = NULL, *message_uri = NULL;
+int line_len;
+size_t offset, end_offset, line_size = 0;
+notmuch_directory_t *directory;
+int content_length = -1, is_headers;
+
+if (stat (path, st)) {
+   fprintf (stderr, Error reading mbox file %s: %s\n,
+path, strerror (errno));
+   return NOTMUCH_STATUS_FILE_ERROR;
+}
+
+stat_time = time (NULL);
+if (! S_ISREG (st.st_mode)) {
+   fprintf (stderr, Error: %s is not a file.\n, path);
+   return NOTMUCH_STATUS_FILE_ERROR;
+}
+
+fs_mtime = st.st_mtime;
+
+path_uri = talloc_asprintf (notmuch, mbox://%s, path);
+status = notmuch_database_get_directory (notmuch, path_uri, directory);
+if (status) {
+   ret = status;
+   goto DONE;
+}
+db_mtime = directory ? notmuch_directory_get_mtime (directory) : 0;
+
+if (directory  db_mtime == fs_mtime) {
+   goto DONE;
+}
+
+mbox = fopen (path, r);
+if (mbox == NULL) {
+   fprintf (stderr, Error: couldn't open %s for reading.\n,
+path);
+   ret = NOTMUCH_STATUS_FILE_ERROR;
+   goto DONE;
+}
+
+line_len = getline (line, line_size, mbox);
+
+if (line_len == -1) {
+   fprintf (stderr, Error: reading from %s failed: %s\n,
+path, strerror (errno));
+   ret = NOTMUCH_STATUS_FILE_ERROR;
+   goto DONE;
+}
+
+if (strncmp (line, From , 5) != 0) {
+   fprintf (stderr, Note: Ignoring non-mbox file: %s\n,
+path);
+   ret = NOTMUCH_STATUS_FILE_ERROR;
+   goto DONE;
+}
+
+/* Loop invariant: At the beginning of the loop, we have just read
+ * a From_ line, but haven't yet read any of the headers.
+ */
+while (! feof (mbox)) {
+   is_headers = 1;
+   offset = ftell (mbox);
+   content_length = -1;
+
+   /* Read lines until we either get to the next From_ header, or
+* we find a Content-Length header (mboxcl) and we run out of headers.
+*/
+   do {
+   /* Get the offset before we read, in case we got another From_ 
header. */
+   end_offset = ftell (mbox);
+
+   line_len = getline (line, line_size, mbox);
+
+   /* Check to see if this line is a content-length header,
+* or the end of the headers. */
+   if (is_headers  strncasecmp (line, Content-Length: ,
+  strlen (Content-Length: )) == 0)
+   content_length = strtol (line + strlen (Content-Length: ),
+NULL, 10);
+
+   if (is_headers  strlen (line) == 1  *line == '\n') {
+   is_headers = 0;
+   /* If we got a content_length, skip the message body. */
+   if (content_length != -1) {
+   fseek (mbox, content_length, SEEK_CUR);
+   line_len = getline (line, line_size, mbox);
+
+   /* We should be at the end

Re: [notmuch] [PATCH] Calls to notmuch get queued and executed asynchronously.

2012-10-11 Thread Ethan Glasser-Camp
James Vasile ja...@hackervisions.org writes:

 Added notmuch-enqueue-asynch to replace calls to
 notmuch-call-notmuch-process.  Calls to notmuch are then queued and
 executed asynchronously.  If the db is busy and we get an error saying
 it was locked, keep trying until the db is no longer busy.  Errors go
 in a buffer as per usual.

Hi! I decided to review some of the outstanding patches on the nmbug
queue.

This one doesn't apply cleanly. First, notmuch.el is now in
emacs/. Secondly, tagging happens only one place in notmuch-tag.el.

The follow-up patch doesn't apply either, and neither does the WIP patch
posted later in the thread by Aaron Ecay.

It seems like this patch series is useful for some people, but there are
some design issues yet to be worked out.

I propose that this thread be tagged notmuch::stale.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: More ideas about logging.

2012-10-12 Thread Ethan Glasser-Camp
Austin Clements amdra...@mit.edu writes:

 The trouble with this approach is that the OS doesn't have to flush
 logfile to the disk platters in any particular order relative to the
 updates to Xapian.  So, after someone trips over your plug, you could
 come back with Xapian saying you have 500 log entries when your
 logfile comes back with only 20.  The only way I know of to fix this
 is to fsync after the logfile write, which would obviously have
 performance issues.  But maybe there are cleverer ways?

Sorry to jump in almost a year after the fact, but..

How bad do you think those performance issues are going to be? I don't
see them as prohibitive, even in the case where you write a log entry
for every message being tagged. Xapian's doing an fsync each time we
commit, isn't it? (Or is there some cute trick where it rename()s the
database?)

If, instead of truncating, you replay logged operations, I think you can
get away with just writing log entries for user-level operations (like
notmuch tag +mytag to:somequery) which could touch a lot of
messages. This would then only require one fsync on the log file before
doing a lot of tag updates.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] test: add check for expected filename argument for test_expect_equal_file

2012-10-12 Thread Ethan Glasser-Camp
Dmitry Kurochkin dmitry.kuroch...@gmail.com writes:

 Actually, we can do both: check file name for consistent diff order
 (from expected to actual) and use file names that the caller provides.

Hi! Reviewing the patch queue a little bit here. It seems like this
patch ended up getting dropped because the other approach (using the
filenames that the caller provided) won out and was even pushed. I'm
therefore marking this series notmuch::obsolete (and also
notmuch::stale since it doesn't apply cleanly).

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2] emacs: Add more processing of displayed headers.

2012-10-12 Thread Ethan Glasser-Camp
Hi! Just going through the patch queue.

This is definitely a nice effect, but I'm not sure of the approach. It
doesn't indent the message's tags, and it doesn't work when you resize the
window. (You can get some very ugly wrapping if you put your mind to
it.)

Is there no better way to do this using visual-line-mode? I know that
the rest of notmuch uses hard filling, but that's no reason to make a
bad situation worse. It looks like you can put wrap-prefix text
properties all over, as done in the adaptive-wrap package:

http://elpa.gnu.org/packages/adaptive-wrap-0.1.el

Slightly more nit-picky comments below.

David Edmondson d...@dme.org writes:

 -(defvar notmuch-show-markup-headers-hook '(notmuch-show-colour-headers)
 +(defcustom notmuch-show-markup-headers-hook '(notmuch-show-colour-headers
 +   notmuch-show-fill-headers
 +   notmuch-show-indent-continuations)
A list of functions called to decorate the headers listed in
 -`notmuch-message-headers'.)
 +`notmuch-message-headers'.
 +  :type 'hook
 +  :options '(notmuch-show-colour-headers
 +  notmuch-show-fill-headers
 +  notmuch-show-indent-continuations)
 +  :group 'notmuch-show)

This hook is not normal because it takes an argument, and so should have
a name ending in -hooks or -functions. Also, since it's a defcustom now,
it should probably have a better explanation of how it works, that it
takes an argument, and what that argument means.

It seems extremely dicey to me that you can put
notmuch-show-indent-continuations in this list before, or without,
notmuch-show-fill-headers.

 +(defun notmuch-show-fill-headers (depth)
 +  Wrap the text of the current headers.
 +
 +  ;; '-5' to allow for the indentation code.
 +  (let ((fill-column (- (window-width) depth 5)))

It took me a little while to figure out what this meant. How about,
underfill by 5 so that inserting indentation doesn't cause more
wrapping? Is it possible to be smart enough to let

 +(defun notmuch-show-indent-continuations (depth)
 +  Indent any continuation lines.
 +  (goto-char (point-min))
 +  (while (not (eobp))
 +(if (not (looking-at ^[A-Za-z][-A-Za-z0-9]*:))
 + ;; Four spaces tends to work well with 'To' and 'Cc' headers.
 + (insert ))
 +(forward-line)))

I'm not crazy about this but I'm not sure I can say why exactly. Why
can't we just run indent-rigidly over the whole thing?

The comment isn't terribly useful. Only those headers? Tends to work
well?

  ;; Override `notmuch-message-headers' to force `From' to be
  ;; displayed.
  (let ((notmuch-message-headers '(From Subject To Cc Date)))
 -  (notmuch-show-insert-headers (plist-get message :headers)))
 +  (notmuch-show-insert-headers (plist-get message :headers) 0))

This took me a long while to figure out, especially because it looks
like the depth is being passed normally to notmuch-show-insert-bodypart,
just below. A comment like depth = 0 because we reindent below would
have been really helpful. A good test message is
id:87ocabvp0y@wsheee.2x.cz.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: another test wrt ignoring user-specified files and directories

2012-10-12 Thread Ethan Glasser-Camp
From: Pieter Praet pie...@praet.org

Demonstrates that *every* file/directory which matches one of the values
in 'new.ignore' will be ignored, independent of its depth/location in
the mail store.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
This is the trivial modification of Pieter's patch that makes it work
no matter where your test directory is.

 test/new |   19 +++
 1 file changed, 19 insertions(+)

diff --git a/test/new b/test/new
index cab7c01..cc2af72 100755
--- a/test/new
+++ b/test/new
@@ -183,5 +183,24 @@ touch ${MAIL_DIR}/.git # change .git's mtime for notmuch 
new to rescan.
 output=$(NOTMUCH_NEW 21)
 test_expect_equal $output Added 1 new message to the database.
 
+test_begin_subtest Ignore files and directories specified in new.ignore 
(multiple occurrences)
+notmuch config set new.ignore .git ignored_file .ignored_hidden_file
+touch ${MAIL_DIR}/.git # change .git's mtime for notmuch new to rescan.
+mkdir -p ${MAIL_DIR}/one/two/three/.git
+notmuch new  /dev/null # ensure that files/folders will be printed in ASCII 
order.
+touch ${MAIL_DIR}/{one,one/two,one/two/three}/ignored_file
+output=$(NOTMUCH_NEW --debug 21)
+test_expect_equal $output \
+(D) add_files_recursive, pass 1: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/.ignored_hidden_file
+(D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/ignored_file
+No new mail.
+
 
 test_done
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: handle filenames that have directories in them

2012-10-12 Thread Ethan Glasser-Camp
Since $TEST_DIRECTORY is an absolute path, any filenames generated
with it will be complete paths. Only use the basename to generate
suffixes for filenames.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
Discovered this while reviewing the patch queue. test/emacs generates
filenames using $TEST_DIRECTORY, which is generated using pwd(1). Test
failures then cause failures in the test harness.

 test/test-lib.sh |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 7448b45..8de5e32 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -498,16 +498,18 @@ test_expect_equal_file ()
error bug in the test script: not 2 or 3 parameters to 
test_expect_equal
 
file1=$1
+   basename1=`basename $file1`
file2=$2
+   basename2=`basename $file2`
if ! test_skip $test_subtest_name
then
if diff -q $file1 $file2 /dev/null ; then
test_ok_ $test_subtest_name
else
testname=$this_test.$test_count
-   cp $file1 $testname.$file1
-   cp $file2 $testname.$file2
-   test_failure_ $test_subtest_name $(diff -u 
$testname.$file1 $testname.$file2)
+   cp $file1 $testname.$basename1
+   cp $file2 $testname.$basename2
+   test_failure_ $test_subtest_name $(diff -u 
$testname.$basename1 $testname.$basename2)
fi
 fi
 }
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v3 2/2] test: Update test to match previous patch.

2012-10-12 Thread Ethan Glasser-Camp
David Edmondson d...@dme.org writes:

 Indentation now uses tabs where possible.

Hi! Just working through the patch queue. This patch is tagged
notmuch::moreinfo, although it seems like the rest of the series may
have been tagged notmuch::stale or notmuch::pushed. It's a little hard
to figure out, because the thread structure is really messed up. Is this
patch still viable? It now *breaks* the emacs test on my machine.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'

2012-10-12 Thread Ethan Glasser-Camp
Pieter Praet pie...@praet.org writes:

 * emacs/notmuch-show.el

   (notmuch-show-toggle-headers):
 Rename to `notmuch-show-toggle-visibility-headers'.

This patch, and its predecessors, all look great to me. The following
patches were already marked stale (and indeed they don't apply). But
it looks like David Edmonson gave his +1 on the idea, so maybe you
should rebase them.

Side note: how do you write these tests? How do you generate what the
output should look like? Copy/paste from a real emacs session? You
have generated an awful lot of tests :)

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v4 2/9] parse-time-string: add a date/time parser to notmuch

2012-10-14 Thread Ethan Glasser-Camp
Jani Nikula j...@nikula.org writes:

Hi! I commend you for your work and persistence. This represents a lot
of work and I think it's good enough to be merged. I would certainly
love to see last and ago supported but this patch series, and this
patch especially, is cumbersome enough that I'd really rather it be
merged before it gets any worse.

If this patch series isn't accepted, I'd suggest making a patch series
that makes a null date parser, which just takes strings of date
timestamps the way notmuch does now: 12345 - (time_t) 12345. We're
going to need a date parser anyhow, so a patch series that sets up the
scaffolding -- separate directory, tests -- could be merged without the
actual parser. Then you'd have fewer patches to juggle the next time
around.

I agree with Tomi Ollila that whether the parser is built in bison or in
straight C, as this one is, isn't important. The difficult part of
parsing dates isn't the translation from text to parse tree; it's
dealing with all the semantic difficulty that comes YYMMDD versus DDMMYY
and the difference between last week and last Thursday.

I have a few minor quibbles that I would be happy to see addressed after
this was merged, or not at all.

 +/* Parse a previously postponed number if one exists. */
 +static int parse_postponed_number (struct state *state, int v, int n, char 
 d);
 +static int
 +handle_postponed_number (struct state *state, enum field next_field)
 +{
 +int v = state-postponed_value;
 +int n = state-postponed_length;
 +char d = state-postponed_delim;
 +int r;
 +
 +if (!n)
 + return 0;
 +
 +state-postponed_value = 0;
 +state-postponed_length = 0;
 +state-postponed_delim = 0;

This could be refactored to be a call to get_postponed_number. Also, I'd
prefer parse_postponed_number be up here, closer to its sole caller 
(handle_postponed_number).

 +/*
 + * Postpone a number to be handled later. If one exists already,
 + * handle it first. n may be -1 to indicate a keyword that has no
 + * number length.
 + */
 +static int
 +set_postponed_number (struct state *state, int v, int n)
 +{
 +int r;
 +char d = state-delim;
 +
 +/* Parse a previously postponed number, if any. */
 +r = handle_postponed_number (state, TM_NONE);
 +if (r)
 + return r;

I would love a comment explaining under what circumstances this could
occur and what the caller is expected to do.

 +/*
 + * Accepted keywords.
 + */
 +static struct keyword keywords[] = {
 +/* Weekdays. */
 +{ N_(sun|day), TM_ABS_WDAY,0,  NULL },
 +{ N_(mon|day), TM_ABS_WDAY,1,  NULL },

Maybe it's just my history with Python, but I'd prefer keywords, which
is a global and a constant, to be written in all caps (KEYWORDS).

 +/*
 + * Compare strings s and keyword. Return number of matching chars on
 + * match, 0 for no match. Match must be at least n chars, or all of
 + * keyword if n  0, otherwise it's not a match. Use match_case for
 + * case sensitive matching.
 + */
 +static size_t
 +stringcmp (const char *s, const char *keyword, ssize_t n, bool match_case)
 +{

The name of this function makes it look uncomfortably like strcmp(3),
which has a very different calling semantics (specifically the -1, 0, 1
return value). I'd prefer a name like string_match_keyword.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 3/4] Private strsep implementation

2012-10-14 Thread Ethan Glasser-Camp
vladimir.ma...@oracle.com writes:

 From: Vladimir Marek vlma...@volny.cz

 strsep is not available on Solaris 10, so we stole the one used by
 mutt.

Hi! Just going through the patch queue. This patch looks fine to me, but
it no longer applies cleanly to master. Can you rebase it? It'll have my
+1.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] test: emacs mailto: URI handling

2012-10-15 Thread Ethan Glasser-Camp
Jameson Graef Rollins jroll...@finestructure.net writes:

 This adds a test for proposed rfc6068 mailto:; URI handling.  The
 proposed function would be called 'notmuch-mua-mailto'.  The test
 provides an example mailto: string that should test some subset of the
 rfc6068 specification: http://tools.ietf.org/html/rfc6068

Hi! Just going through the patch queue. This test, at least, looks
sane, so why not apply it?

As for the mailto: handler itself, you wrote in
id:87vcin2fo6@servo.finestructure.net that you would add something
to the next version, thereby implying that there would be a next
version. Do you still intend to release such a new version soon? I would
love to review it. This version looks fine to me, with one minor concern.

 +;; FIXME: need to decode all html encodings in uri.
 +(mailto (replace-regexp-in-string amp;  mailto))

This is odd. Shouldn't the navigator be decoding all that stuff while
parsing, before you even get to it? How often do you get mailto links
with amp; in them? You obviously wouldn't have put it in there if it
weren't necessary.

I would be +1 on merging this patch or the next version.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v8 0/8] reworked crypto toggle, plus a few other toggles

2012-10-15 Thread Ethan Glasser-Camp
Pieter Praet pie...@praet.org writes:

 Great work!

 Here's some tests.

Hi! These look fine to me. With Mark's review
(id:87k41e45hi@qmul.ac.uk), I'm removing the needs-review tag.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 3/3] new: don't read unchanged directories from disk

2012-10-15 Thread Ethan Glasser-Camp
Sascha Silbe sascha-...@silbe.org writes:

 Previously, notmuch new listed all directories on disk, even if they
 were unchanged from the state recorded in the database. This could take
 a huge amount of time for large numbers of mails as it would list each
 individual mail.

 By iterating over the subdirectories recorded in the database we can
 avoid accessing the file system for each unchanged directory. If the
 modification time does not match we fall back to a full file system scan
 so new subdirectories will get picked up and scanned recursively.

Hi! Just going through the patch queue. These patches look fine to me,
but I think architecturally, notmuch needs to decide what it wants to do
-- if it wants to optimize directory-rescanning, or if it wants to
provide a better way to just add files known to be new, and let mail
delivery hook into that system. I'd prefer the second, but I think it's
OK to merge working code that does the first in the meantime. The
perfect is the enemy of the good.

Austin raised some concerns on this patch set that I'm not sure were
completely addressed. For my part, it seems intuitive that a stat()
would be faster than a scandir() and so I would have no problem with
merging this series.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 2/6] test: emacs: new test notmuch-show: change tags of all messages in current buffer

2012-10-15 Thread Ethan Glasser-Camp
Pieter Praet pie...@praet.org writes:

 * test/emacs:

   New subtest notmuch-show: change tags of all messages in current buffer:
   `notmuch-show-tag-all' (*) changes tags of *all* messages in current 
 buffer.
 ---
  test/emacs |   15 +++
  1 files changed, 15 insertions(+), 0 deletions(-)

 diff --git a/test/emacs b/test/emacs
 index ec1dbb0..d2dbafc 100755
 --- a/test/emacs
 +++ b/test/emacs
 @@ -136,6 +136,21 @@ test_emacs (notmuch-show \$os_x_darwin_thread\)
  output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
  test_expect_equal $output thread:XXX   2009-11-18 [4/4] Jjgod Jiang, 
 Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox 
 unread)

 +test_begin_subtest notmuch-show: change tags of all messages in current 
 buffer
 +query=$os_x_darwin_thread
 +filter=from:Jiang
 +add_tag=notmuch-show-tag-all
 +del_tag=inbox
 +count_total=$(notmuch count -- $query)   # = 4
 +count_match=$(notmuch count -- $query AND $filter) # = 2

In this test, what use is count_match? Just so that the tests' forms
are the same? Maybe you want to put an assertion that count_total !=
count_match, just for sanity's sake?

Otherwise, patches 1-3 look fine to me.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'

2012-10-15 Thread Ethan Glasser-Camp
Pieter Praet pie...@praet.org writes:

 * emacs/notmuch-show.el (notmuch-show-mapc):

   If provided with optional argument PREDICATE, only call
   FUNCTION if calling PREDICATE returns non-nil.

   Also correct original docstring: 's/thread/buffer/'.
 ---

This patch was marked stale, but isn't.

 -(defun notmuch-show-mapc (function)
 -  Iterate through all messages in the current thread with
 +(defun notmuch-show-mapc (function optional predicate)
 +  Iterate through all messages in the current buffer with
  `notmuch-show-goto-message-next' and call FUNCTION for side
 -effects.
 +effects.
 +
 +If provided with optional argument PREDICATE, only call
 +FUNCTION if calling PREDICATE returns non-nil.
(save-excursion
  (goto-char (point-min))
 -(loop do (funcall function)
 +(loop do (if predicate
 +  (if (funcall predicate)
 +  (funcall function))
 +(funcall function))

I don't like the way this if-structure looks, since I have to squint to
see whether the else clause matches the inner or the outer if. Maybe
change the inner if to a when or an and?

Ethan

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'

2012-10-15 Thread Ethan Glasser-Camp
Mark Walters markwalters1...@gmail.com writes:

 The original function feels a little fragile to me as to what happens if
 predicate or function move point. Eg what happens if function collapses
 the message: where does point go, and so where does
 notmuch-show-goto-message-next go. Is this just my naivete as a lisp
 beginner? Is there someway of writing it so the user doesn't need to
 worry about such things?

Although collapsing the message doesn't seem to move point, it would
probably be a good idea to wrap the calls to predicate and function in
save-excursion, as a guard against subtle and hard-to-spot bugs with
operations not being applied to all the right messages..

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 5/6] emacs: simplify `notmuch-show-get-messages-ids{, -search}'

2012-10-15 Thread Ethan Glasser-Camp
Mark Walters markwalters1...@gmail.com writes:

 I like the use of separator rather than hard-wiring  or . My personal
 preference would be to make that change but keep the two functions
 separate (my Cness makes me like functions that have clear return
 types!) But I am happy with the change too.

I agree with this comment. Especially in dynamic languages, it's a good
idea for each function to have only one return type.

Also, it seems that the commit message has a TAB character in it 
(,TAB-search).

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 6/6] emacs: `notmuch-show-tag-all' with prefix arg only tags open messages

2012-10-15 Thread Ethan Glasser-Camp
Pieter Praet pie...@praet.org writes:

 * emacs/notmuch-show.el

   (notmuch-show-get-messages-ids):
 If provided with optional argument PREDICATE, only return
 Message-Id's of messages for which PREDICATE returns non-nil.

   (notmuch-show-tag-all):
 New argument ONLY-OPEN (set to `current-prefix-arg' if running
 interactively): if non-nil, only change tags of *open* messages.
 Also correct original docstring: 's/thread/buffer/'.

   (notmuch-show-archive-thread):
 Update wrt changes to `notmuch-show-tag-all'.

 * test/emacs

   - Subtest notmuch-show: change tags of open messages in current buffer
 is no longer broken...

This patch is stale, but in case it helps..

 ---
  emacs/notmuch-show.el |   33 -
  test/emacs|1 -
  2 files changed, 24 insertions(+), 10 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index 05606fc..4bd1a7c 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -1339,14 +1339,18 @@ (defun notmuch-show-get-message-id ()
Return the message id of the current message.
(concat id:\ (notmuch-show-get-prop :id) \))
  
 -(defun notmuch-show-get-messages-ids (optional separator)
 +(defun notmuch-show-get-messages-ids (optional separator predicate)
Return a list of Message-Id's of all messages in the current buffer.
  
  If provided with optional argument SEPARATOR, return a string
 -instead, consisting of all Message-Id's separated by SEPARATOR.
 +instead, consisting of all Message-Id's separated by SEPARATOR.
 +
 +If provided with optional argument PREDICATE, only return
 +Message-Id's of messages for which PREDICATE returns non-nil.
(let ((message-ids))
  (notmuch-show-mapc
 - (lambda () (push (notmuch-show-get-message-id) message-ids)))
 + (lambda () (push (notmuch-show-get-message-id) message-ids))
 + predicate)
  (if separator
   (mapconcat 'identity message-ids separator)
message-ids)))
 @@ -1633,18 +1637,29 @@ (defun notmuch-show-tag (optional initial-input)
 initial-input (notmuch-show-get-message-id
  (apply 'notmuch-show-tag-message tag-changes)))
  
 -(defun notmuch-show-tag-all (rest tag-changes)
 -  Change tags for all messages in the current thread.
 +(defun notmuch-show-tag-all (only-open rest tag-changes)
 +  Change tags of all messages in the current buffer.

I'm not crazy about notmuch-show-tag-all having an argument to control
whether or not it tags all. Introduce another function, or perhaps
change this one's name?

I also don't really like that the only-open argument comes before the
tag changes. This means changing every caller (although I guess there's
just one right now). I think tag-changes are more important and should
come first. (tag-changes are optional instead of rest in master, so
you can just put only-open after instead of before.)

 +
 +If ONLY-OPEN is non-nil, only change tags of *open* messages in
 +the current buffer.
  
  TAG-CHANGES is a list of tag operations for `notmuch-tag'.
 -  (interactive (notmuch-read-tag-changes nil notmuch-show-thread-id))
 -  (apply 'notmuch-tag (notmuch-show-get-messages-ids  or ) tag-changes)
 +  (interactive (cons current-prefix-arg
 +  (notmuch-read-tag-changes nil notmuch-show-thread-id)))
 +  (apply 'notmuch-tag
 +  (notmuch-show-get-messages-ids
 +or 
 +   `(lambda ()
 +  ,(if only-open '(notmuch-show-message-visible-p) t)))
 +  tag-changes)

This is a very awkward use of backquote, to my eyes. Besides, can't
you just replace this with (if only-open 'notmuch-show-message-visible-p
nil) ?

(notmuch-show-mapc
 (lambda ()
   (let* ((current-tags (notmuch-show-get-tags))
   (new-tags (notmuch-update-tags current-tags tag-changes)))
 (unless (equal current-tags new-tags)
 -  (notmuch-show-set-tags new-tags))
 +  (notmuch-show-set-tags new-tags
 +   `(lambda ()
 +  ,(if only-open '(notmuch-show-message-visible-p) t

Same.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] Add notmuch_database_flush method

2012-10-17 Thread Ethan Glasser-Camp
Adrien Bustany adr...@bustany.org writes:

 This method explicitly flushes the pending modifications to disk. It is
 useful if your program has various threads, each with a read only DB and
 one writer thread with a read/write DB. In that case, you most likely
 want the writer to sync the changes to disk so that the readers can see
 them, without having to close and reopen the database completely.

These patches are pretty straightforward. But to conform to notmuch style..

 +notmuch_status_t
 +notmuch_database_flush(notmuch_database_t *notmuch)
 +{
 + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;

Indent is 4 spaces. (You have tabs here, which are 8 spaces, according
to devel/STYLE.)

 + try {
 + if (notmuch-xapian_db != NULL 

if should be more indented than try. (So when you pull try back to 4
spaces, leave if at 8 spaces.)

 + notmuch-mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
 + (static_cast Xapian::WritableDatabase * 
 (notmuch-xapian_db))-flush ();

This line is 90 characters, and will remain at 86 once you indent using
the in-house style. I'm not sure if it's worth reformatting.
notmuch_database_close calls flush() using exactly the same 86-character
line. I'd say don't make it worse, but personally I think breaking
this line might be worse.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/3] test: emacs: toggle eliding of non-matching messages in `notmuch-show'

2012-10-17 Thread Ethan Glasser-Camp
From: Pieter Praet pie...@praet.org

See commits 44a544ed, 866ce8b1, 668b66ec.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
I am embarrassed to admit I didn't try to apply these patches before I
removed the needs-review tag. This one didn't apply. Here's the
trivial fix. The tests are still placed at the bottom of test/emacs
and not in test/emacs-show. The other two patches should apply without
change.

 test/emacs |   20 
 .../notmuch-show-process-crypto-mime-parts-off |   31 +++
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 3 files changed, 83 insertions(+)
 create mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
 create mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on

diff --git a/test/emacs b/test/emacs
index 1f84b91..58ea59a 100755
--- a/test/emacs
+++ b/test/emacs
@@ -783,4 +783,24 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 
+test_begin_subtest don't process cryptographic MIME parts
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-off
+
+test_begin_subtest process cryptographic MIME parts
+test_emacs '(let ((notmuch-crypto-process-mime t))
+   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
+
+test_begin_subtest process cryptographic MIME parts (w/ 
notmuch-show-toggle-process-crypto)
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
+   (notmuch-show-toggle-process-crypto)
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
+
+
 test_done
diff --git 
a/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off 
b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
new file mode 100644
index 000..076083a
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
@@ -0,0 +1,31 @@
+Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-17) (inbox signed)
+Subject: [notmuch] Working with Maildir storage?
+ Mikhail Gusarov dotted...@dottedmag.net (2009-11-17) (inbox signed unread)
+  Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-17) (inbox signed)
+  Subject: Re: [notmuch] Working with Maildir storage?
+  To: Mikhail Gusarov dotted...@dottedmag.net
+  Cc: notmuch@notmuchmail.org
+  Date: Tue, 17 Nov 2009 15:33:01 -0500
+
+  [ multipart/mixed ]
+  [ multipart/signed ]
+  [ text/plain ]
+   See the patch just posted here.
+
+  Is the list archived anywhere?  The obvious archives
+  (http://notmuchmail.org/pipermail/notmuch/) aren't available, and I
+  think I subscribed too late to get the patch (I only just saw the
+  discussion about it).
+
+  It doesn't look like the patch is in git yet.
+
+  -- Lars
+
+  [ 4-line signature. Click/Enter to show. ]
+  [ application/pgp-signature ]
+  [ text/plain ]
+  [ 4-line signature. Click/Enter to show. ]
+   Mikhail Gusarov dotted...@dottedmag.net (2009-11-17) (inbox unread)
+   Keith Packard kei...@keithp.com (2009-11-17) (inbox unread)
+Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-18) (inbox signed 
unread)
+ Carl Worth cwo...@cworth.org (2009-11-18) (inbox unread)
diff --git 
a/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on 
b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on
new file mode 100644
index 000..588f38f
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on
@@ -0,0 +1,32 @@
+Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-17) (inbox signed)
+Subject: [notmuch] Working with Maildir storage?
+ Mikhail Gusarov dotted...@dottedmag.net (2009-11-17) (inbox signed unread)
+  Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-17) (inbox signed)
+  Subject: Re: [notmuch] Working with Maildir storage?
+  To: Mikhail Gusarov dotted...@dottedmag.net
+  Cc: notmuch@notmuchmail.org
+  Date: Tue, 17 Nov 2009 15:33:01 -0500
+
+  [ multipart/mixed ]
+  [ multipart/signed ]
+  [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
+  [ text/plain ]
+   See the patch just posted here.
+
+  Is the list archived anywhere?  The obvious archives
+  (http://notmuchmail.org/pipermail/notmuch/) aren't available, and I
+  think I subscribed too late to get the patch (I only just saw the
+  discussion about it).
+
+  It doesn't look like the patch is in git yet.
+
+  -- Lars
+
+  [ 4-line signature. Click/Enter to show. ]
+  [ application/pgp-signature ]
+  [ text/plain ]
+  [ 4-line signature. Click/Enter to show. ]
+   Mikhail Gusarov dotted

Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'

2012-10-17 Thread Ethan Glasser-Camp
Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 This patch, and its predecessors, all look great to me.

But a note: many of the first lines in your commit messages ({show,
hide} message headers) contain tabs. I hate tabs. Is this intentional?
I have noticed it on other patches you've sent (such as
id:1330122640-18895-6-git-send-email-pie...@praet.org and
id:1330122640-18895-7-git-send-email-pie...@praet.org).

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: Move tests from emacs to emacs-show

2012-10-17 Thread Ethan Glasser-Camp
This requires changing the contents of the crypto tests, as one thread
that was marked read by the earlier tests in test/emacs is no longer
marked read.

This moves tests for:

- 09d19ac test: emacs: toggle eliding of non-matching messages in
   `notmuch-show', which should have actually read: test: emacs:
   toggle processing of cryptographic MIME parts in `notmuch-show'.
   See commit 19ec74c5.

- 5ea1dbe test: emacs: toggle eliding of non-matching messages in
  `notmuch-show'

- 345faab test: emacs: toggle thread content indentation in
  `notmuch-show'

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
I screwed up with git commit --amend or something on the last patch,
so David Bremner suggested that I take advantage of the situation to
write this patch, which does something useful as a side effect.

 test/emacs |   67 -
 test/emacs-show|   71 ++
 .../notmuch-show-elide-non-matching-messages-off   |   79 
 .../notmuch-show-elide-non-matching-messages-on|   75 +++
 .../notmuch-show-indent-thread-content-off |   79 
 .../notmuch-show-process-crypto-mime-parts-off |   31 
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 .../notmuch-show-elide-non-matching-messages-off   |   79 
 .../notmuch-show-elide-non-matching-messages-on|   75 ---
 .../notmuch-show-indent-thread-content-off |   79 
 .../notmuch-show-process-crypto-mime-parts-off |   31 
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 12 files changed, 367 insertions(+), 363 deletions(-)
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-on
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-indent-thread-content-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-on
 delete mode 100644 
test/emacs.expected-output/notmuch-show-elide-non-matching-messages-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-elide-non-matching-messages-on
 delete mode 100644 
test/emacs.expected-output/notmuch-show-indent-thread-content-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on

diff --git a/test/emacs b/test/emacs
index e9d8239..1f84b91 100755
--- a/test/emacs
+++ b/test/emacs
@@ -783,71 +783,4 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 
-test_begin_subtest don't process cryptographic MIME parts
-test_emacs '(let ((notmuch-crypto-process-mime nil))
-   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-off
-
-test_begin_subtest process cryptographic MIME parts
-test_emacs '(let ((notmuch-crypto-process-mime t))
-   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
-
-test_begin_subtest process cryptographic MIME parts (w/ 
notmuch-show-toggle-process-crypto)
-test_emacs '(let ((notmuch-crypto-process-mime nil))
-   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
-   (notmuch-show-toggle-process-crypto)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
-
-test_begin_subtest notmuch-show: don't elide non-matching messages
-test_emacs '(let ((notmuch-show-only-matching-messages nil))
-   (notmuch-search from:l...@seas.harvard.edu and subject:\Maildir 
storage\)
-   (notmuch-test-wait)
-   (notmuch-search-show-thread)
-   (notmuch-test-wait)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-elide-non-matching-messages-off
-
-test_begin_subtest notmuch-show: elide non-matching messages
-test_emacs '(let ((notmuch-show-only-matching-messages t))
-   (notmuch-search from:l...@seas.harvard.edu and subject:\Maildir 
storage\)
-   (notmuch-test-wait)
-   (notmuch-search-show-thread)
-   (notmuch-test-wait)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-elide-non-matching-messages-on
-
-test_begin_subtest notmuch-show: elide non-matching messages (w/ 
notmuch-show-toggle-elide-non-matching)
-test_emacs '(let ((notmuch-show-only-matching-messages nil))
-   (notmuch-search from:l...@seas.harvard.edu and subject:\Maildir 
storage\)
-   (notmuch-test

Re: [PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

2012-10-18 Thread Ethan Glasser-Camp
Adrien Bustany adr...@bustany.org writes:

 The code of the patches in unchanged, but the formatting issues are now
 hopefully fixed.

These look fine to me, and they're pretty trivial.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: Notmuch scripts (again), now with more usenet

2012-10-18 Thread Ethan Glasser-Camp
c...@webprojekty.cz writes:

 Hello, for quite some time my set of scripts just lied in my repo and
 waited for polish before release. So tonight I finally managed to update
 the docs, remove old stuff, rewrite some unfortunate things etc.

 One notable addition is slrn2maildir script which can convert NNTP
 spool, eg. gmane mailing lists or blogs, as fetched by slrnpull to
 maildir format. This way you can follow plethora of mailing lists
 without subscribing, any blog that publishes full atom/rss feed or
 usenet newsgroup.

 For details see the readme:
   http://webprojekty.cz/ccx/loggerhead/zmuch/view/head:/README
 or check out the code:
   bzr branch http://webprojekty.cz/ccx/bzr/zmuch

 I hope it's now in the form acceptable for inclusion to contrib.

Hi! Sorry about the delay, but I'm going through the patch queue now and
it seems like this branch is just completely gone. I get 502 Bad Gateway
errors when I follow the first link. Did it move or is there a problem
with your site?

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

2012-10-18 Thread Ethan Glasser-Camp
Peter Wang noval...@gmail.com writes:

 Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
 cover all four values of search --exclude in the cli.

This series looks good to me. It's a nice clean up and a nice new
feature. Patches all apply.

However, I'm getting test failures like:

 FAIL   Search, exclude deleted messages from message search --exclude=false
--- excludes.3.expected 2012-10-19 04:45:06.900518377 +
+++ excludes.3.output   2012-10-19 04:45:06.900518377 +
@@ -1,2 +1,2 @@
-id:msg-001@notmuch-test-suite
 id:msg-002@notmuch-test-suite
+id:msg-001@notmuch-test-suite

 FAIL   Search, don't exclude deleted messages when --exclude=flag specified
--- excludes.7.expected 2012-10-19 04:45:07.004518378 +
+++ excludes.7.output   2012-10-19 04:45:07.004518378 +
@@ -1,2 +1,2 @@
-thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)
 thread:XXX   2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply 
(deleted inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)

 FAIL   Search, don't exclude deleted messages from search if not configured
--- excludes.8.expected 2012-10-19 04:45:07.028518377 +
+++ excludes.8.output   2012-10-19 04:45:07.028518377 +
@@ -1,2 +1,2 @@
-thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)
 thread:XXX   2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted 
inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)

In other words, threads and messages are coming up out of order. I'm not
sure of the right way to fix this. If you would like me to try sticking
| sort here and there in the tests I will do so. I'm not sure if the
test suite is guaranteed to scan messages in a certain order.

Mark Walters wrote in
id:1340198947-29370-5-git-send-email-noval...@gmail.com that he
thought patch 1/8 seemed more intrusive than he liked. Maybe I just have
a higher standard for intrusive than he does ;) but I thought it was
fine.

It looks like you have better wording for patch 4/8 so I'd like to see
you resend it.

 - if (query-omit_excluded != NOTMUCH_EXCLUDE_FALSE)
 + if (query-omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
 + query-omit_excluded == NOTMUCH_EXCLUDE_ALL)
 + {
   final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
final_query, exclude_query);
 - else {
 + } else {

House style is to not put braces around one-line then-clauses. This is
the only place where you did that.

I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] Add NEWS item for multipart/alternative toggle

2012-10-19 Thread Ethan Glasser-Camp
Users who relied on notmuch-show-all-multipart/alternative-parts
might need to know that it is now buffer-local.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
Hi! I'm trying to figure out the status of this patch series, which
seems to have fallen through the cracks. It looks like Jani's solution
exists and works, though it isn't perfect. I would like to get Jani's
solution applied until someone who is sufficiently motivated decides to
work on fancy cycle-multipart-message things. It seems like the last
hurdle is that Mark would like Jani to put the buffer-localization of
notmuch-show-all-multipart/alternative-parts in NEWS. Here's that.

However, it also seems like there was a disagreement about design
direction -- should multiple alternative parts be toggled so you can
see them all at once, or cycled so you can see one at a time? See
id:87d32yadl5@qmul.ac.uk. I'm for the code that works now which
is Jani's patch.

 NEWS |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/NEWS b/NEWS
index 2b50ba3..fc9b50a 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,14 @@
+Emacs Interface
+---
+
+Display of multipart messages
+
+  Displaying multipart/alternative parts has been made a toggle, the
+  same way as indentation and message decryption.
+  notmuch-show-all-multipart/alternative-parts is now buffer-local, so
+  if you have set it using setq, you will have to update that to use
+  setq-default.
+
 Notmuch 0.14 (2012-08-20)
 =
 
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts

2012-10-19 Thread Ethan Glasser-Camp
Mark Walters markwalters1...@gmail.com writes:

 Some messages are sent as multipart/alternative but the alternatives
 contain different information. This allows the user to cycle which
 part to view. By default this is bound to 'W'.
 ---

 This version at least uses the notmuch escaping for message-id which
 makes me a bit happier: it probably doesn't have any nasty security
 flaws. I do still feel that the lisp is a bit ugly though.

For what it's worth, I don't feel that this code is horrible. It seems
like there remain design decisions to be made about how notmuch show
ought to handle multipart/alternatives, but I can at least comment on
this code.

First, the use of a plist looks fine to me because most of the time it's
going to have length 0. At most it will have one entry per message -- a
few hundred. So I'm not worried about efficiency concerns.

  (defcustom notmuch-show-stash-mlarchive-link-alist
'((Gmane . http://mid.gmane.org/;)
  (MARC . http://marc.info/?i=;)
 @@ -536,9 +540,19 @@ message at DEPTH in the current thread.
  
  (defun notmuch-show-insert-part-multipart/alternative (msg part content-type 
 nth depth declared-type)
(notmuch-show-insert-part-header nth declared-type content-type nil)
 -  (let ((chosen-type (car (notmuch-multipart/alternative-choose 
 (notmuch-show-multipart/*-to-list part
 - (inner-parts (plist-get part :content))
 - (start (point)))
 +  (let* ((chosen-nth (or (lax-plist-get 
 notmuch-show-message-multipart/alternative-display-part
 + (notmuch-id-to-query (plist-get msg 
 :id))) 0))
 +  (chosen-type (nth chosen-nth
 +   (notmuch-multipart/alternative-choose 
 (notmuch-show-multipart/*-to-list part
 +  (inner-parts (plist-get part :content))
 +  (start (point)))

Changing let to let* makes me the slightest bit uneasy, although I'm not
sure I could explain why.

It would be nice if you could wrap the manipulation of
notmuch-show-message-multipart/alternative-display-part in functions,
ideally with names that are shorter than the variable they
manipulate. Specifically, I think the definition of chosen-nth (which is
almost repeated below) could be its own function, something like
(notmuch-show-message-current-multipart msg), which could take a msg-id
or a plist and do the plist-get and id-to-query that you do here.

 +;; If we have run out of possible content-types restart from the 
 beginning
 +(unless chosen-type
 +  (setq chosen-type (car (notmuch-multipart/alternative-choose 
 (notmuch-show-multipart/*-to-list part
 +  (setq notmuch-show-message-multipart/alternative-display-part
 + (lax-plist-put 
 notmuch-show-message-multipart/alternative-display-part
 +(notmuch-id-to-query (plist-get msg :id)) 0)))
 +
  ;; This inserts all parts of the chosen type rather than just one,
  ;; but it's not clear that this is the wrong thing to do - which
  ;; should be chosen if there are more than one that match?
 @@ -942,6 +956,16 @@ message at DEPTH in the current thread.
Not processing cryptographic MIME parts.))
(notmuch-show-refresh-view))
  
 +(defun notmuch-show-cycle-message-multipart ()
 +  Cycle which part to display of a multipart messageToggle the display of 
 non-matching messages.

This docstring is broken.

 +  (interactive)
 +  (let* ((msg-id (notmuch-show-get-message-id))
 +  (next-part (1+ (or (lax-plist-get 
 notmuch-show-message-multipart/alternative-display-part msg-id) 0
 +(setq notmuch-show-message-multipart/alternative-display-part
 + (lax-plist-put notmuch-show-message-multipart/alternative-display-part 
 msg-id next-part))
 +(message Cycling multipart/alternative for current message)
 +(notmuch-show-refresh-view)))

Maybe move the reset-and-go-back-to-zero behavior to this function
instead of in the display function. This opens you up to weird
situations if one of the multipart/alternatives should disappear from a
message or if some other function should change the alternative on
display for a given message, but both of these seem unlikely to me..

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

2012-10-19 Thread Ethan Glasser-Camp
Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 It looks like you have better wording for patch 4/8 so I'd like to see
 you resend it.

 I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!

It turns out that patch 4 already has a v2 in the thread, but I didn't
see it due to some kind of selective blindness. It would be nice if
nmbug had grouped it as part of the same patch series. 

I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] emacs: functions to import sender or recipient into BBDB

2012-10-19 Thread Ethan Glasser-Camp
Daniel Bergey ber...@alum.mit.edu writes:

 From a show buffer, bbdb/notmuch-snarf-from imports the sender into
 bbdb.  bbdb/notmuch-snarf-to attempts to import all recipients.  BBDB
 displays a buffer with each contact; C-g displays the next contact, or
 returns to the notmuch-show buffer.

 This is my first notmuch patch.  Comments very welcome.

Hi!

  emacs/notmuch-show.el |   28 

I don't think this belongs in notmuch-show. My first inclination is that
this should go into a new file contrib/notmuch-bbdb.el (assuming there's
no other notmuch-bbdb integration stuff floating around).

  1 file changed, 28 insertions(+)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index 6335d45..3bc1da0 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -1895,6 +1895,34 @@ the user (see 
 `notmuch-show-stash-mlarchive-link-alist').
  (button-get button :notmuch-filename)
  (button-get button :notmuch-content-type)))

 +;; bbdb interaction functions, awaiting user keybindings
 +
 +(defun bbdb/snarf-between-commas ()
 +  ; What about names written Surname, First M u...@server.tld?

Most comments in emacslisp start with two semicolons.

I do think more sophisticated parsing is necessary. If you're lucky,
somebody else already has a library to parse email addresses in this
form.

 +  (goto-char (point-min))

I'm not crazy about this. It's probably fine for something limited to
bbdb users (especially since bbdb-snarf uses a very similar technique),
but I think the better approach here is to just take a region and go
from region-beginning and region-end.

 +  (let ((comma (point)))
 +(while (search-forward , nil end)

The third argument of search-forward is NOERROR. I don't understand what
the value end means. The help says Optional third argument, if t...

 +  (bbdb-snarf-region comma (point))
 +  (setq comma (point)))
 +(bbdb-snarf-region comma (point)) ; last entry
 +   ))

Doesn't this cause snarf the comma into any of those entries? It seems
like point starts before the first entry but then goes before each
comma. Obviously this wouldn't be here if it didn't work. I thought
bbdb-snarf handled this kind of thing, but it doesn't. Could you explain
this?

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 (Draft)] emacs: split async json parser into utility function

2012-10-19 Thread Ethan Glasser-Camp
Mark Walters markwalters1...@gmail.com writes:

 Split out the json parser into a utility function.
 ---

 Most of this patch is code movement: but I don't see how to arrange the
 patch to show that.

Hi! This looks like a straightforward patch and if it will make
notmuch-pick more efficient, I'm in favor.

I tagged this patch moreinfo because David Bremner's suggestions that
you expand on the docstrings for notmuch-json-parser and
notmuch-json-state are good ideas. I'd also propose that you split this
patch into two patches -- one that pulls out the variables and performs
the renames, and the other which breaks out the code into its own
function. This should make the code movement more obvious. I haven't
started full-time work yet so if you would like me to do this, I can ;)

Based on David Bremner's feedback that it might be a good idea to have a
commit message that explains exactly what is code movement and isn't,
here's my proposal for a commit message.

Split out the json parser into a utility function.

This patch breaks out a chunk of notmuch-search-process-filter, with the
following changes:

- notmuch-search-json-parser becomes notmuch-json-parser.
- notmuch-search-parser-state becomes notmuch-json-state.

We also rearrange the json-error case but are careful to always call
error-function in the results buffer.



Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/4] show: indicate length of omitted body content (json)

2012-10-19 Thread Ethan Glasser-Camp
Peter Wang noval...@gmail.com writes:

 If a leaf part's body content is omitted, return the content length in
 --format=json output.  This information may be used by the consumer,
 e.g. to decide whether to download a large attachment over a slow link.

It looks like this patch series was thoroughly reviewed and then
obsoleted by the series in
id:1344428872-12374-1-git-send-email-noval...@gmail.com. I'm therefore
marking it notmuch::obsolete, and will review the other patch set.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 3/3] test: conform to content length, encoding fields

2012-10-19 Thread Ethan Glasser-Camp
Peter Wang noval...@gmail.com writes:

 Update tests to expect content-length and content-transfer-encoding
 fields in show --format=json output, for leaf parts with omitted body
 content.

These three patches all look fine to me, except for the following
problem.

 diff --git a/test/json b/test/json
 index ac8fa8e..8ce2e8a 100755
 --- a/test/json
 +++ b/test/json
 @@ -45,7 +45,7 @@ emacs_deliver_message \
   (insert \Message-ID: $id\n\)
  output=$(notmuch show --format=json id:$id)
  filename=$(notmuch search --output=files id:$id)
 -test_expect_equal_json $output [[[{\id\: \$id\, \match\: true, 
 \excluded\: false, \filename\: \$filename\, \timestamp\: 946728000, 
 \date_relative\: \2000-01-01\, \tags\: [\inbox\], \headers\: 
 {\Subject\: \$subject\, \From\: \Notmuch Test Suite 
 test_su...@notmuchmail.org\, \To\: \test_su...@notmuchmail.org\, 
 \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, \body\: [{\id\: 1, 
 \content-type\: \multipart/mixed\, \content\: [{\id\: 2, 
 \content-type\: \text/plain\, \content\: \This is a test message with 
 inline attachment with a filename\}, {\id\: 3, \content-type\: 
 \application/octet-stream\, \filename\: \README\}]}]}, [
 +test_expect_equal_json $output [[[{\id\: \$id\, \match\: true, 
 \excluded\: false, \filename\: \$filename\, \timestamp\: 946728000, 
 \date_relative\: \2000-01-01\, \tags\: [\inbox\], \headers\: 
 {\Subject\: \$subject\, \From\: \Notmuch Test Suite 
 test_su...@notmuchmail.org\, \To\: \test_su...@notmuchmail.org\, 
 \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, \body\: [{\id\: 1, 
 \content-type\: \multipart/mixed\, \content\: [{\id\: 2, 
 \content-type\: \text/plain\, \content\: \This is a test message with 
 inline attachment with a filename\}, {\id\: 3, \content-type\: 
 \application/octet-stream\, \content-length\: 12392, 
 \content-transfer-encoding\: \base64\, \filename\: \README\}]}]}, 
 [

This test fails for me. You're encoding the content-length of
test/README. test/README certainly hasn't changed in the last six months
so that seems like a reasonable thing to do... except then why is it
12392 on your machine, and 12380 on mine? I don't object to using
test/README as a simple file to test with, but then you certainly
shouldn't hard-code its length. You could pipe test/README through
base64 and then through wc -c to get an accurate length, but for my
machine a newline gets appended by base64 I think, and it gives me
12381.

I'm tagging this patch moreinfo but you would have my +1 if you fix
this.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative

2012-10-19 Thread Ethan Glasser-Camp
Jameson Graef Rollins jroll...@finestructure.net writes:

 +diff OUTPUT.{text,html} OUTPUT.diff
 +cat EOF EXPECTED.diff
 +7,9c7,10
 + [ text/html (not shown) ]
 + [ text/plain ]
 + This is the text/plain part of a multipart/alternative.
 +---
 + [ text/html ]
 + This is the text/html part of a multipart/alternative.
 + 
 + [ text/plain (not shown) ]
 +EOF

Hmm, old-school diff. I guess this (instead of unified diff) is so that
if the diff isn't like this, then the diff of the diffs are clearly
readable? :)

git am complains at me because this patch introduces trailing whitespace
(to mimic, of course, the output of diff which will have trailing
whitespace as well). I would love for you to write this file using some
other technique which would not require putting trailing whitespace in
the test file.

Additionally the patch is stale, perhaps because my patch that moved some tests
to emacs-show was just pushed too.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: random corpus generator, v3

2012-10-19 Thread Ethan Glasser-Camp
da...@tethera.net writes:

 This obsoletes the series at:

  id:134431-4301-1-git-send-email-brem...@debian.org

 Changes since v2:

 - clean up new test-binaries and objects

 - remove the set -o pipefail leftover from debugging.  Possibly this
   makes sense as a global setting, but in a seperate patch.

 - add hex-escape to test/basic

 - rebase against updated master.

Hi! This looks pretty good to me and I am for improving the test
infrastructure.

Some minor problems:

- Patch 2 doesn't apply; neither do patches 4 or 5, presumably due to changes
  that weren't made due to patch 2.

- Commit message discipline: the subject line of patch 4 ends in a
  period. Seperate is spelled by most people as separate, though I
  would encourage you to buck the trend if you are so inclined.

- In patch 4:

 +if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
 +   _notmuch_message_add_term (message, type, mail);
 +} else {
 +   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
 +}

Why not switch the branches? That is, check for private_status !=
NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND and return immediately?

- In patch 5:

 +for (count = 0; count  num_messages; count++) {
 + int j;
 + int num_tags = random () % (max_tags + 1);
 + int this_mid_len = random () % message_id_len + 1;

This looks odd. I'm pretty sure it's correct, but my brain keeps saying,
Why are there no parentheses on (message_id_len + 1)? Maybe just a
comment that message ids must be at least one character long, or the
ranges of values necessary for both of these variables.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

2012-10-20 Thread Ethan Glasser-Camp
Jani Nikula j...@nikula.org writes:

 On Wed, 17 Oct 2012, Adrien Bustany adr...@bustany.org wrote:
 The code of the patches in unchanged, but the formatting issues are now
 hopefully fixed.

 Hi Adrien, please check at what version flush and reopen have been
 introduced to xapian. If they are new-ish (I don't know, didn't have the
 time to check), please add appropriate #ifdefs. [1] lays the groundwork
 for this. We'll also need to decide what is the minimum xapian version
 required in general, i.e. features earlier than that don't need
 conditional compilation.

Hi! The new versions of these patches are still pretty trivial and they
still look OK to me, but based on Jani's prompting I decided to look up
the methods. Seems that flush() is a very old (pre-1.1.0, 2009-04) name
for commit(), which is the preferred name these days. You should
probably therefore rename the function notmuch_database_commit, and have
it call the WritableDatabase::commit() method.

reopen() is a very very old method, seems like it has been around since
2004.

So I think Adrien is safe from having to do version checks, but we
should probably use commit() instead of flush().

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCHv3] notmuch-show: include Bcc header in json output

2012-10-20 Thread Ethan Glasser-Camp
Michal Nazarewicz m...@google.com writes:

 From: Michal Nazarewicz min...@mina86.com

 With this change, emacs users can use notmuch-message-headers
 variable to configure notmuch-show display Bcc header.
 ---

This patch looks pretty straightforward and has seen a certain amount of
review so I'm taking off needs-review.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

2012-10-20 Thread Ethan Glasser-Camp
Tomi Ollila tomi.oll...@iki.fi writes:

 From: Tomi Ollila t...@iki.fi

 When shell executes background process using '' the scheduling of
 that new process is arbitrary. It could be that smtp-dummy doesn't
 get execution time to listen() it's server socket until some other
 process attempts to connect() to it. The --background option in
 smtp-dummy makes it to go background *after* it started to listen
 its server socket.

This looks fine to me, Michal Sojka's reviewed this version, and this
patch has been bouncing around for almost a year! I'm taking off
needs-review.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v3] test: conform to content length, encoding fields

2012-10-21 Thread Ethan Glasser-Camp
Peter Wang noval...@gmail.com writes:

 Update tests to expect content-length and content-transfer-encoding
 fields in show --format=json output, for leaf parts with omitted body
 content.

OK, this whole series looks good to me.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

2012-10-21 Thread Ethan Glasser-Camp
Peter Wang noval...@gmail.com writes:

 Does it help if you add a sleep 1 before the second generate_message
 call, i.e. on line 35?

It turns out that this test failure is sporadic (perhaps due to the fact
that I'm running on tmpfs) and exists even before this series. Doing
sleep 1 makes it go away, but that fix is unrelated to this series
and ought to be fixed elsewhere.

 I have to disagree.  The condition is wrapped over two lines.  The then
 part is wrapped over two lines.  The else part already has braces.
 All suggest braces around the then part.

If Tomi is OK with it, then I guess it's fine. And it's true that there
are a couple of places where braces are used with long conditions and
then-parts.

This series has my +1.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: new: Fix intermittent test failures with --debug

2012-10-21 Thread Ethan Glasser-Camp
Although messages are created in a particular order, it seems that
when they are created on a tmpfs, they do not always come back in the
same order, leading to the same files being ignored but being output
in a different order. This causes the test to fail because the outputs
being compared are the same.

Fix the failures by sorting the output of notmuch --debug and
comparing this to a hand-sorted version of its output.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 test/new |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/new b/test/new
index cc2af72..587aa11 100755
--- a/test/new
+++ b/test/new
@@ -189,17 +189,17 @@ touch ${MAIL_DIR}/.git # change .git's mtime for 
notmuch new to rescan.
 mkdir -p ${MAIL_DIR}/one/two/three/.git
 notmuch new  /dev/null # ensure that files/folders will be printed in ASCII 
order.
 touch ${MAIL_DIR}/{one,one/two,one/two/three}/ignored_file
-output=$(NOTMUCH_NEW --debug 21)
+output=$(NOTMUCH_NEW --debug 21 | sort)
 test_expect_equal $output \
 (D) add_files_recursive, pass 1: explicitly ignoring ${MAIL_DIR}/.git
 (D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/ignored_file
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/ignored_file
 (D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/.git
 (D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/.ignored_hidden_file
 (D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
 No new mail.
 
 
-- 
1.7.9.5

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v3 2/3] emacs: Rename incremental JSON internal variables.

2012-10-23 Thread Ethan Glasser-Camp
Mark Walters markwalters1...@gmail.com writes:

 This patch just renames the internal variables for the JSON parser now
 it is no longer specific to search mode. It also fixes up the white
 space after the previous patch. There should be no functional changes.

This series looks very good to me. I still have a couple of quibbles that I 
wouldn't say
should hold the series up. First, your commit messages have periods at
the end of the first line. I think it is considered good practice that
they not (see, e.g.,
http://blogs.gnome.org/danni/2011/10/25/a-guide-to-writing-git-commit-messages/ 
).

 -(defvar notmuch-search-process-state nil
 -  Parsing state of the search process filter.)
 +(defvar notmuch-json-state nil
 +  State of the internal JSON parser.)

How about,

State of the shared JSON parser.

This variable will be a symbol, corresponding to the state of the JSON
parser's state machine. 'begin means we haven't yet entered a JSON
list. 'result means we are ready to parse a JSON object. 'end means we
have exited the JSON list.

(Or whatever the correct meanings of the parser's states are...)

 -(defvar notmuch-search-json-parser nil
 -  Incremental JSON parser for the search process filter.)
 +(defvar notmuch-json-parser nil
 +  Internal Incremental JSON parser Object.)

How about,

Shared notmuch JSON parser object, as created by
(notmuch-json-create-parser).

Any notmuch code can parse part of a JSON list object by setting this
variable to their JSON parser and calling
notmuch-json-parse-partial-list (which see).

(Or whatever one is supposed to do with the parser object...)

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: Fix HTML rendering test

2012-10-24 Thread Ethan Glasser-Camp
Austin Clements amdra...@mit.edu writes:

 The test designed to exercise Emacs' rendering of HTML emails
 containing images inadvertently assumed w3m was available under Emacs
 23.  The real point of this test was to exercise Emacs 24's shr
 renderer, so if shr isn't available, we now fall back to html2text,
 which comes with Emacs.

Hi! I'm eager to apply any patch here that makes this better. But this
one doesn't fix it for me (24.1.1, although it seems to work with
23.4.1). OUTPUT is *\nsmiley  (no space after the asterisk or before
the word smiley, but after). I see that this sed command is supposed to
normalize things, but at least on my setup, it doesn't. I also see nil
written to console, but I have no idea what that's about.

More generally, I guess I don't understand exactly what this test is
supposed to be exercising. The commit message says the shr renderer,
but what about it? In
id:1348941314-8377-4-git-send-email-amdra...@mit.edu you write that
using shr raised a void-variable error previously, so maybe we're making
sure that error doesn't show up? In that case, even my semi-broken
output is good enough.

In a perfect world, test probably shouldn't succeed if shr isn't
present, but should note that it wasn't run. Maybe the emacs lisp code
can check for shr, and if it's not present, write shr not present to
an output file, and the shell code can grep for that and then call
test_skip if it sees it?

Still, I'm excited that you're working on this so please let's get it
fixed!

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: Fix HTML rendering test

2012-10-25 Thread Ethan Glasser-Camp
Austin Clements amdra...@mit.edu writes:

 Quoth Ethan Glasser-Camp on Oct 24 at  9:59 pm:
 Austin Clements amdra...@mit.edu writes:

 Emacs seems to have as many ways to convert HTML to text as there are
 people trying to run this test.  What's the value of
 mm-text-html-renderer for you in Emacs 24?

I get html2text.

I wanted to write more about what the test should and should not do, but
really I don't know and don't have the time to write about it!

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Support OpenBSD

2012-10-25 Thread Ethan Glasser-Camp
Austin Clements amdra...@mit.edu writes:

 OpenBSD's build flags are identical to FreeBSD, except that libraries
 need to be explicitly linked against libc.  No code changes are
 necessary.

 From: Cody Cutler ccut...@csail.mit.edu
 ---

OK, looks fine.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] tag: Disallow adding malformed tags to messages

2012-10-26 Thread Ethan Glasser-Camp
Tomi Ollila tomi.oll...@iki.fi writes:

 LGTM (NEWS too)

Yep! Removing needs-review.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/3] emacs: Introduce generic boolean term escaping function

2012-10-27 Thread Ethan Glasser-Camp
Tomi Ollila tomi.oll...@iki.fi writes:

 These 3 patches LGTM.

Me too. But I wouldn't be averse to some tests :)

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/3] contrib: add notmuch-pick.el file itself

2012-10-27 Thread Ethan Glasser-Camp
Mark Walters markwalters1...@gmail.com writes:

 +(defvar notmuch-pick-json-parser nil
 +  Incremental JSON parser for the search process filter.)
 +
 +(defun notmuch-pick-process-filter (proc string)
 +  Process and filter the output of \notmuch show\ (for pick)
 +  (let ((results-buf (process-buffer proc))
 +(parse-buf (process-get proc 'parse-buf))
 +(inhibit-read-only t)
 +done)
 +(if (not (buffer-live-p results-buf))
 +(delete-process proc)
 +  (with-current-buffer parse-buf
 +;; Insert new data
 +(save-excursion
 +  (goto-char (point-max))
 +  (insert string)))
 +  (with-current-buffer results-buf
 + (save-excursion
 +   (goto-char (point-max))
 +   (while (not done)
 + (condition-case nil
 + (case notmuch-pick-process-state

This looks awfully familiar. Not looking too close, but why can't this
re-use the JSON parser from your other patch? Just not to rely on the
other patch series?

Still, let's get this pushed.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH (draft) 1/2] emacs: allow the user to toggle the visibility of multipart/alternative parts

2012-10-27 Thread Ethan Glasser-Camp
Mark Walters markwalters1...@gmail.com writes:

 This patch adds a keybinding to the buttons in the notmuch-show emacs
 buffer to allow the user to toggle the visibility of each part of a
 message in the show buffer.  This is particularly useful for
 multipart/alternative parts where the parts are not really
 alternatives but contain different information.
 ---
  emacs/notmuch-show.el |   47 +++
  1 files changed, 39 insertions(+), 8 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index 0f54259..9157669 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -155,6 +155,10 @@ indentation.
  (make-variable-buffer-local 'notmuch-show-indent-content)
  (put 'notmuch-show-indent-content 'permanent-local t)

 +(defvar notmuch-show-message-multipart/alternative-display-parts nil)
 +(make-variable-buffer-local 
 'notmuch-show-message-multipart/alternative-display-parts)
 +(put 'notmuch-show-message-multipart/alternative-display-parts 
 'permanent-local t)
 +
  (defcustom notmuch-show-stash-mlarchive-link-alist
'((Gmane . http://mid.gmane.org/;)
  (MARC . http://marc.info/?i=;)
 @@ -455,6 +459,7 @@ message at DEPTH in the current thread.
  (define-key map v 'notmuch-show-part-button-view)
  (define-key map o 'notmuch-show-part-button-interactively-view)
  (define-key map | 'notmuch-show-part-button-pipe)
 +(define-key map t 'notmuch-show-part-button-internally-show)
  map)
Submap for button commands)
  (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
 @@ -531,6 +536,16 @@ message at DEPTH in the current thread.
  (let ((handle (mm-make-handle (current-buffer) (list content-type
(mm-pipe-part handle

 +(defun notmuch-show-internally-show-part (message-id nth optional filename 
 content-type)
 +  Set a part to be displayed internally
 +  (let ((current-parts (lax-plist-get 
 notmuch-show-message-multipart/alternative-display-parts message-id)))
 +(setq notmuch-show-message-multipart/alternative-display-parts
 +   (lax-plist-put 
 notmuch-show-message-multipart/alternative-display-parts message-id
 +  (if (memq nth current-parts)
 +  (delq nth current-parts)
 +(cons nth current-parts)
 +  (notmuch-show-refresh-view))
 +
  (defun notmuch-show-multipart/*-to-list (part)
(mapcar (lambda (inner-part) (plist-get inner-part :content-type))
 (plist-get part :content)))
 @@ -543,12 +558,15 @@ message at DEPTH in the current thread.
  ;; This inserts all parts of the chosen type rather than just one,
  ;; but it's not clear that this is the wrong thing to do - which
  ;; should be chosen if there are more than one that match?
 +
 +;; The variable user-parts says which parts should override the
 +;; default so we use xor (handcoded since lisp does not have it).

I don't follow the comment. user-parts isn't used in this
function. Neither is xor.

  (mapc (lambda (inner-part)
   (let ((inner-type (plist-get inner-part :content-type)))
 -   (if (or notmuch-show-all-multipart/alternative-parts
 -   (string= chosen-type inner-type))
 -   (notmuch-show-insert-bodypart msg inner-part depth)
 - (notmuch-show-insert-part-header (plist-get inner-part :id) 
 inner-type inner-type nil  (not shown)
 +   (notmuch-show-insert-bodypart msg inner-part depth
 + (not (or 
 notmuch-show-all-multipart/alternative-parts
 +  (string=
   chosen-type inner-type))

For what it's worth, I found this not-shown logic very confusing, and
have had to think about it seven or eight different times to make sure I
understood what's going on. I'm not sure why exactly this is, though I
could offer hypotheses -- the fact that it's split across two functions,
or the fiddling with mime-types. I'm satisfied that it's correct, but I
wish it could be made clearer.

This is just armchair hypothesizing, but here are some ideas that might
make it more obvious what's going on: bringing the user-parts logic into
this function; making user-parts, instead of a t meaning user has
toggled this, something like 'opened or 'closed and if user-parts for
this message is absent, falling back to this calculation; alternately,
prefilling user-parts with t when show is invoked, according to this
calculation, and then not using it any more; moving this
not-shown calculation into a separate function, something like 
notmuch-show-get-message-visibility.

I guess I jumped into this series halfway, but why are we doing this
with the wipe/redraw technique instead of just using invisible overlays,
like we do more generally with notmuch-show? I think I agree that
toggling individual parts is a good UI approach, and this isn't a bad
way to implement it, but I wonder 

Re: [PATCH v2 0/2] include Reply-To headers in json output

2012-11-05 Thread Ethan Glasser-Camp
Peter Wang noval...@gmail.com writes:

 This obsoletes the series 1340508470-16606-1-git-send-email-noval...@gmail.com
 Only json output is affected now.

 Peter Wang (2):
   show: include Reply-To header in json output
   test: add test for showing Reply-To headers

LGTM. Removed needs-review, added notmuch::trivial.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] fix notmuch_database_open call in addrlookup

2012-11-05 Thread Ethan Glasser-Camp
James Vasile ja...@hackervisions.org writes:

 What's the best way to submit changes to addrlookup?  Right now, it is
 out of date vs the latest libnotmuch.  The addrlookup repo is vala code
 but the wiki [1] points to a generated c file [2].

 [1] http://github.com/spaetz/vala-notmuch/raw/static-sources/src/addrlookup.c
 [2] http://notmuchmail.org/emacstips/

 At any rate, a patch to that c file is below.  If you upgraded notmuch
 and now addrlookup gives errors about not finding libnotmuch.so.2, this
 patch might be what you need.

I'd try to contact the author of the code, perhaps by submitting an
issue or pull request on the github page for the project. I'm sure he'd
prefer a patch to the Vala source.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/1] uncrustify.cfg: added 3 new types for uncrustify to know

2012-11-05 Thread Ethan Glasser-Camp
Tomi Ollila tomi.oll...@iki.fi writes:

 Added FILE, notmuch_show_params_t and sprinter_t to be
 types when uncrustifying sources. This affect spacing
 when uncrustify is deciding for type declaration instead
 of binary multiplication operation.

This looks good to me. If you had plenty of time and no more patches to
review, I'd prefer the slightly cleaner English:

This affects how uncrustify puts spacing around pointers to these types,
since it can parse them as type declarations instead of binary
multiplication operations.


... but even suggesting this indicates I've moved past the bike shed and
into somebody else's kitchen.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: emacs: Handling external dependencies

2012-11-13 Thread Ethan Glasser-Camp
Damien Cassou damien.cas...@gmail.com writes:

 4) distribute the dependency with the rest of notmuch (in a separate
 fallback-libs/ directory) and load it only when requiring the
 library with the standard load-path does not work. Jonas Bernoulli
 gave me a way to do that:

 ,
 | (or (require 'THE-LIB nil t)
 | (let ((load-path
 |   (cons (expand-file-name
 |  fallback-libs
 |  (file-name-directory (or load-file-name buffer-file-name)))
 | load-path)))
 |   (require 'THE-LIB)))
 `

 What do you think?

Why not just append it to the *end* of load-path? Then it won't shadow
anything.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2] emacs: display tags in notmuch-show with links

2012-11-13 Thread Ethan Glasser-Camp
Damien Cassou damien.cas...@gmail.com writes:

 +(defun notmuch-tagger-present-tags (tags optional headerline)
 +  Return a property list which nicely presents all TAGS.
 +
 +If HEADERLINE is non-nil the returned list will be ready for
 +inclusion in the buffer's header-line. HEADERLINE must be nil in
 +all other cases.
 +  (list
 +   (
 +   (notmuch-tagger-separate-elems (notmuch-tagger-format-tags tags 
 headerline)  )
 +   )))

It is kind of appalling that it takes 128 lines just to do this. It
seems like there has to be an easier way, or several easier
ways. Unfortunately, I don't see any.

 diff --git a/test/emacs b/test/emacs
 index 44f641e..ecdc841 100755
 --- a/test/emacs
 +++ b/test/emacs
 @@ -820,5 +820,66 @@ Date: Fri, 05 Jan 2001 15:43:57 +
  EOF
  test_expect_equal_file OUTPUT EXPECTED

 +test_begin_subtest Extracting all tags from a thread
 +add_message \
 +'[subject]=Extracting all tags from a thread' \
 +'[body]=body 1'
 +parent=${gen_msg_id}
 +add_message \
 +'[subject]=Extracting all tags from a thread' \
 +'[body]=body 2' \
 +[in-reply-to]=\$parent\
 +add_message \
 +'[subject]=Extracting all tags from a thread' \
 +'[body]=body 3' \
 +[in-reply-to]=\$parent\
 +latest=${gen_msg_id}
 +# Extract the thread-id from one of the emails
 +thread_id=$(notmuch search id:${latest} | sed -e 
 s/thread:\([a-f0-9]*\).*/\1/)

I think the accepted idiom is to use notmuch search
--output=threads. This will output just a string like
thread:00b9, so if you really need just the ID, you could
still use sed here...

 +# Add tag mytagfoo to one of the emails
 +notmuch tag +mytagfoo id:${latest}
 +test_emacs_expect_t \
 +(notmuch-show \thread:${thread_id}\)

... but it seems like thread:... is good enough for you.

 +   (error \We must be in notmch-show at this point but we are in %s.\ 
 major-mode))
 +(push-button) ;; simulate a press on the RET key
 +(if (eq major-mode 'notmuch-search-mode)
 +t
 +   (format \We must be in notmch-search at this point but we are in 
 %s.\ major-mode))

s/notmch/notmuch/ here.

Otherwise I think the code looks fine. I think the design concerns
raised by Mark Walters are probably valid, though.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 0/3] Better id: link buttonization

2012-11-13 Thread Ethan Glasser-Camp
Austin Clements amdra...@mit.edu writes:

 This is v2 of id:1351650561-7331-1-git-send-email-amdra...@mit.edu.
 This makes Jani's suggested additions to the regexp and adds support
 for RFC 2392 mid: links, as suggested by Sascha.

This series looks fine to me.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2] test: Fix HTML rendering test

2012-11-13 Thread Ethan Glasser-Camp
Austin Clements amdra...@mit.edu writes:

 The test designed to exercise Emacs' rendering of HTML emails
 containing images inadvertently assumed w3m was available under Emacs
 23.  The real point of this test was to check that Emacs 24's shr
 renderer didn't crash when given img tags, so use shr if it's
 available, html2text otherwise (which is built in), and do only a
 simple sanity check of the result.

This fixes the failing test on my machine.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: add nontrivial test for restore --accumulate.

2012-11-16 Thread Ethan Glasser-Camp
da...@tethera.net writes:

 From: David Bremner brem...@debian.org

 It seems we have never tested the case that restore --accumulate
 actually adds tags. I noticed this when I started optimizing and no
 tests failed.

 I also had to modify the next test. Perhaps a seperate patch could
 make these tests more independent of the previous ones.
 ---
  test/dump-restore |   14 --
  1 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/test/dump-restore b/test/dump-restore
 index f25f7cf..ca7a730 100755
 --- a/test/dump-restore
 +++ b/test/dump-restore
 @@ -29,18 +29,20 @@ test_expect_success 'Accumulate original tags' \
notmuch dump  dump.actual 
test_cmp dump-ABC_DEF.expected dump.actual'

 -test_expect_success 'Restoring original tags' \
 -  'notmuch restore --input=dump.expected 
 -  notmuch dump  dump.actual 
 -  test_cmp dump.expected dump.actual'
 -

I guess you're removing this test because it just shows that restore can
remove tags, and we already see that in earlier tests?

  test_expect_success 'Restore with nothing to do' \
'notmuch restore  dump.expected 
notmuch dump  dump.actual 
test_cmp dump.expected dump.actual'

Maybe change the name of this test, as now it certainly does something?

 +test_expect_success 'Accumulate with changes' \
 +  'notmuch restore --input=dump.expected 
 +   notmuch restore --accumulate --input=dump-ABC_DEF.expected 
 +  notmuch dump   OUTPUT.$test_count 
 +  test_cmp dump-ABC_DEF.expected OUTPUT.$test_count'

Alignment? I think each line should start with two spaces.

 +
  test_expect_success 'Restore with nothing to do, II' \
 -  'notmuch restore --accumulate --input=dump.expected 
 +  'notmuch restore --input=dump.expected 
 +  notmuch restore --accumulate --input=dump.expected 
notmuch dump  dump.actual 
test_cmp dump.expected dump.actual'

Maybe change the name? Accumulate with nothing to do, for instance?

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 2/2] test: add nontrivial test for restore --accumulate.

2012-11-17 Thread Ethan Glasser-Camp
da...@tethera.net writes:

 From: David Bremner brem...@debian.org

 It seems we have never tested the case that restore --accumulate
 actually adds tags. I noticed this when I started optimizing and no
 tests failed.

 The bracketing with restore --input=dump.expected are to make sure
 we start in a known state, and we leave the database in a known state
 for the next test.

OK, these LGTM.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


  1   2   3   >