From: Tomi Ollila Date: Sun, 21 Oct 2012 12:10:40 +0000 (+0300) Subject: Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=fe37e2ce20f7b572d882ebaeeef071b3bd4c1cfc;p=notmuch-archives.git Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t --- diff --git a/3d/9bbd39cff995f76015445af4a6745a06b71aee b/3d/9bbd39cff995f76015445af4a6745a06b71aee new file mode 100644 index 000000000..7b9dad945 --- /dev/null +++ b/3d/9bbd39cff995f76015445af4a6745a06b71aee @@ -0,0 +1,122 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id EC2B1431FAF + for ; Sun, 21 Oct 2012 05:10:41 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0 +X-Spam-Level: +X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] + autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id IZv4SIPkh8wD for ; + Sun, 21 Oct 2012 05:10:39 -0700 (PDT) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by olra.theworths.org (Postfix) with ESMTP id 01192431FAE + for ; Sun, 21 Oct 2012 05:10:38 -0700 (PDT) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id 40F2210036F; + Sun, 21 Oct 2012 15:10:40 +0300 (EEST) +From: Tomi Ollila +To: Peter Wang , notmuch@notmuchmail.org +Subject: Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t +In-Reply-To: <20121021130427.GA4820@hili.localdomain> +References: <1340198947-29370-1-git-send-email-novalazy@gmail.com> + <1340198947-29370-8-git-send-email-novalazy@gmail.com> + <87txtr6o0c.fsf@betacantrips.com> + <20121021130427.GA4820@hili.localdomain> +User-Agent: Notmuch/0.14+51~g62cd13b (http://notmuchmail.org) Emacs/24.2.1 + (x86_64-unknown-linux-gnu) +X-Face: HhBM'cA~ +MIME-Version: 1.0 +Content-Type: text/plain +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Sun, 21 Oct 2012 12:10:42 -0000 + +On Sun, Oct 21 2012, Peter Wang wrote: + +> On Fri, 19 Oct 2012 01:15:31 -0400, Ethan Glasser-Camp wrote: +>> Peter Wang writes: +>> +>> > Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can +>> > cover all four values of search --exclude in the cli. +>> +>> This series looks good to me. It's a nice clean up and a nice new +>> feature. Patches all apply. +> +> Thanks for the review. +> +>> However, I'm getting test failures like: +>> +>> FAIL Search, exclude "deleted" messages from message search --exclude=false +>> --- excludes.3.expected 2012-10-19 04:45:06.900518377 +0000 +>> +++ excludes.3.output 2012-10-19 04:45:06.900518377 +0000 +>> @@ -1,2 +1,2 @@ +>> -id:msg-001@notmuch-test-suite +>> id:msg-002@notmuch-test-suite +>> +id:msg-001@notmuch-test-suite +>> +>> FAIL Search, don't exclude "deleted" messages when --exclude=flag specified +>> --- excludes.7.expected 2012-10-19 04:45:07.004518378 +0000 +>> +++ excludes.7.output 2012-10-19 04:45:07.004518378 +0000 +>> @@ -1,2 +1,2 @@ +>> -thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread) +>> thread:XXX 2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply (deleted inbox unread) +>> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread) +>> +>> FAIL Search, don't exclude "deleted" messages from search if not configured +>> --- excludes.8.expected 2012-10-19 04:45:07.028518377 +0000 +>> +++ excludes.8.output 2012-10-19 04:45:07.028518377 +0000 +>> @@ -1,2 +1,2 @@ +>> -thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread) +>> thread:XXX 2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread) +>> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread) +>> +>> In other words, threads and messages are coming up out of order. I'm not +>> sure of the right way to fix this. If you would like me to try sticking +>> "| sort" here and there in the tests I will do so. I'm not sure if the +>> test suite is guaranteed to scan messages in a certain order. +> +> Does it help if you add a "sleep 1" before the second generate_message +> call, i.e. on line 35? +> +>> > - if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE) +>> > + if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE || +>> > + query->omit_excluded == NOTMUCH_EXCLUDE_ALL) +>> > + { +>> > final_query = Xapian::Query (Xapian::Query::OP_AND_NOT, +>> > final_query, exclude_query); +>> > - else { +>> > + } else { +>> +>> "House style" is to not put braces around one-line then-clauses. This is +>> the only place where you did that. +> +> I have to disagree. The condition is wrapped over two lines. The then +> part is wrapped over two lines. The else part already has braces. +> All suggest braces around the then part. + +Well, I personally would count none of these as convincing suggestions ;), +but IMHO the braces are OK here (I don't start judging which I'd like more). + +> Peter + +Tomi