unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/6] notmuch cli config changes
@ 2013-01-29 21:46 Jani Nikula
  2013-01-29 21:46 ` [PATCH 1/6] cli: keep track of whether the config is newly created Jani Nikula
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-29 21:46 UTC (permalink / raw)
  To: notmuch

Hi all, the goal here is to add support for --config=FILE option at the
notmuch top level (e.g. 'notmuch --config=FILE search foo'). In order to
achieve this neatly, I ended up moving config open/close to main() from
subcommands. This isn't a bad thing, because all notmuch commands opened
the config file anyway.

As an added bonus, after this it should be trivial to 1) add top level
command line arguments to override config, or 2) add global command line
parameters passed on to subcommands via config (even if not stored in
the config file).

In the end this results in a net reduction of code.

BR,
Jani.


Jani Nikula (6):
  cli: keep track of whether the config is newly created
  cli: make notmuch_config_open() "is new" parameter input only
  cli: abstract subcommand finding into a new function
  cli: plug main notmuch command into subcommand machinery
  cli: move config open/close to main() from subcommands
  cli: add top level --config=FILE option

 notmuch-client.h     |   35 ++++++-------
 notmuch-config.c     |   73 +++++++++-----------------
 notmuch-count.c      |   11 ++--
 notmuch-dump.c       |    7 +--
 notmuch-new.c        |   17 +++----
 notmuch-reply.c      |   15 ++----
 notmuch-restore.c    |   11 ++--
 notmuch-search.c     |   15 ++----
 notmuch-setup.c      |   22 ++++----
 notmuch-show.c       |   15 ++----
 notmuch-tag.c        |   15 ++----
 notmuch.c            |  138 +++++++++++++++++++++++++++-----------------------
 test/random-corpus.c |    2 +-
 13 files changed, 159 insertions(+), 217 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/6] cli: keep track of whether the config is newly created
  2013-01-29 21:46 [PATCH 0/6] notmuch cli config changes Jani Nikula
@ 2013-01-29 21:46 ` Jani Nikula
  2013-03-03 16:13   ` David Bremner
  2013-01-29 21:46 ` [PATCH 2/6] cli: make notmuch_config_open() "is new" parameter input only Jani Nikula
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2013-01-29 21:46 UTC (permalink / raw)
  To: notmuch

Add notmuch_config_is_new() accessor function to check this.
---
 notmuch-client.h |    3 +++
 notmuch-config.c |   11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index 5f28836..07367e0 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -256,6 +256,9 @@ notmuch_config_close (notmuch_config_t *config);
 int
 notmuch_config_save (notmuch_config_t *config);
 
+notmuch_bool_t
+notmuch_config_is_new (notmuch_config_t *config);
+
 const char *
 notmuch_config_get_database_path (notmuch_config_t *config);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index b5c2066..e733e92 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -104,6 +104,7 @@ static const char search_config_comment[] =
 struct _notmuch_config {
     char *filename;
     GKeyFile *key_file;
+    notmuch_bool_t is_new;
 
     char *database_path;
     char *user_name;
@@ -266,6 +267,7 @@ notmuch_config_open (void *ctx,
 
     config->key_file = g_key_file_new ();
 
+    config->is_new = FALSE;
     config->database_path = NULL;
     config->user_name = NULL;
     config->user_primary_email = NULL;
@@ -435,6 +437,8 @@ notmuch_config_open (void *ctx,
     if (is_new_ret)
 	*is_new_ret = is_new;
 
+    config->is_new = is_new;
+
     return config;
 }
 
@@ -482,6 +486,13 @@ notmuch_config_save (notmuch_config_t *config)
     return 0;
 }
 
+notmuch_bool_t
+notmuch_config_is_new (notmuch_config_t *config)
+{
+    return config->is_new;
+}
+
+
 static const char **
 _config_get_list (notmuch_config_t *config,
 		  const char *section, const char *key,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/6] cli: make notmuch_config_open() "is new" parameter input only
  2013-01-29 21:46 [PATCH 0/6] notmuch cli config changes Jani Nikula
  2013-01-29 21:46 ` [PATCH 1/6] cli: keep track of whether the config is newly created Jani Nikula
@ 2013-01-29 21:46 ` Jani Nikula
  2013-03-03 16:17   ` David Bremner
  2013-01-29 21:46 ` [PATCH 3/6] cli: abstract subcommand finding into a new function Jani Nikula
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2013-01-29 21:46 UTC (permalink / raw)
  To: notmuch

Use the notmuch_config_is_new() function instead.
---
 notmuch-client.h     |    2 +-
 notmuch-config.c     |   32 +++++++++++---------------------
 notmuch-count.c      |    2 +-
 notmuch-dump.c       |    2 +-
 notmuch-new.c        |    2 +-
 notmuch-reply.c      |    2 +-
 notmuch-restore.c    |    2 +-
 notmuch-search.c     |    2 +-
 notmuch-setup.c      |    7 +++----
 notmuch-show.c       |    2 +-
 notmuch-tag.c        |    2 +-
 notmuch.c            |    5 ++---
 test/random-corpus.c |    2 +-
 13 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 07367e0..b3dcb21 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -248,7 +248,7 @@ typedef struct _notmuch_config notmuch_config_t;
 notmuch_config_t *
 notmuch_config_open (void *ctx,
 		     const char *filename,
-		     notmuch_bool_t *is_new_ret);
+		     notmuch_bool_t create_new);
 
 void
 notmuch_config_close (notmuch_config_t *config);
diff --git a/notmuch-config.c b/notmuch-config.c
index e733e92..247fbe4 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -233,10 +233,9 @@ get_username_from_passwd_file (void *ctx)
 notmuch_config_t *
 notmuch_config_open (void *ctx,
 		     const char *filename,
-		     notmuch_bool_t *is_new_ret)
+		     notmuch_bool_t create_new)
 {
     GError *error = NULL;
-    int is_new = 0;
     size_t tmp;
     char *notmuch_config_env = NULL;
     int file_had_database_group;
@@ -245,9 +244,6 @@ notmuch_config_open (void *ctx,
     int file_had_maildir_group;
     int file_had_search_group;
 
-    if (is_new_ret)
-	*is_new_ret = 0;
-
     notmuch_config_t *config = talloc (ctx, notmuch_config_t);
     if (config == NULL) {
 	fprintf (stderr, "Out of memory.\n");
@@ -286,17 +282,16 @@ notmuch_config_open (void *ctx,
 				     G_KEY_FILE_KEEP_COMMENTS,
 				     &error))
     {
-	/* If the caller passed a non-NULL value for is_new_ret, then
-	 * the caller is prepared for a default configuration file in
-	 * the case of FILE NOT FOUND. Otherwise, any read failure is
-	 * an error.
+	/* If create_new is true, then the caller is prepared for a
+	 * default configuration file in the case of FILE NOT
+	 * FOUND. Otherwise, any read failure is an error.
 	 */
-	if (is_new_ret &&
+	if (create_new &&
 	    error->domain == G_FILE_ERROR &&
 	    error->code == G_FILE_ERROR_NOENT)
 	{
 	    g_error_free (error);
-	    is_new = 1;
+	    config->is_new = TRUE;
 	}
 	else
 	{
@@ -379,7 +374,7 @@ notmuch_config_open (void *ctx,
     }
 
     if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) {
-	if (is_new) {
+	if (config->is_new) {
 	    const char *tags[] = { "deleted", "spam" };
 	    notmuch_config_set_search_exclude_tags (config, tags, 2);
 	} else {
@@ -399,7 +394,7 @@ notmuch_config_open (void *ctx,
     /* Whenever we know of configuration sections that don't appear in
      * the configuration file, we add some comments to help the user
      * understand what can be done. */
-    if (is_new)
+    if (config->is_new)
     {
 	g_key_file_set_comment (config->key_file, NULL, NULL,
 				toplevel_config_comment, NULL);
@@ -434,11 +429,6 @@ notmuch_config_open (void *ctx,
 				search_config_comment, NULL);
     }
 
-    if (is_new_ret)
-	*is_new_ret = is_new;
-
-    config->is_new = is_new;
-
     return config;
 }
 
@@ -719,7 +709,7 @@ notmuch_config_command_get (void *ctx, char *item)
 {
     notmuch_config_t *config;
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
@@ -781,7 +771,7 @@ notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
     if (_item_split (item, &group, &key))
 	return 1;
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
@@ -818,7 +808,7 @@ notmuch_config_command_list (void *ctx)
     char **groups;
     size_t g, groups_length;
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch-count.c b/notmuch-count.c
index 2f98128..61722ed 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -62,7 +62,7 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch-dump.c b/notmuch-dump.c
index a3244e0..845a67e 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -34,7 +34,7 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     notmuch_tags_t *tags;
     const char *query_str = "";
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch-new.c b/notmuch-new.c
index feb9c32..4915418 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -875,7 +875,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 22c58ff..9da42b9 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -762,7 +762,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 
     notmuch_exit_if_unsupported_format ();
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch-restore.c b/notmuch-restore.c
index cf26a42..dd2507f 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -139,7 +139,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     int opt_index;
     int input_format = DUMP_FORMAT_AUTO;
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 0b0a879..fac6663 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -371,7 +371,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 
     notmuch_exit_if_unsupported_format ();
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch-setup.c b/notmuch-setup.c
index 94d0aa7..72d862a 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -130,7 +130,6 @@ notmuch_setup_command (unused (void *ctx),
     size_t old_other_emails_len;
     GPtrArray *other_emails;
     unsigned int i;
-    int is_new;
     const char **new_tags;
     size_t new_tags_len;
     const char **search_exclude_tags;
@@ -147,9 +146,9 @@ notmuch_setup_command (unused (void *ctx),
 	chomp_newline (response);				\
     } while (0)
 
-    config = notmuch_config_open (ctx, NULL, &is_new);
+    config = notmuch_config_open (ctx, NULL, TRUE);
 
-    if (is_new)
+    if (notmuch_config_is_new (config))
 	welcome_message_pre_setup ();
 
     prompt ("Your full name [%s]: ", notmuch_config_get_user_name (config));
@@ -229,7 +228,7 @@ notmuch_setup_command (unused (void *ctx),
 
 
     if (! notmuch_config_save (config)) {
-	if (is_new)
+	if (notmuch_config_is_new (config))
 	  welcome_message_post_setup ();
 	return 0;
     } else {
diff --git a/notmuch-show.c b/notmuch-show.c
index cbfc2d1..5ae5d7d 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1176,7 +1176,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     else
 	params.entire_thread = FALSE;
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch-tag.c b/notmuch-tag.c
index d9daf8f..148e856 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -236,7 +236,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 	    return 1;
     }
 
-    config = notmuch_config_open (ctx, NULL, NULL);
+    config = notmuch_config_open (ctx, NULL, FALSE);
     if (config == NULL)
 	return 1;
 
diff --git a/notmuch.c b/notmuch.c
index a674481..b413b53 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -187,16 +187,15 @@ static int
 notmuch (void *ctx)
 {
     notmuch_config_t *config;
-    notmuch_bool_t is_new;
     char *db_path;
     struct stat st;
 
-    config = notmuch_config_open (ctx, NULL, &is_new);
+    config = notmuch_config_open (ctx, NULL, TRUE);
 
     /* If the user has never configured notmuch, then run
      * notmuch_setup_command which will give a nice welcome message,
      * and interactively guide the user through the configuration. */
-    if (is_new) {
+    if (notmuch_config_is_new (config)) {
 	notmuch_config_close (config);
 	return notmuch_setup_command (ctx, 0, NULL);
     }
diff --git a/test/random-corpus.c b/test/random-corpus.c
index 8b7748e..790193d 100644
--- a/test/random-corpus.c
+++ b/test/random-corpus.c
@@ -160,7 +160,7 @@ main (int argc, char **argv)
 	exit (1);
     }
 
-    config = notmuch_config_open (ctx, config_path, NULL);
+    config = notmuch_config_open (ctx, config_path, FALSE);
     if (config == NULL)
 	return 1;
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/6] cli: abstract subcommand finding into a new function
  2013-01-29 21:46 [PATCH 0/6] notmuch cli config changes Jani Nikula
  2013-01-29 21:46 ` [PATCH 1/6] cli: keep track of whether the config is newly created Jani Nikula
  2013-01-29 21:46 ` [PATCH 2/6] cli: make notmuch_config_open() "is new" parameter input only Jani Nikula
@ 2013-01-29 21:46 ` Jani Nikula
  2013-03-03 16:23   ` David Bremner
  2013-01-29 21:46 ` [PATCH 4/6] cli: plug main notmuch command into subcommand machinery Jani Nikula
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2013-01-29 21:46 UTC (permalink / raw)
  To: notmuch

Clean up code.
---
 notmuch.c |   68 +++++++++++++++++++++++++++++++------------------------------
 1 file changed, 35 insertions(+), 33 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index b413b53..c47b998 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -70,6 +70,18 @@ static command_t commands[] = {
       "This message, or more detailed help for the named command." }
 };
 
+static command_t *
+find_command (const char *name)
+{
+    size_t i;
+
+    for (i = 0; i < ARRAY_SIZE (commands); i++)
+	if (strcmp (name, commands[i].name) == 0)
+	    return &commands[i];
+
+    return NULL;
+}
+
 int notmuch_format_version;
 
 static void
@@ -139,7 +151,6 @@ static int
 notmuch_help_command (void *ctx, int argc, char *argv[])
 {
     command_t *command;
-    unsigned int i;
 
     argc--; argv++; /* Ignore "help" */
 
@@ -158,13 +169,10 @@ notmuch_help_command (void *ctx, int argc, char *argv[])
 	return 0;
     }
 
-    for (i = 0; i < ARRAY_SIZE (commands); i++) {
-	command = &commands[i];
-
-	if (strcmp (argv[0], command->name) == 0) {
-	    char *page = talloc_asprintf (ctx, "notmuch-%s", command->name);
-	    exec_man (page);
-	}
+    command = find_command (argv[0]);
+    if (command) {
+	char *page = talloc_asprintf (ctx, "notmuch-%s", command->name);
+	exec_man (page);
     }
 
     if (strcmp (argv[0], "search-terms") == 0) {
@@ -246,10 +254,11 @@ int
 main (int argc, char *argv[])
 {
     void *local;
+    char *talloc_report;
     command_t *command;
-    unsigned int i;
     notmuch_bool_t print_help=FALSE, print_version=FALSE;
     int opt_index;
+    int ret = 0;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_BOOLEAN, &print_help, "help", 'h', 0 },
@@ -284,37 +293,30 @@ main (int argc, char *argv[])
 	return 0;
     }
 
-    for (i = 0; i < ARRAY_SIZE (commands); i++) {
-	command = &commands[i];
-
-	if (strcmp (argv[opt_index], command->name) == 0) {
-	    int ret;
-	    char *talloc_report;
-
-	    ret = (command->function)(local, argc - opt_index, argv + opt_index);
+    command = find_command (argv[opt_index]);
+    if (!command) {
+	fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
+		 argv[opt_index]);
+	return 1;
+    }
 
-	    /* in the future support for this environment variable may
-	     * be supplemented or replaced by command line arguments
-	     * --leak-report and/or --leak-report-full */
+    ret = (command->function)(local, argc - opt_index, argv + opt_index);
 
-	    talloc_report = getenv ("NOTMUCH_TALLOC_REPORT");
+    /* in the future support for this environment variable may
+     * be supplemented or replaced by command line arguments
+     * --leak-report and/or --leak-report-full */
 
-	    /* this relies on the previous call to
-	     * talloc_enable_null_tracking */
+    talloc_report = getenv ("NOTMUCH_TALLOC_REPORT");
 
-	    if (talloc_report && strcmp (talloc_report, "") != 0) {
-		FILE *report = fopen (talloc_report, "w");
-		talloc_report_full (NULL, report);
-	    }
+    /* this relies on the previous call to
+     * talloc_enable_null_tracking */
 
-	    return ret;
-	}
+    if (talloc_report && strcmp (talloc_report, "") != 0) {
+	FILE *report = fopen (talloc_report, "w");
+	talloc_report_full (NULL, report);
     }
 
-    fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
-	     argv[1]);
-
     talloc_free (local);
 
-    return 1;
+    return ret;
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/6] cli: plug main notmuch command into subcommand machinery
  2013-01-29 21:46 [PATCH 0/6] notmuch cli config changes Jani Nikula
                   ` (2 preceding siblings ...)
  2013-01-29 21:46 ` [PATCH 3/6] cli: abstract subcommand finding into a new function Jani Nikula
@ 2013-01-29 21:46 ` Jani Nikula
  2013-03-03 16:28   ` David Bremner
  2013-01-29 21:46 ` [PATCH 5/6] cli: move config open/close to main() from subcommands Jani Nikula
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2013-01-29 21:46 UTC (permalink / raw)
  To: notmuch

This also allows the main notmuch command to have arguments (though
they are not used).
---
 notmuch.c |   26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index c47b998..67d5772 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -34,7 +34,13 @@ typedef struct command {
 static int
 notmuch_help_command (void *ctx, int argc, char *argv[]);
 
+static int
+notmuch_command (void *ctx, int argc, char *argv[]);
+
 static command_t commands[] = {
+    { NULL, notmuch_command,
+      NULL,
+      "Notmuch main command." },
     { "setup", notmuch_setup_command,
       NULL,
       "Interactively setup notmuch for first use." },
@@ -76,7 +82,8 @@ find_command (const char *name)
     size_t i;
 
     for (i = 0; i < ARRAY_SIZE (commands); i++)
-	if (strcmp (name, commands[i].name) == 0)
+	if ((!name && !commands[i].name) ||
+	    (name && commands[i].name && strcmp (name, commands[i].name) == 0))
 	    return &commands[i];
 
     return NULL;
@@ -101,8 +108,8 @@ usage (FILE *out)
     for (i = 0; i < ARRAY_SIZE (commands); i++) {
 	command = &commands[i];
 
-	fprintf (out, "  %-11s  %s\n",
-		 command->name, command->summary);
+	if (command->name)
+	    fprintf (out, "  %-11s  %s\n", command->name, command->summary);
     }
 
     fprintf (out, "\n");
@@ -192,7 +199,7 @@ notmuch_help_command (void *ctx, int argc, char *argv[])
  * to be more clever about this in the future.
  */
 static int
-notmuch (void *ctx)
+notmuch_command (void *ctx, unused(int argc), unused(char *argv[]))
 {
     notmuch_config_t *config;
     char *db_path;
@@ -255,6 +262,7 @@ main (int argc, char *argv[])
 {
     void *local;
     char *talloc_report;
+    const char *command_name = NULL;
     command_t *command;
     notmuch_bool_t print_help=FALSE, print_version=FALSE;
     int opt_index;
@@ -276,9 +284,6 @@ main (int argc, char *argv[])
     /* Globally default to the current output format version. */
     notmuch_format_version = NOTMUCH_FORMAT_CUR;
 
-    if (argc == 1)
-	return notmuch (local);
-
     opt_index = parse_arguments (argc, argv, options, 1);
     if (opt_index < 0) {
 	/* diagnostics already printed */
@@ -293,10 +298,13 @@ main (int argc, char *argv[])
 	return 0;
     }
 
-    command = find_command (argv[opt_index]);
+    if (opt_index < argc)
+	command_name = argv[opt_index];
+
+    command = find_command (command_name);
     if (!command) {
 	fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
-		 argv[opt_index]);
+		 command_name);
 	return 1;
     }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/6] cli: move config open/close to main() from subcommands
  2013-01-29 21:46 [PATCH 0/6] notmuch cli config changes Jani Nikula
                   ` (3 preceding siblings ...)
  2013-01-29 21:46 ` [PATCH 4/6] cli: plug main notmuch command into subcommand machinery Jani Nikula
@ 2013-01-29 21:46 ` Jani Nikula
  2013-01-29 21:46 ` [PATCH 6/6] cli: add top level --config=FILE option Jani Nikula
  2013-02-06 17:50 ` [PATCH 0/6] notmuch cli config changes Jameson Graef Rollins
  6 siblings, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-29 21:46 UTC (permalink / raw)
  To: notmuch

This allows specifying config file as a top level argument to notmuch,
and generally makes it possible to override config file options in
main(), without having to touch the subcommands.

This also makes notmuch config the talloc context for subcommands.
---
 notmuch-client.h  |   30 +++++++++++---------------
 notmuch-config.c  |   40 +++++++----------------------------
 notmuch-count.c   |   11 +++-------
 notmuch-dump.c    |    7 +-----
 notmuch-new.c     |   17 ++++++---------
 notmuch-reply.c   |   15 +++++--------
 notmuch-restore.c |   11 +++-------
 notmuch-search.c  |   15 +++++--------
 notmuch-setup.c   |   17 ++++++---------
 notmuch-show.c    |   15 +++++--------
 notmuch-tag.c     |   15 +++++--------
 notmuch.c         |   61 +++++++++++++++++++++++++++--------------------------
 12 files changed, 91 insertions(+), 163 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index b3dcb21..45749a6 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -150,6 +150,8 @@ chomp_newline (char *str)
  */
 extern int notmuch_format_version;
 
+typedef struct _notmuch_config notmuch_config_t;
+
 /* Commands that support structured output should support the
  * following argument
  *  { NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 }
@@ -169,40 +171,34 @@ int
 notmuch_crypto_cleanup (notmuch_crypto_t *crypto);
 
 int
-notmuch_count_command (void *ctx, int argc, char *argv[]);
-
-int
-notmuch_dump_command (void *ctx, int argc, char *argv[]);
+notmuch_count_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_new_command (void *ctx, int argc, char *argv[]);
+notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_reply_command (void *ctx, int argc, char *argv[]);
+notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_restore_command (void *ctx, int argc, char *argv[]);
+notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_search_command (void *ctx, int argc, char *argv[]);
+notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_setup_command (void *ctx, int argc, char *argv[]);
+notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_show_command (void *ctx, int argc, char *argv[]);
+notmuch_setup_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_tag_command (void *ctx, int argc, char *argv[]);
+notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_search_tags_command (void *ctx, int argc, char *argv[]);
+notmuch_tag_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
-notmuch_cat_command (void *ctx, int argc, char *argv[]);
-
-int
-notmuch_config_command (void *ctx, int argc, char *argv[]);
+notmuch_config_command (notmuch_config_t *config, int argc, char *argv[]);
 
 const char *
 notmuch_time_relative_date (const void *ctx, time_t then);
@@ -243,8 +239,6 @@ json_quote_str (const void *ctx, const char *str);
 
 /* notmuch-config.c */
 
-typedef struct _notmuch_config notmuch_config_t;
-
 notmuch_config_t *
 notmuch_config_open (void *ctx,
 		     const char *filename,
diff --git a/notmuch-config.c b/notmuch-config.c
index 247fbe4..48312e3 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -705,14 +705,8 @@ _item_split (char *item, char **group, char **key)
 }
 
 static int
-notmuch_config_command_get (void *ctx, char *item)
+notmuch_config_command_get (notmuch_config_t *config, char *item)
 {
-    notmuch_config_t *config;
-
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     if (strcmp(item, "database.path") == 0) {
 	printf ("%s\n", notmuch_config_get_database_path (config));
     } else if (strcmp(item, "user.name") == 0) {
@@ -756,25 +750,17 @@ notmuch_config_command_get (void *ctx, char *item)
 	g_strfreev (value);
     }
 
-    notmuch_config_close (config);
-
     return 0;
 }
 
 static int
-notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
+notmuch_config_command_set (notmuch_config_t *config, char *item, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     char *group, *key;
-    int ret;
 
     if (_item_split (item, &group, &key))
 	return 1;
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     /* With only the name of an item, we clear it from the
      * configuration file.
      *
@@ -795,23 +781,15 @@ notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
 	break;
     }
 
-    ret = notmuch_config_save (config);
-    notmuch_config_close (config);
-
-    return ret;
+    return notmuch_config_save (config);
 }
 
 static int
-notmuch_config_command_list (void *ctx)
+notmuch_config_command_list (notmuch_config_t *config)
 {
-    notmuch_config_t *config;
     char **groups;
     size_t g, groups_length;
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     groups = g_key_file_get_groups (config->key_file, &groups_length);
     if (groups == NULL)
 	return 1;
@@ -841,13 +819,11 @@ notmuch_config_command_list (void *ctx)
 
     g_strfreev (groups);
 
-    notmuch_config_close (config);
-
     return 0;
 }
 
 int
-notmuch_config_command (void *ctx, int argc, char *argv[])
+notmuch_config_command (notmuch_config_t *config, int argc, char *argv[])
 {
     argc--; argv++; /* skip subcommand argument */
 
@@ -862,16 +838,16 @@ notmuch_config_command (void *ctx, int argc, char *argv[])
 		     "one argument.\n");
 	    return 1;
 	}
-	return notmuch_config_command_get (ctx, argv[1]);
+	return notmuch_config_command_get (config, argv[1]);
     } else if (strcmp (argv[0], "set") == 0) {
 	if (argc < 2) {
 	    fprintf (stderr, "Error: notmuch config set requires at least "
 		     "one argument.\n");
 	    return 1;
 	}
-	return notmuch_config_command_set (ctx, argv[1], argc - 2, argv + 2);
+	return notmuch_config_command_set (config, argv[1], argc - 2, argv + 2);
     } else if (strcmp (argv[0], "list") == 0) {
-	return notmuch_config_command_list (ctx);
+	return notmuch_config_command_list (config);
     }
 
     fprintf (stderr, "Unrecognized argument for notmuch config: %s\n",
diff --git a/notmuch-count.c b/notmuch-count.c
index 61722ed..390794f 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -33,9 +33,8 @@ enum {
 };
 
 int
-notmuch_count_command (void *ctx, int argc, char *argv[])
+notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_query_t *query;
     char *query_str;
@@ -62,22 +61,18 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     if (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
-    query_str = query_string_from_args (ctx, argc-opt_index, argv+opt_index);
+    query_str = query_string_from_args (config, argc-opt_index, argv+opt_index);
     if (query_str == NULL) {
 	fprintf (stderr, "Out of memory.\n");
 	return 1;
     }
 
     if (*query_str == '\0') {
-	query_str = talloc_strdup (ctx, "");
+	query_str = talloc_strdup (config, "");
     }
 
     query = notmuch_query_create (notmuch, query_str);
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 845a67e..2024e30 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -23,9 +23,8 @@
 #include "string-util.h"
 
 int
-notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
+notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_query_t *query;
     FILE *output = stdout;
@@ -34,10 +33,6 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     notmuch_tags_t *tags;
     const char *query_str = "";
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     if (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
diff --git a/notmuch-new.c b/notmuch-new.c
index 4915418..faa33f1 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -840,9 +840,8 @@ _remove_directory (void *ctx,
 }
 
 int
-notmuch_new_command (void *ctx, int argc, char *argv[])
+notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     add_files_state_t add_files_state;
     double elapsed;
@@ -875,10 +874,6 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);
     add_files_state.new_ignore = notmuch_config_get_new_ignore (config, &add_files_state.new_ignore_length);
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
@@ -890,7 +885,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	    return ret;
     }
 
-    dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
+    dot_notmuch_path = talloc_asprintf (config, "%s/%s", db_path, ".notmuch");
 
     if (stat (dot_notmuch_path, &st)) {
 	int count;
@@ -941,9 +936,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     add_files_state.removed_messages = add_files_state.renamed_messages = 0;
     gettimeofday (&add_files_state.tv_start, NULL);
 
-    add_files_state.removed_files = _filename_list_create (ctx);
-    add_files_state.removed_directories = _filename_list_create (ctx);
-    add_files_state.directory_mtimes = _filename_list_create (ctx);
+    add_files_state.removed_files = _filename_list_create (config);
+    add_files_state.removed_directories = _filename_list_create (config);
+    add_files_state.directory_mtimes = _filename_list_create (config);
 
     if (! debugger_is_active () && add_files_state.output_is_a_tty
 	&& ! add_files_state.verbose) {
@@ -970,7 +965,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     gettimeofday (&tv_start, NULL);
     for (f = add_files_state.removed_directories->head, i = 0; f && !interrupted; f = f->next, i++) {
-	ret = _remove_directory (ctx, notmuch, f->filename, &add_files_state);
+	ret = _remove_directory (config, notmuch, f->filename, &add_files_state);
 	if (ret)
 	    goto DONE;
 	if (do_print_progress) {
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9da42b9..e151f78 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -702,9 +702,8 @@ enum {
 };
 
 int
-notmuch_reply_command (void *ctx, int argc, char *argv[])
+notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_query_t *query;
     char *query_string;
@@ -752,21 +751,17 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	reply_format_func = notmuch_reply_format_headers_only;
     } else if (format == FORMAT_JSON) {
 	reply_format_func = notmuch_reply_format_sprinter;
-	sp = sprinter_json_create (ctx, stdout);
+	sp = sprinter_json_create (config, stdout);
     } else if (format == FORMAT_SEXP) {
 	reply_format_func = notmuch_reply_format_sprinter;
-	sp = sprinter_sexp_create (ctx, stdout);
+	sp = sprinter_sexp_create (config, stdout);
     } else {
 	reply_format_func = notmuch_reply_format_default;
     }
 
     notmuch_exit_if_unsupported_format ();
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
-    query_string = query_string_from_args (ctx, argc-opt_index, argv+opt_index);
+    query_string = query_string_from_args (config, argc-opt_index, argv+opt_index);
     if (query_string == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return 1;
@@ -787,7 +782,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    if (reply_format_func (ctx, config, query, &params, reply_all, sp) != 0)
+    if (reply_format_func (config, config, query, &params, reply_all, sp) != 0)
 	return 1;
 
     notmuch_crypto_cleanup (&params.crypto);
diff --git a/notmuch-restore.c b/notmuch-restore.c
index dd2507f..1419621 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -120,9 +120,8 @@ parse_sup_line (void *ctx, char *line,
 }
 
 int
-notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
+notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_bool_t accumulate = FALSE;
     tag_op_flag_t flags = 0;
@@ -139,10 +138,6 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     int opt_index;
     int input_format = DUMP_FORMAT_AUTO;
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     if (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
 	return 1;
@@ -187,7 +182,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	return 1;
     }
 
-    tag_ops = tag_op_list_create (ctx);
+    tag_ops = tag_op_list_create (config);
     if (tag_ops == NULL) {
 	fprintf (stderr, "Out of memory.\n");
 	return 1;
@@ -226,7 +221,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	if (line_ctx != NULL)
 	    talloc_free (line_ctx);
 
-	line_ctx = talloc_new (ctx);
+	line_ctx = talloc_new (config);
 	if (input_format == DUMP_FORMAT_SUP) {
 	    ret = parse_sup_line (line_ctx, line, &query_string, tag_ops);
 	} else {
diff --git a/notmuch-search.c b/notmuch-search.c
index fac6663..e658639 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -290,9 +290,8 @@ enum {
 };
 
 int
-notmuch_search_command (void *ctx, int argc, char *argv[])
+notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_query_t *query;
     char *query_str;
@@ -349,20 +348,20 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
-	format = sprinter_text_create (ctx, stdout);
+	format = sprinter_text_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_TEXT0:
 	if (output == OUTPUT_SUMMARY) {
 	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
 	    return 1;
 	}
-	format = sprinter_text0_create (ctx, stdout);
+	format = sprinter_text0_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_JSON:
-	format = sprinter_json_create (ctx, stdout);
+	format = sprinter_json_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_SEXP:
-	format = sprinter_sexp_create (ctx, stdout);
+	format = sprinter_sexp_create (config, stdout);
 	break;
     default:
 	/* this should never happen */
@@ -371,10 +370,6 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 
     notmuch_exit_if_unsupported_format ();
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     if (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
diff --git a/notmuch-setup.c b/notmuch-setup.c
index 72d862a..475248b 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -120,12 +120,11 @@ parse_tag_list (void *ctx, char *response)
 }
 
 int
-notmuch_setup_command (unused (void *ctx),
+notmuch_setup_command (notmuch_config_t *config,
 		       unused (int argc), unused (char *argv[]))
 {
     char *response = NULL;
     size_t response_size = 0;
-    notmuch_config_t *config;
     const char **old_other_emails;
     size_t old_other_emails_len;
     GPtrArray *other_emails;
@@ -146,8 +145,6 @@ notmuch_setup_command (unused (void *ctx),
 	chomp_newline (response);				\
     } while (0)
 
-    config = notmuch_config_open (ctx, NULL, TRUE);
-
     if (notmuch_config_is_new (config))
 	welcome_message_pre_setup ();
 
@@ -167,16 +164,16 @@ notmuch_setup_command (unused (void *ctx),
     for (i = 0; i < old_other_emails_len; i++) {
 	prompt ("Additional email address [%s]: ", old_other_emails[i]);
 	if (strlen (response))
-	    g_ptr_array_add (other_emails, talloc_strdup (ctx, response));
+	    g_ptr_array_add (other_emails, talloc_strdup (config, response));
 	else
-	    g_ptr_array_add (other_emails, talloc_strdup (ctx,
+	    g_ptr_array_add (other_emails, talloc_strdup (config,
 							 old_other_emails[i]));
     }
 
     do {
 	prompt ("Additional email address [Press 'Enter' if none]: ");
 	if (strlen (response))
-	    g_ptr_array_add (other_emails, talloc_strdup (ctx, response));
+	    g_ptr_array_add (other_emails, talloc_strdup (config, response));
     } while (strlen (response));
     if (other_emails->len)
 	notmuch_config_set_user_other_email (config,
@@ -190,7 +187,7 @@ notmuch_setup_command (unused (void *ctx),
     if (strlen (response)) {
 	const char *absolute_path;
 
-	absolute_path = make_path_absolute (ctx, response);
+	absolute_path = make_path_absolute (config, response);
 	notmuch_config_set_database_path (config, absolute_path);
     }
 
@@ -201,7 +198,7 @@ notmuch_setup_command (unused (void *ctx),
     prompt ("]: ");
 
     if (strlen (response)) {
-	GPtrArray *tags = parse_tag_list (ctx, response);
+	GPtrArray *tags = parse_tag_list (config, response);
 
 	notmuch_config_set_new_tags (config, (const char **) tags->pdata,
 				     tags->len);
@@ -217,7 +214,7 @@ notmuch_setup_command (unused (void *ctx),
     prompt ("]: ");
 
     if (strlen (response)) {
-	GPtrArray *tags = parse_tag_list (ctx, response);
+	GPtrArray *tags = parse_tag_list (config, response);
 
 	notmuch_config_set_search_exclude_tags (config,
 						(const char **) tags->pdata,
diff --git a/notmuch-show.c b/notmuch-show.c
index 5ae5d7d..c2ec122 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1056,9 +1056,8 @@ enum {
 };
 
 int
-notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
+notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_query_t *query;
     char *query_string;
@@ -1176,11 +1175,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     else
 	params.entire_thread = FALSE;
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
-    query_string = query_string_from_args (ctx, argc-opt_index, argv+opt_index);
+    query_string = query_string_from_args (config, argc-opt_index, argv+opt_index);
     if (query_string == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return 1;
@@ -1202,11 +1197,11 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     }
 
     /* Create structure printer. */
-    sprinter = format->new_sprinter(ctx, stdout);
+    sprinter = format->new_sprinter(config, stdout);
 
     /* If a single message is requested we do not use search_excludes. */
     if (params.part >= 0)
-	ret = do_show_single (ctx, query, format, sprinter, &params);
+	ret = do_show_single (config, query, format, sprinter, &params);
     else {
 	/* We always apply set the exclude flag. The
 	 * exclude=true|false option controls whether or not we return
@@ -1225,7 +1220,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	    params.omit_excluded = FALSE;
 	}
 
-	ret = do_show (ctx, query, format, sprinter, &params);
+	ret = do_show (config, query, format, sprinter, &params);
     }
 
     notmuch_crypto_cleanup (&params.crypto);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 148e856..0e73197 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -178,11 +178,10 @@ tag_file (void *ctx, notmuch_database_t *notmuch, tag_op_flag_t flags,
 }
 
 int
-notmuch_tag_command (void *ctx, int argc, char *argv[])
+notmuch_tag_command (notmuch_config_t *config, int argc, char *argv[])
 {
     tag_op_list_t *tag_ops = NULL;
     char *query_string = NULL;
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     struct sigaction action;
     tag_op_flag_t tag_flags = TAG_FLAG_NONE;
@@ -225,21 +224,17 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 	    return 1;
 	}
     } else {
-	tag_ops = tag_op_list_create (ctx);
+	tag_ops = tag_op_list_create (config);
 	if (tag_ops == NULL) {
 	    fprintf (stderr, "Out of memory.\n");
 	    return 1;
 	}
 
-	if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
+	if (parse_tag_command_line (config, argc - opt_index, argv + opt_index,
 				    &query_string, tag_ops))
 	    return 1;
     }
 
-    config = notmuch_config_open (ctx, NULL, FALSE);
-    if (config == NULL)
-	return 1;
-
     if (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
 	return 1;
@@ -248,9 +243,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 	tag_flags |= TAG_FLAG_MAILDIR_SYNC;
 
     if (batch)
-	ret = tag_file (ctx, notmuch, tag_flags, input);
+	ret = tag_file (config, notmuch, tag_flags, input);
     else
-	ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
+	ret = tag_query (config, notmuch, query_string, tag_ops, tag_flags);
 
     notmuch_database_destroy (notmuch);
 
diff --git a/notmuch.c b/notmuch.c
index 67d5772..f4bfeaa 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -22,56 +22,57 @@
 
 #include "notmuch-client.h"
 
-typedef int (*command_function_t) (void *ctx, int argc, char *argv[]);
+typedef int (*command_function_t) (notmuch_config_t *config, int argc, char *argv[]);
 
 typedef struct command {
     const char *name;
     command_function_t function;
+    notmuch_bool_t create_config;
     const char *arguments;
     const char *summary;
 } command_t;
 
 static int
-notmuch_help_command (void *ctx, int argc, char *argv[]);
+notmuch_help_command (notmuch_config_t *config, int argc, char *argv[]);
 
 static int
-notmuch_command (void *ctx, int argc, char *argv[]);
+notmuch_command (notmuch_config_t *config, int argc, char *argv[]);
 
 static command_t commands[] = {
-    { NULL, notmuch_command,
+    { NULL, notmuch_command, TRUE,
       NULL,
       "Notmuch main command." },
-    { "setup", notmuch_setup_command,
+    { "setup", notmuch_setup_command, TRUE,
       NULL,
       "Interactively setup notmuch for first use." },
-    { "new", notmuch_new_command,
+    { "new", notmuch_new_command, FALSE,
       "[options...]",
       "Find and import new messages to the notmuch database." },
-    { "search", notmuch_search_command,
+    { "search", notmuch_search_command, FALSE,
       "[options...] <search-terms> [...]",
       "Search for messages matching the given search terms." },
-    { "show", notmuch_show_command,
+    { "show", notmuch_show_command, FALSE,
       "<search-terms> [...]",
       "Show all messages matching the search terms." },
-    { "count", notmuch_count_command,
+    { "count", notmuch_count_command, FALSE,
       "[options...] <search-terms> [...]",
       "Count messages matching the search terms." },
-    { "reply", notmuch_reply_command,
+    { "reply", notmuch_reply_command, FALSE,
       "[options...] <search-terms> [...]",
       "Construct a reply template for a set of messages." },
-    { "tag", notmuch_tag_command,
+    { "tag", notmuch_tag_command, FALSE,
       "+<tag>|-<tag> [...] [--] <search-terms> [...]" ,
       "Add/remove tags for all messages matching the search terms." },
-    { "dump", notmuch_dump_command,
+    { "dump", notmuch_dump_command, FALSE,
       "[<filename>] [--] [<search-terms>]",
       "Create a plain-text dump of the tags for each message." },
-    { "restore", notmuch_restore_command,
+    { "restore", notmuch_restore_command, FALSE,
       "[--accumulate] [<filename>]",
       "Restore the tags from the given dump file (see 'dump')." },
-    { "config", notmuch_config_command,
+    { "config", notmuch_config_command, FALSE,
       "[get|set] <section>.<item> [value ...]",
       "Get or set settings in the notmuch configuration file." },
-    { "help", notmuch_help_command,
+    { "help", notmuch_help_command, FALSE,
       "[<command>]",
       "This message, or more detailed help for the named command." }
 };
@@ -155,7 +156,7 @@ exec_man (const char *page)
 }
 
 static int
-notmuch_help_command (void *ctx, int argc, char *argv[])
+notmuch_help_command (notmuch_config_t *config, int argc, char *argv[])
 {
     command_t *command;
 
@@ -178,7 +179,7 @@ notmuch_help_command (void *ctx, int argc, char *argv[])
 
     command = find_command (argv[0]);
     if (command) {
-	char *page = talloc_asprintf (ctx, "notmuch-%s", command->name);
+	char *page = talloc_asprintf (config, "notmuch-%s", command->name);
 	exec_man (page);
     }
 
@@ -199,28 +200,23 @@ notmuch_help_command (void *ctx, int argc, char *argv[])
  * to be more clever about this in the future.
  */
 static int
-notmuch_command (void *ctx, unused(int argc), unused(char *argv[]))
+notmuch_command (notmuch_config_t *config,
+		 unused(int argc), unused(char *argv[]))
 {
-    notmuch_config_t *config;
     char *db_path;
     struct stat st;
 
-    config = notmuch_config_open (ctx, NULL, TRUE);
-
     /* If the user has never configured notmuch, then run
      * notmuch_setup_command which will give a nice welcome message,
      * and interactively guide the user through the configuration. */
-    if (notmuch_config_is_new (config)) {
-	notmuch_config_close (config);
-	return notmuch_setup_command (ctx, 0, NULL);
-    }
+    if (notmuch_config_is_new (config))
+	return notmuch_setup_command (config, 0, NULL);
 
     /* Notmuch is already configured, but is there a database? */
-    db_path = talloc_asprintf (ctx, "%s/%s",
+    db_path = talloc_asprintf (config, "%s/%s",
 			       notmuch_config_get_database_path (config),
 			       ".notmuch");
     if (stat (db_path, &st)) {
-	notmuch_config_close (config);
 	if (errno != ENOENT) {
 	    fprintf (stderr, "Error looking for notmuch database at %s: %s\n",
 		     db_path, strerror (errno));
@@ -252,8 +248,6 @@ notmuch_command (void *ctx, unused(int argc), unused(char *argv[]))
 	    notmuch_config_get_user_name (config),
 	    notmuch_config_get_user_primary_email (config));
 
-    notmuch_config_close (config);
-
     return 0;
 }
 
@@ -264,6 +258,7 @@ main (int argc, char *argv[])
     char *talloc_report;
     const char *command_name = NULL;
     command_t *command;
+    notmuch_config_t *config;
     notmuch_bool_t print_help=FALSE, print_version=FALSE;
     int opt_index;
     int ret = 0;
@@ -308,7 +303,13 @@ main (int argc, char *argv[])
 	return 1;
     }
 
-    ret = (command->function)(local, argc - opt_index, argv + opt_index);
+    config = notmuch_config_open (local, NULL, command->create_config);
+    if (!config)
+	return 1;
+
+    ret = (command->function)(config, argc - opt_index, argv + opt_index);
+
+    notmuch_config_close (config);
 
     /* in the future support for this environment variable may
      * be supplemented or replaced by command line arguments
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/6] cli: add top level --config=FILE option
  2013-01-29 21:46 [PATCH 0/6] notmuch cli config changes Jani Nikula
                   ` (4 preceding siblings ...)
  2013-01-29 21:46 ` [PATCH 5/6] cli: move config open/close to main() from subcommands Jani Nikula
@ 2013-01-29 21:46 ` Jani Nikula
  2013-03-03 16:32   ` David Bremner
  2013-02-06 17:50 ` [PATCH 0/6] notmuch cli config changes Jameson Graef Rollins
  6 siblings, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2013-01-29 21:46 UTC (permalink / raw)
  To: notmuch

Let the user specify the config file on the command line.
---
 notmuch.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/notmuch.c b/notmuch.c
index f4bfeaa..71cd0d6 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -258,6 +258,7 @@ main (int argc, char *argv[])
     char *talloc_report;
     const char *command_name = NULL;
     command_t *command;
+    char *config_file_name = NULL;
     notmuch_config_t *config;
     notmuch_bool_t print_help=FALSE, print_version=FALSE;
     int opt_index;
@@ -266,6 +267,7 @@ main (int argc, char *argv[])
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_BOOLEAN, &print_help, "help", 'h', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &print_version, "version", 'v', 0 },
+	{ NOTMUCH_OPT_STRING, &config_file_name, "config", 'c', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -303,7 +305,7 @@ main (int argc, char *argv[])
 	return 1;
     }
 
-    config = notmuch_config_open (local, NULL, command->create_config);
+    config = notmuch_config_open (local, config_file_name, command->create_config);
     if (!config)
 	return 1;
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-01-29 21:46 [PATCH 0/6] notmuch cli config changes Jani Nikula
                   ` (5 preceding siblings ...)
  2013-01-29 21:46 ` [PATCH 6/6] cli: add top level --config=FILE option Jani Nikula
@ 2013-02-06 17:50 ` Jameson Graef Rollins
  2013-02-07  7:43   ` Jani Nikula
  2013-02-07 12:07   ` David Bremner
  6 siblings, 2 replies; 22+ messages in thread
From: Jameson Graef Rollins @ 2013-02-06 17:50 UTC (permalink / raw)
  To: Jani Nikula, notmuch

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

On Tue, Jan 29 2013, Jani Nikula <jani@nikula.org> wrote:
> Hi all, the goal here is to add support for --config=FILE option at the
> notmuch top level (e.g. 'notmuch --config=FILE search foo'). In order to
> achieve this neatly, I ended up moving config open/close to main() from
> subcommands. This isn't a bad thing, because all notmuch commands opened
> the config file anyway.

Hi, Jani.  I appreciate you've put a lot of work into this series, but
I'll be the same devil's advocate that I was to David previously.  Why
do we need a command line option here when we already have an
environment variable that handles just this?  Is there some benefit to
having a command line option for this that I don't see?  I see this as
another instance of an option that regular users will rarely use, if
ever.

In general, I am a strong advocate of keeping the CLI slim.  IMHO,
adding more options makes the interface clunkier, and the manual harder
to parse, and I'm against adding things that a normal user would likely
never use.  In retrospect, I should have had the same objection to the
--format-version option, which I think could have just been an env var
as well.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-06 17:50 ` [PATCH 0/6] notmuch cli config changes Jameson Graef Rollins
@ 2013-02-07  7:43   ` Jani Nikula
  2013-02-07 12:07   ` David Bremner
  1 sibling, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-02-07  7:43 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch

On Wed, 06 Feb 2013, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Tue, Jan 29 2013, Jani Nikula <jani@nikula.org> wrote:
>> Hi all, the goal here is to add support for --config=FILE option at the
>> notmuch top level (e.g. 'notmuch --config=FILE search foo'). In order to
>> achieve this neatly, I ended up moving config open/close to main() from
>> subcommands. This isn't a bad thing, because all notmuch commands opened
>> the config file anyway.
>
> Hi, Jani.  I appreciate you've put a lot of work into this series, but
> I'll be the same devil's advocate that I was to David previously.  Why
> do we need a command line option here when we already have an
> environment variable that handles just this?  Is there some benefit to
> having a command line option for this that I don't see?  I see this as
> another instance of an option that regular users will rarely use, if
> ever.

Fair enough.

In any case I see patches 1-4, and to an extent also patch 5, as useful
refactoring and cleanup. Also, if we ever end up having any top level
arguments that need to be passed to subcommands, I think patches 1-5 are
good prep work. The config struct could be extended to include both
settings from the config file and settings from the top level
arguments. But this is just a bonus in addition to the refactoring part.

> In general, I am a strong advocate of keeping the CLI slim.  IMHO,
> adding more options makes the interface clunkier, and the manual harder
> to parse, and I'm against adding things that a normal user would likely
> never use.  In retrospect, I should have had the same objection to the
> --format-version option, which I think could have just been an env var
> as well.

What is a "normal user"? I'm a user too.


BR,
Jani.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-06 17:50 ` [PATCH 0/6] notmuch cli config changes Jameson Graef Rollins
  2013-02-07  7:43   ` Jani Nikula
@ 2013-02-07 12:07   ` David Bremner
  2013-02-12  7:50     ` Jameson Graef Rollins
  1 sibling, 1 reply; 22+ messages in thread
From: David Bremner @ 2013-02-07 12:07 UTC (permalink / raw)
  To: Jameson Graef Rollins, Jani Nikula, notmuch

Jameson Graef Rollins <jrollins@finestructure.net> writes:

>  Is there some benefit to having a command line option for this that I
> don't see?

In my experience the environment variable is somewhat dangerous to use
while testing. If left set to the wrong value, it can lead the loss of
tag information.

>  I see this as another instance of an option that regular users will
> rarely use, if ever.

I don't see this as really comparable with a debugging option:
temporarily changing the configuration is something that can be useful
outside of debugging and testing. Although I'm fuzzy on the details now,
I remember telling someone to do this, I think to temporarily modify
what was ignored.

> In general, I am a strong advocate of keeping the CLI slim.  IMHO,
> adding more options makes the interface clunkier, and the manual harder
> to parse, and I'm against adding things that a normal user would likely
> never use. 

Well, it's are reasonable heuristic, although I might disagree in
general where the cutoff for "normal use" is, as I do in this case.

> In retrospect, I should have had the same objection to the
> --format-version option, which I think could have just been an env var
> as well.

I don't follow this part of your argument. I don't think replacing
command line arguments by environment variables makes the interface or
documentation simpler; at least if we document the environment variables
properly.  Note that non-shell based invokation (which is often
desirable from e.g. scripting languages or Emacs) is typically, more
complicated using environment variables. It's also more complicated for
the user if they do have to use these options in a shell or a script.

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-07 12:07   ` David Bremner
@ 2013-02-12  7:50     ` Jameson Graef Rollins
  2013-02-12 11:42       ` David Bremner
  0 siblings, 1 reply; 22+ messages in thread
From: Jameson Graef Rollins @ 2013-02-12  7:50 UTC (permalink / raw)
  To: David Bremner, Jani Nikula, notmuch

[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]

On Thu, Feb 07 2013, David Bremner <david@tethera.net> wrote:
> In my experience the environment variable is somewhat dangerous to use
> while testing. If left set to the wrong value, it can lead the loss of
> tag information.

I have never noticed this to be an issue, but if it is variables can be
applied only at run time as well:

NOTMUCH_CONFIG=/path/to/foo notmuch ....

>> In general, I am a strong advocate of keeping the CLI slim.  IMHO,
>> adding more options makes the interface clunkier, and the manual harder
>> to parse, and I'm against adding things that a normal user would likely
>> never use. 
>
> Well, it's are reasonable heuristic, although I might disagree in
> general where the cutoff for "normal use" is, as I do in this case.

My main point is that shoving every possible thing that can be tweaked
into CLI options is imho a bad idea.  Look at gpg.  The interface is
horrible, and the man page is basically impenetrable because it's so
loaded with options that it's impossible to figure out how to do the one
basic operation you're looking for.

But you're right that I'm making a pretty arbitrary distinction.  The
notmuch CLI already includes options to handle output formatting,
etc. that normal users are probably never going to use in their
infrequent use of the CLI.  In that regard an option to point to a
alternate config file doesn't seem that unreasonable.

I just don't want to see notmuch fall into the same UI black hole that
e.g. gpg did.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-12  7:50     ` Jameson Graef Rollins
@ 2013-02-12 11:42       ` David Bremner
  2013-02-12 16:54         ` Jameson Graef Rollins
  0 siblings, 1 reply; 22+ messages in thread
From: David Bremner @ 2013-02-12 11:42 UTC (permalink / raw)
  To: Jameson Graef Rollins, Jani Nikula, notmuch

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> But you're right that I'm making a pretty arbitrary distinction.  The
> notmuch CLI already includes options to handle output formatting,
> etc. that normal users are probably never going to use in their
> infrequent use of the CLI.  

I don't know if this last is just an off the cuff remark, or it
highlights a difference of opinion. In my view "normal" users of notmuch
use the CLI frequently.  Arguably that means we should take even more
care over the design of the UI, if we can do that doesn't descend too
far into bikeshedding territory.

> I just don't want to see notmuch fall into the same UI black hole that
> e.g. gpg did.

GPG is indeed a cautionary tale. FWIW my biggest gripe with GPG is that
on top of the profusion of options, I almost always have to use
"with-colons" mode to do what I actually want. I don't know what the
lesson there is, other than maximum UI fail.

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-12 11:42       ` David Bremner
@ 2013-02-12 16:54         ` Jameson Graef Rollins
  2013-02-12 18:53           ` David Bremner
  2013-02-12 20:06           ` Jani Nikula
  0 siblings, 2 replies; 22+ messages in thread
From: Jameson Graef Rollins @ 2013-02-12 16:54 UTC (permalink / raw)
  To: David Bremner, Jani Nikula, notmuch

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

On Tue, Feb 12 2013, David Bremner <david@tethera.net> wrote:
> I don't know if this last is just an off the cuff remark, or it
> highlights a difference of opinion. In my view "normal" users of notmuch
> use the CLI frequently.  Arguably that means we should take even more
> care over the design of the UI, if we can do that doesn't descend too
> far into bikeshedding territory.

Probably a difference of opinion.  Under my guise as "normal user" I
hardly ever use the CLI (I'm not counting the emacs UI use of the CLI
under the hood).  In my developer hat I use it more frequently.  But
maybe (likely?) all of our users are also developers.  Even as a
developer, though, most of my CLI usage is scripted at this point.

But don't get me wrong, the CLI is one of the things that makes notmuch
so incredibly awesome.  It's an email swiss army knife that's there when
you need it.  But given that even I often need to look at the man page
during my occasional CLI usage, I just don't want to see it get to
overcrowded.

In any event, I've certainly said more than my peace on this issue.
Jani's series looks good to me (although, ironically, it's missing
documentation updates!).

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-12 16:54         ` Jameson Graef Rollins
@ 2013-02-12 18:53           ` David Bremner
  2013-02-13  7:51             ` Mark Walters
  2013-02-12 20:06           ` Jani Nikula
  1 sibling, 1 reply; 22+ messages in thread
From: David Bremner @ 2013-02-12 18:53 UTC (permalink / raw)
  To: Jameson Graef Rollins, Jani Nikula, notmuch

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> But don't get me wrong, the CLI is one of the things that makes notmuch
> so incredibly awesome.  It's an email swiss army knife that's there when
> you need it.  But given that even I often need to look at the man page
> during my occasional CLI usage, I just don't want to see it get to
> overcrowded.

Well, maybe this should be a discussion about how to organize the CLI
documentation so that more commonly used options are easy to find. That
would indeed be a side effect of making less commonly used options
controlled by environment variables, and documenting the environment
variables at the bottom of the man pages as per tradition. But I don't
think this is the only way or even the best way to achieve good
documentation.

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-12 16:54         ` Jameson Graef Rollins
  2013-02-12 18:53           ` David Bremner
@ 2013-02-12 20:06           ` Jani Nikula
  2013-03-03 16:36             ` David Bremner
  1 sibling, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2013-02-12 20:06 UTC (permalink / raw)
  To: Jameson Graef Rollins, David Bremner, notmuch

On Tue, 12 Feb 2013, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> But don't get me wrong, the CLI is one of the things that makes notmuch
> so incredibly awesome.  It's an email swiss army knife that's there when
> you need it.  But given that even I often need to look at the man page
> during my occasional CLI usage, I just don't want to see it get to
> overcrowded.

Retrospectively, I observe that my series (up to but not including the
last patch) might help reduce the clutter in the future by facilitating
the move of common options from subcommands to top level. For example,
*all* the options to notmuch new would make (more) sense at the top
level (--verbose, --debug, --no-hooks) and could be useful in other
subcommands.

> In any event, I've certainly said more than my peace on this issue.
> Jani's series looks good to me (although, ironically, it's missing
> documentation updates!).

Heh, I just wanted to see what the response was first before putting in
the effort to document it. The code comes easy. :)

BR,
Jani.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-12 18:53           ` David Bremner
@ 2013-02-13  7:51             ` Mark Walters
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Walters @ 2013-02-13  7:51 UTC (permalink / raw)
  To: David Bremner, Jameson Graef Rollins, Jani Nikula, notmuch


> Jameson Graef Rollins <jrollins@finestructure.net> writes:
>
>> But don't get me wrong, the CLI is one of the things that makes notmuch
>> so incredibly awesome.  It's an email swiss army knife that's there when
>> you need it.  But given that even I often need to look at the man page
>> during my occasional CLI usage, I just don't want to see it get to
>> overcrowded.
>
> Well, maybe this should be a discussion about how to organize the CLI
> documentation so that more commonly used options are easy to find. That
> would indeed be a side effect of making less commonly used options
> controlled by environment variables, and documenting the environment
> variables at the bottom of the man pages as per tradition. But I don't
> think this is the only way or even the best way to achieve good
> documentation.

I think one of the problems for documentation is that we have two
classes of "user" of the cli: one is actually people and one is front
ends (and yes scripting does blur the boundary). Would it be worth
splitting based on that. For example, the --format-version is really
only for front-end use. --format=text is for human use, whereas the
other options are mostly front-end (text0 might be an exception)

Best wishes

Mark

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/6] cli: keep track of whether the config is newly created
  2013-01-29 21:46 ` [PATCH 1/6] cli: keep track of whether the config is newly created Jani Nikula
@ 2013-03-03 16:13   ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2013-03-03 16:13 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> Add notmuch_config_is_new() accessor function to check this.
> ---

This patch is pretty trivial, but it would be nice to know why it is
needed.

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/6] cli: make notmuch_config_open() "is new" parameter input only
  2013-01-29 21:46 ` [PATCH 2/6] cli: make notmuch_config_open() "is new" parameter input only Jani Nikula
@ 2013-03-03 16:17   ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2013-03-03 16:17 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> Use the notmuch_config_is_new() function instead.

I think it would be worth describing the API change here. 

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/6] cli: abstract subcommand finding into a new function
  2013-01-29 21:46 ` [PATCH 3/6] cli: abstract subcommand finding into a new function Jani Nikula
@ 2013-03-03 16:23   ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2013-03-03 16:23 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> Clean up code.
> ---
>  notmuch.c |   68 +++++++++++++++++++++++++++++++------------------------------
>  1 file changed, 35 insertions(+), 33 deletions(-)

This looks OK, and independent from the other patches in the series, but
it will need to be rebased for changes in calling talloc_report
(d0370409)

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/6] cli: plug main notmuch command into subcommand machinery
  2013-01-29 21:46 ` [PATCH 4/6] cli: plug main notmuch command into subcommand machinery Jani Nikula
@ 2013-03-03 16:28   ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2013-03-03 16:28 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> This also allows the main notmuch command to have arguments (though
> they are not used).

If the motivation here is to actually add arguments (i.e. --config), I
would say so here.

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] cli: add top level --config=FILE option
  2013-01-29 21:46 ` [PATCH 6/6] cli: add top level --config=FILE option Jani Nikula
@ 2013-03-03 16:32   ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2013-03-03 16:32 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> Let the user specify the config file on the command line.
>  
> -    config = notmuch_config_open (local, NULL, command->create_config);
> +    config = notmuch_config_open (local, config_file_name, command->create_config);
>      if (!config)
>  	return 1;

LGTM

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] notmuch cli config changes
  2013-02-12 20:06           ` Jani Nikula
@ 2013-03-03 16:36             ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2013-03-03 16:36 UTC (permalink / raw)
  To: Jani Nikula, Jameson Graef Rollins, notmuch

Jani Nikula <jani@nikula.org> writes:
>
> Heh, I just wanted to see what the response was first before putting in
> the effort to document it. The code comes easy. :)
>

It turns out this series is actually needed (or would be useful) to fix
a bug in the emacs interface; see id:874nguxbvq.fsf@tu-dortmund.de

So I'm inclined to push an updated version.

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2013-03-03 16:36 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-29 21:46 [PATCH 0/6] notmuch cli config changes Jani Nikula
2013-01-29 21:46 ` [PATCH 1/6] cli: keep track of whether the config is newly created Jani Nikula
2013-03-03 16:13   ` David Bremner
2013-01-29 21:46 ` [PATCH 2/6] cli: make notmuch_config_open() "is new" parameter input only Jani Nikula
2013-03-03 16:17   ` David Bremner
2013-01-29 21:46 ` [PATCH 3/6] cli: abstract subcommand finding into a new function Jani Nikula
2013-03-03 16:23   ` David Bremner
2013-01-29 21:46 ` [PATCH 4/6] cli: plug main notmuch command into subcommand machinery Jani Nikula
2013-03-03 16:28   ` David Bremner
2013-01-29 21:46 ` [PATCH 5/6] cli: move config open/close to main() from subcommands Jani Nikula
2013-01-29 21:46 ` [PATCH 6/6] cli: add top level --config=FILE option Jani Nikula
2013-03-03 16:32   ` David Bremner
2013-02-06 17:50 ` [PATCH 0/6] notmuch cli config changes Jameson Graef Rollins
2013-02-07  7:43   ` Jani Nikula
2013-02-07 12:07   ` David Bremner
2013-02-12  7:50     ` Jameson Graef Rollins
2013-02-12 11:42       ` David Bremner
2013-02-12 16:54         ` Jameson Graef Rollins
2013-02-12 18:53           ` David Bremner
2013-02-13  7:51             ` Mark Walters
2013-02-12 20:06           ` Jani Nikula
2013-03-03 16:36             ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).