From: Tomi Ollila <tomi.ollila@iki.fi>
To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH 3/5] cli: add --batch option to notmuch count
In-Reply-To:
 <9465478105bbda3d21921906b7abf24dbbaa7ec9.1358273133.git.jani@nikula.org>
References: <cover.1358273133.git.jani@nikula.org>
	<9465478105bbda3d21921906b7abf24dbbaa7ec9.1358273133.git.jani@nikula.org>
User-Agent: Notmuch/0.15+11~ge1e719d (http://notmuchmail.org) Emacs/24.2.1
	(x86_64-unknown-linux-gnu)
Date: Wed, 23 Jan 2013 15:36:45 +0200
Message-ID: <m2a9s0hvgy.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Tue, Jan 15 2013, Jani Nikula <jani@nikula.org> wrote:

> Add support for reading queries from stdin, one per line, and writing
> results to stdin, one per line.

Jani's parts LGTM, except one IMPORTANT part -- results are written
to STDOUT! (hopefully...yes) instead of STDIN! ;)

I don't see a reason why this particular way to get counts could
not be supported in notmuch into foreseeable future; the implementation
is simple and clean.

Next to look Mark's changes...

Tomi

>
> This will bring considerable performance improvements when utilized in
> Emacs notmuch-hello, especially so when running remote notmuch.
> ---
>  notmuch-count.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-count.c b/notmuch-count.c
> index 0e14b48..4bc4215 100644
> --- a/notmuch-count.c
> +++ b/notmuch-count.c
> @@ -62,6 +62,27 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
>      return 0;
>  }
>  
> +static int
> +count_file (notmuch_database_t *notmuch, FILE *input, const char **exclude_tags,
> +	    size_t exclude_tags_length, int output)
> +{
> +    char *line = NULL;
> +    ssize_t line_len;
> +    size_t line_size;
> +    int ret = 0;
> +
> +    while (!ret && (line_len = getline (&line, &line_size, input)) != -1) {
> +	chomp_newline (line);
> +	ret = print_count (notmuch, line, exclude_tags, exclude_tags_length,
> +			   output);
> +    }
> +
> +    if (line)
> +	free (line);
> +
> +    return ret;
> +}
> +
>  int
>  notmuch_count_command (void *ctx, int argc, char *argv[])
>  {
> @@ -73,6 +94,9 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
>      int exclude = EXCLUDE_TRUE;
>      const char **search_exclude_tags = NULL;
>      size_t search_exclude_tags_length = 0;
> +    notmuch_bool_t batch = FALSE;
> +    FILE *input = stdin;
> +    char *input_file_name = NULL;
>      int ret;
>  
>      notmuch_opt_desc_t options[] = {
> @@ -84,6 +108,8 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
>  	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
>  				  { "false", EXCLUDE_FALSE },
>  				  { 0, 0 } } },
> +	{ NOTMUCH_OPT_BOOLEAN, &batch, "batch", 0, 0 },
> +	{ NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -93,6 +119,21 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
>  	return 1;
>      }
>  
> +    if (input_file_name) {
> +	batch = TRUE;
> +	input = fopen (input_file_name, "r");
> +	if (input == NULL) {
> +	    fprintf (stderr, "Error opening %s for reading: %s\n",
> +		     input_file_name, strerror (errno));
> +	    return 1;
> +	}
> +    }
> +
> +    if (batch && opt_index != argc) {
> +	fprintf (stderr, "--batch and query string are not compatible\n");
> +	return 1;
> +    }
> +
>      config = notmuch_config_open (ctx, NULL, NULL);
>      if (config == NULL)
>  	return 1;
> @@ -112,10 +153,17 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
>  	    (config, &search_exclude_tags_length);
>      }
>  
> -    ret = print_count (notmuch, query_str, search_exclude_tags,
> -		       search_exclude_tags_length, output);
> +    if (batch)
> +	ret = count_file (notmuch, input, search_exclude_tags,
> +			  search_exclude_tags_length, output);
> +    else
> +	ret = print_count (notmuch, query_str, search_exclude_tags,
> +			   search_exclude_tags_length, output);
>  
>      notmuch_database_destroy (notmuch);
>  
> +    if (input != stdin)
> +	fclose (input);
> +
>      return ret;
>  }
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch