From: Pieter Praet <pieter@praet.org>
To: Austin Clements <amdragon@MIT.EDU>, David Bremner <david@tethera.net>
Subject: Re: [PATCH 6/6] emacs: make `notmuch-search-operate-all' operate on
	threads, not messages
In-Reply-To: <20111112163502.GE2658@mit.edu>
References: <1310313335-4159-1-git-send-email-pieter@praet.org>
	<1310313335-4159-7-git-send-email-pieter@praet.org>
	<87fwht2u9k.fsf@rocinante.cs.unb.ca>
	<20111112163502.GE2658@mit.edu>
User-Agent: Notmuch/0.9+76~g2fd88e6 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-unknown-linux-gnu)
Date: Wed, 16 Nov 2011 14:55:41 +0100
Message-ID: <871ut8f9ya.fsf@praet.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Notmuch Mail <notmuch@notmuchmail.org>
Precedence: list

On Sat, 12 Nov 2011 11:35:02 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth David Bremner on Nov 12 at 11:13 am:
> > On Sun, 10 Jul 2011 17:55:35 +0200, Pieter Praet <pieter@praet.org> wrote:
> > > In order to be consistent with `notmuch-search-{add,remove}-tag' ("+"/"-"),
> > > `notmuch-search-operate-all' ("*") should operate on matching threads
> > > instead of matching messages.
> > > 
> > 
> > As far as I can tell, the follow-up series for the race condition kindof
> > got stalled.  Am I right in thinking this bug fix should still be
> > applied? I didn't see any review/feedback on the list.
> 
> We concluded that fixing the tagging race correctly was actually a lot
> of work, which should be done but hasn't yet.  We have to add message
> IDs or docids to the search results, which is difficult to do with the
> current text format, so rather than further entrenching ourselves, we
> should first we should migrate Emacs to using the JSON-based search
> output.
> 

Yeah, sorry I haven't replied there yet.  Still haven't found a
sufficiently uninterrupted stretch of time to give the *massive*
amount of work you did the attention it deserves.

> However, this series doesn't actually have much to do with the race.

Correct.  Only patch #4 is more or less relevant to fixing the
`notmuch-search-operate-all' race condition (safety net for when
I make stupid mistakes).

Patches #1-3 should have been in a separate thread (or as updates in
their original thread [1]), but since #1 and #2 are mainly there to
support #3 and #3 is tagging-related, I though it wouldn't hurt to
include them.

Patches #5-6 are a matter of opinion:

> I think the question here is whether notmuch-search-operate-all should
> affect only matched messages or entire threads.  It seems to me it
> should affect all threads, since that's what you're seeing visually,
> but other people may disagree.
> 

Same here.  I don't use it that often, but if its name includes
"operate-all", it should do just that, or the function should be
renamed.

> The test patches seem reasonable, though they could use a little
> review before being pushed.

I'd really appreciate it.

AFAIC, increasing test coverage should be a top priority.


Peace

-- 
Pieter

[1] id:"1305275652-22956-1-git-send-email-pieter@praet.org"