MIME-Version: 1.0
In-Reply-To: <87lhhlhryd.fsf@csail.mit.edu>
References:
 <CABPZE7L5yX3tJH+1vMotxL7BHYoQd2BJSbzFuKTAxHgC5+6RHA@mail.gmail.com>
 <87618qfk5l.fsf@maritornes.cs.unb.ca>
 <CABPZE7JGzfSMWKpxKqJeCbpsDhX4xj_HoNoKFL8MbYv9xYpQ7w@mail.gmail.com>
 <87lhhlhryd.fsf@csail.mit.edu>
Date: Wed, 22 Apr 2015 04:01:13 +0200
Message-ID:
 <CABPZE7JSE-WwTGBNpYZuZSepG=TrQSsMx-C0e2vU+7PT50HLbQ@mail.gmail.com>
Subject: Re: notmuch_thread_get_authors
From: Ronny Chevalier <chevalier.ronny@gmail.com>
To: Austin Clements <aclements@csail.mit.edu>
Content-Type: text/plain; charset=UTF-8
Cc: notmuch@notmuchmail.org
Precedence: list

On Wed, Apr 22, 2015 at 3:28 AM, Austin Clements
<aclements@csail.mit.edu> wrote:
> On Tue, 21 Apr 2015, Ronny Chevalier <chevalier.ronny@gmail.com> wrote:
>> On Tue, Apr 21, 2015 at 1:35 AM, David Bremner <david@tethera.net> wrote:
>>> Ronny Chevalier <chevalier.ronny@gmail.com> writes:
>> Austin Clements wrote:
>>> And I think there's a fairly easy way to do it in C code that will
>>> also prevent library interface bloat: instead of introducing new
>>> library APIs to get at this information, just use the existing
>>> notmuch_thread_get_messages API and construct the matched and
>>> non-matched lists in the CLI.  Doing it in the CLI wouldn't require
>>> the library to export yet another string list structure, which is
>>> always a huge pain (thanks C!), and wouldn't introduce more "helper"
>>> functions into the library API.
>>
>> I disagree with what Austin said. Because this does not solve the
>> issue at all (or I misunderstood). The issue is with the notmuch API,
>> if someone is using this library there no way it can parse properly
>> the authors.
>> In my case I am not using the CLI but the notmuch library, fixing this
>> in the CLI is just an hack, and it does not fix the issue for the
>> library users.
>
> My suggestion was in no way specific to the CLI. That was the context of
> the discussion at the time, but for the purposes of this discussion, the
> CLI is just another library user.

Ok, sorry for misunderstanding.

>
> You're completely right that there's no way to reliably parse the
> authors list returned by notmuch_thread_get_authors. So don't do
> that. Just use notmuch_thread_get_messages, walk the messages list, and
> build your own authors list. There's no need to introduce additional
> complexity and surface area into the library API for this specific use
> case (IMO, even notmuch_thread_get_authors shouldn't exist, but it's
> there for legacy reasons.) Then you can get author lists for matched,
> non-matched, matching a specific tag, just the to, just the from, counts
> of how many times each author appeared, whatever you want.
>

Ok thanks!

If I read the code correctly, _notmuch_thread_create in lib/thread.cc
process every message to get information like tags, subject and
authors. Since notmuch_thread_get_authors is here for legacy reasons,
would it be better to generate the list of authors only when requested
with notmuch_thread_get_authors (and cache the result of course)?
Because, new code will not use this and will do this work manually,
the generation of the list in intern consumes resources for nothing.

>> Furthermore, I do not see why providing a string list NULL-terminated
>> in C is a huge pain?
>
> See the notmuch_tags_* and notmuch_filesnames_* APIs. Those are just
> string lists.
>
>> Otherwise, I agree with Mark Walters comments on the patch.
>>
>> If no one is working to fix this at the moment, I can send a patch?
>>
>> Ronny