[ANN] New awesome vim plug-in using Ruby bindings

[Adam Wolfe Gordon]

Hi Felipe,

This work sounds nice - it's good to have lots of interface choices.
One question below:

On Sun, Apr 22, 2012 at 19:12, Felipe Contreras
 wrote:
> doesn't have support for that), and even learning emacs, to use what
> most people here use (but it turns out the HTML messages don't work
> correctly there either). I also tried the various mutt+notmuch

HTML emails should work properly in emacs, and work even better if you
have w3.el installed. Replying to HTML emails isn't in git yet, but
there are patches for it in progress [1]. If there's a bug in the HTML
support, we should probably fix it, so could you explain what doesn't
work for you?

Thanks,

-- Adam

[1] id:"1335056093-17621-1-git-send-email-awg+notmuch at xvx.ca"

[PATCH 2/7] NEWS: Document the notmuch_database_close split

[Felipe Contreras]

On Sun, Apr 22, 2012 at 3:07 PM, Justus Winter
<4winter at informatik.uni-hamburg.de> wrote:
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
> ---
> ?NEWS | ? 12 
> ?1 file changed, 12 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index c1704e0..a2cd080 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -73,6 +73,18 @@ leaving Mutt. ?notmuch-mutt, formerly distributed under 
> the name
> ?"mutt-notmuch" by Stefano Zacchiroli, will be maintained as a notmuch
> ?contrib/ from now on.
>
> +Library changes
> +---
> +
> +API changes
> +
> + ?The function notmuch_database_close has been split into
> + ?notmuch_database_close and notmuch_database_destroy.
> +
> + ?This makes it possible for long running programs to close the xapian
> + ?database and thus release the lock associated with it without
> + ?destroying the data structures obtained from it.
> +

I haven't following this change. Who can an application take advantage
of this? I call _close(), now what do I do to use the database again?

Cheers.

-- 
Felipe Contreras

folder searches don't work (sometimes) with current git

[David Riebenbauer]

Hi,

just a short notice, before I start further investigation into this
bug.

For some reason folder: searches won't work for new messages.
After I tag a message taht won't be found there, by running `notmuch
tag` it is found again.

More details as soon as I find time for more investigation.

Regards,

-- 
David Riebenbauer
Jabber: davrieb at jabber.ccc.de - ICQ: 322056002 
http://liegesta.at

[PATCH 7/7] python: wrap and use notmuch_database_destroy as destructor

[Justus Winter]

Adapt the python bindings to the notmuch_database_close split.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 bindings/python/notmuch/database.py |   14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 44d40fd..268e952 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -161,8 +161,13 @@ class Database(object):
 else:
 self.create(path)

+_destroy = nmlib.notmuch_database_destroy
+_destroy.argtypes = [NotmuchDatabaseP]
+_destroy.restype = None
+
 def __del__(self):
-self.close()
+if self._db:
+self._destroy(self._db)

 def _assert_db_is_initialized(self):
 """Raises :exc:`NotInitializedError` if self._db is `None`"""
@@ -218,10 +223,11 @@ class Database(object):
 _close.restype = None

 def close(self):
-"""Close and free the notmuch database if needed"""
-if self._db is not None:
+'''
+Closes the notmuch database.
+'''
+if self._db:
 self._close(self._db)
-self._db = None

 def __enter__(self):
 '''
-- 
1.7.10

[PATCH 6/7] ruby: Use notmuch_database_destroy instead of notmuch_database_close

[Justus Winter]

Adapt the ruby bindings to the notmuch_database_close split.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 bindings/ruby/database.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 982fd59..ba9a139 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -110,7 +110,7 @@ notmuch_rb_database_close (VALUE self)
 notmuch_database_t *db;

 Data_Get_Notmuch_Database (self, db);
-notmuch_database_close (db);
+notmuch_database_destroy (db);
 DATA_PTR (self) = NULL;

 return Qnil;
-- 
1.7.10

[PATCH 5/7] go: Use notmuch_database_destroy instead of notmuch_database_close

[Justus Winter]

Adapt the go bindings to the notmuch_database_close split.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 bindings/go/pkg/notmuch.go |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bindings/go/pkg/notmuch.go b/bindings/go/pkg/notmuch.go
index c6844ef..d32901d 100644
--- a/bindings/go/pkg/notmuch.go
+++ b/bindings/go/pkg/notmuch.go
@@ -114,7 +114,7 @@ func NewDatabase(path string) *Database {
  * An existing notmuch database can be identified by the presence of a
  * directory named ".notmuch" below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -140,7 +140,7 @@ func OpenDatabase(path string, mode DatabaseMode) *Database 
{
 /* Close the given notmuch database, freeing all associated
  * resources. See notmuch_database_open. */
 func (self *Database) Close() {
-   C.notmuch_database_close(self.db)
+   C.notmuch_database_destroy(self.db)
 }

 /* Return the database path of the given database.
-- 
1.7.10

[PATCH 4/7] Use notmuch_database_destroy instead of notmuch_database_close

[Justus Winter]

Adapt notmuch-deliver to the notmuch_database_close split.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 contrib/notmuch-deliver/src/main.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/notmuch-deliver/src/main.c 
b/contrib/notmuch-deliver/src/main.c
index 6f32f73..37d2919 100644
--- a/contrib/notmuch-deliver/src/main.c
+++ b/contrib/notmuch-deliver/src/main.c
@@ -455,7 +455,7 @@ main(int argc, char **argv)
g_strfreev(opt_rtags);
g_free(mail);

-   notmuch_database_close(db);
+   notmuch_database_destroy(db);

return 0;
 }
-- 
1.7.10

[PATCH 3/7] Use notmuch_database_destroy instead of notmuch_database_close

[Justus Winter]

Adapt the notmuch binaries source to the notmuch_database_close split.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 notmuch-count.c   |2 +-
 notmuch-dump.c|2 +-
 notmuch-new.c |2 +-
 notmuch-reply.c   |2 +-
 notmuch-restore.c |2 +-
 notmuch-search.c  |2 +-
 notmuch-show.c|2 +-
 notmuch-tag.c |2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index b76690c..9c2ad7b 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -107,7 +107,7 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
 }

 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);

 return 0;
 }
diff --git a/notmuch-dump.c b/notmuch-dump.c
index a735875..71ab0ea 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -116,7 +116,7 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
fclose (output);

 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);

 return 0;
 }
diff --git a/notmuch-new.c b/notmuch-new.c
index 4f13535..f078753 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1017,7 +1017,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
notmuch_status_to_string (ret));
 }

-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);

 if (run_hooks && !ret && !interrupted)
ret = notmuch_run_hook (db_path, "post-new");
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 0949d9f..da99a13 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -755,7 +755,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
return 1;

 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);

 if (params.cryptoctx)
g_object_unref(params.cryptoctx);
diff --git a/notmuch-restore.c b/notmuch-restore.c
index d3b9246..02b563c 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -192,7 +192,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 if (line)
free (line);

-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 if (input != stdin)
fclose (input);

diff --git a/notmuch-search.c b/notmuch-search.c
index 1cc8430..7dfd270 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -545,7 +545,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 }

 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);

 return ret;
 }
diff --git a/notmuch-show.c b/notmuch-show.c
index da4a797..3b6667c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1117,7 +1117,7 @@ notmuch_show_command (void *ctx, unused (int argc), 
unused (char *argv[]))
 }

 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);

 if (params.cryptoctx)
g_object_unref(params.cryptoctx);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 05feed3..bd56fd1 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -238,7 +238,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])

 ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags);

-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);

 return ret;
 }
-- 
1.7.10

[PATCH 2/7] NEWS: Document the notmuch_database_close split

[Justus Winter]

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 NEWS |   12 
 1 file changed, 12 insertions(+)

diff --git a/NEWS b/NEWS
index c1704e0..a2cd080 100644
--- a/NEWS
+++ b/NEWS
@@ -73,6 +73,18 @@ leaving Mutt.  notmuch-mutt, formerly distributed under the 
name
 "mutt-notmuch" by Stefano Zacchiroli, will be maintained as a notmuch
 contrib/ from now on.

+Library changes
+---
+
+API changes
+
+  The function notmuch_database_close has been split into
+  notmuch_database_close and notmuch_database_destroy.
+
+  This makes it possible for long running programs to close the xapian
+  database and thus release the lock associated with it without
+  destroying the data structures obtained from it.
+
 Notmuch 0.12 (2012-03-20)
 =

-- 
1.7.10

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   15 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
 "   read-write mode.\n",
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error ) {
fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }

@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }

 delete notmuch->term_gen;
+notmuch->term_gen = NULL;
 delete notmuch->query_parser;
+notmuch->query_parser = NULL;
 delete notmuch->xapian_db;
+notmuch->xapian_db = NULL;
 delete notmuch->value_range_processor;
+notmuch->value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 673c423..84c9265 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named ".notmuch" below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,18 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);

-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * This function is called by notmuch_database_destroy and can be
+ * called multiple times. */
 void
 notmuch_database_close (notmuch_database_t *database);

+/* Destroy the notmuch database freeing all associated
+ * resources */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.10

[PATCH 1/7] Split notmuch_database_close into two functions

[Austin Clements]

On Sun, 22 Apr 2012, Justus Winter <4winter at informatik.uni-hamburg.de> wrote:
> Formerly notmuch_database_close closed the xapian database and
> destroyed the talloc structure associated with the notmuch database
> object. Split notmuch_database_close into notmuch_database_close and
> notmuch_database_destroy.
>
> This makes it possible for long running programs to close the xapian
> database and thus release the lock associated with it without
> destroying the data structures obtained from it.
>
> This also makes the api more consistent since every other data
> structure has a destructor function.
>
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>

Other than my comments comment, this series LGTM.  I think adding those
comments should also address Felipe's question.

[PATCH 7/7] python: wrap and use notmuch_database_destroy as destructor

[Justus Winter]

Quoting Austin Clements (2012-04-12 19:02:49)
> Quoth Justus Winter on Mar 21 at  1:55 am:
> > Adapt the go bindings to the notmuch_database_close split.
> 
> Typo.

Duh >,<

> > Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
> > ---
> >  bindings/python/notmuch/database.py |   17 +++--
> >  1 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/bindings/python/notmuch/database.py 
> > b/bindings/python/notmuch/database.py
> > index 44d40fd..9a1896b 100644
> > --- a/bindings/python/notmuch/database.py
> > +++ b/bindings/python/notmuch/database.py
> > @@ -218,9 +218,22 @@ class Database(object):
> >  _close.restype = None
> >  
> >  def close(self):
> > -"""Close and free the notmuch database if needed"""
> > -if self._db is not None:
> > +'''
> > +Closes the notmuch database.
> > +'''
> > +if self._db:
> >  self._close(self._db)
> > +
> > +_destroy = nmlib.notmuch_database_destroy
> > +_destroy.argtypes = [NotmuchDatabaseP]
> > +_destroy.restype = None
> > +
> > +def destroy(self):
> 
> Should this be __del__?  The existing __del__ is certainly wrong with
> this change, since it only closes the database and doesn't free it.  I
> think this function is exactly what you want __del__ to be now.
> 
> (I think it also doesn't make sense to expose notmuch_database_destroy
> as a general, public method since it will free all of the other C
> objects out from under the bindings, resulting in exactly the double
> free-type crashes that you're trying to avoid.  It appears that none
> of the other Python classes have a destroy method.)

Yes, of course...

I'm going to send an rebased and updated patch series as a follow up
to this message. Thanks for the input everyone :)

Justus

[PATCH 1/7] Split notmuch_database_close into two functions

[Austin Clements]

Quoth Justus Winter on Apr 22 at  2:07 pm:
> Formerly notmuch_database_close closed the xapian database and
> destroyed the talloc structure associated with the notmuch database
> object. Split notmuch_database_close into notmuch_database_close and
> notmuch_database_destroy.
> 
> This makes it possible for long running programs to close the xapian
> database and thus release the lock associated with it without
> destroying the data structures obtained from it.
> 
> This also makes the api more consistent since every other data
> structure has a destructor function.
> 
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
> ---
>  lib/database.cc |   14 --
>  lib/notmuch.h   |   15 +++
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/database.cc b/lib/database.cc
> index 16c4354..2fefcad 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
>"   read-write mode.\n",
>notmuch_path, version, NOTMUCH_DATABASE_VERSION);
>   notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
> - notmuch_database_close (notmuch);
> + notmuch_database_destroy (notmuch);
>   notmuch = NULL;
>   goto DONE;
>   }
> @@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
>  } catch (const Xapian::Error ) {
>   fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
>error.get_msg().c_str());
> - notmuch_database_close (notmuch);
> + notmuch_database_destroy (notmuch);
>   notmuch = NULL;
>  }
>  
> @@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
>  }
>  
>  delete notmuch->term_gen;
> +notmuch->term_gen = NULL;
>  delete notmuch->query_parser;
> +notmuch->query_parser = NULL;
>  delete notmuch->xapian_db;
> +notmuch->xapian_db = NULL;
>  delete notmuch->value_range_processor;
> +notmuch->value_range_processor = NULL;
> +}
> +
> +void
> +notmuch_database_destroy (notmuch_database_t *notmuch)
> +{
> +notmuch_database_close (notmuch);
>  talloc_free (notmuch);
>  }
>  
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 673c423..84c9265 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
>   *
>   * After a successful call to notmuch_database_create, the returned
>   * database will be open so the caller should call
> - * notmuch_database_close when finished with it.
> + * notmuch_database_destroy when finished with it.
>   *
>   * The database will not yet have any data in it
>   * (notmuch_database_create itself is a very cheap function). Messages
> @@ -165,7 +165,7 @@ typedef enum {
>   * An existing notmuch database can be identified by the presence of a
>   * directory named ".notmuch" below 'path'.
>   *
> - * The caller should call notmuch_database_close when finished with
> + * The caller should call notmuch_database_destroy when finished with
>   * this database.
>   *
>   * In case of any failure, this function returns NULL, (after printing
> @@ -175,11 +175,18 @@ notmuch_database_t *
>  notmuch_database_open (const char *path,
>  notmuch_database_mode_t mode);
>  
> -/* Close the given notmuch database, freeing all associated
> - * resources. See notmuch_database_open. */
> +/* Close the given notmuch database.
> + *
> + * This function is called by notmuch_database_destroy and can be
> + * called multiple times. */

This needs a comment explaining the implications of closing the
database.  Perhaps something like this (modeled on
Xapian::Database::close's documentation)

/* Close the given notmuch database.
 *
 * After notmuch_database_close has been called, calls to other
 * functions on objects derived from this database may either behave
 * as if the database had not been closed (e.g., if the required data
 * has been cached) or may fail with a
 * NOTMUCH_STATUS_XAPIAN_EXCEPTION.
 *
 * notmuch_database_close can be called multiple times.  Later calls
 * have no affect. 
 */

>  void
>  notmuch_database_close (notmuch_database_t *database);
>  
> +/* Destroy the notmuch database freeing all associated
> + * resources */

This should mention that this also closes the database (currently you
mention this in the doc for notmuch_database_close, but it's a reverse
reference there; that could probably even be omitted).  Perhaps

/* Destroy the notmuch database, closing it if necessary and freeing
 * all associated resources. */

> +void
> +notmuch_database_destroy (notmuch_database_t *database);
> +
>  /* Return the database path of the given database.
>   *
>   * The return value is a string owned by notmuch so should not be

[PATCH v3 3/4] new: Print final fatal error message to stderr

[Austin Clements]

This was going to stdout.  I removed the newline at the beginning of
printing the fatal error message because it wouldn't make sense if you
were only looking at the stderr stream (e.g., you had redirected
stdout to /dev/null).
---
 notmuch-new.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 2faf2f8..bf9b120 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1035,8 +1035,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 printf ("\n");

 if (ret)
-   printf ("\nNote: A fatal error was encountered: %s\n",
-   notmuch_status_to_string (ret));
+   fprintf (stderr, "Note: A fatal error was encountered: %s\n",
+notmuch_status_to_string (ret));

 notmuch_database_close (notmuch);

-- 
1.7.9.1

[PATCH v3 2/4] new: Handle fatal errors in remove_filename and _remove_directory

[Austin Clements]

Previously such errors were simply ignored.  Now they cause an
immediate cleanup and abort.
---
 notmuch-new.c |   25 +++--
 1 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 15c0b36..2faf2f8 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -785,8 +785,10 @@ remove_filename (notmuch_database_t *notmuch,
add_files_state->renamed_messages++;
if (add_files_state->synchronize_flags == TRUE)
notmuch_message_maildir_flags_to_tags (message);
-} else
+   status = NOTMUCH_STATUS_SUCCESS;
+} else if (status == NOTMUCH_STATUS_SUCCESS) {
add_files_state->removed_messages++;
+}
 notmuch_message_destroy (message);
 notmuch_database_end_atomic (notmuch);
 return status;
@@ -794,12 +796,13 @@ remove_filename (notmuch_database_t *notmuch,

 /* Recursively remove all filenames from the database referring to
  * 'path' (or to any of its children). */
-static void
+static notmuch_status_t
 _remove_directory (void *ctx,
   notmuch_database_t *notmuch,
   const char *path,
   add_files_state_t *add_files_state)
 {
+notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 notmuch_directory_t *directory;
 notmuch_filenames_t *files, *subdirs;
 char *absolute;
@@ -812,8 +815,10 @@ _remove_directory (void *ctx,
 {
absolute = talloc_asprintf (ctx, "%s/%s", path,
notmuch_filenames_get (files));
-   remove_filename (notmuch, absolute, add_files_state);
+   status = remove_filename (notmuch, absolute, add_files_state);
talloc_free (absolute);
+   if (status)
+   goto DONE;
 }

 for (subdirs = notmuch_directory_get_child_directories (directory);
@@ -822,11 +827,15 @@ _remove_directory (void *ctx,
 {
absolute = talloc_asprintf (ctx, "%s/%s", path,
notmuch_filenames_get (subdirs));
-   _remove_directory (ctx, notmuch, absolute, add_files_state);
+   status = _remove_directory (ctx, notmuch, absolute, add_files_state);
talloc_free (absolute);
+   if (status)
+   goto DONE;
 }

+  DONE:
 notmuch_directory_destroy (directory);
+return status;
 }

 int
@@ -944,7 +953,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])

 gettimeofday (_start, NULL);
 for (f = add_files_state.removed_files->head; f && !interrupted; f = 
f->next) {
-   remove_filename (notmuch, f->filename, _files_state);
+   ret = remove_filename (notmuch, f->filename, _files_state);
+   if (ret)
+   goto DONE;
if (do_print_progress) {
do_print_progress = 0;
generic_print_progress ("Cleaned up", "messages",
@@ -955,7 +966,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])

 gettimeofday (_start, NULL);
 for (f = add_files_state.removed_directories->head, i = 0; f && 
!interrupted; f = f->next, i++) {
-   _remove_directory (ctx, notmuch, f->filename, _files_state);
+   ret = _remove_directory (ctx, notmuch, f->filename, _files_state);
+   if (ret)
+   goto DONE;
if (do_print_progress) {
do_print_progress = 0;
generic_print_progress ("Cleaned up", "directories",
-- 
1.7.9.1

[PATCH v3 1/4] new: Consistently treat fatal errors as fatal

[Austin Clements]

Previously, fatal errors in add_files_recursive were not treated as
fatal by its callers (including itself!).  This makes
add_files_recursive errors consistently fatal and updates all callers
to treat them as fatal.
---
 notmuch-new.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 4f13535..15c0b36 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -308,6 +308,10 @@ add_files_recursive (notmuch_database_t *notmuch,
 if (num_fs_entries == -1) {
fprintf (stderr, "Error opening directory %s: %s\n",
 path, strerror (errno));
+   /* We consider this a fatal error because, if a user moved a
+* message from another directory that we were able to scan
+* into this directory, skipping this directory will cause
+* that message to be lost. */
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
@@ -351,8 +355,10 @@ add_files_recursive (notmuch_database_t *notmuch,

next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
status = add_files_recursive (notmuch, next, state);
-   if (status && ret == NOTMUCH_STATUS_SUCCESS)
+   if (status) {
ret = status;
+   goto DONE;
+   }
talloc_free (next);
next = NULL;
 }
@@ -933,6 +939,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 }

 ret = add_files (notmuch, db_path, _files_state);
+if (ret)
+   goto DONE;

 gettimeofday (_start, NULL);
 for (f = add_files_state.removed_files->head; f && !interrupted; f = 
f->next) {
@@ -965,6 +973,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
}
 }

+  DONE:
 talloc_free (add_files_state.removed_files);
 talloc_free (add_files_state.removed_directories);
 talloc_free (add_files_state.directory_mtimes);
@@ -1012,10 +1021,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])

 printf ("\n");

-if (ret) {
-   printf ("\nNote: At least one error was encountered: %s\n",
+if (ret)
+   printf ("\nNote: A fatal error was encountered: %s\n",
notmuch_status_to_string (ret));
-}

 notmuch_database_close (notmuch);

-- 
1.7.9.1

[PATCH v3 0/4] Fix some error handling in notmuch new

[Austin Clements]

This version fixes a silly mistake Mark pointed out.  No further
review is necessary, per id:"877gx8kyad.fsf at qmul.ac.uk".

[PATCH v2 2/4] new: Handle fatal errors in remove_filename and _remove_directory

[Austin Clements]

Quoth Mark Walters on Apr 22 at  8:34 am:
> On Sun, 22 Apr 2012, Austin Clements  wrote:
> > Previously such errors were simply ignored.  Now they cause an
> > immediate cleanup and abort.
> > ---
> >  notmuch-new.c |   25 +++--
> >  1 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/notmuch-new.c b/notmuch-new.c
> > index 15c0b36..92e0489 100644
> > --- a/notmuch-new.c
> > +++ b/notmuch-new.c
> > @@ -785,8 +785,10 @@ remove_filename (notmuch_database_t *notmuch,
> > add_files_state->renamed_messages++;
> > if (add_files_state->synchronize_flags == TRUE)
> > notmuch_message_maildir_flags_to_tags (message);
> > -} else
> > +   status = NOTMUCH_STATUS_SUCCESS;
> > +} else if (status == NOTMUCH_STATUS_SUCCESS) {
> > add_files_state->removed_messages++;
> > +}
> >  notmuch_message_destroy (message);
> >  notmuch_database_end_atomic (notmuch);
> >  return status;
> > @@ -794,12 +796,13 @@ remove_filename (notmuch_database_t *notmuch,
> >  
> >  /* Recursively remove all filenames from the database referring to
> >   * 'path' (or to any of its children). */
> > -static void
> > +static notmuch_status_t
> >  _remove_directory (void *ctx,
> >notmuch_database_t *notmuch,
> >const char *path,
> >add_files_state_t *add_files_state)
> >  {
> > +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
> >  notmuch_directory_t *directory;
> >  notmuch_filenames_t *files, *subdirs;
> >  char *absolute;
> > @@ -812,8 +815,10 @@ _remove_directory (void *ctx,
> >  {
> > absolute = talloc_asprintf (ctx, "%s/%s", path,
> > notmuch_filenames_get (files));
> > -   remove_filename (notmuch, absolute, add_files_state);
> > +   status = remove_filename (notmuch, absolute, add_files_state);
> > talloc_free (absolute);
> > +   if (status)
> > +   goto DONE;
> >  }
> >  
> >  for (subdirs = notmuch_directory_get_child_directories (directory);
> > @@ -822,11 +827,15 @@ _remove_directory (void *ctx,
> >  {
> > absolute = talloc_asprintf (ctx, "%s/%s", path,
> > notmuch_filenames_get (subdirs));
> > -   _remove_directory (ctx, notmuch, absolute, add_files_state);
> > +   status = _remove_directory (ctx, notmuch, absolute, add_files_state);
> > talloc_free (absolute);
> > +   if (status)
> > +   goto DONE;
> >  }
> >  
> > +  DONE:
> >  notmuch_directory_destroy (directory);
> > +return NOTMUCH_STATUS_SUCCESS;
> 
> Doesn't this need to be return status or something?

Arrg.  Yes, of course.  I wish I knew a good way to test this stuff.

[PATCH 2/2] emacs: Correctly quote non-text/plain parts in reply

[Mark Walters]

On Sun, 22 Apr 2012, Adam Wolfe Gordon  wrote:
> Quote non-text parts nicely by displaying them with mm-display-part
> before calling message-cite-original to quote them. HTML-only emails
> can now be quoted correctly.

My instinct would have been to do this with a temporary buffer rather
than the narrowing but I am *definitely* too much of a lisp beginner to
say that either is better.

(I think notmuch-show-mm-display-part-inline uses the temporary buffer
approach.)

Anyway, as it is, it looks correct and seems to work! (and is obviously
useful functionality)

Best wishes

Mark

> Mark the test for this feature as not broken.
> ---
>  emacs/notmuch-mua.el |   20 +++-
>  test/emacs   |1 -
>  2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 87bd88d..f7af789 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -21,6 +21,7 @@
>  
>  (require 'json)
>  (require 'message)
> +(require 'mm-view)
>  (require 'format-spec)
>  
>  (require 'notmuch-lib)
> @@ -90,6 +91,19 @@ list."
>   else if (notmuch-match-content-type (plist-get part :content-type) 
> "text/*")
> collect part))
>  
> +(defun notmuch-mua-insert-quotable-part (message part)
> +  (save-restriction
> +(narrow-to-region (point) (point))
> +(insert (notmuch-get-bodypart-content message part
> +   (plist-get part :id)
> +   notmuch-show-process-crypto))
> +(let ((handle (mm-make-handle (current-buffer)
> +   (list (plist-get part :content-type
> +   (end-of-orig (point-max)))
> +  (mm-display-part handle)
> +  (kill-region (point-min) end-of-orig))
> +(goto-char (point-max
> +
>  ;; There is a bug in emacs 23's message.el that results in a newline
>  ;; not being inserted after the References header, so the next header
>  ;; is concatenated to the end of it. This function fixes the problem,
> @@ -169,11 +183,7 @@ list."
>   ;; Get the parts of the original message that should be quoted; this 
> includes
>   ;; all the text parts, except the non-preferred ones in a 
> multipart/alternative.
>   (let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get 
> original :body
> -   (mapc (lambda (part)
> -   (insert (notmuch-get-bodypart-content original part
> - (plist-get part :id)
> - 
> notmuch-show-process-crypto)))
> - quotable-parts))
> +   (mapc (apply-partially 'notmuch-mua-insert-quotable-part original) 
> quotable-parts))
>  
>   (set-mark (point))
>   (goto-char start)
> diff --git a/test/emacs b/test/emacs
> index e648f80..579844f 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -445,7 +445,6 @@ EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
>  test_begin_subtest "Reply within emacs to an html-only message"
> -test_subtest_known_broken
>  add_message '[content-type]="text/html"' \
>   '[body]="Hi,This is an HTML test message. />OK?"'
>  test_emacs "(let ((message-hidden-headers '()))
> -- 
> 1.7.5.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 4/4] new: Fix missing end_atomic in remove_filename on error

[Mark Walters]

On Sun, 22 Apr 2012, Austin Clements  wrote:
> Previously, if we failed to find the message by filename in
> remove_filename, we would return immediately from the function without
> ending its atomic block.  Now this code follows the usual goto DONE
> idiom to perform cleanup.

The whole series looks good to me modulo the return value in Patch 2/4.

Best wishes

Mark 

> ---
>  notmuch-new.c |5 -
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index 2103b18..9eebea4 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -779,7 +779,8 @@ remove_filename (notmuch_database_t *notmuch,
>   return status;
>  status = notmuch_database_find_message_by_filename (notmuch, path, 
> );
>  if (status || message == NULL)
> - return status;
> + goto DONE;
> +
>  status = notmuch_database_remove_message (notmuch, path);
>  if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
>   add_files_state->renamed_messages++;
> @@ -790,6 +791,8 @@ remove_filename (notmuch_database_t *notmuch,
>   add_files_state->removed_messages++;
>  }
>  notmuch_message_destroy (message);
> +
> +  DONE:
>  notmuch_database_end_atomic (notmuch);
>  return status;
>  }
> -- 
> 1.7.9.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 2/4] new: Handle fatal errors in remove_filename and _remove_directory

[Mark Walters]

On Sun, 22 Apr 2012, Austin Clements  wrote:
> Previously such errors were simply ignored.  Now they cause an
> immediate cleanup and abort.
> ---
>  notmuch-new.c |   25 +++--
>  1 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index 15c0b36..92e0489 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -785,8 +785,10 @@ remove_filename (notmuch_database_t *notmuch,
>   add_files_state->renamed_messages++;
>   if (add_files_state->synchronize_flags == TRUE)
>   notmuch_message_maildir_flags_to_tags (message);
> -} else
> + status = NOTMUCH_STATUS_SUCCESS;
> +} else if (status == NOTMUCH_STATUS_SUCCESS) {
>   add_files_state->removed_messages++;
> +}
>  notmuch_message_destroy (message);
>  notmuch_database_end_atomic (notmuch);
>  return status;
> @@ -794,12 +796,13 @@ remove_filename (notmuch_database_t *notmuch,
>  
>  /* Recursively remove all filenames from the database referring to
>   * 'path' (or to any of its children). */
> -static void
> +static notmuch_status_t
>  _remove_directory (void *ctx,
>  notmuch_database_t *notmuch,
>  const char *path,
>  add_files_state_t *add_files_state)
>  {
> +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
>  notmuch_directory_t *directory;
>  notmuch_filenames_t *files, *subdirs;
>  char *absolute;
> @@ -812,8 +815,10 @@ _remove_directory (void *ctx,
>  {
>   absolute = talloc_asprintf (ctx, "%s/%s", path,
>   notmuch_filenames_get (files));
> - remove_filename (notmuch, absolute, add_files_state);
> + status = remove_filename (notmuch, absolute, add_files_state);
>   talloc_free (absolute);
> + if (status)
> + goto DONE;
>  }
>  
>  for (subdirs = notmuch_directory_get_child_directories (directory);
> @@ -822,11 +827,15 @@ _remove_directory (void *ctx,
>  {
>   absolute = talloc_asprintf (ctx, "%s/%s", path,
>   notmuch_filenames_get (subdirs));
> - _remove_directory (ctx, notmuch, absolute, add_files_state);
> + status = _remove_directory (ctx, notmuch, absolute, add_files_state);
>   talloc_free (absolute);
> + if (status)
> + goto DONE;
>  }
>  
> +  DONE:
>  notmuch_directory_destroy (directory);
> +return NOTMUCH_STATUS_SUCCESS;

Doesn't this need to be return status or something?

Best wishes

Mark


>  }
>  
>  int
> @@ -944,7 +953,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
>  
>  gettimeofday (_start, NULL);
>  for (f = add_files_state.removed_files->head; f && !interrupted; f = 
> f->next) {
> - remove_filename (notmuch, f->filename, _files_state);
> + ret = remove_filename (notmuch, f->filename, _files_state);
> + if (ret)
> + goto DONE;
>   if (do_print_progress) {
>   do_print_progress = 0;
>   generic_print_progress ("Cleaned up", "messages",
> @@ -955,7 +966,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
>  
>  gettimeofday (_start, NULL);
>  for (f = add_files_state.removed_directories->head, i = 0; f && 
> !interrupted; f = f->next, i++) {
> - _remove_directory (ctx, notmuch, f->filename, _files_state);
> + ret = _remove_directory (ctx, notmuch, f->filename, _files_state);
> + if (ret)
> + goto DONE;
>   if (do_print_progress) {
>   do_print_progress = 0;
>   generic_print_progress ("Cleaned up", "directories",
> -- 
> 1.7.9.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 4/4] new: Fix missing end_atomic in remove_filename on error

[Austin Clements]

Previously, if we failed to find the message by filename in
remove_filename, we would return immediately from the function without
ending its atomic block.  Now this code follows the usual goto DONE
idiom to perform cleanup.
---
 notmuch-new.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 2103b18..9eebea4 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -779,7 +779,8 @@ remove_filename (notmuch_database_t *notmuch,
return status;
 status = notmuch_database_find_message_by_filename (notmuch, path, 
);
 if (status || message == NULL)
-   return status;
+   goto DONE;
+
 status = notmuch_database_remove_message (notmuch, path);
 if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
add_files_state->renamed_messages++;
@@ -790,6 +791,8 @@ remove_filename (notmuch_database_t *notmuch,
add_files_state->removed_messages++;
 }
 notmuch_message_destroy (message);
+
+  DONE:
 notmuch_database_end_atomic (notmuch);
 return status;
 }
-- 
1.7.9.1

[PATCH v2 3/4] new: Print final fatal error message to stderr

[Austin Clements]

This was going to stdout.  I removed the newline at the beginning of
printing the fatal error message because it wouldn't make sense if you
were only looking at the stderr stream (e.g., you had redirected
stdout to /dev/null).
---
 notmuch-new.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 92e0489..2103b18 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1035,8 +1035,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 printf ("\n");

 if (ret)
-   printf ("\nNote: A fatal error was encountered: %s\n",
-   notmuch_status_to_string (ret));
+   fprintf (stderr, "Note: A fatal error was encountered: %s\n",
+notmuch_status_to_string (ret));

 notmuch_database_close (notmuch);

-- 
1.7.9.1

[PATCH v2 2/4] new: Handle fatal errors in remove_filename and _remove_directory

[Austin Clements]

Previously such errors were simply ignored.  Now they cause an
immediate cleanup and abort.
---
 notmuch-new.c |   25 +++--
 1 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 15c0b36..92e0489 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -785,8 +785,10 @@ remove_filename (notmuch_database_t *notmuch,
add_files_state->renamed_messages++;
if (add_files_state->synchronize_flags == TRUE)
notmuch_message_maildir_flags_to_tags (message);
-} else
+   status = NOTMUCH_STATUS_SUCCESS;
+} else if (status == NOTMUCH_STATUS_SUCCESS) {
add_files_state->removed_messages++;
+}
 notmuch_message_destroy (message);
 notmuch_database_end_atomic (notmuch);
 return status;
@@ -794,12 +796,13 @@ remove_filename (notmuch_database_t *notmuch,

 /* Recursively remove all filenames from the database referring to
  * 'path' (or to any of its children). */
-static void
+static notmuch_status_t
 _remove_directory (void *ctx,
   notmuch_database_t *notmuch,
   const char *path,
   add_files_state_t *add_files_state)
 {
+notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 notmuch_directory_t *directory;
 notmuch_filenames_t *files, *subdirs;
 char *absolute;
@@ -812,8 +815,10 @@ _remove_directory (void *ctx,
 {
absolute = talloc_asprintf (ctx, "%s/%s", path,
notmuch_filenames_get (files));
-   remove_filename (notmuch, absolute, add_files_state);
+   status = remove_filename (notmuch, absolute, add_files_state);
talloc_free (absolute);
+   if (status)
+   goto DONE;
 }

 for (subdirs = notmuch_directory_get_child_directories (directory);
@@ -822,11 +827,15 @@ _remove_directory (void *ctx,
 {
absolute = talloc_asprintf (ctx, "%s/%s", path,
notmuch_filenames_get (subdirs));
-   _remove_directory (ctx, notmuch, absolute, add_files_state);
+   status = _remove_directory (ctx, notmuch, absolute, add_files_state);
talloc_free (absolute);
+   if (status)
+   goto DONE;
 }

+  DONE:
 notmuch_directory_destroy (directory);
+return NOTMUCH_STATUS_SUCCESS;
 }

 int
@@ -944,7 +953,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])

 gettimeofday (_start, NULL);
 for (f = add_files_state.removed_files->head; f && !interrupted; f = 
f->next) {
-   remove_filename (notmuch, f->filename, _files_state);
+   ret = remove_filename (notmuch, f->filename, _files_state);
+   if (ret)
+   goto DONE;
if (do_print_progress) {
do_print_progress = 0;
generic_print_progress ("Cleaned up", "messages",
@@ -955,7 +966,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])

 gettimeofday (_start, NULL);
 for (f = add_files_state.removed_directories->head, i = 0; f && 
!interrupted; f = f->next, i++) {
-   _remove_directory (ctx, notmuch, f->filename, _files_state);
+   ret = _remove_directory (ctx, notmuch, f->filename, _files_state);
+   if (ret)
+   goto DONE;
if (do_print_progress) {
do_print_progress = 0;
generic_print_progress ("Cleaned up", "directories",
-- 
1.7.9.1

[PATCH v2 1/4] new: Consistently treat fatal errors as fatal

[Austin Clements]

Previously, fatal errors in add_files_recursive were not treated as
fatal by its callers (including itself!).  This makes
add_files_recursive errors consistently fatal and updates all callers
to treat them as fatal.
---
 notmuch-new.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 4f13535..15c0b36 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -308,6 +308,10 @@ add_files_recursive (notmuch_database_t *notmuch,
 if (num_fs_entries == -1) {
fprintf (stderr, "Error opening directory %s: %s\n",
 path, strerror (errno));
+   /* We consider this a fatal error because, if a user moved a
+* message from another directory that we were able to scan
+* into this directory, skipping this directory will cause
+* that message to be lost. */
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
@@ -351,8 +355,10 @@ add_files_recursive (notmuch_database_t *notmuch,

next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
status = add_files_recursive (notmuch, next, state);
-   if (status && ret == NOTMUCH_STATUS_SUCCESS)
+   if (status) {
ret = status;
+   goto DONE;
+   }
talloc_free (next);
next = NULL;
 }
@@ -933,6 +939,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 }

 ret = add_files (notmuch, db_path, _files_state);
+if (ret)
+   goto DONE;

 gettimeofday (_start, NULL);
 for (f = add_files_state.removed_files->head; f && !interrupted; f = 
f->next) {
@@ -965,6 +973,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
}
 }

+  DONE:
 talloc_free (add_files_state.removed_files);
 talloc_free (add_files_state.removed_directories);
 talloc_free (add_files_state.directory_mtimes);
@@ -1012,10 +1021,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])

 printf ("\n");

-if (ret) {
-   printf ("\nNote: At least one error was encountered: %s\n",
+if (ret)
+   printf ("\nNote: A fatal error was encountered: %s\n",
notmuch_status_to_string (ret));
-}

 notmuch_database_close (notmuch);

-- 
1.7.9.1

[PATCH v2 0/4] Fix some error handling in notmuch new

[Austin Clements]

Version 2 should address Mark's comments.  It also adds a patch to fix
an additional error handling error he pointed out in remove_filename.

[PATCH 2/3] new: Handle fatal errors in remove_filename and _remove_directory

[Austin Clements]

Quoth Mark Walters on Apr 16 at  5:02 pm:
> On Mon, 27 Feb 2012, Austin Clements  wrote:
> > Previously such errors were simply ignored.  Now they cause an
> > immediate cleanup and abort.
> 
> This one looks fine except for a minor query.
> 
> > ---
> >  notmuch-new.c |   24 ++--
> >  1 files changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/notmuch-new.c b/notmuch-new.c
> > index bd9786a..0cbd479 100644
> > --- a/notmuch-new.c
> > +++ b/notmuch-new.c
> > @@ -780,8 +780,10 @@ remove_filename (notmuch_database_t *notmuch,
> > add_files_state->renamed_messages++;
> > if (add_files_state->synchronize_flags == TRUE)
> > notmuch_message_maildir_flags_to_tags (message);
> > -} else
> > +   status = NOTMUCH_STATUS_SUCCESS;
> > +} else if (status == NOTMUCH_STATUS_SUCCESS) {
> > add_files_state->removed_messages++;
> > +}
> >  notmuch_message_destroy (message);
> >  notmuch_database_end_atomic (notmuch);
> >  return status;
> > @@ -789,12 +791,13 @@ remove_filename (notmuch_database_t *notmuch,
> >  
> >  /* Recursively remove all filenames from the database referring to
> >   * 'path' (or to any of its children). */
> > -static void
> > +static notmuch_status_t
> >  _remove_directory (void *ctx,
> >notmuch_database_t *notmuch,
> >const char *path,
> >add_files_state_t *add_files_state)
> >  {
> > +notmuch_status_t status;
> >  notmuch_directory_t *directory;
> >  notmuch_filenames_t *files, *subdirs;
> >  char *absolute;
> > @@ -807,8 +810,10 @@ _remove_directory (void *ctx,
> >  {
> > absolute = talloc_asprintf (ctx, "%s/%s", path,
> > notmuch_filenames_get (files));
> > -   remove_filename (notmuch, absolute, add_files_state);
> > +   status = remove_filename (notmuch, absolute, add_files_state);
> > talloc_free (absolute);
> > +   if (status)
> > +   return status;
> >  }
> >  
> >  for (subdirs = notmuch_directory_get_child_directories (directory);
> > @@ -817,11 +822,14 @@ _remove_directory (void *ctx,
> >  {
> > absolute = talloc_asprintf (ctx, "%s/%s", path,
> > notmuch_filenames_get (subdirs));
> > -   _remove_directory (ctx, notmuch, absolute, add_files_state);
> > +   status = _remove_directory (ctx, notmuch, absolute, add_files_state);
> > talloc_free (absolute);
> > +   if (status)
> > +   return status;
> >  }
> >  
> >  notmuch_directory_destroy (directory);
> > +return NOTMUCH_STATUS_SUCCESS;
> >  }
> 
> In the two "return status" lines above seem to mean we don't call
> notmuch_directory_destroy. Does that matter?

Good point.  I've fixed this to use the usual goto DONE cleanup idiom.

> The other query is not actually about this patch: just something that
> came up when reading it. Should notmuch_database_begin_atomic and
> notmuch_database_end_atomic always be paired? One of the (existing)
> error cases in remove_filename seems to return without calling end.

Yes, they should be.  I've added a patch to fix that.

> Best wishes
> 
> Mark
> 
> >  int
> > @@ -939,7 +947,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> >  
> >  gettimeofday (_start, NULL);
> >  for (f = add_files_state.removed_files->head; f && !interrupted; f = 
> > f->next) {
> > -   remove_filename (notmuch, f->filename, _files_state);
> > +   ret = remove_filename (notmuch, f->filename, _files_state);
> > +   if (ret)
> > +   goto DONE;
> > if (do_print_progress) {
> > do_print_progress = 0;
> > generic_print_progress ("Cleaned up", "messages",
> > @@ -950,7 +960,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> >  
> >  gettimeofday (_start, NULL);
> >  for (f = add_files_state.removed_directories->head, i = 0; f && 
> > !interrupted; f = f->next, i++) {
> > -   _remove_directory (ctx, notmuch, f->filename, _files_state);
> > +   ret = _remove_directory (ctx, notmuch, f->filename, _files_state);
> > +   if (ret)
> > +   goto DONE;
> > if (do_print_progress) {
> > do_print_progress = 0;
> > generic_print_progress ("Cleaned up", "directories",

[PATCH 1/3] new: Consistently treat fatal errors as fatal

[Austin Clements]

Quoth Mark Walters on Apr 16 at  4:53 pm:
> 
> On Mon, 27 Feb 2012, Austin Clements  wrote:
> > Previously, fatal errors in add_files_recursive were not treated as
> > fatal by its callers (including itself!) and add_files_recursive
> > sometimes returned errors on non-fatal conditions.  This makes
> > add_files_recursive errors consistently fatal and updates all callers
> > to treat them as fatal.
> 
> Hi I have attempted to review this but am feeling a little out of my
> depth. This patch seems fine except for one thing I am unsure about:
> 
> > ---
> >  notmuch-new.c |   13 -
> >  1 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/notmuch-new.c b/notmuch-new.c
> > index 4f13535..bd9786a 100644
> > --- a/notmuch-new.c
> > +++ b/notmuch-new.c
> > @@ -308,7 +308,6 @@ add_files_recursive (notmuch_database_t *notmuch,
> >  if (num_fs_entries == -1) {
> > fprintf (stderr, "Error opening directory %s: %s\n",
> >  path, strerror (errno));
> > -   ret = NOTMUCH_STATUS_FILE_ERROR;
> > goto DONE;
> >  }
> >  
> 
> If I understand this, then this change makes a failure to open a
> directory non-fatal. In the comment before the function it says
> 
>  * Also, we don't immediately act on file/directory removal since we
>  * must ensure that in the case of a rename that the new filename is
>  * added before the old filename is removed, (so that no information
>  * is lost from the database).
>  
> I am  worried that we could fail to find some files because of the
> file error above, and then we delete them from the database. Maybe this
> could only happen if those emails have just been moved to this
> file-error directory?

Hmm.  This won't *actively* remove files, since that only happens if
we successfully scan a directory and find a message that's in the
database but not in that directory (*not* scanning the directory won't
add anything to the remove list).  However, you are right that if a
message is moved from some other directory into a directory that we
fail to open, that message will be deleted "by omission".

I've added the error back, along with a comment explaining it.

> Best wishes
> 
> Mark
> 
> > @@ -351,8 +350,10 @@ add_files_recursive (notmuch_database_t *notmuch,
> >  
> > next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
> > status = add_files_recursive (notmuch, next, state);
> > -   if (status && ret == NOTMUCH_STATUS_SUCCESS)
> > +   if (status) {
> > ret = status;
> > +   goto DONE;
> > +   }
> > talloc_free (next);
> > next = NULL;
> >  }
> > @@ -933,6 +934,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> >  }
> >  
> >  ret = add_files (notmuch, db_path, _files_state);
> > +if (ret)
> > +   goto DONE;
> >  
> >  gettimeofday (_start, NULL);
> >  for (f = add_files_state.removed_files->head; f && !interrupted; f = 
> > f->next) {
> > @@ -965,6 +968,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> > }
> >  }
> >  
> > +  DONE:
> >  talloc_free (add_files_state.removed_files);
> >  talloc_free (add_files_state.removed_directories);
> >  talloc_free (add_files_state.directory_mtimes);
> > @@ -1012,10 +1016,9 @@ notmuch_new_command (void *ctx, int argc, char 
> > *argv[])
> >  
> >  printf ("\n");
> >  
> > -if (ret) {
> > -   printf ("\nNote: At least one error was encountered: %s\n",
> > +if (ret)
> > +   printf ("\nNote: A fatal error was encountered: %s\n",
> > notmuch_status_to_string (ret));
> > -}
> >  
> >  notmuch_database_close (notmuch);

Re: [PATCH v2 2/4] new: Handle fatal errors in remove_filename and _remove_directory

[Mark Walters]

On Sun, 22 Apr 2012, Austin Clements amdra...@mit.edu wrote:
 Previously such errors were simply ignored.  Now they cause an
 immediate cleanup and abort.
 ---
  notmuch-new.c |   25 +++--
  1 files changed, 19 insertions(+), 6 deletions(-)

 diff --git a/notmuch-new.c b/notmuch-new.c
 index 15c0b36..92e0489 100644
 --- a/notmuch-new.c
 +++ b/notmuch-new.c
 @@ -785,8 +785,10 @@ remove_filename (notmuch_database_t *notmuch,
   add_files_state-renamed_messages++;
   if (add_files_state-synchronize_flags == TRUE)
   notmuch_message_maildir_flags_to_tags (message);
 -} else
 + status = NOTMUCH_STATUS_SUCCESS;
 +} else if (status == NOTMUCH_STATUS_SUCCESS) {
   add_files_state-removed_messages++;
 +}
  notmuch_message_destroy (message);
  notmuch_database_end_atomic (notmuch);
  return status;
 @@ -794,12 +796,13 @@ remove_filename (notmuch_database_t *notmuch,
  
  /* Recursively remove all filenames from the database referring to
   * 'path' (or to any of its children). */
 -static void
 +static notmuch_status_t
  _remove_directory (void *ctx,
  notmuch_database_t *notmuch,
  const char *path,
  add_files_state_t *add_files_state)
  {
 +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
  notmuch_directory_t *directory;
  notmuch_filenames_t *files, *subdirs;
  char *absolute;
 @@ -812,8 +815,10 @@ _remove_directory (void *ctx,
  {
   absolute = talloc_asprintf (ctx, %s/%s, path,
   notmuch_filenames_get (files));
 - remove_filename (notmuch, absolute, add_files_state);
 + status = remove_filename (notmuch, absolute, add_files_state);
   talloc_free (absolute);
 + if (status)
 + goto DONE;
  }
  
  for (subdirs = notmuch_directory_get_child_directories (directory);
 @@ -822,11 +827,15 @@ _remove_directory (void *ctx,
  {
   absolute = talloc_asprintf (ctx, %s/%s, path,
   notmuch_filenames_get (subdirs));
 - _remove_directory (ctx, notmuch, absolute, add_files_state);
 + status = _remove_directory (ctx, notmuch, absolute, add_files_state);
   talloc_free (absolute);
 + if (status)
 + goto DONE;
  }
  
 +  DONE:
  notmuch_directory_destroy (directory);
 +return NOTMUCH_STATUS_SUCCESS;

Doesn't this need to be return status or something?

Best wishes

Mark


  }
  
  int
 @@ -944,7 +953,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
  
  gettimeofday (tv_start, NULL);
  for (f = add_files_state.removed_files-head; f  !interrupted; f = 
 f-next) {
 - remove_filename (notmuch, f-filename, add_files_state);
 + ret = remove_filename (notmuch, f-filename, add_files_state);
 + if (ret)
 + goto DONE;
   if (do_print_progress) {
   do_print_progress = 0;
   generic_print_progress (Cleaned up, messages,
 @@ -955,7 +966,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
  
  gettimeofday (tv_start, NULL);
  for (f = add_files_state.removed_directories-head, i = 0; f  
 !interrupted; f = f-next, i++) {
 - _remove_directory (ctx, notmuch, f-filename, add_files_state);
 + ret = _remove_directory (ctx, notmuch, f-filename, add_files_state);
 + if (ret)
 + goto DONE;
   if (do_print_progress) {
   do_print_progress = 0;
   generic_print_progress (Cleaned up, directories,
 -- 
 1.7.9.1

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

Re: [PATCH v2 4/4] new: Fix missing end_atomic in remove_filename on error

[Mark Walters]

On Sun, 22 Apr 2012, Austin Clements amdra...@mit.edu wrote:
 Previously, if we failed to find the message by filename in
 remove_filename, we would return immediately from the function without
 ending its atomic block.  Now this code follows the usual goto DONE
 idiom to perform cleanup.

The whole series looks good to me modulo the return value in Patch 2/4.

Best wishes

Mark 

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

 diff --git a/notmuch-new.c b/notmuch-new.c
 index 2103b18..9eebea4 100644
 --- a/notmuch-new.c
 +++ b/notmuch-new.c
 @@ -779,7 +779,8 @@ remove_filename (notmuch_database_t *notmuch,
   return status;
  status = notmuch_database_find_message_by_filename (notmuch, path, 
 message);
  if (status || message == NULL)
 - return status;
 + goto DONE;
 +
  status = notmuch_database_remove_message (notmuch, path);
  if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
   add_files_state-renamed_messages++;
 @@ -790,6 +791,8 @@ remove_filename (notmuch_database_t *notmuch,
   add_files_state-removed_messages++;
  }
  notmuch_message_destroy (message);
 +
 +  DONE:
  notmuch_database_end_atomic (notmuch);
  return status;
  }
 -- 
 1.7.9.1

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

Re: [PATCH 2/2] emacs: Correctly quote non-text/plain parts in reply

[Mark Walters]

On Sun, 22 Apr 2012, Adam Wolfe Gordon awg+notm...@xvx.ca wrote:
 Quote non-text parts nicely by displaying them with mm-display-part
 before calling message-cite-original to quote them. HTML-only emails
 can now be quoted correctly.

My instinct would have been to do this with a temporary buffer rather
than the narrowing but I am *definitely* too much of a lisp beginner to
say that either is better.

(I think notmuch-show-mm-display-part-inline uses the temporary buffer
approach.)

Anyway, as it is, it looks correct and seems to work! (and is obviously
useful functionality)

Best wishes

Mark

 Mark the test for this feature as not broken.
 ---
  emacs/notmuch-mua.el |   20 +++-
  test/emacs   |1 -
  2 files changed, 15 insertions(+), 6 deletions(-)

 diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
 index 87bd88d..f7af789 100644
 --- a/emacs/notmuch-mua.el
 +++ b/emacs/notmuch-mua.el
 @@ -21,6 +21,7 @@
  
  (require 'json)
  (require 'message)
 +(require 'mm-view)
  (require 'format-spec)
  
  (require 'notmuch-lib)
 @@ -90,6 +91,19 @@ list.
   else if (notmuch-match-content-type (plist-get part :content-type) 
 text/*)
 collect part))
  
 +(defun notmuch-mua-insert-quotable-part (message part)
 +  (save-restriction
 +(narrow-to-region (point) (point))
 +(insert (notmuch-get-bodypart-content message part
 +   (plist-get part :id)
 +   notmuch-show-process-crypto))
 +(let ((handle (mm-make-handle (current-buffer)
 +   (list (plist-get part :content-type
 +   (end-of-orig (point-max)))
 +  (mm-display-part handle)
 +  (kill-region (point-min) end-of-orig))
 +(goto-char (point-max
 +
  ;; There is a bug in emacs 23's message.el that results in a newline
  ;; not being inserted after the References header, so the next header
  ;; is concatenated to the end of it. This function fixes the problem,
 @@ -169,11 +183,7 @@ list.
   ;; Get the parts of the original message that should be quoted; this 
 includes
   ;; all the text parts, except the non-preferred ones in a 
 multipart/alternative.
   (let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get 
 original :body
 -   (mapc (lambda (part)
 -   (insert (notmuch-get-bodypart-content original part
 - (plist-get part :id)
 - 
 notmuch-show-process-crypto)))
 - quotable-parts))
 +   (mapc (apply-partially 'notmuch-mua-insert-quotable-part original) 
 quotable-parts))
  
   (set-mark (point))
   (goto-char start)
 diff --git a/test/emacs b/test/emacs
 index e648f80..579844f 100755
 --- a/test/emacs
 +++ b/test/emacs
 @@ -445,7 +445,6 @@ EOF
  test_expect_equal_file OUTPUT EXPECTED
  
  test_begin_subtest Reply within emacs to an html-only message
 -test_subtest_known_broken
  add_message '[content-type]=text/html' \
   '[body]=Hi,br /This is an bHTML/b test message.br /br 
 /OK?'
  test_emacs (let ((message-hidden-headers '()))
 -- 
 1.7.5.4

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

Re: [PATCH 7/7] python: wrap and use notmuch_database_destroy as destructor

[Justus Winter]

Quoting Austin Clements (2012-04-12 19:02:49)
 Quoth Justus Winter on Mar 21 at  1:55 am:
  Adapt the go bindings to the notmuch_database_close split.
 
 Typo.

Duh ,

  Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
  ---
   bindings/python/notmuch/database.py |   17 +++--
   1 files changed, 15 insertions(+), 2 deletions(-)
  
  diff --git a/bindings/python/notmuch/database.py 
  b/bindings/python/notmuch/database.py
  index 44d40fd..9a1896b 100644
  --- a/bindings/python/notmuch/database.py
  +++ b/bindings/python/notmuch/database.py
  @@ -218,9 +218,22 @@ class Database(object):
   _close.restype = None
   
   def close(self):
  -Close and free the notmuch database if needed
  -if self._db is not None:
  +'''
  +Closes the notmuch database.
  +'''
  +if self._db:
   self._close(self._db)
  +
  +_destroy = nmlib.notmuch_database_destroy
  +_destroy.argtypes = [NotmuchDatabaseP]
  +_destroy.restype = None
  +
  +def destroy(self):
 
 Should this be __del__?  The existing __del__ is certainly wrong with
 this change, since it only closes the database and doesn't free it.  I
 think this function is exactly what you want __del__ to be now.
 
 (I think it also doesn't make sense to expose notmuch_database_destroy
 as a general, public method since it will free all of the other C
 objects out from under the bindings, resulting in exactly the double
 free-type crashes that you're trying to avoid.  It appears that none
 of the other Python classes have a destroy method.)

Yes, of course...

I'm going to send an rebased and updated patch series as a follow up
to this message. Thanks for the input everyone :)

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

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   15 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
read-write mode.\n,
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch-mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error error) {
fprintf (stderr, A Xapian exception occurred opening database: %s\n,
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }
 
@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }
 
 delete notmuch-term_gen;
+notmuch-term_gen = NULL;
 delete notmuch-query_parser;
+notmuch-query_parser = NULL;
 delete notmuch-xapian_db;
+notmuch-xapian_db = NULL;
 delete notmuch-value_range_processor;
+notmuch-value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 673c423..84c9265 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named .notmuch below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,18 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);
 
-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * This function is called by notmuch_database_destroy and can be
+ * called multiple times. */
 void
 notmuch_database_close (notmuch_database_t *database);
 
+/* Destroy the notmuch database freeing all associated
+ * resources */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.10

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

[PATCH 2/7] NEWS: Document the notmuch_database_close split

[Justus Winter]

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 NEWS |   12 
 1 file changed, 12 insertions(+)

diff --git a/NEWS b/NEWS
index c1704e0..a2cd080 100644
--- a/NEWS
+++ b/NEWS
@@ -73,6 +73,18 @@ leaving Mutt.  notmuch-mutt, formerly distributed under the 
name
 mutt-notmuch by Stefano Zacchiroli, will be maintained as a notmuch
 contrib/ from now on.
 
+Library changes
+---
+
+API changes
+
+  The function notmuch_database_close has been split into
+  notmuch_database_close and notmuch_database_destroy.
+
+  This makes it possible for long running programs to close the xapian
+  database and thus release the lock associated with it without
+  destroying the data structures obtained from it.
+
 Notmuch 0.12 (2012-03-20)
 =
 
-- 
1.7.10

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

[PATCH 4/7] Use notmuch_database_destroy instead of notmuch_database_close

[Justus Winter]

Adapt notmuch-deliver to the notmuch_database_close split.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 contrib/notmuch-deliver/src/main.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/notmuch-deliver/src/main.c 
b/contrib/notmuch-deliver/src/main.c
index 6f32f73..37d2919 100644
--- a/contrib/notmuch-deliver/src/main.c
+++ b/contrib/notmuch-deliver/src/main.c
@@ -455,7 +455,7 @@ main(int argc, char **argv)
g_strfreev(opt_rtags);
g_free(mail);
 
-   notmuch_database_close(db);
+   notmuch_database_destroy(db);
 
return 0;
 }
-- 
1.7.10

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

[PATCH 3/7] Use notmuch_database_destroy instead of notmuch_database_close

[Justus Winter]

Adapt the notmuch binaries source to the notmuch_database_close split.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 notmuch-count.c   |2 +-
 notmuch-dump.c|2 +-
 notmuch-new.c |2 +-
 notmuch-reply.c   |2 +-
 notmuch-restore.c |2 +-
 notmuch-search.c  |2 +-
 notmuch-show.c|2 +-
 notmuch-tag.c |2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index b76690c..9c2ad7b 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -107,7 +107,7 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
 }
 
 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 
 return 0;
 }
diff --git a/notmuch-dump.c b/notmuch-dump.c
index a735875..71ab0ea 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -116,7 +116,7 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
fclose (output);
 
 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 
 return 0;
 }
diff --git a/notmuch-new.c b/notmuch-new.c
index 4f13535..f078753 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1017,7 +1017,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
notmuch_status_to_string (ret));
 }
 
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 
 if (run_hooks  !ret  !interrupted)
ret = notmuch_run_hook (db_path, post-new);
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 0949d9f..da99a13 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -755,7 +755,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
return 1;
 
 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 
 if (params.cryptoctx)
g_object_unref(params.cryptoctx);
diff --git a/notmuch-restore.c b/notmuch-restore.c
index d3b9246..02b563c 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -192,7 +192,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 if (line)
free (line);
 
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 if (input != stdin)
fclose (input);
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 1cc8430..7dfd270 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -545,7 +545,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 }
 
 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 
 return ret;
 }
diff --git a/notmuch-show.c b/notmuch-show.c
index da4a797..3b6667c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1117,7 +1117,7 @@ notmuch_show_command (void *ctx, unused (int argc), 
unused (char *argv[]))
 }
 
 notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 
 if (params.cryptoctx)
g_object_unref(params.cryptoctx);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 05feed3..bd56fd1 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -238,7 +238,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 
 ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags);
 
-notmuch_database_close (notmuch);
+notmuch_database_destroy (notmuch);
 
 return ret;
 }
-- 
1.7.10

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

[PATCH 6/7] ruby: Use notmuch_database_destroy instead of notmuch_database_close

[Justus Winter]

Adapt the ruby bindings to the notmuch_database_close split.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 bindings/ruby/database.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 982fd59..ba9a139 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -110,7 +110,7 @@ notmuch_rb_database_close (VALUE self)
 notmuch_database_t *db;
 
 Data_Get_Notmuch_Database (self, db);
-notmuch_database_close (db);
+notmuch_database_destroy (db);
 DATA_PTR (self) = NULL;
 
 return Qnil;
-- 
1.7.10

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

[PATCH 5/7] go: Use notmuch_database_destroy instead of notmuch_database_close

[Justus Winter]

Adapt the go bindings to the notmuch_database_close split.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 bindings/go/pkg/notmuch.go |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bindings/go/pkg/notmuch.go b/bindings/go/pkg/notmuch.go
index c6844ef..d32901d 100644
--- a/bindings/go/pkg/notmuch.go
+++ b/bindings/go/pkg/notmuch.go
@@ -114,7 +114,7 @@ func NewDatabase(path string) *Database {
  * An existing notmuch database can be identified by the presence of a
  * directory named .notmuch below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -140,7 +140,7 @@ func OpenDatabase(path string, mode DatabaseMode) *Database 
{
 /* Close the given notmuch database, freeing all associated
  * resources. See notmuch_database_open. */
 func (self *Database) Close() {
-   C.notmuch_database_close(self.db)
+   C.notmuch_database_destroy(self.db)
 }
 
 /* Return the database path of the given database.
-- 
1.7.10

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

[PATCH 7/7] python: wrap and use notmuch_database_destroy as destructor

[Justus Winter]

Adapt the python bindings to the notmuch_database_close split.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 bindings/python/notmuch/database.py |   14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/bindings/python/notmuch/database.py 
b/bindings/python/notmuch/database.py
index 44d40fd..268e952 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -161,8 +161,13 @@ class Database(object):
 else:
 self.create(path)
 
+_destroy = nmlib.notmuch_database_destroy
+_destroy.argtypes = [NotmuchDatabaseP]
+_destroy.restype = None
+
 def __del__(self):
-self.close()
+if self._db:
+self._destroy(self._db)
 
 def _assert_db_is_initialized(self):
 Raises :exc:`NotInitializedError` if self._db is `None`
@@ -218,10 +223,11 @@ class Database(object):
 _close.restype = None
 
 def close(self):
-Close and free the notmuch database if needed
-if self._db is not None:
+'''
+Closes the notmuch database.
+'''
+if self._db:
 self._close(self._db)
-self._db = None
 
 def __enter__(self):
 '''
-- 
1.7.10

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

folder searches don't work (sometimes) with current git

[David Riebenbauer]

Hi,

just a short notice, before I start further investigation into this
bug.

For some reason folder:path searches won't work for new messages.
After I tag a message taht won't be found there, by running `notmuch
tag` it is found again.

More details as soon as I find time for more investigation.

Regards,

-- 
David Riebenbauer
Jabber: davr...@jabber.ccc.de - ICQ: 322056002 
http://liegesta.at
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/7] NEWS: Document the notmuch_database_close split

[Felipe Contreras]

On Sun, Apr 22, 2012 at 3:07 PM, Justus Winter
4win...@informatik.uni-hamburg.de wrote:
 Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
 ---
  NEWS |   12 
  1 file changed, 12 insertions(+)

 diff --git a/NEWS b/NEWS
 index c1704e0..a2cd080 100644
 --- a/NEWS
 +++ b/NEWS
 @@ -73,6 +73,18 @@ leaving Mutt.  notmuch-mutt, formerly distributed under 
 the name
  mutt-notmuch by Stefano Zacchiroli, will be maintained as a notmuch
  contrib/ from now on.

 +Library changes
 +---
 +
 +API changes
 +
 +  The function notmuch_database_close has been split into
 +  notmuch_database_close and notmuch_database_destroy.
 +
 +  This makes it possible for long running programs to close the xapian
 +  database and thus release the lock associated with it without
 +  destroying the data structures obtained from it.
 +

I haven't following this change. Who can an application take advantage
of this? I call _close(), now what do I do to use the database again?

Cheers.

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

Re: [PATCH v2 2/4] new: Handle fatal errors in remove_filename and _remove_directory

[Austin Clements]

Quoth Mark Walters on Apr 22 at  8:34 am:
 On Sun, 22 Apr 2012, Austin Clements amdra...@mit.edu wrote:
  Previously such errors were simply ignored.  Now they cause an
  immediate cleanup and abort.
  ---
   notmuch-new.c |   25 +++--
   1 files changed, 19 insertions(+), 6 deletions(-)
 
  diff --git a/notmuch-new.c b/notmuch-new.c
  index 15c0b36..92e0489 100644
  --- a/notmuch-new.c
  +++ b/notmuch-new.c
  @@ -785,8 +785,10 @@ remove_filename (notmuch_database_t *notmuch,
  add_files_state-renamed_messages++;
  if (add_files_state-synchronize_flags == TRUE)
  notmuch_message_maildir_flags_to_tags (message);
  -} else
  +   status = NOTMUCH_STATUS_SUCCESS;
  +} else if (status == NOTMUCH_STATUS_SUCCESS) {
  add_files_state-removed_messages++;
  +}
   notmuch_message_destroy (message);
   notmuch_database_end_atomic (notmuch);
   return status;
  @@ -794,12 +796,13 @@ remove_filename (notmuch_database_t *notmuch,
   
   /* Recursively remove all filenames from the database referring to
* 'path' (or to any of its children). */
  -static void
  +static notmuch_status_t
   _remove_directory (void *ctx,
 notmuch_database_t *notmuch,
 const char *path,
 add_files_state_t *add_files_state)
   {
  +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
   notmuch_directory_t *directory;
   notmuch_filenames_t *files, *subdirs;
   char *absolute;
  @@ -812,8 +815,10 @@ _remove_directory (void *ctx,
   {
  absolute = talloc_asprintf (ctx, %s/%s, path,
  notmuch_filenames_get (files));
  -   remove_filename (notmuch, absolute, add_files_state);
  +   status = remove_filename (notmuch, absolute, add_files_state);
  talloc_free (absolute);
  +   if (status)
  +   goto DONE;
   }
   
   for (subdirs = notmuch_directory_get_child_directories (directory);
  @@ -822,11 +827,15 @@ _remove_directory (void *ctx,
   {
  absolute = talloc_asprintf (ctx, %s/%s, path,
  notmuch_filenames_get (subdirs));
  -   _remove_directory (ctx, notmuch, absolute, add_files_state);
  +   status = _remove_directory (ctx, notmuch, absolute, add_files_state);
  talloc_free (absolute);
  +   if (status)
  +   goto DONE;
   }
   
  +  DONE:
   notmuch_directory_destroy (directory);
  +return NOTMUCH_STATUS_SUCCESS;
 
 Doesn't this need to be return status or something?

Arrg.  Yes, of course.  I wish I knew a good way to test this stuff.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v3 1/4] new: Consistently treat fatal errors as fatal

[Austin Clements]

Previously, fatal errors in add_files_recursive were not treated as
fatal by its callers (including itself!).  This makes
add_files_recursive errors consistently fatal and updates all callers
to treat them as fatal.
---
 notmuch-new.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 4f13535..15c0b36 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -308,6 +308,10 @@ add_files_recursive (notmuch_database_t *notmuch,
 if (num_fs_entries == -1) {
fprintf (stderr, Error opening directory %s: %s\n,
 path, strerror (errno));
+   /* We consider this a fatal error because, if a user moved a
+* message from another directory that we were able to scan
+* into this directory, skipping this directory will cause
+* that message to be lost. */
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
@@ -351,8 +355,10 @@ add_files_recursive (notmuch_database_t *notmuch,
 
next = talloc_asprintf (notmuch, %s/%s, path, entry-d_name);
status = add_files_recursive (notmuch, next, state);
-   if (status  ret == NOTMUCH_STATUS_SUCCESS)
+   if (status) {
ret = status;
+   goto DONE;
+   }
talloc_free (next);
next = NULL;
 }
@@ -933,6 +939,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 }
 
 ret = add_files (notmuch, db_path, add_files_state);
+if (ret)
+   goto DONE;
 
 gettimeofday (tv_start, NULL);
 for (f = add_files_state.removed_files-head; f  !interrupted; f = 
f-next) {
@@ -965,6 +973,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
}
 }
 
+  DONE:
 talloc_free (add_files_state.removed_files);
 talloc_free (add_files_state.removed_directories);
 talloc_free (add_files_state.directory_mtimes);
@@ -1012,10 +1021,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
 printf (\n);
 
-if (ret) {
-   printf (\nNote: At least one error was encountered: %s\n,
+if (ret)
+   printf (\nNote: A fatal error was encountered: %s\n,
notmuch_status_to_string (ret));
-}
 
 notmuch_database_close (notmuch);
 
-- 
1.7.9.1

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

[PATCH v3 3/4] new: Print final fatal error message to stderr

[Austin Clements]

This was going to stdout.  I removed the newline at the beginning of
printing the fatal error message because it wouldn't make sense if you
were only looking at the stderr stream (e.g., you had redirected
stdout to /dev/null).
---
 notmuch-new.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 2faf2f8..bf9b120 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1035,8 +1035,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 printf (\n);
 
 if (ret)
-   printf (\nNote: A fatal error was encountered: %s\n,
-   notmuch_status_to_string (ret));
+   fprintf (stderr, Note: A fatal error was encountered: %s\n,
+notmuch_status_to_string (ret));
 
 notmuch_database_close (notmuch);
 
-- 
1.7.9.1

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

[PATCH v3 4/4] new: Fix missing end_atomic in remove_filename on error

[Austin Clements]

Previously, if we failed to find the message by filename in
remove_filename, we would return immediately from the function without
ending its atomic block.  Now this code follows the usual goto DONE
idiom to perform cleanup.
---
 notmuch-new.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index bf9b120..473201e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -779,7 +779,8 @@ remove_filename (notmuch_database_t *notmuch,
return status;
 status = notmuch_database_find_message_by_filename (notmuch, path, 
message);
 if (status || message == NULL)
-   return status;
+   goto DONE;
+
 status = notmuch_database_remove_message (notmuch, path);
 if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
add_files_state-renamed_messages++;
@@ -790,6 +791,8 @@ remove_filename (notmuch_database_t *notmuch,
add_files_state-removed_messages++;
 }
 notmuch_message_destroy (message);
+
+  DONE:
 notmuch_database_end_atomic (notmuch);
 return status;
 }
-- 
1.7.9.1

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

Re: [PATCH 1/7] Split notmuch_database_close into two functions

[Austin Clements]

Quoth Justus Winter on Apr 22 at  2:07 pm:
 Formerly notmuch_database_close closed the xapian database and
 destroyed the talloc structure associated with the notmuch database
 object. Split notmuch_database_close into notmuch_database_close and
 notmuch_database_destroy.
 
 This makes it possible for long running programs to close the xapian
 database and thus release the lock associated with it without
 destroying the data structures obtained from it.
 
 This also makes the api more consistent since every other data
 structure has a destructor function.
 
 Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
 ---
  lib/database.cc |   14 --
  lib/notmuch.h   |   15 +++
  2 files changed, 23 insertions(+), 6 deletions(-)
 
 diff --git a/lib/database.cc b/lib/database.cc
 index 16c4354..2fefcad 100644
 --- a/lib/database.cc
 +++ b/lib/database.cc
 @@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
   read-write mode.\n,
notmuch_path, version, NOTMUCH_DATABASE_VERSION);
   notmuch-mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
 - notmuch_database_close (notmuch);
 + notmuch_database_destroy (notmuch);
   notmuch = NULL;
   goto DONE;
   }
 @@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
  } catch (const Xapian::Error error) {
   fprintf (stderr, A Xapian exception occurred opening database: %s\n,
error.get_msg().c_str());
 - notmuch_database_close (notmuch);
 + notmuch_database_destroy (notmuch);
   notmuch = NULL;
  }
  
 @@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
  }
  
  delete notmuch-term_gen;
 +notmuch-term_gen = NULL;
  delete notmuch-query_parser;
 +notmuch-query_parser = NULL;
  delete notmuch-xapian_db;
 +notmuch-xapian_db = NULL;
  delete notmuch-value_range_processor;
 +notmuch-value_range_processor = NULL;
 +}
 +
 +void
 +notmuch_database_destroy (notmuch_database_t *notmuch)
 +{
 +notmuch_database_close (notmuch);
  talloc_free (notmuch);
  }
  
 diff --git a/lib/notmuch.h b/lib/notmuch.h
 index 673c423..84c9265 100644
 --- a/lib/notmuch.h
 +++ b/lib/notmuch.h
 @@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
   *
   * After a successful call to notmuch_database_create, the returned
   * database will be open so the caller should call
 - * notmuch_database_close when finished with it.
 + * notmuch_database_destroy when finished with it.
   *
   * The database will not yet have any data in it
   * (notmuch_database_create itself is a very cheap function). Messages
 @@ -165,7 +165,7 @@ typedef enum {
   * An existing notmuch database can be identified by the presence of a
   * directory named .notmuch below 'path'.
   *
 - * The caller should call notmuch_database_close when finished with
 + * The caller should call notmuch_database_destroy when finished with
   * this database.
   *
   * In case of any failure, this function returns NULL, (after printing
 @@ -175,11 +175,18 @@ notmuch_database_t *
  notmuch_database_open (const char *path,
  notmuch_database_mode_t mode);
  
 -/* Close the given notmuch database, freeing all associated
 - * resources. See notmuch_database_open. */
 +/* Close the given notmuch database.
 + *
 + * This function is called by notmuch_database_destroy and can be
 + * called multiple times. */

This needs a comment explaining the implications of closing the
database.  Perhaps something like this (modeled on
Xapian::Database::close's documentation)

/* Close the given notmuch database.
 *
 * After notmuch_database_close has been called, calls to other
 * functions on objects derived from this database may either behave
 * as if the database had not been closed (e.g., if the required data
 * has been cached) or may fail with a
 * NOTMUCH_STATUS_XAPIAN_EXCEPTION.
 *
 * notmuch_database_close can be called multiple times.  Later calls
 * have no affect. 
 */

  void
  notmuch_database_close (notmuch_database_t *database);
  
 +/* Destroy the notmuch database freeing all associated
 + * resources */

This should mention that this also closes the database (currently you
mention this in the doc for notmuch_database_close, but it's a reverse
reference there; that could probably even be omitted).  Perhaps

/* Destroy the notmuch database, closing it if necessary and freeing
 * all associated resources. */

 +void
 +notmuch_database_destroy (notmuch_database_t *database);
 +
  /* Return the database path of the given database.
   *
   * The return value is a string owned by notmuch so should not be
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v3 1/4] emacs: Let the user choose where to compose new mails

[Thomas Jost]

Le 15 avril 2012 à 16:52 CEST, David Bremner a écrit :
 Jameson Graef Rollins jroll...@finestructure.net writes:

 I think the issues that David was experiencing have to do with flakiness
 in emacs's dedicated windows, not in this patch itself.

 Thomas,

 Did you have a change to investigate this as proposed in
 id:87zke0aifa@thor.loria.fr?

David,

Sorry for the delay. I did investigate a little bit, but I did not try
to write a patch to fix the wrong behaviour in Emacs 23.

AFAICT, Emacs 23 is just buggy in this case. By reading the code of
message-send-and-exit and message-bury [1], here is what happens when
you call message-send-buffer-and-exit with message-kill-buffer-on-exit
set to nil:
- message is sent
- buffer is buried with burry-buffer
- message-bury: if the window is dedicated and its frame not the only
  visible frame, then this frame is deleted

which explains what happens in Emacs 23 both in daemon and non-daemon
mode.

In Emacs 24 [2], here is what happens:
- message is sent
- message-bury: buffer is buried with bury-buffer

which is (obviously?) correct.

Really, this looks like a bug in Emacs 23 to me. Emacs 24 has been fixed
by Gnus commits [3] and [4] (maybe [3] is enough, I didn't try). Users
of Emacs 23 can probably just use an up-to-date version of Gnus to have
this issue fixed.

So I'm not sure it would make sense to try to come up with a workaround
in my patch, nor if it would be worth it. Maybe just adding a message
suggesting Emacs 23 users to enable message-kill-buffer-on-exit if they
use the Gnus version shipped with Emacs?

Other than that, Jameson's commit [5] is exactly the same as the one in
my tree with a better commit message, so I'm in favor of pulling it.

[1] 
http://bzr.savannah.gnu.org/lh/emacs/emacs-23/annotate/head:/lisp/gnus/message.el
[2] 
http://bzr.savannah.gnu.org/lh/emacs/emacs-24/annotate/head:/lisp/gnus/message.el
[3] 
http://git.gnus.org/cgit/gnus.git/commit/?id=30eb6d24d30bc028fce91a0c62044c5dc1fdd90e
[4] 
http://git.gnus.org/cgit/gnus.git/commit/?id=e3fc7cb33eb07dd3b48cfc72f0cada1f1edbcb85
[5] id:1334436137-6099-1-git-send-email-jroll...@finestructure.net

Regards,

--
Thomas/Schnouki


pgpU4xJp7PmDM.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[ANN] New awesome vim plug-in using Ruby bindings

[Felipe Contreras]

Hi,

I've never been particularly happy with the code of the vim plug-in,
but it sort of did the job, after some fixes, and has been working
great so far for most of my needs even though it's clearly very rough
on the edges.

However, I'm recently in need of been able to read HTML mails, and
just trying to add that code was a nightmare, so I decided to look for
alternatives, including Anton's Python vim plug-in (which is nice, but
doesn't have support for that), and even learning emacs, to use what
most people here use (but it turns out the HTML messages don't work
correctly there either). I also tried the various mutt+notmuch
options, and none fit the bill.

So, since I'm a big fan of Ruby, I decided to try my luck writing a
plug-in from scratch. It took me one weekend, but I'm pretty happy
with the result. This plug-in has already essentially all the
functionality of the current one, but it's much, *much* simpler (only
600) lines of code.

And in addition has many more features:

 * Gradual searches; you don't have to wait for the whole search to finish,
   sort of like the 'less' command
 * Proper multi-part handling; finds out if there's text/plain, or if
   text/html, converts it using elinks
 * Extract all attachments
 * Open message with mutt (or any external application that can open an mbox)
 * More proper UTF-8 handling
 * Configurable key mappings
 * Much simpler, cleaner, beautiful, and extensible code (only 600 lines!)

I just added support to reply mails today, and after trying a bit I
got complaints from the vger.kernel.org server, but people using mutt
have had the same complaint, so I don't know, I wouldn't reply totally
on that. *But* you can open the mail with mutt, or any other client
that you want, as a fall-back option (the command to run is
configurable).

Sure, it depends on the Ruby bindings from notmuch (but those are easy
to compile), and on the 'mail' library from Ruby (easy to install),
but it makes things much, *much* easier. There might be ways to make
certain dependencies optional, and make this, and the current plug-in
converge somehow (maybe even the python one too), but for now I don't
see any reason to look back.

I can't wait to start using it for real :)

Enjoy ;)

https://github.com/felipec/notmuch-vim-ruby

P.S. I CC'ed a bunch of people that have showed interest in the vim
interface, I hope you don't mind

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

Re: [ANN] New awesome vim plug-in using Ruby bindings

[Adam Wolfe Gordon]

Hi Felipe,

This work sounds nice - it's good to have lots of interface choices.
One question below:

On Sun, Apr 22, 2012 at 19:12, Felipe Contreras
felipe.contre...@gmail.com wrote:
 doesn't have support for that), and even learning emacs, to use what
 most people here use (but it turns out the HTML messages don't work
 correctly there either). I also tried the various mutt+notmuch

HTML emails should work properly in emacs, and work even better if you
have w3.el installed. Replying to HTML emails isn't in git yet, but
there are patches for it in progress [1]. If there's a bug in the HTML
support, we should probably fix it, so could you explain what doesn't
work for you?

Thanks,

-- Adam

[1] id:1335056093-17621-1-git-send-email-awg+notm...@xvx.ca
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch