[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
cover all four values of search --exclude in the cli.
---
lib/notmuch.h|1 +
lib/query.cc |6 --
notmuch-search.c |2 +-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 1280afd..f4381ae 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -504,6 +504,7 @@ notmuch_query_get_query_string (notmuch_query_t *query);
typedef enum {
NOTMUCH_EXCLUDE_FALSE,
NOTMUCH_EXCLUDE_TRUE,
+NOTMUCH_EXCLUDE_FLAG,
NOTMUCH_EXCLUDE_ALL
} notmuch_exclude_t;
diff --git a/lib/query.cc b/lib/query.cc
index f752452..1e0f292 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -221,10 +221,12 @@ notmuch_query_search_messages (notmuch_query_t *query)
if (query->exclude_terms) {
exclude_query = _notmuch_exclude_tags (query, final_query);
- 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 {
exclude_query = Xapian::Query (Xapian::Query::OP_AND,
exclude_query, final_query);
diff --git a/notmuch-search.c b/notmuch-search.c
index 89b5bf9..027923d 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -527,7 +527,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
for (i = 0; i < search_exclude_tags_length; i++)
notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
if (exclude == EXCLUDE_FLAG)
- notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FALSE);
+ notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);
if (exclude == EXCLUDE_ALL)
notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
}
--
1.7.4.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
cover all four values of search --exclude in the cli.
---
lib/notmuch.h|1 +
lib/query.cc |6 --
notmuch-search.c |2 +-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 1280afd..f4381ae 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -504,6 +504,7 @@ notmuch_query_get_query_string (notmuch_query_t *query);
typedef enum {
NOTMUCH_EXCLUDE_FALSE,
NOTMUCH_EXCLUDE_TRUE,
+NOTMUCH_EXCLUDE_FLAG,
NOTMUCH_EXCLUDE_ALL
} notmuch_exclude_t;
diff --git a/lib/query.cc b/lib/query.cc
index f752452..1e0f292 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -221,10 +221,12 @@ notmuch_query_search_messages (notmuch_query_t *query)
if (query->exclude_terms) {
exclude_query = _notmuch_exclude_tags (query, final_query);
- 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 {
exclude_query = Xapian::Query (Xapian::Query::OP_AND,
exclude_query, final_query);
diff --git a/notmuch-search.c b/notmuch-search.c
index 89b5bf9..027923d 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -527,7 +527,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
for (i = 0; i < search_exclude_tags_length; i++)
notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
if (exclude == EXCLUDE_FLAG)
- notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FALSE);
+ notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);
if (exclude == EXCLUDE_ALL)
notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
}
--
1.7.4.4
[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
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.
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 +
+++ excludes.3.output 2012-10-19 04:45:06.900518377 +
@@ -1,2 +1,2 @@
-id:msg-001 at notmuch-test-suite
id:msg-002 at notmuch-test-suite
+id:msg-001 at notmuch-test-suite
FAIL Search, don't exclude "deleted" messages when --exclude=flag specified
--- excludes.7.expected 2012-10-19 04:45:07.004518378 +
+++ excludes.7.output 2012-10-19 04:45:07.004518378 +
@@ -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 +
+++ excludes.8.output 2012-10-19 04:45:07.028518377 +
@@ -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.
Mark Walters wrote in
id:"1340198947-29370-5-git-send-email-novalazy at gmail.com" that he
thought patch 1/8 seemed more intrusive than he liked. Maybe I just have
a higher standard for "intrusive" than he does ;) but I thought it was
fine.
It looks like you have better wording for patch 4/8 so I'd like to see
you resend it.
> - 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'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!
Ethan
[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
Ethan Glasser-Camp writes: > It looks like you have better wording for patch 4/8 so I'd like to see > you resend it. > > I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit! It turns out that patch 4 already has a v2 in the thread, but I didn't see it due to some kind of selective blindness. It would be nice if nmbug had grouped it as part of the same patch series. I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain. Ethan
[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
On Sat, Oct 20 2012, Ethan Glasser-Camp wrote: > Ethan Glasser-Camp writes: > >> It looks like you have better wording for patch 4/8 so I'd like to see >> you resend it. >> >> I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit! > > It turns out that patch 4 already has a v2 in the thread, but I didn't > see it due to some kind of selective blindness. It would be nice if > nmbug had grouped it as part of the same patch series. I noticed the same today when I tagged your NEWS patch. By looking contrib/nmbug/nmbug-status the reason seems obvious: it iterates over messages and resets thread when thread id changes (and message ordering is not based on threads). There is 2 ways to "fix" this: 1) Iterate over threads and then messges over these threads. I looked the bindings and this didn't seem so easy as one could imagine.. 2) Iterate over messages like now, but cache the content to an objects in hash where thread-id is the key. If no-one points me an easy way to do (1), I'll attempt (2) (in a distant future ;) > > I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain. > > Ethan Tomi
[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
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 +
> +++ excludes.3.output 2012-10-19 04:45:06.900518377 +
> @@ -1,2 +1,2 @@
> -id:msg-001 at notmuch-test-suite
> id:msg-002 at notmuch-test-suite
> +id:msg-001 at notmuch-test-suite
>
> FAIL Search, don't exclude "deleted" messages when --exclude=flag specified
> --- excludes.7.expected 2012-10-19 04:45:07.004518378 +
> +++ excludes.7.output 2012-10-19 04:45:07.004518378 +
> @@ -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 +
> +++ excludes.8.output 2012-10-19 04:45:07.028518377 +
> @@ -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.
Peter
[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
On Sun, Oct 21 2012, Peter Wang wrote:
> On Fri, 19 Oct 2012 01:15:31 -0400, Ethan Glasser-Camp gmail.com> 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 +
>> +++ excludes.3.output 2012-10-19 04:45:06.900518377 +
>> @@ -1,2 +1,2 @@
>> -id:msg-001 at notmuch-test-suite
>> id:msg-002 at notmuch-test-suite
>> +id:msg-001 at notmuch-test-suite
>>
>> FAIL Search, don't exclude "deleted" messages when --exclude=flag
>> specified
>> --- excludes.7.expected 2012-10-19 04:45:07.004518378 +
>> +++ excludes.7.output 2012-10-19 04:45:07.004518378 +
>> @@ -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 +
>> +++ excludes.8.output 2012-10-19 04:45:07.028518377 +
>> @@ -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
[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
Peter Wang writes: > Does it help if you add a "sleep 1" before the second generate_message > call, i.e. on line 35? It turns out that this test failure is sporadic (perhaps due to the fact that I'm running on tmpfs) and exists even before this series. Doing "sleep 1" makes it go away, but that "fix" is unrelated to this series and ought to be fixed elsewhere. > 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. If Tomi is OK with it, then I guess it's fine. And it's true that there are a couple of places where braces are used with long conditions and then-parts. This series has my +1. Ethan
Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
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.
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 +
+++ excludes.3.output 2012-10-19 04:45:06.900518377 +
@@ -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 +
+++ excludes.7.output 2012-10-19 04:45:07.004518378 +
@@ -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 +
+++ excludes.8.output 2012-10-19 04:45:07.028518377 +
@@ -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.
Mark Walters wrote in
id:"1340198947-29370-5-git-send-email-noval...@gmail.com" that he
thought patch 1/8 seemed more intrusive than he liked. Maybe I just have
a higher standard for "intrusive" than he does ;) but I thought it was
fine.
It looks like you have better wording for patch 4/8 so I'd like to see
you resend it.
> - 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'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!
Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
Ethan Glasser-Camp writes: > It looks like you have better wording for patch 4/8 so I'd like to see > you resend it. > > I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit! It turns out that patch 4 already has a v2 in the thread, but I didn't see it due to some kind of selective blindness. It would be nice if nmbug had grouped it as part of the same patch series. I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain. Ethan ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
On Sat, Oct 20 2012, Ethan Glasser-Camp wrote: > Ethan Glasser-Camp writes: > >> It looks like you have better wording for patch 4/8 so I'd like to see >> you resend it. >> >> I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit! > > It turns out that patch 4 already has a v2 in the thread, but I didn't > see it due to some kind of selective blindness. It would be nice if > nmbug had grouped it as part of the same patch series. I noticed the same today when I tagged your NEWS patch. By looking contrib/nmbug/nmbug-status the reason seems obvious: it iterates over messages and resets thread when thread id changes (and message ordering is not based on threads). There is 2 ways to "fix" this: 1) Iterate over threads and then messges over these threads. I looked the bindings and this didn't seem so easy as one could imagine.. 2) Iterate over messages like now, but cache the content to an objects in hash where thread-id is the key. If no-one points me an easy way to do (1), I'll attempt (2) (in a distant future ;) > > I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain. > > Ethan Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
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 +
> +++ excludes.3.output 2012-10-19 04:45:06.900518377 +
> @@ -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 +
> +++ excludes.7.output 2012-10-19 04:45:07.004518378 +
> @@ -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 +
> +++ excludes.8.output 2012-10-19 04:45:07.028518377 +
> @@ -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.
Peter
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
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 +
>> +++ excludes.3.output 2012-10-19 04:45:06.900518377 +
>> @@ -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 +
>> +++ excludes.7.output 2012-10-19 04:45:07.004518378 +
>> @@ -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 +
>> +++ excludes.8.output 2012-10-19 04:45:07.028518377 +
>> @@ -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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
Peter Wang writes: > Does it help if you add a "sleep 1" before the second generate_message > call, i.e. on line 35? It turns out that this test failure is sporadic (perhaps due to the fact that I'm running on tmpfs) and exists even before this series. Doing "sleep 1" makes it go away, but that "fix" is unrelated to this series and ought to be fixed elsewhere. > 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. If Tomi is OK with it, then I guess it's fine. And it's true that there are a couple of places where braces are used with long conditions and then-parts. This series has my +1. Ethan ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
