From: Mark Walters Date: Wed, 29 Oct 2014 15:15:29 +0000 (+0000) Subject: Re: [PATCH] Avoid empty thread names if possible. X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=f009e68b75901fa7983d4db9350143f75034283c;p=notmuch-archives.git Re: [PATCH] Avoid empty thread names if possible. --- diff --git a/58/07c068cedb89fba7337b16c6f25727a69533e2 b/58/07c068cedb89fba7337b16c6f25727a69533e2 new file mode 100644 index 000000000..3f94c812d --- /dev/null +++ b/58/07c068cedb89fba7337b16c6f25727a69533e2 @@ -0,0 +1,167 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id E218C431FC2 + for ; Wed, 29 Oct 2014 08:15:40 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -1.098 +X-Spam-Level: +X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5 + tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001, + NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id 9sDaPYZ9ez9M for ; + Wed, 29 Oct 2014 08:15:36 -0700 (PDT) +Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id CECF8431FB6 + for ; Wed, 29 Oct 2014 08:15:35 -0700 (PDT) +Received: from smtp.qmul.ac.uk ([138.37.6.40]) + by mail2.qmul.ac.uk with esmtp (Exim 4.71) + (envelope-from ) + id 1XjUyA-0004fQ-5C; Wed, 29 Oct 2014 15:15:30 +0000 +Received: from 5751dfa2.skybroadband.com ([87.81.223.162] helo=localhost) + by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71) + (envelope-from ) + id 1XjUy9-0006Fi-Of; Wed, 29 Oct 2014 15:15:29 +0000 +From: Mark Walters +To: Jesse Rosenthal , notmuch@notmuchmail.org +Subject: Re: [PATCH] Avoid empty thread names if possible. +In-Reply-To: <87ppdb9eqs.fsf@jhu.edu> +References: <87oatnakqy.fsf@jhu.edu> <87tx2nuvec.fsf@qmul.ac.uk> + <87ppdb9eqs.fsf@jhu.edu> +User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/23.4.1 + (x86_64-pc-linux-gnu) +Date: Wed, 29 Oct 2014 15:15:29 +0000 +Message-ID: <87tx2maovi.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Sender-Host-Address: 87.81.223.162 +X-QM-Geographic: According to ripencc, + this message was delivered by a machine in Britain (UK) (GB). +X-QM-SPAM-Info: Sender has good ham record. :) +X-QM-Body-MD5: 95ba90fb2b3b95c06b27d3b844fe64cd (of first 20000 bytes) +X-SpamAssassin-Score: -0.1 +X-SpamAssassin-SpamBar: / +X-SpamAssassin-Report: The QM spam filters have analysed this message to + determine if it is + spam. We require at least 5.0 points to mark a message as spam. + This message scored -0.1 points. + Summary of the scoring: + * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail + provider * (markwalters1009[at]gmail.com) + * -0.1 AWL AWL: From: address is in the auto white-list +X-QM-Scan-Virus: ClamAV says the message is clean +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +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: Wed, 29 Oct 2014 15:15:41 -0000 + + +Hi + +On Wed, 29 Oct 2014, Jesse Rosenthal wrote: +> [I'm not sure why the below reply did not go to the list. Later replies +> did, so I assume there must have been so problem in the sending. Mark, +> apologies if you get this twice.] +> +> Hi, +> +> Thanks for taking a look at this. +> +> Mark Walters writes: +>> I approve of the change in the output but I am unsure about the +>> implementation. It would be nice to have a clear rule about which +>> subject is taken. Eg: +>> +>> if sort is oldest first then it is the subject of the oldest +>> matching message with a non-empty subject. Similarly if sort +>> is newest first. +> +> The rule is actually in a four-year-old commit message (4971b85c4), in +> almost exactly the same words you used: +> +> ...name threads based on (a) matches for the query, and (b) the +> search order. If the search order is oldest-first (as in the default +> inbox) it chooses the oldest matching message as the subject. If the +> search order is newest-first it chooses the newest one. +> +> Reply prefixes ("Re: ", "Aw: ", "Sv: ", "Vs: ") are ignored +> (case-insensitively) so a Re: won't change the subject. +> +> So we would, essentially, just need to add "non-empty" to this +> phrasing. Where would be the right place to put it? Commit message? +> NEWS? `search` man page? + +First I just wanted to check that I knew exactly what behaviour was +intended. Having the new rule in the commit message might well be +sufficient. + +>> Also, it would be nice if the implementation did not rely on what order +>> we call _thread_add_matched_message on the matching messages in the +>> thread. I think in some ways we already rely on the order (for the order +>> of the author list), but if you want to rely on the order here I think +>> it at least deserves a comment. +> +> That would require a rethinking, I think, of naming -- since it's +> traditionally worked in terms of renaming. When a better option comes, +> we throw out the old one. So order is pretty essential. (Not saying +> that's the best way, just pointing out that it's the way it's been done +> since Carl's initial alpha release.) + +I think that the current code does not depend on the order the messages +are given to _thread_add_matched_message: regardless of the order the +thread will get the subject of the oldest matching message (in +sort=oldest first) + +In contrast your code will give different subjects depending in the +order the messages are fed to _thread_add_matched_message. + +>> So looking at the above I think the oldest first gives the subject in +>> my suggestion above (since the messages are supplied in oldest first +>> order). But newest first may not: indeed if the subject starts out as +>> something and becomes empty then this will set the subject empty and +>> then leave it +> +>> (Note b_thread_set_subject_from_message calls notmuch_message_get_header +>> which returns an empty string "" if the subject line is empty or not +>> present). +> +> Hmmm... I was looking at the following line in +> _thread_set_subject_from_message: +> +> subject = notmuch_message_get_header (message, "subject"); +> if (! subject) +> return; + +but subject="" is not null; subject is only null if +notmuch_message_get_header throws an error. See the documentation for +notmuch_message_get_header. + +Best wishes + +Mark + + + +> +> So, I don't think we ever actually change a content-ful string subject +> to an empty one, as you describe above? If there's a non-empty string +> there, and we get an empty subject, we leave the non-empty string in +> place, right? +> +> Best, +> Jesse