[PATCH] Reordering of thread authors to list matching authors first

[Carl Worth]

On Wed, 21 Apr 2010 20:58:27 -0700, Dirk Hohndel <hohndel at infradead.org> wrote:
> When displaying threads as result of a search it makes sense to list those
> authors first who match the search. The matching authors are separated from the
> non-matching ones with a '|' instead of a ','

It seems a reasonable feature to me.

Some notes on the patch:

> +void
> +notmuch_message_set_author (notmuch_message_t *message,
> +			    const char *author)
> +{
> +    message->author = talloc_strdup(message, author);
> +    return;
> +}

This is leaking any previously set author value, (admittedly, it's only
a "talloc leak" so it will still get cleaned up when the message is
cleaned up, but still.

> +/* Set the author member of 'message' - this is the representation used
> + * when displaying the message
> + */
> +void
> +notmuch_message_set_author (notmuch_message_t *message, const char *author);
> +
> +/* Get the author member of 'message'
> + */
> +const char *
> +notmuch_message_get_author (notmuch_message_t *message);

The notmuch.h file is our publicly installed header file for the library
interface. I don't think the feature here requires any new library
interface. Even if it did, we wouldn't want a public function like
set_author that could simply scramble internal state and change the
result of future calls to get_author.

> +/*
> + * move authors of matched messages in the thread to 
> + * the front of the authors list, but keep them in
> + * existing order within their group
> + */
> +static void
> +_thread_move_matched_author (notmuch_thread_t *thread,
> +			     const char *author)

The implementation here seems a bit fiddly.

We already have two passes over the messages, (first all messages, and
then all matched messages). And we're currently calling add_author from
the first pass.

How about simply calling a new add_matched_author from the second pass
which would look very much like the existing add_author. Then we could
change add_author to accumulate authors into an array rather than a
string. Then, finally, we would append any of these authors not already
in the matched_authors hash tabled onto the final string.

That should be less code and easier to understand I think.

I can take a whack at that later if you don't beat me to it.

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20100423/dda8f466/attachment.pgp>