a4/8857eac6ebfea2007dc38b73848f29813135d8

   1 Return-Path: <aaronecay@gmail.com>\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 2FD0D431FAF\r
   6         for <notmuch@notmuchmail.org>; Sun,  3 Mar 2013 15:46:22 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 1.7\r
  10 X-Spam-Level: *\r
  11 X-Spam-Status: No, score=1.7 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
  13         FREEMAIL_FROM=0.001, FREEMAIL_REPLY=2.499, RCVD_IN_DNSWL_LOW=-0.7]\r
  14         autolearn=disabled\r
  15 Received: from olra.theworths.org ([127.0.0.1])\r
  16         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  17         with ESMTP id itfX0F7TynOB for <notmuch@notmuchmail.org>;\r
  18         Sun,  3 Mar 2013 15:46:21 -0800 (PST)\r
  19 Received: from mail-qe0-f54.google.com (mail-qe0-f54.google.com\r
  20         [209.85.128.54]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  21         (No client certificate requested)\r
  22         by olra.theworths.org (Postfix) with ESMTPS id 4CCB0431FAE\r
  23         for <notmuch@notmuchmail.org>; Sun,  3 Mar 2013 15:46:21 -0800 (PST)\r
  24 Received: by mail-qe0-f54.google.com with SMTP id i11so2674234qej.27\r
  25         for <notmuch@notmuchmail.org>; Sun, 03 Mar 2013 15:46:20 -0800 (PST)\r
  26 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\r
  27         h=x-received:from:to:cc:subject:in-reply-to:references:user-agent\r
  28         :date:message-id:mime-version:content-type:content-transfer-encoding;\r
  29         bh=XeVIO0io3ByeZxaPSIdKKwTeTcZKfzZwQRrpoPdQ2uA=;\r
  30         b=xr6wNjvgKUQL0IGZBCoLzpXgfkFys+fmchU0uI75Y75Zt1X4uDNw7PT8VN4XU+tIv+\r
  31         i8skIPlntYNVTxO9eRDySeA5ZR+YeJhHsE485pczbndalRt6hTt4IdiGAWYGj9l20HLy\r
  32         9NbqS4ipgMlWcPCXLbuFpi4+SGMG9aDJhkcZroMg64sP0Sg/6Ad99DovFKiS9Er8KL98\r
  33         7u5wzYIoRjK08Nsm52iA1s3kf3YmH5NGPOEVSlYlMpwbe/NeP8DcogFWwweCCFMAOUL7\r
  34         8qmTPmVI8ymg6ku6TlQmLzjcqryKZEwskQpD04BQJgK9RhyI8Z2kRMJBt5S+JtnKncc5\r
  35         qiYA==\r
  36 X-Received: by 10.229.175.25 with SMTP id v25mr6301859qcz.36.1362354379700;\r
  37         Sun, 03 Mar 2013 15:46:19 -0800 (PST)\r
  38 Received: from localhost (c-68-80-94-73.hsd1.pa.comcast.net. [68.80.94.73])\r
  39         by mx.google.com with ESMTPS id g6sm34092051qav.6.2013.03.03.15.46.18\r
  40         (version=TLSv1.2 cipher=RC4-SHA bits=128/128);\r
  41         Sun, 03 Mar 2013 15:46:19 -0800 (PST)\r
  42 From: Aaron Ecay <aaronecay@gmail.com>\r
  43 To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org\r
  44 Subject: Re: [RFC] [PATCH] lib/database.cc: change how the parent of a message\r
  45         is calculated\r
  46 In-Reply-To: <871ubzt5gr.fsf@nikula.org>\r
  47 References: <1361836225-17279-1-git-send-email-aaronecay@gmail.com>\r
  48         <87621cteeb.fsf@nikula.org> <871ubzt5gr.fsf@nikula.org>\r
  49 User-Agent: Notmuch/0.15.2+33~g0c0a530 (http://notmuchmail.org)\r
  50         Emacs/24.3.50.2 (x86_64-unknown-linux-gnu)\r
  51 Date: Sun, 03 Mar 2013 18:46:18 -0500\r
  52 Message-ID: <87wqtovygl.fsf@gmail.com>\r
  53 MIME-Version: 1.0\r
  54 Content-Type: text/plain; charset=utf-8\r
  55 Content-Transfer-Encoding: quoted-printable\r
  56 X-BeenThere: notmuch@notmuchmail.org\r
  57 X-Mailman-Version: 2.1.13\r
  58 Precedence: list\r
  59 List-Id: "Use and development of the notmuch mail system."\r
  60         <notmuch.notmuchmail.org>\r
  61 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  63 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  64 List-Post: <mailto:notmuch@notmuchmail.org>\r
  65 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  66 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  67         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  68 X-List-Received-Date: Sun, 03 Mar 2013 23:46:22 -0000\r
  69 \r
  70 Hi Jani,\r
  71 \r
  72 Thanks to you and Austin for the comments.\r
  73 \r
  74 2013ko martxoak 1an, Jani Nikula-ek idatzi zuen:\r
  75 >> I think the background is that RFC 822 defines In-Reply-To (and\r
  76 >> References too for that matter) as *(phrase / msg-id), while RFC 2822\r
  77 >> defines them as 1*msg-id. I'd like something about RFC 822 being\r
  78 >> mentioned in the commit message.\r
  79 >>=20\r
  80 >> The problem in the gmane message you link to in\r
  81 >> id:87liaa3luc.fsf@gmail.com is likely related to the FAQ item 05.26\r
  82 >> "How do I fix a bogus In-Reply-To or missing References field?" in\r
  83 >> the MH FAQ http://www.newt.com/faq/mh.html.\r
  84 \r
  85 Likely yes.  But I think notmuch should handle these messages, since\r
  86 they are seen in the wild (and I don=E2=80=99t think you disagree with me on\r
  87 this point?)\r
  88 \r
  89 \r
  90 >>=20\r
  91 >> As the comment for the function says, we explicitly avoid including\r
  92 >> self-references. I think I'd err on the safe side and return NULL if\r
  93 >> the last ref equals message-id.\r
  94 \r
  95 Done.\r
  96 \r
  97 >>=20\r
  98 >> I don't know how you got this non-change hunk here, but please remove\r
  99 >> it. :)\r
 100 \r
 101 That=E2=80=99s what I get for setting my editor to delete trailing whitespa=\r
 102 ce on\r
 103 save (then not reading outgoing patches carefully).  Fixed.\r
 104 \r
 105 >> I wonder if you should reuse your parse_references() change here, so\r
 106 >> you'd set in_reply_to_message_id to the last message-id in\r
 107 >> In-Reply-To. This might tackle some of the problematic cases\r
 108 >> directly, but should still be all right per RFC 2822. I didn't verify\r
 109 >> how the parser handles an RFC 2822 violating free form header though.\r
 110 >=20\r
 111 > Strike that based on http://www.jwz.org/doc/threading.html:\r
 112 >=20\r
 113 > "If there are multiple things in In-Reply-To that look like\r
 114 > Message-IDs, only use the first one of them: odds are that the later\r
 115 > ones are actually email addresses, not IDs."\r
 116 \r
 117 Hmm.  I think it=E2=80=99s a toss-up which of multiple quasi-message-ids is=\r
 118  the\r
 119 real one.  In the email message example I linked upthread, it was the\r
 120 last one that was real.  I decided to use the last one, because it\r
 121 allows the self-reference checking to be pushed entirely into\r
 122 parse_references.  If you feel strongly that we should use the first\r
 123 one, I can change it back.\r
 124 \r
 125 > I talked to Austin (CC) about the patch on IRC, and his comment was,\r
 126 > perceptive as always:\r
 127 >=20\r
 128 >  23:38 amdragon Is the logic in that patch equivalent to always using\r
 129 > the last message ID in references unless there is no references\r
 130 > header?  Seems like it is, but in a convoluted way.\r
 131 >=20\r
 132 > And that's actually the case, isn't it? To make the code reflect that,\r
 133 > you should use last_ref_message_id, and if that's NULL, fallback to\r
 134 > in_reply_to_message_id.\r
 135 \r
 136 Yes.  Fixed.\r
 137 \r
 138 >=20\r
 139 >> I suggest adding an else if branch (or revamp the above if condition)\r
 140 >> to tackle the missing In-Reply-To header:\r
 141 >>=20\r
 142 >> else if (!in_reply_to_message_id && last_ref_message_id) {\r
 143 >> in_reply_to_message_id =3D last_ref_message_id; }\r
 144 >=20\r
 145 > Strike that, it should be the other way round.\r
 146 \r
 147 Now that the self-reference check is in parse_references, the\r
 148 conditional is much simpler.\r
 149 \r
 150 One additional change I made in this version was to factor out 3 calls\r
 151 to =E2=80=9Cnotmuch_message_get_message_id (message)=E2=80=9D into a variab=\r
 152 le inside the\r
 153 _notmuch_database_link_message_to_parents function, for a small boost to\r
 154 readability (and perhaps speed, depending on how clever the compiler is\r
 155 I guess).\r
 156 \r
 157 I also added tests =E2=80=93 those are the first of two patches that will f=\r
 158 ollow\r
 159 this email, the second being the code to make them pass.\r
 160 \r
 161 --=20\r
 162 Aaron Ecay\r