Re: [PATCH 1/2] cli/config: don't try to open config file for 'notmuch help'

2017-02-28 Thread David Bremner
Jani Nikula  writes:

> The help command does not really need to try to open the config
> file. So don't.

series pushed to master
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] cli/config: don't try to open config file for 'notmuch help'

2017-02-27 Thread Tomi Ollila
On Sun, Feb 26 2017, Jani Nikula  wrote:

> The help command does not really need to try to open the config
> file. So don't.
>
> ---

This series looks good, tests pass, repost of my patch seems to be
unmodified (the power of running 2 emacses w/ notmuch rules!)

In addition to `make test` I ran the following 3 commands:

  104  HOME=/tmp/xyxz ./notmuch help
  105  HOME=/tmp/xyxz ./notmuch nre
  106  HOME=/tmp/xyxz ./notmuch new
  107  HOME=/tmp/xyxz ./notmuch setup

and these worked as expected.

Tomi

>
> This will allow better error reporting such as
> id:1483570332-11820-1-git-send-email-tomi.oll...@iki.fi while still
> ensuring 'notmuch help' succeeds in the absence of config files.
> ---
>  notmuch-client.h |  7 ++-
>  notmuch-config.c | 12 
>  notmuch.c| 34 +-
>  3 files changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 21b087980a17..1099122426af 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -263,10 +263,15 @@ json_quote_str (const void *ctx, const char *str);
>  
>  /* notmuch-config.c */
>  
> +typedef enum {
> +NOTMUCH_CONFIG_OPEN  = 1 << 0,
> +NOTMUCH_CONFIG_CREATE = 1 << 1,
> +} notmuch_config_mode_t;
> +
>  notmuch_config_t *
>  notmuch_config_open (void *ctx,
>const char *filename,
> -  notmuch_bool_t create_new);
> +  notmuch_config_mode_t config_mode);
>  
>  void
>  notmuch_config_close (notmuch_config_t *config);
> diff --git a/notmuch-config.c b/notmuch-config.c
> index b202bb1e2299..959410cce7b8 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -322,7 +322,7 @@ out:
>  notmuch_config_t *
>  notmuch_config_open (void *ctx,
>const char *filename,
> -  notmuch_bool_t create_new)
> +  notmuch_config_mode_t config_mode)
>  {
>  GError *error = NULL;
>  size_t tmp;
> @@ -356,9 +356,13 @@ notmuch_config_open (void *ctx,
>  
>  config->key_file = g_key_file_new ();
>  
> -if (! get_config_from_file (config, create_new)) {
> - talloc_free (config);
> - return NULL;
> +if (config_mode & NOTMUCH_CONFIG_OPEN) {
> + notmuch_bool_t create_new = (config_mode & NOTMUCH_CONFIG_CREATE) != 0;
> +
> + if (! get_config_from_file (config, create_new)) {
> + talloc_free (config);
> + return NULL;
> + }
>  }
>  
>  /* Whenever we know of configuration sections that don't appear in
> diff --git a/notmuch.c b/notmuch.c
> index b9c320329dd5..8e332ce64410 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -33,7 +33,7 @@ typedef int (*command_function_t) (notmuch_config_t 
> *config, int argc, char *arg
>  typedef struct command {
>  const char *name;
>  command_function_t function;
> -notmuch_bool_t create_config;
> +notmuch_config_mode_t config_mode;
>  const char *summary;
>  } command_t;
>  
> @@ -97,35 +97,35 @@ int notmuch_minimal_options (const char *subcommand_name,
>  }
>  
>  static command_t commands[] = {
> -{ NULL, notmuch_command, TRUE,
> +{ NULL, notmuch_command, NOTMUCH_CONFIG_OPEN | NOTMUCH_CONFIG_CREATE,
>"Notmuch main command." },
> -{ "setup", notmuch_setup_command, TRUE,
> +{ "setup", notmuch_setup_command, NOTMUCH_CONFIG_OPEN | 
> NOTMUCH_CONFIG_CREATE,
>"Interactively set up notmuch for first use." },
> -{ "new", notmuch_new_command, FALSE,
> +{ "new", notmuch_new_command, NOTMUCH_CONFIG_OPEN,
>"Find and import new messages to the notmuch database." },
> -{ "insert", notmuch_insert_command, FALSE,
> +{ "insert", notmuch_insert_command, NOTMUCH_CONFIG_OPEN,
>"Add a new message into the maildir and notmuch database." },
> -{ "search", notmuch_search_command, FALSE,
> +{ "search", notmuch_search_command, NOTMUCH_CONFIG_OPEN,
>"Search for messages matching the given search terms." },
> -{ "address", notmuch_address_command, FALSE,
> +{ "address", notmuch_address_command, NOTMUCH_CONFIG_OPEN,
>"Get addresses from messages matching the given search terms." },
> -{ "show", notmuch_show_command, FALSE,
> +{ "show", notmuch_show_command, NOTMUCH_CONFIG_OPEN,
>"Show all messages matching the search terms." },
> -{ "count", notmuch_count_command, FALSE,
> +{ "count", notmuch_count_command, NOTMUCH_CONFIG_OPEN,
>"Count messages matching the search terms." },
> -{ "reply", notmuch_reply_command, FALSE,
> +{ "reply", notmuch_reply_command, NOTMUCH_CONFIG_OPEN,
>"Construct a reply template for a set of messages." },
> -{ "tag", notmuch_tag_command, FALSE,
> +{ "tag", notmuch_tag_command, NOTMUCH_CONFIG_OPEN,
>"Add/remove tags for all messages matching the search terms." },
> -{ "dump", notmuch_dump_command, FALSE,
> +{ "dump", notmuch_dump_command, NOTMUCH_CONFIG_OPEN,
>"Create a plain-text dump of the ta

Re: [PATCH 1/2] cli/config: don't try to open config file for 'notmuch help'

2017-02-26 Thread Tomi Ollila
On Sun, Feb 26 2017, Jani Nikula  wrote:

> The help command does not really need to try to open the config
> file. So don't.
>
> ---

I've forgotten this ... added notmuch::0.24 to this series and will
check carefully and test tomorrow...

Tomi

>
> This will allow better error reporting such as
> id:1483570332-11820-1-git-send-email-tomi.oll...@iki.fi while still
> ensuring 'notmuch help' succeeds in the absence of config files.
> ---
>  notmuch-client.h |  7 ++-
>  notmuch-config.c | 12 
>  notmuch.c| 34 +-
>  3 files changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 21b087980a17..1099122426af 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -263,10 +263,15 @@ json_quote_str (const void *ctx, const char *str);
>  
>  /* notmuch-config.c */
>  
> +typedef enum {
> +NOTMUCH_CONFIG_OPEN  = 1 << 0,
> +NOTMUCH_CONFIG_CREATE = 1 << 1,
> +} notmuch_config_mode_t;
> +
>  notmuch_config_t *
>  notmuch_config_open (void *ctx,
>const char *filename,
> -  notmuch_bool_t create_new);
> +  notmuch_config_mode_t config_mode);
>  
>  void
>  notmuch_config_close (notmuch_config_t *config);
> diff --git a/notmuch-config.c b/notmuch-config.c
> index b202bb1e2299..959410cce7b8 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -322,7 +322,7 @@ out:
>  notmuch_config_t *
>  notmuch_config_open (void *ctx,
>const char *filename,
> -  notmuch_bool_t create_new)
> +  notmuch_config_mode_t config_mode)
>  {
>  GError *error = NULL;
>  size_t tmp;
> @@ -356,9 +356,13 @@ notmuch_config_open (void *ctx,
>  
>  config->key_file = g_key_file_new ();
>  
> -if (! get_config_from_file (config, create_new)) {
> - talloc_free (config);
> - return NULL;
> +if (config_mode & NOTMUCH_CONFIG_OPEN) {
> + notmuch_bool_t create_new = (config_mode & NOTMUCH_CONFIG_CREATE) != 0;
> +
> + if (! get_config_from_file (config, create_new)) {
> + talloc_free (config);
> + return NULL;
> + }
>  }
>  
>  /* Whenever we know of configuration sections that don't appear in
> diff --git a/notmuch.c b/notmuch.c
> index b9c320329dd5..8e332ce64410 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -33,7 +33,7 @@ typedef int (*command_function_t) (notmuch_config_t 
> *config, int argc, char *arg
>  typedef struct command {
>  const char *name;
>  command_function_t function;
> -notmuch_bool_t create_config;
> +notmuch_config_mode_t config_mode;
>  const char *summary;
>  } command_t;
>  
> @@ -97,35 +97,35 @@ int notmuch_minimal_options (const char *subcommand_name,
>  }
>  
>  static command_t commands[] = {
> -{ NULL, notmuch_command, TRUE,
> +{ NULL, notmuch_command, NOTMUCH_CONFIG_OPEN | NOTMUCH_CONFIG_CREATE,
>"Notmuch main command." },
> -{ "setup", notmuch_setup_command, TRUE,
> +{ "setup", notmuch_setup_command, NOTMUCH_CONFIG_OPEN | 
> NOTMUCH_CONFIG_CREATE,
>"Interactively set up notmuch for first use." },
> -{ "new", notmuch_new_command, FALSE,
> +{ "new", notmuch_new_command, NOTMUCH_CONFIG_OPEN,
>"Find and import new messages to the notmuch database." },
> -{ "insert", notmuch_insert_command, FALSE,
> +{ "insert", notmuch_insert_command, NOTMUCH_CONFIG_OPEN,
>"Add a new message into the maildir and notmuch database." },
> -{ "search", notmuch_search_command, FALSE,
> +{ "search", notmuch_search_command, NOTMUCH_CONFIG_OPEN,
>"Search for messages matching the given search terms." },
> -{ "address", notmuch_address_command, FALSE,
> +{ "address", notmuch_address_command, NOTMUCH_CONFIG_OPEN,
>"Get addresses from messages matching the given search terms." },
> -{ "show", notmuch_show_command, FALSE,
> +{ "show", notmuch_show_command, NOTMUCH_CONFIG_OPEN,
>"Show all messages matching the search terms." },
> -{ "count", notmuch_count_command, FALSE,
> +{ "count", notmuch_count_command, NOTMUCH_CONFIG_OPEN,
>"Count messages matching the search terms." },
> -{ "reply", notmuch_reply_command, FALSE,
> +{ "reply", notmuch_reply_command, NOTMUCH_CONFIG_OPEN,
>"Construct a reply template for a set of messages." },
> -{ "tag", notmuch_tag_command, FALSE,
> +{ "tag", notmuch_tag_command, NOTMUCH_CONFIG_OPEN,
>"Add/remove tags for all messages matching the search terms." },
> -{ "dump", notmuch_dump_command, FALSE,
> +{ "dump", notmuch_dump_command, NOTMUCH_CONFIG_OPEN,
>"Create a plain-text dump of the tags for each message." },
> -{ "restore", notmuch_restore_command, FALSE,
> +{ "restore", notmuch_restore_command, NOTMUCH_CONFIG_OPEN,
>"Restore the tags from the given dump file (see 'dump')." },
> -{ "compact", notmuch_compact_command, FALSE