On Fri, 09 Apr 2010 08:07:27 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote: > On Wed, 07 Apr 2010, Dirk Hohndel wrote: > > > > This is based in part on some discussion on IRC today. > > When a thread is displayed in the search results, previously the authors > > were always displayed in chronological order. But if only part of the > > thread matches the query, that may or may not be the intuitive thing to > > do. > > thanks for the patch. It is a very interesting feature. Thanks - I've been using it for a few days now and am fairly happy with it. > > > > +/* > > + * move authors of matched messages in the thread to > > + * the front of the authors list, but keep them in > > + * oldest first order within their group > > + */ > > +static void > > +_thread_move_matched_author (notmuch_thread_t *thread, > > + const char *author) > > +{ > > + char *authorscopy; > > + char *currentauthor; > > + int idx,nmstart,author_len,authors_len; > > + > > + if (thread->authors == NULL) > > + return; > > + if (thread->nonmatched_authors == NULL) > > + thread->nonmatched_authors = thread->authors; > > + author_len = strlen(author); > > + authors_len = strlen(thread->authors); > > + if (author_len == authors_len) { > > + /* just one author */ > > + thread->nonmatched_authors += author_len; > > + return; > > + } > > + currentauthor = strcasestr(thread->authors, author); > > + if (currentauthor == NULL) > > + return; > > + idx = currentauthor - thread->nonmatched_authors; > > + if (idx < 0) { > > + /* already among matched authors */ > > + return; > > + } > > + if (thread->nonmatched_authors + author_len < thread->authors + authors_len) { > > What does the above condition exactly mean? Eh - it's ugly. thread->authors + authors_len points to the trailing '\0' of the list of all my authors. At this point in the code we know that the current position of this author is at or after nonmatched_authors. So if nonmatched_authors + author_len is less than the end of the all authors that means that there was another author in the list behind this one - and we need to move things around. In the else clause we just move the pointer to nonmatched_authors to the end of this author. So yeah, ugly, should be rewritten to make it easier to understand (or at least commented better). > I was not able to decode it > and it seems to be wrong. I expect that the "|" is used to delimit > matched and non-matched authors. If I run notmuch search > thread:XXXXXXXXXXXXXXXX, which matches all messages in the thread, I see > all authors delimited by "|", which I consider wrong. Why do you think it's wrong? It's consistent. The thing that I actually DISlike about the current solution is that the last author has no delimiter (since there's no trailing ',' in the list and I didn't want to realloc the space for the authors string). So you can't tell with one glance if all authors match or all but the last one match. I haven't come up with a better visual way to indicate this... Suggestions? /D -- Dirk Hohndel Intel Open Source Technology Center