[PATCH v2 00/15] cli: argument parsing changes

[PATCH v2 00/15] cli: argument parsing changes

[Jani Nikula]

This series combines the designated initializers for argument parsing
from id:20170930213239.15392-1-jani@nikula.org and the argument parsing
refactoring from id:cover.1505853159.git.jani@nikula.org.

Additionally patch 1 handles some const confusion in notmuch-insert
before it becomes a problem in patch 2, and patches 5-7 add support for
the .present field to opt desc discussed in
id:87lgkvrhpa.fsf@nikula.org. Some more tests are sprinkled here and
there too.

BR,
Jani.


Jani Nikula (15):
  cli: strip trailing "/" from the final maildir path in notmuch insert
  cli: use designated initializers for opt desc
  test: add boolean argument to arg-test
  test: add opt_inherit to arg-test
  cli: add .present field to opt desc to check if the arg was present
  test: expand argument parsing tests
  cli: use the arg parser .present feature to handle show
    --entire-thread
  hex-xcode: use notmuch_bool_t for boolean arguments
  cli: use notmuch_bool_t for boolean argument in show
  cli: refactor boolean argument processing
  cli: change while to for in keyword argument processing
  cli: reduce indent in keyword argument processing
  cli: add support for --no- prefixed boolean and keyword flag arguments
  cli: use the negating boolean support for new and insert --no-hooks
  test: expand argument parsing sanity checks

 command-line-arguments.c      | 174 ++++++++++++++++++++++++++----------------
 command-line-arguments.h      |  41 ++++------
 notmuch-client.h              |   2 +-
 notmuch-compact.c             |   8 +-
 notmuch-count.c               |  16 ++--
 notmuch-dump.c                |  14 ++--
 notmuch-insert.c              |  48 ++++++------
 notmuch-new.c                 |  18 ++---
 notmuch-reindex.c             |   4 +-
 notmuch-reply.c               |  12 +--
 notmuch-restore.c             |  14 ++--
 notmuch-search.c              |  46 +++++------
 notmuch-show.c                |  41 +++++-----
 notmuch-tag.c                 |  12 +--
 notmuch.c                     |  22 +++---
 test/T410-argument-parsing.sh |  53 ++++++++++++-
 test/arg-test.c               |  56 +++++++++-----
 test/hex-xcode.c              |  12 +--
 test/random-corpus.c          |  20 ++---
 19 files changed, 350 insertions(+), 263 deletions(-)

-- 
2.11.0

[PATCH v2 01/15] cli: strip trailing "/" from the final maildir path in notmuch insert

[Jani Nikula]

Several subtle interconnected changes here:

- If the folder name passed as argument is the empty string "" or
  slash "/", the final maildir path would end up having "//" in it. We
  should strip the final maildir path, not folder.

- The folder variable should really be const char *, another reason
  not to modify it.

- The maildir variable is only const to let us point it at db_path
  directly.

To be able to strip the maildir variable, always allocate it. Default
folder to the empty string "", and don't treat folder not being
present on the command line as anything special.

As a side effect, we also create the cur/new/tmp in the top level
directory if they're not there and --create-folder is given.
---
 notmuch-insert.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 648bd944a7b1..040b6aa0de3b 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -452,12 +452,12 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     size_t new_tags_length;
     tag_op_list_t *tag_ops;
     char *query_string = NULL;
-    char *folder = NULL;
+    const char *folder = "";
     notmuch_bool_t create_folder = FALSE;
     notmuch_bool_t keep = FALSE;
     notmuch_bool_t no_hooks = FALSE;
     notmuch_bool_t synchronize_flags;
-    const char *maildir;
+    char *maildir;
     char *newpath;
     int opt_index;
     unsigned int i;
@@ -509,23 +509,21 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    if (folder == NULL) {
-	maildir = db_path;
-    } else {
-	strip_trailing (folder, '/');
-	if (! is_valid_folder_name (folder)) {
-	    fprintf (stderr, "Error: invalid folder name: '%s'\n", folder);
-	    return EXIT_FAILURE;
-	}
-	maildir = talloc_asprintf (config, "%s/%s", db_path, folder);
-	if (! maildir) {
-	    fprintf (stderr, "Out of memory\n");
-	    return EXIT_FAILURE;
-	}
-	if (create_folder && ! maildir_create_folder (config, maildir))
-	    return EXIT_FAILURE;
+    if (! is_valid_folder_name (folder)) {
+	fprintf (stderr, "Error: invalid folder name: '%s'\n", folder);
+	return EXIT_FAILURE;
+    }
+
+    maildir = talloc_asprintf (config, "%s/%s", db_path, folder);
+    if (! maildir) {
+	fprintf (stderr, "Out of memory\n");
+	return EXIT_FAILURE;
     }
 
+    strip_trailing (maildir, '/');
+    if (create_folder && ! maildir_create_folder (config, maildir))
+	return EXIT_FAILURE;
+
     /* Set up our handler for SIGINT. We do not set SA_RESTART so that copying
      * from standard input may be interrupted. */
     memset (&action, 0, sizeof (struct sigaction));
-- 
2.11.0

[PATCH v2 02/15] cli: use designated initializers for opt desc

[Jani Nikula]

Several changes at once, just to not have to change the same lines
several times over:

- Use designated initializers to initialize opt desc arrays.

- Only initialize the needed fields.

- Remove arg_id (short options) as unused.

- Replace opt_type and output_var with several type safe output
  variables, where the output variable being non-NULL determines the
  type. Introduce checks to ensure only one is set. The downside is
  some waste of const space per argument; this could be saved by
  retaining opt_type and using a union, but that's still pretty
  verbose.

- Fix some variables due to the type safety. Mostly a good thing, but
  leads to some enums being changed to ints. This is pedantically
  correct, but somewhat annoying. We could also cast, but that defeats
  the purpose a bit.

- Terminate the opt desc arrays using {}.

The output variable type safety and the ability to add new fields for
just some output types or arguments are the big wins. For example, if
we wanted to add a variable to set when the argument is present, we
could do so for just the arguments that need it.

Beauty is in the eye of the beholder, but I think this looks nice when
defining the arguments, and reduces some of the verbosity we have
there.

---

v2: don't use unnamed struct in notmuch_opt_desc_t to cater for old
compilers
---
 command-line-arguments.c | 87 ++++++++++++++++++++++++++----------------------
 command-line-arguments.h | 38 ++++++++-------------
 notmuch-client.h         |  2 +-
 notmuch-compact.c        |  8 ++---
 notmuch-count.c          | 16 ++++-----
 notmuch-dump.c           | 14 ++++----
 notmuch-insert.c         | 12 +++----
 notmuch-new.c            | 12 +++----
 notmuch-reindex.c        |  4 +--
 notmuch-reply.c          | 12 +++----
 notmuch-restore.c        | 14 ++++----
 notmuch-search.c         | 46 ++++++++++++-------------
 notmuch-show.c           | 22 ++++++------
 notmuch-tag.c            | 12 +++----
 notmuch.c                | 22 ++++++------
 test/arg-test.c          | 21 ++++++------
 test/hex-xcode.c         | 10 +++---
 test/random-corpus.c     | 20 +++++------
 18 files changed, 185 insertions(+), 187 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index dc517b06ff60..f1a5b2324337 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -22,12 +22,10 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
 
     while (keywords->name) {
 	if (strcmp (arg_str, keywords->name) == 0) {
-	    if (arg_desc->output_var) {
-		if (arg_desc->opt_type == NOTMUCH_OPT_KEYWORD_FLAGS)
-		    *((int *)arg_desc->output_var) |= keywords->value;
-		else
-		    *((int *)arg_desc->output_var) = keywords->value;
-	    }
+	    if (arg_desc->opt_flags)
+		*arg_desc->opt_flags |= keywords->value;
+	    else
+		*arg_desc->opt_keyword = keywords->value;
 	    return TRUE;
 	}
 	keywords++;
@@ -43,15 +41,15 @@ static notmuch_bool_t
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
 
     if (next == '\0') {
-	*((notmuch_bool_t *)arg_desc->output_var) = TRUE;
+	*arg_desc->opt_bool = TRUE;
 	return TRUE;
     }
     if (strcmp (arg_str, "false") == 0) {
-	*((notmuch_bool_t *)arg_desc->output_var) = FALSE;
+	*arg_desc->opt_bool = FALSE;
 	return TRUE;
     }
     if (strcmp (arg_str, "true") == 0) {
-	*((notmuch_bool_t *)arg_desc->output_var) = TRUE;
+	*arg_desc->opt_bool = TRUE;
 	return TRUE;
     }
     fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", arg_str, arg_desc->name);
@@ -67,7 +65,7 @@ _process_int_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg
 	return FALSE;
     }
 
-    *((int *)arg_desc->output_var) = strtol (arg_str, &endptr, 10);
+    *arg_desc->opt_int = strtol (arg_str, &endptr, 10);
     if (*endptr == '\0')
 	return TRUE;
 
@@ -87,10 +85,35 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *
 	fprintf (stderr, "String argument for option \"%s\" must be non-empty.\n", arg_desc->name);
 	return FALSE;
     }
-    *((const char **)arg_desc->output_var) = arg_str;
+    *arg_desc->opt_string = arg_str;
     return TRUE;
 }
 
+/* Return number of non-NULL opt_* fields in opt_desc. */
+static int _opt_set_count (const notmuch_opt_desc_t *opt_desc)
+{
+    return
+	!!opt_desc->opt_inherit +
+	!!opt_desc->opt_bool +
+	!!opt_desc->opt_int +
+	!!opt_desc->opt_keyword +
+	!!opt_desc->opt_flags +
+	!!opt_desc->opt_string +
+	!!opt_desc->opt_position;
+}
+
+/* Return TRUE if opt_desc is valid. */
+static notmuch_bool_t _opt_valid (const notmuch_opt_desc_t *opt_desc)
+{
+    int n = _opt_set_count (opt_desc);
+
+    if (n > 1)
+	INTERNAL_ERROR ("more than one non-NULL opt_* field for argument \"%s\"",
+			opt_desc->name);
+
+    return n > 0;
+}
+
 /*
    Search for the {pos_arg_index}th position argument, return FALSE if
    that does not exist.
@@ -101,12 +124,10 @@ parse_position_arg (const char *arg_str, int pos_arg_index,
 		    const notmuch_opt_desc_t *arg_desc) {
 
     int pos_arg_counter = 0;
-    while (arg_desc->opt_type != NOTMUCH_OPT_END){
-	if (arg_desc->opt_type == NOTMUCH_OPT_POSITION) {
+    while (_opt_valid (arg_desc)) {
+	if (arg_desc->opt_position) {
 	    if (pos_arg_counter == pos_arg_index) {
-		if (arg_desc->output_var) {
-		    *((const char **)arg_desc->output_var) = arg_str;
-		}
+		*arg_desc->opt_position = arg_str;
 		return TRUE;
 	    }
 	    pos_arg_counter++;
@@ -138,9 +159,9 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
     if (opt_index < argc - 1  && strncmp (argv[opt_index + 1], "--", 2) != 0)
 	next_arg = argv[opt_index + 1];
 
-    for (try = options; try->opt_type != NOTMUCH_OPT_END; try++) {
-	if (try->opt_type == NOTMUCH_OPT_INHERIT) {
-	    int new_index = parse_option (argc, argv, try->output_var, opt_index);
+    for (try = options; _opt_valid (try); try++) {
+	if (try->opt_inherit) {
+	    int new_index = parse_option (argc, argv, try->opt_inherit, opt_index);
 	    if (new_index >= 0)
 		return new_index;
 	}
@@ -163,36 +184,24 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
 	if (next != '=' && next != ':' && next != '\0')
 	    continue;
 
-	if (next == '\0' && next_arg != NULL && try->opt_type != NOTMUCH_OPT_BOOLEAN) {
+	if (next == '\0' && next_arg != NULL && ! try->opt_bool) {
 	    next = ' ';
 	    value = next_arg;
 	    opt_index ++;
 	}
 
-	if (try->output_var == NULL)
-	    INTERNAL_ERROR ("output pointer NULL for option %s", try->name);
-
 	notmuch_bool_t opt_status = FALSE;
-	switch (try->opt_type) {
-	case NOTMUCH_OPT_KEYWORD:
-	case NOTMUCH_OPT_KEYWORD_FLAGS:
+	if (try->opt_keyword || try->opt_flags)
 	    opt_status = _process_keyword_arg (try, next, value);
-	    break;
-	case NOTMUCH_OPT_BOOLEAN:
+	else if (try->opt_bool)
 	    opt_status = _process_boolean_arg (try, next, value);
-	    break;
-	case NOTMUCH_OPT_INT:
+	else if (try->opt_int)
 	    opt_status = _process_int_arg (try, next, value);
-	    break;
-	case NOTMUCH_OPT_STRING:
+	else if (try->opt_string)
 	    opt_status = _process_string_arg (try, next, value);
-	    break;
-	case NOTMUCH_OPT_POSITION:
-	case NOTMUCH_OPT_END:
-	default:
-	    INTERNAL_ERROR ("unknown or unhandled option type %d", try->opt_type);
-	    /*UNREACHED*/
-	}
+	else
+	    INTERNAL_ERROR ("unknown or unhandled option \"%s\"", try->name);
+
 	if (opt_status)
 	    return opt_index+1;
 	else
diff --git a/command-line-arguments.h b/command-line-arguments.h
index 4c4d240e10dc..ff51abceb117 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -3,17 +3,6 @@
 
 #include "notmuch.h"
 
-enum notmuch_opt_type {
-    NOTMUCH_OPT_END = 0,
-    NOTMUCH_OPT_INHERIT,	/* another options table */
-    NOTMUCH_OPT_BOOLEAN,	/* --verbose              */
-    NOTMUCH_OPT_INT,		/* --frob=8               */
-    NOTMUCH_OPT_KEYWORD,	/* --format=raw|json|text */
-    NOTMUCH_OPT_KEYWORD_FLAGS,	/* the above with values OR'd together */
-    NOTMUCH_OPT_STRING,		/* --file=/tmp/gnarf.txt  */
-    NOTMUCH_OPT_POSITION	/* notmuch dump pos_arg   */
-};
-
 /*
  * Describe one of the possibilities for a keyword option
  * 'value' will be copied to the output variable
@@ -24,22 +13,21 @@ typedef struct notmuch_keyword {
     int value;
 } notmuch_keyword_t;
 
-/*
- * Describe one option.
- *
- * First two parameters are mandatory.
- *
- * name is mandatory _except_ for positional arguments.
- *
- * arg_id is currently unused, but could define short arguments.
- *
- * keywords is a (possibly NULL) pointer to an array of keywords
- */
+/* Describe one option. */
 typedef struct notmuch_opt_desc {
-    enum notmuch_opt_type opt_type;
-    void *output_var;
+    /* One and only one of opt_* must be set. */
+    const struct notmuch_opt_desc *opt_inherit;
+    notmuch_bool_t *opt_bool;
+    int *opt_int;
+    int *opt_keyword;
+    int *opt_flags;
+    const char **opt_string;
+    const char **opt_position;
+
+    /* Must be set except for opt_inherit and opt_position. */
     const char *name;
-    int  arg_id;
+
+    /* Must be set for opt_keyword and opt_flags. */
     const struct notmuch_keyword *keywords;
 } notmuch_opt_desc_t;
 
diff --git a/notmuch-client.h b/notmuch-client.h
index 9d0f367d5e4e..c68538fcc0a2 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -508,7 +508,7 @@ status_to_exit (notmuch_status_t status);
 
 #include "command-line-arguments.h"
 
-extern char *notmuch_requested_db_uuid;
+extern const char *notmuch_requested_db_uuid;
 extern const notmuch_opt_desc_t  notmuch_shared_options [];
 void notmuch_exit_if_unmatched_db_uuid (notmuch_database_t *notmuch);
 
diff --git a/notmuch-compact.c b/notmuch-compact.c
index 855545d73576..ae464e4805cb 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -36,10 +36,10 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_STRING, &backup_path, "backup", 0, 0 },
-	{ NOTMUCH_OPT_BOOLEAN,  &quiet, "quiet", 'q', 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0}
+	{ .opt_string = &backup_path, .name = "backup" },
+	{ .opt_bool =  &quiet, .name = "quiet" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-count.c b/notmuch-count.c
index 97281374b222..b8b03cdbc0d4 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -166,24 +166,24 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_bool_t batch = FALSE;
     notmuch_bool_t print_lastmod = FALSE;
     FILE *input = stdin;
-    char *input_file_name = NULL;
+    const char *input_file_name = NULL;
     int ret;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
+	{ .opt_keyword = &output, .name = "output", .keywords =
 	  (notmuch_keyword_t []){ { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
 				  { "files", OUTPUT_FILES },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
+	{ .opt_keyword = &exclude, .name = "exclude", .keywords =
 	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
 				  { "false", EXCLUDE_FALSE },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_BOOLEAN, &print_lastmod, "lastmod", 'l', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &batch, "batch", 0, 0 },
-	{ NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_bool = &print_lastmod, .name = "lastmod" },
+	{ .opt_bool = &batch, .name = "batch" },
+	{ .opt_string = &input_file_name, .name = "input" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 5cc3b2f62b46..03e64d608c85 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -369,7 +369,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_exit_if_unmatched_db_uuid (notmuch);
 
-    char *output_file_name = NULL;
+    const char *output_file_name = NULL;
     int opt_index;
 
     int output_format = DUMP_FORMAT_BATCH_TAG;
@@ -377,18 +377,18 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_bool_t gzip_output = 0;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &output_format, "format", 'f',
+	{ .opt_keyword = &output_format, .name = "format", .keywords =
 	  (notmuch_keyword_t []){ { "sup", DUMP_FORMAT_SUP },
 				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD_FLAGS, &include, "include", 'I',
+	{ .opt_flags = &include, .name = "include", .keywords =
 	  (notmuch_keyword_t []){ { "config", DUMP_INCLUDE_CONFIG },
 				  { "properties", DUMP_INCLUDE_PROPERTIES },
 				  { "tags", DUMP_INCLUDE_TAGS} } },
-	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
-	{ NOTMUCH_OPT_BOOLEAN, &gzip_output, "gzip", 'z', 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_string = &output_file_name, .name = "output" },
+	{ .opt_bool = &gzip_output, .name = "gzip" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 040b6aa0de3b..bbbc29ea103d 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -463,12 +463,12 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     unsigned int i;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_STRING, &folder, "folder", 0, 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &create_folder, "create-folder", 0, 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &keep, "keep", 0, 0 },
-	{ NOTMUCH_OPT_BOOLEAN,  &no_hooks, "no-hooks", 'n', 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ NOTMUCH_OPT_END, 0, 0, 0, 0 }
+	{ .opt_string = &folder, .name = "folder" },
+	{ .opt_bool = &create_folder, .name = "create-folder" },
+	{ .opt_bool = &keep, .name = "keep" },
+	{ .opt_bool =  &no_hooks, .name = "no-hooks" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-new.c b/notmuch-new.c
index faeb8f0a5896..342e2189d5d3 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -953,12 +953,12 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_status_t status;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_BOOLEAN,  &quiet, "quiet", 'q', 0 },
-	{ NOTMUCH_OPT_BOOLEAN,  &verbose, "verbose", 'v', 0 },
-	{ NOTMUCH_OPT_BOOLEAN,  &add_files_state.debug, "debug", 'd', 0 },
-	{ NOTMUCH_OPT_BOOLEAN,  &no_hooks, "no-hooks", 'n', 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_bool = &quiet, .name = "quiet" },
+	{ .opt_bool = &verbose, .name = "verbose" },
+	{ .opt_bool = &add_files_state.debug, .name = "debug" },
+	{ .opt_bool = &no_hooks, .name = "no-hooks" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-reindex.c b/notmuch-reindex.c
index bceac7228464..57ff59040153 100644
--- a/notmuch-reindex.c
+++ b/notmuch-reindex.c
@@ -99,8 +99,8 @@ notmuch_reindex_command (notmuch_config_t *config, int argc, char *argv[])
     sigaction (SIGINT, &action, NULL);
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 929f3077862f..e7ead79d755d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -705,20 +705,20 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
     int reply_all = TRUE;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
+	{ .opt_keyword = &format, .name = "format", .keywords =
 	  (notmuch_keyword_t []){ { "default", FORMAT_DEFAULT },
 				  { "json", FORMAT_JSON },
 				  { "sexp", FORMAT_SEXP },
 				  { "headers-only", FORMAT_HEADERS_ONLY },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
-	{ NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
+	{ .opt_int = &notmuch_format_version, .name = "format-version" },
+	{ .opt_keyword = &reply_all, .name = "reply-to", .keywords =
 	  (notmuch_keyword_t []){ { "all", TRUE },
 				  { "sender", FALSE },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_bool = &params.crypto.decrypt, .name = "decrypt" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-restore.c b/notmuch-restore.c
index d6429efb4045..0025e2c316be 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -227,7 +227,7 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
     tag_op_flag_t flags = 0;
     tag_op_list_t *tag_ops;
 
-    char *input_file_name = NULL;
+    const char *input_file_name = NULL;
     const char *name_for_error = NULL;
     gzFile input = NULL;
     char *line = NULL;
@@ -247,20 +247,20 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
 	flags |= TAG_FLAG_MAILDIR_SYNC;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &input_format, "format", 'f',
+	{ .opt_keyword = &input_format, .name = "format", .keywords =
 	  (notmuch_keyword_t []){ { "auto", DUMP_FORMAT_AUTO },
 				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
 				  { "sup", DUMP_FORMAT_SUP },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD_FLAGS, &include, "include", 'I',
+	{ .opt_flags = &include, .name = "include", .keywords =
 	  (notmuch_keyword_t []){ { "config", DUMP_INCLUDE_CONFIG },
 				  { "properties", DUMP_INCLUDE_PROPERTIES },
 				  { "tags", DUMP_INCLUDE_TAGS} } },
 
-	{ NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
-	{ NOTMUCH_OPT_BOOLEAN,  &accumulate, "accumulate", 'a', 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_string = &input_file_name, .name = "input" },
+	{ .opt_bool = &accumulate, .name = "accumulate" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-search.c b/notmuch-search.c
index 380e9d8fbd8f..2ea658d325d6 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -51,17 +51,17 @@ typedef enum {
 
 typedef struct {
     notmuch_database_t *notmuch;
-    format_sel_t format_sel;
+    int format_sel;
     sprinter_t *format;
-    notmuch_exclude_t exclude;
+    int exclude;
     notmuch_query_t *query;
-    notmuch_sort_t sort;
-    output_t output;
+    int sort;
+    int output;
     int offset;
     int limit;
     int dupe;
     GHashTable *addresses;
-    dedup_t dedup;
+    int dedup;
 } search_context_t;
 
 typedef struct {
@@ -786,18 +786,18 @@ static search_context_t search_context = {
 };
 
 static const notmuch_opt_desc_t common_options[] = {
-    { NOTMUCH_OPT_KEYWORD, &search_context.sort, "sort", 's',
+    { .opt_keyword = &search_context.sort, .name = "sort", .keywords =
       (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
 			      { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
 			      { 0, 0 } } },
-    { NOTMUCH_OPT_KEYWORD, &search_context.format_sel, "format", 'f',
+    { .opt_keyword = &search_context.format_sel, .name = "format", .keywords =
       (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
 			      { "sexp", NOTMUCH_FORMAT_SEXP },
 			      { "text", NOTMUCH_FORMAT_TEXT },
 			      { "text0", NOTMUCH_FORMAT_TEXT0 },
 			      { 0, 0 } } },
-    { NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
-    { 0, 0, 0, 0, 0 }
+    { .opt_int = &notmuch_format_version, .name = "format-version" },
+    { }
 };
 
 int
@@ -807,25 +807,25 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index, ret;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &ctx->output, "output", 'o',
+	{ .opt_keyword = &ctx->output, .name = "output", .keywords =
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
 				  { "files", OUTPUT_FILES },
 				  { "tags", OUTPUT_TAGS },
 				  { 0, 0 } } },
-        { NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x',
+        { .opt_keyword = &ctx->exclude, .name = "exclude", .keywords =
           (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
                                   { "false", NOTMUCH_EXCLUDE_FALSE },
                                   { "flag", NOTMUCH_EXCLUDE_FLAG },
                                   { "all", NOTMUCH_EXCLUDE_ALL },
                                   { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &ctx->offset, "offset", 'O', 0 },
-	{ NOTMUCH_OPT_INT, &ctx->limit, "limit", 'L', 0  },
-	{ NOTMUCH_OPT_INT, &ctx->dupe, "duplicate", 'D', 0  },
-	{ NOTMUCH_OPT_INHERIT, (void *) &common_options, NULL, 0, 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_int = &ctx->offset, .name = "offset" },
+	{ .opt_int = &ctx->limit, .name = "limit" },
+	{ .opt_int = &ctx->dupe, .name = "duplicate" },
+	{ .opt_inherit = common_options },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     ctx->output = OUTPUT_SUMMARY;
@@ -873,23 +873,23 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index, ret;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
+	{ .opt_flags = &ctx->output, .name = "output", .keywords =
 	  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
 				  { "recipients", OUTPUT_RECIPIENTS },
 				  { "count", OUTPUT_COUNT },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x',
+	{ .opt_keyword = &ctx->exclude, .name = "exclude", .keywords =
 	  (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
 				  { "false", NOTMUCH_EXCLUDE_FALSE },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD, &ctx->dedup, "deduplicate", 'D',
+	{ .opt_keyword = &ctx->dedup, .name = "deduplicate", .keywords =
 	  (notmuch_keyword_t []){ { "no", DEDUP_NONE },
 				  { "mailbox", DEDUP_MAILBOX },
 				  { "address", DEDUP_ADDRESS },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_INHERIT, (void *) &common_options, NULL, 0, 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_inherit = common_options },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-show.c b/notmuch-show.c
index cdcc2a982bd9..367536ff9532 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1093,23 +1093,23 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_bool_t single_message;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
+	{ .opt_keyword = &format, .name = "format", .keywords =
 	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
 				  { "text", NOTMUCH_FORMAT_TEXT },
 				  { "sexp", NOTMUCH_FORMAT_SEXP },
 				  { "mbox", NOTMUCH_FORMAT_MBOX },
 				  { "raw", NOTMUCH_FORMAT_RAW },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &exclude, "exclude", 'x', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &entire_thread, "entire-thread", 't', 0 },
-	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.verify, "verify", 'v', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &params.output_body, "body", 'b', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &params.include_html, "include-html", 0, 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_int = &notmuch_format_version, .name = "format-version" },
+	{ .opt_bool = &exclude, .name = "exclude" },
+	{ .opt_bool = &entire_thread, .name = "entire-thread" },
+	{ .opt_int = &params.part, .name = "part" },
+	{ .opt_bool = &params.crypto.decrypt, .name = "decrypt" },
+	{ .opt_bool = &params.crypto.verify, .name = "verify" },
+	{ .opt_bool = &params.output_body, .name = "body" },
+	{ .opt_bool = &params.include_html, .name = "include-html" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 130de6343f8e..630efa65399c 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -197,7 +197,7 @@ notmuch_tag_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_bool_t batch = FALSE;
     notmuch_bool_t remove_all = FALSE;
     FILE *input = stdin;
-    char *input_file_name = NULL;
+    const char *input_file_name = NULL;
     int opt_index;
     int ret;
 
@@ -209,11 +209,11 @@ notmuch_tag_command (notmuch_config_t *config, int argc, char *argv[])
     sigaction (SIGINT, &action, NULL);
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_BOOLEAN, &batch, "batch", 0, 0 },
-	{ NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &remove_all, "remove-all", 0, 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_bool = &batch, .name = "batch" },
+	{ .opt_string = &input_file_name, .name = "input" },
+	{ .opt_bool = &remove_all, .name = "remove-all" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/notmuch.c b/notmuch.c
index 201c7454ee73..cc9c34aefb30 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -47,13 +47,13 @@ static int
 _help_for (const char *topic);
 
 static notmuch_bool_t print_version = FALSE, print_help = FALSE;
-char *notmuch_requested_db_uuid = NULL;
+const char *notmuch_requested_db_uuid = NULL;
 
 const notmuch_opt_desc_t notmuch_shared_options [] = {
-    { NOTMUCH_OPT_BOOLEAN, &print_version, "version", 'v', 0 },
-    { NOTMUCH_OPT_BOOLEAN, &print_help, "help", 'h', 0 },
-    { NOTMUCH_OPT_STRING, &notmuch_requested_db_uuid, "uuid", 'u', 0 },
-    {0, 0, 0, 0, 0}
+    { .opt_bool = &print_version, .name = "version" },
+    { .opt_bool = &print_help, .name = "help" },
+    { .opt_string = &notmuch_requested_db_uuid, .name = "uuid" },
+    { }
 };
 
 /* any subcommand wanting to support these options should call
@@ -82,8 +82,8 @@ int notmuch_minimal_options (const char *subcommand_name,
     int opt_index;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     opt_index = parse_arguments (argc, argv, options, 1);
@@ -405,15 +405,15 @@ main (int argc, char *argv[])
     char *talloc_report;
     const char *command_name = NULL;
     command_t *command;
-    char *config_file_name = NULL;
+    const char *config_file_name = NULL;
     notmuch_config_t *config = NULL;
     int opt_index;
     int ret;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_STRING, &config_file_name, "config", 'c', 0 },
-	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_string = &config_file_name, .name = "config" },
+	{ .opt_inherit = notmuch_shared_options },
+	{ }
     };
 
     talloc_enable_null_tracking ();
diff --git a/test/arg-test.c b/test/arg-test.c
index 736686ded2c0..10dc06834513 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -9,25 +9,26 @@ int main(int argc, char **argv){
     int kw_val=0;
     int fl_val=0;
     int int_val=0;
-    char *pos_arg1=NULL;
-    char *pos_arg2=NULL;
-    char *string_val=NULL;
+    const char *pos_arg1=NULL;
+    const char *pos_arg2=NULL;
+    const char *string_val=NULL;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &kw_val, "keyword", 'k',
+	{ .opt_keyword = &kw_val, .name = "keyword", .keywords =
 	  (notmuch_keyword_t []){ { "one", 1 },
 				  { "two", 2 },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD_FLAGS, &fl_val, "flag", 'f',
+	{ .opt_flags = &fl_val, .name = "flag", .keywords =
 	  (notmuch_keyword_t []){ { "one",   1 << 0},
 				  { "two",   1 << 1 },
 				  { "three", 1 << 2 },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &int_val, "int", 'i', 0},
-	{ NOTMUCH_OPT_STRING, &string_val, "string", 's', 0},
-	{ NOTMUCH_OPT_POSITION, &pos_arg1, 0,0, 0},
-	{ NOTMUCH_OPT_POSITION, &pos_arg2, 0,0, 0},
-	{ 0, 0, 0, 0, 0 } };
+	{ .opt_int = &int_val, .name = "int" },
+	{ .opt_string = &string_val, .name = "string" },
+	{ .opt_position = &pos_arg1 },
+	{ .opt_position = &pos_arg2 },
+	{ }
+    };
 
     opt_index = parse_arguments(argc, argv, options, 1);
 
diff --git a/test/hex-xcode.c b/test/hex-xcode.c
index 65d49564a3e1..bc2df713b2a3 100644
--- a/test/hex-xcode.c
+++ b/test/hex-xcode.c
@@ -44,17 +44,17 @@ int
 main (int argc, char **argv)
 {
 
-    enum direction dir = DECODE;
+    int dir = DECODE;
     int omit_newline = FALSE;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &dir, "direction", 'd',
+	{ .opt_keyword = &dir, .name = "direction", .keywords =
 	  (notmuch_keyword_t []){ { "encode", ENCODE },
 				  { "decode", DECODE },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_BOOLEAN, &omit_newline, "omit-newline", 'n', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &inplace, "in-place", 'i', 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_bool = &omit_newline, .name = "omit-newline" },
+	{ .opt_bool = &inplace, .name = "in-place" },
+	{ }
     };
 
     int opt_index = parse_arguments (argc, argv, options, 1);
diff --git a/test/random-corpus.c b/test/random-corpus.c
index aca694a3c1d3..e3b855e1efd8 100644
--- a/test/random-corpus.c
+++ b/test/random-corpus.c
@@ -116,10 +116,10 @@ random_utf8_string (void *ctx, size_t char_count)
 
 /* stubs since we cannot link with notmuch.o */
 const notmuch_opt_desc_t notmuch_shared_options[] = {
-	{ 0, 0, 0, 0, 0 }
+	{ }
 };
 
-char *notmuch_requested_db_uuid = NULL;
+const char *notmuch_requested_db_uuid = NULL;
 
 void
 notmuch_process_shared_options (unused (const char *dummy))
@@ -140,7 +140,7 @@ main (int argc, char **argv)
 
     void *ctx = talloc_new (NULL);
 
-    char *config_path  = NULL;
+    const char *config_path = NULL;
     notmuch_config_t *config;
     notmuch_database_t *notmuch;
 
@@ -155,13 +155,13 @@ main (int argc, char **argv)
     int seed = 734569;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_STRING, &config_path, "config-path", 'c', 0 },
-	{ NOTMUCH_OPT_INT, &num_messages, "num-messages", 'n', 0 },
-	{ NOTMUCH_OPT_INT, &max_tags, "max-tags", 'm', 0 },
-	{ NOTMUCH_OPT_INT, &message_id_len, "message-id-len", 'M', 0 },
-	{ NOTMUCH_OPT_INT, &tag_len, "tag-len", 't', 0 },
-	{ NOTMUCH_OPT_INT, &seed, "seed", 's', 0 },
-	{ 0, 0, 0, 0, 0 }
+	{ .opt_string = &config_path, .name = "config-path" },
+	{ .opt_int = &num_messages, .name = "num-messages" },
+	{ .opt_int = &max_tags, .name = "max-tags" },
+	{ .opt_int = &message_id_len, .name = "message-id-len" },
+	{ .opt_int = &tag_len, .name = "tag-len" },
+	{ .opt_int = &seed, .name = "seed" },
+	{ }
     };
 
     int opt_index = parse_arguments (argc, argv, options, 1);
-- 
2.11.0

[PATCH v2 03/15] test: add boolean argument to arg-test

[Jani Nikula]

Surprisingly it's not there.
---
 test/T410-argument-parsing.sh | 3 ++-
 test/arg-test.c               | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index fad134e305c5..4505c58301ea 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -3,8 +3,9 @@ test_description="argument parsing"
 . ./test-lib.sh || exit 1
 
 test_begin_subtest "sanity check"
-$TEST_DIRECTORY/arg-test  pos1  --keyword=one --string=foo pos2 --int=7 --flag=one --flag=three > OUTPUT
+$TEST_DIRECTORY/arg-test  pos1  --keyword=one --boolean --string=foo pos2 --int=7 --flag=one --flag=three > OUTPUT
 cat <<EOF > EXPECTED
+boolean 1
 keyword 1
 flags 5
 int 7
diff --git a/test/arg-test.c b/test/arg-test.c
index 10dc06834513..9d13618bd17c 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -12,8 +12,10 @@ int main(int argc, char **argv){
     const char *pos_arg1=NULL;
     const char *pos_arg2=NULL;
     const char *string_val=NULL;
+    notmuch_bool_t bool_val = FALSE;
 
     notmuch_opt_desc_t options[] = {
+	{ .opt_bool = &bool_val, .name = "boolean" },
 	{ .opt_keyword = &kw_val, .name = "keyword", .keywords =
 	  (notmuch_keyword_t []){ { "one", 1 },
 				  { "two", 2 },
@@ -35,6 +37,9 @@ int main(int argc, char **argv){
     if (opt_index < 0)
 	return 1;
 
+    if (bool_val)
+	printf("boolean %d\n", bool_val);
+
     if (kw_val)
 	printf("keyword %d\n", kw_val);
 
-- 
2.11.0

[PATCH v2 04/15] test: add opt_inherit to arg-test

[Jani Nikula]

Just split the arguments to two opt desc arrays.
---
 test/arg-test.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/test/arg-test.c b/test/arg-test.c
index 9d13618bd17c..a379f23e308a 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -14,18 +14,23 @@ int main(int argc, char **argv){
     const char *string_val=NULL;
     notmuch_bool_t bool_val = FALSE;
 
-    notmuch_opt_desc_t options[] = {
-	{ .opt_bool = &bool_val, .name = "boolean" },
-	{ .opt_keyword = &kw_val, .name = "keyword", .keywords =
-	  (notmuch_keyword_t []){ { "one", 1 },
-				  { "two", 2 },
-				  { 0, 0 } } },
+    notmuch_opt_desc_t parent_options[] = {
 	{ .opt_flags = &fl_val, .name = "flag", .keywords =
 	  (notmuch_keyword_t []){ { "one",   1 << 0},
 				  { "two",   1 << 1 },
 				  { "three", 1 << 2 },
 				  { 0, 0 } } },
 	{ .opt_int = &int_val, .name = "int" },
+	{ }
+    };
+
+    notmuch_opt_desc_t options[] = {
+	{ .opt_bool = &bool_val, .name = "boolean" },
+	{ .opt_keyword = &kw_val, .name = "keyword", .keywords =
+	  (notmuch_keyword_t []){ { "one", 1 },
+				  { "two", 2 },
+				  { 0, 0 } } },
+	{ .opt_inherit = parent_options },
 	{ .opt_string = &string_val, .name = "string" },
 	{ .opt_position = &pos_arg1 },
 	{ .opt_position = &pos_arg2 },
-- 
2.11.0

[PATCH v2 05/15] cli: add .present field to opt desc to check if the arg was present

[Jani Nikula]

Add pointer to boolean .present field to opt desc, which (if non-NULL)
will be set to TRUE if the argument in question is present on the
command line. Unchanged otherwise.
---
 command-line-arguments.c | 11 ++++++++---
 command-line-arguments.h |  3 +++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index f1a5b2324337..39940d5fb9fd 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -128,6 +128,8 @@ parse_position_arg (const char *arg_str, int pos_arg_index,
 	if (arg_desc->opt_position) {
 	    if (pos_arg_counter == pos_arg_index) {
 		*arg_desc->opt_position = arg_str;
+		if (arg_desc->present)
+		    *arg_desc->present = TRUE;
 		return TRUE;
 	    }
 	    pos_arg_counter++;
@@ -202,10 +204,13 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
 	else
 	    INTERNAL_ERROR ("unknown or unhandled option \"%s\"", try->name);
 
-	if (opt_status)
-	    return opt_index+1;
-	else
+	if (! opt_status)
 	    return -1;
+
+	if (try->present)
+	    *try->present = TRUE;
+
+	return opt_index+1;
     }
     return -1;
 }
diff --git a/command-line-arguments.h b/command-line-arguments.h
index ff51abceb117..dfc808bdab78 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -27,6 +27,9 @@ typedef struct notmuch_opt_desc {
     /* Must be set except for opt_inherit and opt_position. */
     const char *name;
 
+    /* Optional, if non-NULL, set to TRUE if the option is present. */
+    notmuch_bool_t *present;
+
     /* Must be set for opt_keyword and opt_flags. */
     const struct notmuch_keyword *keywords;
 } notmuch_opt_desc_t;
-- 
2.11.0

[PATCH v2 06/15] test: expand argument parsing tests

[Jani Nikula]

Test and use the new .present field, only output the parameters
given. Test space between parameter name and value.
---
 test/T410-argument-parsing.sh | 22 ++++++++++++++++++++++
 test/arg-test.c               | 33 ++++++++++++++++++---------------
 2 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 4505c58301ea..4a2b25c6486d 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -15,4 +15,26 @@ positional arg 2 pos2
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "sanity check zero values"
+$TEST_DIRECTORY/arg-test --keyword=zero --boolean=false --int=0 > OUTPUT
+cat <<EOF > EXPECTED
+boolean 0
+keyword 0
+int 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "space instead of = between parameter name and value"
+# Note: spaces aren't allowed for booleans. false turns into a positional arg!
+$TEST_DIRECTORY/arg-test --keyword one --boolean false --string foo --int 7 --flag one --flag three > OUTPUT
+cat <<EOF > EXPECTED
+boolean 1
+keyword 1
+flags 5
+int 7
+string foo
+positional arg 1 false
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
diff --git a/test/arg-test.c b/test/arg-test.c
index a379f23e308a..64751df4ada1 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -13,27 +13,30 @@ int main(int argc, char **argv){
     const char *pos_arg2=NULL;
     const char *string_val=NULL;
     notmuch_bool_t bool_val = FALSE;
+    notmuch_bool_t fl_set = FALSE, int_set = FALSE, bool_set = FALSE,
+	kw_set = FALSE, string_set = FALSE, pos1_set = FALSE, pos2_set = FALSE;
 
     notmuch_opt_desc_t parent_options[] = {
-	{ .opt_flags = &fl_val, .name = "flag", .keywords =
+	{ .opt_flags = &fl_val, .name = "flag", .present = &fl_set, .keywords =
 	  (notmuch_keyword_t []){ { "one",   1 << 0},
 				  { "two",   1 << 1 },
 				  { "three", 1 << 2 },
 				  { 0, 0 } } },
-	{ .opt_int = &int_val, .name = "int" },
+	{ .opt_int = &int_val, .name = "int", .present = &int_set },
 	{ }
     };
 
     notmuch_opt_desc_t options[] = {
-	{ .opt_bool = &bool_val, .name = "boolean" },
-	{ .opt_keyword = &kw_val, .name = "keyword", .keywords =
-	  (notmuch_keyword_t []){ { "one", 1 },
+	{ .opt_bool = &bool_val, .name = "boolean", .present = &bool_set },
+	{ .opt_keyword = &kw_val, .name = "keyword", .present = &kw_set, .keywords =
+	  (notmuch_keyword_t []){ { "zero", 0 },
+				  { "one", 1 },
 				  { "two", 2 },
 				  { 0, 0 } } },
 	{ .opt_inherit = parent_options },
-	{ .opt_string = &string_val, .name = "string" },
-	{ .opt_position = &pos_arg1 },
-	{ .opt_position = &pos_arg2 },
+	{ .opt_string = &string_val, .name = "string", .present = &string_set },
+	{ .opt_position = &pos_arg1, .present = &pos1_set },
+	{ .opt_position = &pos_arg2, .present = &pos2_set },
 	{ }
     };
 
@@ -42,25 +45,25 @@ int main(int argc, char **argv){
     if (opt_index < 0)
 	return 1;
 
-    if (bool_val)
+    if (bool_set)
 	printf("boolean %d\n", bool_val);
 
-    if (kw_val)
+    if (kw_set)
 	printf("keyword %d\n", kw_val);
 
-    if (fl_val)
+    if (fl_set)
 	printf("flags %d\n", fl_val);
 
-    if (int_val)
+    if (int_set)
 	printf("int %d\n", int_val);
 
-    if (string_val)
+    if (string_set)
 	printf("string %s\n", string_val);
 
-    if (pos_arg1)
+    if (pos1_set)
 	printf("positional arg 1 %s\n", pos_arg1);
 
-    if (pos_arg2)
+    if (pos2_set)
 	printf("positional arg 2 %s\n", pos_arg2);
 
 
-- 
2.11.0

[PATCH v2 07/15] cli: use the arg parser .present feature to handle show --entire-thread

[Jani Nikula]

The --entire-thread default depends on other arguments, so we'll have
to figure out if it was explicitly set by the user or not. The arg
parser .present feature helps us clean up the code here.
---
 notmuch-show.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 367536ff9532..d0e86f412e80 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1086,10 +1086,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     };
     int format = NOTMUCH_FORMAT_NOT_SPECIFIED;
     int exclude = TRUE;
-
-    /* This value corresponds to neither true nor false being passed
-     * on the command line */
-    int entire_thread = -1;
+    notmuch_bool_t entire_thread_set = FALSE;
     notmuch_bool_t single_message;
 
     notmuch_opt_desc_t options[] = {
@@ -1102,7 +1099,8 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 				  { 0, 0 } } },
 	{ .opt_int = &notmuch_format_version, .name = "format-version" },
 	{ .opt_bool = &exclude, .name = "exclude" },
-	{ .opt_bool = &entire_thread, .name = "entire-thread" },
+	{ .opt_bool = &params.entire_thread, .name = "entire-thread",
+	  .present = &entire_thread_set },
 	{ .opt_int = &params.part, .name = "part" },
 	{ .opt_bool = &params.crypto.decrypt, .name = "decrypt" },
 	{ .opt_bool = &params.crypto.verify, .name = "verify" },
@@ -1147,14 +1145,9 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 
     /* Default is entire-thread = FALSE except for format=json and
      * format=sexp. */
-    if (entire_thread != FALSE && entire_thread != TRUE) {
-	if (format == NOTMUCH_FORMAT_JSON || format == NOTMUCH_FORMAT_SEXP)
-	    params.entire_thread = TRUE;
-	else
-	    params.entire_thread = FALSE;
-    } else {
-	params.entire_thread = entire_thread;
-    }
+    if (! entire_thread_set &&
+	(format == NOTMUCH_FORMAT_JSON || format == NOTMUCH_FORMAT_SEXP))
+	params.entire_thread = TRUE;
 
     if (!params.output_body) {
 	if (params.part > 0) {
-- 
2.11.0

[PATCH v2 08/15] hex-xcode: use notmuch_bool_t for boolean arguments

[Jani Nikula]

Pedantically correct, although they're the same underlying type.
---
 test/hex-xcode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/hex-xcode.c b/test/hex-xcode.c
index bc2df713b2a3..221ccdb90843 100644
--- a/test/hex-xcode.c
+++ b/test/hex-xcode.c
@@ -45,7 +45,7 @@ main (int argc, char **argv)
 {
 
     int dir = DECODE;
-    int omit_newline = FALSE;
+    notmuch_bool_t omit_newline = FALSE;
 
     notmuch_opt_desc_t options[] = {
 	{ .opt_keyword = &dir, .name = "direction", .keywords =
-- 
2.11.0

[PATCH v2 09/15] cli: use notmuch_bool_t for boolean argument in show

[Jani Nikula]

Pedantically correct, although they're the same underlying type.
---
 notmuch-show.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index d0e86f412e80..3d11a40c6a59 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1085,7 +1085,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	.output_body = TRUE,
     };
     int format = NOTMUCH_FORMAT_NOT_SPECIFIED;
-    int exclude = TRUE;
+    notmuch_bool_t exclude = TRUE;
     notmuch_bool_t entire_thread_set = FALSE;
     notmuch_bool_t single_message;
 
-- 
2.11.0

[PATCH v2 10/15] cli: refactor boolean argument processing

[Jani Nikula]

Clean up the control flow to prepare for future changes. No functional
changes.
---
 command-line-arguments.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 39940d5fb9fd..ee8be18942d0 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -39,21 +39,20 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
 
 static notmuch_bool_t
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
-
-    if (next == '\0') {
-	*arg_desc->opt_bool = TRUE;
-	return TRUE;
-    }
-    if (strcmp (arg_str, "false") == 0) {
-	*arg_desc->opt_bool = FALSE;
-	return TRUE;
-    }
-    if (strcmp (arg_str, "true") == 0) {
-	*arg_desc->opt_bool = TRUE;
-	return TRUE;
+    notmuch_bool_t value;
+
+    if (next == '\0' || strcmp (arg_str, "true") == 0) {
+	value = TRUE;
+    } else if (strcmp (arg_str, "false") == 0) {
+	value = FALSE;
+    } else {
+	fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", arg_str, arg_desc->name);
+	return FALSE;
     }
-    fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", arg_str, arg_desc->name);
-    return FALSE;
+
+    *arg_desc->opt_bool = value;
+
+    return TRUE;
 }
 
 static notmuch_bool_t
-- 
2.11.0

[PATCH v2 11/15] cli: change while to for in keyword argument processing

[Jani Nikula]

Using a for loop makes it easier to use continue, in preparation for
future changes. No functional changes.
---
 command-line-arguments.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index ee8be18942d0..c591dcbec7cc 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -13,14 +13,14 @@
 static notmuch_bool_t
 _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
 
-    const notmuch_keyword_t *keywords = arg_desc->keywords;
+    const notmuch_keyword_t *keywords;
 
     if (next == '\0') {
 	/* No keyword given */
 	arg_str = "";
     }
 
-    while (keywords->name) {
+    for (keywords = arg_desc->keywords; keywords->name; keywords++) {
 	if (strcmp (arg_str, keywords->name) == 0) {
 	    if (arg_desc->opt_flags)
 		*arg_desc->opt_flags |= keywords->value;
@@ -28,7 +28,6 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
 		*arg_desc->opt_keyword = keywords->value;
 	    return TRUE;
 	}
-	keywords++;
     }
     if (next != '\0')
 	fprintf (stderr, "Unknown keyword argument \"%s\" for option \"%s\".\n", arg_str, arg_desc->name);
-- 
2.11.0

[PATCH v2 12/15] cli: reduce indent in keyword argument processing

[Jani Nikula]

Reducing indent makes future changes easier. No functional changes.
---
 command-line-arguments.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index c591dcbec7cc..3fa8d9044966 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -21,13 +21,15 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
     }
 
     for (keywords = arg_desc->keywords; keywords->name; keywords++) {
-	if (strcmp (arg_str, keywords->name) == 0) {
-	    if (arg_desc->opt_flags)
-		*arg_desc->opt_flags |= keywords->value;
-	    else
-		*arg_desc->opt_keyword = keywords->value;
-	    return TRUE;
-	}
+	if (strcmp (arg_str, keywords->name) != 0)
+	    continue;
+
+	if (arg_desc->opt_flags)
+	    *arg_desc->opt_flags |= keywords->value;
+	else
+	    *arg_desc->opt_keyword = keywords->value;
+
+	return TRUE;
     }
     if (next != '\0')
 	fprintf (stderr, "Unknown keyword argument \"%s\" for option \"%s\".\n", arg_str, arg_desc->name);
-- 
2.11.0

[PATCH v2 13/15] cli: add support for --no- prefixed boolean and keyword flag arguments

[Jani Nikula]

Add transparent support for negating boolean and keyword flag
arguments using --no-argument style on the command line. That is, if
the option description contains a boolean or a keyword flag argument
named "argument", --no-argument will match and negate it.

For boolean arguments this obviously means the logical NOT. For
keyword flag arguments this means bitwise AND of the bitwise NOT,
i.e. masking out the specified bits instead of OR'ing them in.

For example, you can use --no-exclude instead of --exclude=false in
notmuch show. If we had keyword flag arguments with some flags
defaulting to on, say --include=tags in notmuch dump/restore, this
would allow --no-include=tags to switch that off while not affecting
other flags.

As a curiosity, you should be able to warp your brain using
--no-exclude=true meaning false and --no-exclude=false meaning true if
you wish.

Specifying both "argument" and "no-argument" style arguments in the
same option description should be avoided. In this case, --no-argument
would match whichever is specified first, and --argument would only
match "argument".
---
 command-line-arguments.c | 48 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 3fa8d9044966..058408a789fb 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -11,8 +11,9 @@
 */
 
 static notmuch_bool_t
-_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
-
+_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next,
+		      const char *arg_str, notmuch_bool_t negate)
+{
     const notmuch_keyword_t *keywords;
 
     if (next == '\0') {
@@ -24,7 +25,9 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
 	if (strcmp (arg_str, keywords->name) != 0)
 	    continue;
 
-	if (arg_desc->opt_flags)
+	if (arg_desc->opt_flags && negate)
+	    *arg_desc->opt_flags &= ~keywords->value;
+	else if (arg_desc->opt_flags)
 	    *arg_desc->opt_flags |= keywords->value;
 	else
 	    *arg_desc->opt_keyword = keywords->value;
@@ -39,7 +42,9 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
 }
 
 static notmuch_bool_t
-_process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
+_process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next,
+		      const char *arg_str, notmuch_bool_t negate)
+{
     notmuch_bool_t value;
 
     if (next == '\0' || strcmp (arg_str, "true") == 0) {
@@ -51,7 +56,7 @@ _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
 	return FALSE;
     }
 
-    *arg_desc->opt_bool = value;
+    *arg_desc->opt_bool = negate ? !value : value;
 
     return TRUE;
 }
@@ -139,6 +144,8 @@ parse_position_arg (const char *arg_str, int pos_arg_index,
     return FALSE;
 }
 
+#define NEGATIVE_PREFIX "no-"
+
 /*
  * Search for a non-positional (i.e. starting with --) argument matching arg,
  * parse a possible value, and assign to *output_var
@@ -155,6 +162,14 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
     assert(options);
 
     const char *arg = _arg + 2; /* _arg starts with -- */
+    const char *negative_arg = NULL;
+
+    /* See if this is a --no-argument */
+    if (strlen (arg) > strlen (NEGATIVE_PREFIX) &&
+	strncmp (arg, NEGATIVE_PREFIX, strlen (NEGATIVE_PREFIX)) == 0) {
+	negative_arg = arg + strlen (NEGATIVE_PREFIX);
+    }
+
     const notmuch_opt_desc_t *try;
 
     const char *next_arg = NULL;
@@ -171,11 +186,22 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
 	if (! try->name)
 	    continue;
 
-	if (strncmp (arg, try->name, strlen (try->name)) != 0)
+	char next;
+	const char *value;
+	notmuch_bool_t negate = FALSE;
+
+	if (strncmp (arg, try->name, strlen (try->name)) == 0) {
+	    next = arg[strlen (try->name)];
+	    value = arg + strlen (try->name) + 1;
+	} else if (negative_arg && (try->opt_bool || try->opt_flags) &&
+		   strncmp (negative_arg, try->name, strlen (try->name)) == 0) {
+	    next = negative_arg[strlen (try->name)];
+	    value = negative_arg + strlen (try->name) + 1;
+	    /* The argument part of --no-argument matches, negate the result. */
+	    negate = TRUE;
+	} else {
 	    continue;
-
-	char next = arg[strlen (try->name)];
-	const char *value = arg + strlen(try->name) + 1;
+	}
 
 	/*
 	 * If we have not reached the end of the argument (i.e. the
@@ -194,9 +220,9 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
 
 	notmuch_bool_t opt_status = FALSE;
 	if (try->opt_keyword || try->opt_flags)
-	    opt_status = _process_keyword_arg (try, next, value);
+	    opt_status = _process_keyword_arg (try, next, value, negate);
 	else if (try->opt_bool)
-	    opt_status = _process_boolean_arg (try, next, value);
+	    opt_status = _process_boolean_arg (try, next, value, negate);
 	else if (try->opt_int)
 	    opt_status = _process_int_arg (try, next, value);
 	else if (try->opt_string)
-- 
2.11.0

[PATCH v2 14/15] cli: use the negating boolean support for new and insert --no-hooks

[Jani Nikula]

This lets us use the positive hooks variable in code, increasing
clarity.
---
 notmuch-insert.c | 6 +++---
 notmuch-new.c    | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index bbbc29ea103d..7048e8ae2d7f 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -455,7 +455,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     const char *folder = "";
     notmuch_bool_t create_folder = FALSE;
     notmuch_bool_t keep = FALSE;
-    notmuch_bool_t no_hooks = FALSE;
+    notmuch_bool_t hooks = TRUE;
     notmuch_bool_t synchronize_flags;
     char *maildir;
     char *newpath;
@@ -466,7 +466,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	{ .opt_string = &folder, .name = "folder" },
 	{ .opt_bool = &create_folder, .name = "create-folder" },
 	{ .opt_bool = &keep, .name = "keep" },
-	{ .opt_bool =  &no_hooks, .name = "no-hooks" },
+	{ .opt_bool = &hooks, .name = "hooks" },
 	{ .opt_inherit = notmuch_shared_options },
 	{ }
     };
@@ -573,7 +573,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	}
     }
 
-    if (! no_hooks && status == NOTMUCH_STATUS_SUCCESS) {
+    if (hooks && status == NOTMUCH_STATUS_SUCCESS) {
 	/* Ignore hook failures. */
 	notmuch_run_hook (db_path, "post-insert");
     }
diff --git a/notmuch-new.c b/notmuch-new.c
index 342e2189d5d3..084cc786ea32 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -948,7 +948,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index;
     unsigned int i;
     notmuch_bool_t timer_is_active = FALSE;
-    notmuch_bool_t no_hooks = FALSE;
+    notmuch_bool_t hooks = TRUE;
     notmuch_bool_t quiet = FALSE, verbose = FALSE;
     notmuch_status_t status;
 
@@ -956,7 +956,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	{ .opt_bool = &quiet, .name = "quiet" },
 	{ .opt_bool = &verbose, .name = "verbose" },
 	{ .opt_bool = &add_files_state.debug, .name = "debug" },
-	{ .opt_bool = &no_hooks, .name = "no-hooks" },
+	{ .opt_bool = &hooks, .name = "hooks" },
 	{ .opt_inherit = notmuch_shared_options },
 	{ }
     };
@@ -989,7 +989,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	}
     }
 
-    if (!no_hooks) {
+    if (hooks) {
 	ret = notmuch_run_hook (db_path, "pre-new");
 	if (ret)
 	    return EXIT_FAILURE;
@@ -1154,7 +1154,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_database_destroy (notmuch);
 
-    if (!no_hooks && !ret && !interrupted)
+    if (hooks && !ret && !interrupted)
 	ret = notmuch_run_hook (db_path, "post-new");
 
     if (ret || interrupted)
-- 
2.11.0

[PATCH v2 15/15] test: expand argument parsing sanity checks

[Jani Nikula]

Test the various boolean formats and --no- prefixed boolean and
keyword flag arguments.
---
 test/T410-argument-parsing.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 4a2b25c6486d..243d0241b9b6 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -37,4 +37,32 @@ positional arg 1 false
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "--boolean=true"
+$TEST_DIRECTORY/arg-test --boolean=true > OUTPUT
+cat <<EOF > EXPECTED
+boolean 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--boolean=false"
+$TEST_DIRECTORY/arg-test --boolean=false > OUTPUT
+cat <<EOF > EXPECTED
+boolean 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--no-boolean"
+$TEST_DIRECTORY/arg-test --no-boolean > OUTPUT
+cat <<EOF > EXPECTED
+boolean 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--no-flag"
+$TEST_DIRECTORY/arg-test --flag=one --flag=three --no-flag=three > OUTPUT
+cat <<EOF > EXPECTED
+flags 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.11.0

Re: [PATCH v2 13/15] cli: add support for --no- prefixed boolean and keyword flag arguments

[William Casarin]

Jani Nikula <jani@nikula.org> writes:

> @@ -171,11 +186,22 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
>  	if (! try->name)
>  	    continue;
>  
> -	if (strncmp (arg, try->name, strlen (try->name)) != 0)
> +	char next;
> +	const char *value;
> +	notmuch_bool_t negate = FALSE;
> +
> +	if (strncmp (arg, try->name, strlen (try->name)) == 0) {
> +	    next = arg[strlen (try->name)];
> +	    value = arg + strlen (try->name) + 1;
> +	} else if (negative_arg && (try->opt_bool || try->opt_flags) &&
> +		   strncmp (negative_arg, try->name, strlen (try->name)) == 0) {
> +	    next = negative_arg[strlen (try->name)];
> +	    value = negative_arg + strlen (try->name) + 1;
> +	    /* The argument part of --no-argument matches, negate the result. */
> +	    negate = TRUE;
> +	} else {
>  	    continue;
> -
> -	char next = arg[strlen (try->name)];
> -	const char *value = arg + strlen(try->name) + 1;
> +	}

nit: I see strlen (try->name) computed 6 times here, any reason not to pull
this out into a variable?

Re: [PATCH v2 12/15] cli: reduce indent in keyword argument processing

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> Reducing indent makes future changes easier. No functional changes.

First 12 patches LGTM.

d

Re: [PATCH v2 13/15] cli: add support for --no- prefixed boolean and keyword flag arguments

[Jani Nikula]

On Sun, 01 Oct 2017, William Casarin <jb55@jb55.com> wrote:
> Jani Nikula <jani@nikula.org> writes:
>
>> @@ -171,11 +186,22 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
>>  	if (! try->name)
>>  	    continue;
>>  
>> -	if (strncmp (arg, try->name, strlen (try->name)) != 0)
>> +	char next;
>> +	const char *value;
>> +	notmuch_bool_t negate = FALSE;
>> +
>> +	if (strncmp (arg, try->name, strlen (try->name)) == 0) {
>> +	    next = arg[strlen (try->name)];
>> +	    value = arg + strlen (try->name) + 1;
>> +	} else if (negative_arg && (try->opt_bool || try->opt_flags) &&
>> +		   strncmp (negative_arg, try->name, strlen (try->name)) == 0) {
>> +	    next = negative_arg[strlen (try->name)];
>> +	    value = negative_arg + strlen (try->name) + 1;
>> +	    /* The argument part of --no-argument matches, negate the result. */
>> +	    negate = TRUE;
>> +	} else {
>>  	    continue;
>> -
>> -	char next = arg[strlen (try->name)];
>> -	const char *value = arg + strlen(try->name) + 1;
>> +	}
>
> nit: I see strlen (try->name) computed 6 times here, any reason not to pull
> this out into a variable?

I pretty much thought the change was so controversial that I wouldn't
bother with that kind of fixes until we'd agreed we want this. Other
than that, agreed.

BR,
Jani.

[PATCH] cli: allow empty strings for notmuch insert --folder argument

[Jani Nikula]

Now that it's easy to add argument specific modifiers in opt
descriptions, add a new .allow_empty field to allow empty strings for
individual string arguments while retaining strict checks
elsewhere. Use this for notmuch insert --folder, where the empty
string means top level folder.

---

This patch addresses id:87y3owr22c.fsf@nikula.org

Depends on most of the series, but specifically not on the more
controversial patches 13-15.
---
 command-line-arguments.c    | 2 +-
 command-line-arguments.h    | 3 +++
 doc/man1/notmuch-insert.rst | 3 ++-
 notmuch-insert.c            | 2 +-
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 3fa8d9044966..b84bfe8168b5 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -81,7 +81,7 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *
 	fprintf (stderr, "Option \"%s\" needs a string argument.\n", arg_desc->name);
 	return FALSE;
     }
-    if (arg_str[0] == '\0') {
+    if (arg_str[0] == '\0' && ! arg_desc->allow_empty) {
 	fprintf (stderr, "String argument for option \"%s\" must be non-empty.\n", arg_desc->name);
 	return FALSE;
     }
diff --git a/command-line-arguments.h b/command-line-arguments.h
index dfc808bdab78..04b04b939cba 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -30,6 +30,9 @@ typedef struct notmuch_opt_desc {
     /* Optional, if non-NULL, set to TRUE if the option is present. */
     notmuch_bool_t *present;
 
+    /* Optional, allow empty strings for opt_string. */
+    notmuch_bool_t allow_empty;
+
     /* Must be set for opt_keyword and opt_flags. */
     const struct notmuch_keyword *keywords;
 } notmuch_opt_desc_t;
diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index f79600d6571f..2f2466a6588b 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -34,7 +34,8 @@ Supported options for **insert** include
     ``--folder=<``\ folder\ **>**
         Deliver the message to the specified folder, relative to the
         top-level directory given by the value of **database.path**. The
-        default is to deliver to the top-level directory.
+        default is the empty string, which means delivering to the
+        top-level directory.
 
     ``--create-folder``
         Try to create the folder named by the ``--folder`` option, if it
diff --git a/notmuch-insert.c b/notmuch-insert.c
index bbbc29ea103d..2758723ab2fb 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -463,7 +463,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     unsigned int i;
 
     notmuch_opt_desc_t options[] = {
-	{ .opt_string = &folder, .name = "folder" },
+	{ .opt_string = &folder, .name = "folder", .allow_empty = TRUE },
 	{ .opt_bool = &create_folder, .name = "create-folder" },
 	{ .opt_bool = &keep, .name = "keep" },
 	{ .opt_bool =  &no_hooks, .name = "no-hooks" },
-- 
2.11.0

Re: [PATCH v2 12/15] cli: reduce indent in keyword argument processing

[David Bremner]

David Bremner <david@tethera.net> writes:

> Jani Nikula <jani@nikula.org> writes:
>
>> Reducing indent makes future changes easier. No functional changes.
>
> First 12 patches LGTM.
>

Pushed the first twelve patches