From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id A26CB6DE0318 for ; Sat, 1 Sep 2018 13:30:07 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0.47 X-Spam-Level: X-Spam-Status: No, score=0.47 tagged_above=-999 required=5 tests=[AWL=-0.182, SPF_NEUTRAL=0.652] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2S9pzCK9QLCf for ; Sat, 1 Sep 2018 13:30:07 -0700 (PDT) Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) by arlo.cworth.org (Postfix) with ESMTP id AFE676DE02CA for ; Sat, 1 Sep 2018 13:30:06 -0700 (PDT) Received: from guru.guru-group.fi (localhost [IPv6:::1]) by guru.guru-group.fi (Postfix) with ESMTP id 0CB0E1000AF; Sat, 1 Sep 2018 23:29:59 +0300 (EEST) From: Tomi Ollila To: David Bremner , notmuch@notmuchmail.org Subject: Re: [PATCH 15/15] lib: change parent strategy to use In-Reply-To if it looks sane In-Reply-To: <20180830112915.11761-16-david@tethera.net> References: <20180830112915.11761-1-david@tethera.net> <20180830112915.11761-16-david@tethera.net> User-Agent: Notmuch/0.27+14~ge594ae3 (https://notmuchmail.org) Emacs/25.2.1 (x86_64-unknown-linux-gnu) X-Face: HhBM'cA~ MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Sep 2018 20:30:07 -0000 This series looks pretty good to me (comments sent separately), meaning that I did not see any obvious problems there. I did not run the tests (at least yet). Tomi On Thu, Aug 30 2018, David Bremner wrote: > As reported by Sean Whitton, there are mailers (in particular the > Debian Bug Tracking System) that have sensible In-Reply-To headers, > but un-useful-for-notmuch References (in particular with the BTS, the > oldest reference is last). I looked at a sample of about 200K > messages, and only about 0.5% these had something other than a single > message-id in In-Reply-To. On this basis, if we see a single > message-id in In-Reply-To, consider that as authoritative. > --- > lib/add-message.cc | 20 +++++++++++++++----- > test/T510-thread-replies.sh | 1 - > 2 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/lib/add-message.cc b/lib/add-message.cc > index f5fac8be..da37032c 100644 > --- a/lib/add-message.cc > +++ b/lib/add-message.cc > @@ -227,7 +227,7 @@ _notmuch_database_link_message_to_parents (notmuch_database_t *notmuch, > const char **thread_id) > { > GHashTable *parents = NULL; > - const char *refs, *in_reply_to, *in_reply_to_message_id; > + const char *refs, *in_reply_to, *in_reply_to_message_id, *strict_message_id = NULL; > const char *last_ref_message_id, *this_message_id; > GList *l, *keys = NULL; > notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS; > @@ -242,14 +242,24 @@ _notmuch_database_link_message_to_parents (notmuch_database_t *notmuch, > parents, refs); > > in_reply_to = _notmuch_message_file_get_header (message_file, "in-reply-to"); > + if (in_reply_to) > + strict_message_id = _notmuch_message_id_parse_strict (message, > + in_reply_to); > + > in_reply_to_message_id = parse_references (message, > this_message_id, > parents, in_reply_to); > > - /* For the parent of this message, use the last message ID of the > - * References header, if available. If not, fall back to the > - * first message ID in the In-Reply-To header. */ > - if (last_ref_message_id) { > + /* For the parent of this message, use > + * 1) the In-Reply-To header, if it looks sane, otherwise > + * 2) the last message ID of the References header, if available. > + * 3) Otherwise, fall back to the first message ID in > + * the In-Reply-To header. > + */ > + > + if (strict_message_id) { > + _notmuch_message_add_term (message, "replyto", strict_message_id); > + } else if (last_ref_message_id) { > _notmuch_message_add_term (message, "replyto", > last_ref_message_id); > } else if (in_reply_to_message_id) { > diff --git a/test/T510-thread-replies.sh b/test/T510-thread-replies.sh > index 87dae2df..4688c0ab 100755 > --- a/test/T510-thread-replies.sh > +++ b/test/T510-thread-replies.sh > @@ -210,7 +210,6 @@ EOF > test_expect_equal_file EXPECTED OUTPUT > > test_begin_subtest "trusting reply-to (tree view)" > -test_subtest_known_broken > test_emacs '(notmuch-tree "id:B00-root@example.org") > (notmuch-test-wait) > (test-output) > -- > 2.18.0 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch