From: Jani Nikula <jani@nikula.org>
To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org
Subject: Re: [PATCH 3/4] notmuch-restore: convert to notmuch-opts argument
	handling.
In-Reply-To: <1323013675-6929-4-git-send-email-david@tethera.net>
References: <1323013675-6929-1-git-send-email-david@tethera.net>
	<1323013675-6929-4-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.10.1+59~ga1814f2 (http://notmuchmail.org) Emacs/23.3.1
	(i686-pc-linux-gnu)
Date: Tue, 06 Dec 2011 22:48:50 +0200
Message-ID: <87ty5dxw7h.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: David Bremner <bremner@debian.org>
Precedence: list

On Sun,  4 Dec 2011 11:47:54 -0400, David Bremner <david@tethera.net> wrote:
> From: David Bremner <bremner@debian.org>
> 
> The new argument handling is a bit more concise, and bit more
> flexible.  It allows the input file name to go before the --accumulate
> option.
> ---
>  notmuch-restore.c |   38 ++++++++++++++++----------------------
>  1 files changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index 13b4325..d0aa7a8 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -18,9 +18,8 @@
>   * Author: Carl Worth <cworth@cworth.org>
>   */
>  
> -#include <getopt.h>
> -
>  #include "notmuch-client.h"
> +#include "notmuch-opts.h"

I guess this will eventually be included almost everywhere, so in that
sense could be put in notmuch-client.h too.

>  
>  int
>  notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
> @@ -29,12 +28,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
>      notmuch_database_t *notmuch;
>      notmuch_bool_t synchronize_flags;
>      notmuch_bool_t accumulate = FALSE;
> +    char *input_file_name = NULL;
>      FILE *input = stdin;
>      char *line = NULL;
>      size_t line_size;
>      ssize_t line_len;
>      regex_t regex;
>      int rerr;
> +    int opt_index;
>  
>      config = notmuch_config_open (ctx, NULL, NULL);
>      if (config == NULL)
> @@ -47,37 +48,30 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
>  
>      synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
>  
> -    struct option options[] = {
> -	{ "accumulate",   no_argument,       0, 'a' },
> -	{ 0, 0, 0, 0}
> +    notmuch_opt_desc_t options[] = {
> +	{ "in-file", 'i', NOTMUCH_OPT_POSITION, 0, &input_file_name },

Unless I'm mistaken in review of patch 1, specifying the input file
using --in-file=file doesn't work; it only works as a positional
argument.

> +	{ "accumulate", 'a',  NOTMUCH_OPT_BOOLEAN, 0, &accumulate },
> +	{ 0, 0, 0, 0, 0 }
>      };
>  
> -    int opt;
> -    do {
> -	opt = getopt_long (argc, argv, "", options, NULL);
> +    opt_index = notmuch_parse_args (argc, argv, options, 1);
>  
> -	switch (opt) {
> -	case 'a':
> -	    accumulate = 1;
> -	    break;
> -	case '?':
> -	    return 1;
> -	    break;
> -	}
> -
> -    } while (opt != -1);
> +    if (opt_index < 0) {
> +	/* diagnostics already printed */
> +	exit(1);

return 1 rather than exit(1)?

BR,
Jani.

> +    }
>  
> -    if (optind < argc) {
> -	input = fopen (argv[optind], "r");
> +    if (input_file_name) {
> +	input = fopen (input_file_name, "r");
>  	if (input == NULL) {
>  	    fprintf (stderr, "Error opening %s for reading: %s\n",
> -		     argv[optind], strerror (errno));
> +		     input_file_name, strerror (errno));
>  	    return 1;
>  	}
>  	optind++;
>      }
>  
> -    if (optind < argc) {
> +    if (opt_index < argc) {
>  	fprintf (stderr,
>  	 "Cannot read dump from more than one file: %s\n",
>  		 argv[optind]);
> -- 
> 1.7.7.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch