From: Jani Nikula <jani@nikula.org>
To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org
Subject: Re: [PATCH 2/4] cli: update to use new count API
In-Reply-To: <1425717797-19990-3-git-send-email-david@tethera.net>
References: <1419971380-10307-6-git-send-email-david@tethera.net>
	<1425717797-19990-1-git-send-email-david@tethera.net>
	<1425717797-19990-3-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.19+74~g6aeef2e (http://notmuchmail.org) Emacs/24.4.1
	(x86_64-pc-linux-gnu)
Date: Sat, 07 Mar 2015 16:00:53 +0200
Message-ID: <87sidgj4a2.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

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