From: Mark Walters <markwalters1009@gmail.com>
To: David Bremner <david@tethera.net>, David Bremner <david@tethera.net>,
 notmuch@notmuchmail.org
Subject: Re: [Patch v2 4/4] cli: add standard option processing to config and
 setup
In-Reply-To: <1428322958-2887-5-git-send-email-david@tethera.net>
References: <87d23ixnr7.fsf@maritornes.cs.unb.ca>
 <1428322958-2887-1-git-send-email-david@tethera.net>
 <1428322958-2887-5-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/23.4.1
 (x86_64-pc-linux-gnu)
Date: Tue, 07 Apr 2015 08:24:43 +0100
Message-ID: <87vbh8qu2s.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


Hi

On Mon, 06 Apr 2015, David Bremner <david@tethera.net> wrote:
> In particular this fixes a recently encountered bug where the
> "--config" argument to "notmuch setup" is silently ignored, which the
> unpleasant consequence of overwriting the users config file.
> ---
>  notmuch-config.c     | 16 +++++++++++++++-
>  notmuch-setup.c      | 12 ++++++++++++
>  test/random-corpus.c |  9 +++++++++
>  3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/notmuch-config.c b/notmuch-config.c
> index 2d5c297..f2cd6a8 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -872,8 +872,22 @@ int
>  notmuch_config_command (notmuch_config_t *config, int argc, char *argv[])
>  {
>      int ret;
> +    int opt_index;
>  
> -    argc--; argv++; /* skip subcommand argument */
> +    notmuch_opt_desc_t options[] = {
> +	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
> +	{ 0, 0, 0, 0, 0 }
> +    };
> +
> +    opt_index = parse_arguments (argc, argv, options, 1);
> +    if (opt_index < 0)
> +	return EXIT_FAILURE;
> +
> +    notmuch_process_shared_options (argv[0]);
> +
> +    /* skip at least subcommand argument */
> +    argc-= opt_index;
> +    argv+= opt_index;
>  
>      if (argc < 1) {
>  	fprintf (stderr, "Error: notmuch config requires at least one argument.\n");
> diff --git a/notmuch-setup.c b/notmuch-setup.c
> index 36a6171..6a020dc 100644
> --- a/notmuch-setup.c
> +++ b/notmuch-setup.c
> @@ -133,6 +133,7 @@ notmuch_setup_command (notmuch_config_t *config,
>      size_t new_tags_len;
>      const char **search_exclude_tags;
>      size_t search_exclude_tags_len;
> +    int opt_index;
>  
>  #define prompt(format, ...)					\
>      do {							\
> @@ -145,6 +146,17 @@ notmuch_setup_command (notmuch_config_t *config,
>  	chomp_newline (response);				\
>      } while (0)
>  
> +    notmuch_opt_desc_t options[] = {
> +	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
> +	{ 0, 0, 0, 0, 0 }
> +    };
> +
> +    opt_index = parse_arguments (argc, argv, options, 1);
> +    if (opt_index < 0)
> +	return EXIT_FAILURE;
> +
> +    notmuch_process_shared_options ("setup");

Why "setup" here rather than argv[0] or similar? It seems inconsistent
with the other cases.

Best wishes

Mark



> +
>      if (notmuch_config_is_new (config))
>  	welcome_message_pre_setup ();
>  
> diff --git a/test/random-corpus.c b/test/random-corpus.c
> index 790193d..6c467bb 100644
> --- a/test/random-corpus.c
> +++ b/test/random-corpus.c
> @@ -114,6 +114,15 @@ random_utf8_string (void *ctx, size_t char_count)
>      return buf;
>  }
>  
> +/* stubs since we cannot link with notmuch.o */
> +const notmuch_opt_desc_t notmuch_shared_options[] = {
> +	{ 0, 0, 0, 0, 0 }
> +};
> +
> +void
> +notmuch_process_shared_options (unused (const char *dummy))
> +{
> +}
>  
>  int
>  main (int argc, char **argv)
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch