* [Patch v7 01/14] parse_tag_line: use enum for return value.
2012-12-14 13:34 v7 batch tagging series david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 02/14] tag-util: factor out rules for illegal tags, use in parse_tag_line david
` (12 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 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 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tag-util.c b/tag-util.c
index eab482f..3d588d3 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -40,7 +40,7 @@ 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);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Patch v7 02/14] tag-util: factor out rules for illegal tags, use in parse_tag_line
2012-12-14 13:34 v7 batch tagging series david
2012-12-14 13:34 ` [Patch v7 01/14] parse_tag_line: use enum for return value david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 03/14] notmuch-tag.c: convert to use tag-utils david
` (11 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 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 3d588d3..13d9035 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] 29+ messages in thread* [Patch v7 03/14] notmuch-tag.c: convert to use tag-utils
2012-12-14 13:34 v7 batch tagging series david
2012-12-14 13:34 ` [Patch v7 01/14] parse_tag_line: use enum for return value david
2012-12-14 13:34 ` [Patch v7 02/14] tag-util: factor out rules for illegal tags, use in parse_tag_line david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 04/14] notmuch-tag: factor out double quoting routine david
` (10 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 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 | 109 +++++++++++++--------------------------------------------
tag-util.c | 51 +++++++++++++++++++++++++--
tag-util.h | 15 ++++++++
3 files changed, 89 insertions(+), 86 deletions(-)
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 88d559b..0965ee7 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 ";
}
@@ -111,17 +108,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. */
@@ -144,21 +139,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 +151,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 +166,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)
@@ -244,9 +184,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
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 13d9035..f89669a 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)
@@ -163,6 +164,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 (ctx, 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 99b0fa0..2889736 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] 29+ messages in thread* [Patch v7 04/14] notmuch-tag: factor out double quoting routine
2012-12-14 13:34 v7 batch tagging series david
` (2 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 03/14] notmuch-tag.c: convert to use tag-utils david
@ 2012-12-14 13:34 ` david
2012-12-15 17:55 ` Mark Walters
2012-12-15 22:20 ` Jani Nikula
2012-12-14 13:34 ` [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries david
` (9 subsequent siblings)
13 siblings, 2 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
From: David Bremner <bremner@debian.org>
This could live in tag-util as well, but it is really nothing specific
to tags (although the conventions are arguable specific to Xapian).
The API is changed from "caller-allocates" to "readline-like". The scan for
max tag length is pushed down into the double quoting routine.
---
notmuch-tag.c | 50 ++++++++++++++++----------------------------------
util/string-util.c | 34 ++++++++++++++++++++++++++++++++++
util/string-util.h | 8 ++++++++
3 files changed, 58 insertions(+), 34 deletions(-)
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 0965ee7..13f2268 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -20,6 +20,7 @@
#include "notmuch-client.h"
#include "tag-util.h"
+#include "string-util.h"
static volatile sig_atomic_t interrupted;
@@ -37,25 +38,6 @@ handle_sigint (unused (int sig))
}
static char *
-_escape_tag (char *buf, const char *tag)
-{
- const char *in = tag;
- char *out = buf;
-
- /* Boolean terms surrounded by double quotes can contain any
- * character. Double quotes are quoted by doubling them. */
- *out++ = '"';
- while (*in) {
- if (*in == '"')
- *out++ = '"';
- *out++ = *in++;
- }
- *out++ = '"';
- *out = 0;
- return buf;
-}
-
-static char *
_optimize_tag_query (void *ctx, const char *orig_query_string,
const tag_op_list_t *list)
{
@@ -67,44 +49,44 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
* parenthesize and the exclusion part of the query must not use
* the '-' operator (though the NOT operator is fine). */
- char *escaped, *query_string;
+ char *escaped = NULL;
+ size_t escaped_len = 0;
+ char *query_string;
const char *join = "";
size_t i;
- unsigned int max_tag_len = 0;
/* Don't optimize if there are no tag changes. */
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; 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;
-
/* Build the new query string */
if (strcmp (orig_query_string, "*") == 0)
query_string = talloc_strdup (ctx, "(");
else
query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
+
+ /* Boolean terms surrounded by double quotes can contain any
+ * character. Double quotes are quoted by doubling them. */
+
for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
+ double_quote_str (ctx,
+ tag_op_list_tag (list, i),
+ &escaped, &escaped_len);
+
query_string = talloc_asprintf_append_buffer (
query_string, "%s%stag:%s", join,
tag_op_list_isremove (list, i) ? "" : "not ",
- _escape_tag (escaped, tag_op_list_tag (list, i)));
+ escaped);
join = " or ";
}
if (query_string)
query_string = talloc_strdup_append_buffer (query_string, ")");
- talloc_free (escaped);
+ if (escaped)
+ talloc_free (escaped);
+
return query_string;
}
diff --git a/util/string-util.c b/util/string-util.c
index 44f8cd3..ea7c25b 100644
--- a/util/string-util.c
+++ b/util/string-util.c
@@ -20,6 +20,7 @@
#include "string-util.h"
+#include "talloc.h"
char *
strtok_len (char *s, const char *delim, size_t *len)
@@ -32,3 +33,36 @@ strtok_len (char *s, const char *delim, size_t *len)
return *len ? s : NULL;
}
+
+
+int
+double_quote_str (void *ctx, const char *str,
+ char **buf, size_t *len)
+{
+ const char *in;
+ char *out;
+ size_t needed = 3;
+
+ for (in = str; *in; in++)
+ needed += (*in == '"') ? 2 : 1;
+
+ if (needed > *len)
+ *buf = talloc_realloc (ctx, *buf, char, 2*needed);
+
+ if (! *buf)
+ return 1;
+
+ out = *buf;
+
+ *out++ = '"';
+ in = str;
+ while (*in) {
+ if (*in == '"')
+ *out++ = '"';
+ *out++ = *in++;
+ }
+ *out++ = '"';
+ *out = 0;
+
+ return 0;
+}
diff --git a/util/string-util.h b/util/string-util.h
index ac7676c..b593bc7 100644
--- a/util/string-util.h
+++ b/util/string-util.h
@@ -19,4 +19,12 @@
char *strtok_len (char *s, const char *delim, size_t *len);
+/* Copy str to dest, surrounding with double quotes.
+ * Any internal double-quotes are doubled, i.e. a"b -> "a""b"
+ *
+ * Output is into buf; it may be talloc_realloced
+ * return 0 on success, non-zero on failure.
+ */
+int double_quote_str (void *talloc_ctx, const char *str,
+ char **buf, size_t *len);
#endif
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Patch v7 04/14] notmuch-tag: factor out double quoting routine
2012-12-14 13:34 ` [Patch v7 04/14] notmuch-tag: factor out double quoting routine david
@ 2012-12-15 17:55 ` Mark Walters
2012-12-15 22:20 ` Jani Nikula
1 sibling, 0 replies; 29+ messages in thread
From: Mark Walters @ 2012-12-15 17:55 UTC (permalink / raw)
To: david, notmuch; +Cc: David Bremner
On Fri, 14 Dec 2012, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> This could live in tag-util as well, but it is really nothing specific
> to tags (although the conventions are arguable specific to Xapian).
>
> The API is changed from "caller-allocates" to "readline-like". The scan for
> max tag length is pushed down into the double quoting routine.
> ---
> notmuch-tag.c | 50 ++++++++++++++++----------------------------------
> util/string-util.c | 34 ++++++++++++++++++++++++++++++++++
> util/string-util.h | 8 ++++++++
> 3 files changed, 58 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 0965ee7..13f2268 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -20,6 +20,7 @@
>
> #include "notmuch-client.h"
> #include "tag-util.h"
> +#include "string-util.h"
>
> static volatile sig_atomic_t interrupted;
>
> @@ -37,25 +38,6 @@ handle_sigint (unused (int sig))
> }
>
> static char *
> -_escape_tag (char *buf, const char *tag)
> -{
> - const char *in = tag;
> - char *out = buf;
> -
> - /* Boolean terms surrounded by double quotes can contain any
> - * character. Double quotes are quoted by doubling them. */
> - *out++ = '"';
> - while (*in) {
> - if (*in == '"')
> - *out++ = '"';
> - *out++ = *in++;
> - }
> - *out++ = '"';
> - *out = 0;
> - return buf;
> -}
> -
> -static char *
> _optimize_tag_query (void *ctx, const char *orig_query_string,
> const tag_op_list_t *list)
> {
> @@ -67,44 +49,44 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
> * parenthesize and the exclusion part of the query must not use
> * the '-' operator (though the NOT operator is fine). */
>
> - char *escaped, *query_string;
> + char *escaped = NULL;
> + size_t escaped_len = 0;
> + char *query_string;
> const char *join = "";
> size_t i;
> - unsigned int max_tag_len = 0;
>
> /* Don't optimize if there are no tag changes. */
> 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; 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;
> -
> /* Build the new query string */
> if (strcmp (orig_query_string, "*") == 0)
> query_string = talloc_strdup (ctx, "(");
> else
> query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
>
> +
> + /* Boolean terms surrounded by double quotes can contain any
> + * character. Double quotes are quoted by doubling them. */
> +
> for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
> + double_quote_str (ctx,
> + tag_op_list_tag (list, i),
> + &escaped, &escaped_len);
> +
> query_string = talloc_asprintf_append_buffer (
> query_string, "%s%stag:%s", join,
> tag_op_list_isremove (list, i) ? "" : "not ",
> - _escape_tag (escaped, tag_op_list_tag (list, i)));
> + escaped);
> join = " or ";
> }
>
> if (query_string)
> query_string = talloc_strdup_append_buffer (query_string, ")");
>
> - talloc_free (escaped);
> + if (escaped)
> + talloc_free (escaped);
> +
> return query_string;
> }
>
> diff --git a/util/string-util.c b/util/string-util.c
> index 44f8cd3..ea7c25b 100644
> --- a/util/string-util.c
> +++ b/util/string-util.c
> @@ -20,6 +20,7 @@
>
>
> #include "string-util.h"
> +#include "talloc.h"
>
> char *
> strtok_len (char *s, const char *delim, size_t *len)
> @@ -32,3 +33,36 @@ strtok_len (char *s, const char *delim, size_t *len)
>
> return *len ? s : NULL;
> }
> +
> +
> +int
> +double_quote_str (void *ctx, const char *str,
> + char **buf, size_t *len)
> +{
> + const char *in;
> + char *out;
> + size_t needed = 3;
> +
> + for (in = str; *in; in++)
> + needed += (*in == '"') ? 2 : 1;
> +
> + if (needed > *len)
> + *buf = talloc_realloc (ctx, *buf, char, 2*needed);
> +
> + if (! *buf)
> + return 1;
> +
> + out = *buf;
> +
> + *out++ = '"';
> + in = str;
> + while (*in) {
> + if (*in == '"')
> + *out++ = '"';
> + *out++ = *in++;
> + }
> + *out++ = '"';
> + *out = 0;
Just a triviality: isn't '\0' preferred?
Best wishes
Mark
> +
> + return 0;
> +}
> diff --git a/util/string-util.h b/util/string-util.h
> index ac7676c..b593bc7 100644
> --- a/util/string-util.h
> +++ b/util/string-util.h
> @@ -19,4 +19,12 @@
>
> char *strtok_len (char *s, const char *delim, size_t *len);
>
> +/* Copy str to dest, surrounding with double quotes.
> + * Any internal double-quotes are doubled, i.e. a"b -> "a""b"
> + *
> + * Output is into buf; it may be talloc_realloced
> + * return 0 on success, non-zero on failure.
> + */
> +int double_quote_str (void *talloc_ctx, const char *str,
> + char **buf, size_t *len);
> #endif
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Patch v7 04/14] notmuch-tag: factor out double quoting routine
2012-12-14 13:34 ` [Patch v7 04/14] notmuch-tag: factor out double quoting routine david
2012-12-15 17:55 ` Mark Walters
@ 2012-12-15 22:20 ` Jani Nikula
1 sibling, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2012-12-15 22:20 UTC (permalink / raw)
To: david, notmuch; +Cc: David Bremner
On Fri, 14 Dec 2012, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> This could live in tag-util as well, but it is really nothing specific
> to tags (although the conventions are arguable specific to Xapian).
>
> The API is changed from "caller-allocates" to "readline-like". The scan for
> max tag length is pushed down into the double quoting routine.
> ---
> notmuch-tag.c | 50 ++++++++++++++++----------------------------------
> util/string-util.c | 34 ++++++++++++++++++++++++++++++++++
> util/string-util.h | 8 ++++++++
> 3 files changed, 58 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 0965ee7..13f2268 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -20,6 +20,7 @@
>
> #include "notmuch-client.h"
> #include "tag-util.h"
> +#include "string-util.h"
>
> static volatile sig_atomic_t interrupted;
>
> @@ -37,25 +38,6 @@ handle_sigint (unused (int sig))
> }
>
> static char *
> -_escape_tag (char *buf, const char *tag)
> -{
> - const char *in = tag;
> - char *out = buf;
> -
> - /* Boolean terms surrounded by double quotes can contain any
> - * character. Double quotes are quoted by doubling them. */
> - *out++ = '"';
> - while (*in) {
> - if (*in == '"')
> - *out++ = '"';
> - *out++ = *in++;
> - }
> - *out++ = '"';
> - *out = 0;
> - return buf;
> -}
> -
> -static char *
> _optimize_tag_query (void *ctx, const char *orig_query_string,
> const tag_op_list_t *list)
> {
> @@ -67,44 +49,44 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
> * parenthesize and the exclusion part of the query must not use
> * the '-' operator (though the NOT operator is fine). */
>
> - char *escaped, *query_string;
> + char *escaped = NULL;
> + size_t escaped_len = 0;
> + char *query_string;
> const char *join = "";
> size_t i;
> - unsigned int max_tag_len = 0;
>
> /* Don't optimize if there are no tag changes. */
> 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; 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;
> -
> /* Build the new query string */
> if (strcmp (orig_query_string, "*") == 0)
> query_string = talloc_strdup (ctx, "(");
> else
> query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
>
> +
> + /* Boolean terms surrounded by double quotes can contain any
> + * character. Double quotes are quoted by doubling them. */
> +
> for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
> + double_quote_str (ctx,
> + tag_op_list_tag (list, i),
> + &escaped, &escaped_len);
Check return value?
> +
> query_string = talloc_asprintf_append_buffer (
> query_string, "%s%stag:%s", join,
> tag_op_list_isremove (list, i) ? "" : "not ",
> - _escape_tag (escaped, tag_op_list_tag (list, i)));
> + escaped);
> join = " or ";
> }
>
> if (query_string)
> query_string = talloc_strdup_append_buffer (query_string, ")");
>
> - talloc_free (escaped);
> + if (escaped)
> + talloc_free (escaped);
> +
> return query_string;
> }
>
> diff --git a/util/string-util.c b/util/string-util.c
> index 44f8cd3..ea7c25b 100644
> --- a/util/string-util.c
> +++ b/util/string-util.c
> @@ -20,6 +20,7 @@
>
>
> #include "string-util.h"
> +#include "talloc.h"
>
> char *
> strtok_len (char *s, const char *delim, size_t *len)
> @@ -32,3 +33,36 @@ strtok_len (char *s, const char *delim, size_t *len)
>
> return *len ? s : NULL;
> }
> +
> +
> +int
> +double_quote_str (void *ctx, const char *str,
> + char **buf, size_t *len)
> +{
> + const char *in;
> + char *out;
> + size_t needed = 3;
> +
> + for (in = str; *in; in++)
> + needed += (*in == '"') ? 2 : 1;
> +
> + if (needed > *len)
> + *buf = talloc_realloc (ctx, *buf, char, 2*needed);
You fail to set *len to 2*needed, leading to doing realloc every time.
Also, I think you should follow the getline pattern like you did in
hex_encode: if *buf == NULL, the input value of *len is ignored.
BR,
Jani.
> +
> + if (! *buf)
> + return 1;
> +
> + out = *buf;
> +
> + *out++ = '"';
> + in = str;
> + while (*in) {
> + if (*in == '"')
> + *out++ = '"';
> + *out++ = *in++;
> + }
> + *out++ = '"';
> + *out = 0;
> +
> + return 0;
> +}
> diff --git a/util/string-util.h b/util/string-util.h
> index ac7676c..b593bc7 100644
> --- a/util/string-util.h
> +++ b/util/string-util.h
> @@ -19,4 +19,12 @@
>
> char *strtok_len (char *s, const char *delim, size_t *len);
>
> +/* Copy str to dest, surrounding with double quotes.
> + * Any internal double-quotes are doubled, i.e. a"b -> "a""b"
> + *
> + * Output is into buf; it may be talloc_realloced
> + * return 0 on success, non-zero on failure.
> + */
> +int double_quote_str (void *talloc_ctx, const char *str,
> + char **buf, size_t *len);
> #endif
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
2012-12-14 13:34 v7 batch tagging series david
` (3 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 04/14] notmuch-tag: factor out double quoting routine david
@ 2012-12-14 13:34 ` david
2012-12-15 17:49 ` Mark Walters
2012-12-15 23:21 ` [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries Jani Nikula
2012-12-14 13:34 ` [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser david
` (8 subsequent siblings)
13 siblings, 2 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
From: David Bremner <bremner@debian.org>
The query is split into tokens, with ' ' and ':' as delimiters. Any
token containing some hex-escaped character is quoted according to
Xapian rules. This maps id:foo%20%22bar to id:"foo ""bar".
This intentionally does not quote prefixes, so they still work as prefixes.
---
tag-util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/tag-util.c b/tag-util.c
index f89669a..e1181f8 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -56,6 +56,56 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
return NULL;
}
+static tag_parse_status_t
+quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,
+ char **query_string)
+{
+ char *tok = encoded;
+ size_t tok_len = 0;
+ char *buf = NULL;
+ size_t buf_len = 0;
+ tag_parse_status_t ret = TAG_PARSE_SUCCESS;
+
+ *query_string = talloc_strdup (ctx, "");
+
+ while (*query_string &&
+ (tok = strtok_len (tok + tok_len, ": ", &tok_len)) != NULL) {
+ char delim = tok[tok_len];
+
+ *(tok + tok_len++) = '\0';
+
+ if (strcspn (tok, "%") < tok_len - 1) {
+ /* something to decode */
+ if (hex_decode_inplace (tok) != HEX_SUCCESS) {
+ ret = line_error (TAG_PARSE_INVALID, line_for_error,
+ "hex decoding of token '%s' failed", tok);
+ goto DONE;
+ }
+
+ if (double_quote_str (ctx, tok, &buf, &buf_len)) {
+ ret = line_error (TAG_PARSE_OUT_OF_MEMORY,
+ line_for_error, "aborting");
+ goto DONE;
+ }
+ *query_string = talloc_asprintf_append_buffer (
+ *query_string, "%s%c", buf, delim);
+
+ } else {
+ /* This is not just an optimization, but used to preserve
+ * prefixes like id:, which cannot be quoted.
+ */
+ *query_string = talloc_asprintf_append_buffer (
+ *query_string, "%s%c", tok, delim);
+ }
+
+ }
+
+ DONE:
+ if (ret != TAG_PARSE_SUCCESS && *query_string)
+ talloc_free (*query_string);
+ return ret;
+}
+
tag_parse_status_t
parse_tag_line (void *ctx, char *line,
tag_op_flag_t flags,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
2012-12-14 13:34 ` [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries david
@ 2012-12-15 17:49 ` Mark Walters
2012-12-15 18:58 ` David Bremner
2012-12-15 23:21 ` [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries Jani Nikula
1 sibling, 1 reply; 29+ messages in thread
From: Mark Walters @ 2012-12-15 17:49 UTC (permalink / raw)
To: david, notmuch; +Cc: David Bremner
On Fri, 14 Dec 2012, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> The query is split into tokens, with ' ' and ':' as delimiters. Any
> token containing some hex-escaped character is quoted according to
> Xapian rules. This maps id:foo%20%22bar to id:"foo ""bar".
> This intentionally does not quote prefixes, so they still work as prefixes.
> ---
> tag-util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/tag-util.c b/tag-util.c
> index f89669a..e1181f8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -56,6 +56,56 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
> return NULL;
> }
>
> +static tag_parse_status_t
> +quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,
> + char **query_string)
> +{
Would decode_and_quote_query be a better name given the order these two
happen? Also a comment describing the function would be nice.
> + char *tok = encoded;
> + size_t tok_len = 0;
> + char *buf = NULL;
> + size_t buf_len = 0;
> + tag_parse_status_t ret = TAG_PARSE_SUCCESS;
> +
> + *query_string = talloc_strdup (ctx, "");
> +
> + while (*query_string &&
> + (tok = strtok_len (tok + tok_len, ": ", &tok_len)) != NULL) {
> + char delim = tok[tok_len];
> +
> + *(tok + tok_len++) = '\0';
These two look a little odd: I would prefer either array or pointer in
both cases.
> +
> + if (strcspn (tok, "%") < tok_len - 1) {
> + /* something to decode */
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of token '%s' failed", tok);
> + goto DONE;
> + }
> +
> + if (double_quote_str (ctx, tok, &buf, &buf_len)) {
> + ret = line_error (TAG_PARSE_OUT_OF_MEMORY,
> + line_for_error, "aborting");
> + goto DONE;
> + }
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", buf, delim);
> +
> + } else {
> + /* This is not just an optimization, but used to preserve
> + * prefixes like id:, which cannot be quoted.
> + */
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", tok, delim);
> + }
What happens if a message id (for example) contains a ':'? Is a query of
the form
id:stuff"encoded_stuff"
acceptable? (As far as I can see from the man page ':' does not need to
be in hex.)
Best wishes
Mark
> +
> + }
> +
> + DONE:
> + if (ret != TAG_PARSE_SUCCESS && *query_string)
> + talloc_free (*query_string);
> + return ret;
> +}
> +
> tag_parse_status_t
> parse_tag_line (void *ctx, char *line,
> tag_op_flag_t flags,
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
2012-12-15 17:49 ` Mark Walters
@ 2012-12-15 18:58 ` David Bremner
2012-12-15 20:09 ` [PATCH] fixup for hex encoding desription in notmuch-tag.1 david
0 siblings, 1 reply; 29+ messages in thread
From: David Bremner @ 2012-12-15 18:58 UTC (permalink / raw)
To: Mark Walters, notmuch
Mark Walters <markwalters1009@gmail.com> writes:
> What happens if a message id (for example) contains a ':'? Is a query of
> the form
>
> id:stuff"encoded_stuff"
>
> acceptable? (As far as I can see from the man page ':' does not need to
> be in hex.)
The updated version of the notmuch-dump man page does say that : will be
hex encoded, so I think the fix here is to update the notmuch tag man
page.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH] fixup for hex encoding desription in notmuch-tag.1
2012-12-15 18:58 ` David Bremner
@ 2012-12-15 20:09 ` david
2012-12-15 23:10 ` Jani Nikula
0 siblings, 1 reply; 29+ messages in thread
From: david @ 2012-12-15 20:09 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
From: David Bremner <bremner@debian.org>
---
and here is the updated description.
man/man1/notmuch-tag.1 | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index ac2c8d7..a203f53 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -67,13 +67,22 @@ The input must consist of lines of the format:
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>
+characters in
+.RI < tag >
+and
+.RI < 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>
+be hex-encoded with %NN where NN is the hexadecimal value of the
+character (more precisely with %NN[%MM...] where NN, MM, etc... are
+the hexadecimal values of the bytes in the UTF-8 encoding of
+the character). Any characters in <tag> and <search-term> not
+matching the regex
+.B [A-Za-z0-9@=.,_+-]
.B must
-be hex encoded (using %20 and %25, respectively). Any characters that
-are not part of <tag> or <search-terms>
+be hex-encoded. Any characters that are not part of
+.RI < tag >
+or
+.RI < search-term >
.B must not
be hex encoded.
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH] fixup for hex encoding desription in notmuch-tag.1
2012-12-15 20:09 ` [PATCH] fixup for hex encoding desription in notmuch-tag.1 david
@ 2012-12-15 23:10 ` Jani Nikula
0 siblings, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2012-12-15 23:10 UTC (permalink / raw)
To: david, notmuch; +Cc: David Bremner
On Sat, 15 Dec 2012, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> ---
> and here is the updated description.
>
> man/man1/notmuch-tag.1 | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
> index ac2c8d7..a203f53 100644
> --- a/man/man1/notmuch-tag.1
> +++ b/man/man1/notmuch-tag.1
> @@ -67,13 +67,22 @@ The input must consist of lines of the format:
> 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>
> +characters in
> +.RI < tag >
> +and
> +.RI < 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>
> +be hex-encoded with %NN where NN is the hexadecimal value of the
> +character (more precisely with %NN[%MM...] where NN, MM, etc... are
> +the hexadecimal values of the bytes in the UTF-8 encoding of
> +the character). Any characters in <tag> and <search-term> not
> +matching the regex
> +.B [A-Za-z0-9@=.,_+-]
And this actually means you can't have "id:foo" there, as : needs to be
encoded. Not user friendly at all. If it should really mean : is okay
un-encoded as prefix delimiter but not otherwise... it gets a bit messy
to document.
BR,
Jani.
> .B must
> -be hex encoded (using %20 and %25, respectively). Any characters that
> -are not part of <tag> or <search-terms>
> +be hex-encoded. Any characters that are not part of
> +.RI < tag >
> +or
> +.RI < search-term >
> .B must not
> be hex encoded.
>
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
2012-12-14 13:34 ` [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries david
2012-12-15 17:49 ` Mark Walters
@ 2012-12-15 23:21 ` Jani Nikula
2012-12-16 3:39 ` David Bremner
1 sibling, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2012-12-15 23:21 UTC (permalink / raw)
To: david, notmuch; +Cc: David Bremner
On Fri, 14 Dec 2012, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> The query is split into tokens, with ' ' and ':' as delimiters. Any
> token containing some hex-escaped character is quoted according to
> Xapian rules. This maps id:foo%20%22bar to id:"foo ""bar".
> This intentionally does not quote prefixes, so they still work as prefixes.
> ---
> tag-util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/tag-util.c b/tag-util.c
> index f89669a..e1181f8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -56,6 +56,56 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
> return NULL;
> }
>
> +static tag_parse_status_t
> +quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,
> + char **query_string)
> +{
> + char *tok = encoded;
> + size_t tok_len = 0;
> + char *buf = NULL;
> + size_t buf_len = 0;
> + tag_parse_status_t ret = TAG_PARSE_SUCCESS;
> +
> + *query_string = talloc_strdup (ctx, "");
> +
> + while (*query_string &&
> + (tok = strtok_len (tok + tok_len, ": ", &tok_len)) != NULL) {
strtok_len() will eat all the leading delimiters at each call, and will
not return a zero-length token if you have multiple consecutive
delimiters. Which means you may end up losing stuff here. Whether that
matters or not I'm too tired to tell...
BR,
Jani.
> + char delim = tok[tok_len];
> +
> + *(tok + tok_len++) = '\0';
> +
> + if (strcspn (tok, "%") < tok_len - 1) {
> + /* something to decode */
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of token '%s' failed", tok);
> + goto DONE;
> + }
> +
> + if (double_quote_str (ctx, tok, &buf, &buf_len)) {
> + ret = line_error (TAG_PARSE_OUT_OF_MEMORY,
> + line_for_error, "aborting");
> + goto DONE;
> + }
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", buf, delim);
> +
> + } else {
> + /* This is not just an optimization, but used to preserve
> + * prefixes like id:, which cannot be quoted.
> + */
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", tok, delim);
> + }
> +
> + }
> +
> + DONE:
> + if (ret != TAG_PARSE_SUCCESS && *query_string)
> + talloc_free (*query_string);
> + return ret;
> +}
> +
> tag_parse_status_t
> parse_tag_line (void *ctx, char *line,
> tag_op_flag_t flags,
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
2012-12-14 13:34 v7 batch tagging series david
` (4 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries david
@ 2012-12-14 13:34 ` david
2012-12-15 17:54 ` Mark Walters
2012-12-15 23:04 ` [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser Jani Nikula
2012-12-14 13:34 ` [Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag" david
` (7 subsequent siblings)
13 siblings, 2 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
From: David Bremner <bremner@debian.org>
We are able to detect more errors by looking at the string before it
is hex-decoded. We also need this to avoid the query quoting for more
general queries (to be written) that will mess up raw message-ids.
---
notmuch-restore.c | 18 +-----------------
tag-util.c | 26 ++++++++++++++++++++------
tag-util.h | 5 ++++-
test/dump-restore | 5 ++---
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 40596a8..112f2f3 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -208,24 +208,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
if (input_format == DUMP_FORMAT_SUP) {
ret = parse_sup_line (ctx, line, &query_string, tag_ops);
} else {
- ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+ ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | TAG_FLAG_ID_ONLY,
&query_string, tag_ops);
-
- if (ret == 0) {
- if (strncmp ("id:", query_string, 3) != 0) {
- fprintf (stderr, "Warning: unsupported query: %s\n", query_string);
- continue;
- }
- /* delete id: from front of string; tag_message
- * expects a raw message-id.
- *
- * XXX: Note that query string id:foo and bar will be
- * interpreted as a message id "foo and bar". This
- * should eventually be fixed to give a better error
- * message.
- */
- query_string = query_string + 3;
- }
}
if (ret > 0)
diff --git a/tag-util.c b/tag-util.c
index e1181f8..8fea76c 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -201,14 +201,28 @@ parse_tag_line (void *ctx, char *line,
}
/* tok now points to the query string */
- if (hex_decode_inplace (tok) != HEX_SUCCESS) {
- ret = line_error (TAG_PARSE_INVALID, line_for_error,
- "hex decoding of query %s failed", tok);
- goto DONE;
+ if (flags & TAG_FLAG_ID_ONLY) {
+ /* this is under the assumption that any whitespace in the
+ * message-id must be hex-encoded. The check is probably not
+ * perfect for exotic unicode whitespace; as fallback the
+ * search for strange message-ids will fail */
+ if ((strncmp ("id:", tok, 3) != 0) ||
+ (strcspn (tok, " \t") < strlen (tok))) {
+ ret = line_error (TAG_PARSE_INVALID, line_for_error,
+ "query '%s' is not 'id:<message-id>'", tok);
+ goto DONE;
+ }
+ if (hex_decode_inplace (tok) != HEX_SUCCESS) {
+ ret = line_error (TAG_PARSE_INVALID, line_for_error,
+ "hex decoding of query %s failed", tok);
+ goto DONE;
+ }
+ /* skip 'id:' */
+ *query_string = tok + 3;
+ } else {
+ ret = quote_and_decode_query (ctx, tok, line_for_error, query_string);
}
- *query_string = tok;
-
DONE:
talloc_free (line_for_error);
return ret;
diff --git a/tag-util.h b/tag-util.h
index 2889736..7674051 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -26,7 +26,10 @@ typedef enum {
/* Accept strange tags that might be user error;
* intended for use by notmuch-restore.
*/
- TAG_FLAG_BE_GENEROUS = (1 << 3)
+ TAG_FLAG_BE_GENEROUS = (1 << 3),
+
+ /* Query consists of a single id:$message-id */
+ TAG_FLAG_ID_ONLY = (1 << 4)
} tag_op_flag_t;
diff --git a/test/dump-restore b/test/dump-restore
index 6a989b6..eb7933a 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -199,19 +199,18 @@ a
# the next non-comment line should report an an empty tag error for
# batch tagging, but not for restore
+ +e -- id:20091117232137.GA7669@griffis1.net
-# highlight the sketchy id parsing; this should be last
+g -- id:foo and bar
EOF
cat <<EOF > EXPECTED
-Warning: unsupported query: a
+Warning: query 'a' is not 'id:<message-id>' [a]
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: hex decoding of query id:%yy failed [+e +f id:%yy]
-Warning: cannot apply tags to missing message: foo and bar
+Warning: query 'id:foo and bar' is not 'id:<message-id>' [+g -- id:foo and bar]
EOF
test_expect_equal_file EXPECTED OUTPUT
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
2012-12-14 13:34 ` [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser david
@ 2012-12-15 17:54 ` Mark Walters
2012-12-15 18:18 ` David Bremner
2012-12-15 19:21 ` [PATCH] fixup: clarify TAG_FLAG_ID_ONLY comments and name david
2012-12-15 23:04 ` [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser Jani Nikula
1 sibling, 2 replies; 29+ messages in thread
From: Mark Walters @ 2012-12-15 17:54 UTC (permalink / raw)
To: david, notmuch; +Cc: David Bremner
On Fri, 14 Dec 2012, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> We are able to detect more errors by looking at the string before it
> is hex-decoded. We also need this to avoid the query quoting for more
> general queries (to be written) that will mess up raw message-ids.
> ---
> notmuch-restore.c | 18 +-----------------
> tag-util.c | 26 ++++++++++++++++++++------
> tag-util.h | 5 ++++-
> test/dump-restore | 5 ++---
> 4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index 40596a8..112f2f3 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -208,24 +208,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
> if (input_format == DUMP_FORMAT_SUP) {
> ret = parse_sup_line (ctx, line, &query_string, tag_ops);
> } else {
> - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
> + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | TAG_FLAG_ID_ONLY,
> &query_string, tag_ops);
> -
> - if (ret == 0) {
> - if (strncmp ("id:", query_string, 3) != 0) {
> - fprintf (stderr, "Warning: unsupported query: %s\n", query_string);
> - continue;
> - }
> - /* delete id: from front of string; tag_message
> - * expects a raw message-id.
> - *
> - * XXX: Note that query string id:foo and bar will be
> - * interpreted as a message id "foo and bar". This
> - * should eventually be fixed to give a better error
> - * message.
> - */
> - query_string = query_string + 3;
> - }
> }
>
> if (ret > 0)
> diff --git a/tag-util.c b/tag-util.c
> index e1181f8..8fea76c 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -201,14 +201,28 @@ parse_tag_line (void *ctx, char *line,
> }
>
> /* tok now points to the query string */
> - if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> - ret = line_error (TAG_PARSE_INVALID, line_for_error,
> - "hex decoding of query %s failed", tok);
> - goto DONE;
> + if (flags & TAG_FLAG_ID_ONLY) {
> + /* this is under the assumption that any whitespace in the
> + * message-id must be hex-encoded. The check is probably not
> + * perfect for exotic unicode whitespace; as fallback the
> + * search for strange message-ids will fail */
> + if ((strncmp ("id:", tok, 3) != 0) ||
> + (strcspn (tok, " \t") < strlen (tok))) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "query '%s' is not 'id:<message-id>'", tok);
> + goto DONE;
> + }
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of query %s failed", tok);
> + goto DONE;
> + }
> + /* skip 'id:' */
> + *query_string = tok + 3;
This looks like it doesn't double_quote the query_string in this (the
TAG_FLAG_ID_ONLY) case. Is that deliberate?
Best wishes
Mark
> + } else {
> + ret = quote_and_decode_query (ctx, tok, line_for_error, query_string);
> }
>
> - *query_string = tok;
> -
> DONE:
> talloc_free (line_for_error);
> return ret;
> diff --git a/tag-util.h b/tag-util.h
> index 2889736..7674051 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -26,7 +26,10 @@ typedef enum {
> /* Accept strange tags that might be user error;
> * intended for use by notmuch-restore.
> */
> - TAG_FLAG_BE_GENEROUS = (1 << 3)
> + TAG_FLAG_BE_GENEROUS = (1 << 3),
> +
> + /* Query consists of a single id:$message-id */
> + TAG_FLAG_ID_ONLY = (1 << 4)
>
> } tag_op_flag_t;
>
> diff --git a/test/dump-restore b/test/dump-restore
> index 6a989b6..eb7933a 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -199,19 +199,18 @@ a
> # the next non-comment line should report an an empty tag error for
> # batch tagging, but not for restore
> + +e -- id:20091117232137.GA7669@griffis1.net
> -# highlight the sketchy id parsing; this should be last
> +g -- id:foo and bar
> EOF
>
> cat <<EOF > EXPECTED
> -Warning: unsupported query: a
> +Warning: query 'a' is not 'id:<message-id>' [a]
> 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: hex decoding of query id:%yy failed [+e +f id:%yy]
> -Warning: cannot apply tags to missing message: foo and bar
> +Warning: query 'id:foo and bar' is not 'id:<message-id>' [+g -- id:foo and bar]
> EOF
>
> test_expect_equal_file EXPECTED OUTPUT
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
2012-12-15 17:54 ` Mark Walters
@ 2012-12-15 18:18 ` David Bremner
2012-12-15 19:21 ` [PATCH] fixup: clarify TAG_FLAG_ID_ONLY comments and name david
1 sibling, 0 replies; 29+ messages in thread
From: David Bremner @ 2012-12-15 18:18 UTC (permalink / raw)
To: Mark Walters, notmuch
Mark Walters <markwalters1009@gmail.com> writes:
>> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
>> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
>> + "hex decoding of query %s failed", tok);
>> + goto DONE;
>> + }
>> + /* skip 'id:' */
>> + *query_string = tok + 3;
>
> This looks like it doesn't double_quote the query_string in this (the
> TAG_FLAG_ID_ONLY) case. Is that deliberate?
Yes, that's what I meant by
,----
| We also need this to avoid the query quoting for more
| general queries (to be written) that will mess up raw message-ids.
`----
perhaps it deserves a comment in the code.
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH] fixup: clarify TAG_FLAG_ID_ONLY comments and name
2012-12-15 17:54 ` Mark Walters
2012-12-15 18:18 ` David Bremner
@ 2012-12-15 19:21 ` david
1 sibling, 0 replies; 29+ messages in thread
From: david @ 2012-12-15 19:21 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
From: David Bremner <bremner@debian.org>
---
After some chatter on IRC, Mark and I converged to the following
notmuch-restore.c | 2 +-
tag-util.c | 2 +-
tag-util.h | 6 ++++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 112f2f3..1b66e76 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -208,7 +208,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
if (input_format == DUMP_FORMAT_SUP) {
ret = parse_sup_line (ctx, line, &query_string, tag_ops);
} else {
- ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | TAG_FLAG_ID_ONLY,
+ ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | TAG_FLAG_ID_DIRECT,
&query_string, tag_ops);
}
diff --git a/tag-util.c b/tag-util.c
index 8fea76c..37bffd5 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -201,7 +201,7 @@ parse_tag_line (void *ctx, char *line,
}
/* tok now points to the query string */
- if (flags & TAG_FLAG_ID_ONLY) {
+ if (flags & TAG_FLAG_ID_DIRECT) {
/* this is under the assumption that any whitespace in the
* message-id must be hex-encoded. The check is probably not
* perfect for exotic unicode whitespace; as fallback the
diff --git a/tag-util.h b/tag-util.h
index 7674051..eec00cf 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -28,8 +28,10 @@ typedef enum {
*/
TAG_FLAG_BE_GENEROUS = (1 << 3),
- /* Query consists of a single id:$message-id */
- TAG_FLAG_ID_ONLY = (1 << 4)
+ /* Directly look up messages by hex-decoded message-id, rather
+ * than parsing a general query. The query MUST be of the form
+ * id:$message-id. */
+ TAG_FLAG_ID_DIRECT = (1 << 4)
} tag_op_flag_t;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
2012-12-14 13:34 ` [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser david
2012-12-15 17:54 ` Mark Walters
@ 2012-12-15 23:04 ` Jani Nikula
1 sibling, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2012-12-15 23:04 UTC (permalink / raw)
To: david, notmuch; +Cc: David Bremner
On Fri, 14 Dec 2012, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> We are able to detect more errors by looking at the string before it
> is hex-decoded. We also need this to avoid the query quoting for more
> general queries (to be written) that will mess up raw message-ids.
> ---
> notmuch-restore.c | 18 +-----------------
> tag-util.c | 26 ++++++++++++++++++++------
> tag-util.h | 5 ++++-
> test/dump-restore | 5 ++---
> 4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index 40596a8..112f2f3 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -208,24 +208,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
> if (input_format == DUMP_FORMAT_SUP) {
> ret = parse_sup_line (ctx, line, &query_string, tag_ops);
> } else {
> - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
> + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | TAG_FLAG_ID_ONLY,
> &query_string, tag_ops);
I realize that we've screwed up a bit here already in the restore
series. parse_sup_line() allocates query_string for each input line, but
doesn't free it during parsing. parse_tag_line() sets query_string to
point to the query string within line. And now it gets worse when
query_string is allocated vs. set to point to another buffer depending
on TAG_FLAG_ID_ONLY, so it's not an easy interface to use.
> -
> - if (ret == 0) {
> - if (strncmp ("id:", query_string, 3) != 0) {
> - fprintf (stderr, "Warning: unsupported query: %s\n", query_string);
> - continue;
> - }
> - /* delete id: from front of string; tag_message
> - * expects a raw message-id.
> - *
> - * XXX: Note that query string id:foo and bar will be
> - * interpreted as a message id "foo and bar". This
> - * should eventually be fixed to give a better error
> - * message.
> - */
> - query_string = query_string + 3;
> - }
> }
>
> if (ret > 0)
> diff --git a/tag-util.c b/tag-util.c
> index e1181f8..8fea76c 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -201,14 +201,28 @@ parse_tag_line (void *ctx, char *line,
> }
>
> /* tok now points to the query string */
> - if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> - ret = line_error (TAG_PARSE_INVALID, line_for_error,
> - "hex decoding of query %s failed", tok);
> - goto DONE;
> + if (flags & TAG_FLAG_ID_ONLY) {
> + /* this is under the assumption that any whitespace in the
> + * message-id must be hex-encoded. The check is probably not
> + * perfect for exotic unicode whitespace; as fallback the
> + * search for strange message-ids will fail */
> + if ((strncmp ("id:", tok, 3) != 0) ||
The current wording is, "Any characters in <tag> and <search-term> MAY
be hex encoded with %NN...", but the above no longer matches that, as
"id:" must not be encoded. In the interest of keeping the documentation
concise, I think you should move the above check after
hex_decode_inplace(), but keep the below check here. That should do it,
right?
> + (strcspn (tok, " \t") < strlen (tok))) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "query '%s' is not 'id:<message-id>'", tok);
> + goto DONE;
> + }
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of query %s failed", tok);
> + goto DONE;
> + }
> + /* skip 'id:' */
> + *query_string = tok + 3;
> + } else {
> + ret = quote_and_decode_query (ctx, tok, line_for_error, query_string);
> }
It's not pretty that query_string gets allocated or not depending on a
flag that doesn't seem to have anything to do with that. I don't have a
good suggestion now, though, and I'd like to keep the restore path like
it is, without allocation.
>
> - *query_string = tok;
> -
> DONE:
> talloc_free (line_for_error);
> return ret;
> diff --git a/tag-util.h b/tag-util.h
> index 2889736..7674051 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -26,7 +26,10 @@ typedef enum {
> /* Accept strange tags that might be user error;
> * intended for use by notmuch-restore.
> */
> - TAG_FLAG_BE_GENEROUS = (1 << 3)
> + TAG_FLAG_BE_GENEROUS = (1 << 3),
> +
> + /* Query consists of a single id:$message-id */
> + TAG_FLAG_ID_ONLY = (1 << 4)
>
> } tag_op_flag_t;
>
> diff --git a/test/dump-restore b/test/dump-restore
> index 6a989b6..eb7933a 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -199,19 +199,18 @@ a
> # the next non-comment line should report an an empty tag error for
> # batch tagging, but not for restore
> + +e -- id:20091117232137.GA7669@griffis1.net
> -# highlight the sketchy id parsing; this should be last
> +g -- id:foo and bar
> EOF
>
> cat <<EOF > EXPECTED
> -Warning: unsupported query: a
> +Warning: query 'a' is not 'id:<message-id>' [a]
> 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: hex decoding of query id:%yy failed [+e +f id:%yy]
> -Warning: cannot apply tags to missing message: foo and bar
> +Warning: query 'id:foo and bar' is not 'id:<message-id>' [+g -- id:foo and bar]
> EOF
>
> test_expect_equal_file EXPECTED OUTPUT
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag"
2012-12-14 13:34 v7 batch tagging series david
` (5 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser david
@ 2012-12-14 13:34 ` david
2012-12-15 23:14 ` Jani Nikula
2012-12-16 0:23 ` [PATCH] " david
2012-12-14 13:34 ` [Patch v7 08/14] test/tagging: add test for error messages of tag --batch david
` (6 subsequent siblings)
13 siblings, 2 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 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> [...] [--] <search-term> [...]
Each line is interpreted similarly to "notmuch tag" command line
arguments. The delimiter is one or more spaces ' '. Any characters in
<tag> and <search-term> MAY 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
<search-term> 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 13f2268..a81d911 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -130,6 +130,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[])
{
@@ -139,6 +176,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 */
@@ -148,15 +189,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)
@@ -169,9 +238,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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag"
2012-12-14 13:34 ` [Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag" david
@ 2012-12-15 23:14 ` Jani Nikula
2012-12-16 0:23 ` [PATCH] " david
1 sibling, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2012-12-15 23:14 UTC (permalink / raw)
To: david, notmuch
On Fri, 14 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-term> [...]
>
> Each line is interpreted similarly to "notmuch tag" command line
> arguments. The delimiter is one or more spaces ' '. Any characters in
> <tag> and <search-term> MAY 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
> <search-term> 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 13f2268..a81d911 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -130,6 +130,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;
Hey, your changelog in the cover letter says you fixed the tag_query
return value propagation, but it's not here?
BR,
Jani.
> + }
> +
> + if (line)
> + free (line);
> +
> + return ret;
> +}
> +
> int
> notmuch_tag_command (void *ctx, int argc, char *argv[])
> {
> @@ -139,6 +176,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 */
> @@ -148,15 +189,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)
> @@ -169,9 +238,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
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH] cli: add support for batch tagging operations to "notmuch tag"
2012-12-14 13:34 ` [Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag" david
2012-12-15 23:14 ` Jani Nikula
@ 2012-12-16 0:23 ` david
1 sibling, 0 replies; 29+ messages in thread
From: david @ 2012-12-16 0:23 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> [...] [--] <search-term> [...]
Each line is interpreted similarly to "notmuch tag" command line
arguments. The delimiter is one or more spaces ' '. Any characters in
<tag> and <search-term> MAY 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
<search-term> 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>
---
I missed somehow the change catch non-zero return from tag_query in tag_file.
This version is slightly cleaned up.
notmuch-tag.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 83 insertions(+), 8 deletions(-)
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 13f2268..44b5bb4 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -130,6 +130,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[])
{
@@ -139,6 +179,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 */
@@ -148,15 +192,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)
@@ -169,9 +241,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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Patch v7 08/14] test/tagging: add test for error messages of tag --batch
2012-12-14 13:34 v7 batch tagging series david
` (6 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag" david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 09/14] test/tagging: add basic tests for batch tagging functionality david
` (5 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 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 | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/test/tagging b/test/tagging
index 980ff92..30cec48 100755
--- a/test/tagging
+++ b/test/tagging
@@ -46,6 +46,43 @@ 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
++e +f id:%yy
+# 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: hex decoding of token '%yy' failed [+e +f id:%yy]
+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] 29+ messages in thread* [Patch v7 09/14] test/tagging: add basic tests for batch tagging functionality
2012-12-14 13:34 v7 batch tagging series david
` (7 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 08/14] test/tagging: add test for error messages of tag --batch david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 10/14] test/tagging: add tests for exotic tags david
` (4 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 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 30cec48..b454d38 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] 29+ messages in thread* [Patch v7 10/14] test/tagging: add tests for exotic tags
2012-12-14 13:34 v7 batch tagging series david
` (8 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 09/14] test/tagging: add basic tests for batch tagging functionality david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 11/14] test/tagging: add test for exotic message-ids and batch tagging david
` (3 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 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 b454d38..18f532e 100755
--- a/test/tagging
+++ b/test/tagging
@@ -134,6 +134,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] 29+ messages in thread* [Patch v7 11/14] test/tagging: add test for exotic message-ids and batch tagging
2012-12-14 13:34 v7 batch tagging series david
` (9 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 10/14] test/tagging: add tests for exotic tags david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 12/14] test/tagging: add test for compound queries with " david
` (2 subsequent siblings)
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 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 18f532e..1d59af0 100755
--- a/test/tagging
+++ b/test/tagging
@@ -200,6 +200,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] 29+ messages in thread* [Patch v7 12/14] test/tagging: add test for compound queries with batch tagging
2012-12-14 13:34 v7 batch tagging series david
` (10 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 11/14] test/tagging: add test for exotic message-ids and batch tagging david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 13/14] notmuch-tag.1: tidy synopsis formatting david
2012-12-14 13:34 ` [Patch v7 14/14] man: document notmuch tag --batch, --input options david
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
From: David Bremner <bremner@debian.org>
This is to watch for errors in quoting the query.
---
test/tagging | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/test/tagging b/test/tagging
index 1d59af0..bd8eeb5 100755
--- a/test/tagging
+++ b/test/tagging
@@ -174,6 +174,28 @@ notmuch dump --format=batch-tag | sort > OUTPUT
notmuch restore --format=batch-tag < BACKUP
test_expect_equal_file EXPECTED OUTPUT
+test_begin_subtest '--batch: compound queries'
+notmuch dump --format=batch-tag > BACKUP
+
+notmuch tag --batch <<EOF
++compound1 -- One and Two
++compound2 -- One or Two
++tag%20with%20spaces -- *
++compound3 -- id:msg-002@notmuch-test-suite and Two
++compound3 -- Two and id:msg-002@notmuch-test-suite
+-unread -- tag:tag%20with%20spaces and id:msg-002@notmuch-test-suite
+-tag%20with%20spaces -- tag:unread and tag:compound2
+EOF
+
+cat <<EOF > EXPECTED
++compound2 +compound3 +inbox +tag%20with%20spaces +tag4 +tag5 -- id:msg-002@notmuch-test-suite
++compound2 +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
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Patch v7 13/14] notmuch-tag.1: tidy synopsis formatting
2012-12-14 13:34 v7 batch tagging series david
` (11 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 12/14] test/tagging: add test for compound queries with " david
@ 2012-12-14 13:34 ` david
2012-12-14 13:34 ` [Patch v7 14/14] man: document notmuch tag --batch, --input options david
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Patch v7 14/14] man: document notmuch tag --batch, --input options
2012-12-14 13:34 v7 batch tagging series david
` (12 preceding siblings ...)
2012-12-14 13:34 ` [Patch v7 13/14] notmuch-tag.1: tidy synopsis formatting david
@ 2012-12-14 13:34 ` david
13 siblings, 0 replies; 29+ messages in thread
From: david @ 2012-12-14 13:34 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 29+ messages in thread