On Wed, 21 Apr 2010 20:58:27 -0700, Dirk Hohndel <hohndel@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