update count API round 2

update count API round 2

[David Bremner]

This obsoletes

     id:1419855291-31972-1-git-send-email-david@tethera.net
     
It got bigger when I updated the python and ruby bindings. The go
bindings are still broken by this series.

The first 4 commits are just updating the test suite so that I can
actually see the breakage in the API bindings introduced by the last
commit.

[Patch v2 1/5] build: integrate building ruby bindings into notmuch build process

[David Bremner]

Because ruby generates a Makefile, we have to use recursive make.
Because mkmf.rb hardcodes the name Makefile, put our Makefile{.local}
in the parent directory.
---
 bindings/Makefile       |  7 +++++++
 bindings/Makefile.local | 18 ++++++++++++++++++
 configure               | 18 ++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 bindings/Makefile
 create mode 100644 bindings/Makefile.local

diff --git a/bindings/Makefile b/bindings/Makefile
new file mode 100644
index 0000000..de492a7
--- /dev/null
+++ b/bindings/Makefile
@@ -0,0 +1,7 @@
+# See Makefile.local for the list of files to be compiled in this
+# directory.
+all:
+	$(MAKE) -C .. all
+
+.DEFAULT:
+	$(MAKE) -C .. $@
diff --git a/bindings/Makefile.local b/bindings/Makefile.local
new file mode 100644
index 0000000..f395c81
--- /dev/null
+++ b/bindings/Makefile.local
@@ -0,0 +1,18 @@
+# -*- makefile -*-
+
+dir := bindings
+
+# force the shared library to be build
+ruby-bindings: lib/libnotmuch.so
+ifeq ($(HAVE_RUBY_DEV),1)
+	cd $(dir)/ruby && ruby extconf.rb
+	$(MAKE) -C $(dir)/ruby
+else
+	@echo Missing dependency, skipping ruby bindings
+endif
+
+CLEAN += $(patsubst %,$(dir)/ruby/%, \
+	Makefile database.o directory.o filenames.o\
+	init.o message.o messages.o mkmf.log notmuch.so query.o \
+	status.o tags.o thread.o threads.o)
+
diff --git a/configure b/configure
index d14e7d1..7df3b29 100755
--- a/configure
+++ b/configure
@@ -21,6 +21,7 @@ srcdir=$(dirname "$0")
 
 subdirs="util compat lib parse-time-string completion doc emacs"
 subdirs="${subdirs} performance-test test test/test-databases"
+subdirs="${subdirs} bindings"
 
 # For a non-srcdir configure invocation (such as ../configure), create
 # the directory structure and copy Makefiles.
@@ -426,6 +427,15 @@ else
     have_doxygen=0
 fi
 
+printf "Checking for ruby development files... "
+if ruby -e "require 'mkmf'"> /dev/null 2>&1; then
+    printf "Yes.\n"
+    have_ruby_dev=1
+else
+    printf "No (skipping ruby bindings)\n"
+    have_ruby_dev=0
+fi
+
 printf "Checking if sphinx is available and supports nroff output... "
 if hash sphinx-build > /dev/null 2>&1 && python -m sphinx.writers.manpage > /dev/null 2>&1 ; then
     printf "Yes.\n"
@@ -848,6 +858,10 @@ HAVE_CANONICALIZE_FILE_NAME = ${have_canonicalize_file_name}
 # build its own version)
 HAVE_GETLINE = ${have_getline}
 
+# Are the ruby development files (and ruby) available? If not skip
+# building/testing ruby bindings.
+HAVE_RUBY_DEV = ${have_ruby_dev}
+
 # Whether the strcasestr function is available (if not, then notmuch will
 # build its own version)
 HAVE_STRCASESTR = ${have_strcasestr}
@@ -958,6 +972,10 @@ NOTMUCH_HAVE_XAPIAN_COMPACT=${have_xapian_compact}
 # Whether there's either sphinx or rst2man available for building
 # documentation
 NOTMUCH_HAVE_MAN=$((have_sphinx || have_rst2man))
+
+# Are the ruby development files (and ruby) available? If not skip
+# building/testing ruby bindings.
+NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
 EOF
 
 # Finally, after everything configured, inform the user how to continue.
-- 
2.1.3

[Patch v2 2/5] test: add python tests for query.count_{messages,threads}

[David Bremner]

These are more or less cargo culted from the existing python tests. In
particular they compare against the results of doing an analogous
query using the CLI.
---
 test/T390-python.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/test/T390-python.sh b/test/T390-python.sh
index 3f03a2e..ddbf164 100755
--- a/test/T390-python.sh
+++ b/test/T390-python.sh
@@ -36,4 +36,24 @@ print db.find_message_by_filename("i-dont-exist")
 EOF
 test_expect_equal "$(cat OUTPUT)" "None"
 
+test_begin_subtest "count messages"
+test_python <<EOF
+import notmuch
+db = notmuch.Database(mode=notmuch.Database.MODE.READ_ONLY)
+q_new = notmuch.Query(db, 'tag:inbox')
+print q_new.count_messages()
+EOF
+notmuch count --output=messages tag:inbox > EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "count threads"
+test_python <<EOF
+import notmuch
+db = notmuch.Database(mode=notmuch.Database.MODE.READ_ONLY)
+q_new = notmuch.Query(db, 'tag:inbox')
+print q_new.count_threads()
+EOF
+notmuch count --output=threads tag:inbox > EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
 test_done
-- 
2.1.3

[Patch v2 3/5] test: port existing python tests to ruby

[David Bremner]

This is pretty much a line by line translation.  Add a dependency of
the test suite on the ruby bindings, as these are currently not built
by default.
---
 test/Makefile.local |  2 +-
 test/T395-ruby.sh   | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/test-lib.sh    |  5 ++++
 3 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100755 test/T395-ruby.sh

diff --git a/test/Makefile.local b/test/Makefile.local
index 2331ceb..9423680 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -55,7 +55,7 @@ TEST_BINARIES := $(TEST_BINARIES:.cc=)
 
 test-binaries: $(TEST_BINARIES)
 
-test:	all test-binaries
+test:	all test-binaries ruby-bindings
 	@${test_src_dir}/notmuch-test $(OPTIONS)
 
 check: test
diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
new file mode 100755
index 0000000..6b89a5d
--- /dev/null
+++ b/test/T395-ruby.sh
@@ -0,0 +1,86 @@
+#!/usr/bin/env bash
+test_description="ruby bindings"
+. ./test-lib.sh
+
+if [ "${NOTMUCH_HAVE_RUBY_DEV}" = "0" ]; then
+    test_subtest_missing_external_prereq_["ruby development files"]=t
+fi
+
+add_email_corpus
+
+test_begin_subtest "compare thread ids"
+test_ruby <<"EOF"
+require 'notmuch'
+$maildir = ENV['MAIL_DIR']
+if not $maildir then
+  abort('environment variable MAIL_DIR must be set')
+end
+@db = Notmuch::Database.new($maildir)
+@q = @db.query('tag:inbox')
+@q.sort = Notmuch::SORT_OLDEST_FIRST
+for t in @q.search_threads do
+  print t.thread_id, "\n"
+end
+EOF
+notmuch search --sort=oldest-first --output=threads tag:inbox | sed s/^thread:// > EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "compare message ids"
+test_ruby <<"EOF"
+require 'notmuch'
+$maildir = ENV['MAIL_DIR']
+if not $maildir then
+  abort('environment variable MAIL_DIR must be set')
+end
+@db = Notmuch::Database.new($maildir)
+@q = @db.query('tag:inbox')
+@q.sort = Notmuch::SORT_OLDEST_FIRST
+for m in @q.search_messages do
+  print m.message_id, "\n"
+end
+EOF
+notmuch search --sort=oldest-first --output=messages tag:inbox | sed s/^id:// > EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "get non-existent file"
+test_ruby <<"EOF"
+require 'notmuch'
+$maildir = ENV['MAIL_DIR']
+if not $maildir then
+  abort('environment variable MAIL_DIR must be set')
+end
+@db = Notmuch::Database.new($maildir)
+result = @db.find_message_by_filename('i-dont-exist')
+print (result == nil)
+EOF
+test_expect_equal "$(cat OUTPUT)" "true"
+
+test_begin_subtest "count messages"
+test_ruby <<"EOF"
+require 'notmuch'
+$maildir = ENV['MAIL_DIR']
+if not $maildir then
+  abort('environment variable MAIL_DIR must be set')
+end
+@db = Notmuch::Database.new($maildir)
+@q = @db.query('tag:inbox')
+print @q.count_messages(),"\n"
+EOF
+notmuch count --output=messages tag:inbox > EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "count messages"
+test_ruby <<"EOF"
+require 'notmuch'
+$maildir = ENV['MAIL_DIR']
+if not $maildir then
+  abort('environment variable MAIL_DIR must be set')
+end
+@db = Notmuch::Database.new($maildir)
+@q = @db.query('tag:inbox')
+print @q.count_threads(),"\n"
+EOF
+notmuch count --output=threads tag:inbox > EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
+test_done
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 53db9ca..3bbdc03 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1156,6 +1156,11 @@ test_python() {
 		| $cmd -
 }
 
+test_ruby() {
+    export LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib
+    MAIL_DIR=$MAIL_DIR ruby -I $TEST_DIRECTORY/../bindings/ruby> OUTPUT
+}
+
 # Creates a script that counts how much time it is executed and calls
 # notmuch.  $notmuch_counter_command is set to the path to the
 # generated script.  Use notmuch_counter_value() function to get the
-- 
2.1.3

[Patch v2 4/5] bindings/ruby: gitignore *.o

[David Bremner]

---
 bindings/ruby/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bindings/ruby/.gitignore b/bindings/ruby/.gitignore
index fa25752..d682798 100644
--- a/bindings/ruby/.gitignore
+++ b/bindings/ruby/.gitignore
@@ -4,3 +4,4 @@
 Makefile
 mkmf.log
 notmuch.so
+*.o
-- 
2.1.3

[Patch v2 5/5] lib: add status return to notmuch_query_count_{message,threads}

[David Bremner]

We follow many other notmuch_ functions by having a status return and
an output parameter.  I decided not to add compatibility wrappers but
just to break the API because the old API seems worth encouraging
people to move away from.  This change is large than might otherwise be desirable because it updates the CLI, the python bindings and the ruby bindings (but not the go bindings).
---
 bindings/python/notmuch/globals.py |  4 ++--
 bindings/python/notmuch/query.py   | 18 +++++++++++++-----
 bindings/ruby/query.c              | 26 ++++++++++++++++----------
 lib/Makefile.local                 |  4 ++--
 lib/database.cc                    |  8 +++++++-
 lib/notmuch.h                      | 26 +++++++++++++++++++-------
 lib/query.cc                       | 24 +++++++++++++-----------
 notmuch-count.c                    | 14 ++++++++++++--
 notmuch-reply.c                    |  8 +++++++-
 notmuch-search.c                   | 14 ++++++++++++--
 notmuch-show.c                     |  8 +++++++-
 11 files changed, 110 insertions(+), 44 deletions(-)

diff --git a/bindings/python/notmuch/globals.py b/bindings/python/notmuch/globals.py
index 24b25d3..85aab39 100644
--- a/bindings/python/notmuch/globals.py
+++ b/bindings/python/notmuch/globals.py
@@ -24,9 +24,9 @@ from ctypes import CDLL, Structure, POINTER
 try:
     from os import uname
     if uname()[0] == 'Darwin':
-        nmlib = CDLL("libnotmuch.4.dylib")
+        nmlib = CDLL("libnotmuch.5.dylib")
     else:
-        nmlib = CDLL("libnotmuch.so.4")
+        nmlib = CDLL("libnotmuch.so.5")
 except:
     raise ImportError("Could not find shared 'notmuch' library.")
 
diff --git a/bindings/python/notmuch/query.py b/bindings/python/notmuch/query.py
index 94773ac..96fc1cd 100644
--- a/bindings/python/notmuch/query.py
+++ b/bindings/python/notmuch/query.py
@@ -17,7 +17,7 @@ along with notmuch.  If not, see <http://www.gnu.org/licenses/>.
 Copyright 2010 Sebastian Spaeth <Sebastian@SSpaeth.de>
 """
 
-from ctypes import c_char_p, c_uint
+from ctypes import c_char_p, c_uint, POINTER, byref
 from .globals import (
     nmlib,
     Enum,
@@ -179,7 +179,7 @@ class Query(object):
         return Messages(msgs_p, self)
 
     _count_messages = nmlib.notmuch_query_count_messages
-    _count_messages.argtypes = [NotmuchQueryP]
+    _count_messages.argtypes = [NotmuchQueryP, POINTER(c_uint)]
     _count_messages.restype = c_uint
 
     def count_messages(self):
@@ -191,10 +191,14 @@ class Query(object):
         :rtype:   int
         '''
         self._assert_query_is_initialized()
-        return Query._count_messages(self._query)
+        count = c_uint(0)
+        status = Query._count_messages(self._query, byref(count))
+        if status != 0:
+            raise NotmuchError(status)
+        return count.value
 
     _count_threads = nmlib.notmuch_query_count_threads
-    _count_threads.argtypes = [NotmuchQueryP]
+    _count_threads.argtypes = [NotmuchQueryP, POINTER(c_uint)]
     _count_threads.restype = c_uint
 
     def count_threads(self):
@@ -210,7 +214,11 @@ class Query(object):
         :rtype:   int
         '''
         self._assert_query_is_initialized()
-        return Query._count_threads(self._query)
+        count = c_uint(0)
+        status = Query._count_threads(self._query, byref(count))
+        if status != 0:
+            raise NotmuchError(status)
+        return count.value
 
     _destroy = nmlib.notmuch_query_destroy
     _destroy.argtypes = [NotmuchQueryP]
diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index a7dacba..d39b6bc 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -173,14 +173,17 @@ VALUE
 notmuch_rb_query_count_messages (VALUE self)
 {
     notmuch_query_t *query;
+    notmuch_status_t status;
+    unsigned count;
 
     Data_Get_Notmuch_Query (self, query);
 
-    /* Xapian exceptions are not handled properly.
-     * (function may return 0 after printing a message)
-     * Thus there is nothing we can do here...
-     */
-    return UINT2NUM(notmuch_query_count_messages(query));
+    status = notmuch_query_count_messages(query, &count);
+
+    if (status)
+	notmuch_rb_status_raise (status);
+    else
+	return UINT2NUM(count);
 }
 
 /*
@@ -192,12 +195,15 @@ VALUE
 notmuch_rb_query_count_threads (VALUE self)
 {
     notmuch_query_t *query;
+    notmuch_status_t status;
+    unsigned count;
 
     Data_Get_Notmuch_Query (self, query);
 
-    /* Xapian exceptions are not handled properly.
-     * (function may return 0 after printing a message)
-     * Thus there is nothing we can do here...
-     */
-    return UINT2NUM(notmuch_query_count_threads(query));
+    status = notmuch_query_count_threads(query, &count);
+    if (status)
+	notmuch_rb_status_raise (status);
+    else
+	return UINT2NUM(count);
+
 }
diff --git a/lib/Makefile.local b/lib/Makefile.local
index 4120390..4bf8116 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -5,13 +5,13 @@
 # the library interface, (such as the deletion of an API or a major
 # semantic change that breaks formerly functioning code).
 #
-LIBNOTMUCH_VERSION_MAJOR = 4
+LIBNOTMUCH_VERSION_MAJOR = 5
 
 # The minor version of the library interface. This should be incremented at
 # the time of release for any additions to the library interface,
 # (and when it is incremented, the release version of the library should
 #  be reset to 0).
-LIBNOTMUCH_VERSION_MINOR = 1
+LIBNOTMUCH_VERSION_MINOR = 0
 
 # The release version the library interface. This should be incremented at
 # the time of release if there have been no changes to the interface, (but
diff --git a/lib/database.cc b/lib/database.cc
index 3601f9d..b7fbc63 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1268,7 +1268,13 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
     if (new_features &
 	(NOTMUCH_FEATURE_FILE_TERMS | NOTMUCH_FEATURE_BOOL_FOLDER)) {
 	notmuch_query_t *query = notmuch_query_create (notmuch, "");
-	total += notmuch_query_count_messages (query);
+	unsigned msg_count;
+
+	status = notmuch_query_count_messages (query, &msg_count);
+	if (status)
+	    goto DONE;
+
+	total += msg_count;
 	notmuch_query_destroy (query);
     }
     if (new_features & NOTMUCH_FEATURE_DIRECTORY_DOCS) {
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 220839b..37bf0bd 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -898,11 +898,15 @@ notmuch_threads_destroy (notmuch_threads_t *threads);
  * This function performs a search and returns Xapian's best
  * guess as to number of matching messages.
  *
- * If a Xapian exception occurs, this function may return 0 (after
- * printing a message).
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: query complete successfully.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The
+ *      value of *count is not defined.
  */
-unsigned
-notmuch_query_count_messages (notmuch_query_t *query);
+notmuch_status_t
+notmuch_query_count_messages (notmuch_query_t *query, unsigned *count);
 
 /**
  * Return the number of threads matching a search.
@@ -914,10 +918,18 @@ notmuch_query_count_messages (notmuch_query_t *query);
  * Note that this is a significantly heavier operation than
  * notmuch_query_count_messages().
  *
- * If an error occurs, this function may return 0.
+ * Return value:
+ *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Memory allocation failed. The value
+ *      of *count is not defined
+
+ * NOTMUCH_STATUS_SUCCESS: query complete successfully.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The
+ *      value of *count is not defined.
  */
-unsigned
-notmuch_query_count_threads (notmuch_query_t *query);
+notmuch_status_t
+notmuch_query_count_threads (notmuch_query_t *query, unsigned *count);
 
 /**
  * Get the thread ID of 'thread'.
diff --git a/lib/query.cc b/lib/query.cc
index 60ff8bd..10ecf2e 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -508,8 +508,8 @@ notmuch_threads_destroy (notmuch_threads_t *threads)
     talloc_free (threads);
 }
 
-unsigned
-notmuch_query_count_messages (notmuch_query_t *query)
+notmuch_status_t
+notmuch_query_count_messages (notmuch_query_t *query, unsigned *count_out)
 {
     notmuch_database_t *notmuch = query->notmuch;
     const char *query_string = query->query_string;
@@ -565,30 +565,32 @@ notmuch_query_count_messages (notmuch_query_t *query)
 	fprintf (stderr, "A Xapian exception occurred: %s\n",
 		 error.get_msg().c_str());
 	fprintf (stderr, "Query string was: %s\n", query->query_string);
+	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
 
-    return count;
+    *count_out=count;
+    return NOTMUCH_STATUS_SUCCESS;
 }
 
-unsigned
-notmuch_query_count_threads (notmuch_query_t *query)
+notmuch_status_t
+notmuch_query_count_threads (notmuch_query_t *query, unsigned *count)
 {
     notmuch_messages_t *messages;
     GHashTable *hash;
-    unsigned int count;
     notmuch_sort_t sort;
+    notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
 
     sort = query->sort;
     query->sort = NOTMUCH_SORT_UNSORTED;
     messages = notmuch_query_search_messages (query);
     query->sort = sort;
     if (messages == NULL)
-	return 0;
+	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 
     hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
     if (hash == NULL) {
 	talloc_free (messages);
-	return 0;
+	return NOTMUCH_STATUS_OUT_OF_MEMORY;
     }
 
     while (notmuch_messages_valid (messages)) {
@@ -597,7 +599,7 @@ notmuch_query_count_threads (notmuch_query_t *query)
 	char *thread_id_copy = talloc_strdup (messages, thread_id);
 	if (unlikely (thread_id_copy == NULL)) {
 	    notmuch_message_destroy (message);
-	    count = 0;
+	    ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	    goto DONE;
 	}
 	g_hash_table_insert (hash, thread_id_copy, NULL);
@@ -605,11 +607,11 @@ notmuch_query_count_threads (notmuch_query_t *query)
 	notmuch_messages_move_to_next (messages);
     }
 
-    count = g_hash_table_size (hash);
+    *count = g_hash_table_size (hash);
 
   DONE:
     g_hash_table_unref (hash);
     talloc_free (messages);
 
-    return count;
+    return ret;
 }
diff --git a/notmuch-count.c b/notmuch-count.c
index 6058f7c..7e3c1b7 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -71,6 +71,8 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 {
     notmuch_query_t *query;
     size_t i;
+    unsigned count;
+    notmuch_status_t status;
 
     query = notmuch_query_create (notmuch, query_str);
     if (query == NULL) {
@@ -83,10 +85,18 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 
     switch (output) {
     case OUTPUT_MESSAGES:
-	printf ("%u\n", notmuch_query_count_messages (query));
+	if ((status = notmuch_query_count_messages (query, &count))) {
+	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
+	    return 1;
+	}
+	printf ("%u\n", count);
 	break;
     case OUTPUT_THREADS:
-	printf ("%u\n", notmuch_query_count_threads (query));
+	if ((status = notmuch_query_count_threads (query, &count))) {
+	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
+	    return 1;
+	}
+	printf ("%u\n", count);
 	break;
     case OUTPUT_FILES:
 	printf ("%u\n", count_files (query));
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 7c1c809..c5f1b92 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -650,8 +650,14 @@ notmuch_reply_format_sprinter(void *ctx,
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     mime_node_t *node;
+    unsigned count;
+    notmuch_status_t status;
 
-    if (notmuch_query_count_messages (query) != 1) {
+    if ((status = notmuch_query_count_messages (query, &count))) {
+	fprintf (stderr, "Error: %s.\n", notmuch_status_to_string (status));
+	return 1;
+    }
+    if (count != 1) {
 	fprintf (stderr, "Error: search term did not match precisely one message.\n");
 	return 1;
     }
diff --git a/notmuch-search.c b/notmuch-search.c
index 14b9f01..cadfc0c 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -113,7 +113,14 @@ do_search_threads (search_context_t *ctx)
     int i;
 
     if (ctx->offset < 0) {
-	ctx->offset += notmuch_query_count_threads (ctx->query);
+	unsigned count;
+	notmuch_status_t status;
+	if ((status = notmuch_query_count_threads (ctx->query, &count))) {
+	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
+	    return 1;
+	}
+
+	ctx->offset += count;
 	if (ctx->offset < 0)
 	    ctx->offset = 0;
     }
@@ -414,7 +421,10 @@ do_search_messages (search_context_t *ctx)
     int i;
 
     if (ctx->offset < 0) {
-	ctx->offset += notmuch_query_count_messages (ctx->query);
+	unsigned count;
+	if (notmuch_query_count_messages (ctx->query, &count))
+	    return 1;
+	ctx->offset += count;
 	if (ctx->offset < 0)
 	    ctx->offset = 0;
     }
diff --git a/notmuch-show.c b/notmuch-show.c
index d416fbd..16835a3 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -982,8 +982,14 @@ do_show_single (void *ctx,
 {
     notmuch_messages_t *messages;
     notmuch_message_t *message;
+    unsigned count;
+    notmuch_status_t status;
+    if ((status = notmuch_query_count_messages (query, &count))) {
+	fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
+	return 1;
+    }
 
-    if (notmuch_query_count_messages (query) != 1) {
+    if (count != 1) {
 	fprintf (stderr, "Error: search term did not match precisely one message.\n");
 	return 1;
     }
-- 
2.1.3

update count API, round 3

[David Bremner]

This obsoletes the remainder of

     id:1419971380-10307-1-git-send-email-david@tethera.net

In the version, I only deprecate the old versions of count_messages
and count threads. This allows breaking the patch up into a series;
hopefully it's easier to review this way.

Patch 3/4 depends on id:1425679073-30439-1-git-send-email-david@tethera.net

I haven't updated the go bindings in this series, because a similar
patch to integrate go into the build/test procecess doesn't exist yet.

[PATCH 1/4] lib: add versions of notmuch_query_count_{message,threads} with status return

[David Bremner]

Although I think it's a pretty bad idea to continue using the old API,
this allows both a more gentle transition for clients of the library,
and allows us to break one monolithic change into a series
---
 lib/database.cc |  8 +++++++-
 lib/notmuch.h   | 34 ++++++++++++++++++++++++++++++----
 lib/query.cc    | 36 +++++++++++++++++++++++++++++-------
 3 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3974e2e..d92eec0 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1275,7 +1275,13 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
     if (new_features &
 	(NOTMUCH_FEATURE_FILE_TERMS | NOTMUCH_FEATURE_BOOL_FOLDER)) {
 	notmuch_query_t *query = notmuch_query_create (notmuch, "");
-	total += notmuch_query_count_messages (query);
+	unsigned msg_count;
+
+	status = notmuch_query_count_messages_st (query, &msg_count);
+	if (status)
+	    goto DONE;
+
+	total += msg_count;
 	notmuch_query_destroy (query);
     }
     if (new_features & NOTMUCH_FEATURE_DIRECTORY_DOCS) {
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 3e326bf..a0fc276 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -914,12 +914,23 @@ notmuch_threads_destroy (notmuch_threads_t *threads);
  * This function performs a search and returns Xapian's best
  * guess as to number of matching messages.
  *
- * If a Xapian exception occurs, this function may return 0 (after
- * printing a message).
+ * The version returning the count directly instead of a status value 
+ * is deprecated.
+ *
+ * Return value (of the _st) version:
+ *
+ * NOTMUCH_STATUS_SUCCESS: query complete successfully.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The
+ *      value of *count is not defined.
  */
+
 unsigned
 notmuch_query_count_messages (notmuch_query_t *query);
 
+notmuch_status_t
+notmuch_query_count_messages_st (notmuch_query_t *query, unsigned *count);
+
 /**
  * Return the number of threads matching a search.
  *
@@ -928,13 +939,28 @@ notmuch_query_count_messages (notmuch_query_t *query);
  * search.
  *
  * Note that this is a significantly heavier operation than
- * notmuch_query_count_messages().
+ * notmuch_query_count_messages{_st}().
+ *
+ * The version returning the count directly instead of a status value 
+ * is deprecated.
  *
- * If an error occurs, this function may return 0.
+ * Return values (of the _st version):
+ *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Memory allocation failed. The value
+ *      of *count is not defined
+
+ * NOTMUCH_STATUS_SUCCESS: query complete successfully.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The
+ *      value of *count is not defined.
  */
+
 unsigned
 notmuch_query_count_threads (notmuch_query_t *query);
 
+notmuch_status_t
+notmuch_query_count_threads_st (notmuch_query_t *query, unsigned *count);
+
 /**
  * Get the thread ID of 'thread'.
  *
diff --git a/lib/query.cc b/lib/query.cc
index 9279915..883d128 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -541,6 +541,16 @@ notmuch_threads_destroy (notmuch_threads_t *threads)
 unsigned
 notmuch_query_count_messages (notmuch_query_t *query)
 {
+    notmuch_status_t status;
+    unsigned count;
+
+    status = notmuch_query_count_messages_st (query, &count);
+    return status ? 0 : count;
+}
+
+notmuch_status_t
+notmuch_query_count_messages_st (notmuch_query_t *query, unsigned *count_out)
+{
     notmuch_database_t *notmuch = query->notmuch;
     const char *query_string = query->query_string;
     Xapian::doccount count = 0;
@@ -595,30 +605,42 @@ notmuch_query_count_messages (notmuch_query_t *query)
 	fprintf (stderr, "A Xapian exception occurred: %s\n",
 		 error.get_msg().c_str());
 	fprintf (stderr, "Query string was: %s\n", query->query_string);
+	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
 
-    return count;
+    *count_out=count;
+    return NOTMUCH_STATUS_SUCCESS;
 }
 
 unsigned
 notmuch_query_count_threads (notmuch_query_t *query)
 {
+    notmuch_status_t status;
+    unsigned count;
+
+    status = notmuch_query_count_threads_st (query, &count);
+    return status ? 0 : count;
+}
+    
+notmuch_status_t
+notmuch_query_count_threads_st (notmuch_query_t *query, unsigned *count)
+{
     notmuch_messages_t *messages;
     GHashTable *hash;
-    unsigned int count;
     notmuch_sort_t sort;
+    notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
 
     sort = query->sort;
     query->sort = NOTMUCH_SORT_UNSORTED;
     messages = notmuch_query_search_messages (query);
     query->sort = sort;
     if (messages == NULL)
-	return 0;
+	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 
     hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
     if (hash == NULL) {
 	talloc_free (messages);
-	return 0;
+	return NOTMUCH_STATUS_OUT_OF_MEMORY;
     }
 
     while (notmuch_messages_valid (messages)) {
@@ -627,7 +649,7 @@ notmuch_query_count_threads (notmuch_query_t *query)
 	char *thread_id_copy = talloc_strdup (messages, thread_id);
 	if (unlikely (thread_id_copy == NULL)) {
 	    notmuch_message_destroy (message);
-	    count = 0;
+	    ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	    goto DONE;
 	}
 	g_hash_table_insert (hash, thread_id_copy, NULL);
@@ -635,11 +657,11 @@ notmuch_query_count_threads (notmuch_query_t *query)
 	notmuch_messages_move_to_next (messages);
     }
 
-    count = g_hash_table_size (hash);
+    *count = g_hash_table_size (hash);
 
   DONE:
     g_hash_table_unref (hash);
     talloc_free (messages);
 
-    return count;
+    return ret;
 }
-- 
2.1.4

[PATCH 2/4] cli: update to use new count API

[David Bremner]

Essentially replace each call to notmuch_count_* with an if block.
---
 notmuch-count.c  | 14 ++++++++++++--
 notmuch-reply.c  |  8 +++++++-
 notmuch-search.c | 14 ++++++++++++--
 notmuch-show.c   |  8 +++++++-
 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index 6058f7c..cd6b0c9 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -71,6 +71,8 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 {
     notmuch_query_t *query;
     size_t i;
+    unsigned count;
+    notmuch_status_t status;
 
     query = notmuch_query_create (notmuch, query_str);
     if (query == NULL) {
@@ -83,10 +85,18 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 
     switch (output) {
     case OUTPUT_MESSAGES:
-	printf ("%u\n", notmuch_query_count_messages (query));
+	if ((status = notmuch_query_count_messages_st (query, &count))) {
+	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
+	    return 1;
+	}
+	printf ("%u\n", count);
 	break;
     case OUTPUT_THREADS:
-	printf ("%u\n", notmuch_query_count_threads (query));
+	if ((status = notmuch_query_count_threads_st (query, &count))) {
+	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
+	    return 1;
+	}
+	printf ("%u\n", count);
 	break;
     case OUTPUT_FILES:
 	printf ("%u\n", count_files (query));
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 7c1c809..0b3e5fd 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -650,8 +650,14 @@ notmuch_reply_format_sprinter(void *ctx,
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     mime_node_t *node;
+    unsigned count;
+    notmuch_status_t status;
 
-    if (notmuch_query_count_messages (query) != 1) {
+    if ((status = notmuch_query_count_messages_st (query, &count))) {
+	fprintf (stderr, "Error: %s.\n", notmuch_status_to_string (status));
+	return 1;
+    }
+    if (count != 1) {
 	fprintf (stderr, "Error: search term did not match precisely one message.\n");
 	return 1;
     }
diff --git a/notmuch-search.c b/notmuch-search.c
index a591d45..ba631c0 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -113,7 +113,14 @@ do_search_threads (search_context_t *ctx)
     int i;
 
     if (ctx->offset < 0) {
-	ctx->offset += notmuch_query_count_threads (ctx->query);
+	unsigned count;
+	notmuch_status_t status;
+	if ((status = notmuch_query_count_threads_st (ctx->query, &count))) {
+	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
+	    return 1;
+	}
+
+	ctx->offset += count;
 	if (ctx->offset < 0)
 	    ctx->offset = 0;
     }
@@ -414,7 +421,10 @@ do_search_messages (search_context_t *ctx)
     int i;
 
     if (ctx->offset < 0) {
-	ctx->offset += notmuch_query_count_messages (ctx->query);
+	unsigned count;
+	if (notmuch_query_count_messages_st (ctx->query, &count))
+	    return 1;
+	ctx->offset += count;
 	if (ctx->offset < 0)
 	    ctx->offset = 0;
     }
diff --git a/notmuch-show.c b/notmuch-show.c
index d416fbd..749b1af 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -982,8 +982,14 @@ do_show_single (void *ctx,
 {
     notmuch_messages_t *messages;
     notmuch_message_t *message;
+    unsigned count;
+    notmuch_status_t status;
+    if ((status = notmuch_query_count_messages_st (query, &count))) {
+	fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
+	return 1;
+    }
 
-    if (notmuch_query_count_messages (query) != 1) {
+    if (count != 1) {
 	fprintf (stderr, "Error: search term did not match precisely one message.\n");
 	return 1;
     }
-- 
2.1.4

[PATCH 3/4] ruby: update bindings for new count API

[David Bremner]

Compared to the CLI changes, this does something clearly useful,
transforming an error status into a ruby exception.
---
 bindings/ruby/query.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index a7dacba..ea9ca37 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -173,14 +173,17 @@ VALUE
 notmuch_rb_query_count_messages (VALUE self)
 {
     notmuch_query_t *query;
+    notmuch_status_t status;
+    unsigned count;
 
     Data_Get_Notmuch_Query (self, query);
 
-    /* Xapian exceptions are not handled properly.
-     * (function may return 0 after printing a message)
-     * Thus there is nothing we can do here...
-     */
-    return UINT2NUM(notmuch_query_count_messages(query));
+    status = notmuch_query_count_messages_st (query, &count);
+
+    if (status)
+	notmuch_rb_status_raise (status);
+    else
+	return UINT2NUM(count);
 }
 
 /*
@@ -192,12 +195,15 @@ VALUE
 notmuch_rb_query_count_threads (VALUE self)
 {
     notmuch_query_t *query;
+    notmuch_status_t status;
+    unsigned count;
 
     Data_Get_Notmuch_Query (self, query);
 
-    /* Xapian exceptions are not handled properly.
-     * (function may return 0 after printing a message)
-     * Thus there is nothing we can do here...
-     */
-    return UINT2NUM(notmuch_query_count_threads(query));
+    status = notmuch_query_count_threads_st (query, &count);
+    if (status)
+	notmuch_rb_status_raise (status);
+    else
+	return UINT2NUM(count);
+
 }
-- 
2.1.4

[PATCH 4/4] python: update bindings for new count API

[David Bremner]

Note that any mismatches are not detected until runtime (if at all)
with the python bindings, so tests are crucial
---
 bindings/python/notmuch/query.py | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/bindings/python/notmuch/query.py b/bindings/python/notmuch/query.py
index 94773ac..378aa47 100644
--- a/bindings/python/notmuch/query.py
+++ b/bindings/python/notmuch/query.py
@@ -17,7 +17,7 @@ along with notmuch.  If not, see <http://www.gnu.org/licenses/>.
 Copyright 2010 Sebastian Spaeth <Sebastian@SSpaeth.de>
 """
 
-from ctypes import c_char_p, c_uint
+from ctypes import c_char_p, c_uint, POINTER, byref
 from .globals import (
     nmlib,
     Enum,
@@ -178,8 +178,8 @@ class Query(object):
             raise NullPointerError
         return Messages(msgs_p, self)
 
-    _count_messages = nmlib.notmuch_query_count_messages
-    _count_messages.argtypes = [NotmuchQueryP]
+    _count_messages = nmlib.notmuch_query_count_messages_st
+    _count_messages.argtypes = [NotmuchQueryP, POINTER(c_uint)]
     _count_messages.restype = c_uint
 
     def count_messages(self):
@@ -191,10 +191,14 @@ class Query(object):
         :rtype:   int
         '''
         self._assert_query_is_initialized()
-        return Query._count_messages(self._query)
-
-    _count_threads = nmlib.notmuch_query_count_threads
-    _count_threads.argtypes = [NotmuchQueryP]
+        count = c_uint(0)
+        status = Query._count_messages(self._query, byref(count))
+        if status != 0:
+            raise NotmuchError(status)
+        return count.value
+
+    _count_threads = nmlib.notmuch_query_count_threads_st
+    _count_threads.argtypes = [NotmuchQueryP, POINTER(c_uint)]
     _count_threads.restype = c_uint
 
     def count_threads(self):
@@ -210,7 +214,11 @@ class Query(object):
         :rtype:   int
         '''
         self._assert_query_is_initialized()
-        return Query._count_threads(self._query)
+        count = c_uint(0)
+        status = Query._count_threads(self._query, byref(count))
+        if status != 0:
+            raise NotmuchError(status)
+        return count.value
 
     _destroy = nmlib.notmuch_query_destroy
     _destroy.argtypes = [NotmuchQueryP]
-- 
2.1.4

Re: [PATCH 1/4] lib: add versions of notmuch_query_count_{message, threads} with status return

[Jani Nikula]

On Sat, 07 Mar 2015, David Bremner <david@tethera.net> wrote:
> Although I think it's a pretty bad idea to continue using the old API,
> this allows both a more gentle transition for clients of the library,
> and allows us to break one monolithic change into a series

We should probably bump LIBNOTMUCH_MINOR_VERSION for this.

See nitpicks inline below, up to you if you decide if they're worthwhile
changes, generally LGTM.

> ---
>  lib/database.cc |  8 +++++++-
>  lib/notmuch.h   | 34 ++++++++++++++++++++++++++++++----
>  lib/query.cc    | 36 +++++++++++++++++++++++++++++-------
>  3 files changed, 66 insertions(+), 12 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 3974e2e..d92eec0 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1275,7 +1275,13 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
>      if (new_features &
>  	(NOTMUCH_FEATURE_FILE_TERMS | NOTMUCH_FEATURE_BOOL_FOLDER)) {
>  	notmuch_query_t *query = notmuch_query_create (notmuch, "");
> -	total += notmuch_query_count_messages (query);
> +	unsigned msg_count;

Personal preference, I always want to specify "unsigned int" instead of
just "unsigned". Same all around patches 1 and 2.

> +
> +	status = notmuch_query_count_messages_st (query, &msg_count);
> +	if (status)
> +	    goto DONE;
> +
> +	total += msg_count;
>  	notmuch_query_destroy (query);
>      }
>      if (new_features & NOTMUCH_FEATURE_DIRECTORY_DOCS) {
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 3e326bf..a0fc276 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -914,12 +914,23 @@ notmuch_threads_destroy (notmuch_threads_t *threads);
>   * This function performs a search and returns Xapian's best
>   * guess as to number of matching messages.
>   *
> - * If a Xapian exception occurs, this function may return 0 (after
> - * printing a message).
> + * The version returning the count directly instead of a status value 
> + * is deprecated.
> + *
> + * Return value (of the _st) version:
> + *
> + * NOTMUCH_STATUS_SUCCESS: query complete successfully.
> + *
> + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The
> + *      value of *count is not defined.
>   */

I'd really like to have separate doc comments for both the
functions. The original and now deprecated version can be be a brief
one, with a reference to the _st version documentation. If you move the
_st version declaration here, above the original declaration, the diff
should remain small.

Perhaps reference the library version number when this was added?

> +
>  unsigned
>  notmuch_query_count_messages (notmuch_query_t *query);
>  
> +notmuch_status_t
> +notmuch_query_count_messages_st (notmuch_query_t *query, unsigned *count);
> +
>  /**
>   * Return the number of threads matching a search.
>   *
> @@ -928,13 +939,28 @@ notmuch_query_count_messages (notmuch_query_t *query);
>   * search.
>   *
>   * Note that this is a significantly heavier operation than
> - * notmuch_query_count_messages().
> + * notmuch_query_count_messages{_st}().
> + *
> + * The version returning the count directly instead of a status value 
> + * is deprecated.
>   *
> - * If an error occurs, this function may return 0.
> + * Return values (of the _st version):
> + *
> + * NOTMUCH_STATUS_OUT_OF_MEMORY: Memory allocation failed. The value
> + *      of *count is not defined
> +
> + * NOTMUCH_STATUS_SUCCESS: query complete successfully.
> + *
> + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The
> + *      value of *count is not defined.
>   */

Same as above.

> +
>  unsigned
>  notmuch_query_count_threads (notmuch_query_t *query);
>  
> +notmuch_status_t
> +notmuch_query_count_threads_st (notmuch_query_t *query, unsigned *count);
> +
>  /**
>   * Get the thread ID of 'thread'.
>   *
> diff --git a/lib/query.cc b/lib/query.cc
> index 9279915..883d128 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -541,6 +541,16 @@ notmuch_threads_destroy (notmuch_threads_t *threads)
>  unsigned
>  notmuch_query_count_messages (notmuch_query_t *query)
>  {
> +    notmuch_status_t status;
> +    unsigned count;
> +
> +    status = notmuch_query_count_messages_st (query, &count);
> +    return status ? 0 : count;
> +}
> +
> +notmuch_status_t
> +notmuch_query_count_messages_st (notmuch_query_t *query, unsigned *count_out)
> +{
>      notmuch_database_t *notmuch = query->notmuch;
>      const char *query_string = query->query_string;
>      Xapian::doccount count = 0;
> @@ -595,30 +605,42 @@ notmuch_query_count_messages (notmuch_query_t *query)
>  	fprintf (stderr, "A Xapian exception occurred: %s\n",
>  		 error.get_msg().c_str());
>  	fprintf (stderr, "Query string was: %s\n", query->query_string);
> +	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
>      }
>  
> -    return count;
> +    *count_out=count;

Spaces around "="?

> +    return NOTMUCH_STATUS_SUCCESS;
>  }
>  
>  unsigned
>  notmuch_query_count_threads (notmuch_query_t *query)
>  {
> +    notmuch_status_t status;
> +    unsigned count;
> +
> +    status = notmuch_query_count_threads_st (query, &count);
> +    return status ? 0 : count;
> +}
> +    
> +notmuch_status_t
> +notmuch_query_count_threads_st (notmuch_query_t *query, unsigned *count)
> +{
>      notmuch_messages_t *messages;
>      GHashTable *hash;
> -    unsigned int count;
>      notmuch_sort_t sort;
> +    notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
>  
>      sort = query->sort;
>      query->sort = NOTMUCH_SORT_UNSORTED;
>      messages = notmuch_query_search_messages (query);
>      query->sort = sort;
>      if (messages == NULL)
> -	return 0;
> +	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
>  
>      hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
>      if (hash == NULL) {
>  	talloc_free (messages);
> -	return 0;
> +	return NOTMUCH_STATUS_OUT_OF_MEMORY;
>      }
>  
>      while (notmuch_messages_valid (messages)) {
> @@ -627,7 +649,7 @@ notmuch_query_count_threads (notmuch_query_t *query)
>  	char *thread_id_copy = talloc_strdup (messages, thread_id);
>  	if (unlikely (thread_id_copy == NULL)) {
>  	    notmuch_message_destroy (message);
> -	    count = 0;
> +	    ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
>  	    goto DONE;
>  	}
>  	g_hash_table_insert (hash, thread_id_copy, NULL);
> @@ -635,11 +657,11 @@ notmuch_query_count_threads (notmuch_query_t *query)
>  	notmuch_messages_move_to_next (messages);
>      }
>  
> -    count = g_hash_table_size (hash);
> +    *count = g_hash_table_size (hash);
>  
>    DONE:
>      g_hash_table_unref (hash);
>      talloc_free (messages);
>  
> -    return count;
> +    return ret;
>  }
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/4] cli: update to use new count API

[Jani Nikula]

On Sat, 07 Mar 2015, David Bremner <david@tethera.net> wrote:
> Essentially replace each call to notmuch_count_* with an if block.

LGTM, nitpicks inline.

> ---
>  notmuch-count.c  | 14 ++++++++++++--
>  notmuch-reply.c  |  8 +++++++-
>  notmuch-search.c | 14 ++++++++++++--
>  notmuch-show.c   |  8 +++++++-
>  4 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/notmuch-count.c b/notmuch-count.c
> index 6058f7c..cd6b0c9 100644
> --- a/notmuch-count.c
> +++ b/notmuch-count.c
> @@ -71,6 +71,8 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
>  {
>      notmuch_query_t *query;
>      size_t i;
> +    unsigned count;

As I said in patch 1, I'd personally prefer "unsigned int".

> +    notmuch_status_t status;
>  
>      query = notmuch_query_create (notmuch, query_str);
>      if (query == NULL) {
> @@ -83,10 +85,18 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
>  
>      switch (output) {
>      case OUTPUT_MESSAGES:
> -	printf ("%u\n", notmuch_query_count_messages (query));
> +	if ((status = notmuch_query_count_messages_st (query, &count))) {

I'd rather have the assignment and check in separate lines. Somehow I
feel it emphasizes the distinction between the happy day flow and the
unusual cases. Same for other hunks in this patch.

> +	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
> +	    return 1;
> +	}
> +	printf ("%u\n", count);
>  	break;
>      case OUTPUT_THREADS:
> -	printf ("%u\n", notmuch_query_count_threads (query));
> +	if ((status = notmuch_query_count_threads_st (query, &count))) {
> +	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
> +	    return 1;
> +	}
> +	printf ("%u\n", count);
>  	break;
>      case OUTPUT_FILES:
>  	printf ("%u\n", count_files (query));
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 7c1c809..0b3e5fd 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -650,8 +650,14 @@ notmuch_reply_format_sprinter(void *ctx,
>      notmuch_messages_t *messages;
>      notmuch_message_t *message;
>      mime_node_t *node;
> +    unsigned count;
> +    notmuch_status_t status;
>  
> -    if (notmuch_query_count_messages (query) != 1) {
> +    if ((status = notmuch_query_count_messages_st (query, &count))) {
> +	fprintf (stderr, "Error: %s.\n", notmuch_status_to_string (status));
> +	return 1;
> +    }
> +    if (count != 1) {
>  	fprintf (stderr, "Error: search term did not match precisely one message.\n");
>  	return 1;
>      }
> diff --git a/notmuch-search.c b/notmuch-search.c
> index a591d45..ba631c0 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -113,7 +113,14 @@ do_search_threads (search_context_t *ctx)
>      int i;
>  
>      if (ctx->offset < 0) {
> -	ctx->offset += notmuch_query_count_threads (ctx->query);
> +	unsigned count;
> +	notmuch_status_t status;
> +	if ((status = notmuch_query_count_threads_st (ctx->query, &count))) {
> +	    fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
> +	    return 1;
> +	}
> +
> +	ctx->offset += count;
>  	if (ctx->offset < 0)
>  	    ctx->offset = 0;
>      }
> @@ -414,7 +421,10 @@ do_search_messages (search_context_t *ctx)
>      int i;
>  
>      if (ctx->offset < 0) {
> -	ctx->offset += notmuch_query_count_messages (ctx->query);
> +	unsigned count;
> +	if (notmuch_query_count_messages_st (ctx->query, &count))
> +	    return 1;
> +	ctx->offset += count;
>  	if (ctx->offset < 0)
>  	    ctx->offset = 0;
>      }
> diff --git a/notmuch-show.c b/notmuch-show.c
> index d416fbd..749b1af 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -982,8 +982,14 @@ do_show_single (void *ctx,
>  {
>      notmuch_messages_t *messages;
>      notmuch_message_t *message;
> +    unsigned count;
> +    notmuch_status_t status;
> +    if ((status = notmuch_query_count_messages_st (query, &count))) {
> +	fprintf (stderr, "Error: %s\n", notmuch_status_to_string (status));
> +	return 1;
> +    }
>  
> -    if (notmuch_query_count_messages (query) != 1) {
> +    if (count != 1) {
>  	fprintf (stderr, "Error: search term did not match precisely one message.\n");
>  	return 1;
>      }
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch