From: Jesse Rosenthal <jrosenthal@jhu.edu>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH] Avoid empty thread names if possible.
In-Reply-To: <87tx2nuvec.fsf@qmul.ac.uk>
References: <87oatnakqy.fsf@jhu.edu> <87tx2nuvec.fsf@qmul.ac.uk>
User-Agent: Notmuch/0.18.2+155~gafa8535 (http://notmuchmail.org) Emacs/24.4.1
	(i686-pc-linux-gnu)
Date: Wed, 29 Oct 2014 09:05:24 -0400
Message-ID: <87d29b2fhn.fsf@jhu.edu>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

Hi,

Thanks for taking a look at this.

Mark Walters <markwalters1009@gmail.com> 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?

> 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.)

> 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;

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