Re: [patch v3 06/12] lib: index message files with duplicate message-ids
Daniel Kahn Gillmorwrites: > On Sun 2017-06-04 09:32:29 -0300, David Bremner wrote: >> The corresponding xapian document just gets more terms added to it, >> but this doesn't seem to break anything. Values on the other hand get >> overwritten, which is a bit annoying, but arguably it is not worse to >> take the values (from, subject, date) from the last file indexed >> rather than the first. [snip] > for example, i could follow up on the current message with another > message with Message-Id: 20170604123235.24466-7-da...@tethera.net and > give it a subject "Re: [patch v3 06/12] lib: do *not* index message > files with duplicate message-ids". that's a bit odd, no? Yes, I agree that's a bit strange. We should make some effort to display the subject that belongs with a given message body. I think it's not too hard [1] to preserve the old behaviour of keeping the first subject, date, and from. This leaves us with a version of the original hiding message attack, but only for the special case of regex searches, since those rely exclusively on the value slots. [1]: should be just a matter of guarding the call to _notmuch_message_set_header_values() with if (is_new || is_ghost), but that needs testing. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [patch v3 06/12] lib: index message files with duplicate message-ids
On Sun 2017-06-04 09:32:29 -0300, David Bremner wrote: > The corresponding xapian document just gets more terms added to it, > but this doesn't seem to break anything. Values on the other hand get > overwritten, which is a bit annoying, but arguably it is not worse to > take the values (from, subject, date) from the last file indexed > rather than the first. It's certainly a change in behavior, though. This suggests that i can send you mail and have it change how an existing message shows up in the summary view, for example. for example, i could follow up on the current message with another message with Message-Id: 20170604123235.24466-7-da...@tethera.net and give it a subject "Re: [patch v3 06/12] lib: do *not* index message files with duplicate message-ids". that's a bit odd, no? I'm not saying it's wrong, i'm just hoping to acknowledge that this might be a controversial change. this whole series is pretty elaborate, of course, but so far it's looking like reasonable steps toward a clearer and more hackable codebase. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[patch v2 2/3] cli: change api of parse_option
The idea is to allow it (in a future commit) advance to the next argv element to get a value --- command-line-arguments.c | 37 - command-line-arguments.h | 4 ++-- test/T690-command-line-args.sh | 11 +++ 3 files changed, 33 insertions(+), 19 deletions(-) create mode 100755 test/T690-command-line-args.sh diff --git a/command-line-arguments.c b/command-line-arguments.c index de6b4536..72b950e9 100644 --- a/command-line-arguments.c +++ b/command-line-arguments.c @@ -121,18 +121,24 @@ parse_position_arg (const char *arg_str, int pos_arg_index, * parse a possible value, and assign to *output_var */ -notmuch_bool_t -parse_option (const char *_arg, const notmuch_opt_desc_t *options) +int +parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_index) { +assert(argv); + +const char *_arg = argv[opt_index]; + assert(_arg); assert(options); const char *arg = _arg + 2; /* _arg starts with -- */ const notmuch_opt_desc_t *try; for (try = options; try->opt_type != NOTMUCH_OPT_END; try++) { - if (try->opt_type == NOTMUCH_OPT_INHERIT && - parse_option (_arg, try->output_var)) - return TRUE; + if (try->opt_type == NOTMUCH_OPT_INHERIT) { + int new_index = parse_option (argc, argv, try->output_var, opt_index); + if (new_index >= 0) + return new_index; + } if (! try->name) continue; @@ -158,13 +164,13 @@ parse_option (const char *_arg, const notmuch_opt_desc_t *options) switch (try->opt_type) { case NOTMUCH_OPT_KEYWORD: case NOTMUCH_OPT_KEYWORD_FLAGS: - return _process_keyword_arg (try, next, value); + return _process_keyword_arg (try, next, value) ? opt_index + 1 : (-1); case NOTMUCH_OPT_BOOLEAN: - return _process_boolean_arg (try, next, value); + return _process_boolean_arg (try, next, value) ? opt_index + 1 : (-1); case NOTMUCH_OPT_INT: - return _process_int_arg (try, next, value); + return _process_int_arg (try, next, value) ? opt_index + 1 : (-1); case NOTMUCH_OPT_STRING: - return _process_string_arg (try, next, value); + return _process_string_arg (try, next, value) ? opt_index + 1 : (-1); case NOTMUCH_OPT_POSITION: case NOTMUCH_OPT_END: default: @@ -172,7 +178,7 @@ parse_option (const char *_arg, const notmuch_opt_desc_t *options) /*UNREACHED*/ } } -return FALSE; +return -1; } /* See command-line-arguments.h for description */ @@ -194,18 +200,15 @@ parse_arguments (int argc, char **argv, } } else { - if (strlen (argv[opt_index]) == 2) return opt_index+1; - more_args = parse_option (argv[opt_index], options); - if (more_args) { - opt_index++; - } else { + opt_index = parse_option (argc, argv, options, opt_index); + /* this is currently broken as -1 is never returned */ + if (opt_index < 0) { fprintf (stderr, "Unrecognized option: %s\n", argv[opt_index]); - opt_index = -1; + more_args = FALSE; } - } } diff --git a/command-line-arguments.h b/command-line-arguments.h index 309aaf2b..4c4d240e 100644 --- a/command-line-arguments.h +++ b/command-line-arguments.h @@ -70,8 +70,8 @@ parse_arguments (int argc, char **argv, const notmuch_opt_desc_t *options, int o * functions. */ -notmuch_bool_t -parse_option (const char *arg, const notmuch_opt_desc_t* options); +int +parse_option (int argc, char **argv, const notmuch_opt_desc_t* options, int opt_index); notmuch_bool_t parse_position_arg (const char *arg, diff --git a/test/T690-command-line-args.sh b/test/T690-command-line-args.sh new file mode 100755 index ..01592749 --- /dev/null +++ b/test/T690-command-line-args.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash + +test_description="command line arguments" +. ./test-lib.sh || exit 1 + +NOTMUCH_NEW > /dev/null + +test_begin_subtest 'bad option to show' +test_expect_code 1 'notmuch show --this-is-not-an-option' + +test_done -- 2.11.0 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Enhance argument parsing to allow spaces as separators (v2)
This obsoletes id:20170509021719.13086-2-da...@tethera.net Compared to the previous version, this drops the use of enums for exclude and entire-thread, adds a couple of tests, and some documentation of option separators. Interdiff follows diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst index fbd7f381..5e238ae4 100644 --- a/doc/man1/notmuch.rst +++ b/doc/man1/notmuch.rst @@ -125,6 +125,20 @@ users to have their own notmuch related tools to be run via the notmuch command. By design, this does not allow notmuch's own commands to be overriden using external commands. +OPTION SYNTAX +- + +All options accepting an argument can be used with '=' or ':' as a +separator. For the cases where it's not ambiguous (in particular +excluding boolean options), a space can also be used. The following +are all equivalent: + +:: + + notmuch --config=alt-config config get user.name + notmuch --config:alt-config config get user.name + notmuch --config alt-config config get user.name + ENVIRONMENT === diff --git a/notmuch-show.c b/notmuch-show.c index f8d62a1a..472c8767 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1009,18 +1009,6 @@ static const notmuch_show_format_t *formatters[] = { [NOTMUCH_FORMAT_RAW] = _raw, }; -enum { -ENTIRE_THREAD_DEFAULT = -1, -ENTIRE_THREAD_FALSE = FALSE, -ENTIRE_THREAD_TRUE = TRUE, -}; - -/* The following is to allow future options to be added more easily */ -enum { -EXCLUDE_FALSE = FALSE, -EXCLUDE_TRUE = TRUE, -}; - int notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]) { @@ -1036,8 +1024,11 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]) .output_body = TRUE, }; int format = NOTMUCH_FORMAT_NOT_SPECIFIED; -int exclude = EXCLUDE_TRUE; -int entire_thread = ENTIRE_THREAD_DEFAULT; +int exclude = TRUE; + +/* This value corresponds to neither true nor false being passed + * on the command line */ +int entire_thread = -1; notmuch_bool_t single_message; notmuch_opt_desc_t options[] = { @@ -1095,7 +1086,7 @@ 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 == ENTIRE_THREAD_DEFAULT) { +if (entire_thread != FALSE && entire_thread != TRUE) { if (format == NOTMUCH_FORMAT_JSON || format == NOTMUCH_FORMAT_SEXP) params.entire_thread = TRUE; else @@ -1175,7 +1166,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]) } } - if (exclude == EXCLUDE_FALSE) { + if (exclude == FALSE) { notmuch_query_set_omit_excluded (query, FALSE); params.omit_excluded = FALSE; } diff --git a/test/T030-config.sh b/test/T030-config.sh index 0915abdb..79a24948 100755 --- a/test/T030-config.sh +++ b/test/T030-config.sh @@ -69,6 +69,14 @@ notmuch --config=alt-config config set user.name "Another Name" test_expect_equal "$(notmuch --config=alt-config config get user.name)" \ "Another Name" +test_begin_subtest "Top level --config:FILE option" +test_expect_equal "$(notmuch --config:alt-config config get user.name)" \ +"Another Name" + +test_begin_subtest "Top level --configFILE option" +test_expect_equal "$(notmuch --config alt-config config get user.name)" \ +"Another Name" + test_begin_subtest "Top level --config=FILE option changed the right file" test_expect_equal "$(notmuch config get user.name)" \ "Notmuch Test Suite" ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[patch v2 3/3] cli: add space separator for keyword, string, and int arguments
Defer the complication of optional boolean arguments for later (never?). --- command-line-arguments.c | 11 ++ doc/man1/notmuch.rst | 14 + test/T030-config.sh| 8 test/T690-command-line-args.sh | 46 +- 4 files changed, 78 insertions(+), 1 deletion(-) diff --git a/command-line-arguments.c b/command-line-arguments.c index 72b950e9..91e9e06f 100644 --- a/command-line-arguments.c +++ b/command-line-arguments.c @@ -133,6 +133,11 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_ const char *arg = _arg + 2; /* _arg starts with -- */ const notmuch_opt_desc_t *try; + +const char *next_arg = NULL; +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); @@ -158,6 +163,12 @@ 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) { + next = ' '; + value = next_arg; + opt_index ++; + } + if (try->output_var == NULL) INTERNAL_ERROR ("output pointer NULL for option %s", try->name); diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst index fbd7f381..5e238ae4 100644 --- a/doc/man1/notmuch.rst +++ b/doc/man1/notmuch.rst @@ -125,6 +125,20 @@ users to have their own notmuch related tools to be run via the notmuch command. By design, this does not allow notmuch's own commands to be overriden using external commands. +OPTION SYNTAX +- + +All options accepting an argument can be used with '=' or ':' as a +separator. For the cases where it's not ambiguous (in particular +excluding boolean options), a space can also be used. The following +are all equivalent: + +:: + + notmuch --config=alt-config config get user.name + notmuch --config:alt-config config get user.name + notmuch --config alt-config config get user.name + ENVIRONMENT === diff --git a/test/T030-config.sh b/test/T030-config.sh index 0915abdb..79a24948 100755 --- a/test/T030-config.sh +++ b/test/T030-config.sh @@ -69,6 +69,14 @@ notmuch --config=alt-config config set user.name "Another Name" test_expect_equal "$(notmuch --config=alt-config config get user.name)" \ "Another Name" +test_begin_subtest "Top level --config:FILE option" +test_expect_equal "$(notmuch --config:alt-config config get user.name)" \ +"Another Name" + +test_begin_subtest "Top level --configFILE option" +test_expect_equal "$(notmuch --config alt-config config get user.name)" \ +"Another Name" + test_begin_subtest "Top level --config=FILE option changed the right file" test_expect_equal "$(notmuch config get user.name)" \ "Notmuch Test Suite" diff --git a/test/T690-command-line-args.sh b/test/T690-command-line-args.sh index 01592749..01de88eb 100755 --- a/test/T690-command-line-args.sh +++ b/test/T690-command-line-args.sh @@ -3,9 +3,53 @@ test_description="command line arguments" . ./test-lib.sh || exit 1 -NOTMUCH_NEW > /dev/null +add_message test_begin_subtest 'bad option to show' test_expect_code 1 'notmuch show --this-is-not-an-option' +test_begin_subtest 'string option with space' +cp /dev/null EXPECTED +notmuch dump --output foo.txt '*' >& OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest 'string option with =' +cp /dev/null EXPECTED +notmuch dump --output=foo.txt '*' >& OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest 'single keyword option with space' +cat < EXPECTED +id:msg-001@notmuch-test-suite +EOF +notmuch search --output messages '*' >& OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest 'multiple keyword options with space' +cat < EXPECTED +["msg-001@notmuch-test-suite"] +EOF +notmuch search --output messages --format json '*' >& OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest 'mixed space and = delimiters' +cat < EXPECTED +["msg-001@notmuch-test-suite"] +EOF +notmuch search --output messages --format=json '*' >& OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest 'keyword option with =' +cat < EXPECTED +["msg-001@notmuch-test-suite"] +EOF +notmuch search --output=messages --format=json '*' >& OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest 'show --entire-thread' +test_expect_success 'notmuch show --entire-thread tag:test > /dev/null' + +test_begin_subtest 'show --exclude' +test_expect_success 'notmuch show --exclude tag:test > /dev/null' + test_done -- 2.11.0 ___ notmuch mailing list
[patch v2 1/3] cli/show: convert keyword options to booleans
There are two keyword options here that impliment boolean options. It is simpler to use the built-in boolean argument handling, and also more robust against divergence in parsing boolean and keyword arguments. --- notmuch-show.c | 34 +- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index accea48a..472c8767 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1009,18 +1009,6 @@ static const notmuch_show_format_t *formatters[] = { [NOTMUCH_FORMAT_RAW] = _raw, }; -enum { -ENTIRE_THREAD_DEFAULT = -1, -ENTIRE_THREAD_FALSE = FALSE, -ENTIRE_THREAD_TRUE = TRUE, -}; - -/* The following is to allow future options to be added more easily */ -enum { -EXCLUDE_TRUE, -EXCLUDE_FALSE, -}; - int notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]) { @@ -1036,8 +1024,11 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]) .output_body = TRUE, }; int format = NOTMUCH_FORMAT_NOT_SPECIFIED; -int exclude = EXCLUDE_TRUE; -int entire_thread = ENTIRE_THREAD_DEFAULT; +int exclude = TRUE; + +/* This value corresponds to neither true nor false being passed + * on the command line */ +int entire_thread = -1; notmuch_bool_t single_message; notmuch_opt_desc_t options[] = { @@ -1049,15 +1040,8 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]) { "raw", NOTMUCH_FORMAT_RAW }, { 0, 0 } } }, { NOTMUCH_OPT_INT, _format_version, "format-version", 0, 0 }, - { NOTMUCH_OPT_KEYWORD, , "exclude", 'x', - (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE }, - { "false", EXCLUDE_FALSE }, - { 0, 0 } } }, - { NOTMUCH_OPT_KEYWORD, _thread, "entire-thread", 't', - (notmuch_keyword_t []){ { "true", ENTIRE_THREAD_TRUE }, - { "false", ENTIRE_THREAD_FALSE }, - { "", ENTIRE_THREAD_TRUE }, - { 0, 0 } } }, + { NOTMUCH_OPT_BOOLEAN, , "exclude", 'x', 0 }, + { NOTMUCH_OPT_BOOLEAN, _thread, "entire-thread", 't', 0 }, { NOTMUCH_OPT_INT, , "part", 'p', 0 }, { NOTMUCH_OPT_BOOLEAN, , "decrypt", 'd', 0 }, { NOTMUCH_OPT_BOOLEAN, , "verify", 'v', 0 }, @@ -1102,7 +1086,7 @@ 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 == ENTIRE_THREAD_DEFAULT) { +if (entire_thread != FALSE && entire_thread != TRUE) { if (format == NOTMUCH_FORMAT_JSON || format == NOTMUCH_FORMAT_SEXP) params.entire_thread = TRUE; else @@ -1182,7 +1166,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]) } } - if (exclude == EXCLUDE_FALSE) { + if (exclude == FALSE) { notmuch_query_set_omit_excluded (query, FALSE); params.omit_excluded = FALSE; } -- 2.11.0 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch