date:20150408

[PATCH 1/2] test: be consistent about spelling `nonexistent'

2015-04-08 Thread Tomi Ollila

On Wed, Apr 08 2015, David Bremner  wrote:

> Apparently most of the misspellings are my fault.

Series LGTM. Tests pass.

Tomi


> ---
>  test/T360-symbol-hiding.sh | 2 +-
>  test/T560-lib-error.sh | 2 +-
>  test/symbol-test.cc| 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/test/T360-symbol-hiding.sh b/test/T360-symbol-hiding.sh
> index 8fc4bdf..d2b5d1f 100755
> --- a/test/T360-symbol-hiding.sh
> +++ b/test/T360-symbol-hiding.sh
> @@ -16,7 +16,7 @@ run_test(){
>  }
>  
>  output="A Xapian exception occurred opening database: Couldn't stat 
> 'fakedb/.notmuch/xapian'
> -caught No chert database found at path \`./nonexistant'"
> +caught No chert database found at path \`./nonexistent'"
>  
>  mkdir -p fakedb/.notmuch
>  
> diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
> index 828a44b..6947aa8 100755
> --- a/test/T560-lib-error.sh
> +++ b/test/T560-lib-error.sh
> @@ -202,7 +202,7 @@ test_begin_subtest "Xapian exception finding message"
>  cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
> {
> notmuch_message_t *message = NULL;
> -   stat = notmuch_database_find_message (db, "id:nonexistant", );
> +   stat = notmuch_database_find_message (db, "id:nonexistent", );
> }
>  EOF
>  sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
> diff --git a/test/symbol-test.cc b/test/symbol-test.cc
> index d979f83..f17ddc8 100644
> --- a/test/symbol-test.cc
> +++ b/test/symbol-test.cc
> @@ -15,7 +15,7 @@ int main() {
>}
>  
>try {
> -(void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);
> +(void) new Xapian::WritableDatabase("./nonexistent", Xapian::DB_OPEN);
>} catch (const Xapian::Error ) {
>  printf("caught %s\n", error.get_msg().c_str());
>  return 0;
> -- 
> 2.1.4

argument parsing refactoring round3

2015-04-08 Thread guyzmo

Hi David,

On Wed, Apr 08, 2015 at 04:30:38AM +0900, David Bremner wrote:
> I think a dealt with all of Mark's comments, even "notmuch help
> --help".
> 
> I ended up creating a new function for the places where we want to
> process _only_ the shared options (config, setup, and help)

I see you patching and repatching notmuch's CLI to improve it, and I was
wondering whether you had considered actually using `docopt` to generate
the CLI parser from the output.

It's possible to chain docopts to create a CLI UI very much alike the
git command, and it's more easily maintainable, as you're actually
generating the code from the `--help` page instead of the other way
around, making you focus on how you want the user to use the CLI only.

Here's the link to the C parser generator:

https://github.com/docopt/docopt.c

it might not be perfect as is, but it could be worth trying out? I
actually never tried the .c version of docopt.

I had a more extensive experience with the python version, and since
then I totally dropped argparse.

https://github.com/docopt/docopt.c

I even tend to believe that one could create a full CLI using
python+docopt to actually control notmuch using the python notmuch
interfaceâ¦ Even though I'm pretty sure some would yell at me and
want to burn me as an heretic just for suggesting that :-)

what do you believe?

-- 
Guyzmo

[PATCH 4/4] cli: add standard option processing to config, help and setup

2015-04-08 Thread David Bremner

In particular this fixes a recently encountered bug where the
"--config" argument to "notmuch setup" is silently ignored, which the
unpleasant consequence of overwriting the users config file.
---
 notmuch-client.h |  2 ++
 notmuch-config.c |  9 -
 notmuch-setup.c  |  3 +++
 notmuch.c| 32 +++-
 test/random-corpus.c |  9 +
 5 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 8ecfac6..78680aa 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -468,4 +468,6 @@ notmuch_database_dump (notmuch_database_t *notmuch,
 #include "command-line-arguments.h"
 extern const notmuch_opt_desc_t  notmuch_shared_options [];
 void notmuch_process_shared_options (const char* subcommand_name);
+int notmuch_minimal_options (const char* subcommand_name,
+int argc, char **argv);
 #endif
diff --git a/notmuch-config.c b/notmuch-config.c
index 2d5c297..9348278 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -872,8 +872,15 @@ int
 notmuch_config_command (notmuch_config_t *config, int argc, char *argv[])
 {
 int ret;
+int opt_index;

-argc--; argv++; /* skip subcommand argument */
+opt_index = notmuch_minimal_options ("config", argc, argv);
+if (opt_index < 0)
+   return EXIT_FAILURE;
+
+/* skip at least subcommand argument */
+argc-= opt_index;
+argv+= opt_index;

 if (argc < 1) {
fprintf (stderr, "Error: notmuch config requires at least one 
argument.\n");
diff --git a/notmuch-setup.c b/notmuch-setup.c
index 36a6171..7dd5822 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -145,6 +145,9 @@ notmuch_setup_command (notmuch_config_t *config,
chomp_newline (response);   \
 } while (0)

+if (notmuch_minimal_options ("setup", argc, argv) < 0)
+   return EXIT_FAILURE;
+
 if (notmuch_config_is_new (config))
welcome_message_pre_setup ();

diff --git a/notmuch.c b/notmuch.c
index 3a9da90..2198b73 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -71,6 +71,28 @@ notmuch_process_shared_options (const char *subcommand_name) 
{
 }
 }

+/* This is suitable for subcommands that do not actually open the
+ * database.
+ */
+int notmuch_minimal_options (const char *subcommand_name,
+ int argc, char **argv)
+{
+int opt_index;
+
+notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
+   { 0, 0, 0, 0, 0 }
+};
+
+opt_index = parse_arguments (argc, argv, options, 1);
+
+if (opt_index < 0)
+   return -1;
+
+/* We can't use argv here as it is sometimes NULL */
+notmuch_process_shared_options (subcommand_name);
+return opt_index;
+}

 static command_t commands[] = {
 { NULL, notmuch_command, TRUE,
@@ -250,7 +272,15 @@ _help_for (const char *topic_name)
 static int
 notmuch_help_command (unused (notmuch_config_t * config), int argc, char 
*argv[])
 {
-argc--; argv++; /* Ignore "help" */
+int opt_index;
+
+opt_index = notmuch_minimal_options ("help", argc, argv);
+if (opt_index < 0)
+   return EXIT_FAILURE;
+
+/* skip at least subcommand argument */
+argc-= opt_index;
+argv+= opt_index;

 if (argc == 0) {
return _help_for (NULL);
diff --git a/test/random-corpus.c b/test/random-corpus.c
index 790193d..6c467bb 100644
--- a/test/random-corpus.c
+++ b/test/random-corpus.c
@@ -114,6 +114,15 @@ random_utf8_string (void *ctx, size_t char_count)
 return buf;
 }

+/* stubs since we cannot link with notmuch.o */
+const notmuch_opt_desc_t notmuch_shared_options[] = {
+   { 0, 0, 0, 0, 0 }
+};
+
+void
+notmuch_process_shared_options (unused (const char *dummy))
+{
+}

 int
 main (int argc, char **argv)
-- 
2.1.4

[PATCH 3/4] cli: define shared options, use for --help and --version

2015-04-08 Thread David Bremner

Unfortunately it seems trickier to support --config globally

The non-trivial changes are in notmuch.c; most of the other changes
consists of blindly inserting two lines into every subcommand.
---
 notmuch-client.h  |  2 ++
 notmuch-compact.c |  4 
 notmuch-count.c   |  3 +++
 notmuch-dump.c|  3 +++
 notmuch-insert.c  |  3 +++
 notmuch-new.c |  3 +++
 notmuch-reply.c   |  3 +++
 notmuch-restore.c |  2 ++
 notmuch-search.c  |  6 ++
 notmuch-show.c|  3 +++
 notmuch-tag.c |  3 +++
 notmuch.c | 52 +++-
 12 files changed, 66 insertions(+), 21 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index fb3021c..8ecfac6 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -466,4 +466,6 @@ notmuch_database_dump (notmuch_database_t *notmuch,
   notmuch_bool_t gzip_output);

 #include "command-line-arguments.h"
+extern const notmuch_opt_desc_t  notmuch_shared_options [];
+void notmuch_process_shared_options (const char* subcommand_name);
 #endif
diff --git a/notmuch-compact.c b/notmuch-compact.c
index 2fc012a..5be551d 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -38,12 +38,16 @@ notmuch_compact_command (notmuch_config_t *config, int 
argc, char *argv[])
 notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_STRING, _path, "backup", 0, 0 },
{ NOTMUCH_OPT_BOOLEAN,  , "quiet", 'q', 0 },
+   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
+   { 0, 0, 0, 0, 0}
 };

 opt_index = parse_arguments (argc, argv, options, 1);
 if (opt_index < 0)
return EXIT_FAILURE;

+notmuch_process_shared_options (argv[0]);
+
 if (! quiet)
printf ("Compacting database...\n");
 ret = notmuch_database_compact (path, backup_path,
diff --git a/notmuch-count.c b/notmuch-count.c
index 6058f7c..57a88a8 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -146,6 +146,7 @@ notmuch_count_command (notmuch_config_t *config, int argc, 
char *argv[])
  { 0, 0 } } },
{ NOTMUCH_OPT_BOOLEAN, , "batch", 0, 0 },
{ NOTMUCH_OPT_STRING, _file_name, "input", 'i', 0 },
+   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
{ 0, 0, 0, 0, 0 }
 };

@@ -153,6 +154,8 @@ notmuch_count_command (notmuch_config_t *config, int argc, 
char *argv[])
 if (opt_index < 0)
return EXIT_FAILURE;

+notmuch_process_shared_options (argv[0]);
+
 if (input_file_name) {
batch = TRUE;
input = fopen (input_file_name, "r");
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 9c6ad7f..fab22bd 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -228,6 +228,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, 
char *argv[])
  { 0, 0 } } },
{ NOTMUCH_OPT_STRING, _file_name, "output", 'o', 0  },
{ NOTMUCH_OPT_BOOLEAN, _output, "gzip", 'z', 0 },
+   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
{ 0, 0, 0, 0, 0 }
 };

@@ -235,6 +236,8 @@ notmuch_dump_command (notmuch_config_t *config, int argc, 
char *argv[])
 if (opt_index < 0)
return EXIT_FAILURE;

+notmuch_process_shared_options (argv[0]);
+
 if (opt_index < argc) {
query_str = query_string_from_args (notmuch, argc - opt_index, argv + 
opt_index);
if (query_str == NULL) {
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 90fe3ba..697880f 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -466,6 +466,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
{ NOTMUCH_OPT_BOOLEAN, _folder, "create-folder", 0, 0 },
{ NOTMUCH_OPT_BOOLEAN, , "keep", 0, 0 },
{ NOTMUCH_OPT_BOOLEAN,  _hooks, "no-hooks", 'n', 0 },
+   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
{ NOTMUCH_OPT_END, 0, 0, 0, 0 }
 };

@@ -473,6 +474,8 @@ notmuch_insert_command (notmuch_config_t *config, int argc, 
char *argv[])
 if (opt_index < 0)
return EXIT_FAILURE;

+notmuch_process_shared_options (argv[0]);
+
 db_path = notmuch_config_get_database_path (config);
 new_tags = notmuch_config_get_new_tags (config, _tags_length);
 synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
diff --git a/notmuch-new.c b/notmuch-new.c
index e6c283e..895f5d9 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -934,6 +934,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
{ NOTMUCH_OPT_BOOLEAN,  , "verbose", 'v', 0 },
{ NOTMUCH_OPT_BOOLEAN,  _files_state.debug, "debug", 'd', 0 },
{ NOTMUCH_OPT_BOOLEAN,  _hooks, "no-hooks", 'n', 0 },
+   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
{ 0, 0, 0, 0, 0 }
 };

@@ -941,6 +942,8 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
 if (opt_index < 0)
return EXIT_FAILURE;

+

[PATCH 2/4] cli: refactor notmuch_help_command

2015-04-08 Thread David Bremner

Create a new private entry point _help_for so that we can call help
without simulating a command line invokation to set up the arguments.
---
 notmuch.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index 31672c3..bff941f 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -177,21 +177,19 @@ exec_man (const char *page)
 }

 static int
-notmuch_help_command (unused (notmuch_config_t * config), int argc, char 
*argv[])
+_help_for (const char *topic_name)
 {
 command_t *command;
 help_topic_t *topic;
 unsigned int i;

-argc--; argv++; /* Ignore "help" */
-
-if (argc == 0) {
+if (!topic_name) {
printf ("The notmuch mail system.\n\n");
usage (stdout);
return EXIT_SUCCESS;
 }

-if (strcmp (argv[0], "help") == 0) {
+if (strcmp (topic_name, "help") == 0) {
printf ("The notmuch help system.\n\n"
"\tNotmuch uses the man command to display help. In case\n"
"\tof difficulties check that MANPATH includes the pages\n"
@@ -200,7 +198,7 @@ notmuch_help_command (unused (notmuch_config_t * config), 
int argc, char *argv[]
return EXIT_SUCCESS;
 }

-command = find_command (argv[0]);
+command = find_command (topic_name);
 if (command) {
char *page = talloc_asprintf (NULL, "notmuch-%s", command->name);
exec_man (page);
@@ -208,7 +206,7 @@ notmuch_help_command (unused (notmuch_config_t * config), 
int argc, char *argv[]

 for (i = 0; i < ARRAY_SIZE (help_topics); i++) {
topic = _topics[i];
-   if (strcmp (argv[0], topic->name) == 0) {
+   if (strcmp (topic_name, topic->name) == 0) {
char *page = talloc_asprintf (NULL, "notmuch-%s", topic->name);
exec_man (page);
}
@@ -216,10 +214,22 @@ notmuch_help_command (unused (notmuch_config_t * config), 
int argc, char *argv[]

 fprintf (stderr,
 "\nSorry, %s is not a known command. There's not much I can do to 
help.\n\n",
-argv[0]);
+topic_name);
 return EXIT_FAILURE;
 }

+static int
+notmuch_help_command (unused (notmuch_config_t * config), int argc, char 
*argv[])
+{
+argc--; argv++; /* Ignore "help" */
+
+if (argc == 0) {
+   return _help_for (NULL);
+}
+
+return _help_for (argv[0]);
+}
+
 /* Handle the case of "notmuch" being invoked with no command
  * argument. For now we just call notmuch_setup_command, but we plan
  * to be more clever about this in the future.
-- 
2.1.4

[PATCH 1/4] cli: ignore config argument of notmuch_help_command

2015-04-08 Thread David Bremner

We call it with NULL at one point anyway, so it needs to work with
NULL. Since the only place we use talloc is right before exec, there
is no harm in always using NULL.
---
 notmuch.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index a5b2877..31672c3 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -177,7 +177,7 @@ exec_man (const char *page)
 }

 static int
-notmuch_help_command (notmuch_config_t *config, int argc, char *argv[])
+notmuch_help_command (unused (notmuch_config_t * config), int argc, char 
*argv[])
 {
 command_t *command;
 help_topic_t *topic;
@@ -202,14 +202,14 @@ notmuch_help_command (notmuch_config_t *config, int argc, 
char *argv[])

 command = find_command (argv[0]);
 if (command) {
-   char *page = talloc_asprintf (config, "notmuch-%s", command->name);
+   char *page = talloc_asprintf (NULL, "notmuch-%s", command->name);
exec_man (page);
 }

 for (i = 0; i < ARRAY_SIZE (help_topics); i++) {
topic = _topics[i];
if (strcmp (argv[0], topic->name) == 0) {
-   char *page = talloc_asprintf (config, "notmuch-%s", topic->name);
+   char *page = talloc_asprintf (NULL, "notmuch-%s", topic->name);
exec_man (page);
}
 }
-- 
2.1.4

argument parsing refactoring round3

2015-04-08 Thread David Bremner

I think a dealt with all of Mark's comments, even "notmuch help
--help".

I ended up creating a new function for the places where we want to
process _only_ the shared options (config, setup, and help)

diff --git a/notmuch-client.h b/notmuch-client.h
index ab0d188..78680aa 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -467,5 +467,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,

 #include "command-line-arguments.h"
 extern const notmuch_opt_desc_t  notmuch_shared_options [];
-void notmuch_process_shared_options (const char* help_name);
+void notmuch_process_shared_options (const char* subcommand_name);
+int notmuch_minimal_options (const char* subcommand_name,
+int argc, char **argv);
 #endif
diff --git a/notmuch-config.c b/notmuch-config.c
index f2cd6a8..9348278 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -874,17 +874,10 @@ notmuch_config_command (notmuch_config_t *config, int 
argc, char *argv[])
 int ret;
 int opt_index;

-notmuch_opt_desc_t options[] = {
-   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
-   { 0, 0, 0, 0, 0 }
-};
-
-opt_index = parse_arguments (argc, argv, options, 1);
+opt_index = notmuch_minimal_options ("config", argc, argv);
 if (opt_index < 0)
return EXIT_FAILURE;

-notmuch_process_shared_options (argv[0]);
-
 /* skip at least subcommand argument */
 argc-= opt_index;
 argv+= opt_index;
diff --git a/notmuch-search.c b/notmuch-search.c
index 994ae58..b89a17e 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -740,6 +740,7 @@ notmuch_address_command (notmuch_config_t *config, int 
argc, char *argv[])
  { "false", NOTMUCH_EXCLUDE_FALSE },
  { 0, 0 } } },
{ NOTMUCH_OPT_INHERIT, (void *) _options, NULL, 0, 0 },
+   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
{ 0, 0, 0, 0, 0 }
 };

@@ -747,6 +748,8 @@ notmuch_address_command (notmuch_config_t *config, int 
argc, char *argv[])
 if (opt_index < 0)
return EXIT_FAILURE;

+notmuch_process_shared_options (argv[0]);
+
 if (! (ctx->output & (OUTPUT_SENDER | OUTPUT_RECIPIENTS)))
ctx->output |= OUTPUT_SENDER;

diff --git a/notmuch-setup.c b/notmuch-setup.c
index 21d074a..7dd5822 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -133,7 +133,6 @@ notmuch_setup_command (notmuch_config_t *config,
 size_t new_tags_len;
 const char **search_exclude_tags;
 size_t search_exclude_tags_len;
-int opt_index;

 #define prompt(format, ...)\
 do {   \
@@ -146,18 +145,9 @@ notmuch_setup_command (notmuch_config_t *config,
chomp_newline (response);   \
 } while (0)

-notmuch_opt_desc_t options[] = {
-   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
-   { 0, 0, 0, 0, 0 }
-};
-
-opt_index = parse_arguments (argc, argv, options, 1);
-if (opt_index < 0)
+if (notmuch_minimal_options ("setup", argc, argv) < 0)
return EXIT_FAILURE;

-/* We can't use argv here as it is sometimes NULL */
-notmuch_process_shared_options ("setup");
-
 if (notmuch_config_is_new (config))
welcome_message_pre_setup ();

diff --git a/notmuch.c b/notmuch.c
index c7f8c8f..2198b73 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -59,18 +59,40 @@ const notmuch_opt_desc_t notmuch_shared_options [] = {
  * notmuch_process_shared_options (subcommand_name);
  */
 void
-notmuch_process_shared_options (const char *help_name) {
+notmuch_process_shared_options (const char *subcommand_name) {
 if (print_version) {
printf ("notmuch " STRINGIFY(NOTMUCH_VERSION) "\n");
exit (EXIT_SUCCESS);
 }

 if (print_help) {
-   int ret = _help_for (help_name);
+   int ret = _help_for (subcommand_name);
exit (ret);
 }
 }

+/* This is suitable for subcommands that do not actually open the
+ * database.
+ */
+int notmuch_minimal_options (const char *subcommand_name,
+ int argc, char **argv)
+{
+int opt_index;
+
+notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
+   { 0, 0, 0, 0, 0 }
+};
+
+opt_index = parse_arguments (argc, argv, options, 1);
+
+if (opt_index < 0)
+   return -1;
+
+/* We can't use argv here as it is sometimes NULL */
+notmuch_process_shared_options (subcommand_name);
+return opt_index;
+}

 static command_t commands[] = {
 { NULL, notmuch_command, TRUE,
@@ -250,7 +272,15 @@ _help_for (const char *topic_name)
 static int
 notmuch_help_command (unused (notmuch_config_t * config), int argc, char 
*argv[])
 {
-argc--; argv++; /* Ignore "help" */
+int opt_index;
+
+opt_index = notmuch_minimal_options ("help", argc, argv);
+if (opt_index <

Re: argument parsing refactoring round3

2015-04-08 Thread David Bremner

guyzmo guyzmo+notm...@m0g.net writes:

 Hi David,

[...]

 I see you patching and repatching notmuch's CLI to improve it, and I was
 wondering whether you had considered actually using `docopt` to generate
 the CLI parser from the output.

 It's possible to chain docopts to create a CLI UI very much alike the
 git command, and it's more easily maintainable, as you're actually
 generating the code from the `--help` page instead of the other way
 around, making you focus on how you want the user to use the CLI only.

[...]

 what do you believe?

It's an interesting idea, and if I was faced with writing CLI parser
from scratch (i.e. 4 years ago) I would investigate it more seriously.
As it stands, I'm not particularly annoyed with the notmuch argument
parsing code, so I mainly see negative issues about your proposal.

- I'm not sure how much this change would ripple through the rest of the
  notmuch code. At least all of the variables set by the current
  argument parsing code would have to be set foo=args.foo, or replaced
  everywhere with args.foo.
   
- It would require modifying the notmuch CLI to conform to the
  conventions of docopt. Of course, you might consider this a feature,
  but I think as many people would be annoyed as would be happy.

- The most dramatic example of an appartently missing feature from
  docopt is keyword arguments of the form
  
--output=(messages|threads|summary).

  These are the reason we rolled our own parser in the first place,
  rather than using e.g. gnu getopt.  docopt says it doesn't do data
  validation, which is fine philosophically, but by the time we add back
  in validation code, I'm not sure we win very much in the
  maintainability department.
  
- I don't really know about the health of the docopt.c project, compared
  to the python version. It seems somewhat unfinished [1]; in particular
  a lack of positional arguments seems like a showstopper for us.
  arguments. There has never been a release (which is the norm for
  github, but not for dependencies of notmuch), and the last commit was
  in December of 2014.

- Since docopt_c_py is not widely in distros (it isn't in Debian, for
  example), we'd have to emded it the notmuch source.  It's only 217
  lines of fairly simple python, but embedding 3rd party code is
  something we try pretty hard to avoid.

As always, my lack of enthusiasm doesn't prevent someone else from
investigating further, but hopefully the points I listed above give
anyone doing such investigation some hints as to what I (and I suspect
not just I) would object to about any hypothetical patches.

David

[1]: From the README.md Note, at this point the code generator handles
only options (positional arguments, commands and pattern matching will
follow).
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/2] test: be consistent about spelling `nonexistent'

2015-04-08 Thread David Bremner

Tomi Ollila tomi.oll...@iki.fi writes:

 On Wed, Apr 08 2015, David Bremner da...@tethera.net wrote:

 Apparently most of the misspellings are my fault.

 Series LGTM. Tests pass.

 Tomi

pushed,

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

Re: argument parsing refactoring round3

2015-04-08 Thread guyzmo

Hi David,

On Wed, Apr 08, 2015 at 04:30:38AM +0900, David Bremner wrote:
 I think a dealt with all of Mark's comments, even notmuch help
 --help.
 
 I ended up creating a new function for the places where we want to
 process _only_ the shared options (config, setup, and help)

I see you patching and repatching notmuch's CLI to improve it, and I was
wondering whether you had considered actually using `docopt` to generate
the CLI parser from the output.

It's possible to chain docopts to create a CLI UI very much alike the
git command, and it's more easily maintainable, as you're actually
generating the code from the `--help` page instead of the other way
around, making you focus on how you want the user to use the CLI only.

Here's the link to the C parser generator:

https://github.com/docopt/docopt.c

it might not be perfect as is, but it could be worth trying out? I
actually never tried the .c version of docopt.

I had a more extensive experience with the python version, and since
then I totally dropped argparse.

https://github.com/docopt/docopt.c

I even tend to believe that one could create a full CLI using
python+docopt to actually control notmuch using the python notmuch
interface… Even though I'm pretty sure some would yell at me and
want to burn me as an heretic just for suggesting that :-)

what do you believe?

-- 
Guyzmo
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/2] test: be consistent about spelling `nonexistent'

2015-04-08 Thread David Bremner

Apparently most of the misspellings are my fault.
---
 test/T360-symbol-hiding.sh | 2 +-
 test/T560-lib-error.sh | 2 +-
 test/symbol-test.cc| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/T360-symbol-hiding.sh b/test/T360-symbol-hiding.sh
index 8fc4bdf..d2b5d1f 100755
--- a/test/T360-symbol-hiding.sh
+++ b/test/T360-symbol-hiding.sh
@@ -16,7 +16,7 @@ run_test(){
 }
 
 output=A Xapian exception occurred opening database: Couldn't stat 
'fakedb/.notmuch/xapian'
-caught No chert database found at path \`./nonexistant'
+caught No chert database found at path \`./nonexistent'
 
 mkdir -p fakedb/.notmuch
 
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 828a44b..6947aa8 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -202,7 +202,7 @@ test_begin_subtest Xapian exception finding message
 cat c_head - c_tail 'EOF' | test_C ${MAIL_DIR}
{
notmuch_message_t *message = NULL;
-   stat = notmuch_database_find_message (db, id:nonexistant, message);
+   stat = notmuch_database_find_message (db, id:nonexistent, message);
}
 EOF
 sed 's/^\(A Xapian exception [^:]*\):.*$/\1/'  OUTPUT  OUTPUT.clean
diff --git a/test/symbol-test.cc b/test/symbol-test.cc
index d979f83..f17ddc8 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -15,7 +15,7 @@ int main() {
   }
 
   try {
-(void) new Xapian::WritableDatabase(./nonexistant, Xapian::DB_OPEN);
+(void) new Xapian::WritableDatabase(./nonexistent, Xapian::DB_OPEN);
   } catch (const Xapian::Error error) {
 printf(caught %s\n, error.get_msg().c_str());
 return 0;
-- 
2.1.4

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

[PATCH] fixup! cli: add standard option processing to config, help and setup

2015-04-08 Thread David Bremner

---
 test/random-corpus.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/test/random-corpus.c b/test/random-corpus.c
index 6c467bb..b377eb4 100644
--- a/test/random-corpus.c
+++ b/test/random-corpus.c
@@ -125,6 +125,14 @@ notmuch_process_shared_options (unused (const char *dummy))
 }
 
 int
+notmuch_minimal_options (unused (const char *subcommand),
+unused (int argc),
+unused (char **argv))
+{
+return 0;
+}
+
+int
 main (int argc, char **argv)
 {
 
-- 
2.1.4

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

Re: [PATCH 1/2] test: be consistent about spelling `nonexistent'

2015-04-08 Thread Tomi Ollila

On Wed, Apr 08 2015, David Bremner da...@tethera.net wrote:

 Apparently most of the misspellings are my fault.

Series LGTM. Tests pass.

Tomi


 ---
  test/T360-symbol-hiding.sh | 2 +-
  test/T560-lib-error.sh | 2 +-
  test/symbol-test.cc| 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/test/T360-symbol-hiding.sh b/test/T360-symbol-hiding.sh
 index 8fc4bdf..d2b5d1f 100755
 --- a/test/T360-symbol-hiding.sh
 +++ b/test/T360-symbol-hiding.sh
 @@ -16,7 +16,7 @@ run_test(){
  }
  
  output=A Xapian exception occurred opening database: Couldn't stat 
 'fakedb/.notmuch/xapian'
 -caught No chert database found at path \`./nonexistant'
 +caught No chert database found at path \`./nonexistent'
  
  mkdir -p fakedb/.notmuch
  
 diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
 index 828a44b..6947aa8 100755
 --- a/test/T560-lib-error.sh
 +++ b/test/T560-lib-error.sh
 @@ -202,7 +202,7 @@ test_begin_subtest Xapian exception finding message
  cat c_head - c_tail 'EOF' | test_C ${MAIL_DIR}
 {
 notmuch_message_t *message = NULL;
 -   stat = notmuch_database_find_message (db, id:nonexistant, message);
 +   stat = notmuch_database_find_message (db, id:nonexistent, message);
 }
  EOF
  sed 's/^\(A Xapian exception [^:]*\):.*$/\1/'  OUTPUT  OUTPUT.clean
 diff --git a/test/symbol-test.cc b/test/symbol-test.cc
 index d979f83..f17ddc8 100644
 --- a/test/symbol-test.cc
 +++ b/test/symbol-test.cc
 @@ -15,7 +15,7 @@ int main() {
}
  
try {
 -(void) new Xapian::WritableDatabase(./nonexistant, Xapian::DB_OPEN);
 +(void) new Xapian::WritableDatabase(./nonexistent, Xapian::DB_OPEN);
} catch (const Xapian::Error error) {
  printf(caught %s\n, error.get_msg().c_str());
  return 0;
 -- 
 2.1.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/2] test: be consistent about spelling `nonexistent'

argument parsing refactoring round3

[PATCH 4/4] cli: add standard option processing to config, help and setup

[PATCH 3/4] cli: define shared options, use for --help and --version

[PATCH 2/4] cli: refactor notmuch_help_command

[PATCH 1/4] cli: ignore config argument of notmuch_help_command

argument parsing refactoring round3

Re: argument parsing refactoring round3

Re: [PATCH 1/2] test: be consistent about spelling `nonexistent'

Re: argument parsing refactoring round3

[PATCH 1/2] test: be consistent about spelling `nonexistent'

[PATCH] fixup! cli: add standard option processing to config, help and setup

Re: [PATCH 1/2] test: be consistent about spelling `nonexistent'

13 matches

Site Navigation

Mail list logo

Footer information