12/6630607a343272bac81e3bbc6e4d865314ba6e

   1 Return-Path: <jani@nikula.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 343EC431FAF\r
   6         for <notmuch@notmuchmail.org>; Wed,  9 Oct 2013 00:41:37 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id 2v91nAVV-QQ1 for <notmuch@notmuchmail.org>;\r
  16         Wed,  9 Oct 2013 00:41:31 -0700 (PDT)\r
  17 Received: from mail-ea0-f182.google.com (mail-ea0-f182.google.com\r
  18         [209.85.215.182]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id D12D0431FAE\r
  21         for <notmuch@notmuchmail.org>; Wed,  9 Oct 2013 00:41:30 -0700 (PDT)\r
  22 Received: by mail-ea0-f182.google.com with SMTP id o10so182340eaj.41\r
  23         for <notmuch@notmuchmail.org>; Wed, 09 Oct 2013 00:41:29 -0700 (PDT)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=1e100.net; s=20130820;\r
  26         h=x-gm-message-state:from:to:subject:in-reply-to:references\r
  27         :user-agent:date:message-id:mime-version:content-type\r
  28         :content-transfer-encoding;\r
  29         bh=VryUUpM0hLEW2dkWppzpGgaza/4RZEWLuf3PF7f5Ltg=;\r
  30         b=TNWI5zx1JVofAU3UUN+k1Q1N3aX26lsXFjZXSf38qZL6IioPMBEDsaZkoOsBQd56aF\r
  31         Z3wbv8BE9uswAWi3fOzKAaxxJpcuAabqOZC+qULkFTRAIDrx+fKGiz6j7tWxpM3jgJvs\r
  32         JP/QapekHkr3Mmt/W4pJO1l6wnBe3RQ+Eesp39dTg95JC5zMtgKX4ouIxx2Si7ayH7JZ\r
  33         cjiIYuZCpic0DRZnnBpLPV43bZDiAM1kyI1GCleUp14M77B7nbaOQoqI5UaJddQrJy61\r
  34         vQ3BkRP3JBiBOf6QxDxRSBXQNQyhH2128WGed25/+upCnpzIDuTjHWYHgOCUrwhuDfBV\r
  35         V3wg==\r
  36 X-Gm-Message-State:\r
  37  ALoCoQkylFmUfSS6zL6y7kE4HBK2jpFMPRUNmSyLvVvixKEEvYNDS/q2iy/wAHfAvrcI3gYSvnqS\r
  38 X-Received: by 10.14.199.200 with SMTP id x48mr1048932een.65.1381304489705;\r
  39         Wed, 09 Oct 2013 00:41:29 -0700 (PDT)\r
  40 Received: from localhost ([2001:4b98:dc0:43:216:3eff:fe1b:25f3])\r
  41         by mx.google.com with ESMTPSA id\r
  42         x47sm85546562eea.16.1969.12.31.16.00.00\r
  43         (version=TLSv1.1 cipher=RC4-SHA bits=128/128);\r
  44         Wed, 09 Oct 2013 00:41:28 -0700 (PDT)\r
  45 From: Jani Nikula <jani@nikula.org>\r
  46 To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
  47 Subject: Re: [PATCH 08/11] search: Add stable queries to thread search results\r
  48 In-Reply-To: <1381185201-25197-9-git-send-email-amdragon@mit.edu>\r
  49 References: <1381185201-25197-1-git-send-email-amdragon@mit.edu>\r
  50         <1381185201-25197-9-git-send-email-amdragon@mit.edu>\r
  51 User-Agent: Notmuch/0.16+62~g9f2ae2e (http://notmuchmail.org) Emacs/23.2.1\r
  52         (x86_64-pc-linux-gnu)\r
  53 Date: Wed, 09 Oct 2013 09:41:17 +0200\r
  54 Message-ID: <87fvsaao2q.fsf@nikula.org>\r
  55 MIME-Version: 1.0\r
  56 Content-Type: text/plain; charset=utf-8\r
  57 Content-Transfer-Encoding: quoted-printable\r
  58 X-BeenThere: notmuch@notmuchmail.org\r
  59 X-Mailman-Version: 2.1.13\r
  60 Precedence: list\r
  61 List-Id: "Use and development of the notmuch mail system."\r
  62         <notmuch.notmuchmail.org>\r
  63 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  64         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  65 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  66 List-Post: <mailto:notmuch@notmuchmail.org>\r
  67 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  68 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  69         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  70 X-List-Received-Date: Wed, 09 Oct 2013 07:41:37 -0000\r
  71 \r
  72 On Tue, 08 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:\r
  73 > These queries will match exactly the set of messages currently in the\r
  74 > thread, even if more messages later arrive.  Two queries are provided:\r
  75 > one for matched messages and one for unmatched messages.\r
  76 >\r
  77 > This can be used to fix race conditions with tagging threads from\r
  78 > search results.  While tagging based on a thread: query can affect\r
  79 > messages that arrived after the search, tagging based on stable\r
  80 > queries affects only the messages the user was shown in the search UI.\r
  81 >\r
  82 > Since we want clients to be able to depend on the presence of these\r
  83 > queries, this ushers in schema version 2.\r
  84 > ---\r
  85 >  devel/schemata       | 22 +++++++++++++++++--\r
  86 >  notmuch-client.h     |  2 +-\r
  87 >  notmuch-search.c     | 60 ++++++++++++++++++++++++++++++++++++++++++++++=\r
  88 ++++++\r
  89 >  test/json            |  2 ++\r
  90 >  test/missing-headers |  6 ++++--\r
  91 >  test/sexp            |  4 ++--\r
  92 >  6 files changed, 89 insertions(+), 7 deletions(-)\r
  93 >\r
  94 > diff --git a/devel/schemata b/devel/schemata\r
  95 > index cdd0e43..41dc4a6 100644\r
  96 > --- a/devel/schemata\r
  97 > +++ b/devel/schemata\r
  98 > @@ -14,7 +14,17 @@ are interleaved. Keys are printed as keywords (symbols=\r
  99  preceded by a\r
 100 >  colon), e.g. (:id "123" :time 54321 :from "foobar"). Null is printed as\r
 101 >  nil, true as t and false as nil.\r
 102 >=20=20\r
 103 > -This is version 1 of the structured output format.\r
 104 > +This is version 2 of the structured output format.\r
 105 > +\r
 106 > +Version history\r
 107 > +---------------\r
 108 > +\r
 109 > +v1\r
 110 > +- First versioned schema release.\r
 111 > +- Added part.content-length and part.content-transfer-encoding fields.\r
 112 > +\r
 113 > +v2\r
 114 > +- Added the thread_summary.query field.\r
 115 >=20=20\r
 116 >  Common non-terminals\r
 117 >  --------------------\r
 118 > @@ -145,7 +155,15 @@ thread_summary =3D {\r
 119 >      authors:        string,   # comma-separated names with | between\r
 120 >                                # matched and unmatched\r
 121 >      subject:        string,\r
 122 > -    tags:           [string*]\r
 123 > +    tags:           [string*],\r
 124 > +\r
 125 > +    # Two stable query strings identifying exactly the matched and\r
 126 > +    # unmatched messages currently in this thread.  The messages\r
 127 > +    # matched by these queries will not change even if more messages\r
 128 > +    # arrive in the thread.  If there are no matched or unmatched\r
 129 > +    # messages, the corresponding query will be null (there is no\r
 130 > +    # query that matches nothing).  (Added in schema version 2.)\r
 131 > +    query:          [string|null, string|null],\r
 132 >  }\r
 133 >=20=20\r
 134 >  notmuch reply schema\r
 135 > diff --git a/notmuch-client.h b/notmuch-client.h\r
 136 > index 8d986f4..1b14910 100644\r
 137 > --- a/notmuch-client.h\r
 138 > +++ b/notmuch-client.h\r
 139 > @@ -138,7 +138,7 @@ chomp_newline (char *str)\r
 140 >   * this.  New (required) map fields can be added without increasing\r
 141 >   * this.\r
 142 >   */\r
 143 > -#define NOTMUCH_FORMAT_CUR 1\r
 144 > +#define NOTMUCH_FORMAT_CUR 2\r
 145 >  /* The minimum supported structured output format version.  Requests\r
 146 >   * for format versions below this will return an error. */\r
 147 >  #define NOTMUCH_FORMAT_MIN 1\r
 148 > diff --git a/notmuch-search.c b/notmuch-search.c\r
 149 > index d9d39ec..1d14651 100644\r
 150 > --- a/notmuch-search.c\r
 151 > +++ b/notmuch-search.c\r
 152 > @@ -20,6 +20,7 @@\r
 153 >=20=20\r
 154 >  #include "notmuch-client.h"\r
 155 >  #include "sprinter.h"\r
 156 > +#include "string-util.h"\r
 157 >=20=20\r
 158 >  typedef enum {\r
 159 >      OUTPUT_SUMMARY,\r
 160 > @@ -46,6 +47,46 @@ sanitize_string (const void *ctx, const char *str)\r
 161 >      return out;\r
 162 >  }\r
 163 >=20=20\r
 164 > +/* Return two stable query strings that identify exactly the matched\r
 165 > + * and unmatched messages currently in thread.  If there are no\r
 166 > + * matched or unmatched messages, the returned buffers will be\r
 167 > + * NULL. */\r
 168 > +static int\r
 169 > +get_thread_query (notmuch_thread_t *thread,\r
 170 > +               char **matched_out, char **unmached_out)\r
 171 > +{\r
 172 > +    notmuch_messages_t *messages;\r
 173 > +    char *escaped =3D NULL;\r
 174 > +    size_t escaped_len =3D 0;\r
 175 > +\r
 176 > +    *matched_out =3D *unmached_out =3D NULL;\r
 177 > +\r
 178 > +    for (messages =3D notmuch_thread_get_messages (thread);\r
 179 > +      notmuch_messages_valid (messages);\r
 180 > +      notmuch_messages_move_to_next (messages))\r
 181 > +    {\r
 182 > +     notmuch_message_t *message =3D notmuch_messages_get (messages);\r
 183 > +     const char *mid =3D notmuch_message_get_message_id (message);\r
 184 > +     /* Determine which query buffer to extend */\r
 185 > +     char **buf =3D notmuch_message_get_flag (\r
 186 > +         message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmached_out;\r
 187 > +     /* Allocate the query buffer is this is the first message */\r
 188 > +     if (!*buf && (*buf =3D talloc_strdup (thread, "")) =3D=3D NULL)\r
 189 > +         return -1;\r
 190 \r
 191 I think it would improve clarity if you dropped the above...\r
 192 \r
 193 > +     /* Add this message's id: query.  Since "id" is an exclusive\r
 194 > +      * prefix, it is implicitly 'or'd together, so we only need to\r
 195 > +      * join queries with a space. */\r
 196 > +     if (make_boolean_term (thread, "id", mid, &escaped, &escaped_len) < 0)\r
 197 > +         return -1;\r
 198 > +     *buf =3D talloc_asprintf_append_buffer (\r
 199 > +         *buf, "%s%s", **buf ? " " : "", escaped);\r
 200 \r
 201 ...and turned this into:\r
 202 \r
 203         if (*buf)\r
 204             *buf =3D talloc_asprintf_append_buffer (*buf, " %s", escaped);\r
 205         else\r
 206             *buf =3D talloc_strdup (thread, escaped);\r
 207 \r
 208 Also one talloc less. Which brings me to the main worry:\r
 209 performance. What's the impact?\r
 210 \r
 211 BR,\r
 212 Jani.\r
 213 \r
 214 \r
 215 > +     if (!*buf)\r
 216 > +         return -1;\r
 217 > +    }\r
 218 > +    talloc_free (escaped);\r
 219 > +    return 0;\r
 220 > +}\r
 221 > +\r
 222 >  static int\r
 223 >  do_search_threads (sprinter_t *format,\r
 224 >                  notmuch_query_t *query,\r
 225 > @@ -131,6 +172,25 @@ do_search_threads (sprinter_t *format,\r
 226 >               format->string (format, authors);\r
 227 >               format->map_key (format, "subject");\r
 228 >               format->string (format, subject);\r
 229 > +             if (notmuch_format_version >=3D 2) {\r
 230 > +                 char *matched_query, *unmatched_query;\r
 231 > +                 if (get_thread_query (thread, &matched_query,\r
 232 > +                                       &unmatched_query) < 0) {\r
 233 > +                     fprintf (stderr, "Out of memory\n");\r
 234 > +                     return 1;\r
 235 > +                 }\r
 236 > +                 format->map_key (format, "query");\r
 237 > +                 format->begin_list (format);\r
 238 > +                 if (matched_query)\r
 239 > +                     format->string (format, matched_query);\r
 240 > +                 else\r
 241 > +                     format->null (format);\r
 242 > +                 if (unmatched_query)\r
 243 > +                     format->string (format, unmatched_query);\r
 244 > +                 else\r
 245 > +                     format->null (format);\r
 246 > +                 format->end (format);\r
 247 > +             }\r
 248 >           }\r
 249 >=20=20\r
 250 >           talloc_free (ctx_quote);\r
 251 > diff --git a/test/json b/test/json\r
 252 > index b87b7f6..e07a290 100755\r
 253 > --- a/test/json\r
 254 > +++ b/test/json\r
 255 > @@ -26,6 +26,7 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",\r
 256 >   \"total\": 1,\r
 257 >   \"authors\": \"Notmuch Test Suite\",\r
 258 >   \"subject\": \"json-search-subject\",\r
 259 > + \"query\": [\"id:$gen_msg_id\", null],\r
 260 >   \"tags\": [\"inbox\",\r
 261 >   \"unread\"]}]"\r
 262 >=20=20\r
 263 > @@ -59,6 +60,7 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",\r
 264 >   \"total\": 1,\r
 265 >   \"authors\": \"Notmuch Test Suite\",\r
 266 >   \"subject\": \"json-search-utf8-body-s=C3=BCbj=C3=A9ct\",\r
 267 > + \"query\": [\"id:$gen_msg_id\", null],\r
 268 >   \"tags\": [\"inbox\",\r
 269 >   \"unread\"]}]"\r
 270 >=20=20\r
 271 > diff --git a/test/missing-headers b/test/missing-headers\r
 272 > index f14b878..43e861b 100755\r
 273 > --- a/test/missing-headers\r
 274 > +++ b/test/missing-headers\r
 275 > @@ -43,7 +43,8 @@ test_expect_equal_json "$output" '\r
 276 >          ],\r
 277 >          "thread": "XXX",\r
 278 >          "timestamp": 978709437,\r
 279 > -        "total": 1\r
 280 > +        "total": 1,\r
 281 > +        "query": ["id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01=\r
 282 161", null]\r
 283 >      },\r
 284 >      {\r
 285 >          "authors": "Notmuch Test Suite",\r
 286 > @@ -56,7 +57,8 @@ test_expect_equal_json "$output" '\r
 287 >          ],\r
 288 >          "thread": "XXX",\r
 289 >          "timestamp": 0,\r
 290 > -        "total": 1\r
 291 > +        "total": 1,\r
 292 > +        "query": ["id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401=\r
 293 334", null]\r
 294 >      }\r
 295 >  ]'\r
 296 >=20=20\r
 297 > diff --git a/test/sexp b/test/sexp\r
 298 > index 492a82f..be815e1 100755\r
 299 > --- a/test/sexp\r
 300 > +++ b/test/sexp\r
 301 > @@ -19,7 +19,7 @@ test_expect_equal "$output" "((((:id \"${gen_msg_id}\" =\r
 302 :match t :excluded nil :f\r
 303 >  test_begin_subtest "Search message: sexp"\r
 304 >  add_message "[subject]=3D\"sexp-search-subject\"" "[date]=3D\"Sat, 01 Ja=\r
 305 n 2000 12:00:00 -0000\"" "[body]=3D\"sexp-search-message\""\r
 306 >  output=3D$(notmuch search --format=3Dsexp "sexp-search-message" | notmuc=\r
 307 h_search_sanitize)\r
 308 > -test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 9=\r
 309 46728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmu=\r
 310 ch Test Suite\" :subject \"sexp-search-subject\" :tags (\"inbox\" \"unread\=\r
 311 ")))"\r
 312 > +test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 9=\r
 313 46728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmu=\r
 314 ch Test Suite\" :subject \"sexp-search-subject\" :query (\"id:$gen_msg_id\"=\r
 315  nil) :tags (\"inbox\" \"unread\")))"\r
 316 >=20=20\r
 317 >  test_begin_subtest "Show message: sexp, utf-8"\r
 318 >  add_message "[subject]=3D\"sexp-show-utf8-body-s=C3=BCbj=C3=A9ct\"" "[da=\r
 319 te]=3D\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=3D\"js=C3=B6n-show-m=C3=\r
 320 =A9ssage\""\r
 321 > @@ -44,7 +44,7 @@ test_expect_equal "$output" "((((:id \"$id\" :match t :=\r
 322 excluded nil :filename \"\r
 323 >  test_begin_subtest "Search message: sexp, utf-8"\r
 324 >  add_message "[subject]=3D\"sexp-search-utf8-body-s=C3=BCbj=C3=A9ct\"" "[=\r
 325 date]=3D\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=3D\"js=C3=B6n-search-m=\r
 326 =C3=A9ssage\""\r
 327 >  output=3D$(notmuch search --format=3Dsexp "js=C3=B6n-search-m=C3=A9ssage=\r
 328 " | notmuch_search_sanitize)\r
 329 > -test_expect_equal "$output" "((:thread \"0000000000000005\" :timestamp 9=\r
 330 46728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmu=\r
 331 ch Test Suite\" :subject \"sexp-search-utf8-body-s=C3=BCbj=C3=A9ct\" :tags =\r
 332 (\"inbox\" \"unread\")))"\r
 333 > +test_expect_equal "$output" "((:thread \"0000000000000005\" :timestamp 9=\r
 334 46728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmu=\r
 335 ch Test Suite\" :subject \"sexp-search-utf8-body-s=C3=BCbj=C3=A9ct\" :query=\r
 336  (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"\r
 337 >=20=20\r
 338 >=20=20\r
 339 >  test_done\r
 340 > --=20\r
 341 > 1.8.4.rc3\r
 342 >\r
 343 > _______________________________________________\r
 344 > notmuch mailing list\r
 345 > notmuch@notmuchmail.org\r
 346 > http://notmuchmail.org/mailman/listinfo/notmuch\r