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"