[PATCH 26/28] CLI/config: remove calls to notmuch_config_open from top level

[David Bremner <david@tethera.net>]

This will allow simplifying the subcommand interface.

Change the internal API to notmuch_config_open to not tie it to the
implementation of subcommands in notmuch.c.

It also fixes a previously broken test, since notmuch_config_open does
not understand the notion of the empty string as a config file name.
---
 notmuch-client.h         |  3 +--
 notmuch-config.c         | 28 +++++++++++++++++-----------
 notmuch-setup.c          | 11 ++++++++++-
 notmuch.c                | 40 ++++++++++++++++++----------------------
 test/T055-path-config.sh |  1 -
 5 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index a1d8dfe9..a5b3c518 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -267,7 +267,6 @@ json_quote_str (const void *ctx, const char *str);
 /* notmuch-config.c */
 
 typedef enum {
-    NOTMUCH_COMMAND_CONFIG_OPEN		= 1 << 0,
     NOTMUCH_COMMAND_CONFIG_CREATE	= 1 << 1,
     NOTMUCH_COMMAND_DATABASE_EARLY	= 1 << 2,
     NOTMUCH_COMMAND_DATABASE_WRITE	= 1 << 3,
@@ -278,7 +277,7 @@ typedef enum {
 notmuch_config_t *
 notmuch_config_open (notmuch_database_t *notmuch,
 		     const char *filename,
-		     notmuch_command_mode_t config_mode);
+		     bool create);
 
 void
 notmuch_config_close (notmuch_config_t *config);
diff --git a/notmuch-config.c b/notmuch-config.c
index c4283576..7aac8e94 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -248,7 +248,7 @@ get_config_from_file (notmuch_config_t *config, bool create_new)
 notmuch_config_t *
 notmuch_config_open (notmuch_database_t *notmuch,
 		     const char *filename,
-		     notmuch_command_mode_t config_mode)
+		     bool create)
 {
     char *notmuch_config_env = NULL;
 
@@ -272,13 +272,9 @@ notmuch_config_open (notmuch_database_t *notmuch,
 
     config->key_file = g_key_file_new ();
 
-    if (config_mode & NOTMUCH_COMMAND_CONFIG_OPEN) {
-	bool create_new = (config_mode & NOTMUCH_COMMAND_CONFIG_CREATE) != 0;
-
-	if (! get_config_from_file (config, create_new)) {
-	    talloc_free (config);
-	    return NULL;
-	}
+    if (! get_config_from_file (config, create)) {
+	talloc_free (config);
+	return NULL;
     }
 
     if (config->is_new)
@@ -582,13 +578,14 @@ _set_db_config (notmuch_database_t *notmuch, const char *key, int argc, char **a
 }
 
 static int
-notmuch_config_command_set (notmuch_config_t *config, notmuch_database_t *notmuch,
+notmuch_config_command_set (unused(notmuch_config_t *config), notmuch_database_t *notmuch,
 			    int argc, char *argv[])
 {
     char *group, *key;
     config_key_info_t *key_info;
+    notmuch_config_t *config;
     bool update_database = false;
-    int opt_index;
+    int opt_index, ret;
     char *item;
 
     notmuch_opt_desc_t options[] = {
@@ -629,6 +626,11 @@ notmuch_config_command_set (notmuch_config_t *config, notmuch_database_t *notmuc
     if (_item_split (item, &group, &key))
 	return 1;
 
+    config = notmuch_config_open (notmuch,
+				  notmuch_config_path (notmuch), false);
+    if (! config)
+	return 1;
+
     /* With only the name of an item, we clear it from the
      * configuration file.
      *
@@ -649,7 +651,11 @@ notmuch_config_command_set (notmuch_config_t *config, notmuch_database_t *notmuc
 	break;
     }
 
-    return notmuch_config_save (config);
+    ret = notmuch_config_save (config);
+
+    notmuch_config_close (config);
+
+    return ret;
 }
 
 static
diff --git a/notmuch-setup.c b/notmuch-setup.c
index e9b81be8..221ce934 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -124,7 +124,7 @@ parse_tag_list (void *ctx, char *response)
 }
 
 int
-notmuch_setup_command (notmuch_config_t *config,
+notmuch_setup_command (unused(notmuch_config_t *config),
 		       notmuch_database_t *notmuch,
 		       int argc, char *argv[])
 {
@@ -132,6 +132,7 @@ notmuch_setup_command (notmuch_config_t *config,
     size_t response_size = 0;
     GPtrArray *other_emails;
     notmuch_config_values_t *new_tags, *search_exclude_tags, *emails;
+    notmuch_config_t *config;
 
 #define prompt(format, ...)                                     \
     do {                                                        \
@@ -151,6 +152,11 @@ notmuch_setup_command (notmuch_config_t *config,
 	fprintf (stderr, "Warning: ignoring --uuid=%s\n",
 		 notmuch_requested_db_uuid);
 
+    config = notmuch_config_open (notmuch,
+				  notmuch_config_path (notmuch), true);
+    if (! config)
+	return EXIT_FAILURE;
+
     if (notmuch_config_is_new (config))
 	welcome_message_pre_setup ();
 
@@ -232,6 +238,9 @@ notmuch_setup_command (notmuch_config_t *config,
     if (notmuch_config_save (config))
 	return EXIT_FAILURE;
 
+    if (config)
+	notmuch_config_close (config);
+
     if (notmuch_config_is_new (config))
 	welcome_message_post_setup ();
 
diff --git a/notmuch.c b/notmuch.c
index 61663908..757d0eae 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -141,11 +141,9 @@ notmuch_process_shared_indexing_options (notmuch_database_t *notmuch)
 
 
 static command_t commands[] = {
-    { NULL, notmuch_command, NOTMUCH_COMMAND_CONFIG_OPEN | NOTMUCH_COMMAND_CONFIG_CREATE
-      | NOTMUCH_COMMAND_CONFIG_LOAD,
+    { NULL, notmuch_command, NOTMUCH_COMMAND_CONFIG_CREATE | NOTMUCH_COMMAND_CONFIG_LOAD,
       "Notmuch main command." },
-    { "setup", notmuch_setup_command, NOTMUCH_COMMAND_CONFIG_OPEN | NOTMUCH_COMMAND_CONFIG_CREATE
-      | NOTMUCH_COMMAND_CONFIG_LOAD,
+    { "setup", notmuch_setup_command, NOTMUCH_COMMAND_CONFIG_CREATE | NOTMUCH_COMMAND_CONFIG_LOAD,
       "Interactively set up notmuch for first use." },
     { "new", notmuch_new_command,
       NOTMUCH_COMMAND_DATABASE_EARLY | NOTMUCH_COMMAND_DATABASE_WRITE |
@@ -177,7 +175,7 @@ static command_t commands[] = {
     { "reindex", notmuch_reindex_command, NOTMUCH_COMMAND_DATABASE_EARLY |
       NOTMUCH_COMMAND_DATABASE_WRITE,
       "Re-index all messages matching the search terms." },
-    { "config", notmuch_config_command, NOTMUCH_COMMAND_CONFIG_OPEN | NOTMUCH_COMMAND_CONFIG_LOAD,
+    { "config", notmuch_config_command, NOTMUCH_COMMAND_CONFIG_LOAD,
       "Get or set settings in the notmuch configuration file." },
 #if WITH_EMACS
     { "emacs-mua", NULL, 0,
@@ -376,15 +374,26 @@ notmuch_help_command (unused (notmuch_config_t *config), unused(notmuch_database
  * to be more clever about this in the future.
  */
 static int
-notmuch_command (notmuch_config_t *config,
+notmuch_command (unused(notmuch_config_t *config),
 		 notmuch_database_t *notmuch,
 		 unused(int argc), unused(char **argv))
 {
-    /* If the user has never configured notmuch, then run
+
+    const char *config_path;
+
+    /* If the user has not created a configuration file, 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))
-	return notmuch_setup_command (config, notmuch, 0, NULL);
+    config_path = notmuch_config_path (notmuch);
+    if (access (config_path, R_OK | F_OK) == -1) {
+	if (errno != ENOENT) {
+	    fprintf (stderr, "Error: %s config file access failed: %s\n", config_path,
+		     strerror (errno));
+	    return EXIT_FAILURE;
+	} else {
+	    return notmuch_setup_command (NULL, notmuch, 0, NULL);
+	}
+    }
 
     printf ("Notmuch is configured and appears to have a database. Excellent!\n\n"
 	    "At this point you can start exploring the functionality of notmuch by\n"
@@ -580,22 +589,9 @@ main (int argc, char *argv[])
 
     }
 
-    if (command->mode & NOTMUCH_COMMAND_CONFIG_OPEN) {
-	if (! config_file_name)
-	    config_file_name = notmuch_config_path (notmuch);
-
-	config = notmuch_config_open (notmuch, config_file_name, command->mode);
-	if (! config) {
-	    ret = EXIT_FAILURE;
-	    goto DONE;
-	}
-    }
     ret = (command->function)(config, notmuch, argc - opt_index, argv + opt_index);
 
   DONE:
-    if (config)
-	notmuch_config_close (config);
-
     talloc_report = getenv ("NOTMUCH_TALLOC_REPORT");
     if (talloc_report && strcmp (talloc_report, "") != 0) {
 	/* this relies on the previous call to
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index 535c41e9..2045a555 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -258,7 +258,6 @@ EOF
    case $config in
        XDG*)
 	   test_begin_subtest "Set shadowed config value in database ($config)"
-	   test_subtest_known_broken
 	   name=${RANDOM}
 	   value=${RANDOM}
 	   key=test${test_count}.${name}
-- 
2.30.2