Re: notmuch release 0.5 now available
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
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
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
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()
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
_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
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
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
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
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
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
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
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
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
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
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
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
_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
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
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
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
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
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.
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.
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
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.
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
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
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.
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'
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
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
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
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
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
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
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'
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'
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}'
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
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
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'
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'
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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