Hi On Wed, 29 Oct 2014, Jesse Rosenthal <jrosenthal@jhu.edu> 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 <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? 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