98/751f4487ae63899aefa12f1229554871b0dcc2

   1 Return-Path: <david@tethera.net>\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 EB318431FB6\r
   6         for <notmuch@notmuchmail.org>; Mon, 29 Apr 2013 18:12:32 -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\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         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 5c9T9YLBPAOG for <notmuch@notmuchmail.org>;\r
  16         Mon, 29 Apr 2013 18:12:28 -0700 (PDT)\r
  17 Received: from tesseract.cs.unb.ca (tesseract.cs.unb.ca [131.202.240.238])\r
  18         (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 5C71D431FAF\r
  21         for <notmuch@notmuchmail.org>; Mon, 29 Apr 2013 18:12:28 -0700 (PDT)\r
  22 Received: from fctnnbsc30w-156034082078.dhcp-dynamic.fibreop.nb.bellaliant.net\r
  23         ([156.34.82.78] helo=zancas.localnet)\r
  24         by tesseract.cs.unb.ca with esmtpsa\r
  25         (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80)\r
  26         (envelope-from <david@tethera.net>)\r
  27         id 1UWz7G-0004a6-VW; Mon, 29 Apr 2013 22:12:25 -0300\r
  28 Received: from bremner by zancas.localnet with local (Exim 4.80)\r
  29         (envelope-from <david@tethera.net>)\r
  30         id 1UWz7A-0003yr-SL; Mon, 29 Apr 2013 22:12:16 -0300\r
  31 From: David Bremner <david@tethera.net>\r
  32 To: "Alexey I. Froloff" <raorn@raorn.name>, notmuch@notmuchmail.org\r
  33 Subject: Re: [PATCH] lib: Add a new prefix "list" to the search-terms syntax\r
  34 In-Reply-To: <1365549369-12776-1-git-send-email-raorn@raorn.name>\r
  35 References: <20130409083010.GA27675@raorn.name>\r
  36         <1365549369-12776-1-git-send-email-raorn@raorn.name>\r
  37 User-Agent: Notmuch/0.15.2+84~g12d5e4e (http://notmuchmail.org) Emacs/24.2.1\r
  38         (x86_64-pc-linux-gnu)\r
  39 Date: Mon, 29 Apr 2013 22:12:16 -0300\r
  40 Message-ID: <87vc74vn2n.fsf@zancas.localnet>\r
  41 MIME-Version: 1.0\r
  42 Content-Type: text/plain\r
  43 X-Spam_bar: -\r
  44 X-BeenThere: notmuch@notmuchmail.org\r
  45 X-Mailman-Version: 2.1.13\r
  46 Precedence: list\r
  47 List-Id: "Use and development of the notmuch mail system."\r
  48         <notmuch.notmuchmail.org>\r
  49 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  50         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  51 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  52 List-Post: <mailto:notmuch@notmuchmail.org>\r
  53 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  54 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  55         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  56 X-List-Received-Date: Tue, 30 Apr 2013 01:12:33 -0000\r
  57 \r
  58 \r
  59 Hi Alexey, \r
  60 \r
  61 Thanks for working on this. I think the boolean prefix version makes\r
  62 more sense, and it seems to work OK. I have a few comments below\r
  63 \r
  64 "Alexey I. Froloff" <raorn@raorn.name> writes:\r
  65 \r
  66 > +    begin_list_id = strrchr (list_id_header, '<');\r
  67 > +    if (!begin_list_id) {\r
  68 > +     fprintf (stderr, "Warning: Not indexing mailformed List-Id tag.\n");\r
  69 > +     return;\r
  70 > +    }\r
  71 \r
  72 - I guess this should say "malformed". \r
  73 \r
  74 - I got about 1800 lines of such messages when indexing 280k\r
  75   messages. That might strike some people as excessive. On the otherhand\r
  76   I guess we need to re-think error reporting overall.\r
  77 \r
  78   What do you think about printing filename or message-id here its\r
  79   easier to double check that it is not a bug?\r
  80 \r
  81 > +    end_list_id = strrchr(begin_list_id, '>');\r
  82 > +    if (!end_list_id || (end_list_id - begin_list_id < 2)) {\r
  83 > +     fprintf (stderr, "Warning: Not indexing mailformed List-Id tag.\n");\r
  84 > +     return;\r
  85 > +    }\r
  86 > +\r
  87 \r
  88 Same comments here.\r
  89 \r
  90 > +    void *local = talloc_new (message);\r
  91 > +\r
  92 > +    /* We extract the list id between the angle brackets */\r
  93 > +    const char *list_id = talloc_strndup (local, begin_list_id + 1,\r
  94 > +                                       end_list_id - begin_list_id - 1);\r
  95 > +\r
  96       we should handle ENOMEM here, I think.\r
  97 \r
  98 > +    /* _notmuch_message_add_term() may return\r
  99 > +     * NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG here.  We can't fix it, but\r
 100 > +     * this is not a reason to exit with error... */\r
 101 > +    if (_notmuch_message_add_term (message, "list", list_id))\r
 102 > +     fprintf (stderr, "Warning: Not indexing List-Id: <%s>\n", list_id);\r
 103 \r
 104 This should say why the indexing failed.\r
 105 \r
 106 Other than that:\r
 107 \r
 108 - We need a couple tests for this code; tests/search should give some\r
 109   hints how to proceed.\r
 110 \r
 111 - We need a patch for NEWS, explaining what people need to do take\r
 112   advantage of the new functionality.  I think that adding new prefixes\r
 113   to an existing database is OK, but I'd welcome confirmation.\r
 114 \r
 115 BTW, my not too scientific tests show no detectable bloat in the\r
 116 database, at least after running xapian-compact. I'd be curious what\r
 117 other people report.\r
 118 \r