Re: [PATCH 00/11] add recipients to search output

Subject: Re: [PATCH 00/11] add recipients to search output

Date: Thu, 23 Aug 2012 10:21:33 +0300

To: Jameson Graef Rollins, Notmuch Mail

Cc:

From: Tomi Ollila


On Mon, Aug 20 2012, Jameson Graef Rollins wrote:

> This series is an attempt to add thread recipients to the search
> output.
>
> My personal overall goal of this series is to support the handling of
> drafts in the emacs ui.  For drafts we want to see recipients, instead
> of authors, in the search output.  I can imagine other uses for this
> series as well, though.
>
> The first four patches generalize the author list handling in thread
> objects to handle any address list.  These patches could be applied
> regardless of if the rest of the series is accepted.
>
> After that we modify the thread constructor such that it can hold
> thread recipients as well.  Since there is overhead in retrieving
> thread recipients from the message files (recipients are not stored in
> the database) this is handled with a switch.
>
> Further patches add the new switch to the search CLI that adds thread
> recipients to the structured output formats.  I didn't modify the text
> output format, since there is no way to extend it.  I can imagine
> tweaking the text output such that the author field is instead
> replaced by the recipients (as is done for the emacs UI at the end of
> the series), but that's not done here.
>
> In the emacs UI, I add a new toggle function that will toggle display
> of thread authors or recipients in the 'authors' field of the search
> output.  It's unfortunate that this ambiguity in that field name
> remains, but I didn't know how to change that cleanly.  I'm working on
> some tests for the new emacs functionality that I'll include in the
> inevitable v2 of this series.

I did not read much of this introduction before browsing to the code, I
was about to comment whether attempt yo do less trivial tests are
to be done.

> The last patch is mostly just a tickle to suggest adding the
> recipients to the database.  It would make the --include-recipient
> searches much faster of course, but it might be overhead in the
> database that folks aren't interested in.

I got tickled... adding To (and Cc?!) to the database would also give
(future notmuch?) address completion more addresses to match for.

We should discuss whether to add other headers too? IIRC someone (Austin?)
mentioned that everything (except Received:) headers could be there ?

> As always, feedback, review, and comments are much appreciated.

Overall, the code looks good (to me).

> jamie.

Tomi

Thread: