Xapian-quoting based batch tagging.

Xapian-quoting based batch tagging.

[david]

This is essentially just a rebase of 

     id:1356464567-21779-1-git-send-email-david@tethera.net

with some commit-message fixups for 

[Patch v2 3/9] cli: add support for batch tagging operations to

as suggested by Jani

[Patch v2 1/9] tag-util: factor out rules for illegal tags, use in parse_tag_line

[david]

From: David Bremner <bremner@debian.org>

This will allow us to be consistent between batch tagging and command
line tagging as far as what is an illegal tag.
---
 tag-util.c |   36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/tag-util.c b/tag-util.c
index ca12b3b..0a4fe78 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -31,6 +31,30 @@ line_error (tag_parse_status_t status,
     return status;
 }
 
+/*
+ * Test tags for some forbidden cases.
+ *
+ * return: NULL if OK,
+ *	   explanatory message otherwise.
+ */
+
+static const char *
+illegal_tag (const char *tag, notmuch_bool_t remove)
+{
+
+    if (*tag == '\0' && ! remove)
+	return "empty tag forbidden";
+
+    /* This disallows adding the non-removable tag "-" and
+     * enables notmuch tag to take long options more easily.
+     */
+
+    if (*tag == '-' && ! remove)
+	return "tag starting with '-' forbidden";
+
+    return NULL;
+}
+
 tag_parse_status_t
 parse_tag_line (void *ctx, char *line,
 		tag_op_flag_t flags,
@@ -95,11 +119,13 @@ parse_tag_line (void *ctx, char *line,
 	remove = (*tok == '-');
 	tag = tok + 1;
 
-	/* Maybe refuse empty tags. */
-	if (! (flags & TAG_FLAG_BE_GENEROUS) && *tag == '\0') {
-	    ret = line_error (TAG_PARSE_INVALID, line_for_error,
-			      "empty tag");
-	    goto DONE;
+	/* Maybe refuse illegal tags. */
+	if (! (flags & TAG_FLAG_BE_GENEROUS)) {
+	    const char *msg = illegal_tag (tag, remove);
+	    if (msg) {
+		ret = line_error (TAG_PARSE_INVALID, line_for_error, msg);
+		goto DONE;
+	    }
 	}
 
 	/* Decode tag. */
-- 
1.7.10.4

[Patch v2 2/9] notmuch-tag.c: convert to use tag-utils

[david]

From: David Bremner <bremner@debian.org>

Command line parsing is factored out into a function
parse_tag_command_line in tag-utils.c.

There is some duplicated code eliminated in tag_query, and a bunch of
translation from using the bare tag_op structs to using that tag-utils
API.
---
 notmuch-tag.c |   99 ++++++++++++---------------------------------------------
 tag-util.c    |   51 +++++++++++++++++++++++++++--
 tag-util.h    |   15 +++++++++
 3 files changed, 84 insertions(+), 81 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index fc9d43a..8129912 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -19,6 +19,7 @@
  */
 
 #include "notmuch-client.h"
+#include "tag-util.h"
 #include "string-util.h"
 
 static volatile sig_atomic_t interrupted;
@@ -36,14 +37,10 @@ handle_sigint (unused (int sig))
     interrupted = 1;
 }
 
-typedef struct {
-    const char *tag;
-    notmuch_bool_t remove;
-} tag_operation_t;
 
 static char *
 _optimize_tag_query (void *ctx, const char *orig_query_string,
-		     const tag_operation_t *tag_ops)
+		     const tag_op_list_t *list)
 {
     /* This is subtler than it looks.  Xapian ignores the '-' operator
      * at the beginning both queries and parenthesized groups and,
@@ -60,7 +57,7 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
     size_t i;
 
     /* Don't optimize if there are no tag changes. */
-    if (tag_ops[0].tag == NULL)
+    if (tag_op_list_size (list) == 0)
 	return talloc_strdup (ctx, orig_query_string);
 
     /* Build the new query string */
@@ -69,17 +66,17 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
     else
 	query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
 
-    for (i = 0; tag_ops[i].tag && query_string; i++) {
+    for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
 	/* XXX in case of OOM, query_string will be deallocated when
 	 * ctx is, which might be at shutdown */
 	if (make_boolean_term (ctx,
-			       "tag", tag_ops[i].tag,
+			       "tag", tag_op_list_tag (list, i),
 			       &escaped, &escaped_len))
 	    return NULL;
 
 	query_string = talloc_asprintf_append_buffer (
 	    query_string, "%s%s%s", join,
-	    tag_ops[i].remove ? "" : "not ",
+	    tag_op_list_isremove (list, i) ? "" : "not ",
 	    escaped);
 	join = " or ";
     }
@@ -91,17 +88,15 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
     return query_string;
 }
 
-/* Tag messages matching 'query_string' according to 'tag_ops', which
- * must be an array of tagging operations terminated with an empty
- * element. */
+/* Tag messages matching 'query_string' according to 'tag_ops'
+ */
 static int
 tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
-	   tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags)
+	   tag_op_list_t *tag_ops, tag_op_flag_t flags)
 {
     notmuch_query_t *query;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
-    int i;
 
     /* Optimize the query so it excludes messages that already have
      * the specified set of tags. */
@@ -124,21 +119,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
 	 notmuch_messages_valid (messages) && ! interrupted;
 	 notmuch_messages_move_to_next (messages)) {
 	message = notmuch_messages_get (messages);
-
-	notmuch_message_freeze (message);
-
-	for (i = 0; tag_ops[i].tag; i++) {
-	    if (tag_ops[i].remove)
-		notmuch_message_remove_tag (message, tag_ops[i].tag);
-	    else
-		notmuch_message_add_tag (message, tag_ops[i].tag);
-	}
-
-	notmuch_message_thaw (message);
-
-	if (synchronize_flags)
-	    notmuch_message_tags_to_maildir_flags (message);
-
+	tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);
 	notmuch_message_destroy (message);
     }
 
@@ -150,15 +131,13 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
 int
 notmuch_tag_command (void *ctx, int argc, char *argv[])
 {
-    tag_operation_t *tag_ops;
-    int tag_ops_count = 0;
-    char *query_string;
+    tag_op_list_t *tag_ops = NULL;
+    char *query_string = NULL;
     notmuch_config_t *config;
     notmuch_database_t *notmuch;
     struct sigaction action;
-    notmuch_bool_t synchronize_flags;
-    int i;
-    int ret;
+    tag_op_flag_t tag_flags = TAG_FLAG_NONE;
+    int ret = 0;
 
     /* Setup our handler for SIGINT */
     memset (&action, 0, sizeof (struct sigaction));
@@ -167,54 +146,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
     action.sa_flags = SA_RESTART;
     sigaction (SIGINT, &action, NULL);
 
-    argc--; argv++; /* skip subcommand argument */
-
-    /* Array of tagging operations (add or remove), terminated with an
-     * empty element. */
-    tag_ops = talloc_array (ctx, tag_operation_t, argc + 1);
+    tag_ops = tag_op_list_create (ctx);
     if (tag_ops == NULL) {
 	fprintf (stderr, "Out of memory.\n");
 	return 1;
     }
 
-    for (i = 0; i < argc; i++) {
-	if (strcmp (argv[i], "--") == 0) {
-	    i++;
-	    break;
-	}
-	if (argv[i][0] == '+' || argv[i][0] == '-') {
-	    if (argv[i][0] == '+' && argv[i][1] == '\0') {
-		fprintf (stderr, "Error: tag names cannot be empty.\n");
-		return 1;
-	    }
-	    if (argv[i][0] == '+' && argv[i][1] == '-') {
-		/* This disallows adding the non-removable tag "-" and
-		 * enables notmuch tag to take long options in the
-		 * future. */
-		fprintf (stderr, "Error: tag names must not start with '-'.\n");
-		return 1;
-	    }
-	    tag_ops[tag_ops_count].tag = argv[i] + 1;
-	    tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
-	    tag_ops_count++;
-	} else {
-	    break;
-	}
-    }
-
-    tag_ops[tag_ops_count].tag = NULL;
-
-    if (tag_ops_count == 0) {
-	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
-	return 1;
-    }
-
-    query_string = query_string_from_args (ctx, argc - i, &argv[i]);
-
-    if (*query_string == '\0') {
-	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
+    if (parse_tag_command_line (ctx, argc - 1, argv + 1,
+				&query_string, tag_ops))
 	return 1;
-    }
 
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
@@ -224,9 +164,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
 	return 1;
 
-    synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
+    if (notmuch_config_get_maildir_synchronize_flags (config))
+	tag_flags |= TAG_FLAG_MAILDIR_SYNC;
 
-    ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags);
+    ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
 
     notmuch_database_destroy (notmuch);
 
diff --git a/tag-util.c b/tag-util.c
index 0a4fe78..701d329 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -45,8 +45,9 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
     if (*tag == '\0' && ! remove)
 	return "empty tag forbidden";
 
-    /* This disallows adding the non-removable tag "-" and
-     * enables notmuch tag to take long options more easily.
+    /* This disallows adding tags starting with "-", in particular the
+     * non-removable tag "-" and enables notmuch tag to take long
+     * options more easily.
      */
 
     if (*tag == '-' && ! remove)
@@ -157,6 +158,52 @@ parse_tag_line (void *ctx, char *line,
     return ret;
 }
 
+tag_parse_status_t
+parse_tag_command_line (void *ctx, int argc, char **argv,
+			char **query_str, tag_op_list_t *tag_ops)
+{
+
+    int i;
+
+    tag_op_list_reset (tag_ops);
+
+    for (i = 0; i < argc; i++) {
+	if (strcmp (argv[i], "--") == 0) {
+	    i++;
+	    break;
+	}
+
+	if (argv[i][0] != '+' && argv[i][0] != '-')
+	    break;
+
+	notmuch_bool_t is_remove = argv[i][0] == '-';
+	const char *msg;
+
+	msg = illegal_tag (argv[i] + 1, is_remove);
+	if (msg) {
+	    fprintf (stderr, "Error: %s", msg);
+	    return TAG_PARSE_INVALID;
+	}
+
+	tag_op_list_append (tag_ops, argv[i] + 1, is_remove);
+    }
+
+    if (tag_op_list_size (tag_ops) == 0) {
+	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
+	return TAG_PARSE_INVALID;
+    }
+
+    *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
+
+    if (*query_str == NULL || **query_str == '\0') {
+	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
+	return TAG_PARSE_INVALID;
+    }
+
+    return TAG_PARSE_SUCCESS;
+}
+
+
 static inline void
 message_error (notmuch_message_t *message,
 	       notmuch_status_t status,
diff --git a/tag-util.h b/tag-util.h
index c07bfde..246de85 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -72,6 +72,21 @@ parse_tag_line (void *ctx, char *line,
 		tag_op_flag_t flags,
 		char **query_str, tag_op_list_t *ops);
 
+
+
+/* Parse a command line of the following format:
+ *
+ * +<tag>|-<tag> [...] [--] <search-terms>
+ *
+ * Output Parameters:
+ *	ops	contains a list of tag operations
+ *	query_str the search terms.
+ */
+
+tag_parse_status_t
+parse_tag_command_line (void *ctx, int argc, char **argv,
+			char **query_str, tag_op_list_t *ops);
+
 /*
  * Create an empty list of tag operations
  *
-- 
1.7.10.4

[Patch v2 3/9] cli: add support for batch tagging operations to "notmuch tag"

[david]

From: Jani Nikula <jani@nikula.org>

Add support for batch tagging operations through stdin to "notmuch
tag". This can be enabled with the new --batch command line option to
"notmuch tag". The input must consist of lines of the format:

+<tag>|-<tag> [...] [--] <query> [...]

Each line is interpreted similarly to "notmuch tag" command line
arguments. The delimiter is one or more spaces ' '. Any characters in
<tag> MAY be hex encoded with %NN where NN is the hexadecimal value of
the character. Any ' ' and '%' characters in <tag> and MUST be hex
encoded (using %20 and %25, respectively). For future-proofing, any
'"' characters in <tag> SHOULD be hex-encoded.

Any characters that are not part of <tag> or
MUST NOT be hex encoded.

<query> is passed verbatim to Xapian

Leading and trailing space ' ' is ignored. Empty lines and lines
beginning with '#' are ignored.

Signed-off-by: Jani Nikula <jani@nikula.org>

Hacked-like-crazy-by: David Bremner <david@tethera.net>
---
 notmuch-tag.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 86 insertions(+), 8 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 8129912..7fc614d 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -128,6 +128,46 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
     return interrupted;
 }
 
+static int
+tag_file (void *ctx, notmuch_database_t *notmuch, tag_op_flag_t flags,
+	  FILE *input)
+{
+    char *line = NULL;
+    char *query_string = NULL;
+    size_t line_size = 0;
+    ssize_t line_len;
+    int ret = 0;
+    tag_op_list_t *tag_ops;
+
+    tag_ops = tag_op_list_create (ctx);
+    if (tag_ops == NULL) {
+	fprintf (stderr, "Out of memory.\n");
+	return 1;
+    }
+
+    while ((line_len = getline (&line, &line_size, input)) != -1 &&
+	   ! interrupted) {
+
+	ret = parse_tag_line (ctx, line, TAG_FLAG_NONE,
+			      &query_string, tag_ops);
+
+	if (ret > 0)
+	    continue;
+
+	if (ret < 0)
+	    break;
+
+	ret = tag_query (ctx, notmuch, query_string, tag_ops, flags);
+	if (ret)
+	    break;
+    }
+
+    if (line)
+	free (line);
+
+    return ret;
+}
+
 int
 notmuch_tag_command (void *ctx, int argc, char *argv[])
 {
@@ -137,6 +177,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
     notmuch_database_t *notmuch;
     struct sigaction action;
     tag_op_flag_t tag_flags = TAG_FLAG_NONE;
+    notmuch_bool_t batch = FALSE;
+    FILE *input = stdin;
+    char *input_file_name = NULL;
+    int opt_index;
     int ret = 0;
 
     /* Setup our handler for SIGINT */
@@ -146,15 +190,43 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
     action.sa_flags = SA_RESTART;
     sigaction (SIGINT, &action, NULL);
 
-    tag_ops = tag_op_list_create (ctx);
-    if (tag_ops == NULL) {
-	fprintf (stderr, "Out of memory.\n");
+    notmuch_opt_desc_t options[] = {
+	{ NOTMUCH_OPT_BOOLEAN, &batch, "batch", 0, 0 },
+	{ NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
+	{ 0, 0, 0, 0, 0 }
+    };
+
+    opt_index = parse_arguments (argc, argv, options, 1);
+    if (opt_index < 0)
 	return 1;
+
+    if (input_file_name) {
+	batch = TRUE;
+	input = fopen (input_file_name, "r");
+	if (input == NULL) {
+	    fprintf (stderr, "Error opening %s for reading: %s\n",
+		     input_file_name, strerror (errno));
+	    return 1;
+	}
     }
 
-    if (parse_tag_command_line (ctx, argc - 1, argv + 1,
-				&query_string, tag_ops))
-	return 1;
+    if (batch) {
+	if (opt_index != argc) {
+	    fprintf (stderr, "Can't specify both cmdline and stdin!\n");
+	    return 1;
+	}
+    } else {
+
+	tag_ops = tag_op_list_create (ctx);
+	if (tag_ops == NULL) {
+	    fprintf (stderr, "Out of memory.\n");
+	    return 1;
+	}
+
+	if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
+				    &query_string, tag_ops))
+	    return 1;
+    }
 
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
@@ -167,9 +239,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
     if (notmuch_config_get_maildir_synchronize_flags (config))
 	tag_flags |= TAG_FLAG_MAILDIR_SYNC;
 
-    ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
+    if (batch)
+	ret = tag_file (ctx, notmuch, tag_flags, input);
+    else
+	ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
 
     notmuch_database_destroy (notmuch);
 
-    return ret;
+    if (input != stdin)
+	fclose (input);
+
+    return ret || interrupted;
 }
-- 
1.7.10.4

[Patch v2 4/9] test/tagging: add test for error messages of tag --batch

[david]

From: David Bremner <bremner@debian.org>

This is based on the similar test for notmuch restore, but the parser
in batch tagging mode is less tolerant of a few cases, in particular
those tested by illegal_tag.
---
 test/tagging |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/test/tagging b/test/tagging
index 980ff92..cd16585 100755
--- a/test/tagging
+++ b/test/tagging
@@ -46,6 +46,41 @@ test_expect_equal "$output" "\
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 unread)"
 
+test_begin_subtest '--batch: checking error messages'
+notmuch dump --format=batch-tag > BACKUP
+notmuch tag --batch <<EOF 2>OUTPUT
+# the next line has a space
+ 
+# this line has no tag operations, but this is permitted in batch format.
+a
++0
++a +b
+# trailing whitespace
++a +b 
++c +d --
+# this is a harmless comment, do not yell about it.
+
+# the previous line was blank; also no yelling please
++%zz -- id:whatever
+# the next non-comment line should report an an empty tag error for
+# batch tagging, but not for restore
++ +e -- id:foo
++- -- id:foo
+EOF
+
+cat <<EOF > EXPECTED
+Warning: no query string [+0]
+Warning: no query string [+a +b]
+Warning: missing query string [+a +b ]
+Warning: no query string after -- [+c +d --]
+Warning: hex decoding of tag %zz failed [+%zz -- id:whatever]
+Warning: empty tag forbidden [+ +e -- id:foo]
+Warning: tag starting with '-' forbidden [+- -- id:foo]
+EOF
+
+notmuch restore --format=batch-tag < BACKUP
+test_expect_equal_file EXPECTED OUTPUT
+
 test_expect_code 1 "Empty tag names" 'notmuch tag + One'
 
 test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One'
-- 
1.7.10.4

[Patch v2 5/9] test/tagging: add basic tests for batch tagging functionality

[david]

From: David Bremner <bremner@debian.org>

This tests argument parsing, blank lines and comments, and basic hex
decoding functionality.
---
 test/tagging |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/test/tagging b/test/tagging
index cd16585..405ad7c 100755
--- a/test/tagging
+++ b/test/tagging
@@ -46,6 +46,57 @@ test_expect_equal "$output" "\
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 unread)"
 
+test_begin_subtest "--batch"
+notmuch tag --batch <<EOF
+# %20 is a space in tag
+-:"%20 -tag1 +tag5 +tag6 -- One
++tag1 -tag1 -tag4 +tag4 -- Two
+-tag6 One
++tag5 Two
+EOF
+output=$(notmuch search \* | notmuch_search_sanitize)
+test_expect_equal "$output" "\
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 unread)"
+
+# generate a common input file for the next several tests.
+cat > batch.in  <<EOF
+# %40 is an @ in tag
++%40 -tag5 +tag6 -- One
++tag1 -tag1 -tag4 +tag4 -- Two
+-tag5 +tag6 Two
+EOF
+
+cat > batch.expected <<EOF
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (@ inbox tag6 unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag6 unread)
+EOF
+
+test_begin_subtest "--input"
+notmuch dump --format=batch-tag > backup.tags
+notmuch tag --input=batch.in
+notmuch search \* | notmuch_search_sanitize > OUTPUT
+notmuch restore --format=batch-tag < backup.tags
+test_expect_equal_file batch.expected OUTPUT
+
+test_begin_subtest "--batch --input"
+notmuch dump --format=batch-tag > backup.tags
+notmuch tag --batch --input=batch.in
+notmuch search \* | notmuch_search_sanitize > OUTPUT
+notmuch restore --format=batch-tag < backup.tags
+test_expect_equal_file batch.expected OUTPUT
+
+test_begin_subtest "--batch, blank lines and comments"
+notmuch dump | sort > EXPECTED
+notmuch tag --batch <<EOF
+# this line is a comment; the next has only white space
+ 	 
+
+# the previous line is empty
+EOF
+notmuch dump | sort > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest '--batch: checking error messages'
 notmuch dump --format=batch-tag > BACKUP
 notmuch tag --batch <<EOF 2>OUTPUT
-- 
1.7.10.4

[Patch v2 6/9] test/tagging: add tests for exotic tags

[david]

From: David Bremner <bremner@debian.org>

We test quotes seperately because they matter to the query escaper.
---
 test/tagging |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/test/tagging b/test/tagging
index 405ad7c..417112b 100755
--- a/test/tagging
+++ b/test/tagging
@@ -132,6 +132,72 @@ EOF
 notmuch restore --format=batch-tag < BACKUP
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest '--batch: tags with quotes'
+notmuch dump --format=batch-tag > BACKUP
+
+notmuch tag --batch <<EOF
++%22%27%22%27%22%22%27%27 -- One
+-%22%27%22%27%22%22%27%27 -- One
++%22%27%22%22%22%27 -- One
++%22%27%22%27%22%22%27%27 -- Two
+EOF
+
+cat <<EOF > EXPECTED
++%22%27%22%22%22%27 +inbox +tag5 +unread -- id:msg-001@notmuch-test-suite
++%22%27%22%27%22%22%27%27 +inbox +tag4 +tag5 +unread -- id:msg-002@notmuch-test-suite
+EOF
+
+notmuch dump --format=batch-tag | sort > OUTPUT
+notmuch restore --format=batch-tag < BACKUP
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest '--batch: tags with punctuation and space'
+notmuch dump --format=batch-tag > BACKUP
+
+notmuch tag --batch <<EOF
++%21@%23%24%25%5e%26%2a%29-_=+%5b%7b%5c%20%7c%3b%3a%27%20%22,.%3c%60%7e -- One
+-%21@%23%24%25%5e%26%2a%29-_=+%5b%7b%5c%20%7c%3b%3a%27%20%22,.%3c%60%7e -- One
++%21@%23%24%25%5e%26%2a%29-_=+%5b%7b%5c%20%7c%3b%3a%27%20%22,.%3c%20%60%7e -- Two
+-%21@%23%24%25%5e%26%2a%29-_=+%5b%7b%5c%20%7c%3b%3a%27%20%22,.%3c%20%60%7e -- Two
++%21@%23%20%24%25%5e%26%2a%29-_=+%5b%7b%5c%20%7c%3b%3a%27%20%22,.%3c%60%7e -- One
++%21@%23%20%24%25%5e%26%2a%29-_=+%5b%7b%5c%20%7c%3b%3a%27%20%22,.%3c%60%7e -- Two
+EOF
+
+cat <<EOF > EXPECTED
++%21@%23%20%24%25%5e%26%2a%29-_=+%5b%7b%5c%20%7c%3b%3a%27%20%22,.%3c%60%7e +inbox +tag4 +tag5 +unread -- id:msg-002@notmuch-test-suite
++%21@%23%20%24%25%5e%26%2a%29-_=+%5b%7b%5c%20%7c%3b%3a%27%20%22,.%3c%60%7e +inbox +tag5 +unread -- id:msg-001@notmuch-test-suite
+EOF
+
+notmuch dump --format=batch-tag | sort > OUTPUT
+notmuch restore --format=batch-tag < BACKUP
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest '--batch: unicode tags'
+notmuch dump --format=batch-tag > BACKUP
+
+notmuch tag --batch <<EOF
++%2a@%7d%cf%b5%f4%85%80%adO3%da%a7 -- One
++=%e0%ac%95%c8%b3+%ef%aa%95%c8%a64w%c7%9d%c9%a2%cf%b3%d6%82%24B%c4%a9%c5%a1UX%ee%99%b0%27E7%ca%a4%d0%8b%5d -- One
++A%e1%a0%bc%de%8b%d5%b2V%d9%9b%f3%b5%a2%a3M%d8%a1u@%f0%a0%ac%948%7e%f0%ab%86%af%27 -- One
++R -- One
++%da%88=f%cc%b9I%ce%af%7b%c9%97%e3%b9%8bH%cb%92X%d2%8c6 -- One
++%dc%9crh%d2%86B%e5%97%a2%22t%ed%99%82d -- One
++L%df%85%ef%a1%a5m@%d3%96%c2%ab%d4%9f%ca%b8%f3%b3%a2%bf%c7%b1_u%d7%b4%c7%b1 -- One
++P%c4%98%2f -- One
++%7e%d1%8b%25%ec%a0%ae%d1%a0M%3b%e3%b6%b7%e9%a4%87%3c%db%9a%cc%a8%e1%96%9d -- One
++%c4%bf7%c7%ab9H%c4%99k%ea%91%bd%c3%8ck%e2%b3%8dk%c5%952V%e4%99%b2%d9%b3%e4%8b%bda%5b%24%c7%9b -- One
++%2a@%7d%cf%b5%f4%85%80%adO3%da%a7  +=%e0%ac%95%c8%b3+%ef%aa%95%c8%a64w%c7%9d%c9%a2%cf%b3%d6%82%24B%c4%a9%c5%a1UX%ee%99%b0%27E7%ca%a4%d0%8b%5d  +A%e1%a0%bc%de%8b%d5%b2V%d9%9b%f3%b5%a2%a3M%d8%a1u@%f0%a0%ac%948%7e%f0%ab%86%af%27  +R  +%da%88=f%cc%b9I%ce%af%7b%c9%97%e3%b9%8bH%cb%92X%d2%8c6  +%dc%9crh%d2%86B%e5%97%a2%22t%ed%99%82d  +L%df%85%ef%a1%a5m@%d3%96%c2%ab%d4%9f%ca%b8%f3%b3%a2%bf%c7%b1_u%d7%b4%c7%b1  +P%c4%98%2f  +%7e%d1%8b%25%ec%a0%ae%d1%a0M%3b%e3%b6%b7%e9%a4%87%3c%db%9a%cc%a8%e1%96%9d  +%c4%bf7%c7%ab9H%c4%99k%ea%91%bd%c3%8ck%e2%b3%8dk%c5%952V%e4%99%b2%d9%b3%e4%8b%bda%5b%24%c7%9b -- Two
+EOF
+
+cat <<EOF > EXPECTED
++%2a@%7d%cf%b5%f4%85%80%adO3%da%a7 +=%e0%ac%95%c8%b3+%ef%aa%95%c8%a64w%c7%9d%c9%a2%cf%b3%d6%82%24B%c4%a9%c5%a1UX%ee%99%b0%27E7%ca%a4%d0%8b%5d +A%e1%a0%bc%de%8b%d5%b2V%d9%9b%f3%b5%a2%a3M%d8%a1u@%f0%a0%ac%948%7e%f0%ab%86%af%27 +L%df%85%ef%a1%a5m@%d3%96%c2%ab%d4%9f%ca%b8%f3%b3%a2%bf%c7%b1_u%d7%b4%c7%b1 +P%c4%98%2f +R +inbox +tag4 +tag5 +unread +%7e%d1%8b%25%ec%a0%ae%d1%a0M%3b%e3%b6%b7%e9%a4%87%3c%db%9a%cc%a8%e1%96%9d +%c4%bf7%c7%ab9H%c4%99k%ea%91%bd%c3%8ck%e2%b3%8dk%c5%952V%e4%99%b2%d9%b3%e4%8b%bda%5b%24%c7%9b +%da%88=f%cc%b9I%ce%af%7b%c9%97%e3%b9%8bH%cb%92X%d2%8c6 +%dc%9crh%d2%86B%e5%97%a2%22t%ed%99%82d -- id:msg-002@notmuch-test-suite
++%2a@%7d%cf%b5%f4%85%80%adO3%da%a7 +=%e0%ac%95%c8%b3+%ef%aa%95%c8%a64w%c7%9d%c9%a2%cf%b3%d6%82%24B%c4%a9%c5%a1UX%ee%99%b0%27E7%ca%a4%d0%8b%5d +A%e1%a0%bc%de%8b%d5%b2V%d9%9b%f3%b5%a2%a3M%d8%a1u@%f0%a0%ac%948%7e%f0%ab%86%af%27 +L%df%85%ef%a1%a5m@%d3%96%c2%ab%d4%9f%ca%b8%f3%b3%a2%bf%c7%b1_u%d7%b4%c7%b1 +P%c4%98%2f +R +inbox +tag5 +unread +%7e%d1%8b%25%ec%a0%ae%d1%a0M%3b%e3%b6%b7%e9%a4%87%3c%db%9a%cc%a8%e1%96%9d +%c4%bf7%c7%ab9H%c4%99k%ea%91%bd%c3%8ck%e2%b3%8dk%c5%952V%e4%99%b2%d9%b3%e4%8b%bda%5b%24%c7%9b +%da%88=f%cc%b9I%ce%af%7b%c9%97%e3%b9%8bH%cb%92X%d2%8c6 +%dc%9crh%d2%86B%e5%97%a2%22t%ed%99%82d -- id:msg-001@notmuch-test-suite
+EOF
+
+notmuch dump --format=batch-tag | sort > OUTPUT
+notmuch restore --format=batch-tag < BACKUP
+test_expect_equal_file EXPECTED OUTPUT
+
 test_expect_code 1 "Empty tag names" 'notmuch tag + One'
 
 test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One'
-- 
1.7.10.4

[Patch v2 7/9] test/tagging: add test for exotic message-ids and batch tagging

[david]

From: David Bremner <bremner@debian.org>

The (now fixed) bug that this test revealed is that unquoted
message-ids with whitespace or other control characters in them are
split into several tokens by the Xapian query parser.
---
 test/tagging |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/tagging b/test/tagging
index 417112b..1717e72 100755
--- a/test/tagging
+++ b/test/tagging
@@ -198,6 +198,24 @@ notmuch dump --format=batch-tag | sort > OUTPUT
 notmuch restore --format=batch-tag < BACKUP
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest '--batch: unicode message-ids'
+
+${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG} \
+     --num-messages=100
+
+notmuch dump --format=batch-tag | sed 's/^.* -- /+common_tag -- /' | \
+    sort > EXPECTED
+
+notmuch dump --format=batch-tag | sed 's/^.* -- /  -- /' | \
+    notmuch restore --format=batch-tag
+
+notmuch tag --batch < EXPECTED
+
+notmuch dump --format=batch-tag| \
+    sort > OUTPUT
+
+test_expect_equal_file EXPECTED OUTPUT
+
 test_expect_code 1 "Empty tag names" 'notmuch tag + One'
 
 test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One'
-- 
1.7.10.4

[Patch v2 8/9] man: document notmuch tag --batch, --input options

[david]

From: Jani Nikula <jani@nikula.org>

---
 man/man1/notmuch-tag.1 |   92 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index 9444aa4..3aa2fa5 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -6,6 +6,11 @@ notmuch-tag \- add/remove tags for all messages matching the search terms
 .B notmuch tag
 .RI "+<" tag ">|\-<" tag "> [...] [\-\-] <" search-term "> [...]"
 
+.B notmuch tag
+.RI "--batch"
+.RI "[ --input=<" filename "> ]"
+
+
 .SH DESCRIPTION
 
 Add/remove tags for all messages matching the search terms.
@@ -30,6 +35,93 @@ updates the maildir flags according to tag changes if the
 configuration option is enabled. See \fBnotmuch-config\fR(1) for
 details.
 
+Supported options for
+.B tag
+include
+.RS 4
+.TP 4
+.BR \-\-batch
+
+Read batch tagging operations from a file (stdin by default). This is more
+efficient than repeated
+.B notmuch tag
+invocations. See
+.B TAG FILE FORMAT
+below for the input format. This option is not compatible with
+specifying tagging on the command line.
+.RE
+
+.RS 4
+.TP 4
+.BR "\-\-input=" <filename>
+
+Read input from given file, instead of from stdin. Implies
+.BR --batch .
+
+.SH TAG FILE FORMAT
+
+The input must consist of lines of the format:
+
+.RI "+<" tag ">|\-<" tag "> [...] [\-\-] <" query ">"
+
+Each line is interpreted similarly to
+.B notmuch tag
+command line arguments. The delimiter is one or more spaces ' '. Any
+characters in
+.RI < tag >
+.B may
+be hex-encoded with %NN where NN is the hexadecimal value of the
+character. To hex-encode a character with a multi-byte UTF-8 encoding,
+hex-encode each byte.
+Any spaces in <tag>
+.B must
+be hex-encoded as %20. Any characters that are not
+part of
+.RI  < tag >
+.B must not
+be hex-encoded.
+
+In the future tag:"tag with spaces" style quoting may be supported for
+.RI < tag >
+as well;
+for this reason all double quote characters in
+.RI < tag >
+.B should
+be hex-encoded.
+
+The
+.RI < query >
+should be quoted using Xapian boolean term quoting rules: if a term
+contains whitespace or a close paren or starts with a double quote, it
+must be enclosed in double quotes (not including any prefix) and
+double quotes inside the term must be doubled (see below for
+examples).
+
+Leading and trailing space ' ' is ignored. Empty lines and lines
+beginning with '#' are ignored.
+
+.SS EXAMPLE
+
+The following shows a valid input to batch tagging. Note that only the
+isolated '*' acts as a wildcard. Also note the two different quotings
+of the tag
+.B space in tags
+.
+.RS
+.nf
++winner *
++foo::bar%25 -- (One and Two) or (One and tag:winner)
++found::it -- tag:foo::bar%
+# ignore this line and the next
+
++space%20in%20tags -- Two
+# add tag '(tags)', among other stunts.
++crazy{ +(tags) +&are +#possible\ -- tag:"space in tags"
++match*crazy -- tag:crazy{
++some_tag -- id:"this is ""nauty)"""
+.fi
+.RE
+
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-- 
1.7.10.4

[Patch v2 9/9] test/tagging: add test for naked punctuation in tags; compare with quoting spaces.

[david]

From: David Bremner <bremner@debian.org>

This test also serves as documentation of the quoting
requirements. The comment lines are so that it exactly matches the man
page. Nothing more embarrassing than having an example in the man page
fail.
---
 test/tagging |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/test/tagging b/test/tagging
index 1717e72..1f5632c 100755
--- a/test/tagging
+++ b/test/tagging
@@ -198,6 +198,31 @@ notmuch dump --format=batch-tag | sort > OUTPUT
 notmuch restore --format=batch-tag < BACKUP
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "--batch: only space and % needs to be encoded."
+notmuch dump --format=batch-tag > BACKUP
+
+notmuch tag --batch <<EOF
++winner *
++foo::bar%25 -- (One and Two) or (One and tag:winner)
++found::it -- tag:foo::bar%
+# ignore this line and the next
+
++space%20in%20tags -- Two
+# add tag '(tags)', among other stunts.
++crazy{ +(tags) +&are +#possible\ -- tag:"space in tags"
++match*crazy -- tag:crazy{
++some_tag -- id:"this is ""nauty)"""
+EOF
+
+cat <<EOF > EXPECTED
++%23possible%5c +%26are +%28tags%29 +crazy%7b +inbox +match%2acrazy +space%20in%20tags +tag4 +tag5 +unread +winner -- id:msg-002@notmuch-test-suite
++foo%3a%3abar%25 +found%3a%3ait +inbox +tag5 +unread +winner -- id:msg-001@notmuch-test-suite
+EOF
+
+notmuch dump --format=batch-tag | sort > OUTPUT
+notmuch restore --format=batch-tag < BACKUP
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest '--batch: unicode message-ids'
 
 ${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG} \
-- 
1.7.10.4

Re: [Patch v2 2/9] notmuch-tag.c: convert to use tag-utils

[Jani Nikula]

On Mon, 07 Jan 2013, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> Command line parsing is factored out into a function
> parse_tag_command_line in tag-utils.c.
>
> There is some duplicated code eliminated in tag_query, and a bunch of
> translation from using the bare tag_op structs to using that tag-utils
> API.
> ---
>  notmuch-tag.c |   99 ++++++++++++---------------------------------------------
>  tag-util.c    |   51 +++++++++++++++++++++++++++--
>  tag-util.h    |   15 +++++++++
>  3 files changed, 84 insertions(+), 81 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index fc9d43a..8129912 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "notmuch-client.h"
> +#include "tag-util.h"
>  #include "string-util.h"
>  
>  static volatile sig_atomic_t interrupted;
> @@ -36,14 +37,10 @@ handle_sigint (unused (int sig))
>      interrupted = 1;
>  }
>  
> -typedef struct {
> -    const char *tag;
> -    notmuch_bool_t remove;
> -} tag_operation_t;
>  
>  static char *
>  _optimize_tag_query (void *ctx, const char *orig_query_string,
> -		     const tag_operation_t *tag_ops)
> +		     const tag_op_list_t *list)
>  {
>      /* This is subtler than it looks.  Xapian ignores the '-' operator
>       * at the beginning both queries and parenthesized groups and,
> @@ -60,7 +57,7 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>      size_t i;
>  
>      /* Don't optimize if there are no tag changes. */
> -    if (tag_ops[0].tag == NULL)
> +    if (tag_op_list_size (list) == 0)
>  	return talloc_strdup (ctx, orig_query_string);
>  
>      /* Build the new query string */
> @@ -69,17 +66,17 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>      else
>  	query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
>  
> -    for (i = 0; tag_ops[i].tag && query_string; i++) {
> +    for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
>  	/* XXX in case of OOM, query_string will be deallocated when
>  	 * ctx is, which might be at shutdown */
>  	if (make_boolean_term (ctx,
> -			       "tag", tag_ops[i].tag,
> +			       "tag", tag_op_list_tag (list, i),
>  			       &escaped, &escaped_len))
>  	    return NULL;
>  
>  	query_string = talloc_asprintf_append_buffer (
>  	    query_string, "%s%s%s", join,
> -	    tag_ops[i].remove ? "" : "not ",
> +	    tag_op_list_isremove (list, i) ? "" : "not ",
>  	    escaped);
>  	join = " or ";
>      }
> @@ -91,17 +88,15 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>      return query_string;
>  }
>  
> -/* Tag messages matching 'query_string' according to 'tag_ops', which
> - * must be an array of tagging operations terminated with an empty
> - * element. */
> +/* Tag messages matching 'query_string' according to 'tag_ops'
> + */
>  static int
>  tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
> -	   tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags)
> +	   tag_op_list_t *tag_ops, tag_op_flag_t flags)
>  {
>      notmuch_query_t *query;
>      notmuch_messages_t *messages;
>      notmuch_message_t *message;
> -    int i;
>  
>      /* Optimize the query so it excludes messages that already have
>       * the specified set of tags. */
> @@ -124,21 +119,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
>  	 notmuch_messages_valid (messages) && ! interrupted;
>  	 notmuch_messages_move_to_next (messages)) {
>  	message = notmuch_messages_get (messages);
> -
> -	notmuch_message_freeze (message);
> -
> -	for (i = 0; tag_ops[i].tag; i++) {
> -	    if (tag_ops[i].remove)
> -		notmuch_message_remove_tag (message, tag_ops[i].tag);
> -	    else
> -		notmuch_message_add_tag (message, tag_ops[i].tag);
> -	}
> -
> -	notmuch_message_thaw (message);
> -
> -	if (synchronize_flags)
> -	    notmuch_message_tags_to_maildir_flags (message);
> -
> +	tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);

I think we probably want to check for errors and bail out on them
here. It's a functional change, so it belongs in a follow-up patch. We
haven't done it before, but it'll be more important with batch tagging.

BR,
Jani.


>  	notmuch_message_destroy (message);
>      }
>  
> @@ -150,15 +131,13 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
>  int
>  notmuch_tag_command (void *ctx, int argc, char *argv[])
>  {
> -    tag_operation_t *tag_ops;
> -    int tag_ops_count = 0;
> -    char *query_string;
> +    tag_op_list_t *tag_ops = NULL;
> +    char *query_string = NULL;
>      notmuch_config_t *config;
>      notmuch_database_t *notmuch;
>      struct sigaction action;
> -    notmuch_bool_t synchronize_flags;
> -    int i;
> -    int ret;
> +    tag_op_flag_t tag_flags = TAG_FLAG_NONE;
> +    int ret = 0;
>  
>      /* Setup our handler for SIGINT */
>      memset (&action, 0, sizeof (struct sigaction));
> @@ -167,54 +146,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>      action.sa_flags = SA_RESTART;
>      sigaction (SIGINT, &action, NULL);
>  
> -    argc--; argv++; /* skip subcommand argument */
> -
> -    /* Array of tagging operations (add or remove), terminated with an
> -     * empty element. */
> -    tag_ops = talloc_array (ctx, tag_operation_t, argc + 1);
> +    tag_ops = tag_op_list_create (ctx);
>      if (tag_ops == NULL) {
>  	fprintf (stderr, "Out of memory.\n");
>  	return 1;
>      }
>  
> -    for (i = 0; i < argc; i++) {
> -	if (strcmp (argv[i], "--") == 0) {
> -	    i++;
> -	    break;
> -	}
> -	if (argv[i][0] == '+' || argv[i][0] == '-') {
> -	    if (argv[i][0] == '+' && argv[i][1] == '\0') {
> -		fprintf (stderr, "Error: tag names cannot be empty.\n");
> -		return 1;
> -	    }
> -	    if (argv[i][0] == '+' && argv[i][1] == '-') {
> -		/* This disallows adding the non-removable tag "-" and
> -		 * enables notmuch tag to take long options in the
> -		 * future. */
> -		fprintf (stderr, "Error: tag names must not start with '-'.\n");
> -		return 1;
> -	    }
> -	    tag_ops[tag_ops_count].tag = argv[i] + 1;
> -	    tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
> -	    tag_ops_count++;
> -	} else {
> -	    break;
> -	}
> -    }
> -
> -    tag_ops[tag_ops_count].tag = NULL;
> -
> -    if (tag_ops_count == 0) {
> -	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
> -	return 1;
> -    }
> -
> -    query_string = query_string_from_args (ctx, argc - i, &argv[i]);
> -
> -    if (*query_string == '\0') {
> -	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
> +    if (parse_tag_command_line (ctx, argc - 1, argv + 1,
> +				&query_string, tag_ops))
>  	return 1;
> -    }
>  
>      config = notmuch_config_open (ctx, NULL, NULL);
>      if (config == NULL)
> @@ -224,9 +164,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>  			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
>  	return 1;
>  
> -    synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
> +    if (notmuch_config_get_maildir_synchronize_flags (config))
> +	tag_flags |= TAG_FLAG_MAILDIR_SYNC;
>  
> -    ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags);
> +    ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
>  
>      notmuch_database_destroy (notmuch);
>  
> diff --git a/tag-util.c b/tag-util.c
> index 0a4fe78..701d329 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -45,8 +45,9 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
>      if (*tag == '\0' && ! remove)
>  	return "empty tag forbidden";
>  
> -    /* This disallows adding the non-removable tag "-" and
> -     * enables notmuch tag to take long options more easily.
> +    /* This disallows adding tags starting with "-", in particular the
> +     * non-removable tag "-" and enables notmuch tag to take long
> +     * options more easily.
>       */
>  
>      if (*tag == '-' && ! remove)
> @@ -157,6 +158,52 @@ parse_tag_line (void *ctx, char *line,
>      return ret;
>  }
>  
> +tag_parse_status_t
> +parse_tag_command_line (void *ctx, int argc, char **argv,
> +			char **query_str, tag_op_list_t *tag_ops)
> +{
> +
> +    int i;
> +
> +    tag_op_list_reset (tag_ops);
> +
> +    for (i = 0; i < argc; i++) {
> +	if (strcmp (argv[i], "--") == 0) {
> +	    i++;
> +	    break;
> +	}
> +
> +	if (argv[i][0] != '+' && argv[i][0] != '-')
> +	    break;
> +
> +	notmuch_bool_t is_remove = argv[i][0] == '-';
> +	const char *msg;
> +
> +	msg = illegal_tag (argv[i] + 1, is_remove);
> +	if (msg) {
> +	    fprintf (stderr, "Error: %s", msg);
> +	    return TAG_PARSE_INVALID;
> +	}
> +
> +	tag_op_list_append (tag_ops, argv[i] + 1, is_remove);
> +    }
> +
> +    if (tag_op_list_size (tag_ops) == 0) {
> +	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
> +	return TAG_PARSE_INVALID;
> +    }
> +
> +    *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
> +
> +    if (*query_str == NULL || **query_str == '\0') {
> +	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
> +	return TAG_PARSE_INVALID;
> +    }
> +
> +    return TAG_PARSE_SUCCESS;
> +}
> +
> +
>  static inline void
>  message_error (notmuch_message_t *message,
>  	       notmuch_status_t status,
> diff --git a/tag-util.h b/tag-util.h
> index c07bfde..246de85 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -72,6 +72,21 @@ parse_tag_line (void *ctx, char *line,
>  		tag_op_flag_t flags,
>  		char **query_str, tag_op_list_t *ops);
>  
> +
> +
> +/* Parse a command line of the following format:
> + *
> + * +<tag>|-<tag> [...] [--] <search-terms>
> + *
> + * Output Parameters:
> + *	ops	contains a list of tag operations
> + *	query_str the search terms.
> + */
> +
> +tag_parse_status_t
> +parse_tag_command_line (void *ctx, int argc, char **argv,
> +			char **query_str, tag_op_list_t *ops);
> +
>  /*
>   * Create an empty list of tag operations
>   *
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: Xapian-quoting based batch tagging.

[Jani Nikula]

On Mon, 07 Jan 2013, david@tethera.net wrote:
> This is essentially just a rebase of 
>
>      id:1356464567-21779-1-git-send-email-david@tethera.net
>
> with some commit-message fixups for 
>
> [Patch v2 3/9] cli: add support for batch tagging operations to
>
> as suggested by Jani

The series LGTM,
Jani.

Re: [Patch v2 2/9] notmuch-tag.c: convert to use tag-utils

[Tomi Ollila]

On Mon, Jan 07 2013, Jani Nikula <jani@nikula.org> wrote:

> On Mon, 07 Jan 2013, david@tethera.net wrote:
>> From: David Bremner <bremner@debian.org>
>>
>> Command line parsing is factored out into a function
>> parse_tag_command_line in tag-utils.c.
>>
>> There is some duplicated code eliminated in tag_query, and a bunch of
>> translation from using the bare tag_op structs to using that tag-utils
>> API.
>> ---
>>  notmuch-tag.c |   99 ++++++++++++---------------------------------------------
>>  tag-util.c    |   51 +++++++++++++++++++++++++++--
>>  tag-util.h    |   15 +++++++++
>>  3 files changed, 84 insertions(+), 81 deletions(-)
>>
>> diff --git a/notmuch-tag.c b/notmuch-tag.c
>> index fc9d43a..8129912 100644
>> --- a/notmuch-tag.c
>> +++ b/notmuch-tag.c
>> @@ -19,6 +19,7 @@
>>   */
>>  
>>  #include "notmuch-client.h"
>> +#include "tag-util.h"
>>  #include "string-util.h"
>>  
>>  static volatile sig_atomic_t interrupted;
>> @@ -36,14 +37,10 @@ handle_sigint (unused (int sig))
>>      interrupted = 1;
>>  }
>>  
>> -typedef struct {
>> -    const char *tag;
>> -    notmuch_bool_t remove;
>> -} tag_operation_t;
>>  
>>  static char *
>>  _optimize_tag_query (void *ctx, const char *orig_query_string,
>> -		     const tag_operation_t *tag_ops)
>> +		     const tag_op_list_t *list)
>>  {
>>      /* This is subtler than it looks.  Xapian ignores the '-' operator
>>       * at the beginning both queries and parenthesized groups and,
>> @@ -60,7 +57,7 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>>      size_t i;
>>  
>>      /* Don't optimize if there are no tag changes. */
>> -    if (tag_ops[0].tag == NULL)
>> +    if (tag_op_list_size (list) == 0)
>>  	return talloc_strdup (ctx, orig_query_string);
>>  
>>      /* Build the new query string */
>> @@ -69,17 +66,17 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>>      else
>>  	query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
>>  
>> -    for (i = 0; tag_ops[i].tag && query_string; i++) {
>> +    for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
>>  	/* XXX in case of OOM, query_string will be deallocated when
>>  	 * ctx is, which might be at shutdown */
>>  	if (make_boolean_term (ctx,
>> -			       "tag", tag_ops[i].tag,
>> +			       "tag", tag_op_list_tag (list, i),
>>  			       &escaped, &escaped_len))
>>  	    return NULL;
>>  
>>  	query_string = talloc_asprintf_append_buffer (
>>  	    query_string, "%s%s%s", join,
>> -	    tag_ops[i].remove ? "" : "not ",
>> +	    tag_op_list_isremove (list, i) ? "" : "not ",
>>  	    escaped);
>>  	join = " or ";
>>      }
>> @@ -91,17 +88,15 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>>      return query_string;
>>  }
>>  
>> -/* Tag messages matching 'query_string' according to 'tag_ops', which
>> - * must be an array of tagging operations terminated with an empty
>> - * element. */
>> +/* Tag messages matching 'query_string' according to 'tag_ops'
>> + */
>>  static int
>>  tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
>> -	   tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags)
>> +	   tag_op_list_t *tag_ops, tag_op_flag_t flags)
>>  {
>>      notmuch_query_t *query;
>>      notmuch_messages_t *messages;
>>      notmuch_message_t *message;
>> -    int i;
>>  
>>      /* Optimize the query so it excludes messages that already have
>>       * the specified set of tags. */
>> @@ -124,21 +119,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
>>  	 notmuch_messages_valid (messages) && ! interrupted;
>>  	 notmuch_messages_move_to_next (messages)) {
>>  	message = notmuch_messages_get (messages);
>> -
>> -	notmuch_message_freeze (message);
>> -
>> -	for (i = 0; tag_ops[i].tag; i++) {
>> -	    if (tag_ops[i].remove)
>> -		notmuch_message_remove_tag (message, tag_ops[i].tag);
>> -	    else
>> -		notmuch_message_add_tag (message, tag_ops[i].tag);
>> -	}
>> -
>> -	notmuch_message_thaw (message);
>> -
>> -	if (synchronize_flags)
>> -	    notmuch_message_tags_to_maildir_flags (message);
>> -
>> +	tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);
>
> I think we probably want to check for errors and bail out on them
> here. It's a functional change, so it belongs in a follow-up patch. We
> haven't done it before, but it'll be more important with batch tagging.


I marked this message as notmuch::bug so it doesn't get forgotten.

Now there is this lengthy message shown in nmbug status page with subject
Re: [Patch v2 2/9] notmuch-tag.c: convert to use tag-utils and Jani's 
name on it ;) 

I guess we can expect everyone checking nmbug page regularly for messages
with their name :D

> BR,
> Jani.


Thanks,

Tomi

>
>
>>  	notmuch_message_destroy (message);
>>      }
>>  
>> @@ -150,15 +131,13 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
>>  int
>>  notmuch_tag_command (void *ctx, int argc, char *argv[])
>>  {
>> -    tag_operation_t *tag_ops;
>> -    int tag_ops_count = 0;
>> -    char *query_string;
>> +    tag_op_list_t *tag_ops = NULL;
>> +    char *query_string = NULL;
>>      notmuch_config_t *config;
>>      notmuch_database_t *notmuch;
>>      struct sigaction action;
>> -    notmuch_bool_t synchronize_flags;
>> -    int i;
>> -    int ret;
>> +    tag_op_flag_t tag_flags = TAG_FLAG_NONE;
>> +    int ret = 0;
>>  
>>      /* Setup our handler for SIGINT */
>>      memset (&action, 0, sizeof (struct sigaction));
>> @@ -167,54 +146,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>>      action.sa_flags = SA_RESTART;
>>      sigaction (SIGINT, &action, NULL);
>>  
>> -    argc--; argv++; /* skip subcommand argument */
>> -
>> -    /* Array of tagging operations (add or remove), terminated with an
>> -     * empty element. */
>> -    tag_ops = talloc_array (ctx, tag_operation_t, argc + 1);
>> +    tag_ops = tag_op_list_create (ctx);
>>      if (tag_ops == NULL) {
>>  	fprintf (stderr, "Out of memory.\n");
>>  	return 1;
>>      }
>>  
>> -    for (i = 0; i < argc; i++) {
>> -	if (strcmp (argv[i], "--") == 0) {
>> -	    i++;
>> -	    break;
>> -	}
>> -	if (argv[i][0] == '+' || argv[i][0] == '-') {
>> -	    if (argv[i][0] == '+' && argv[i][1] == '\0') {
>> -		fprintf (stderr, "Error: tag names cannot be empty.\n");
>> -		return 1;
>> -	    }
>> -	    if (argv[i][0] == '+' && argv[i][1] == '-') {
>> -		/* This disallows adding the non-removable tag "-" and
>> -		 * enables notmuch tag to take long options in the
>> -		 * future. */
>> -		fprintf (stderr, "Error: tag names must not start with '-'.\n");
>> -		return 1;
>> -	    }
>> -	    tag_ops[tag_ops_count].tag = argv[i] + 1;
>> -	    tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
>> -	    tag_ops_count++;
>> -	} else {
>> -	    break;
>> -	}
>> -    }
>> -
>> -    tag_ops[tag_ops_count].tag = NULL;
>> -
>> -    if (tag_ops_count == 0) {
>> -	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
>> -	return 1;
>> -    }
>> -
>> -    query_string = query_string_from_args (ctx, argc - i, &argv[i]);
>> -
>> -    if (*query_string == '\0') {
>> -	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
>> +    if (parse_tag_command_line (ctx, argc - 1, argv + 1,
>> +				&query_string, tag_ops))
>>  	return 1;
>> -    }
>>  
>>      config = notmuch_config_open (ctx, NULL, NULL);
>>      if (config == NULL)
>> @@ -224,9 +164,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>>  			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
>>  	return 1;
>>  
>> -    synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
>> +    if (notmuch_config_get_maildir_synchronize_flags (config))
>> +	tag_flags |= TAG_FLAG_MAILDIR_SYNC;
>>  
>> -    ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags);
>> +    ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
>>  
>>      notmuch_database_destroy (notmuch);
>>  
>> diff --git a/tag-util.c b/tag-util.c
>> index 0a4fe78..701d329 100644
>> --- a/tag-util.c
>> +++ b/tag-util.c
>> @@ -45,8 +45,9 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
>>      if (*tag == '\0' && ! remove)
>>  	return "empty tag forbidden";
>>  
>> -    /* This disallows adding the non-removable tag "-" and
>> -     * enables notmuch tag to take long options more easily.
>> +    /* This disallows adding tags starting with "-", in particular the
>> +     * non-removable tag "-" and enables notmuch tag to take long
>> +     * options more easily.
>>       */
>>  
>>      if (*tag == '-' && ! remove)
>> @@ -157,6 +158,52 @@ parse_tag_line (void *ctx, char *line,
>>      return ret;
>>  }
>>  
>> +tag_parse_status_t
>> +parse_tag_command_line (void *ctx, int argc, char **argv,
>> +			char **query_str, tag_op_list_t *tag_ops)
>> +{
>> +
>> +    int i;
>> +
>> +    tag_op_list_reset (tag_ops);
>> +
>> +    for (i = 0; i < argc; i++) {
>> +	if (strcmp (argv[i], "--") == 0) {
>> +	    i++;
>> +	    break;
>> +	}
>> +
>> +	if (argv[i][0] != '+' && argv[i][0] != '-')
>> +	    break;
>> +
>> +	notmuch_bool_t is_remove = argv[i][0] == '-';
>> +	const char *msg;
>> +
>> +	msg = illegal_tag (argv[i] + 1, is_remove);
>> +	if (msg) {
>> +	    fprintf (stderr, "Error: %s", msg);
>> +	    return TAG_PARSE_INVALID;
>> +	}
>> +
>> +	tag_op_list_append (tag_ops, argv[i] + 1, is_remove);
>> +    }
>> +
>> +    if (tag_op_list_size (tag_ops) == 0) {
>> +	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
>> +	return TAG_PARSE_INVALID;
>> +    }
>> +
>> +    *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
>> +
>> +    if (*query_str == NULL || **query_str == '\0') {
>> +	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
>> +	return TAG_PARSE_INVALID;
>> +    }
>> +
>> +    return TAG_PARSE_SUCCESS;
>> +}
>> +
>> +
>>  static inline void
>>  message_error (notmuch_message_t *message,
>>  	       notmuch_status_t status,
>> diff --git a/tag-util.h b/tag-util.h
>> index c07bfde..246de85 100644
>> --- a/tag-util.h
>> +++ b/tag-util.h
>> @@ -72,6 +72,21 @@ parse_tag_line (void *ctx, char *line,
>>  		tag_op_flag_t flags,
>>  		char **query_str, tag_op_list_t *ops);
>>  
>> +
>> +
>> +/* Parse a command line of the following format:
>> + *
>> + * +<tag>|-<tag> [...] [--] <search-terms>
>> + *
>> + * Output Parameters:
>> + *	ops	contains a list of tag operations
>> + *	query_str the search terms.
>> + */
>> +
>> +tag_parse_status_t
>> +parse_tag_command_line (void *ctx, int argc, char **argv,
>> +			char **query_str, tag_op_list_t *ops);
>> +
>>  /*
>>   * Create an empty list of tag operations
>>   *
>> -- 
>> 1.7.10.4
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: Xapian-quoting based batch tagging.

[David Bremner]

david@tethera.net writes:

> This is essentially just a rebase of 
>
>      id:1356464567-21779-1-git-send-email-david@tethera.net
>
> with some commit-message fixups for 
>
> [Patch v2 3/9] cli: add support for batch tagging operations to

pushed, with one blank line deleted, and one plural corrected, per Tomi
on IRC.

d