V6 batch tagging patches.

V6 batch tagging patches.

[david]

This obsoletes the remaining patches in 

     id:1353792017-31459-1-git-send-email-david@tethera.net

This isn't really v6 of these particular patches, but oh well.

[Patch v6 1/6] 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 |   35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/tag-util.c b/tag-util.c
index eab482f..c071ea8 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -31,6 +31,29 @@ 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 "adding empty tag";
+
+    /* This disallows adding the non-removable tag "-" and
+     * enables notmuch tag to take long options more easily.
+     */
+
+    if (*tag == '-' && !remove)
+	return  "adding tag starting with -";
+
+    return NULL;
+}
+
 tag_parse_status_t
 parse_tag_line (void *ctx, char *line,
 		tag_op_flag_t flags,
@@ -95,11 +118,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 v6 2/6] 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 |  104 +++++++++++++--------------------------------------------
 tag-util.c    |   48 ++++++++++++++++++++++++++
 tag-util.h    |   16 +++++++++
 3 files changed, 87 insertions(+), 81 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 88d559b..b5cd578 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -19,6 +19,7 @@
  */
 
 #include "notmuch-client.h"
+#include "tag-util.h"
 
 static volatile sig_atomic_t interrupted;
 
@@ -54,14 +55,9 @@ _escape_tag (char *buf, const char *tag)
     return buf;
 }
 
-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,
@@ -73,19 +69,20 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
 
     char *escaped, *query_string;
     const char *join = "";
-    int i;
+    size_t i;
     unsigned int max_tag_len = 0;
 
     /* 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);
 
     /* Allocate a buffer for escaping tags.  This is large enough to
      * hold a fully escaped tag with every character doubled plus
      * enclosing quotes and a NUL. */
-    for (i = 0; tag_ops[i].tag; i++)
-	if (strlen (tag_ops[i].tag) > max_tag_len)
-	    max_tag_len = strlen (tag_ops[i].tag);
+    for (i = 0; i < tag_op_list_size (list); i++)
+	if (strlen (tag_op_list_tag (list, i)) > max_tag_len)
+	    max_tag_len = strlen (tag_op_list_tag (list, i));
+
     escaped = talloc_array (ctx, char, max_tag_len * 2 + 3);
     if (! escaped)
 	return NULL;
@@ -96,11 +93,11 @@ _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++) {
 	query_string = talloc_asprintf_append_buffer (
 	    query_string, "%s%stag:%s", join,
-	    tag_ops[i].remove ? "" : "not ",
-	    _escape_tag (escaped, tag_ops[i].tag));
+	    tag_op_list_isremove (list, i) ? "" : "not ",
+	    _escape_tag (escaped, tag_op_list_tag (list, i)));
 	join = " or ";
     }
 
@@ -116,12 +113,11 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
  * element. */
 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. */
@@ -144,21 +140,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);
     }
 
@@ -170,15 +152,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));
@@ -187,54 +167,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,
+				0, &query_string, tag_ops))
 	return 1;
-    }
 
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
@@ -244,9 +185,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 c071ea8..6e3c069 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -162,6 +162,54 @@ parse_tag_line (void *ctx, char *line,
     return ret;
 }
 
+int
+parse_tag_command_line (void *ctx, int argc, char **argv,
+			unused(tag_op_flag_t flags),
+			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] == '-') {
+	    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 1;
+	    }
+
+	    tag_op_list_append (ctx, tag_ops,
+				argv[i] + 1, (argv[i][0] == '-'));
+	} else {
+	    break;
+	}
+    }
+
+    if (tag_op_list_size (tag_ops) == 0) {
+	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
+	return 1;
+    }
+
+    *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
+
+    if (**query_str == '\0') {
+	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
+	return 1;
+    }
+
+    return 0;
+}
+
+
 static inline void
 message_error (notmuch_message_t *message,
 	       notmuch_status_t status,
diff --git a/tag-util.h b/tag-util.h
index 99b0fa0..4956725 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -72,6 +72,22 @@ 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,
+			tag_op_flag_t flags,
+			char **query_str, tag_op_list_t *ops);
+
 /*
  * Create an empty list of tag operations
  *
-- 
1.7.10.4

[Patch v6 3/6] 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> [...] [--] <search-terms>

Each line is interpreted similarly to "notmuch tag" command line
arguments. The delimiter is one or more spaces ' '. Any characters in
<tag> and <search-terms> MAY be hex encoded with %NN where NN is the
hexadecimal value of the character. Any ' ' and '%' characters in
<tag> and <search-terms> MUST be hex encoded (using %20 and %25,
respectively). Any characters that are not part of <tag> or
<search-terms> MUST NOT be hex encoded.

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 |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 80 insertions(+), 8 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index b5cd578..2665037 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -149,6 +149,43 @@ 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 || tag_query (ctx, notmuch, query_string,
+				      tag_ops, flags))
+	    break;
+    }
+
+    if (line)
+	free (line);
+
+    return ret;
+}
+
 int
 notmuch_tag_command (void *ctx, int argc, char *argv[])
 {
@@ -158,6 +195,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 */
@@ -167,15 +208,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,
-				0, &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,
+				    0, &query_string, tag_ops))
+	    return 1;
+    }
 
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
@@ -188,9 +257,12 @@ 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;
+    return ret || interrupted;
 }
-- 
1.7.10.4

[Patch v6 4/6] test: add test for notmuch tag --batch option

[david]

From: Jani Nikula <jani@nikula.org>

Basic test of functionality, along with all combinations of options.

Modified extensively by David Bremner <david@tethera.net>

The choice of @ as a tag is intended to be non-alphanumeric, but still
not too much trouble in the shell and in the old sup dump/restore format.
---
 test/tagging |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/test/tagging b/test/tagging
index 980ff92..0f3d797 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.$test_count
+notmuch restore --format=batch-tag < backup.tags
+test_expect_equal_file batch.expected OUTPUT.$test_count
+
+test_begin_subtest "--batch --input"
+notmuch dump --format=batch-tag > backup.tags
+notmuch tag --batch --input=batch.in
+notmuch search \* | notmuch_search_sanitize > OUTPUT.$test_count
+notmuch restore --format=batch-tag < backup.tags
+test_expect_equal_file batch.expected OUTPUT.$test_count
+
+test_begin_subtest "--batch, blank lines and comments"
+notmuch dump | sort > EXPECTED.$test_count
+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_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
 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 v6 5/6] notmuch-tag.1: tidy synopsis formatting

[david]

From: David Bremner <bremner@debian.org>

Consistently use [...]; one less space.
---
 man/man1/notmuch-tag.1 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index 0f86582..d23700d 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -4,7 +4,7 @@ notmuch-tag \- add/remove tags for all messages matching the search terms
 
 .SH SYNOPSIS
 .B notmuch tag
-.RI  "+<" tag> "|\-<" tag "> [...] [\-\-] <" search-term ">..."
+.RI "+<" tag ">|\-<" tag "> [...] [\-\-] <" search-term "> [...]"
 
 .SH DESCRIPTION
 
-- 
1.7.10.4

[Patch v6 6/6] man: document notmuch tag --batch, --input options

[david]

From: Jani Nikula <jani@nikula.org>

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

diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index d23700d..c4e60d3 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.
@@ -29,6 +34,51 @@ 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 "> [...] [\-\-] <" search-term "> [...]"
+
+Each line is interpreted similarly to
+.B notmuch tag
+command line arguments. The delimiter is one or more spaces ' '. Any
+characters in <tag> and <search-term>
+.B may
+be hex encoded with %NN where NN is the hexadecimal value of the
+character. Any ' ' and '%' characters in <tag> and <search-terms>
+.B must
+be hex encoded (using %20 and %25, respectively). Any characters that
+are not part of <tag> or <search-terms>
+.B must not
+be hex encoded.
+
+Leading and trailing space ' ' is ignored. Empty lines and lines
+beginning with '#' are ignored.
+
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-- 
1.7.10.4

Re: V6 batch tagging patches.

[David Bremner]

david@tethera.net writes:

> This obsoletes the remaining patches in 
>
>      id:1353792017-31459-1-git-send-email-david@tethera.net
>
> This isn't really v6 of these particular patches, but oh well.

I forgot to mention that I will probably add some more tests; several of
the dump/restore batch-tag format tests can be re-used.

Here is a log of changes for the last set of reviews.

commit 120b1aef754cbe969e0421d5557f0308381f73d2
Author: David Bremner <bremner@debian.org>
Date:   Sun Dec 9 15:57:10 2012 -0400

    fixup for id:87txs5qqeb.fsf@nikula.org

diff --git a/notmuch-tag.c b/notmuch-tag.c
index dbd98a0..b8732f6 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -169,7 +169,7 @@ tag_file (void *ctx, notmuch_database_t *notmuch, tag_op_flag_t flags,
     while ((line_len = getline (&line, &line_size, input)) != -1 &&
 	   ! interrupted) {
 
-	ret =  parse_tag_line (ctx, line, TAG_FLAG_NONE,
+	ret = parse_tag_line (ctx, line, TAG_FLAG_NONE,
 			       &query_string, tag_ops);
 
 	if (ret > 0)
@@ -194,7 +194,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
     notmuch_config_t *config;
     notmuch_database_t *notmuch;
     struct sigaction action;
-    tag_op_flag_t synchronize_flags = TAG_FLAG_NONE;
+    tag_op_flag_t tag_flags = TAG_FLAG_NONE;
     notmuch_bool_t batch = FALSE;
     FILE *input = stdin;
     char *input_file_name = NULL;
@@ -255,12 +255,12 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 	return 1;
 
     if (notmuch_config_get_maildir_synchronize_flags (config))
-	synchronize_flags = TAG_FLAG_MAILDIR_SYNC;
+	tag_flags |= TAG_FLAG_MAILDIR_SYNC;
 
     if (batch)
-	ret = tag_file (ctx, notmuch, synchronize_flags, input);
+	ret = tag_file (ctx, notmuch, tag_flags, input);
     else
-	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);
 

commit bf8d4de2b174cff389d5f5fe4bdd10df2323fe77
Author: David Bremner <bremner@debian.org>
Date:   Sun Dec 9 16:19:32 2012 -0400

    changes for id:87pq2tqpvq.fsf@nikula.org

diff --git a/test/tagging b/test/tagging
index 7155e47..0f3d797 100755
--- a/test/tagging
+++ b/test/tagging
@@ -59,8 +59,9 @@ 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
-# %20 is a space in tag
+# %40 is an @ in tag
 +%40 -tag5 +tag6 -- One
 +tag1 -tag1 -tag4 +tag4 -- Two
 -tag5 +tag6 Two
@@ -72,18 +73,18 @@ thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag6 unread)
 EOF
 
 test_begin_subtest "--input"
-notmuch dump > backup.tags
+notmuch dump --format=batch-tag > backup.tags
 notmuch tag --input=batch.in
-notmuch search \* | notmuch_search_sanitize > OUTPUT
-notmuch restore < backup.tags
-test_expect_equal_file OUTPUT batch.expected
+notmuch search \* | notmuch_search_sanitize > OUTPUT.$test_count
+notmuch restore --format=batch-tag < backup.tags
+test_expect_equal_file batch.expected OUTPUT.$test_count
 
 test_begin_subtest "--batch --input"
-notmuch dump > backup.tags
+notmuch dump --format=batch-tag > backup.tags
 notmuch tag --batch --input=batch.in
-notmuch search \* | notmuch_search_sanitize > OUTPUT
-notmuch restore < backup.tags
-test_expect_equal_file OUTPUT batch.expected
+notmuch search \* | notmuch_search_sanitize > OUTPUT.$test_count
+notmuch restore --format=batch-tag < backup.tags
+test_expect_equal_file batch.expected OUTPUT.$test_count
 
 test_begin_subtest "--batch, blank lines and comments"
 notmuch dump | sort > EXPECTED.$test_count

commit 4fa12e718b0eaf82f26b9b8aea48f860fc5e1f72
Author: David Bremner <bremner@debian.org>
Date:   Sun Dec 9 16:24:13 2012 -0400

    changes for id:87sj7xi9j5.fsf@qmul.ac.uk and id:871uf8egi8.fsf@nikula.org

diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index 751db7b..3c15393 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -61,12 +61,12 @@ Read input from given file, instead of from stdin. Implies
 
 The input must consist of lines of the format:
 
-.RI "T +<" tag ">|\-<" tag "> [...] [\-\-] <" search-terms ">"
+.RI "+<" tag ">|\-<" tag "> [...] [\-\-] <" search-term "> [...]"
 
 Each line is interpreted similarly to
 .B notmuch tag
 command line arguments. The delimiter is one or more spaces ' '. Any
-characters in <tag> and <search-terms>
+characters in <tag> and <search-term>
 .B may
 be hex encoded with %NN where NN is the hexadecimal value of the
 character. Any ' ' and '%' characters in <tag> and <search-terms>

Re: [Patch v6 1/6] tag-util: factor out rules for illegal tags, use in parse_tag_line

[Jani Nikula]

On Mon, 10 Dec 2012, david@tethera.net wrote:
> 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.

LGTM, with some nitpick and observations below.

Jani.

> ---
>  tag-util.c |   35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/tag-util.c b/tag-util.c
> index eab482f..c071ea8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -31,6 +31,29 @@ 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) {

Add \n before opening brace.

> +
> +    if (*tag == '\0' && !remove)
> +	return "adding empty tag";
> +
> +    /* This disallows adding the non-removable tag "-" and
> +     * enables notmuch tag to take long options more easily.
> +     */

Maybe make that: "This disallows adding tags starting with "-",
including the non-removable tag "-", and enables ...", or similar?

> +
> +    if (*tag == '-' && !remove)
> +	return  "adding tag starting with -";
> +
> +    return NULL;
> +}
> +
>  tag_parse_status_t
>  parse_tag_line (void *ctx, char *line,
>  		tag_op_flag_t flags,
> @@ -95,11 +118,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;
> +	    }
>  	}

I guess I failed to notice during the dump/restore review that this
function was (and still is until later in the series) always called with
TAG_FLAG_BE_GENEROUS. I guess that's what we want with restore;
otherwise we should warn about such tags during dump.

>  
>  	/* Decode tag. */
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

[Jani Nikula]

On Mon, 10 Dec 2012, 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.

Functionally LGTM, nitpicks below.

BR,
Jani.

> ---
>  notmuch-tag.c |  104 +++++++++++++--------------------------------------------
>  tag-util.c    |   48 ++++++++++++++++++++++++++
>  tag-util.h    |   16 +++++++++
>  3 files changed, 87 insertions(+), 81 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 88d559b..b5cd578 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "notmuch-client.h"
> +#include "tag-util.h"
>  
>  static volatile sig_atomic_t interrupted;
>  
> @@ -54,14 +55,9 @@ _escape_tag (char *buf, const char *tag)
>      return buf;
>  }
>  
> -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,
> @@ -73,19 +69,20 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>  
>      char *escaped, *query_string;
>      const char *join = "";
> -    int i;
> +    size_t i;
>      unsigned int max_tag_len = 0;
>  
>      /* 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);
>  
>      /* Allocate a buffer for escaping tags.  This is large enough to
>       * hold a fully escaped tag with every character doubled plus
>       * enclosing quotes and a NUL. */
> -    for (i = 0; tag_ops[i].tag; i++)
> -	if (strlen (tag_ops[i].tag) > max_tag_len)
> -	    max_tag_len = strlen (tag_ops[i].tag);
> +    for (i = 0; i < tag_op_list_size (list); i++)
> +	if (strlen (tag_op_list_tag (list, i)) > max_tag_len)
> +	    max_tag_len = strlen (tag_op_list_tag (list, i));
> +
>      escaped = talloc_array (ctx, char, max_tag_len * 2 + 3);
>      if (! escaped)
>  	return NULL;
> @@ -96,11 +93,11 @@ _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++) {
>  	query_string = talloc_asprintf_append_buffer (
>  	    query_string, "%s%stag:%s", join,
> -	    tag_ops[i].remove ? "" : "not ",
> -	    _escape_tag (escaped, tag_ops[i].tag));
> +	    tag_op_list_isremove (list, i) ? "" : "not ",
> +	    _escape_tag (escaped, tag_op_list_tag (list, i)));
>  	join = " or ";
>      }
>  
> @@ -116,12 +113,11 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>   * element. */
>  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. */
> @@ -144,21 +140,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);
>      }
>  
> @@ -170,15 +152,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));
> @@ -187,54 +167,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,
> +				0, &query_string, tag_ops))
>  	return 1;
> -    }
>  
>      config = notmuch_config_open (ctx, NULL, NULL);
>      if (config == NULL)
> @@ -244,9 +185,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 c071ea8..6e3c069 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -162,6 +162,54 @@ parse_tag_line (void *ctx, char *line,
>      return ret;
>  }
>  
> +int
> +parse_tag_command_line (void *ctx, int argc, char **argv,
> +			unused(tag_op_flag_t flags),

Flags aren't used in the whole series; do you have something in mind for
the future?

> +			char **query_str, tag_op_list_t *tag_ops){

Add \n before {.

> +
> +    int i;
> +
> +    tag_op_list_reset(tag_ops);

Space before (.

> +
> +    for (i = 0; i < argc; i++) {
> +	if (strcmp (argv[i], "--") == 0) {
> +	    i++;
> +	    break;
> +	}
> +
> +	if (argv[i][0] == '+' || argv[i][0] == '-') {

Could also have

	if (argv[i][0] != '+' && argv[i][0] != '-')
        	break;

here and take the stuff below out of the if block. Would get rid of the
lone break in the else block. But that's just bikeshedding...

> +	    notmuch_bool_t is_remove = argv[i][0] == '-';
> +	    const char *msg;
> +
> +	    msg = illegal_tag(argv[i]+1, is_remove);

Spaces around +.

> +	    if (msg) {
> +		fprintf (stderr, "Error: %s", msg);
> +		return 1;
> +	    }
> +
> +	    tag_op_list_append (ctx, tag_ops,
> +				argv[i] + 1, (argv[i][0] == '-'));

You have is_remove you can use as the last parameter.

> +	} else {
> +	    break;
> +	}
> +    }
> +
> +    if (tag_op_list_size (tag_ops) == 0) {
> +	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
> +	return 1;
> +    }
> +
> +    *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
> +

Pedantically you should check if (*query_string == NULL) here (though
this check didn't exist previously either).

> +    if (**query_str == '\0') {
> +	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
> +	return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static inline void
>  message_error (notmuch_message_t *message,
>  	       notmuch_status_t status,
> diff --git a/tag-util.h b/tag-util.h
> index 99b0fa0..4956725 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -72,6 +72,22 @@ 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

Either make this an int, or change the type in .c to tag_parse_status_t
too and use the enums in the code.

> +parse_tag_command_line (void *ctx, int argc, char **argv,
> +			tag_op_flag_t flags,
> +			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: [Patch v6 3/6] cli: add support for batch tagging operations to "notmuch tag"

[Jani Nikula]

On Mon, 10 Dec 2012, david@tethera.net wrote:
> 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> [...] [--] <search-terms>

Maybe clarify <search-term> [...] here too.

Mostly LGTM, small things below.

BR,
Jani.


> Each line is interpreted similarly to "notmuch tag" command line
> arguments. The delimiter is one or more spaces ' '. Any characters in
> <tag> and <search-terms> MAY be hex encoded with %NN where NN is the
> hexadecimal value of the character. Any ' ' and '%' characters in
> <tag> and <search-terms> MUST be hex encoded (using %20 and %25,
> respectively). Any characters that are not part of <tag> or
> <search-terms> MUST NOT be hex encoded.
>
> 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 |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 80 insertions(+), 8 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index b5cd578..2665037 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -149,6 +149,43 @@ 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 || tag_query (ctx, notmuch, query_string,
> +				      tag_ops, flags))

If tag_query fails the function returns 0.

> +	    break;
> +    }
> +
> +    if (line)
> +	free (line);
> +
> +    return ret;
> +}
> +
>  int
>  notmuch_tag_command (void *ctx, int argc, char *argv[])
>  {
> @@ -158,6 +195,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 */
> @@ -167,15 +208,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,
> -				0, &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,
> +				    0, &query_string, tag_ops))
> +	    return 1;
> +    }
>  
>      config = notmuch_config_open (ctx, NULL, NULL);
>      if (config == NULL)
> @@ -188,9 +257,12 @@ 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);

fclose if not stdin?

> +    else
> +	ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
>  
>      notmuch_database_destroy (notmuch);
>  
> -    return ret;
> +    return ret || interrupted;
>  }
> -- 
> 1.7.10.4

Re: [Patch v6 4/6] test: add test for notmuch tag --batch option

[Jani Nikula]

On Mon, 10 Dec 2012, david@tethera.net wrote:
> From: Jani Nikula <jani@nikula.org>
>
> Basic test of functionality, along with all combinations of options.
>
> Modified extensively by David Bremner <david@tethera.net>
>
> The choice of @ as a tag is intended to be non-alphanumeric, but still
> not too much trouble in the shell and in the old sup dump/restore format.

LGTM.

> ---
>  test/tagging |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/test/tagging b/test/tagging
> index 980ff92..0f3d797 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.$test_count
> +notmuch restore --format=batch-tag < backup.tags
> +test_expect_equal_file batch.expected OUTPUT.$test_count
> +
> +test_begin_subtest "--batch --input"
> +notmuch dump --format=batch-tag > backup.tags
> +notmuch tag --batch --input=batch.in
> +notmuch search \* | notmuch_search_sanitize > OUTPUT.$test_count
> +notmuch restore --format=batch-tag < backup.tags
> +test_expect_equal_file batch.expected OUTPUT.$test_count
> +
> +test_begin_subtest "--batch, blank lines and comments"
> +notmuch dump | sort > EXPECTED.$test_count
> +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_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
>  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

Re: [Patch v6 5/6] notmuch-tag.1: tidy synopsis formatting

[Jani Nikula]

On Mon, 10 Dec 2012, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> Consistently use [...]; one less space.

LGTM.

> ---
>  man/man1/notmuch-tag.1 |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
> index 0f86582..d23700d 100644
> --- a/man/man1/notmuch-tag.1
> +++ b/man/man1/notmuch-tag.1
> @@ -4,7 +4,7 @@ notmuch-tag \- add/remove tags for all messages matching the search terms
>  
>  .SH SYNOPSIS
>  .B notmuch tag
> -.RI  "+<" tag> "|\-<" tag "> [...] [\-\-] <" search-term ">..."
> +.RI "+<" tag ">|\-<" tag "> [...] [\-\-] <" search-term "> [...]"
>  
>  .SH DESCRIPTION
>  
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v6 6/6] man: document notmuch tag --batch, --input options

[Jani Nikula]

LGTM.

On Mon, 10 Dec 2012, david@tethera.net wrote:
> From: Jani Nikula <jani@nikula.org>
>
> ---
>  man/man1/notmuch-tag.1 |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>
> diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
> index d23700d..c4e60d3 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.
> @@ -29,6 +34,51 @@ 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 "> [...] [\-\-] <" search-term "> [...]"
> +
> +Each line is interpreted similarly to
> +.B notmuch tag
> +command line arguments. The delimiter is one or more spaces ' '. Any
> +characters in <tag> and <search-term>
> +.B may
> +be hex encoded with %NN where NN is the hexadecimal value of the
> +character. Any ' ' and '%' characters in <tag> and <search-terms>
> +.B must
> +be hex encoded (using %20 and %25, respectively). Any characters that
> +are not part of <tag> or <search-terms>
> +.B must not
> +be hex encoded.
> +
> +Leading and trailing space ' ' is ignored. Empty lines and lines
> +beginning with '#' are ignored.
> +
>  .SH SEE ALSO
>  
>  \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
> -- 
> 1.7.10.4

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

[Jani Nikula]

On Mon, 10 Dec 2012, 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 |  104 +++++++++++++--------------------------------------------
>  tag-util.c    |   48 ++++++++++++++++++++++++++
>  tag-util.h    |   16 +++++++++
>  3 files changed, 87 insertions(+), 81 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 88d559b..b5cd578 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "notmuch-client.h"
> +#include "tag-util.h"
>  
>  static volatile sig_atomic_t interrupted;
>  
> @@ -54,14 +55,9 @@ _escape_tag (char *buf, const char *tag)
>      return buf;
>  }
>  
> -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,
> @@ -73,19 +69,20 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>  
>      char *escaped, *query_string;
>      const char *join = "";
> -    int i;
> +    size_t i;
>      unsigned int max_tag_len = 0;
>  
>      /* 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);
>  
>      /* Allocate a buffer for escaping tags.  This is large enough to
>       * hold a fully escaped tag with every character doubled plus
>       * enclosing quotes and a NUL. */
> -    for (i = 0; tag_ops[i].tag; i++)
> -	if (strlen (tag_ops[i].tag) > max_tag_len)
> -	    max_tag_len = strlen (tag_ops[i].tag);
> +    for (i = 0; i < tag_op_list_size (list); i++)
> +	if (strlen (tag_op_list_tag (list, i)) > max_tag_len)
> +	    max_tag_len = strlen (tag_op_list_tag (list, i));
> +
>      escaped = talloc_array (ctx, char, max_tag_len * 2 + 3);
>      if (! escaped)
>  	return NULL;
> @@ -96,11 +93,11 @@ _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++) {
>  	query_string = talloc_asprintf_append_buffer (
>  	    query_string, "%s%stag:%s", join,
> -	    tag_ops[i].remove ? "" : "not ",
> -	    _escape_tag (escaped, tag_ops[i].tag));
> +	    tag_op_list_isremove (list, i) ? "" : "not ",
> +	    _escape_tag (escaped, tag_op_list_tag (list, i)));
>  	join = " or ";
>      }
>  
> @@ -116,12 +113,11 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
>   * element. */

The comment "array of tagging operations terminated with an empty
element" needs revision too.

BR,
Jani.

>  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. */
> @@ -144,21 +140,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);
>      }
>  
> @@ -170,15 +152,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));
> @@ -187,54 +167,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,
> +				0, &query_string, tag_ops))
>  	return 1;
> -    }
>  
>      config = notmuch_config_open (ctx, NULL, NULL);
>      if (config == NULL)
> @@ -244,9 +185,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 c071ea8..6e3c069 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -162,6 +162,54 @@ parse_tag_line (void *ctx, char *line,
>      return ret;
>  }
>  
> +int
> +parse_tag_command_line (void *ctx, int argc, char **argv,
> +			unused(tag_op_flag_t flags),
> +			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] == '-') {
> +	    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 1;
> +	    }
> +
> +	    tag_op_list_append (ctx, tag_ops,
> +				argv[i] + 1, (argv[i][0] == '-'));
> +	} else {
> +	    break;
> +	}
> +    }
> +
> +    if (tag_op_list_size (tag_ops) == 0) {
> +	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
> +	return 1;
> +    }
> +
> +    *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
> +
> +    if (**query_str == '\0') {
> +	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
> +	return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static inline void
>  message_error (notmuch_message_t *message,
>  	       notmuch_status_t status,
> diff --git a/tag-util.h b/tag-util.h
> index 99b0fa0..4956725 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -72,6 +72,22 @@ 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,
> +			tag_op_flag_t flags,
> +			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: [Patch v6 1/6] tag-util: factor out rules for illegal tags, use in parse_tag_line

[Mark Walters]

I don't know if this matters but it seems that one can still add a "-"
tag by doing something like

echo "+%2d -- search" | notmuch tag --batch

This might be the right thing to do but I thought I would mention it.

Best wishes

Mark

On Sun, 09 Dec 2012, david@tethera.net wrote:
> 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 |   35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/tag-util.c b/tag-util.c
> index eab482f..c071ea8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -31,6 +31,29 @@ 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 "adding empty tag";
> +
> +    /* This disallows adding the non-removable tag "-" and
> +     * enables notmuch tag to take long options more easily.
> +     */
> +
> +    if (*tag == '-' && !remove)
> +	return  "adding tag starting with -";
> +
> +    return NULL;
> +}
> +
>  tag_parse_status_t
>  parse_tag_line (void *ctx, char *line,
>  		tag_op_flag_t flags,
> @@ -95,11 +118,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
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v6 1/6] tag-util: factor out rules for illegal tags, use in parse_tag_line

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> I don't know if this matters but it seems that one can still add a "-"
> tag by doing something like
>
> echo "+%2d -- search" | notmuch tag --batch
>
> This might be the right thing to do but I thought I would mention it.

I _think_ it's a feature, so I'm tempted to document it and move on
unless somebody convinces me it's a bad idea.

d