[PATCH] emacs: functions to import sender or recipient into BBDB

2012-10-20 Thread Tomi Ollila
On Sat, Oct 20 2012, Jameson Graef Rollins  
wrote:

> On Sat, Oct 20 2012, Tomi Ollila  wrote:
>> I was thinking notmuch-bbdb.el but contrib/notmuch-bbdb.el could be
>> good start.
>
> I find this functionality to be really useful.  I don't know if we have
> a method of distributing/packaging stuff in contrib, and since I would
> prefer to see this actually integrated, lets just put it in
> emacs/notmuch-bbdb.el.

Fine by me. I suggest:

file emacs/notmuch-bbdb.el

functions

 notmuch-bbdb/snarf-between-commas
 notmuch-bbdb/snarf-header

 notmuch-bbdb/snarf-from
 notmuch-bbdb/snarf-to

(or if the format in origina patch is in line what has been done elsewhere, 
then:)

 bbdb/notmuch-snarf-header

 bbdb/notmuch-snarf-from
 bbdb/notmuch-snarf-to

(and, with Ethan's comments addressed)

And then, to notmuch.el or notmuch-show.el:

(autoload 'notmuch-bbdb/snarf-from "notmuch-bbdb" 
  "Import the sender of the current message into BBDB" t)
(autoload 'notmuch-bbdb/snarf-to "notmuch-bbdb" 
  "Import all recipients of the current message into BBDB" t)






>
> jamie.
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] lib: fix warnings when building with clang

2012-10-20 Thread Ethan Glasser-Camp
Jani Nikula  writes:

> Building notmuch with CC=clang and CXX=clang++ produces the warnings:
>
> CC -O2 lib/tags.o
> lib/tags.c:43:5: warning: expression result unused [-Wunused-value]
> talloc_steal (tags, list);
> ^
> /usr/include/talloc.h:345:143: note: expanded from:
>   ...__location__); __talloc_steal_ret; })
> ^~
> 1 warning generated.
>
> CXX -O2 lib/message.o
> lib/message.cc:791:5: warning: expression result unused [-Wunused-value]
> talloc_reference (message, message->tag_list);
> ^
> /usr/include/talloc.h:932:36: note: expanded from:
>   ...(_TALLOC_TYPEOF(ptr))_talloc_reference_loc((ctx),(ptr), __location__)
>  ^
> 1 warning generated.
>
> Check talloc_reference() return value, and explicitly ignore
> talloc_steal() return value as it has no failure modes, to silence the
> warnings.
> ---
>  lib/message.cc |4 +++-
>  lib/tags.c |2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/message.cc b/lib/message.cc
> index 978de06..320901f 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -788,7 +788,9 @@ notmuch_message_get_tags (notmuch_message_t *message)
>   * possible to modify the message tags (which talloc_unlink's the
>   * current list from the message) while still iterating because
>   * the iterator will keep the current list alive. */
> -talloc_reference (message, message->tag_list);
> +if (!talloc_reference (message, message->tag_list))
> + return NULL;
> +
>  return tags;
>  }

Hi! What you did with talloc_steal is obviously fine. 

I'd be happier about what you did with talloc_reference() if there were
precedent, or a clearly-articulated convention for notmuch. Instead this
is the third use in the codebase that I can see, and the other two are
each unique to themselves. In mime-node.c we print an "out-of-memory"
error and in lib/filenames.c we cast (void) talloc_reference (...), I
guess figuring that we're pretty much hosed anyhow if we run out of
memory.

Why return NULL here? It seems like if talloc_reference fails, we're
going to crash eventually, so we should print an error to explain our
impending doom. I'd guess you're uneasy printing anything from lib/, but
still want to signal an error, and the only way you can do so is to
return NULL. I guess that silences the compiler warning, but it's not
really the correct way to handle the error IMO. On the other hand, it's
such a weird corner case that I don't even think it merits a FIXME
comment.

How about an assert instead of a return NULL?

Ethan


[PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

2012-10-20 Thread David Bremner
Tomi Ollila  writes:

> From: Tomi Ollila 
>
> When shell executes background process using '&' the scheduling of
> that new process is arbitrary. It could be that smtp-dummy doesn't
> get execution time to listen() it's server socket until some other
> process attempts to connect() to it. The --background option in
> smtp-dummy makes it to go background *after* it started to listen
> its server socket.

series pushed

d


[PATCH] test: another test wrt ignoring user-specified files and directories

2012-10-20 Thread David Bremner
Ethan Glasser-Camp  writes:
> This is the trivial modification of Pieter's patch that makes it work
> no matter where your test directory is.

pushed patches 3 though 6,

d

And yes, _this_ is the series where I used Ethan's edited patch.


[PATCH v2 1/7] test: emacs: new tests "notmuch-show: {show, hide} message headers"

2012-10-20 Thread David Bremner
Pieter Praet  writes:

> * test/emacs:
>
>   - New subtest "notmuch-show: show message headers":
>   - New subtest "notmuch-show: hide message headers":
>   - New subtest "notmuch-show: hide message headers (w/
> notmuch-show-toggle-head

pushed (with Ethan's edit)

d


[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

2012-10-20 Thread Tomi Ollila
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] emacs: functions to import sender or recipient into BBDB

2012-10-20 Thread Tomi Ollila
On Sat, Oct 20 2012, Ethan Glasser-Camp wrote:

> Daniel Bergey  writes:
>
>> From a show buffer, bbdb/notmuch-snarf-from imports the sender into
>> bbdb.  bbdb/notmuch-snarf-to attempts to import all recipients.  BBDB
>> displays a buffer with each contact; C-g displays the next contact, or
>> returns to the notmuch-show buffer.
>>
>> This is my first notmuch patch.  Comments very welcome.
>
> Hi!
>
>>  emacs/notmuch-show.el |   28 
>
> I don't think this belongs in notmuch-show. My first inclination is that
> this should go into a new file contrib/notmuch-bbdb.el (assuming there's
> no other notmuch-bbdb integration stuff floating around).
>
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index 6335d45..3bc1da0 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -1895,6 +1895,34 @@ the user (see 
>> `notmuch-show-stash-mlarchive-link-alist')."
>>  (button-get button :notmuch-filename)
>>  (button-get button :notmuch-content-type)))
>>
>> +;; bbdb interaction functions, awaiting user keybindings
>> +
>> +(defun bbdb/snarf-between-commas ()
>> +  ; What about names written "Surname, First M" ?
>
> Most comments in emacslisp start with two semicolons.

3 quick comments.

I was thinking notmuch-bbdb.el but contrib/notmuch-bbdb.el could be
good start.

then functions like notmuch-bbdb/snarf-between-commas etc...

and yes, ;; comments -- that also keeps indent-region happy.

Tomi



[PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

2012-10-20 Thread Ethan Glasser-Camp
Tomi Ollila  writes:

> From: Tomi Ollila 
>
> When shell executes background process using '&' the scheduling of
> that new process is arbitrary. It could be that smtp-dummy doesn't
> get execution time to listen() it's server socket until some other
> process attempts to connect() to it. The --background option in
> smtp-dummy makes it to go background *after* it started to listen
> its server socket.

This looks fine to me, Michal Sojka's reviewed this version, and this
patch has been bouncing around for almost a year! I'm taking off
needs-review.

Ethan


[PATCHv3] notmuch-show: include Bcc header in json output

2012-10-20 Thread Ethan Glasser-Camp
Michal Nazarewicz  writes:

> From: Michal Nazarewicz 
>
> With this change, emacs users can use notmuch-message-headers
> variable to configure notmuch-show display Bcc header.
> ---

This patch looks pretty straightforward and has seen a certain amount of
review so I'm taking off needs-review.

Ethan


[PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

2012-10-20 Thread Ethan Glasser-Camp
Jani Nikula  writes:

> On Wed, 17 Oct 2012, Adrien Bustany  wrote:
>> The code of the patches in unchanged, but the formatting issues are now
>> hopefully fixed.
>
> Hi Adrien, please check at what version flush and reopen have been
> introduced to xapian. If they are new-ish (I don't know, didn't have the
> time to check), please add appropriate #ifdefs. [1] lays the groundwork
> for this. We'll also need to decide what is the minimum xapian version
> required in general, i.e. features earlier than that don't need
> conditional compilation.

Hi! The new versions of these patches are still pretty trivial and they
still look OK to me, but based on Jani's prompting I decided to look up
the methods. Seems that flush() is a very old (pre-1.1.0, 2009-04) name
for commit(), which is the preferred name these days. You should
probably therefore rename the function notmuch_database_commit, and have
it call the WritableDatabase::commit() method.

reopen() is a very very old method, seems like it has been around since
2004.

So I think Adrien is safe from having to do version checks, but we
should probably use commit() instead of flush().

Ethan


[PATCH] emacs: functions to import sender or recipient into BBDB

2012-10-20 Thread Jameson Graef Rollins
On Sat, Oct 20 2012, Tomi Ollila  wrote:
> I was thinking notmuch-bbdb.el but contrib/notmuch-bbdb.el could be
> good start.

I find this functionality to be really useful.  I don't know if we have
a method of distributing/packaging stuff in contrib, and since I would
prefer to see this actually integrated, lets just put it in
emacs/notmuch-bbdb.el.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20121020/7d1e5a8c/attachment.pgp>


random corpus generator, v3

2012-10-20 Thread Ethan Glasser-Camp
david at tethera.net writes:

> This obsoletes the series at:
>
>  id:"134431-4301-1-git-send-email-bremner at debian.org"
>
> Changes since v2:
>
> - clean up new test-binaries and objects
>
> - remove the "set -o pipefail" leftover from debugging.  Possibly this
>   makes sense as a global setting, but in a seperate patch.
>
> - add hex-escape to test/basic
>
> - rebase against updated master.

Hi! This looks pretty good to me and I am for improving the test
infrastructure.

Some minor problems:

- Patch 2 doesn't apply; neither do patches 4 or 5, presumably due to changes
  that weren't made due to patch 2.

- Commit message discipline: the subject line of patch 4 ends in a
  period. "Seperate" is spelled by most people as "separate", though I
  would encourage you to buck the trend if you are so inclined.

- In patch 4:

> +if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
> +   _notmuch_message_add_term (message, "type", "mail");
> +} else {
> +   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
> +}

Why not switch the branches? That is, check for private_status !=
NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND and return immediately?

- In patch 5:

> +for (count = 0; count < num_messages; count++) {
> + int j;
> + int num_tags = random () % (max_tags + 1);
> + int this_mid_len = random () % message_id_len + 1;

This looks odd. I'm pretty sure it's correct, but my brain keeps saying,
"Why are there no parentheses on (message_id_len + 1)?" Maybe just a
comment that message ids must be at least one character long, or the
ranges of values necessary for both of these variables.

Ethan


Re: [PATCH] emacs: functions to import sender or recipient into BBDB

2012-10-20 Thread Tomi Ollila
On Sat, Oct 20 2012, Ethan Glasser-Camp wrote:

 Daniel Bergey ber...@alum.mit.edu writes:

 From a show buffer, bbdb/notmuch-snarf-from imports the sender into
 bbdb.  bbdb/notmuch-snarf-to attempts to import all recipients.  BBDB
 displays a buffer with each contact; C-g displays the next contact, or
 returns to the notmuch-show buffer.

 This is my first notmuch patch.  Comments very welcome.

 Hi!

  emacs/notmuch-show.el |   28 

 I don't think this belongs in notmuch-show. My first inclination is that
 this should go into a new file contrib/notmuch-bbdb.el (assuming there's
 no other notmuch-bbdb integration stuff floating around).

  1 file changed, 28 insertions(+)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index 6335d45..3bc1da0 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -1895,6 +1895,34 @@ the user (see 
 `notmuch-show-stash-mlarchive-link-alist').
  (button-get button :notmuch-filename)
  (button-get button :notmuch-content-type)))

 +;; bbdb interaction functions, awaiting user keybindings
 +
 +(defun bbdb/snarf-between-commas ()
 +  ; What about names written Surname, First M u...@server.tld?

 Most comments in emacslisp start with two semicolons.

3 quick comments.

I was thinking notmuch-bbdb.el but contrib/notmuch-bbdb.el could be
good start.

then functions like notmuch-bbdb/snarf-between-commas etc...

and yes, ;; comments -- that also keeps indent-region happy.

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

2012-10-20 Thread Tomi Ollila
On Sat, Oct 20 2012, Ethan Glasser-Camp wrote:

 Ethan Glasser-Camp ethan.glasser.c...@gmail.com 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 v2 0/2] Add flush/reopen methods to notmuch_database_t

2012-10-20 Thread Ethan Glasser-Camp
Jani Nikula j...@nikula.org writes:

 On Wed, 17 Oct 2012, Adrien Bustany adr...@bustany.org wrote:
 The code of the patches in unchanged, but the formatting issues are now
 hopefully fixed.

 Hi Adrien, please check at what version flush and reopen have been
 introduced to xapian. If they are new-ish (I don't know, didn't have the
 time to check), please add appropriate #ifdefs. [1] lays the groundwork
 for this. We'll also need to decide what is the minimum xapian version
 required in general, i.e. features earlier than that don't need
 conditional compilation.

Hi! The new versions of these patches are still pretty trivial and they
still look OK to me, but based on Jani's prompting I decided to look up
the methods. Seems that flush() is a very old (pre-1.1.0, 2009-04) name
for commit(), which is the preferred name these days. You should
probably therefore rename the function notmuch_database_commit, and have
it call the WritableDatabase::commit() method.

reopen() is a very very old method, seems like it has been around since
2004.

So I think Adrien is safe from having to do version checks, but we
should probably use commit() instead of flush().

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCHv3] notmuch-show: include Bcc header in json output

2012-10-20 Thread Ethan Glasser-Camp
Michal Nazarewicz m...@google.com writes:

 From: Michal Nazarewicz min...@mina86.com

 With this change, emacs users can use notmuch-message-headers
 variable to configure notmuch-show display Bcc header.
 ---

This patch looks pretty straightforward and has seen a certain amount of
review so I'm taking off needs-review.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

2012-10-20 Thread Ethan Glasser-Camp
Tomi Ollila tomi.oll...@iki.fi writes:

 From: Tomi Ollila t...@iki.fi

 When shell executes background process using '' the scheduling of
 that new process is arbitrary. It could be that smtp-dummy doesn't
 get execution time to listen() it's server socket until some other
 process attempts to connect() to it. The --background option in
 smtp-dummy makes it to go background *after* it started to listen
 its server socket.

This looks fine to me, Michal Sojka's reviewed this version, and this
patch has been bouncing around for almost a year! I'm taking off
needs-review.

Ethan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] emacs: functions to import sender or recipient into BBDB

2012-10-20 Thread Jameson Graef Rollins
On Sat, Oct 20 2012, Tomi Ollila tomi.oll...@iki.fi wrote:
 I was thinking notmuch-bbdb.el but contrib/notmuch-bbdb.el could be
 good start.

I find this functionality to be really useful.  I don't know if we have
a method of distributing/packaging stuff in contrib, and since I would
prefer to see this actually integrated, lets just put it in
emacs/notmuch-bbdb.el.

jamie.


pgpuJhBAr8AQp.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] emacs: functions to import sender or recipient into BBDB

2012-10-20 Thread Tomi Ollila
On Sat, Oct 20 2012, Jameson Graef Rollins jroll...@finestructure.net wrote:

 On Sat, Oct 20 2012, Tomi Ollila tomi.oll...@iki.fi wrote:
 I was thinking notmuch-bbdb.el but contrib/notmuch-bbdb.el could be
 good start.

 I find this functionality to be really useful.  I don't know if we have
 a method of distributing/packaging stuff in contrib, and since I would
 prefer to see this actually integrated, lets just put it in
 emacs/notmuch-bbdb.el.

Fine by me. I suggest:

file emacs/notmuch-bbdb.el

functions

 notmuch-bbdb/snarf-between-commas
 notmuch-bbdb/snarf-header

 notmuch-bbdb/snarf-from
 notmuch-bbdb/snarf-to

(or if the format in origina patch is in line what has been done elsewhere, 
then:)

 bbdb/notmuch-snarf-header

 bbdb/notmuch-snarf-from
 bbdb/notmuch-snarf-to

(and, with Ethan's comments addressed)

And then, to notmuch.el or notmuch-show.el:

(autoload 'notmuch-bbdb/snarf-from notmuch-bbdb 
  Import the sender of the current message into BBDB t)
(autoload 'notmuch-bbdb/snarf-to notmuch-bbdb 
  Import all recipients of the current message into BBDB t)







 jamie.
 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 1/7] test: emacs: new tests notmuch-show: {show, hide} message headers

2012-10-20 Thread David Bremner
Pieter Praet pie...@praet.org writes:

 * test/emacs:

   - New subtest notmuch-show: show message headers:
   - New subtest notmuch-show: hide message headers:
   - New subtest notmuch-show: hide message headers (w/
 notmuch-show-toggle-head
 
pushed (with Ethan's edit)

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: another test wrt ignoring user-specified files and directories

2012-10-20 Thread David Bremner
Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:
 This is the trivial modification of Pieter's patch that makes it work
 no matter where your test directory is.

pushed patches 3 though 6,

d

And yes, _this_ is the series where I used Ethan's edited patch.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

2012-10-20 Thread David Bremner
Tomi Ollila tomi.oll...@iki.fi writes:

 From: Tomi Ollila t...@iki.fi

 When shell executes background process using '' the scheduling of
 that new process is arbitrary. It could be that smtp-dummy doesn't
 get execution time to listen() it's server socket until some other
 process attempts to connect() to it. The --background option in
 smtp-dummy makes it to go background *after* it started to listen
 its server socket.

series pushed

d
___
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

2012-10-20 Thread Peter Wang
On Fri, 19 Oct 2012 01:15:31 -0400, Ethan Glasser-Camp 
ethan.glasser.c...@gmail.com wrote:
 Peter Wang noval...@gmail.com 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 v2 3/3] test: conform to content length, encoding fields

2012-10-20 Thread Peter Wang
On Fri, 19 Oct 2012 22:21:46 -0400, Ethan Glasser-Camp 
ethan.glasser.c...@gmail.com wrote:
  diff --git a/test/json b/test/json
  index ac8fa8e..8ce2e8a 100755
  --- a/test/json
  +++ b/test/json
  @@ -45,7 +45,7 @@ emacs_deliver_message \
(insert \Message-ID: $id\n\)
   output=$(notmuch show --format=json id:$id)
   filename=$(notmuch search --output=files id:$id)
  -test_expect_equal_json $output [[[{\id\: \$id\, \match\: true, 
  \excluded\: false, \filename\: \$filename\, \timestamp\: 946728000, 
  \date_relative\: \2000-01-01\, \tags\: [\inbox\], \headers\: 
  {\Subject\: \$subject\, \From\: \Notmuch Test Suite 
  test_su...@notmuchmail.org\, \To\: \test_su...@notmuchmail.org\, 
  \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, \body\: [{\id\: 1, 
  \content-type\: \multipart/mixed\, \content\: [{\id\: 2, 
  \content-type\: \text/plain\, \content\: \This is a test message 
  with inline attachment with a filename\}, {\id\: 3, \content-type\: 
  \application/octet-stream\, \filename\: \README\}]}]}, [
  +test_expect_equal_json $output [[[{\id\: \$id\, \match\: true, 
  \excluded\: false, \filename\: \$filename\, \timestamp\: 946728000, 
  \date_relative\: \2000-01-01\, \tags\: [\inbox\], \headers\: 
  {\Subject\: \$subject\, \From\: \Notmuch Test Suite 
  test_su...@notmuchmail.org\, \To\: \test_su...@notmuchmail.org\, 
  \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, \body\: [{\id\: 1, 
  \content-type\: \multipart/mixed\, \content\: [{\id\: 2, 
  \content-type\: \text/plain\, \content\: \This is a test message 
  with inline attachment with a filename\}, {\id\: 3, \content-type\: 
  \application/octet-stream\, \content-length\: 12392, 
  \content-transfer-encoding\: \base64\, \filename\: \README\}]}]}, 
  [
 
 This test fails for me. You're encoding the content-length of
 test/README. test/README certainly hasn't changed in the last six months
 so that seems like a reasonable thing to do... except then why is it
 12392 on your machine, and 12380 on mine?

README was changed in 1ffb38.  Hardcoding the length was clearly a
bad idea, so I will send a replacement patch.

Peter
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3] test: conform to content length, encoding fields

2012-10-20 Thread Peter Wang
Update tests to expect content-length and content-transfer-encoding
fields in show --format=json output, for leaf parts with omitted body
content.
---
 test/crypto| 30 +-
 test/json  |  4 +++-
 test/multipart |  9 +
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/test/crypto b/test/crypto
index 5dd14c4..aa96ec2 100755
--- a/test/crypto
+++ b/test/crypto
@@ -61,7 +61,8 @@ expected='[[[{id: X,
  content-type: text/plain,
  content: This is a test signed message.\n},
  {id: 3,
- content-type: application/pgp-signature}]}]},
+ content-type: application/pgp-signature,
+ content-length: 315}]}]},
  ['
 test_expect_equal_json \
 $output \
@@ -95,7 +96,8 @@ expected='[[[{id: X,
  content-type: text/plain,
  content: This is a test signed message.\n},
  {id: 3,
- content-type: application/pgp-signature}]}]},
+ content-type: application/pgp-signature,
+ content-length: 315}]}]},
  ['
 test_expect_equal_json \
 $output \
@@ -127,7 +129,8 @@ expected='[[[{id: X,
  content-type: text/plain,
  content: This is a test signed message.\n},
  {id: 3,
- content-type: application/pgp-signature}]}]},
+ content-type: application/pgp-signature,
+ content-length: 315}]}]},
  ['
 test_expect_equal_json \
 $output \
@@ -196,7 +199,8 @@ expected='[[[{id: X,
  sigstatus: [],
  content-type: multipart/encrypted,
  content: [{id: 2,
- content-type: application/pgp-encrypted},
+ content-type: application/pgp-encrypted,
+ content-length: 11},
  {id: 3,
  content-type: multipart/mixed,
  content: [{id: 4,
@@ -204,6 +208,8 @@ expected='[[[{id: X,
  content: This is a test encrypted message.\n},
  {id: 5,
  content-type: application/octet-stream,
+ content-length: 28,
+ content-transfer-encoding: base64,
  filename: TESTATTACHMENT}]}]}]},
  ['
 test_expect_equal_json \
@@ -231,9 +237,11 @@ test_expect_equal_file OUTPUT TESTATTACHMENT
 
 test_begin_subtest decryption failure with missing key
 mv ${GNUPGHOME}{,.bak}
+# The length of the encrypted attachment varies so must be normalized.
 output=$(notmuch show --format=json --decrypt subject:test encrypted message 
001 \
 | notmuch_json_show_sanitize \
-| sed -e 's|created: [1234567890]*|created: 946728000|')
+| sed -e 's|created: [1234567890]*|created: 946728000|' \
+| sed -e 's|content-length: 6[1234567890]*|content-length: 652|')
 expected='[[[{id: X,
  match: true,
  excluded: false,
@@ -249,9 +257,11 @@ expected='[[[{id: X,
  encstatus: [{status: bad}],
  content-type: multipart/encrypted,
  content: [{id: 2,
- content-type: application/pgp-encrypted},
+ content-type: application/pgp-encrypted,
+ content-length: 11},
  {id: 3,
- content-type: application/octet-stream}]}]},
+ content-type: application/octet-stream,
+ content-length: 652}]}]},
  ['
 test_expect_equal_json \
 $output \
@@ -287,7 +297,8 @@ expected='[[[{id: X,
  userid:  Notmuch Test Suite test_su...@notmuchmail.org (INSECURE!)}],
  content-type: multipart/encrypted,
  content: [{id: 2,
- content-type: application/pgp-encrypted},
+ content-type: application/pgp-encrypted,
+ content-length: 11},
  {id: 3,
  content-type: text/plain,
  content: This is another test encrypted message.\n}]}]},
@@ -342,7 +353,8 @@ expected='[[[{id: X,
  content-type: text/plain,
  content: This is a test signed message.\n},
  {id: 3,
- content-type: application/pgp-signature}]}]},
+ content-type: application/pgp-signature,
+ content-length: 315}]}]},
  ['
 test_expect_equal_json \
 $output \
diff --git a/test/json b/test/json
index ac8fa8e..9911eeb 100755
--- a/test/json
+++ b/test/json
@@ -45,7 +45,9 @@ emacs_deliver_message \
  (insert \Message-ID: $id\n\)
 output=$(notmuch show --format=json id:$id)
 filename=$(notmuch search --output=files id:$id)
-test_expect_equal_json $output [[[{\id\: \$id\, \match\: true, 
\excluded\: false, \filename\: \$filename\, \timestamp\: 946728000, 
\date_relative\: \2000-01-01\, \tags\: [\inbox\], \headers\: 
{\Subject\: \$subject\, \From\: \Notmuch Test Suite 
test_su...@notmuchmail.org\, \To\: \test_su...@notmuchmail.org\, 
\Date\: \Sat, 01 Jan 2000 12:00:00 +\}, \body\: [{\id\: 1, 
\content-type\: \multipart/mixed\, \content\: [{\id\: 2, 
\content-type\: \text/plain\, \content\: \This is a test message with 
inline attachment with a filename\}, {\id\: 3, \content-type\: 
\application/octet-stream\, \filename\: \README\}]}]}, [
+# Get length of README after base64-encoding, minus additional newline.
+attachment_length=$(( $(base64 $TEST_DIRECTORY/README | wc -c) - 1 ))
+test_expect_equal_json $output [[[{\id\: \$id\, \match\: true, 
\excluded\: false, \filename\: \$filename\, \timestamp\: 946728000, 
\date_relative\: \2000-01-01\, \tags\: [\inbox\], \headers\: 
{\Subject\: \$subject\, \From\: \Notmuch Test Suite 
test_su...@notmuchmail.org\, \To\: \test_su...@notmuchmail.org\, 
\Date\: \Sat, 01 Jan 2000 12:00:00 +\}, \body\: [{\id\: 1,