Re: [PATCH 5/6] cli/new: support // in new.ignore

2017-10-01 Thread David Bremner 
Jani Nikula  writes:
>>
>> One thing we eventually settled on in the query parser is that an
>> opening '/' without a trailing '/' is an errror. But perhaps it's fine
>> to take a more permissive approach here.
>
> I'm fine either way, I just chose to be permissive.
>
> So do I make the function void and drop the return values, or make it
> check and return errors?

I think I'd prefer to start strict, it's easier to become permissive later.

>
>>
>>> +
>>> +if (! state->ignore_regex_length)
>>> +   return FALSE;
>>
>> It's a nitpick, even by the standards of this review, but I'd prefer an
>> explicit '> 0' check.
>
> ITYM (state->ignore_regex_length == 0) but ack.
>

yeah, thought of that after just after I sent it, but yes.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 6/6] test: test regexp based new.ignore

2017-10-01 Thread David Bremner 
Jani Nikula  writes:

> Just some basics.
> ---
>  test/T050-new.sh | 22 ++
>  1 file changed, 22 insertions(+)

Tests fail after applying this patch (output attached).

It sortof looks like the regexp ignore is not working?



T50.out
Description: Binary data
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread David Bremner 
Jani Nikula  writes:

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

First 12 patches LGTM.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Jani Nikula 
On Sun, 01 Oct 2017, Tomi Ollila  wrote:
> On Sun, Oct 01 2017, Jani Nikula wrote:
>
>> On Sun, 01 Oct 2017, Tomi Ollila  wrote:
>>>
>>> Good stuff. It just doesn't longer compile on older compilers (so some
>>> note on commit message should be added):
>>
>> Does this on top fix it? I used the unnamed struct just for clarity, and
>> it doesn't serve a functional purpose.
>
> Yes it does!
>
> (output is noisy, but so is it with gcc 4.8.5 which I just tested to work)

What's so noisy about it...? :o

v2 as part of a bigger series at id:cover.1506890421.git.j...@nikula.org.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread William Casarin 
Jani Nikula  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?
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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 < EXPECTED
+boolean 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--boolean=false"
+$TEST_DIRECTORY/arg-test --boolean=false > OUTPUT
+cat < EXPECTED
+boolean 0
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "--no-boolean"
+$TEST_DIRECTORY/arg-test --no-boolean > OUTPUT
+cat < 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 < EXPECTED
+flags 1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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 = _val, .name = "boolean" },
-   { .opt_keyword = _val, .name = "keyword", .keywords =
- (notmuch_keyword_t []){ { "one", 1 },
- { "two", 2 },
- { 0, 0 } } },
+notmuch_opt_desc_t parent_options[] = {
{ .opt_flags = _val, .name = "flag", .keywords =
  (notmuch_keyword_t []){ { "one",   1 << 0},
  { "two",   1 << 1 },
  { "three", 1 << 2 },
  { 0, 0 } } },
{ .opt_int = _val, .name = "int" },
+   { }
+};
+
+notmuch_opt_desc_t options[] = {
+   { .opt_bool = _val, .name = "boolean" },
+   { .opt_keyword = _val, .name = "keyword", .keywords =
+ (notmuch_keyword_t []){ { "one", 1 },
+ { "two", 2 },
+ { 0, 0 } } },
+   { .opt_inherit = parent_options },
{ .opt_string = _val, .name = "string" },
{ .opt_position = _arg1 },
{ .opt_position = _arg2 },
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 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 = _format_version, .name = "format-version" },
{ .opt_bool = , .name = "exclude" },
-   { .opt_bool = _thread, .name = "entire-thread" },
+   { .opt_bool = _thread, .name = "entire-thread",
+ .present = _thread_set },
{ .opt_int = , .name = "part" },
{ .opt_bool = , .name = "decrypt" },
{ .opt_bool = , .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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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 =

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

2017-10-01 Thread 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 = , .name = "folder" },
{ .opt_bool = _folder, .name = "create-folder" },
{ .opt_bool = , .name = "keep" },
-   { .opt_bool =  _hooks, .name = "no-hooks" },
+   { .opt_bool = , .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 = , .name = "quiet" },
{ .opt_bool = , .name = "verbose" },
{ .opt_bool = _files_state.debug, .name = "debug" },
-   { .opt_bool = _hooks, .name = "no-hooks" },
+   { .opt_bool = , .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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/9] argument parsing fixes and improvements

2017-10-01 Thread Jani Nikula 
On Sun, 01 Oct 2017, David Bremner  wrote:
> Jani Nikula  writes:
>> id:20170930213239.15392-1-j...@nikula.org would make it easier to add,
>> say, a notmuch_bool_t *present field to notmuch_opt_desc_t that we could
>> set whenever we see the option and we want to know the difference.
>
> OK, I've queued that patch. Care to add `present` so we can unblock this
> discussion?

Done. I got a bit carried away, but the important stuff is in the
beginning: id:cover.1506890421.git.j...@nikula.org


BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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 < 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 < 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 = _val, .name = "flag", .keywords =
+   { .opt_flags = _val, .name = "flag", .present = _set, .keywords =
  (notmuch_keyword_t []){ { "one",   1 << 0},
  { "two",   1 << 1 },
  { "three", 1 << 2 },
  { 0, 0 } } },
-   { .opt_int = _val, .name = "int" },
+   { .opt_int = _val, .name = "int", .present = _set },
{ }
 };
 
 notmuch_opt_desc_t options[] = {
-   { .opt_bool = _val, .name = "boolean" },
-   { .opt_keyword = _val, .name = "keyword", .keywords =
- (notmuch_keyword_t []){ { "one", 1 },
+   { .opt_bool = _val, .name = "boolean", .present = _set },
+   { .opt_keyword = _val, .name = "keyword", .present = _set, 
.keywords =
+ (notmuch_keyword_t []){ { "zero", 0 },
+ { "one", 1 },
  { "two", 2 },
  { 0, 0 } } },
{ .opt_inherit = parent_options },
-   { .opt_string = _val, .name = "string" },
-   { .opt_position = _arg1 },
-   { .opt_position = _arg2 },
+   { .opt_string = _val, .name = "string", .present = _set },
+   { .opt_position = _arg1, .present = _set },
+   { .opt_position = _arg2, .present = _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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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 = , .name = "direction", .keywords =
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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 < 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 = _val, .name = "boolean" },
{ .opt_keyword = _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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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 (, 0, sizeof (struct sigaction));
-- 
2.11.0

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

2017-10-01 Thread 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, , 10);
+*arg_desc->opt_int = strtol (arg_str, , 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

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

2017-10-01 Thread Jani Nikula 
This series combines the designated initializers for argument parsing
from id:20170930213239.15392-1-j...@nikula.org and the argument parsing
refactoring from id:cover.1505853159.git.j...@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@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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: notmuch-emacs: Fcc to top-level directory given by database.path

2017-10-01 Thread Arun Isaac 
Mark Walters  writes:

> Incidentally, I think "/" is an alternative for the fcc line which
> already works, which is "\"/\" in notmuch-fcc-dirs.

Perhaps, notmuch should be made to tolerate a "/" at the beginning of the
Fcc folder argument. That is, notmuch should not complain about absolute
paths, and it should interpret these as relative to the root. To
maintain backward compatibility, we could add a "/" at the beginning if
it is not already there.

So,

"/" => database.path
"/sent" => database.path/sent
"sent" => database.path/sent

etc.

Is this a better idea?
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Tomi Ollila 
On Sun, Oct 01 2017, Jani Nikula wrote:

> On Sun, 01 Oct 2017, Tomi Ollila  wrote:
>>
>> Good stuff. It just doesn't longer compile on older compilers (so some
>> note on commit message should be added):
>
> Does this on top fix it? I used the unnamed struct just for clarity, and
> it doesn't serve a functional purpose.

Yes it does!

(output is noisy, but so is it with gcc 4.8.5 which I just tested to work)

Thanks,

Tomi

>
> BR,
> Jani.
>
> diff --git a/command-line-arguments.h b/command-line-arguments.h
> index 97134ad535ee..799b7fef3f65 100644
> --- a/command-line-arguments.h
> +++ b/command-line-arguments.h
> @@ -16,15 +16,13 @@ typedef struct notmuch_keyword {
>  /* Describe one option. */
>  typedef struct notmuch_opt_desc {
>  /* One and only one of these must be set. */
> -struct {
> - 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;
> -};
> +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;
> --
>
>>
>> CC  -g -O2 notmuch.o
>> notmuch.c:53: error: unknown field ‘opt_bool’ specified in initializer
>> notmuch.c:53: warning: missing braces around initializer
>> notmuch.c:53: warning: (near initialization for
>> ‘notmuch_shared_options[0].’)
>> notmuch.c:53: warning: initialization from incompatible pointer type
>> notmuch.c:53: warning: missing initializer
>> notmuch.c:53: warning: (near initialization for
>> ‘notmuch_shared_options[0]..opt_bool’)
>> notmuch.c:54: error: unknown field ‘opt_bool’ specified in initializer
>> notmuch.c:54: warning: initialization from incompatible pointer type
>> notmuch.c:54: warning: missing initializer
>> notmuch.c:54: warning: (near initialization for
>> ‘notmuch_shared_options[1]..opt_bool’)
>> notmuch.c:55: error: unknown field ‘opt_string’ specified in initializer
>> notmuch.c:55: warning: initialization from incompatible pointer type
>> notmuch.c:55: warning: missing initializer
>> notmuch.c:55: warning: (near initialization for
>> ‘notmuch_shared_options[2]..opt_bool’)
>> notmuch.c:56: warning: missing initializer
>> notmuch.c:56: warning: (near initialization for
>> ‘notmuch_shared_options[3].’)
>> notmuch.c: In function ‘notmuch_minimal_options’:
>> notmuch.c:85: error: unknown field ‘opt_inherit’ specified in initializer
>> notmuch.c:85: warning: missing braces around initializer
>> notmuch.c:85: warning: (near initialization for ‘options[0].’)
>> notmuch.c:85: warning: missing initializer
>> notmuch.c:85: warning: (near initialization for
>> ‘options[0]..opt_bool’)
>> notmuch.c:86: warning: missing initializer
>> notmuch.c:86: warning: (near initialization for ‘options[1].’)
>> notmuch.c: In function ‘main’:
>> notmuch.c:414: error: unknown field ‘opt_string’ specified in initializer
>> notmuch.c:414: warning: missing braces around initializer
>> notmuch.c:414: warning: (near initialization for ‘options[0].’)
>> notmuch.c:414: warning: initialization from incompatible pointer type
>> notmuch.c:414: warning: missing initializer
>> notmuch.c:414: warning: (near initialization for
>> ‘options[0]..opt_bool’)
>> notmuch.c:415: error: unknown field ‘opt_inherit’ specified in initializer
>> notmuch.c:415: warning: missing initializer
>> notmuch.c:415: warning: (near initialization for
>> ‘options[1]..opt_bool’)
>> notmuch.c:416: warning: missing initializer
>> notmuch.c:416: warning: (near initialization for ‘options[2].’)
>> make: *** [notmuch.o] Error 1
>>
>> gcc version 4.4.6 20110731 (Red Hat 4.4.6-3) (GCC) 
>>
>> This was on Scientific Linux 6.2 -- will test on CentOS 7 (which IIRC has
>> gcc 4.8) container soon...
>>
>>
>> Tomi
>> ___
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> https://notmuchmail.org/mailman/listinfo/notmuch
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Jani Nikula 
On Sun, 01 Oct 2017, Tomi Ollila  wrote:
> On Sun, Oct 01 2017, Jani Nikula wrote:
>
>> 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.
>
> Good stuff. It just doesn't longer compile on older compilers (so some
> note on commit message should be added):

Does this on top fix it? I used the unnamed struct just for clarity, and
it doesn't serve a functional purpose.

BR,
Jani.

diff --git a/command-line-arguments.h b/command-line-arguments.h
index 97134ad535ee..799b7fef3f65 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -16,15 +16,13 @@ typedef struct notmuch_keyword {
 /* Describe one option. */
 typedef struct notmuch_opt_desc {
 /* One and only one of these must be set. */
-struct {
-   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;
-};
+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;
--

>
> CC  -g -O2 notmuch.o
> notmuch.c:53: error: unknown field ‘opt_bool’ specified in initializer
> notmuch.c:53: warning: missing braces around initializer
> notmuch.c:53: warning: (near initialization for
> ‘notmuch_shared_options[0].’)
> notmuch.c:53: warning: initialization from incompatible pointer type
> notmuch.c:53: warning: missing initializer
> notmuch.c:53: warning: (near initialization for
> ‘notmuch_shared_options[0]..opt_bool’)
> notmuch.c:54: error: unknown field ‘opt_bool’ specified in initializer
> notmuch.c:54: warning: initialization from incompatible pointer type
> notmuch.c:54: warning: missing initializer
> notmuch.c:54: warning: (near initialization for
> ‘notmuch_shared_options[1]..opt_bool’)
> notmuch.c:55: error: unknown field ‘opt_string’ specified in initializer
> notmuch.c:55: warning: initialization from incompatible pointer type
> notmuch.c:55: warning: missing initializer
> notmuch.c:55: warning: (near initialization for
> ‘notmuch_shared_options[2]..opt_bool’)
> notmuch.c:56: warning: missing initializer
> notmuch.c:56: warning: (near initialization for
> ‘notmuch_shared_options[3].’)
> notmuch.c: In function ‘notmuch_minimal_options’:
> notmuch.c:85: error: unknown field ‘opt_inherit’ specified in initializer
> notmuch.c:85: warning: missing braces around initializer
> notmuch.c:85: warning: (near initialization for ‘options[0].’)
> notmuch.c:85: warning: missing initializer
> notmuch.c:85: warning: (near initialization for
> ‘options[0]..opt_bool’)
> notmuch.c:86: warning: missing initializer
> notmuch.c:86: warning: (near initialization for ‘options[1].’)
> notmuch.c: In function ‘main’:
> notmuch.c:414: error: unknown field ‘opt_string’ specified in initializer
> notmuch.c:414: warning: missing braces around initializer
> notmuch.c:414: warning: (near initialization for ‘options[0].’)
> notmuch.c:414: warning: initialization from incompatible pointer type
> notmuch.c:414: warning: missing initializer
> notmuch.c:414: warning: (near initialization for
> ‘options[0]..opt_bool’)
> notmuch.c:415: error: unknown field ‘opt_inherit’ specified in initializer
> notmuch.c:415: warning: missing initializer
> notmuch.c:415: warning: (near initialization for
> ‘options[1]..opt_bool’)
> notmuch.c:416: warning: missing initializer
> notmuch.c:416: warning: (near initialization for ‘options[2].’)
> make: *** [notmuch.o] Error 1
>
> gcc version 4.4.6 20110731 (Red Hat

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Tomi Ollila 
On Sun, Oct 01 2017, Jani Nikula wrote:

> 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.

Good stuff. It just doesn't longer compile on older compilers (so some
note on commit message should be added):

CC  -g -O2 notmuch.o
notmuch.c:53: error: unknown field ‘opt_bool’ specified in initializer
notmuch.c:53: warning: missing braces around initializer
notmuch.c:53: warning: (near initialization for
‘notmuch_shared_options[0].’)
notmuch.c:53: warning: initialization from incompatible pointer type
notmuch.c:53: warning: missing initializer
notmuch.c:53: warning: (near initialization for
‘notmuch_shared_options[0]..opt_bool’)
notmuch.c:54: error: unknown field ‘opt_bool’ specified in initializer
notmuch.c:54: warning: initialization from incompatible pointer type
notmuch.c:54: warning: missing initializer
notmuch.c:54: warning: (near initialization for
‘notmuch_shared_options[1]..opt_bool’)
notmuch.c:55: error: unknown field ‘opt_string’ specified in initializer
notmuch.c:55: warning: initialization from incompatible pointer type
notmuch.c:55: warning: missing initializer
notmuch.c:55: warning: (near initialization for
‘notmuch_shared_options[2]..opt_bool’)
notmuch.c:56: warning: missing initializer
notmuch.c:56: warning: (near initialization for
‘notmuch_shared_options[3].’)
notmuch.c: In function ‘notmuch_minimal_options’:
notmuch.c:85: error: unknown field ‘opt_inherit’ specified in initializer
notmuch.c:85: warning: missing braces around initializer
notmuch.c:85: warning: (near initialization for ‘options[0].’)
notmuch.c:85: warning: missing initializer
notmuch.c:85: warning: (near initialization for
‘options[0]..opt_bool’)
notmuch.c:86: warning: missing initializer
notmuch.c:86: warning: (near initialization for ‘options[1].’)
notmuch.c: In function ‘main’:
notmuch.c:414: error: unknown field ‘opt_string’ specified in initializer
notmuch.c:414: warning: missing braces around initializer
notmuch.c:414: warning: (near initialization for ‘options[0].’)
notmuch.c:414: warning: initialization from incompatible pointer type
notmuch.c:414: warning: missing initializer
notmuch.c:414: warning: (near initialization for
‘options[0]..opt_bool’)
notmuch.c:415: error: unknown field ‘opt_inherit’ specified in initializer
notmuch.c:415: warning: missing initializer
notmuch.c:415: warning: (near initialization for
‘options[1]..opt_bool’)
notmuch.c:416: warning: missing initializer
notmuch.c:416: warning: (near initialization for ‘options[2].’)
make: *** [notmuch.o] Error 1

gcc version 4.4.6 20110731 (Red Hat 4.4.6-3) (GCC) 

This was on Scientific Linux 6.2 -- will test on CentOS 7 (which IIRC has
gcc 4.8) container soon...


Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/9] argument parsing fixes and improvements

2017-10-01 Thread David Bremner 
Jani Nikula  writes:

> On Sat, 30 Sep 2017, Jani Nikula  wrote:
>> Looking at the defaults from another angle, if we don't want the ability
>> to set --foo=default explicitly, I still think passing ints as booleans
>> to the argument parser and checking if a boolean is neither true nor
>> false is the wrong thing to do. I'd also like to convert to stdbool
>> more. But those should not be a reason to convert essentially boolean
>> arguments to keyword arguments. I think we need a way to have the
>> argument parser tell us if an argument was present or not.
>
> id:20170930213239.15392-1-j...@nikula.org would make it easier to add,
> say, a notmuch_bool_t *present field to notmuch_opt_desc_t that we could
> set whenever we see the option and we want to know the difference.

OK, I've queued that patch. Care to add `present` so we can unblock this
discussion?

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread David Bremner 
Jani Nikula  writes:

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

the general approach here looks fine. I didn't see any blockers, but
I'll wait a few days before merging.

This probably causes some rebasing pain for other people, but I think
it's one of those "rip the band-aid off quickly" scenarios.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch