unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Xapian-quoting based batch-tagging.
@ 2012-12-25 19:42 david
  2012-12-25 19:42 ` [PATCH 01/11] parse_tag_line: use enum for return value david
                   ` (12 more replies)
  0 siblings, 13 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch

This is an alternative version of 

     id:1356313183-9266-1-git-send-email-david@tethera.net

batch tagging patches rebased on top of 

     id:1356415076-5692-1-git-send-email-amdragon@mit.edu

This mainly consisted of removing 

     [Patch v9 04/17] notmuch-tag: factor out double quoting routine
     (superceded by one of Austin's patches)

     [Patch v9 05/17] util/string-util: add a new string tokenized function
     [Patch v9 06/17] unhex_and_quote: new function to quote hex-decoded queries
     [Patch v9 07/17] notmuch-restore: move query handling for batch	
     (uneeded if query is passed verbatim to xapian)

     I also removed two tests, since they are about how we handle
     quoting:

     [Patch v9 13/17] test/tagging: add test for compound queries with batch tagging
     [Patch v9 17/17] test/tagging: add test for handling of parenthesized	tag queries.

A few small fixes were needed to the tests, and a fair amount of
changes to the notmuch-tag man page.

Diffstat (against Austin's series) is as follows

 man/man1/notmuch-tag.1 |   99 ++++++++++++++++++++-
 notmuch-tag.c          |  169 ++++++++++++++++++++----------------
 tag-util.c             |   87 +++++++++++++++++--
 tag-util.h             |   15 ++++
 test/tagging           |  195 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 480 insertions(+), 85 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/11] parse_tag_line: use enum for return value.
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
@ 2012-12-25 19:42 ` david
  2012-12-26 13:48   ` David Bremner
  2012-12-25 19:42 ` [PATCH 02/11] tag-util: factor out rules for illegal tags, use in parse_tag_line david
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

This is essentially cosmetic, since success=0 is promised by
the comments in tag-utils.h.
---
 tag-util.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tag-util.c b/tag-util.c
index e4e5dda..ca12b3b 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -40,14 +40,14 @@ parse_tag_line (void *ctx, char *line,
     char *tok = line;
     size_t tok_len = 0;
     char *line_for_error;
-    int ret = 0;
+    tag_parse_status_t ret = TAG_PARSE_SUCCESS;
 
     chomp_newline (line);
 
     line_for_error = talloc_strdup (ctx, line);
     if (line_for_error == NULL) {
 	fprintf (stderr, "Error: out of memory\n");
-	return -1;
+	return TAG_PARSE_OUT_OF_MEMORY;
     }
 
     /* remove leading space */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/11] tag-util: factor out rules for illegal tags, use in parse_tag_line
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
  2012-12-25 19:42 ` [PATCH 01/11] parse_tag_line: use enum for return value david
@ 2012-12-25 19:42 ` david
  2012-12-25 19:42 ` [PATCH 03/11] notmuch-tag.c: convert to use tag-utils david
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/11] notmuch-tag.c: convert to use tag-utils
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
  2012-12-25 19:42 ` [PATCH 01/11] parse_tag_line: use enum for return value david
  2012-12-25 19:42 ` [PATCH 02/11] tag-util: factor out rules for illegal tags, use in parse_tag_line david
@ 2012-12-25 19:42 ` david
  2012-12-25 19:42 ` [PATCH 04/11] cli: add support for batch tagging operations to "notmuch tag" david
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/11] cli: add support for batch tagging operations to "notmuch tag"
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (2 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 03/11] notmuch-tag.c: convert to use tag-utils david
@ 2012-12-25 19:42 ` david
  2013-01-03 18:42   ` Jani Nikula
  2012-12-25 19:42 ` [PATCH 05/11] test/tagging: add test for error messages of tag --batch david
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch

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> MAP be hex encoded with %NN where NN is the
hexadecimal value of the character. Any ' ' and '%' characters in
<tag> and <search-term> MUST be hex encoded (using %20 and %25,
respectively). 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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/11] test/tagging: add test for error messages of tag --batch
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (3 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 04/11] cli: add support for batch tagging operations to "notmuch tag" david
@ 2012-12-25 19:42 ` david
  2012-12-25 19:42 ` [PATCH 06/11] test/tagging: add basic tests for batch tagging functionality david
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/11] test/tagging: add basic tests for batch tagging functionality
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (4 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 05/11] test/tagging: add test for error messages of tag --batch david
@ 2012-12-25 19:42 ` david
  2012-12-25 19:42 ` [PATCH 07/11] test/tagging: add tests for exotic tags david
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/11] test/tagging: add tests for exotic tags
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (5 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 06/11] test/tagging: add basic tests for batch tagging functionality david
@ 2012-12-25 19:42 ` david
  2012-12-25 19:42 ` [PATCH 08/11] test/tagging: add test for exotic message-ids and batch tagging david
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/11] test/tagging: add test for exotic message-ids and batch tagging
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (6 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 07/11] test/tagging: add tests for exotic tags david
@ 2012-12-25 19:42 ` david
  2012-12-25 19:42 ` [PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference david
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (7 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 08/11] test/tagging: add test for exotic message-ids and batch tagging david
@ 2012-12-25 19:42 ` david
  2012-12-26 13:48   ` David Bremner
  2012-12-25 19:42 ` [PATCH 10/11] man: document notmuch tag --batch, --input options david
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

Consistently use [...]; one less space. Use singular <search-term>
---
 man/man1/notmuch-tag.1 |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index 0f86582..9444aa4 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -4,20 +4,21 @@ 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
 
 Add/remove tags for all messages matching the search terms.
 
 See \fBnotmuch-search-terms\fR(7)
-for details of the supported syntax for <search-terms>.
+for details of the supported syntax for
+.RI < search-term >.
 
 Tags prefixed by '+' are added while those prefixed by '\-' are
 removed. For each message, tag removal is performed before tag
 addition.
 
-The beginning of <search-terms> is recognized by the first
+The beginning of the search terms is recognized by the first
 argument that begins with neither '+' nor '\-'. Support for
 an initial search term beginning with '+' or '\-' is provided
 by allowing the user to specify a "\-\-" argument to separate
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/11] man: document notmuch tag --batch, --input options
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (8 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference david
@ 2012-12-25 19:42 ` david
  2012-12-25 19:42 ` [PATCH 11/11] test/tagging: add test for naked punctuation in tags; compare with quoting spaces david
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 11/11] test/tagging: add test for naked punctuation in tags; compare with quoting spaces.
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (9 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 10/11] man: document notmuch tag --batch, --input options david
@ 2012-12-25 19:42 ` david
  2012-12-26  8:52 ` Xapian-quoting based batch-tagging Mark Walters
  2013-01-03 19:59 ` Jani Nikula
  12 siblings, 0 replies; 20+ messages in thread
From: david @ 2012-12-25 19:42 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Xapian-quoting based batch-tagging.
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (10 preceding siblings ...)
  2012-12-25 19:42 ` [PATCH 11/11] test/tagging: add test for naked punctuation in tags; compare with quoting spaces david
@ 2012-12-26  8:52 ` Mark Walters
  2012-12-26 13:23   ` David Bremner
  2013-01-03 19:59 ` Jani Nikula
  12 siblings, 1 reply; 20+ messages in thread
From: Mark Walters @ 2012-12-26  8:52 UTC (permalink / raw)
  To: david, notmuch


On Tue, 25 Dec 2012, david@tethera.net wrote:
> This is an alternative version of 
>
>      id:1356313183-9266-1-git-send-email-david@tethera.net
>
> batch tagging patches rebased on top of 
>
>      id:1356415076-5692-1-git-send-email-amdragon@mit.edu
>
> This mainly consisted of removing 
>
>      [Patch v9 04/17] notmuch-tag: factor out double quoting routine
>      (superceded by one of Austin's patches)
>
>      [Patch v9 05/17] util/string-util: add a new string tokenized function
>      [Patch v9 06/17] unhex_and_quote: new function to quote hex-decoded queries
>      [Patch v9 07/17] notmuch-restore: move query handling for batch	
>      (uneeded if query is passed verbatim to xapian)
>
>      I also removed two tests, since they are about how we handle
>      quoting:
>
>      [Patch v9 13/17] test/tagging: add test for compound queries with batch tagging
>      [Patch v9 17/17] test/tagging: add test for handling of parenthesized	tag queries.
>
> A few small fixes were needed to the tests, and a fair amount of
> changes to the notmuch-tag man page.

Hi

I am unclear about how this is going to deal with queries containing
newlines. For dump/restore I think this is not a problem (as Austin and
others have said), but for batch tagging I think it could be; for
example the query could be for a tag containing a newline.

Best wishes

Mark


>
> Diffstat (against Austin's series) is as follows
>
>  man/man1/notmuch-tag.1 |   99 ++++++++++++++++++++-
>  notmuch-tag.c          |  169 ++++++++++++++++++++----------------
>  tag-util.c             |   87 +++++++++++++++++--
>  tag-util.h             |   15 ++++
>  test/tagging           |  195 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 480 insertions(+), 85 deletions(-)
>
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Xapian-quoting based batch-tagging.
  2012-12-26  8:52 ` Xapian-quoting based batch-tagging Mark Walters
@ 2012-12-26 13:23   ` David Bremner
  2013-01-03 19:41     ` Jani Nikula
  0 siblings, 1 reply; 20+ messages in thread
From: David Bremner @ 2012-12-26 13:23 UTC (permalink / raw)
  To: Mark Walters, notmuch

Mark Walters <markwalters1009@gmail.com> writes:

> I am unclear about how this is going to deal with queries containing
> newlines. For dump/restore I think this is not a problem (as Austin and
> others have said), but for batch tagging I think it could be; for
> example the query could be for a tag containing a newline.

Yes, that's true, this patch series does not support queries with tags
with embedded newlines. They can still be removed (and added) via either
batch tagging or the command line. We could just live with this, or

- The current syntax allows for detecting options at the start of the
  line; perhaps a future fix would be to have the batch tagging and
  command line tagging accept an optionally hex encoded query, something
  like:

        --hex +found%20it -- tag:%22stupid%0Atag%22

- Alternatively, we could add hex decoding on top of xapian quoting for
  queries. One UI downside is that people have to remember that % are
  special.

     +found%25it -- tag:lost%25it

  Another is that quoting is still (surprisingly) necessary for encoded
  spaces
  
     +found%20it -- tag:"lost%20it"
 
  Introducing yet another escape format, e.g. "\n" would require more
  code, and not really much benefit afaict versus re-using hex-encoding.
  Offhand I don't see how to avoid this without some level of query
  pre-processing a-la
  
        id:1356313183-9266-1-git-send-email-david@tethera.net

d

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference
  2012-12-25 19:42 ` [PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference david
@ 2012-12-26 13:48   ` David Bremner
  0 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2012-12-26 13:48 UTC (permalink / raw)
  To: notmuch

david@tethera.net writes:

> From: David Bremner <bremner@debian.org>
>
> Consistently use [...]; one less space. Use singular <search-term>
> ---

pushed this one patch.

d

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 01/11] parse_tag_line: use enum for return value.
  2012-12-25 19:42 ` [PATCH 01/11] parse_tag_line: use enum for return value david
@ 2012-12-26 13:48   ` David Bremner
  0 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2012-12-26 13:48 UTC (permalink / raw)
  To: notmuch

david@tethera.net writes:

> From: David Bremner <bremner@debian.org>
>
> This is essentially cosmetic, since success=0 is promised by
> the comments in tag-utils.h.

Pushed this one patch.

Both of the two I pushed from this series are really nothing to do with
batch tagging, and were just cluttering things up.

d

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 04/11] cli: add support for batch tagging operations to "notmuch tag"
  2012-12-25 19:42 ` [PATCH 04/11] cli: add support for batch tagging operations to "notmuch tag" david
@ 2013-01-03 18:42   ` Jani Nikula
  0 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2013-01-03 18:42 UTC (permalink / raw)
  To: david, notmuch

On Tue, 25 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> [...] [--] <query> [...]
>
> Each line is interpreted similarly to "notmuch tag" command line
> arguments. The delimiter is one or more spaces ' '. Any characters in
> <tag> MAP be hex encoded with %NN where NN is the

MAY

> hexadecimal value of the character. Any ' ' and '%' characters in
> <tag> and <search-term> MUST be hex encoded (using %20 and %25,

If we also required double quotes '"' to be hex encoded, we would have
an easier transition to using xapian quoting for tags too if we so
choose. notmuch dump already does this. If we additionally require % to
be quoted when using xapian quoting, we could detect hex vs. xapian
automatically.

> respectively). Any characters that are not part of <tag> or

-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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Xapian-quoting based batch-tagging.
  2012-12-26 13:23   ` David Bremner
@ 2013-01-03 19:41     ` Jani Nikula
  2013-01-04  5:32       ` Jameson Graef Rollins
  0 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2013-01-03 19:41 UTC (permalink / raw)
  To: David Bremner, Mark Walters, notmuch

On Wed, 26 Dec 2012, David Bremner <david@tethera.net> wrote:
> Mark Walters <markwalters1009@gmail.com> writes:
>
>> I am unclear about how this is going to deal with queries containing
>> newlines. For dump/restore I think this is not a problem (as Austin and
>> others have said), but for batch tagging I think it could be; for
>> example the query could be for a tag containing a newline.
>
> Yes, that's true, this patch series does not support queries with tags
> with embedded newlines. They can still be removed (and added) via either
> batch tagging or the command line. We could just live with this, or

I think we should just live with it. It's a bunch of code with some UI
wrinkles for a marginal feature.

BR,
Jani.


>
> - The current syntax allows for detecting options at the start of the
>   line; perhaps a future fix would be to have the batch tagging and
>   command line tagging accept an optionally hex encoded query, something
>   like:
>
>         --hex +found%20it -- tag:%22stupid%0Atag%22
>
> - Alternatively, we could add hex decoding on top of xapian quoting for
>   queries. One UI downside is that people have to remember that % are
>   special.
>
>      +found%25it -- tag:lost%25it
>
>   Another is that quoting is still (surprisingly) necessary for encoded
>   spaces
>   
>      +found%20it -- tag:"lost%20it"
>  
>   Introducing yet another escape format, e.g. "\n" would require more
>   code, and not really much benefit afaict versus re-using hex-encoding.
>   Offhand I don't see how to avoid this without some level of query
>   pre-processing a-la
>   
>         id:1356313183-9266-1-git-send-email-david@tethera.net
>
> d
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Xapian-quoting based batch-tagging.
  2012-12-25 19:42 Xapian-quoting based batch-tagging david
                   ` (11 preceding siblings ...)
  2012-12-26  8:52 ` Xapian-quoting based batch-tagging Mark Walters
@ 2013-01-03 19:59 ` Jani Nikula
  12 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2013-01-03 19:59 UTC (permalink / raw)
  To: david, notmuch

On Tue, 25 Dec 2012, david@tethera.net wrote:
> This is an alternative version of 
>
>      id:1356313183-9266-1-git-send-email-david@tethera.net
>
> batch tagging patches rebased on top of 
>
>      id:1356415076-5692-1-git-send-email-amdragon@mit.edu

I didn't go through this quite as thoroughly as the previous versions,
but the series LGTM. I like how this simplifies things. In the future,
I'd like us to support xapian quoting also in the tag changes, for
consistency.

BR,
Jani.



>
> This mainly consisted of removing 
>
>      [Patch v9 04/17] notmuch-tag: factor out double quoting routine
>      (superceded by one of Austin's patches)
>
>      [Patch v9 05/17] util/string-util: add a new string tokenized function
>      [Patch v9 06/17] unhex_and_quote: new function to quote hex-decoded queries
>      [Patch v9 07/17] notmuch-restore: move query handling for batch	
>      (uneeded if query is passed verbatim to xapian)
>
>      I also removed two tests, since they are about how we handle
>      quoting:
>
>      [Patch v9 13/17] test/tagging: add test for compound queries with batch tagging
>      [Patch v9 17/17] test/tagging: add test for handling of parenthesized	tag queries.
>
> A few small fixes were needed to the tests, and a fair amount of
> changes to the notmuch-tag man page.
>
> Diffstat (against Austin's series) is as follows
>
>  man/man1/notmuch-tag.1 |   99 ++++++++++++++++++++-
>  notmuch-tag.c          |  169 ++++++++++++++++++++----------------
>  tag-util.c             |   87 +++++++++++++++++--
>  tag-util.h             |   15 ++++
>  test/tagging           |  195 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 480 insertions(+), 85 deletions(-)
>
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Xapian-quoting based batch-tagging.
  2013-01-03 19:41     ` Jani Nikula
@ 2013-01-04  5:32       ` Jameson Graef Rollins
  0 siblings, 0 replies; 20+ messages in thread
From: Jameson Graef Rollins @ 2013-01-04  5:32 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, Mark Walters, notmuch

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Thu, Jan 03 2013, Jani Nikula <jani@nikula.org> wrote:
>>> I am unclear about how this is going to deal with queries containing
>>> newlines. For dump/restore I think this is not a problem (as Austin and
>>> others have said), but for batch tagging I think it could be; for
>>> example the query could be for a tag containing a newline.
>>
>> Yes, that's true, this patch series does not support queries with tags
>> with embedded newlines. They can still be removed (and added) via either
>> batch tagging or the command line. We could just live with this, or
>
> I think we should just live with it. It's a bunch of code with some UI
> wrinkles for a marginal feature.

Tags with newlines are psychotic.  We should just explicitly forbid them
by preventing them from ever being applied and then never have to worry
about them again.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-01-04  5:32 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-25 19:42 Xapian-quoting based batch-tagging david
2012-12-25 19:42 ` [PATCH 01/11] parse_tag_line: use enum for return value david
2012-12-26 13:48   ` David Bremner
2012-12-25 19:42 ` [PATCH 02/11] tag-util: factor out rules for illegal tags, use in parse_tag_line david
2012-12-25 19:42 ` [PATCH 03/11] notmuch-tag.c: convert to use tag-utils david
2012-12-25 19:42 ` [PATCH 04/11] cli: add support for batch tagging operations to "notmuch tag" david
2013-01-03 18:42   ` Jani Nikula
2012-12-25 19:42 ` [PATCH 05/11] test/tagging: add test for error messages of tag --batch david
2012-12-25 19:42 ` [PATCH 06/11] test/tagging: add basic tests for batch tagging functionality david
2012-12-25 19:42 ` [PATCH 07/11] test/tagging: add tests for exotic tags david
2012-12-25 19:42 ` [PATCH 08/11] test/tagging: add test for exotic message-ids and batch tagging david
2012-12-25 19:42 ` [PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference david
2012-12-26 13:48   ` David Bremner
2012-12-25 19:42 ` [PATCH 10/11] man: document notmuch tag --batch, --input options david
2012-12-25 19:42 ` [PATCH 11/11] test/tagging: add test for naked punctuation in tags; compare with quoting spaces david
2012-12-26  8:52 ` Xapian-quoting based batch-tagging Mark Walters
2012-12-26 13:23   ` David Bremner
2013-01-03 19:41     ` Jani Nikula
2013-01-04  5:32       ` Jameson Graef Rollins
2013-01-03 19:59 ` Jani Nikula

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).