--- /dev/null
+Return-Path: <amdragon@mit.edu>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id BA664431FAF\r
+ for <notmuch@notmuchmail.org>; Sat, 24 Nov 2012 16:23:34 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id JDXLZN2cXbGF for <notmuch@notmuchmail.org>;\r
+ Sat, 24 Nov 2012 16:23:30 -0800 (PST)\r
+Received: from dmz-mailsec-scanner-4.mit.edu (DMZ-MAILSEC-SCANNER-4.MIT.EDU\r
+ [18.9.25.15])\r
+ by olra.theworths.org (Postfix) with ESMTP id 89EDC431FAE\r
+ for <notmuch@notmuchmail.org>; Sat, 24 Nov 2012 16:23:30 -0800 (PST)\r
+X-AuditID: 1209190f-b7f636d00000095b-b6-50b16501721c\r
+Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
+ by dmz-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP\r
+ id CA.96.02395.10561B05; Sat, 24 Nov 2012 19:23:29 -0500 (EST)\r
+Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
+ by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id qAP0NSwK027407; \r
+ Sat, 24 Nov 2012 19:23:29 -0500\r
+Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
+ (authenticated bits=0)\r
+ (User authenticated as amdragon@ATHENA.MIT.EDU)\r
+ by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id qAP0NQPG005608\r
+ (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
+ Sat, 24 Nov 2012 19:23:27 -0500 (EST)\r
+Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
+ (envelope-from <amdragon@mit.edu>)\r
+ id 1TcQ0M-0006XZ-GL; Sat, 24 Nov 2012 19:23:26 -0500\r
+Date: Sat, 24 Nov 2012 19:23:26 -0500\r
+From: Austin Clements <amdragon@MIT.EDU>\r
+To: markwalters1009 <markwalters1009@gmail.com>\r
+Subject: Re: [PATCH v2 6/7] cli: allow search mode to include msg-ids with\r
+ JSON output\r
+Message-ID: <20121125002326.GJ4562@mit.edu>\r
+References: <1353763256-32336-1-git-send-email-markwalters1009@gmail.com>\r
+ <1353763256-32336-7-git-send-email-markwalters1009@gmail.com>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Content-Disposition: inline\r
+In-Reply-To: <1353763256-32336-7-git-send-email-markwalters1009@gmail.com>\r
+User-Agent: Mutt/1.5.21 (2010-09-15)\r
+X-Brightmail-Tracker:\r
+ H4sIAAAAAAAAA+NgFprAKsWRmVeSWpSXmKPExsUixG6nrsuYujHA4PMrbovVc3ksrt+cyezA\r
+ 5LFz1l12j2erbjEHMEVx2aSk5mSWpRbp2yVwZfw7som5YLNfxaql65gbGCdYdzFyckgImEj8\r
+ 3NfHDGGLSVy4t56ti5GLQ0hgH6PEkm8PmCCcDYwS16ZvY4VwLjJJPJj6jgXCWcIoseLNHUaQ\r
+ fhYBVYkTZ2+wgNhsAhoS2/YvB4uLCOhL7Flxmw3EZhaQlvj2u5kJxBYWiJD4/fwUWA2vgLZE\r
+ y8UORoihnYwSf4/thkoISpyc+YQFollL4sa/l0DNHGCDlv/jAAlzCnhJHH3Qyw5iiwqoSEw5\r
+ uY1tAqPQLCTds5B0z0LoXsDIvIpRNiW3Sjc3MTOnODVZtzg5MS8vtUjXRC83s0QvNaV0EyMo\r
+ sDkl+XcwfjuodIhRgINRiYf3RuLGACHWxLLiytxDjJIcTEqivFOSgUJ8SfkplRmJxRnxRaU5\r
+ qcWHGCU4mJVEeK1VgXK8KYmVValF+TApaQ4WJXHeqyk3/YUE0hNLUrNTUwtSi2CyMhwcShK8\r
+ 00GGChalpqdWpGXmlCCkmTg4QYbzAA1vAKnhLS5IzC3OTIfIn2LU5Zgzs/0JoxBLXn5eqpQ4\r
+ 71GQIgGQoozSPLg5sIT0ilEc6C1h3osgVTzAZAY36RXQEiagJU9nrwNZUpKIkJJqYJzvJH1j\r
+ 3ZIkuXMFt7N7lryxuj6ja+IuhrDtL/823TON/b/oOfvJxNOTjUyZds1ed2NuFU+G5XnmhN9B\r
+ Ptb9PRxv5QW56qetlbl7XK54DbPE62PvJJ793HhpzcEFmY08t4wdZ+s55+Y2bG9s1JZ4wurJ\r
+ 1npe5+0nDq2JbWcWeHjwa0QkaU5+eFaJpTgj0VCLuag4EQCVq5jlIwMAAA==\r
+Cc: notmuch@notmuchmail.org\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Sun, 25 Nov 2012 00:23:34 -0000\r
+\r
+Quoth markwalters1009 on Nov 24 at 1:20 pm:\r
+> From: Mark Walters <markwalters1009@gmail.com>\r
+> \r
+> This adds a --queries=true option which modifies the summary output of\r
+> notmuch search by including two extra query strings with each result:\r
+> one query string specifies all matching messages and one query string\r
+> all non-matching messages. Currently these are just lists of message\r
+> ids joined with " or " but that could change in future.\r
+> \r
+> Currently this is not implemented for text format.\r
+> ---\r
+> notmuch-search.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++---\r
+> 1 files changed, 89 insertions(+), 6 deletions(-)\r
+> \r
+> diff --git a/notmuch-search.c b/notmuch-search.c\r
+> index 830c4e4..c8fc9a6 100644\r
+> --- a/notmuch-search.c\r
+> +++ b/notmuch-search.c\r
+> @@ -26,7 +26,8 @@ typedef enum {\r
+> OUTPUT_THREADS,\r
+> OUTPUT_MESSAGES,\r
+> OUTPUT_FILES,\r
+> - OUTPUT_TAGS\r
+> + OUTPUT_TAGS,\r
+> + OUTPUT_SUMMARY_WITH_QUERIES\r
+> } output_t;\r
+> \r
+> static char *\r
+> @@ -46,6 +47,57 @@ sanitize_string (const void *ctx, const char *str)\r
+> return out;\r
+> }\r
+> \r
+> +/* This function takes a message id and returns an escaped string\r
+> + * which can be used as a Xapian query. This involves prefixing with\r
+> + * `id:', putting the id inside double quotes, and doubling any\r
+> + * occurence of a double quote in the message id itself.*/\r
+> +static char *\r
+> +xapian_escape_id (const void *ctx,\r
+> + const char *msg_id)\r
+> +{\r
+> + const char *c;\r
+> + char *escaped_msg_id;\r
+> + escaped_msg_id = talloc_strdup (ctx, "id:\"");\r
+\r
+talloc_strdup can fail.\r
+\r
+> + for (c=msg_id; *c; c++)\r
+\r
+Missing spaces around =.\r
+\r
+> + if (*c == '"')\r
+> + escaped_msg_id = talloc_asprintf_append (escaped_msg_id, "\"\"");\r
+> + else\r
+> + escaped_msg_id = talloc_asprintf_append (escaped_msg_id, "%c", *c);\r
+\r
+.. as can talloc_asprintf_append.\r
+\r
+> + escaped_msg_id = talloc_asprintf_append (escaped_msg_id, "\"");\r
+> + return escaped_msg_id;\r
+> +}\r
+\r
+Unfortunately, this approach will cause reallocation and copying for\r
+every character in msg_id (as well as requiring talloc_asprintf_append\r
+to re-scan escaped_msg_id to find the end on every iteration). How\r
+about pre-allocating a large enough buffer and keeping your position\r
+in it, like\r
+\r
+/* This function takes a message id and returns an escaped string\r
+ * which can be used as a Xapian query. This involves prefixing with\r
+ * `id:', putting the id inside double quotes, and doubling any\r
+ * occurence of a double quote in the message id itself. Returns NULL\r
+ * if memory allocation fails. */\r
+static char *\r
+xapian_escape_id (const void *ctx,\r
+ const char *msg_id)\r
+{\r
+ const char *in;\r
+ char *out;\r
+ char *escaped_msg_id = talloc_array (ctx, char, 6 + strlen (msg_id) * 2);\r
+ if (!escaped_msg_id)\r
+ return NULL;\r
+ strcpy (escaped_msg_id, "id:\"");\r
+ out = escaped_msg_id + 4;\r
+ for (in = msg_id; *in; ++in) {\r
+ if (*in == '"')\r
+ *(out++) = '"';\r
+ *(out++) = *in;\r
+ }\r
+ strcpy(out, "\"");\r
+ return escaped_msg_id;\r
+}\r
+\r
+> +\r
+> +static char *\r
+> +output_msg_query (const void *ctx,\r
+> + sprinter_t *format,\r
+> + notmuch_bool_t matching,\r
+> + notmuch_bool_t first,\r
+> + notmuch_messages_t *messages)\r
+> +{\r
+> + notmuch_message_t *message;\r
+> + char *query, *escaped_msg_id;\r
+> + query = talloc_strdup (ctx, "");\r
+> + for (;\r
+> + notmuch_messages_valid (messages);\r
+> + notmuch_messages_move_to_next (messages))\r
+> + {\r
+> + message = notmuch_messages_get (messages);\r
+> + if (notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) == matching) {\r
+> + escaped_msg_id = xapian_escape_id (ctx, notmuch_message_get_message_id (message));\r
+\r
+Two long lines.\r
+\r
+> + if (first) {\r
+> + query = talloc_asprintf_append (query, "%s", escaped_msg_id);\r
+> + first = FALSE;\r
+> + }\r
+> + else\r
+\r
+"} else".\r
+\r
+> + query = talloc_asprintf_append (query, " or %s", escaped_msg_id);\r
+\r
+The "or" is unnecessary, since id is registered with the query parser\r
+as an exclusive boolean term.\r
+\r
+You could simplify this to\r
+\r
+ query = talloc_asprintf_append (query, "%s%s", first ? "" : " ", \r
+ escaped_msg_id);\r
+\r
+Technically this loop has the same O(n^2) problem as xapian_escape_id\r
+and query_string_from_args, but given that threads rarely have more\r
+than a few dozen messages in them, perhaps it doesn't matter. OTOH,\r
+this may deal poorly with pathological threads (autogenerated messages\r
+and such).\r
+\r
+I wonder if we should have some simple linear-time talloc string\r
+accumulation abstraction in util/...\r
+\r
+> + talloc_free (escaped_msg_id);\r
+> + }\r
+> + /* output_msg_query already starts with an ` or' */\r
+> + query = talloc_asprintf_append (query, "%s", output_msg_query (ctx, format, matching, first, notmuch_message_get_replies (message)));\r
+\r
+Oof, how unfortunate. I've got a patch that adds an iterator over all\r
+of the messages in a thread, which would make this much simpler (I\r
+don't know how we've gotten this far without such an API). I'll clean\r
+that up and send it. This shouldn't block this patch, but whichever\r
+goes in second should clean this up.\r
+\r
+Also, long line.\r
+\r
+> + }\r
+> + return query;\r
+> +}\r
+> +\r
+> static int\r
+> do_search_threads (sprinter_t *format,\r
+> notmuch_query_t *query,\r
+> @@ -88,7 +140,7 @@ do_search_threads (sprinter_t *format,\r
+> format->string (format,\r
+> notmuch_thread_get_thread_id (thread));\r
+> format->separator (format);\r
+> - } else { /* output == OUTPUT_SUMMARY */\r
+> + } else { /* output == OUTPUT_SUMMARY or OUTPUT_SUMMARY_WITH_QUERIES */\r
+> void *ctx_quote = talloc_new (thread);\r
+> const char *authors = notmuch_thread_get_authors (thread);\r
+> const char *subject = notmuch_thread_get_subject (thread);\r
+> @@ -108,7 +160,7 @@ do_search_threads (sprinter_t *format,\r
+> relative_date = notmuch_time_relative_date (ctx_quote, date);\r
+> \r
+> if (format->is_text_printer) {\r
+> - /* Special case for the text formatter */\r
+> + /* Special case for the text formatter */\r
+\r
+Unintentional whitespace change? (Actually, this line isn't indented\r
+with tabs either before or after this change; how'd that happen?)\r
+\r
+> printf ("thread:%s %12s [%d/%d] %s; %s (",\r
+> thread_id,\r
+> relative_date,\r
+> @@ -133,8 +185,6 @@ do_search_threads (sprinter_t *format,\r
+> format->string (format, subject);\r
+> }\r
+> \r
+> - talloc_free (ctx_quote);\r
+> -\r
+> format->map_key (format, "tags");\r
+> format->begin_list (format);\r
+> \r
+> @@ -145,7 +195,7 @@ do_search_threads (sprinter_t *format,\r
+> const char *tag = notmuch_tags_get (tags);\r
+> \r
+> if (format->is_text_printer) {\r
+> - /* Special case for the text formatter */\r
+> + /* Special case for the text formatter */\r
+\r
+Same? Looks like here you converted it to tabs.\r
+\r
+> if (first_tag)\r
+> first_tag = FALSE;\r
+> else\r
+> @@ -160,8 +210,25 @@ do_search_threads (sprinter_t *format,\r
+> printf (")");\r
+> \r
+> format->end (format);\r
+> +\r
+> + if (output == OUTPUT_SUMMARY_WITH_QUERIES) {\r
+> + char *query;\r
+> + query = output_msg_query (ctx_quote, format, TRUE, TRUE, notmuch_thread_get_toplevel_messages (thread));\r
+> + if (strlen (query)) {\r
+> + format->map_key (format, "matching_msg_query");\r
+\r
+Maybe just matching_query?\r
+\r
+> + format->string (format, query);\r
+> + }\r
+> + query = output_msg_query (ctx_quote, format, FALSE, TRUE, notmuch_thread_get_toplevel_messages (thread));\r
+> + if (strlen (query)) {\r
+> + format->map_key (format, "nonmatching_msg_query");\r
+\r
+nonmatching_query?\r
+\r
+Also, don't forget to update devel/schema.\r
+\r
+> + format->string (format, query);\r
+> + }\r
+> + }\r
+> +\r
+> format->end (format);\r
+> format->separator (format);\r
+> +\r
+> + talloc_free (ctx_quote);\r
+> }\r
+> \r
+> notmuch_thread_destroy (thread);\r
+> @@ -303,6 +370,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
+> int offset = 0;\r
+> int limit = -1; /* unlimited */\r
+> int exclude = EXCLUDE_TRUE;\r
+> + notmuch_bool_t with_queries = FALSE;\r
+> unsigned int i;\r
+> \r
+> enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }\r
+> @@ -323,12 +391,14 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
+> { "messages", OUTPUT_MESSAGES },\r
+> { "files", OUTPUT_FILES },\r
+> { "tags", OUTPUT_TAGS },\r
+> + { "with-queries", OUTPUT_SUMMARY_WITH_QUERIES },\r
+\r
+Was this intentional?\r
+\r
+> { 0, 0 } } },\r
+> { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',\r
+> (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },\r
+> { "false", EXCLUDE_FALSE },\r
+> { "flag", EXCLUDE_FLAG },\r
+> { 0, 0 } } },\r
+> + { NOTMUCH_OPT_BOOLEAN, &with_queries, "queries", 'b', 0 },\r
+\r
+Wrong indentation (since the next line is indented correctly you can\r
+be both locally and globally consistent).\r
+\r
+> { NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },\r
+> { NOTMUCH_OPT_INT, &limit, "limit", 'L', 0 },\r
+> { 0, 0, 0, 0, 0 }\r
+> @@ -398,6 +468,19 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
+> notmuch_query_set_omit_excluded (query, FALSE);\r
+> }\r
+> \r
+> + if (with_queries) {\r
+> + if (format_sel == NOTMUCH_FORMAT_TEXT) {\r
+> + fprintf (stderr, "Warning: --queries=true not implemented for text format.\n");\r
+> + with_queries = FALSE;\r
+> + }\r
+> + if (output != OUTPUT_SUMMARY) {\r
+> + fprintf (stderr, "Warning: --queries=true only implemented for --output=summary.\n");\r
+> + with_queries = FALSE;\r
+> + }\r
+> + }\r
+> +\r
+> + if (with_queries) output = OUTPUT_SUMMARY_WITH_QUERIES;\r
+\r
+Out of curiosity, why do this as a separate output type, rather than\r
+just passing a with_queries flag into do_search_threads?\r
+\r
+> +\r
+> switch (output) {\r
+> default:\r
+> case OUTPUT_SUMMARY:\r
projects / notmuch-archives.git / commitdiff