Re: [PATCH v6 1/2] emacs: User-defined sections in notmuch-hello

2011-11-27 Thread Michal Sojka
On Mon, 28 Nov 2011, Dmitry Kurochkin wrote:
> Hi Daniel.
> 
> On Thu, 24 Nov 2011 15:01:01 +0100, Daniel Schoepe  wrote:
> > On Thu, 24 Nov 2011 09:54:50 -0400, David Bremner  wrote:
> > > On Mon, 10 Oct 2011 15:39:41 +0200, Daniel Schoepe  
> > > wrote:
> > > > From: Daniel Schoepe 
> > > > 
> > > > This patch makes the notmuch-hello screen fully customizable
> > > > by allowing the user to add and remove arbitrary sections. It
> > > > also provides some convenience functions for constructing sections,
> > > > e.g. showing the unread message count for each tag.
> > > > 
> > > 
> > > Yow, that is a big patch, I think it needs some more review.
> > 
> > Yes; Unfortunately I didn't manage to come up with good partitioning
> > into smaller patches, because the changes are quite interconnected.
> > 
> > Is there something I can do to help with the review?
> > 
> 
> Looking at earlier threads, I found an unanswered email from Austin [1].
> IMO it has some good ideas and I would like to see your answer to it.

It seems to me that all Austin's ideas are already implemented in the
later versions of the patch.

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


Re: [PATCH v3 00/7] test: (hopefully) better test prerequisites

2011-11-27 Thread David Bremner
On Sun, 27 Nov 2011 22:36:12 +0400, Dmitry Kurochkin 
 wrote:
> Changes:
> 
> v3 since v2:
> 
> * rebased the remaining patches on current master

These are all pushed to master now.

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


[PATCH v3 00/7] test: (hopefully) better test prerequisites

2011-11-27 Thread David Bremner
On Sun, 27 Nov 2011 22:36:12 +0400, Dmitry Kurochkin  wrote:
> Changes:
> 
> v3 since v2:
> 
> * rebased the remaining patches on current master

These are all pushed to master now.

d


[PATCH 0/3] emacs: do not call notmuch show for non-inlinable parts

2011-11-27 Thread Austin Clements
LGTM


[PATCH v3 7/7] test: fix "Stashing in notmuch-search" test when emacs is not available

2011-11-27 Thread Dmitry Kurochkin
If emacs is not available, test_expect_equal would be called with only
one argument.  The patch fixes this by quoting the (possibly empty)
$(cat OUTPUT) argument.
---
 test/emacs |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/emacs b/test/emacs
index 198c27b..3f8c72d 100755
--- a/test/emacs
+++ b/test/emacs
@@ -412,7 +412,7 @@ test_emacs '(notmuch-search "id:\"bought\"")
 (yank)
(test-output)'
 sed -i -e 's/^thread:.*$/thread:XXX/' OUTPUT
-test_expect_equal $(cat OUTPUT) "thread:XXX"
+test_expect_equal "$(cat OUTPUT)" "thread:XXX"

 test_begin_subtest 'Hiding message following HTML part'
 test_subtest_known_broken
-- 
1.7.7.3



[PATCH v3 6/7] test: check if emacs is available in the beginning of test_emacs

2011-11-27 Thread Dmitry Kurochkin
Unfortunately, this is needed to avoid the emacs waiting loop.
---
 test/test-lib.sh |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 2422e32..11e6646 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -894,6 +894,10 @@ EOF
 }

 test_emacs () {
+   # test dependencies beforehand to avoid the waiting loop below
+   test_require_external_prereq emacs || return
+   test_require_external_prereq emacsclient || return
+
if [ -z "$EMACS_SERVER" ]; then
server_name="notmuch-test-suite-$$"
# start a detached session with an emacs server
-- 
1.7.7.3



[PATCH v3 5/7] test: add function to explicitly check for external dependencies

2011-11-27 Thread Dmitry Kurochkin
Useful when binary is called indirectly (e.g. from emacs).
---
 test/test-lib.sh |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index fe80e89..2422e32 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -555,12 +555,26 @@ test_declare_external_prereq () {

hash $binary 2>/dev/null || eval "
 $binary () {
+   test_missing_external_prereq_${binary}_=t
echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name 
\" ||

test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
 $name\"
false
 }"
 }

+# Explicitly require external prerequisite.  Useful when binary is
+# called indirectly (e.g. from emacs).
+# Returns success if dependency is available, failure otherwise.
+test_require_external_prereq () {
+   binary="$1"
+   if [ "$(eval echo -n \$test_missing_external_prereq_${binary}_)" = t ]; 
then
+   # dependency is missing, call the replacement function to note 
it
+   eval "$binary"
+   else
+   true
+   fi
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.

-- 
1.7.7.3



[PATCH v3 4/7] test: declare external dependencies for the tests

2011-11-27 Thread Dmitry Kurochkin
That are: dtach(1), emacs(1), emacsclient(1), gdb(1) and gpg(1).
---
 test/test-lib.sh |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 99b9e8b..fe80e89 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1123,3 +1123,10 @@ test -z "$NO_PYTHON" && test_set_prereq PYTHON
 # test whether the filesystem supports symbolic links
 ln -s x y 2>/dev/null && test -h y 2>/dev/null && test_set_prereq SYMLINKS
 rm -f y
+
+# declare prerequisites for external binaries used in tests
+test_declare_external_prereq dtach
+test_declare_external_prereq emacs
+test_declare_external_prereq emacsclient
+test_declare_external_prereq gdb
+test_declare_external_prereq gpg
-- 
1.7.7.3



[PATCH v3 3/7] test: skip all subtests if external dependencies are missing during init

2011-11-27 Thread Dmitry Kurochkin
Some tests (e.g. crypto) do a common initialization required for all
subtests.  The patch adds a check for missing external dependencies
during this initialization.  If any prerequisites are missing, all
subtests are skipped.

The check is run on the first call of test_reset_state_ function, so
no changes for the tests are needed.
---
 test/test-lib.sh |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index df867a5..99b9e8b 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -902,10 +902,20 @@ test_emacs () {
 }

 test_reset_state_ () {
+   test -z "$test_init_done_" && test_init_
+
test_subtest_known_broken_=
test_subtest_missing_external_prereqs_=
 }

+# called once before the first subtest
+test_init_ () {
+   test_init_done_=t
+
+   # skip all tests if there were external prerequisites missing during 
init
+   test_check_missing_external_prereqs_ "all tests in $this_test" && 
test_done
+}
+

 find_notmuch_path ()
 {
-- 
1.7.7.3



[PATCH v3 2/7] test: fix "skipping test" verbose output

2011-11-27 Thread Dmitry Kurochkin
---
 test/test-lib.sh |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0996a74..df867a5 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -657,7 +657,8 @@ test_check_missing_external_prereqs_ () {

 test_report_skip_ () {
test_reset_state_
-   say_color skip >&3 "skipping test: $@"
+   say_color skip >&3 "skipping test:"
+   echo " $@" >&3
say_color skip "%-6s" "SKIP"
echo " $1"
 }
-- 
1.7.7.3



[PATCH v3 1/7] test: add support for external executable dependencies

2011-11-27 Thread Dmitry Kurochkin
There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.

The patch aims to add better support for a particular type of external
dependencies: external executables.  The main idea is to replace
missing external binaries with shell functions that have the same
name.  These functions always fail and keep track of missing
dependencies for a subtest.  The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL.  The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
 test/test-lib.sh |   49 +
 1 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 076f929..0996a74 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -548,6 +548,19 @@ test_have_prereq () {
esac
 }

+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+   binary="$1"
+   test "$#" = 2 && name=$2 || name="$binary(1)"
+
+   hash $binary 2>/dev/null || eval "
+$binary () {
+   echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name 
\" ||
+   
test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
 $name\"
+   false
+}"
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.

@@ -624,18 +637,31 @@ test_skip () {
fi
case "$to_skip" in
t)
-   test_reset_state_
-   say_color skip >&3 "skipping test: $@"
-   say_color skip "%-6s" "SKIP"
-   echo " $1"
-   : true
+   test_report_skip_ "$@"
;;
*)
-   false
+   test_check_missing_external_prereqs_ "$@"
;;
esac
 }

+test_check_missing_external_prereqs_ () {
+   if test -n "$test_subtest_missing_external_prereqs_"; then
+   say_color skip >&3 "missing prerequisites:"
+   echo "$test_subtest_missing_external_prereqs_" >&3
+   test_report_skip_ "$@"
+   else
+   false
+   fi
+}
+
+test_report_skip_ () {
+   test_reset_state_
+   say_color skip >&3 "skipping test: $@"
+   say_color skip "%-6s" "SKIP"
+   echo " $1"
+}
+
 test_subtest_known_broken () {
test_subtest_known_broken_=t
 }
@@ -648,7 +674,10 @@ test_expect_success () {
if ! test_skip "$@"
then
test_run_ "$2"
-   if [ "$?" = 0 -a "$eval_ret" = 0 ]
+   run_ret="$?"
+   # test_run_ may update missing external prerequisites
+   test_check_missing_external_prereqs_ "$@" ||
+   if [ "$run_ret" = 0 -a "$eval_ret" = 0 ]
then
test_ok_ "$1"
else
@@ -665,7 +694,10 @@ test_expect_code () {
if ! test_skip "$@"
then
test_run_ "$3"
-   if [ "$?" = 0 -a "$eval_ret" = "$1" ]
+   run_ret="$?"
+   # test_run_ may update missing external prerequisites,
+   test_check_missing_external_prereqs_ "$@" ||
+   if [ "$run_ret" = 0 -a "$eval_ret" = "$1" ]
then
test_ok_ "$2"
else
@@ -870,6 +902,7 @@ test_emacs () {

 test_reset_state_ () {
test_subtest_known_broken_=
+   test_subtest_missing_external_prereqs_=
 }


-- 
1.7.7.3



[PATCH v3 00/7] test: (hopefully) better test prerequisites

2011-11-27 Thread Dmitry Kurochkin
Changes:

v3 since v2:

* rebased the remaining patches on current master

v2 since v1:

* Add test_require_external_prereq function to explicitly check for
  external dependencies, use it in test_emacs.

* Indenting fixes.

* Use $binary instead of $1 in test_declare_external_prereq.

Regards,
  Dmitry



[PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

2011-11-27 Thread Dmitry Kurochkin
On Sat, 26 Nov 2011 22:14:20 +0100, Amadeusz ?o?nowski  wrote:
> If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
> it's not so easy to pass $(srcdir) which is required to find notmuch.h
> when srcdir and builddir are separate directories.
> ---
>  test/.gitignore |1 +
>  test/Makefile.local |7 +--
>  test/basic  |2 +-
>  test/symbol-hiding  |3 +--
>  4 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/test/.gitignore b/test/.gitignore
> index 9e97052..7e30e8d 100644
> --- a/test/.gitignore
> +++ b/test/.gitignore
> @@ -1,4 +1,5 @@
>  test-results
>  corpus.mail
>  smtp-dummy
> +symbol-test
>  tmp.*
> diff --git a/test/Makefile.local b/test/Makefile.local
> index 9b3308a..646779e 100644
> --- a/test/Makefile.local
> +++ b/test/Makefile.local
> @@ -11,10 +11,13 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
>  $(dir)/smtp-dummy: $(smtp_dummy_modules)
>   $(call quiet,CC) $^ -o $@
>  
> +$(dir)/symbol-test: $(dir)/symbol-test.o
> + $(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
> +
>  .PHONY: test check
> -test:all $(dir)/smtp-dummy
> +test:all $(dir)/smtp-dummy $(dir)/symbol-test
>   @${dir}/notmuch-test $(OPTIONS)
>  
>  check: test
>  
> -CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o
> +CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o $(dir)/symbol-test 
> $(dir)/symbol-test.o
> diff --git a/test/basic b/test/basic
> index f258d1f..4edf831 100755
> --- a/test/basic
> +++ b/test/basic
> @@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be 
> run by notmuch-test'
>  eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
>  tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
>  available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf 
> '%f\n' | \
> -sed -r -e 
> "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose)$/d" | \
> +sed -r -e 
> "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d"
>  | \

Can you please keep this list alphabetically sorted?

Regards,
  Dmitry

>  sort)
>  test_expect_equal "$tests_in_suite" "$available"
>  
> diff --git a/test/symbol-hiding b/test/symbol-hiding
> index d0b31ae..f67b653 100755
> --- a/test/symbol-hiding
> +++ b/test/symbol-hiding
> @@ -12,13 +12,12 @@ test_description='exception symbol hiding'
>  . ./test-lib.sh
>  
>  run_test(){
> -result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
> +result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib 
> $TEST_DIRECTORY/symbol-test 2>&1)
>  }
>  
>  output="A Xapian exception occurred opening database: Couldn't stat 
> 'fakedb/.notmuch/xapian'
>  caught No chert database found at path \`./nonexistant'"
>  
> -g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc 
> -L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
>  mkdir -p fakedb/.notmuch
>  test_expect_success 'running test' run_test
>  test_begin_subtest 'checking output'
> -- 
> 1.7.8.rc3
> 
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/3] emacs: do not call notmuch show for non-inlinable parts

2011-11-27 Thread Austin Clements
LGTM.  We should probably be paying attention to the content
disposition, but this is a definite improvement on what we do now.

As an interesting side-note, this lets us inline a handful of types that
we previously wouldn't, like application/emacs-lisp.

On Sat, 26 Nov 2011 05:44:38 +0400, Dmitry Kurochkin  wrote:
> Before the change, there was a workaround to avoid notmuch show calls
> for parts with application/* Content-Type.  But non-inlinable parts
> are not limited to this Content-Type (e.g. mp3 files have audio/mpeg
> Content-Type and are not inlinable).  For such parts
> `notmuch-show-insert-part-*/*' handler is called which unconditionally
> fetches contents for all parts.
> 
> The patch moves content fetching from `notmuch-show-insert-part-*/*'
> to `notmuch-show-mm-display-part-inline' function after MIME inlinable
> checks are done to avoid useless notmuch show calls.  The
> application/* hack is no longer needed and removed.
> ---
>  emacs/notmuch-show.el |   17 +
>  test/emacs|1 -
>  2 files changed, 5 insertions(+), 13 deletions(-)

Yay!


[PATCH 2/3] test: check that Emacs UI does not call notmuch for non-inlinable parts

2011-11-27 Thread Austin Clements
LGTM (modulo any changes that propagate from the first patch)

On Sat, 26 Nov 2011 05:44:37 +0400, Dmitry Kurochkin  wrote:
> The patch adds two new test cases:
> 
> * Do not call notmuch for non-inlinable application/mpeg parts
> * Do not call notmuch for non-inlinable audio/mpeg parts
> 
> The application/mpeg test passes thanks to a workaround for
> application/* Content-Types.  The audio/mpeg is currently broken.


[PATCH 1/3] test: add functions to count how much times notmuch was called

2011-11-27 Thread Austin Clements
On Sat, 26 Nov 2011 05:44:36 +0400, Dmitry Kurochkin  wrote:
> The patch adds two auxiliary functions and a variable:
> 
>   notmuch_counter_reset
>   $notmuch_counter
>   notmuch_counter
> 
> They allow to count how many times notmuch binary is called.
> notmuch_counter_reset() function generates a script that counts how
> many times it is called and resets the counter to zero.  The function
> sets $notmuch_counter variable to the path to the generated script
> that should be called instead of notmuch to do the counting.  The
> notmuch_counter() function returns the current counter value.

Great idea!

> ---
>  test/README  |   16 ++--
>  test/test-lib.sh |   32 
>  2 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/test/README b/test/README
> index 2481f16..1570f7c 100644
> --- a/test/README
> +++ b/test/README
> @@ -187,8 +187,8 @@ library for your script to use.
> is to summarize successes and failures in the test script and
> exit with an appropriate error code.
>  
> -There are also a number of mail-specific functions which are useful in
> -writing tests:
> +There are also a number of notmuch-specific auxiliary functions and
> +variables which are useful in writing tests:
>  
>generate_message
>  
> @@ -212,3 +212,15 @@ writing tests:
>  will initialize the mail database to a known state of 50 sample
>  messages, (culled from the early history of the notmuch mailing
>  list).
> +
> +  notmuch_counter_reset
> +  $notmuch_counter
> +  notmuch_counter

>From the name, I would expect $notmuch_counter to be the counter value
(ignoring the difficulty of implementing that).  Perhaps
$notmuch_counter_command, since it's meant to be bound to
notmuch-command?

notmuch_counter is okay, but maybe notmuch_counter_value?

> +
> +These allow to count how many times notmuch binary is called.
> +notmuch_counter_reset() function generates a script that counts
> +how many times it is called and resets the counter to zero.  The
> +function sets $notmuch_counter variable to the path to the
> +generated script that should be called instead of notmuch to do
> +the counting.  The notmuch_counter() function returns the current
> +counter value.

It doesn't return the counter value, it prints it.

> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 93867b0..e3b85d0 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -864,6 +864,38 @@ test_emacs () {
>   emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
>  }
>  
> +# Creates a script that counts how much time it is executed and calls
> +# notmuch.  $notmuch_counter is set to the path to the generated
> +# script.  Use notmuch_counter() function to get the current counter
> +# value.
> +notmuch_counter_reset () {
> + notmuch_counter="$TMP_DIRECTORY/notmuch_counter"
> + if [ ! -x "$notmuch_counter" ]; then
> + 
> notmuch_counter_state_path="$TMP_DIRECTORY/notmuch_counter.state"
> + cat <"$notmuch_counter"

This is totally a stylistic pet peeve, but consider
  cat >"$notmuch_counter" < +#!/bin/sh
> +
> +count=\$(cat "$notmuch_counter_state_path")
> +echo -n \$(expr \$count + 1) > "$notmuch_counter_state_path"
> +
> +exec notmuch "\$@"
> +EOF
> + chmod +x "$notmuch_counter" || return
> + fi
> +
> + echo -n 0 > "$notmuch_counter_state_path" || return

What are the || return's for?

> +}
> +
> +# Returns the current notmuch counter value.
> +notmuch_counter () {
> + if [ -r "$notmuch_counter_state_path" ]; then
> + count=$(cat "$notmuch_counter_state_path")
> + else
> + count=0
> + fi
> + echo -n $count
> +}
> +
>  
>  find_notmuch_path ()
>  {
> -- 
> 1.7.7.3
> 
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
> 


[PATCH 1/2] emacs: remove some code duplication in notmuch-show

2011-11-27 Thread Austin Clements
Both patches look good to me.

(I don't think the compiler could warn about the unused `headers'
binding even if it wanted to because of dynamic scoping.)


[PATCH 4/4] show: Rewrite show_message_body to use the MIME tree interface.

2011-11-27 Thread Austin Clements
This removes all of the MIME traversal logic from show_message_body
and leaves only its interaction with the format callbacks.

Besides isolating concerns, since traversal happens behind a trivial
interface, there is now much less code duplication in
show_message_part.  Also, this uses mime_node_seek_dfs to start at the
requested part, eliminating all of the logic about parts being
selected or being in_zone (and reducing the "show state" to only a
part counter).  notmuch_show_params_t no longer needs to be passed
through the recursion because the only two fields that mattered
(related to crypto) are now handled by the MIME tree.

The few remaining complexities in show_message_part highlight
irregularities in the format callbacks with respect to top-level
messages and embedded message parts.

Since this is a rewrite, the diff is not very enlightening.  It's
easier to look at the old code and the new code side-by-side.
---
 show-message.c |  229 +---
 1 files changed, 52 insertions(+), 177 deletions(-)

diff --git a/show-message.c b/show-message.c
index 09fa607..4560aea 100644
--- a/show-message.c
+++ b/show-message.c
@@ -24,154 +24,54 @@

 typedef struct show_message_state {
 int part_count;
-int in_zone;
 } show_message_state_t;

 static void
-show_message_part (GMimeObject *part,
+show_message_part (mime_node_t *node,
   show_message_state_t *state,
   const notmuch_show_format_t *format,
-  notmuch_show_params_t *params,
   int first)
 {
-GMimeObject *decryptedpart = NULL;
-int selected;
+/* Formatters expect the envelope for embedded message parts */
+GMimeObject *part = node->envelope_part ?
+   GMIME_OBJECT (node->envelope_part) : node->part;
+int i;
+
+if (!first)
+   fputs (format->part_sep, stdout);
+
+/* Format this part */
+if (format->part_start)
+   format->part_start (part, &(state->part_count));
+
+if (node->is_encrypted && format->part_encstatus)
+   format->part_encstatus (node->decrypt_success);
+
+if (node->is_signed && format->part_sigstatus)
+   format->part_sigstatus (node->sig_validity);
+
+format->part_content (part);
+
+if (node->envelope_part) {
+   fputs (format->header_start, stdout);
+   if (format->header_message_part)
+   format->header_message_part (GMIME_MESSAGE (node->part));
+   fputs (format->header_end, stdout);
+
+   fputs (format->body_start, stdout);
+}
+
+/* Recurse over the children */
 state->part_count += 1;
+for (i = 0; i < node->children; i++)
+   show_message_part (mime_node_child (node, i), state, format, i == 0);

-if (! (GMIME_IS_PART (part) || GMIME_IS_MULTIPART (part) || 
GMIME_IS_MESSAGE_PART (part))) {
-   fprintf (stderr, "Warning: Not displaying unknown mime part: %s.\n",
-g_type_name (G_OBJECT_TYPE (part)));
-   return;
-}
+/* Finish this part */
+if (node->envelope_part)
+   fputs (format->body_end, stdout);

-selected = (params->part <= 0 || state->part_count == params->part);
-
-if (selected || state->in_zone) {
-   if (!first && (params->part <= 0 || state->in_zone))
-   fputs (format->part_sep, stdout);
-
-   if (format->part_start)
-   format->part_start (part, &(state->part_count));
-}
-
-/* handle PGP/MIME parts */
-if (GMIME_IS_MULTIPART (part) && params->cryptoctx) {
-   GMimeMultipart *multipart = GMIME_MULTIPART (part);
-   GError* err = NULL;
-
-   if (GMIME_IS_MULTIPART_ENCRYPTED (part) && params->decrypt)
-   {
-   if ( g_mime_multipart_get_count (multipart) != 2 ) {
-   /* this violates RFC 3156 section 4, so we won't bother with 
it. */
-   fprintf (stderr,
-"Error: %d part(s) for a multipart/encrypted message 
(should be exactly 2)\n",
-g_mime_multipart_get_count (multipart));
-   } else {
-   GMimeMultipartEncrypted *encrypteddata = 
GMIME_MULTIPART_ENCRYPTED (part);
-   decryptedpart = g_mime_multipart_encrypted_decrypt 
(encrypteddata, params->cryptoctx, &err);
-   if (decryptedpart) {
-   if ((selected || state->in_zone) && format->part_encstatus)
-   format->part_encstatus (1);
-   const GMimeSignatureValidity *sigvalidity = 
g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
-   if (!sigvalidity)
-   fprintf (stderr, "Failed to verify signed part: %s\n", 
(err ? err->message : "no error explanation given"));
-   if ((selected || state->in_zone) && format->part_sigstatus)
-   format->part_sigstatus (sigvalidity);
-   } else {
-   fprintf (stderr, "Failed to decrypt part: %s\n", (err ? 
err->message : "no erro

[PATCH 3/4] Utility function to seek in MIME trees in depth-first order.

2011-11-27 Thread Austin Clements
This function matches how we number parts for the --part argument to
show.  It will allow us to jump directly to the desired part, rather
than traversing the entire tree and carefully tracking whether or not
we're "in the zone".
---
 mime-node.c  |   25 +
 notmuch-client.h |5 +
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 942738b..40fff75 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -232,3 +232,28 @@ mime_node_child (const mime_node_t *parent, int child)
g_type_name (G_OBJECT_TYPE (parent->part)));
 }
 }
+
+static mime_node_t *
+_mime_node_seek_dfs_walk (mime_node_t *node, int *n)
+{
+mime_node_t *ret = NULL;
+int i;
+
+if (*n <= 0)
+   return node;
+
+*n = *n - 1;
+for (i = 0; i < node->children && !ret; i++) {
+   mime_node_t *child = mime_node_child (node, i);
+   ret = _mime_node_seek_dfs_walk (child, n);
+   if (!ret)
+   talloc_free (child);
+}
+return ret;
+}
+
+mime_node_t *
+mime_node_seek_dfs (mime_node_t *node, int n)
+{
+return _mime_node_seek_dfs_walk (node, &n);
+}
diff --git a/notmuch-client.h b/notmuch-client.h
index 58bd21c..8b1454f 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -318,4 +318,9 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
 mime_node_t *
 mime_node_child (const mime_node_t *parent, int child);

+/* Return the nth child of node in a depth-first traversal.  If n is
+ * 0, returns node itself.  Returns NULL if there is no such part. */
+mime_node_t *
+mime_node_seek_dfs (mime_node_t *node, int n);
+
 #endif
-- 
1.7.5.4



[PATCH 2/4] Introduce a generic tree-like abstraction for MIME traversal.

2011-11-27 Thread Austin Clements
This wraps all of the complex MIME part handling in a single, simple
function that gets part N from *any* MIME object, so traversing a MIME
part tree becomes a two-line for loop.  Furthermore, the MIME node
structure provides easy access to envelopes for message parts as well
as cryptographic information.

This code is directly derived from the current show_message_body code
(much of it is identical), but the control relation is inverted:
instead of show_message_body controlling the traversal of the MIME
structure and invoking callbacks, the caller controls the traversal of
the MIME structure.
---
 Makefile.local   |1 +
 mime-node.c  |  234 ++
 notmuch-client.h |   80 ++
 3 files changed, 315 insertions(+), 0 deletions(-)
 create mode 100644 mime-node.c

diff --git a/Makefile.local b/Makefile.local
index c94402b..c46ed26 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -312,6 +312,7 @@ notmuch_client_srcs =   \
notmuch-time.c  \
query-string.c  \
show-message.c  \
+   mime-node.c \
json.c

 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
diff --git a/mime-node.c b/mime-node.c
new file mode 100644
index 000..942738b
--- /dev/null
+++ b/mime-node.c
@@ -0,0 +1,234 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright ? 2009 Carl Worth
+ * Copyright ? 2009 Keith Packard
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Authors: Carl Worth 
+ *  Keith Packard 
+ *  Austin Clements 
+ */
+
+#include "notmuch-client.h"
+
+/* Context that gets inherited from the root node. */
+typedef struct mime_node_context {
+/* Per-message resources.  These are allocated internally and must
+ * be destroyed. */
+FILE *file;
+GMimeStream *stream;
+GMimeParser *parser;
+GMimeMessage *mime_message;
+
+/* Context provided by the caller. */
+GMimeCipherContext *cryptoctx;
+notmuch_bool_t decrypt;
+} mime_node_context_t;
+
+static int
+_mime_node_context_free (mime_node_context_t *res)
+{
+if (res->mime_message)
+   g_object_unref (res->mime_message);
+
+if (res->parser)
+   g_object_unref (res->parser);
+
+if (res->stream)
+   g_object_unref (res->stream);
+
+if (res->file)
+   fclose (res->file);
+
+return 0;
+}
+
+notmuch_status_t
+mime_node_open (const void *ctx, notmuch_message_t *message,
+   GMimeCipherContext* cryptoctx, notmuch_bool_t decrypt,
+   mime_node_t **root_out)
+{
+const char *filename = notmuch_message_get_filename (message);
+mime_node_context_t *mctx;
+mime_node_t *root = NULL;
+notmuch_status_t status;
+
+root = talloc_zero (ctx, mime_node_t);
+if (root == NULL) {
+   fprintf (stderr, "Out of memory.\n");
+   status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+   goto DONE;
+}
+
+/* Create the tree-wide context */
+mctx = talloc_zero (root, mime_node_context_t);
+if (mctx == NULL) {
+   fprintf (stderr, "Out of memory.\n");
+   status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+   goto DONE;
+}
+talloc_set_destructor (mctx, _mime_node_context_free);
+
+mctx->file = fopen (filename, "r");
+if (! mctx->file) {
+   fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
+   status = NOTMUCH_STATUS_FILE_ERROR;
+   goto DONE;
+}
+
+mctx->stream = g_mime_stream_file_new (mctx->file);
+g_mime_stream_file_set_owner (GMIME_STREAM_FILE (mctx->stream), FALSE);
+
+mctx->parser = g_mime_parser_new_with_stream (mctx->stream);
+
+mctx->mime_message = g_mime_parser_construct_message (mctx->parser);
+
+mctx->cryptoctx = cryptoctx;
+mctx->decrypt = decrypt;
+
+/* Create the root node */
+root->part = GMIME_OBJECT (mctx->mime_message);
+root->envelope_file = message;
+root->children = 1;
+root->ctx = mctx;
+
+*root_out = root;
+return NOTMUCH_STATUS_SUCCESS;
+
+DONE:
+talloc_free (root);
+return status;
+}
+
+static int
+_signature_validity_free (GMimeSignatureValidity **proxy)
+{
+g_mime_signature_validity_free (*proxy);
+return 0;
+}
+
+static mime_node_t *
+_mime_node_create (const mime_node_t *parent, GMimeObject *part)
+{
+mime_node_t *out =

[PATCH 1/4] show: Pass notmuch_message_t instead of path to show_message_body.

2011-11-27 Thread Austin Clements
In addition to simplifying the code, we'll need the notmuch_message_t*
in show_message_body shortly.
---
 notmuch-client.h |2 +-
 notmuch-reply.c  |3 +--
 notmuch-show.c   |3 +--
 show-message.c   |3 ++-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index b50cb38..d7fb6ee 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -162,7 +162,7 @@ char *
 query_string_from_args (void *ctx, int argc, char *argv[]);

 notmuch_status_t
-show_message_body (const char *filename,
+show_message_body (notmuch_message_t *message,
   const notmuch_show_format_t *format,
   notmuch_show_params_t *params);

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 7ac879f..f8d5f64 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -546,8 +546,7 @@ notmuch_reply_format_default(void *ctx,
notmuch_message_get_header (message, "date"),
notmuch_message_get_header (message, "from"));

-   show_message_body (notmuch_message_get_filename (message),
-  format, params);
+   show_message_body (message, format, params);

notmuch_message_destroy (message);
 }
diff --git a/notmuch-show.c b/notmuch-show.c
index 603992a..1dee3aa 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -753,8 +753,7 @@ show_message (void *ctx,
 }

 if (format->part_content)
-   show_message_body (notmuch_message_get_filename (message),
-  format, params);
+   show_message_body (message, format, params);

 if (params->part <= 0) {
fputs (format->body_end, stdout);
diff --git a/show-message.c b/show-message.c
index d83f04e..09fa607 100644
--- a/show-message.c
+++ b/show-message.c
@@ -175,7 +175,7 @@ show_message_part (GMimeObject *part,
 }

 notmuch_status_t
-show_message_body (const char *filename,
+show_message_body (notmuch_message_t *message,
   const notmuch_show_format_t *format,
   notmuch_show_params_t *params)
 {
@@ -183,6 +183,7 @@ show_message_body (const char *filename,
 GMimeParser *parser = NULL;
 GMimeMessage *mime_message = NULL;
 notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
+const char *filename = notmuch_message_get_filename (message);
 FILE *file = NULL;
 show_message_state_t state;

-- 
1.7.5.4



[PATCH 0/4] First step of 'show' rewrite

2011-11-27 Thread Austin Clements
This is the first step in my rewrite of notmuch-show.  Currently, MIME
traversal is controlled by show_message_body, which invokes formatter
callbacks.  As a result, formatters have no control over traversal,
which complicates formatters and makes others (like raw) nigh
impossible to implement correctly.  The callback-driven approach
forces formatters to be written in many small pieces and masks the
true control flow.  It also results in an wide interface with
show_message_body that has been getting wider over time.

The goal of this rewrite is to reverse this relationship, so that
formatters control MIME traversal.  This lets formatters traverse the
MIME in the most appropriate way, eliminates the callbacks, makes the
control flow through a formatter clear, and dramatically narrows
interfaces.

This first series doesn't change the way formatters work, but it does
introduce the MIME-traversal API they'll use.  This API consists of a
total of two functions: one to get the root MIME part of a message and
one to get a child of a MIME part.  This allows basic MIME traversal
to be implemented in a simple, two-line for loop.  The series also
updates show_message_body to use this new API, though ultimately
show_message_body will go away entirely.



Re: [PATCH v6 1/2] emacs: User-defined sections in notmuch-hello

2011-11-27 Thread Dmitry Kurochkin
Hi Daniel.

On Thu, 24 Nov 2011 15:01:01 +0100, Daniel Schoepe  wrote:
> On Thu, 24 Nov 2011 09:54:50 -0400, David Bremner  wrote:
> > On Mon, 10 Oct 2011 15:39:41 +0200, Daniel Schoepe  
> > wrote:
> > > From: Daniel Schoepe 
> > > 
> > > This patch makes the notmuch-hello screen fully customizable
> > > by allowing the user to add and remove arbitrary sections. It
> > > also provides some convenience functions for constructing sections,
> > > e.g. showing the unread message count for each tag.
> > > 
> > 
> > Yow, that is a big patch, I think it needs some more review.
> 
> Yes; Unfortunately I didn't manage to come up with good partitioning
> into smaller patches, because the changes are quite interconnected.
> 
> Is there something I can do to help with the review?
> 

Looking at earlier threads, I found an unanswered email from Austin [1].
IMO it has some good ideas and I would like to see your answer to it.

Sorry if I just missed it.  Would appreciate a link if that is the case.

Regards,
  Dmitry

[1] id:"BANLkTinKCLg1epsGvfxtyy-_G2Hm=gj...@mail.gmail.com"

> I think there's also one performance improvement written by Michal
> Sojka, that isn't included in this version yet, but since patches are
> getting applied more timely now, I guess that can be merged afterwards.
> 
> Cheers,
> Daniel
> ___
> 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 0/3] emacs: do not call notmuch show for non-inlinable parts

2011-11-27 Thread Austin Clements
LGTM
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/3] emacs: do not call notmuch show for non-inlinable parts

2011-11-27 Thread Dmitry Kurochkin
Before the change, there was a workaround to avoid notmuch show calls
for parts with application/* Content-Type.  But non-inlinable parts
are not limited to this Content-Type (e.g. mp3 files have audio/mpeg
Content-Type and are not inlinable).  For such parts
`notmuch-show-insert-part-*/*' handler is called which unconditionally
fetches contents for all parts.

The patch moves content fetching from `notmuch-show-insert-part-*/*'
to `notmuch-show-mm-display-part-inline' function after MIME inlinable
checks are done to avoid useless notmuch show calls.  The
application/* hack is no longer needed and removed.
---
 emacs/notmuch-show.el |   17 +
 test/emacs|1 -
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index d7fbbca..d2d4968 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -320,17 +320,17 @@ message at DEPTH in the current thread."
;; ange-ftp, which is reasonable to use here.
(mm-write-region (point-min) (point-max) file nil nil nil 
'no-conversion t)
 
-(defun notmuch-show-mm-display-part-inline (msg part content-type content)
+(defun notmuch-show-mm-display-part-inline (msg part nth content-type)
   "Use the mm-decode/mm-view functions to display a part in the
 current buffer, if possible."
   (let ((display-buffer (current-buffer)))
 (with-temp-buffer
-  (insert content)
   (let ((handle (mm-make-handle (current-buffer) (list content-type
-   (set-buffer display-buffer)
(if (and (mm-inlinable-p handle)
 (mm-inlined-p handle))
-   (progn
+   (let ((content (notmuch-show-get-bodypart-content msg part nth)))
+ (insert content)
+ (set-buffer display-buffer)
  (mm-display-part handle)
  t)
  nil)
@@ -588,17 +588,10 @@ current buffer, if possible."
nil))
  nil
 
-(defun notmuch-show-insert-part-application/* (msg part content-type nth depth 
declared-type
-)
-  ;; do not render random "application" parts
-  (notmuch-show-insert-part-header nth content-type declared-type (plist-get 
part :filename)))
-
 (defun notmuch-show-insert-part-*/* (msg part content-type nth depth 
declared-type)
   ;; This handler _must_ succeed - it is the handler of last resort.
   (notmuch-show-insert-part-header nth content-type declared-type (plist-get 
part :filename))
-  (let ((content (notmuch-show-get-bodypart-content msg part nth)))
-(if content
-   (notmuch-show-mm-display-part-inline msg part content-type content)))
+  (notmuch-show-mm-display-part-inline msg part nth content-type)
   t)
 
 ;; Functions for determining how to handle MIME parts.
diff --git a/test/emacs b/test/emacs
index 20f8449..56ca87b 100755
--- a/test/emacs
+++ b/test/emacs
@@ -488,7 +488,6 @@ test_emacs "(let ((notmuch-command 
\"$notmuch_counter_command\"))
 test_expect_equal $(notmuch_counter_value) 1
 
 test_begin_subtest "Do not call notmuch for non-inlinable audio/mpeg parts"
-test_subtest_known_broken
 id='message-with-audio/mpeg-attachm...@notmuchmail.org'
 emacs_deliver_message \
 'Message with audio/mpeg attachment' \
-- 
1.7.7.3

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


[PATCH 2/3] test: check that Emacs UI does not call notmuch for non-inlinable parts

2011-11-27 Thread Dmitry Kurochkin
The patch adds two new test cases:

* Do not call notmuch for non-inlinable application/mpeg parts
* Do not call notmuch for non-inlinable audio/mpeg parts

The application/mpeg test passes thanks to a workaround for
application/* Content-Types.  The audio/mpeg is currently broken.
---
 test/emacs |   31 +++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 198c27b..20f8449 100755
--- a/test/emacs
+++ b/test/emacs
@@ -472,4 +472,35 @@ test_emacs '(notmuch-show 
"id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
(test-visible-output)'
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Do not call notmuch for non-inlinable application/mpeg 
parts"
+id='message-with-application/mpeg-attachm...@notmuchmail.org'
+emacs_deliver_message \
+'Message with application/mpeg attachment' \
+'' \
+"(message-goto-eoh)
+ (insert \"Message-ID: <$id>\n\")
+ (message-goto-body)
+ (mml-insert-part \"application/mpeg\")
+ (insert \"a fake mp3 file\")"
+notmuch_counter_reset
+test_emacs "(let ((notmuch-command \"$notmuch_counter_command\"))
+ (notmuch-show \"id:$id\"))"
+test_expect_equal $(notmuch_counter_value) 1
+
+test_begin_subtest "Do not call notmuch for non-inlinable audio/mpeg parts"
+test_subtest_known_broken
+id='message-with-audio/mpeg-attachm...@notmuchmail.org'
+emacs_deliver_message \
+'Message with audio/mpeg attachment' \
+'' \
+"(message-goto-eoh)
+ (insert \"Message-ID: <$id>\n\")
+ (message-goto-body)
+ (mml-insert-part \"audio/mpeg\")
+ (insert \"a fake mp3 file\")"
+notmuch_counter_reset
+test_emacs "(let ((notmuch-command \"$notmuch_counter_command\"))
+ (notmuch-show \"id:$id\"))"
+test_expect_equal $(notmuch_counter_value) 1
+
 test_done
-- 
1.7.7.3

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


[PATCH 1/3] test: add functions to count how much times notmuch was called

2011-11-27 Thread Dmitry Kurochkin
The patch adds two auxiliary functions and a variable:

  notmuch_counter_reset
  $notmuch_counter_command
  notmuch_counter_value

They allow to count how many times notmuch binary is called.
notmuch_counter_reset() function generates a script that counts how
many times it is called and resets the counter to zero.  The function
sets $notmuch_counter_command variable to the path to the generated
script that should be called instead of notmuch to do the counting.
The notmuch_counter_value() function returns the current counter
value.
---
 test/README  |   16 ++--
 test/test-lib.sh |   32 
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/test/README b/test/README
index 2481f16..5dc0638 100644
--- a/test/README
+++ b/test/README
@@ -187,8 +187,8 @@ library for your script to use.
is to summarize successes and failures in the test script and
exit with an appropriate error code.
 
-There are also a number of mail-specific functions which are useful in
-writing tests:
+There are also a number of notmuch-specific auxiliary functions and
+variables which are useful in writing tests:
 
   generate_message
 
@@ -212,3 +212,15 @@ writing tests:
 will initialize the mail database to a known state of 50 sample
 messages, (culled from the early history of the notmuch mailing
 list).
+
+  notmuch_counter_reset
+  $notmuch_counter_command
+  notmuch_counter_value
+
+These allow to count how many times notmuch binary is called.
+notmuch_counter_reset() function generates a script that counts
+how many times it is called and resets the counter to zero.  The
+function sets $notmuch_counter_command variable to the path to the
+generated script that should be called instead of notmuch to do
+the counting.  The notmuch_counter_value() function prints the
+current counter value.
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 076f929..880bed9 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -868,6 +868,38 @@ test_emacs () {
emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 }
 
+# Creates a script that counts how much time it is executed and calls
+# notmuch.  $notmuch_counter_command is set to the path to the
+# generated script.  Use notmuch_counter_value() function to get the
+# current counter value.
+notmuch_counter_reset () {
+   notmuch_counter_command="$TMP_DIRECTORY/notmuch_counter"
+   if [ ! -x "$notmuch_counter_command" ]; then
+   
notmuch_counter_state_path="$TMP_DIRECTORY/notmuch_counter.state"
+   cat >"$notmuch_counter_command" < "$notmuch_counter_state_path"
+
+exec notmuch "\$@"
+EOF
+   chmod +x "$notmuch_counter_command" || return
+   fi
+
+   echo -n 0 > "$notmuch_counter_state_path"
+}
+
+# Returns the current notmuch counter value.
+notmuch_counter_value () {
+   if [ -r "$notmuch_counter_state_path" ]; then
+   count=$(cat "$notmuch_counter_state_path")
+   else
+   count=0
+   fi
+   echo -n $count
+}
+
 test_reset_state_ () {
test_subtest_known_broken_=
 }
-- 
1.7.7.3

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


[PATCH 0/3] emacs: do not call notmuch show for non-inlinable parts

2011-11-27 Thread Dmitry Kurochkin
This patch series has all comments from Austin's review [1] fixed.

Changes in v2 since v1:

* Rename $notmuch_counter variable to $notmuch_counter_command.
* Rename notmuch_counter() function to notmuch_counter_value().
* Fix documentation for notmuch_counter_value().
* Removed unneeded and add missing "|| return" in notmuch_counter_reset().
* Coding style fixes.

[1] id:"87ipm52cfs@awakening.csail.mit.edu"

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


Re: [PATCH 1/3] test: add functions to count how much times notmuch was called

2011-11-27 Thread Dmitry Kurochkin
On Sun, 27 Nov 2011 21:44:23 -0500, Austin Clements  wrote:
> On Sat, 26 Nov 2011 05:44:36 +0400, Dmitry Kurochkin 
>  wrote:
> > The patch adds two auxiliary functions and a variable:
> > 
> >   notmuch_counter_reset
> >   $notmuch_counter
> >   notmuch_counter
> > 
> > They allow to count how many times notmuch binary is called.
> > notmuch_counter_reset() function generates a script that counts how
> > many times it is called and resets the counter to zero.  The function
> > sets $notmuch_counter variable to the path to the generated script
> > that should be called instead of notmuch to do the counting.  The
> > notmuch_counter() function returns the current counter value.
> 
> Great idea!
> 
> > ---
> >  test/README  |   16 ++--
> >  test/test-lib.sh |   32 
> >  2 files changed, 46 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/README b/test/README
> > index 2481f16..1570f7c 100644
> > --- a/test/README
> > +++ b/test/README
> > @@ -187,8 +187,8 @@ library for your script to use.
> > is to summarize successes and failures in the test script and
> > exit with an appropriate error code.
> >  
> > -There are also a number of mail-specific functions which are useful in
> > -writing tests:
> > +There are also a number of notmuch-specific auxiliary functions and
> > +variables which are useful in writing tests:
> >  
> >generate_message
> >  
> > @@ -212,3 +212,15 @@ writing tests:
> >  will initialize the mail database to a known state of 50 sample
> >  messages, (culled from the early history of the notmuch mailing
> >  list).
> > +
> > +  notmuch_counter_reset
> > +  $notmuch_counter
> > +  notmuch_counter
> 
> From the name, I would expect $notmuch_counter to be the counter value
> (ignoring the difficulty of implementing that).  Perhaps
> $notmuch_counter_command, since it's meant to be bound to
> notmuch-command?
> 

renamed

> notmuch_counter is okay, but maybe notmuch_counter_value?
> 

renamed

> > +
> > +These allow to count how many times notmuch binary is called.
> > +notmuch_counter_reset() function generates a script that counts
> > +how many times it is called and resets the counter to zero.  The
> > +function sets $notmuch_counter variable to the path to the
> > +generated script that should be called instead of notmuch to do
> > +the counting.  The notmuch_counter() function returns the current
> > +counter value.
> 
> It doesn't return the counter value, it prints it.
> 

indeed, fixed

> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index 93867b0..e3b85d0 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -864,6 +864,38 @@ test_emacs () {
> > emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
> >  }
> >  
> > +# Creates a script that counts how much time it is executed and calls
> > +# notmuch.  $notmuch_counter is set to the path to the generated
> > +# script.  Use notmuch_counter() function to get the current counter
> > +# value.
> > +notmuch_counter_reset () {
> > +   notmuch_counter="$TMP_DIRECTORY/notmuch_counter"
> > +   if [ ! -x "$notmuch_counter" ]; then
> > +   
> > notmuch_counter_state_path="$TMP_DIRECTORY/notmuch_counter.state"
> > +   cat <"$notmuch_counter"
> 
> This is totally a stylistic pet peeve, but consider
>   cat >"$notmuch_counter" < 

fixed

> > +#!/bin/sh
> > +
> > +count=\$(cat "$notmuch_counter_state_path")
> > +echo -n \$(expr \$count + 1) > "$notmuch_counter_state_path"
> > +
> > +exec notmuch "\$@"
> > +EOF
> > +   chmod +x "$notmuch_counter" || return
> > +   fi
> > +
> > +   echo -n 0 > "$notmuch_counter_state_path" || return
> 
> What are the || return's for?
> 

If echo failed by some reason, that is an error.

Thank you for the reviews.  I will send amended series soon.

Regards,
  Dmitry

> > +}
> > +
> > +# Returns the current notmuch counter value.
> > +notmuch_counter () {
> > +   if [ -r "$notmuch_counter_state_path" ]; then
> > +   count=$(cat "$notmuch_counter_state_path")
> > +   else
> > +   count=0
> > +   fi
> > +   echo -n $count
> > +}
> > +
> >  
> >  find_notmuch_path ()
> >  {
> > -- 
> > 1.7.7.3
> > 
> > ___
> > 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 3/3] emacs: do not call notmuch show for non-inlinable parts

2011-11-27 Thread Austin Clements
LGTM.  We should probably be paying attention to the content
disposition, but this is a definite improvement on what we do now.

As an interesting side-note, this lets us inline a handful of types that
we previously wouldn't, like application/emacs-lisp.

On Sat, 26 Nov 2011 05:44:38 +0400, Dmitry Kurochkin 
 wrote:
> Before the change, there was a workaround to avoid notmuch show calls
> for parts with application/* Content-Type.  But non-inlinable parts
> are not limited to this Content-Type (e.g. mp3 files have audio/mpeg
> Content-Type and are not inlinable).  For such parts
> `notmuch-show-insert-part-*/*' handler is called which unconditionally
> fetches contents for all parts.
> 
> The patch moves content fetching from `notmuch-show-insert-part-*/*'
> to `notmuch-show-mm-display-part-inline' function after MIME inlinable
> checks are done to avoid useless notmuch show calls.  The
> application/* hack is no longer needed and removed.
> ---
>  emacs/notmuch-show.el |   17 +
>  test/emacs|1 -
>  2 files changed, 5 insertions(+), 13 deletions(-)

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


Re: [PATCH 2/3] test: check that Emacs UI does not call notmuch for non-inlinable parts

2011-11-27 Thread Austin Clements
LGTM (modulo any changes that propagate from the first patch)

On Sat, 26 Nov 2011 05:44:37 +0400, Dmitry Kurochkin 
 wrote:
> The patch adds two new test cases:
> 
> * Do not call notmuch for non-inlinable application/mpeg parts
> * Do not call notmuch for non-inlinable audio/mpeg parts
> 
> The application/mpeg test passes thanks to a workaround for
> application/* Content-Types.  The audio/mpeg is currently broken.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/3] test: add functions to count how much times notmuch was called

2011-11-27 Thread Austin Clements
On Sat, 26 Nov 2011 05:44:36 +0400, Dmitry Kurochkin 
 wrote:
> The patch adds two auxiliary functions and a variable:
> 
>   notmuch_counter_reset
>   $notmuch_counter
>   notmuch_counter
> 
> They allow to count how many times notmuch binary is called.
> notmuch_counter_reset() function generates a script that counts how
> many times it is called and resets the counter to zero.  The function
> sets $notmuch_counter variable to the path to the generated script
> that should be called instead of notmuch to do the counting.  The
> notmuch_counter() function returns the current counter value.

Great idea!

> ---
>  test/README  |   16 ++--
>  test/test-lib.sh |   32 
>  2 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/test/README b/test/README
> index 2481f16..1570f7c 100644
> --- a/test/README
> +++ b/test/README
> @@ -187,8 +187,8 @@ library for your script to use.
> is to summarize successes and failures in the test script and
> exit with an appropriate error code.
>  
> -There are also a number of mail-specific functions which are useful in
> -writing tests:
> +There are also a number of notmuch-specific auxiliary functions and
> +variables which are useful in writing tests:
>  
>generate_message
>  
> @@ -212,3 +212,15 @@ writing tests:
>  will initialize the mail database to a known state of 50 sample
>  messages, (culled from the early history of the notmuch mailing
>  list).
> +
> +  notmuch_counter_reset
> +  $notmuch_counter
> +  notmuch_counter

>From the name, I would expect $notmuch_counter to be the counter value
(ignoring the difficulty of implementing that).  Perhaps
$notmuch_counter_command, since it's meant to be bound to
notmuch-command?

notmuch_counter is okay, but maybe notmuch_counter_value?

> +
> +These allow to count how many times notmuch binary is called.
> +notmuch_counter_reset() function generates a script that counts
> +how many times it is called and resets the counter to zero.  The
> +function sets $notmuch_counter variable to the path to the
> +generated script that should be called instead of notmuch to do
> +the counting.  The notmuch_counter() function returns the current
> +counter value.

It doesn't return the counter value, it prints it.

> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 93867b0..e3b85d0 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -864,6 +864,38 @@ test_emacs () {
>   emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
>  }
>  
> +# Creates a script that counts how much time it is executed and calls
> +# notmuch.  $notmuch_counter is set to the path to the generated
> +# script.  Use notmuch_counter() function to get the current counter
> +# value.
> +notmuch_counter_reset () {
> + notmuch_counter="$TMP_DIRECTORY/notmuch_counter"
> + if [ ! -x "$notmuch_counter" ]; then
> + 
> notmuch_counter_state_path="$TMP_DIRECTORY/notmuch_counter.state"
> + cat <"$notmuch_counter"

This is totally a stylistic pet peeve, but consider
  cat >"$notmuch_counter" < +#!/bin/sh
> +
> +count=\$(cat "$notmuch_counter_state_path")
> +echo -n \$(expr \$count + 1) > "$notmuch_counter_state_path"
> +
> +exec notmuch "\$@"
> +EOF
> + chmod +x "$notmuch_counter" || return
> + fi
> +
> + echo -n 0 > "$notmuch_counter_state_path" || return

What are the || return's for?

> +}
> +
> +# Returns the current notmuch counter value.
> +notmuch_counter () {
> + if [ -r "$notmuch_counter_state_path" ]; then
> + count=$(cat "$notmuch_counter_state_path")
> + else
> + count=0
> + fi
> + echo -n $count
> +}
> +
>  
>  find_notmuch_path ()
>  {
> -- 
> 1.7.7.3
> 
> ___
> 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 1/2] emacs: remove some code duplication in notmuch-show

2011-11-27 Thread Austin Clements
Both patches look good to me.

(I don't think the compiler could warn about the unused `headers'
binding even if it wanted to because of dynamic scoping.)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 4/4] show: Rewrite show_message_body to use the MIME tree interface.

2011-11-27 Thread Austin Clements
This removes all of the MIME traversal logic from show_message_body
and leaves only its interaction with the format callbacks.

Besides isolating concerns, since traversal happens behind a trivial
interface, there is now much less code duplication in
show_message_part.  Also, this uses mime_node_seek_dfs to start at the
requested part, eliminating all of the logic about parts being
selected or being in_zone (and reducing the "show state" to only a
part counter).  notmuch_show_params_t no longer needs to be passed
through the recursion because the only two fields that mattered
(related to crypto) are now handled by the MIME tree.

The few remaining complexities in show_message_part highlight
irregularities in the format callbacks with respect to top-level
messages and embedded message parts.

Since this is a rewrite, the diff is not very enlightening.  It's
easier to look at the old code and the new code side-by-side.
---
 show-message.c |  229 +---
 1 files changed, 52 insertions(+), 177 deletions(-)

diff --git a/show-message.c b/show-message.c
index 09fa607..4560aea 100644
--- a/show-message.c
+++ b/show-message.c
@@ -24,154 +24,54 @@
 
 typedef struct show_message_state {
 int part_count;
-int in_zone;
 } show_message_state_t;
 
 static void
-show_message_part (GMimeObject *part,
+show_message_part (mime_node_t *node,
   show_message_state_t *state,
   const notmuch_show_format_t *format,
-  notmuch_show_params_t *params,
   int first)
 {
-GMimeObject *decryptedpart = NULL;
-int selected;
+/* Formatters expect the envelope for embedded message parts */
+GMimeObject *part = node->envelope_part ?
+   GMIME_OBJECT (node->envelope_part) : node->part;
+int i;
+
+if (!first)
+   fputs (format->part_sep, stdout);
+
+/* Format this part */
+if (format->part_start)
+   format->part_start (part, &(state->part_count));
+
+if (node->is_encrypted && format->part_encstatus)
+   format->part_encstatus (node->decrypt_success);
+
+if (node->is_signed && format->part_sigstatus)
+   format->part_sigstatus (node->sig_validity);
+
+format->part_content (part);
+
+if (node->envelope_part) {
+   fputs (format->header_start, stdout);
+   if (format->header_message_part)
+   format->header_message_part (GMIME_MESSAGE (node->part));
+   fputs (format->header_end, stdout);
+
+   fputs (format->body_start, stdout);
+}
+
+/* Recurse over the children */
 state->part_count += 1;
+for (i = 0; i < node->children; i++)
+   show_message_part (mime_node_child (node, i), state, format, i == 0);
 
-if (! (GMIME_IS_PART (part) || GMIME_IS_MULTIPART (part) || 
GMIME_IS_MESSAGE_PART (part))) {
-   fprintf (stderr, "Warning: Not displaying unknown mime part: %s.\n",
-g_type_name (G_OBJECT_TYPE (part)));
-   return;
-}
+/* Finish this part */
+if (node->envelope_part)
+   fputs (format->body_end, stdout);
 
-selected = (params->part <= 0 || state->part_count == params->part);
-
-if (selected || state->in_zone) {
-   if (!first && (params->part <= 0 || state->in_zone))
-   fputs (format->part_sep, stdout);
-
-   if (format->part_start)
-   format->part_start (part, &(state->part_count));
-}
-
-/* handle PGP/MIME parts */
-if (GMIME_IS_MULTIPART (part) && params->cryptoctx) {
-   GMimeMultipart *multipart = GMIME_MULTIPART (part);
-   GError* err = NULL;
-
-   if (GMIME_IS_MULTIPART_ENCRYPTED (part) && params->decrypt)
-   {
-   if ( g_mime_multipart_get_count (multipart) != 2 ) {
-   /* this violates RFC 3156 section 4, so we won't bother with 
it. */
-   fprintf (stderr,
-"Error: %d part(s) for a multipart/encrypted message 
(should be exactly 2)\n",
-g_mime_multipart_get_count (multipart));
-   } else {
-   GMimeMultipartEncrypted *encrypteddata = 
GMIME_MULTIPART_ENCRYPTED (part);
-   decryptedpart = g_mime_multipart_encrypted_decrypt 
(encrypteddata, params->cryptoctx, &err);
-   if (decryptedpart) {
-   if ((selected || state->in_zone) && format->part_encstatus)
-   format->part_encstatus (1);
-   const GMimeSignatureValidity *sigvalidity = 
g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
-   if (!sigvalidity)
-   fprintf (stderr, "Failed to verify signed part: %s\n", 
(err ? err->message : "no error explanation given"));
-   if ((selected || state->in_zone) && format->part_sigstatus)
-   format->part_sigstatus (sigvalidity);
-   } else {
-   fprintf (stderr, "Failed to decrypt part: %s\n", (err ? 
err->message : "no 

[PATCH 3/4] Utility function to seek in MIME trees in depth-first order.

2011-11-27 Thread Austin Clements
This function matches how we number parts for the --part argument to
show.  It will allow us to jump directly to the desired part, rather
than traversing the entire tree and carefully tracking whether or not
we're "in the zone".
---
 mime-node.c  |   25 +
 notmuch-client.h |5 +
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 942738b..40fff75 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -232,3 +232,28 @@ mime_node_child (const mime_node_t *parent, int child)
g_type_name (G_OBJECT_TYPE (parent->part)));
 }
 }
+
+static mime_node_t *
+_mime_node_seek_dfs_walk (mime_node_t *node, int *n)
+{
+mime_node_t *ret = NULL;
+int i;
+
+if (*n <= 0)
+   return node;
+
+*n = *n - 1;
+for (i = 0; i < node->children && !ret; i++) {
+   mime_node_t *child = mime_node_child (node, i);
+   ret = _mime_node_seek_dfs_walk (child, n);
+   if (!ret)
+   talloc_free (child);
+}
+return ret;
+}
+
+mime_node_t *
+mime_node_seek_dfs (mime_node_t *node, int n)
+{
+return _mime_node_seek_dfs_walk (node, &n);
+}
diff --git a/notmuch-client.h b/notmuch-client.h
index 58bd21c..8b1454f 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -318,4 +318,9 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
 mime_node_t *
 mime_node_child (const mime_node_t *parent, int child);
 
+/* Return the nth child of node in a depth-first traversal.  If n is
+ * 0, returns node itself.  Returns NULL if there is no such part. */
+mime_node_t *
+mime_node_seek_dfs (mime_node_t *node, int n);
+
 #endif
-- 
1.7.5.4

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


[PATCH 2/4] Introduce a generic tree-like abstraction for MIME traversal.

2011-11-27 Thread Austin Clements
This wraps all of the complex MIME part handling in a single, simple
function that gets part N from *any* MIME object, so traversing a MIME
part tree becomes a two-line for loop.  Furthermore, the MIME node
structure provides easy access to envelopes for message parts as well
as cryptographic information.

This code is directly derived from the current show_message_body code
(much of it is identical), but the control relation is inverted:
instead of show_message_body controlling the traversal of the MIME
structure and invoking callbacks, the caller controls the traversal of
the MIME structure.
---
 Makefile.local   |1 +
 mime-node.c  |  234 ++
 notmuch-client.h |   80 ++
 3 files changed, 315 insertions(+), 0 deletions(-)
 create mode 100644 mime-node.c

diff --git a/Makefile.local b/Makefile.local
index c94402b..c46ed26 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -312,6 +312,7 @@ notmuch_client_srcs =   \
notmuch-time.c  \
query-string.c  \
show-message.c  \
+   mime-node.c \
json.c
 
 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
diff --git a/mime-node.c b/mime-node.c
new file mode 100644
index 000..942738b
--- /dev/null
+++ b/mime-node.c
@@ -0,0 +1,234 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright © 2009 Carl Worth
+ * Copyright © 2009 Keith Packard
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Authors: Carl Worth 
+ *  Keith Packard 
+ *  Austin Clements 
+ */
+
+#include "notmuch-client.h"
+
+/* Context that gets inherited from the root node. */
+typedef struct mime_node_context {
+/* Per-message resources.  These are allocated internally and must
+ * be destroyed. */
+FILE *file;
+GMimeStream *stream;
+GMimeParser *parser;
+GMimeMessage *mime_message;
+
+/* Context provided by the caller. */
+GMimeCipherContext *cryptoctx;
+notmuch_bool_t decrypt;
+} mime_node_context_t;
+
+static int
+_mime_node_context_free (mime_node_context_t *res)
+{
+if (res->mime_message)
+   g_object_unref (res->mime_message);
+
+if (res->parser)
+   g_object_unref (res->parser);
+
+if (res->stream)
+   g_object_unref (res->stream);
+
+if (res->file)
+   fclose (res->file);
+
+return 0;
+}
+
+notmuch_status_t
+mime_node_open (const void *ctx, notmuch_message_t *message,
+   GMimeCipherContext* cryptoctx, notmuch_bool_t decrypt,
+   mime_node_t **root_out)
+{
+const char *filename = notmuch_message_get_filename (message);
+mime_node_context_t *mctx;
+mime_node_t *root = NULL;
+notmuch_status_t status;
+
+root = talloc_zero (ctx, mime_node_t);
+if (root == NULL) {
+   fprintf (stderr, "Out of memory.\n");
+   status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+   goto DONE;
+}
+
+/* Create the tree-wide context */
+mctx = talloc_zero (root, mime_node_context_t);
+if (mctx == NULL) {
+   fprintf (stderr, "Out of memory.\n");
+   status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+   goto DONE;
+}
+talloc_set_destructor (mctx, _mime_node_context_free);
+
+mctx->file = fopen (filename, "r");
+if (! mctx->file) {
+   fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
+   status = NOTMUCH_STATUS_FILE_ERROR;
+   goto DONE;
+}
+
+mctx->stream = g_mime_stream_file_new (mctx->file);
+g_mime_stream_file_set_owner (GMIME_STREAM_FILE (mctx->stream), FALSE);
+
+mctx->parser = g_mime_parser_new_with_stream (mctx->stream);
+
+mctx->mime_message = g_mime_parser_construct_message (mctx->parser);
+
+mctx->cryptoctx = cryptoctx;
+mctx->decrypt = decrypt;
+
+/* Create the root node */
+root->part = GMIME_OBJECT (mctx->mime_message);
+root->envelope_file = message;
+root->children = 1;
+root->ctx = mctx;
+
+*root_out = root;
+return NOTMUCH_STATUS_SUCCESS;
+
+DONE:
+talloc_free (root);
+return status;
+}
+
+static int
+_signature_validity_free (GMimeSignatureValidity **proxy)
+{
+g_mime_signature_validity_free (*proxy);
+return 0;
+}
+
+static mime_node_t *
+_mime_node_create (const mime_node_t *parent, GMimeObject *part)
+{
+mime_node_t *out 

[PATCH 1/4] show: Pass notmuch_message_t instead of path to show_message_body.

2011-11-27 Thread Austin Clements
In addition to simplifying the code, we'll need the notmuch_message_t*
in show_message_body shortly.
---
 notmuch-client.h |2 +-
 notmuch-reply.c  |3 +--
 notmuch-show.c   |3 +--
 show-message.c   |3 ++-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index b50cb38..d7fb6ee 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -162,7 +162,7 @@ char *
 query_string_from_args (void *ctx, int argc, char *argv[]);
 
 notmuch_status_t
-show_message_body (const char *filename,
+show_message_body (notmuch_message_t *message,
   const notmuch_show_format_t *format,
   notmuch_show_params_t *params);
 
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 7ac879f..f8d5f64 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -546,8 +546,7 @@ notmuch_reply_format_default(void *ctx,
notmuch_message_get_header (message, "date"),
notmuch_message_get_header (message, "from"));
 
-   show_message_body (notmuch_message_get_filename (message),
-  format, params);
+   show_message_body (message, format, params);
 
notmuch_message_destroy (message);
 }
diff --git a/notmuch-show.c b/notmuch-show.c
index 603992a..1dee3aa 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -753,8 +753,7 @@ show_message (void *ctx,
 }
 
 if (format->part_content)
-   show_message_body (notmuch_message_get_filename (message),
-  format, params);
+   show_message_body (message, format, params);
 
 if (params->part <= 0) {
fputs (format->body_end, stdout);
diff --git a/show-message.c b/show-message.c
index d83f04e..09fa607 100644
--- a/show-message.c
+++ b/show-message.c
@@ -175,7 +175,7 @@ show_message_part (GMimeObject *part,
 }
 
 notmuch_status_t
-show_message_body (const char *filename,
+show_message_body (notmuch_message_t *message,
   const notmuch_show_format_t *format,
   notmuch_show_params_t *params)
 {
@@ -183,6 +183,7 @@ show_message_body (const char *filename,
 GMimeParser *parser = NULL;
 GMimeMessage *mime_message = NULL;
 notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
+const char *filename = notmuch_message_get_filename (message);
 FILE *file = NULL;
 show_message_state_t state;
 
-- 
1.7.5.4

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


[PATCH 0/4] First step of 'show' rewrite

2011-11-27 Thread Austin Clements
This is the first step in my rewrite of notmuch-show.  Currently, MIME
traversal is controlled by show_message_body, which invokes formatter
callbacks.  As a result, formatters have no control over traversal,
which complicates formatters and makes others (like raw) nigh
impossible to implement correctly.  The callback-driven approach
forces formatters to be written in many small pieces and masks the
true control flow.  It also results in an wide interface with
show_message_body that has been getting wider over time.

The goal of this rewrite is to reverse this relationship, so that
formatters control MIME traversal.  This lets formatters traverse the
MIME in the most appropriate way, eliminates the callbacks, makes the
control flow through a formatter clear, and dramatically narrows
interfaces.

This first series doesn't change the way formatters work, but it does
introduce the MIME-traversal API they'll use.  This API consists of a
total of two functions: one to get the root MIME part of a message and
one to get a child of a MIME part.  This allows basic MIME traversal
to be implemented in a simple, two-line for loop.  The series also
updates show_message_body to use this new API, though ultimately
show_message_body will go away entirely.

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


[PATCH] dump: Don't sort the output by message id.

2011-11-27 Thread Tom Prince
From: Thomas Schwinge 

Asking xapian to sort the messages for us causes suboptimal IO patterns. This
would be useful, if we only wanted the first few results, but since we want
everything anyway, this is pessimization.

On 2011-10-29, a measurement on a 372981 messages instance showed that wall
time can be reduced from 28 minutes (sorted by Message-ID) to 15 minutes
(unsorted).

Timings on 189605 messages:

$ time notmuch.old dump
19.48user 5.83system 12:10.42elapsed 3%CPU (0avgtext+0avgdata 
110656maxresident)k
3629584inputs+22720outputs (33major+7073minor)pagefaults 0swaps
$ echo 3 > /proc/sys/vm/drop_caches
$ time notmuch.new
14.89user 1.20system 3:23.58elapsed 7%CPU (0avgtext+0avgdata 46032maxresident)k
1256264inputs+22464outputs (43major+1990minor)pagefaults 0swaps
---
 This just moves the motivation to the commit message, and adds more detailed 
timing information.

 notmuch-dump.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 126593d..0475eb9 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -73,7 +73,10 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
fprintf (stderr, "Out of memory\n");
return 1;
 }
-notmuch_query_set_sort (query, NOTMUCH_SORT_MESSAGE_ID);
+/* Don't ask xapian to sort by Message-ID. Xapian optimizes returning the
+ * first results quickly at the expense of total time.
+ */
+notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);

 for (messages = notmuch_query_search_messages (query);
 notmuch_messages_valid (messages);
-- 
1.7.6.1



[PATCH] dump: Don't sort the output by message id.

2011-11-27 Thread Tom Prince
From: Thomas Schwinge 

Asking xapian to sort the messages for us causes suboptimal IO patterns. This
would be useful, if we only wanted the first few results, but since we want
everything anyway, this is pessimization.

On 2011-10-29, a measurement on a 372981 messages instance showed that wall
time can be reduced from 28 minutes (sorted by Message-ID) to 15 minutes
(unsorted).

Timings on 189605 messages:

$ time notmuch.old dump
19.48user 5.83system 12:10.42elapsed 3%CPU (0avgtext+0avgdata 
110656maxresident)k
3629584inputs+22720outputs (33major+7073minor)pagefaults 0swaps
$ echo 3 > /proc/sys/vm/drop_caches
$ time notmuch.new
14.89user 1.20system 3:23.58elapsed 7%CPU (0avgtext+0avgdata 46032maxresident)k
1256264inputs+22464outputs (43major+1990minor)pagefaults 0swaps
---
 This just moves the motivation to the commit message, and adds more detailed 
timing information.

 notmuch-dump.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 126593d..0475eb9 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -73,7 +73,10 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
fprintf (stderr, "Out of memory\n");
return 1;
 }
-notmuch_query_set_sort (query, NOTMUCH_SORT_MESSAGE_ID);
+/* Don't ask xapian to sort by Message-ID. Xapian optimizes returning the
+ * first results quickly at the expense of total time.
+ */
+notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);
 
 for (messages = notmuch_query_search_messages (query);
 notmuch_messages_valid (messages);
-- 
1.7.6.1

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


[PATCH v3 7/7] test: fix "Stashing in notmuch-search" test when emacs is not available

2011-11-27 Thread Dmitry Kurochkin
If emacs is not available, test_expect_equal would be called with only
one argument.  The patch fixes this by quoting the (possibly empty)
$(cat OUTPUT) argument.
---
 test/emacs |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/emacs b/test/emacs
index 198c27b..3f8c72d 100755
--- a/test/emacs
+++ b/test/emacs
@@ -412,7 +412,7 @@ test_emacs '(notmuch-search "id:\"bought\"")
 (yank)
(test-output)'
 sed -i -e 's/^thread:.*$/thread:XXX/' OUTPUT
-test_expect_equal $(cat OUTPUT) "thread:XXX"
+test_expect_equal "$(cat OUTPUT)" "thread:XXX"
 
 test_begin_subtest 'Hiding message following HTML part'
 test_subtest_known_broken
-- 
1.7.7.3

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


[PATCH v3 6/7] test: check if emacs is available in the beginning of test_emacs

2011-11-27 Thread Dmitry Kurochkin
Unfortunately, this is needed to avoid the emacs waiting loop.
---
 test/test-lib.sh |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 2422e32..11e6646 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -894,6 +894,10 @@ EOF
 }
 
 test_emacs () {
+   # test dependencies beforehand to avoid the waiting loop below
+   test_require_external_prereq emacs || return
+   test_require_external_prereq emacsclient || return
+
if [ -z "$EMACS_SERVER" ]; then
server_name="notmuch-test-suite-$$"
# start a detached session with an emacs server
-- 
1.7.7.3

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


[PATCH v3 5/7] test: add function to explicitly check for external dependencies

2011-11-27 Thread Dmitry Kurochkin
Useful when binary is called indirectly (e.g. from emacs).
---
 test/test-lib.sh |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index fe80e89..2422e32 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -555,12 +555,26 @@ test_declare_external_prereq () {
 
hash $binary 2>/dev/null || eval "
 $binary () {
+   test_missing_external_prereq_${binary}_=t
echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name 
\" ||

test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
 $name\"
false
 }"
 }
 
+# Explicitly require external prerequisite.  Useful when binary is
+# called indirectly (e.g. from emacs).
+# Returns success if dependency is available, failure otherwise.
+test_require_external_prereq () {
+   binary="$1"
+   if [ "$(eval echo -n \$test_missing_external_prereq_${binary}_)" = t ]; 
then
+   # dependency is missing, call the replacement function to note 
it
+   eval "$binary"
+   else
+   true
+   fi
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
 
-- 
1.7.7.3

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


[PATCH v3 4/7] test: declare external dependencies for the tests

2011-11-27 Thread Dmitry Kurochkin
That are: dtach(1), emacs(1), emacsclient(1), gdb(1) and gpg(1).
---
 test/test-lib.sh |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 99b9e8b..fe80e89 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1123,3 +1123,10 @@ test -z "$NO_PYTHON" && test_set_prereq PYTHON
 # test whether the filesystem supports symbolic links
 ln -s x y 2>/dev/null && test -h y 2>/dev/null && test_set_prereq SYMLINKS
 rm -f y
+
+# declare prerequisites for external binaries used in tests
+test_declare_external_prereq dtach
+test_declare_external_prereq emacs
+test_declare_external_prereq emacsclient
+test_declare_external_prereq gdb
+test_declare_external_prereq gpg
-- 
1.7.7.3

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


[PATCH v3 3/7] test: skip all subtests if external dependencies are missing during init

2011-11-27 Thread Dmitry Kurochkin
Some tests (e.g. crypto) do a common initialization required for all
subtests.  The patch adds a check for missing external dependencies
during this initialization.  If any prerequisites are missing, all
subtests are skipped.

The check is run on the first call of test_reset_state_ function, so
no changes for the tests are needed.
---
 test/test-lib.sh |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index df867a5..99b9e8b 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -902,10 +902,20 @@ test_emacs () {
 }
 
 test_reset_state_ () {
+   test -z "$test_init_done_" && test_init_
+
test_subtest_known_broken_=
test_subtest_missing_external_prereqs_=
 }
 
+# called once before the first subtest
+test_init_ () {
+   test_init_done_=t
+
+   # skip all tests if there were external prerequisites missing during 
init
+   test_check_missing_external_prereqs_ "all tests in $this_test" && 
test_done
+}
+
 
 find_notmuch_path ()
 {
-- 
1.7.7.3

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


[PATCH v3 2/7] test: fix "skipping test" verbose output

2011-11-27 Thread Dmitry Kurochkin
---
 test/test-lib.sh |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0996a74..df867a5 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -657,7 +657,8 @@ test_check_missing_external_prereqs_ () {
 
 test_report_skip_ () {
test_reset_state_
-   say_color skip >&3 "skipping test: $@"
+   say_color skip >&3 "skipping test:"
+   echo " $@" >&3
say_color skip "%-6s" "SKIP"
echo " $1"
 }
-- 
1.7.7.3

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


[PATCH v3 1/7] test: add support for external executable dependencies

2011-11-27 Thread Dmitry Kurochkin
There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.

The patch aims to add better support for a particular type of external
dependencies: external executables.  The main idea is to replace
missing external binaries with shell functions that have the same
name.  These functions always fail and keep track of missing
dependencies for a subtest.  The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL.  The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
 test/test-lib.sh |   49 +
 1 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 076f929..0996a74 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -548,6 +548,19 @@ test_have_prereq () {
esac
 }
 
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+   binary="$1"
+   test "$#" = 2 && name=$2 || name="$binary(1)"
+
+   hash $binary 2>/dev/null || eval "
+$binary () {
+   echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name 
\" ||
+   
test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
 $name\"
+   false
+}"
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
 
@@ -624,18 +637,31 @@ test_skip () {
fi
case "$to_skip" in
t)
-   test_reset_state_
-   say_color skip >&3 "skipping test: $@"
-   say_color skip "%-6s" "SKIP"
-   echo " $1"
-   : true
+   test_report_skip_ "$@"
;;
*)
-   false
+   test_check_missing_external_prereqs_ "$@"
;;
esac
 }
 
+test_check_missing_external_prereqs_ () {
+   if test -n "$test_subtest_missing_external_prereqs_"; then
+   say_color skip >&3 "missing prerequisites:"
+   echo "$test_subtest_missing_external_prereqs_" >&3
+   test_report_skip_ "$@"
+   else
+   false
+   fi
+}
+
+test_report_skip_ () {
+   test_reset_state_
+   say_color skip >&3 "skipping test: $@"
+   say_color skip "%-6s" "SKIP"
+   echo " $1"
+}
+
 test_subtest_known_broken () {
test_subtest_known_broken_=t
 }
@@ -648,7 +674,10 @@ test_expect_success () {
if ! test_skip "$@"
then
test_run_ "$2"
-   if [ "$?" = 0 -a "$eval_ret" = 0 ]
+   run_ret="$?"
+   # test_run_ may update missing external prerequisites
+   test_check_missing_external_prereqs_ "$@" ||
+   if [ "$run_ret" = 0 -a "$eval_ret" = 0 ]
then
test_ok_ "$1"
else
@@ -665,7 +694,10 @@ test_expect_code () {
if ! test_skip "$@"
then
test_run_ "$3"
-   if [ "$?" = 0 -a "$eval_ret" = "$1" ]
+   run_ret="$?"
+   # test_run_ may update missing external prerequisites,
+   test_check_missing_external_prereqs_ "$@" ||
+   if [ "$run_ret" = 0 -a "$eval_ret" = "$1" ]
then
test_ok_ "$2"
else
@@ -870,6 +902,7 @@ test_emacs () {
 
 test_reset_state_ () {
test_subtest_known_broken_=
+   test_subtest_missing_external_prereqs_=
 }
 
 
-- 
1.7.7.3

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


[PATCH v3 00/7] test: (hopefully) better test prerequisites

2011-11-27 Thread Dmitry Kurochkin
Changes:

v3 since v2:

* rebased the remaining patches on current master

v2 since v1:

* Add test_require_external_prereq function to explicitly check for
  external dependencies, use it in test_emacs.

* Indenting fixes.

* Use $binary instead of $1 in test_declare_external_prereq.

Regards,
  Dmitry

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


Re: [PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

2011-11-27 Thread Dmitry Kurochkin
On Sat, 26 Nov 2011 22:14:20 +0100, Amadeusz Żołnowski  
wrote:
> If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
> it's not so easy to pass $(srcdir) which is required to find notmuch.h
> when srcdir and builddir are separate directories.
> ---
>  test/.gitignore |1 +
>  test/Makefile.local |7 +--
>  test/basic  |2 +-
>  test/symbol-hiding  |3 +--
>  4 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/test/.gitignore b/test/.gitignore
> index 9e97052..7e30e8d 100644
> --- a/test/.gitignore
> +++ b/test/.gitignore
> @@ -1,4 +1,5 @@
>  test-results
>  corpus.mail
>  smtp-dummy
> +symbol-test
>  tmp.*
> diff --git a/test/Makefile.local b/test/Makefile.local
> index 9b3308a..646779e 100644
> --- a/test/Makefile.local
> +++ b/test/Makefile.local
> @@ -11,10 +11,13 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
>  $(dir)/smtp-dummy: $(smtp_dummy_modules)
>   $(call quiet,CC) $^ -o $@
>  
> +$(dir)/symbol-test: $(dir)/symbol-test.o
> + $(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
> +
>  .PHONY: test check
> -test:all $(dir)/smtp-dummy
> +test:all $(dir)/smtp-dummy $(dir)/symbol-test
>   @${dir}/notmuch-test $(OPTIONS)
>  
>  check: test
>  
> -CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o
> +CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o $(dir)/symbol-test 
> $(dir)/symbol-test.o
> diff --git a/test/basic b/test/basic
> index f258d1f..4edf831 100755
> --- a/test/basic
> +++ b/test/basic
> @@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be 
> run by notmuch-test'
>  eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
>  tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
>  available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf 
> '%f\n' | \
> -sed -r -e 
> "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose)$/d" | \
> +sed -r -e 
> "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d"
>  | \

Can you please keep this list alphabetically sorted?

Regards,
  Dmitry

>  sort)
>  test_expect_equal "$tests_in_suite" "$available"
>  
> diff --git a/test/symbol-hiding b/test/symbol-hiding
> index d0b31ae..f67b653 100755
> --- a/test/symbol-hiding
> +++ b/test/symbol-hiding
> @@ -12,13 +12,12 @@ test_description='exception symbol hiding'
>  . ./test-lib.sh
>  
>  run_test(){
> -result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
> +result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib 
> $TEST_DIRECTORY/symbol-test 2>&1)
>  }
>  
>  output="A Xapian exception occurred opening database: Couldn't stat 
> 'fakedb/.notmuch/xapian'
>  caught No chert database found at path \`./nonexistant'"
>  
> -g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc 
> -L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
>  mkdir -p fakedb/.notmuch
>  test_expect_success 'running test' run_test
>  test_begin_subtest 'checking output'
> -- 
> 1.7.8.rc3
> 
> ___
> 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] Build symbol-test with make instead of hardcoding in symbol-hiding.

2011-11-27 Thread David Bremner
On Sat, 26 Nov 2011 22:14:20 +0100, Amadeusz Żołnowski  
wrote:
> If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
> it's not so easy to pass $(srcdir) which is required to find notmuch.h
> when srcdir and builddir are separate directories.

pushed,

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


[PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

2011-11-27 Thread David Bremner
On Sat, 26 Nov 2011 22:14:20 +0100, Amadeusz ?o?nowski  wrote:
> If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
> it's not so easy to pass $(srcdir) which is required to find notmuch.h
> when srcdir and builddir are separate directories.

pushed,

d


Re: [PATCH v2 03/10] test: add test state reset to test_expect_* functions that did not have it

2011-11-27 Thread David Bremner
On Thu, 17 Nov 2011 17:05:56 +0400, Dmitry Kurochkin 
 wrote:
> ---
>  test/test-lib.sh |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

I pushed the first 3, but the 4th won't apply, even with git am -3

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


[PATCH v2 03/10] test: add test state reset to test_expect_* functions that did not have it

2011-11-27 Thread David Bremner
On Thu, 17 Nov 2011 17:05:56 +0400, Dmitry Kurochkin  wrote:
> ---
>  test/test-lib.sh |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

I pushed the first 3, but the 4th won't apply, even with git am -3

d


[PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

2011-11-27 Thread Amadeusz Żołnowski
If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
it's not so easy to pass $(srcdir) which is required to find notmuch.h
when srcdir and builddir are separate directories.
---
 test/.gitignore |1 +
 test/Makefile.local |7 +--
 test/basic  |2 +-
 test/symbol-hiding  |3 +--
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/test/.gitignore b/test/.gitignore
index 9e97052..7e30e8d 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -1,4 +1,5 @@
 test-results
 corpus.mail
 smtp-dummy
+symbol-test
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 9b3308a..646779e 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -11,10 +11,13 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
$(call quiet,CC) $^ -o $@
 
+$(dir)/symbol-test: $(dir)/symbol-test.o
+   $(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
+
 .PHONY: test check
-test:  all $(dir)/smtp-dummy
+test:  all $(dir)/smtp-dummy $(dir)/symbol-test
@${dir}/notmuch-test $(OPTIONS)
 
 check: test
 
-CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o
+CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o $(dir)/symbol-test 
$(dir)/symbol-test.o
diff --git a/test/basic b/test/basic
index f258d1f..4edf831 100755
--- a/test/basic
+++ b/test/basic
@@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be 
run by notmuch-test'
 eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
 tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf 
'%f\n' | \
-sed -r -e 
"/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose)$/d" | \
+sed -r -e 
"/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d" 
| \
 sort)
 test_expect_equal "$tests_in_suite" "$available"
 
diff --git a/test/symbol-hiding b/test/symbol-hiding
index d0b31ae..f67b653 100755
--- a/test/symbol-hiding
+++ b/test/symbol-hiding
@@ -12,13 +12,12 @@ test_description='exception symbol hiding'
 . ./test-lib.sh
 
 run_test(){
-result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
+result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib 
$TEST_DIRECTORY/symbol-test 2>&1)
 }
 
 output="A Xapian exception occurred opening database: Couldn't stat 
'fakedb/.notmuch/xapian'
 caught No chert database found at path \`./nonexistant'"
 
-g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc 
-L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
 mkdir -p fakedb/.notmuch
 test_expect_success 'running test' run_test
 test_begin_subtest 'checking output'
-- 
1.7.8.rc3

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


Re: [PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

2011-11-27 Thread Amadeusz Żołnowski
Excerpts from Amadeusz Żołnowski's message of 2011-11-26 21:51:21 +0100:
> If symbol-test is built in symbol-hiding with hardcoded g++
> invokation, it's not so easy to pass $(srcdir) which is required to
> find notmuch.h when srcdir and builddir are separate directories.

Sorry, I've just realised that I've forgot to add symbol-test to clean
target.  Following patch includes clean, too.

-- 
Amadeusz Żołnowski


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


[PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

2011-11-27 Thread Amadeusz Żołnowski
If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
it's not so easy to pass $(srcdir) which is required to find notmuch.h
when srcdir and builddir are separate directories.
---
 test/.gitignore |1 +
 test/Makefile.local |5 -
 test/basic  |2 +-
 test/symbol-hiding  |3 +--
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/test/.gitignore b/test/.gitignore
index 9e97052..7e30e8d 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -1,4 +1,5 @@
 test-results
 corpus.mail
 smtp-dummy
+symbol-test
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 9b3308a..b77f721 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -11,8 +11,11 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
$(call quiet,CC) $^ -o $@
 
+$(dir)/symbol-test: $(dir)/symbol-test.o
+   $(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
+
 .PHONY: test check
-test:  all $(dir)/smtp-dummy
+test:  all $(dir)/smtp-dummy $(dir)/symbol-test
@${dir}/notmuch-test $(OPTIONS)
 
 check: test
diff --git a/test/basic b/test/basic
index f258d1f..4edf831 100755
--- a/test/basic
+++ b/test/basic
@@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be 
run by notmuch-test'
 eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
 tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf 
'%f\n' | \
-sed -r -e 
"/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose)$/d" | \
+sed -r -e 
"/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d" 
| \
 sort)
 test_expect_equal "$tests_in_suite" "$available"
 
diff --git a/test/symbol-hiding b/test/symbol-hiding
index d0b31ae..f67b653 100755
--- a/test/symbol-hiding
+++ b/test/symbol-hiding
@@ -12,13 +12,12 @@ test_description='exception symbol hiding'
 . ./test-lib.sh
 
 run_test(){
-result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
+result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib 
$TEST_DIRECTORY/symbol-test 2>&1)
 }
 
 output="A Xapian exception occurred opening database: Couldn't stat 
'fakedb/.notmuch/xapian'
 caught No chert database found at path \`./nonexistant'"
 
-g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc 
-L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
 mkdir -p fakedb/.notmuch
 test_expect_success 'running test' run_test
 test_begin_subtest 'checking output'
-- 
1.7.8.rc3

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


Re: [PATCH 1/2] Build symbol-test with make instead of hardcoding in symbol-hiding.

2011-11-27 Thread David Bremner
On Fri, 25 Nov 2011 11:54:51 +0100, Amadeusz Żołnowski  
wrote:
> If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
> it's not so easy to pass $(srcdir) which is required to find notmuch.h
> when srcdir and builddir are separate directories.
> ---

Sorry to do this to you, but I pushed Dmitry's patch to test/basic
first, so can you please rebase?

thanks

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


Re: [PATCH 2/2] test: cleanup basic tests

2011-11-27 Thread David Bremner
On Sat, 26 Nov 2011 10:12:26 +0400, Dmitry Kurochkin 
 wrote:
> Basic test 'Ensure that all available tests will be run by
> notmuch-test' compares all tests that are run with listing of test/

Pushed these two to master.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Bug fix release 0.10.1 now available

2011-11-27 Thread David Bremner

Where to obtain notmuch 0.10.1
===
  http://notmuchmail.org/releases/notmuch-0.10.1.tar.gz

Which can be verified with:

  http://notmuchmail.org/releases/notmuch-0.10.1.tar.gz.sha1
  0939709a2e60c59656b8e0761ae8bcb38bba2a06  notmuch-0.10.1.tar.gz

  http://notmuchmail.org/releases/notmuch-0.10.1.tar.gz.sha1.asc
  (signed by David Bremner)

What's new in notmuch 0.10.1
=

Bug-fix release.


Fix --help argument

  Argument processing changes in 0.10 introduced a bug where "notmuch
  --help" crashed while "notmuch help" worked fine. This is fixed in
  0.10.1.

What is notmuch
===
Notmuch is a system for indexing, searching, reading, and tagging
large collections of email messages in maildir or mh format. It uses
the Xapian library to provide fast, full-text search with a convenient
search syntax.

For more about notmuch, see http://notmuchmail.org



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


Re: [PATCH] test: add simple tests for online help

2011-11-27 Thread Jani Nikula
On Fri, 25 Nov 2011 13:36:26 -0500, David Bremner  wrote:
> I think Jani was just making fun of me when he suggested tests for
> notmuch --help, but I thought, why not.

Heh, I was only half joking - it is certainly good practice to create
tests for any bugs found and fixed. It's a bit embarrassing to have
--help segfault, but even more so to have it reappear in the future!
Good job.

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