[RFC] reordering and cleanup of thread authors
Michal Sojka
sojkam1 at fel.cvut.cz
Thu Apr 8 23:07:27 PDT 2010
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.
Hi Dirk,
thanks for the patch. It is a very interesting feature. See my comments
below.
>
> [...]
>
> +/*
> + * 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? 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.
-Michal
> + /* we have to make changes, so let's get a temp copy */
> + authorscopy = strdup(thread->authors);
> + if (authorscopy == NULL)
> + return;
> + /* nmstart is the offset into where the non-matched authors start */
> + nmstart = thread->nonmatched_authors - thread->authors;
> + /* copy this author and add the "| " - the if clause above tells us there's more */
> + strncpy(thread->nonmatched_authors,author,author_len);
> + strncpy(thread->nonmatched_authors+author_len,"| ",2);
> + thread->nonmatched_authors += author_len+2;
> + if (idx > 0) {
> + /* we are actually moving authors around, not just changing the separator
> + * first copy the authors that came BEFORE our author */
> + strncpy(thread->nonmatched_authors, authorscopy+nmstart, idx-2);
> + /* finally, if there are authors AFTER our author, copy those */
> + if(author_len+nmstart+idx < authors_len) {
> + strncpy(thread->nonmatched_authors + idx - 2,", ",2);
> + strncpy(thread->nonmatched_authors + idx, authorscopy+nmstart + idx + author_len + 2,
> + authors_len - (nmstart + idx + author_len + 2));
> + }
> + }
> + free(authorscopy);
> + } else {
> + thread->nonmatched_authors += author_len;
> + }
> + return;
> +}
More information about the notmuch
mailing list