release-candidate/0.6 redux

release-candidate/0.6 redux

[Jameson Graef Rollins]

So what follows is a patch series for a bunch of miscellaneous patches
that should be included in 0.6.  Most of them were originally part of
the release-candiate/0.6 branch, and they are here rebased on top of
notmuch/master at cb8418784c21155ffea79cce8409a7ea3c546937, which
includes all the multipart and crypto rework.

Some of these patches are cleanup, but some are important bug fixes,
some previously sent to the list by others that I have signed off on,
and some by me that were not previously sent to the list.  I highly
recommend that this entire series be incorporated before 0.6 is
released.

jamie.

[PATCH 01/25] fix check for libdir in ldconfig paths

[Jameson Graef Rollins]

The configure script tries to check that the libdir is included in the
ldconfig paths, and then sets the RPATH compiler flag if it's not.
This check was broken, and was always setting RPATH.  This fixes the
path check.
---
 configure |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index bbf30cd..f735c1d 100755
--- a/configure
+++ b/configure
@@ -358,14 +358,14 @@ elif [ $uname = "Linux" ] ; then
     ldconfig_paths=$(/sbin/ldconfig -N -X -v 2>/dev/null | sed -n -e 's,^\(/.*\):\( (.*)\)\?$,\1,p')
     # Separate ldconfig_paths only on newline (not on any potential
     # embedded space characters in any filenames).
-    OLD_IFS=$IFS
-    IFS="$(printf '\n')"
     for path in $ldconfig_paths; do
 	if [ "$path" = "$libdir_expanded" ]; then
 	    libdir_in_ldconfig=1
 	fi
     done
-    IFS=$OLD_IFS
+    if [ "$libdir_in_ldconfig" = '0' ]; then
+	printf "\tlibdir not found in ldconfig paths.  RPATH variable will be set.\n"
+    fi
 else
     printf "Unknown.\n"
     cat <<EOF
-- 
1.7.4.4

[PATCH 02/25] add note about updating the debian symbols file to the RELEASE file

[Jameson Graef Rollins]

---
 RELEASING |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/RELEASING b/RELEASING
index a392d81..475100a 100644
--- a/RELEASING
+++ b/RELEASING
@@ -35,7 +35,21 @@ repository. From here, there are just a few steps to release:
 
 	Commit this change, if any.
 
-3) Upgrade the version in the file "version"
+3) Update the debian/libnotmuch1.symbols file
+
+	If the library version changed at all (step 2) it probably
+	means that symbols have changed/been added, in which case the
+	debian symbols file also needs to be updated:
+
+	       dpkg-buildpackage -uc -us
+	       dpkg-gensymbols -plibnotmuch1 | patch -p0
+
+	Carefully review the changes to debian/libnotmuch1.symbols to
+	make sure there are no unexpected changes.
+
+	Commit this change, if any.
+
+4) Upgrade the version in the file "version"
 
 	The scheme for the release number is as follows:
 
@@ -55,7 +69,7 @@ repository. From here, there are just a few steps to release:
 
 	Commit this change.
 
-4) Create an entry for the new release in debian/changelog
+5) Create an entry for the new release in debian/changelog
 
 	The syntax of this file is tightly restricted, but the
 	available emacs mode (see the dpkg-dev-el package) helps.
@@ -69,7 +83,7 @@ repository. From here, there are just a few steps to release:
 	release, (taking entries from NEWS and the version from the
 	version file, and creating a new commit, etc.)
 
-5) Run "make release" which will perform the following steps.
+6) Run "make release" which will perform the following steps.
 
    Note: If any problem occurs during the process, (such as a lintian
    warning that you decide should be fixed), you can abort at the
@@ -98,7 +112,7 @@ repository. From here, there are just a few steps to release:
 	* Push that tag
 	* Provide some text for the release announcement (see below).
 
-6) Send a message to notmuch@notmuchmail.org to announce the release.
+7) Send a message to notmuch@notmuchmail.org to announce the release.
 
 	Use the text provided from "make release" above, (if for some
 	reason you lose this message, "make release-message" prints
-- 
1.7.4.4

[PATCH 03/25] test: modify search-output test to use the new test_expect_equal_file function

[Jameson Graef Rollins]

These test now properly test for the presence of a newline at the end
of all output.  Right now some of these test will fail because the
search output is currently broken to *not* produce proper newlines in
some cases.
---
 test/search-output |   72 ++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/test/search-output b/test/search-output
index 20341d6..1554eb1 100755
--- a/test/search-output
+++ b/test/search-output
@@ -5,8 +5,8 @@ test_description='various settings for "notmuch search --output="'
 add_email_corpus
 
 test_begin_subtest "notmuch search --output=threads"
-output=$(notmuch search --output=threads '*' | sed -e s/thread:.*/thread:THREADID/)
-test_expect_equal "$output" "thread:THREADID
+notmuch search --output=threads '*' | sed -e s/thread:.*/thread:THREADID/ >OUTPUT
+cat <<EOF >EXPECTED
 thread:THREADID
 thread:THREADID
 thread:THREADID
@@ -27,11 +27,15 @@ thread:THREADID
 thread:THREADID
 thread:THREADID
 thread:THREADID
-thread:THREADID"
+thread:THREADID
+thread:THREADID
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch search --format=json --output=threads"
-output=$(notmuch search --format=json --output=threads '*' | sed -e s/\".*\"/\"THREADID\"/)
-test_expect_equal "$output" '["THREADID",
+notmuch search --format=json --output=threads '*' | sed -e s/\".*\"/\"THREADID\"/ >OUTPUT
+cat <<EOF >EXPECTED
+["THREADID",
 "THREADID",
 "THREADID",
 "THREADID",
@@ -52,11 +56,14 @@ test_expect_equal "$output" '["THREADID",
 "THREADID",
 "THREADID",
 "THREADID",
-"THREADID"]'
+"THREADID"]
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch search --output=messages"
-output=$(notmuch search --output=messages '*')
-test_expect_equal "$output" "id:1258544095-16616-1-git-send-email-chris@chris-wilson.co.uk
+notmuch search --output=messages '*' >OUTPUT
+cat <<EOF >EXPECTED
+id:1258544095-16616-1-git-send-email-chris@chris-wilson.co.uk
 id:877htoqdbo.fsf@yoom.home.cworth.org
 id:878we4qdqf.fsf@yoom.home.cworth.org
 id:87aaykqe24.fsf@yoom.home.cworth.org
@@ -105,11 +112,14 @@ id:87iqd9rn3l.fsf@vertex.dottedmag
 id:20091117190054.GU3165@dottiness.seas.harvard.edu
 id:87lji5cbwo.fsf@yoom.home.cworth.org
 id:1258471718-6781-2-git-send-email-dottedmag@dottedmag.net
-id:1258471718-6781-1-git-send-email-dottedmag@dottedmag.net"
+id:1258471718-6781-1-git-send-email-dottedmag@dottedmag.net
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch search --format=json --output=messages"
-output=$(notmuch search --format=json --output=messages '*')
-test_expect_equal "$output" '["1258544095-16616-1-git-send-email-chris@chris-wilson.co.uk",
+notmuch search --format=json --output=messages '*' >OUTPUT
+cat <<EOF >EXPECTED
+["1258544095-16616-1-git-send-email-chris@chris-wilson.co.uk",
 "877htoqdbo.fsf@yoom.home.cworth.org",
 "878we4qdqf.fsf@yoom.home.cworth.org",
 "87aaykqe24.fsf@yoom.home.cworth.org",
@@ -158,11 +168,14 @@ test_expect_equal "$output" '["1258544095-16616-1-git-send-email-chris@chris-wil
 "20091117190054.GU3165@dottiness.seas.harvard.edu",
 "87lji5cbwo.fsf@yoom.home.cworth.org",
 "1258471718-6781-2-git-send-email-dottedmag@dottedmag.net",
-"1258471718-6781-1-git-send-email-dottedmag@dottedmag.net"]'
+"1258471718-6781-1-git-send-email-dottedmag@dottedmag.net"]
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch search --output=files"
-output=$(notmuch search --output=files '*' | sed -e "s,$MAIL_DIR,MAIL_DIR,")
-test_expect_equal "$output" "MAIL_DIR/cur/50:2,
+notmuch search --output=files '*' | sed -e "s,$MAIL_DIR,MAIL_DIR," >OUTPUT
+cat <<EOF >EXPECTED
+MAIL_DIR/cur/50:2,
 MAIL_DIR/cur/49:2,
 MAIL_DIR/cur/48:2,
 MAIL_DIR/cur/47:2,
@@ -211,11 +224,14 @@ MAIL_DIR/cur/04:2,
 MAIL_DIR/cur/03:2,
 MAIL_DIR/cur/07:2,
 MAIL_DIR/cur/02:2,
-MAIL_DIR/cur/01:2,"
+MAIL_DIR/cur/01:2,
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch search --format=json --output=files"
-output=$(notmuch search --format=json --output=files '*' | sed -e "s,$MAIL_DIR,MAIL_DIR,")
-test_expect_equal "$output" '["MAIL_DIR/cur/50:2,",
+notmuch search --format=json --output=files '*' | sed -e "s,$MAIL_DIR,MAIL_DIR," >OUTPUT
+cat <<EOF >EXPECTED
+["MAIL_DIR/cur/50:2,",
 "MAIL_DIR/cur/49:2,",
 "MAIL_DIR/cur/48:2,",
 "MAIL_DIR/cur/47:2,",
@@ -264,20 +280,28 @@ test_expect_equal "$output" '["MAIL_DIR/cur/50:2,",
 "MAIL_DIR/cur/03:2,",
 "MAIL_DIR/cur/07:2,",
 "MAIL_DIR/cur/02:2,",
-"MAIL_DIR/cur/01:2,"]'
+"MAIL_DIR/cur/01:2,"]
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch search --output=tags"
-output=$(notmuch search --output=tags '*')
-test_expect_equal "$output" "attachment
+notmuch search --output=tags '*' >OUTPUT
+cat <<EOF >EXPECTED
+attachment
 inbox
 signed
-unread"
+unread
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch search --format=json --output=tags"
-output=$(notmuch search --format=json --output=tags '*')
-test_expect_equal "$output" '["attachment",
+notmuch search --format=json --output=tags '*' >OUTPUT
+cat <<EOF >EXPECTED
+["attachment",
 "inbox",
 "signed",
-"unread"]'
+"unread"]
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_done
-- 
1.7.4.4

[PATCH 04/25] test: move "Search for non-existent message prints nothing" to search-output, and add similar test for format=json

[Jameson Graef Rollins]

These tests also use the new test_expect_equal_file test.
---
 test/search        |    5 -----
 test/search-output |   12 ++++++++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/test/search b/test/search
index 0981f66..4835872 100755
--- a/test/search
+++ b/test/search
@@ -118,9 +118,4 @@ add_message '[subject]="utf8-message-body-subject"' '[date]="Sat, 01 Jan 2000 12
 output=$(notmuch search "bödý" | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; utf8-message-body-subject (inbox unread)"
 
-test_expect_success "Search for non-existent message prints nothing" '
-echo -n > expected &&
-notmuch search "no-message-matches-this" > actual &&
-test_cmp expected actual'
-
 test_done
diff --git a/test/search-output b/test/search-output
index 1554eb1..8ba8acf 100755
--- a/test/search-output
+++ b/test/search-output
@@ -304,4 +304,16 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "notmuch search for non-existent message prints nothing"
+notmuch search "no-message-matches-this" > OUTPUT
+echo -n >EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "notmuch search --format=json for non-existent message prints proper empty json"
+notmuch search --format=json "no-message-matches-this" > OUTPUT
+cat <<EOF >EXPECTED
+[]
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_done
-- 
1.7.4.4

[PATCH 05/25] fix trailing newlines in notmuch search

[Jameson Graef Rollins]

A previous commit to fix json formatting for null results
(0b1ddc5f6652bde99d63d9d553777b3d926694cf) accidentally introduced a
regression that removed trailing newlines for non-json output.  (There
wasn't a good test for this previously, but there is now).  The
problem is due to the fundamental differences in formatting between
the json and non-json outputs.  The only way to fix this was to add a
new formatting field that represents the string to output at the end
of a null result.

All output formatting tests should pass now.
---
 notmuch-search.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 8b90121..69af617 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -48,6 +48,7 @@ typedef struct search_format {
     const char *item_sep;
     const char *item_end;
     const char *results_end;
+    const char *results_null;
 } search_format_t;
 
 static void
@@ -72,6 +73,7 @@ static const search_format_t format_text = {
 		"%s", " ",
 	    ")", "\n",
 	"",
+    "\n",
     "",
 };
 
@@ -98,6 +100,7 @@ static const search_format_t format_json = {
 	    "]", ",\n",
 	"}",
     "]\n",
+    "]\n",
 };
 
 static void
@@ -236,7 +239,10 @@ do_search_threads (const search_format_t *format,
 	notmuch_thread_destroy (thread);
     }
 
-    fputs (format->results_end, stdout);
+    if (first_thread)
+	fputs (format->results_null, stdout);
+    else
+	fputs (format->results_end, stdout);
 
     return 0;
 }
@@ -280,7 +286,10 @@ do_search_messages (const search_format_t *format,
 
     notmuch_messages_destroy (messages);
 
-    fputs (format->results_end, stdout);
+    if (first_message)
+	fputs (format->results_null, stdout);
+    else
+	fputs (format->results_end, stdout);
 
     return 0;
 }
@@ -329,7 +338,10 @@ do_search_tags (notmuch_database_t *notmuch,
     if (messages)
 	notmuch_messages_destroy (messages);
 
-    fputs (format->results_end, stdout);
+    if (first_tag)
+	fputs (format->results_null, stdout);
+    else
+	fputs (format->results_end, stdout);
 
     return 0;
 }
-- 
1.7.4.4

[PATCH 06/25] test: move utf-8 subject search test from json to search test script

[Jameson Graef Rollins]

This test doesn't have anything to do with json, and has everything to
do with testing search capability, so I'm not sure why it was in the
wrong place.
---
 test/json   |    5 -----
 test/search |    6 ++++++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/test/json b/test/json
index 7c81856..0badb1d 100755
--- a/test/json
+++ b/test/json
@@ -18,11 +18,6 @@ test_expect_equal "$output" "[{\"thread\": \"XXX\",
 \"subject\": \"json-search-subject\",
 \"tags\": [\"inbox\", \"unread\"]}]"
 
-test_begin_subtest "Search by subject (utf-8):"
-add_message [subject]=utf8-sübjéct "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\""
-output=$(notmuch search subject:utf8-sübjéct | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; utf8-sübjéct (inbox unread)"
-
 test_begin_subtest "Show message: json, utf-8"
 add_message "[subject]=\"json-show-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-show-méssage\""
 output=$(notmuch show --format=json "jsön-show-méssage")
diff --git a/test/search b/test/search
index 4835872..e7c8c54 100755
--- a/test/search
+++ b/test/search
@@ -24,6 +24,11 @@ add_message [subject]=subjectsearchtest '[date]="Sat, 01 Jan 2000 12:00:00 -0000
 output=$(notmuch search subject:subjectsearchtest | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; subjectsearchtest (inbox unread)"
 
+test_begin_subtest "Search by subject (utf-8):"
+add_message [subject]=utf8-sübjéct '[date]="Sat, 01 Jan 2000 12:00:00 -0000"'
+output=$(notmuch search subject:utf8-sübjéct | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; utf8-sübjéct (inbox unread)"
+
 test_begin_subtest "Search by id:"
 add_message '[subject]="search by id"' '[date]="Sat, 01 Jan 2000 12:00:00 -0000"'
 output=$(notmuch search id:${gen_msg_id} | notmuch_search_sanitize)
@@ -101,6 +106,7 @@ thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; body search (inbox unread)
 thread:XXX   2000-01-01 [1/1] searchbyfrom; search by from (inbox unread)
 thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; search by to (inbox unread)
 thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; subjectsearchtest (inbox unread)
+thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; utf8-sübjéct (inbox unread)
 thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; search by id (inbox unread)
 thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; search by tag (inbox searchbytag unread)
 thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; search by thread (inbox unread)
-- 
1.7.4.4

[PATCH 07/25] test: remove json test for search null result, since it's being more properly tested in search-output

[Jameson Graef Rollins]

The test in json was inferior to the one in search-output, since it
wasn't properly testing for the presence of a trailing newline.
---
 test/json          |    4 ----
 test/search-output |    4 +---
 2 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/test/json b/test/json
index 0badb1d..5a2544c 100755
--- a/test/json
+++ b/test/json
@@ -34,8 +34,4 @@ test_expect_equal "$output" "[{\"thread\": \"XXX\",
 \"subject\": \"json-search-utf8-body-sübjéct\",
 \"tags\": [\"inbox\", \"unread\"]}]"
 
-test_begin_subtest "Search returning no messages"
-output=$(notmuch search --format=json "this string had better not match any messages" | notmuch_search_sanitize)
-test_expect_equal "$output" "[]"
-
 test_done
diff --git a/test/search-output b/test/search-output
index 8ba8acf..13f9613 100755
--- a/test/search-output
+++ b/test/search-output
@@ -311,9 +311,7 @@ test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "notmuch search --format=json for non-existent message prints proper empty json"
 notmuch search --format=json "no-message-matches-this" > OUTPUT
-cat <<EOF >EXPECTED
-[]
-EOF
+echo "[]" >EXPECTED
 test_expect_equal_file OUTPUT EXPECTED
 
 test_done
-- 
1.7.4.4

[PATCH 08/25] create and set temporary home directory

[Jameson Graef Rollins]

From: Florian Friesdorf <flo@chaoflow.net>

My first patch send to the list, not sure whether done properly.

In the master branch in test/emacs two tests access the build users home
directory, so does emacs_deliver_message in the crypto branch.

I think the tests should not touch the build user's home directory. The
patch creates a directory in the temporary test directory and sets home
accordingly.

In case of a non-existent home directory, the tests are failing without
this patch.

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 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 922b1ef..6a8cb98 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -973,6 +973,13 @@ rm -fr "$test" || {
 	exit 1
 }
 
+# A temporary home directory is needed by at least:
+# - emacs/"Sending a message via (fake) SMTP"
+# - emacs/"Reply within emacs"
+# - crypto/emacs_deliver_message
+export HOME="${TMP_DIRECTORY}/home"
+mkdir -p "${HOME}"
+
 MAIL_DIR="${TMP_DIRECTORY}/mail"
 export GNUPGHOME="${TMP_DIRECTORY}/gnupg"
 export NOTMUCH_CONFIG="${TMP_DIRECTORY}/notmuch-config"
-- 
1.7.4.4

[PATCH 09/25] emacs: Define several faces for the crypto-status button

[Jameson Graef Rollins]

From: Thomas Jost <schnouki@schnouki.net>

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>

jrollins modified this patch to conform to recent changes in the
crypto processing since this patch was originally sent in.
---
 emacs/notmuch-crypto.el |   42 ++++++++++++++++++++++++++++++++----------
 1 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index cb02840..f03266f 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -36,36 +36,58 @@ search."
   :group 'notmuch
   :type 'boolean)
 
+(defface notmuch-crypto-signature-good
+  '((t (:background "green" :foreground "black")))
+  "Face used for good signatures."
+  :group 'notmuch)
+
+(defface notmuch-crypto-signature-good-key
+  '((t (:background "orange" :foreground "black")))
+  "Face used for good signatures."
+  :group 'notmuch)
+
+(defface notmuch-crypto-signature-bad
+  '((t (:background "red" :foreground "black")))
+  "Face used for bad signatures."
+  :group 'notmuch)
+
+(defface notmuch-crypto-signature-unknown
+  '((t (:background "red" :foreground "black")))
+  "Face used for signatures of unknown status."
+  :group 'notmuch)
+
+(defface notmuch-crypto-decryption
+  '((t (:background "purple" :foreground "black")))
+  "Face used for encryption/decryption status messages."
+  :group 'notmuch)
+
 (define-button-type 'notmuch-crypto-status-button-type
   'action '(lambda (button) (message (button-get button 'help-echo)))
   'follow-link t
-  'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."
-  'face '(:foreground "blue")
-  'mouse-face '(:foreground "blue"))
+  'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")
 
 (defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
   (let* ((status (plist-get sigstatus :status))
 	 (help-msg nil)
 	 (label "Signature not processed")
-	 (face '(:background "red" :foreground "black")))
+	 (face 'notmuch-crypto-signature-unknown))
     (cond
      ((string= status "good")
       ; if userid present, userid has full or greater validity
       (if (plist-member sigstatus :userid)
 	  (let ((userid (plist-get sigstatus :userid)))
 	    (setq label (concat "Good signature by: " userid))
-	    (setq face '(:background "green" :foreground "black")))
+	    (setq face 'notmuch-crypto-signature-good))
 	(let ((fingerprint (concat "0x" (plist-get sigstatus :fingerprint))))
 	  (setq label (concat "Good signature by key: " fingerprint))
-	  (setq face '(:background "orange" :foreground "black")))))
+	  (setq face 'notmuch-crypto-signature-good-key))))
      ((string= status "error")
       (let ((keyid (concat "0x" (plist-get sigstatus :keyid))))
-	(setq label (concat "Unknown key ID " keyid " or unsupported algorithm"))
-	(setq face '(:background "red" :foreground "black"))))
+	(setq label (concat "Unknown key ID " keyid " or unsupported algorithm"))))
      ((string= status "bad")
       (let ((keyid (concat "0x" (plist-get sigstatus :keyid))))
 	(setq label (concat "Bad signature (claimed key ID " keyid ")"))
-	(setq face '(:background "red" :foreground "black"))))
+	(setq face 'notmuch-crypto-signature-bad)))
      (t
       (setq label "Unknown signature status")
       (if status (setq label (concat label " \"" status "\"")))))
@@ -83,7 +105,7 @@ search."
   (let* ((status (plist-get encstatus :status))
 	 (help-msg nil)
 	 (label "Decryption not attempted")
-	 (face '(:background "purple" :foreground "black")))
+	 (face 'notmuch-crypto-decryption))
     (cond
      ((string= status "good")
       (setq label "Decryption successful"))
-- 
1.7.4.4

[PATCH 10/25] Sanitize "Subject:" and "Author:" fields to not contain control characters in notmuch-search

[Jameson Graef Rollins]

From: Andreas Amann <a.amann@ucc.ie>

When a Subject field contained encoded CRLF sequences, these sequences
would appear unfiltered in the output of notmuch search. This confused
the notmuch emacs interface leading to "Unexpected Output"
messages. This is now fixed by replacing all characters with ASCII
code less than 32 with a question mark.

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 notmuch-search.c |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 69af617..530cecc 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -111,6 +111,20 @@ format_item_id_text (unused (const void *ctx),
     printf ("%s%s", item_type, item_id);
 }
 
+static char *
+sanitize_string (const void *ctx, const char *str)
+{
+    char *out, *loop;
+
+    loop = out = talloc_strdup (ctx, str);
+
+    for (; *loop; loop++) {
+	if ((unsigned char)(*loop) < 32)
+	    *loop = '?';
+    }
+    return out;
+}
+
 static void
 format_thread_text (const void *ctx,
 		    const char *thread_id,
@@ -120,13 +134,17 @@ format_thread_text (const void *ctx,
 		    const char *authors,
 		    const char *subject)
 {
+    void *ctx_quote = talloc_new (ctx);
+
     printf ("thread:%s %12s [%d/%d] %s; %s",
 	    thread_id,
 	    notmuch_time_relative_date (ctx, date),
 	    matched,
 	    total,
-	    authors,
-	    subject);
+	    sanitize_string (ctx_quote, authors),
+	    sanitize_string (ctx_quote, subject));
+
+    talloc_free (ctx_quote);
 }
 
 static void
-- 
1.7.4.4

[PATCH 11/25] test: add test for sanitized notmuch-search output

[Jameson Graef Rollins]

From: Florian Friesdorf <flo@chaoflow.net>

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 test/search-output |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/test/search-output b/test/search-output
index 13f9613..8edc26a 100755
--- a/test/search-output
+++ b/test/search-output
@@ -304,6 +304,15 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "santize output for quoted-printable line-breaks in author and subject"
+add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
+	headers'"
+notmuch search id:"$gen_msg_id" >OUTPUT
+cat <<EOF >EXPECTED
+thread:000000000000001b   2001-01-05 [1/1] Notmuch Test Suite; two line? subject headers (inbox unread)
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "notmuch search for non-existent message prints nothing"
 notmuch search "no-message-matches-this" > OUTPUT
 echo -n >EXPECTED
-- 
1.7.4.4

[PATCH 12/25] test: cleanup search-output test names (no functional change)

[Jameson Graef Rollins]

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 test/search-output |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/test/search-output b/test/search-output
index 8edc26a..02cf602 100755
--- a/test/search-output
+++ b/test/search-output
@@ -4,7 +4,7 @@ test_description='various settings for "notmuch search --output="'
 
 add_email_corpus
 
-test_begin_subtest "notmuch search --output=threads"
+test_begin_subtest "--output=threads"
 notmuch search --output=threads '*' | sed -e s/thread:.*/thread:THREADID/ >OUTPUT
 cat <<EOF >EXPECTED
 thread:THREADID
@@ -32,7 +32,7 @@ thread:THREADID
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search --format=json --output=threads"
+test_begin_subtest "--output=threads --format=json"
 notmuch search --format=json --output=threads '*' | sed -e s/\".*\"/\"THREADID\"/ >OUTPUT
 cat <<EOF >EXPECTED
 ["THREADID",
@@ -60,7 +60,7 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search --output=messages"
+test_begin_subtest "--output=messages"
 notmuch search --output=messages '*' >OUTPUT
 cat <<EOF >EXPECTED
 id:1258544095-16616-1-git-send-email-chris@chris-wilson.co.uk
@@ -116,7 +116,7 @@ id:1258471718-6781-1-git-send-email-dottedmag@dottedmag.net
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search --format=json --output=messages"
+test_begin_subtest "--output=messages --format=json"
 notmuch search --format=json --output=messages '*' >OUTPUT
 cat <<EOF >EXPECTED
 ["1258544095-16616-1-git-send-email-chris@chris-wilson.co.uk",
@@ -172,7 +172,7 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search --output=files"
+test_begin_subtest "--output=files"
 notmuch search --output=files '*' | sed -e "s,$MAIL_DIR,MAIL_DIR," >OUTPUT
 cat <<EOF >EXPECTED
 MAIL_DIR/cur/50:2,
@@ -228,7 +228,7 @@ MAIL_DIR/cur/01:2,
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search --format=json --output=files"
+test_begin_subtest "--output=files --format=json"
 notmuch search --format=json --output=files '*' | sed -e "s,$MAIL_DIR,MAIL_DIR," >OUTPUT
 cat <<EOF >EXPECTED
 ["MAIL_DIR/cur/50:2,",
@@ -284,7 +284,7 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search --output=tags"
+test_begin_subtest "--output=tags"
 notmuch search --output=tags '*' >OUTPUT
 cat <<EOF >EXPECTED
 attachment
@@ -294,7 +294,7 @@ unread
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search --format=json --output=tags"
+test_begin_subtest "--output=tags --format=json"
 notmuch search --format=json --output=tags '*' >OUTPUT
 cat <<EOF >EXPECTED
 ["attachment",
@@ -313,12 +313,12 @@ thread:000000000000001b   2001-01-05 [1/1] Notmuch Test Suite; two line? subject
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search for non-existent message prints nothing"
+test_begin_subtest "search for non-existent message prints nothing"
 notmuch search "no-message-matches-this" > OUTPUT
 echo -n >EXPECTED
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "notmuch search --format=json for non-existent message prints proper empty json"
+test_begin_subtest "search --format=json for non-existent message prints proper empty json"
 notmuch search --format=json "no-message-matches-this" > OUTPUT
 echo "[]" >EXPECTED
 test_expect_equal_file OUTPUT EXPECTED
-- 
1.7.4.4

[PATCH 13/25] emacs: Use "message-cited-text" instead of "message-cited-text-face"

[Jameson Graef Rollins]

From: Pieter Praet <pieter@praet.org>

(describe-face 'message-cited-text-face)
> message-cited-text-face is an alias for the face `message-cited-text'.
> This face is obsolete since 22.1; use `message-cited-text' instead.

Signed-off-by: Pieter Praet <pieter@praet.org>
Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 emacs/notmuch-wash.el |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index e8134bf..115c3bb 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -156,7 +156,7 @@ insert before the button, probably for indentation."
     (let* ((cite-start (match-beginning 0))
 	   (cite-end (match-end 0))
 	   (cite-lines (count-lines cite-start cite-end)))
-      (overlay-put (make-overlay cite-start cite-end) 'face 'message-cited-text-face)
+      (overlay-put (make-overlay cite-start cite-end) 'face 'message-cited-text)
       (when (> cite-lines (+ notmuch-wash-citation-lines-prefix
 			     notmuch-wash-citation-lines-suffix
 			     1))
@@ -178,7 +178,7 @@ insert before the button, probably for indentation."
 		  (sig-end-marker (make-marker)))
 	      (set-marker sig-start-marker sig-start)
 	      (set-marker sig-end-marker (point-max))
-	      (overlay-put (make-overlay sig-start-marker sig-end-marker) 'face 'message-cited-text-face)
+	      (overlay-put (make-overlay sig-start-marker sig-end-marker) 'face 'message-cited-text)
 	      (notmuch-wash-region-to-button
 	       sig-start-marker sig-end-marker
 	       "signature" "\n"))))))
-- 
1.7.4.4

[PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Jameson Graef Rollins]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

In notmuch 0.5 notmuch-fcc-dirs style changed.  The previous code
did not correctly identify an old configuration and, as a
consequence, broke new configurations.

The fix was extracted from a bigger patch series by David
Edmondson id:"1290682750-30283-2-git-send-email-dme@dme.org".

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 emacs/notmuch-maildir-fcc.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
index 3f1c124..b79f6a3 100644
--- a/emacs/notmuch-maildir-fcc.el
+++ b/emacs/notmuch-maildir-fcc.el
@@ -88,7 +88,7 @@ will NOT be removed or replaced."
 	   notmuch-fcc-dirs)
 
 	  ((and (listp notmuch-fcc-dirs)
-		(= 1 (length (car notmuch-fcc-dirs))))
+		(stringp (car notmuch-fcc-dirs)))
 	   ;; Old style - no longer works.
 	   (error "Invalid `notmuch-fcc-dirs' setting (old style)"))
 
-- 
1.7.4.4

[PATCH 15/25] Use message-field-value instead of message-fetch-field in FCC header setup.

[Jameson Graef Rollins]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

For message-fetch-field the buffer is expected to be narrowed to
just the header of the message.  That is not the case when
notmuch-fcc-header-setup is run, hence a wrong header value may be
returned.  E.g. when forwarding an
email, (message-fetch-field "From") returns the From header value
of the forwarded email.

Message-field-value is the same as message-fetch-field, only
narrows the buffer to the headers first.

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 emacs/notmuch-maildir-fcc.el |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
index b79f6a3..b6c6e2a 100644
--- a/emacs/notmuch-maildir-fcc.el
+++ b/emacs/notmuch-maildir-fcc.el
@@ -80,7 +80,7 @@ will NOT be removed or replaced."
   (let ((subdir
 	 (cond
 	  ((or (not notmuch-fcc-dirs)
-	       (message-fetch-field "Fcc"))
+	       (message-field-value "Fcc"))
 	   ;; Nothing set or an existing header.
 	   nil)
 
@@ -93,7 +93,7 @@ will NOT be removed or replaced."
 	   (error "Invalid `notmuch-fcc-dirs' setting (old style)"))
 
 	  ((listp notmuch-fcc-dirs)
-	   (let* ((from (message-fetch-field "From"))
+	   (let* ((from (message-field-value "From"))
 		  (match
 		   (catch 'first-match
 		     (dolist (re-folder notmuch-fcc-dirs)
@@ -118,7 +118,7 @@ will NOT be removed or replaced."
 		  (concat (notmuch-database-path) "/" subdir)))))
       
       ;; finally test if fcc points to a valid maildir
-      (let ((fcc-header (message-fetch-field "Fcc")))
+      (let ((fcc-header (message-field-value "Fcc")))
 	(unless (notmuch-maildir-fcc-dir-is-maildir-p fcc-header)
 	  (cond ((not (file-writable-p fcc-header))
 		 (error (format "No permission to create %s, which does not exist"
-- 
1.7.4.4

[PATCH 16/25] lib/message-file: plug three memleaks.

[Jameson Graef Rollins]

From: Anton Khirnov <anton@khirnov.net>

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 lib/message-file.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/lib/message-file.c b/lib/message-file.c
index 7722832..99505c1 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -341,12 +341,17 @@ notmuch_message_file_get_header (notmuch_message_file_t *message,
 		strncpy(combined_header,header_sofar,hdrsofar);
 		*(combined_header+hdrsofar) = ' ';
 		strncpy(combined_header+hdrsofar+1,decoded_value,newhdr+1);
+		free (decoded_value);
 		g_hash_table_insert (message->headers, header, combined_header);
 	    }
 	} else {
 	    if (header_sofar == NULL) {
 		/* Only insert if we don't have a value for this header, yet. */
 		g_hash_table_insert (message->headers, header, decoded_value);
+	    } else {
+		free (header);
+		free (decoded_value);
+		decoded_value = header_sofar;
 	    }
 	}
 	/* if we found a match we can bail - unless of course we are
-- 
1.7.4.4

[PATCH 17/25] avoid segfault when calling sanitize_string() on NULL

[Jameson Graef Rollins]

From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 notmuch-search.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 530cecc..616fe68 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -116,6 +116,9 @@ sanitize_string (const void *ctx, const char *str)
 {
     char *out, *loop;
 
+    if (NULL == str)
+	return NULL;
+
     loop = out = talloc_strdup (ctx, str);
 
     for (; *loop; loop++) {
-- 
1.7.4.4

[PATCH 18/25] Fix search output sanitization test.

[Jameson Graef Rollins]

From: Austin Clements <amdragon@MIT.EDU>

Previously, the test assumed the generated message would be assigned a
specific thread ID; now it doesn't.  Also, spelling fix.

Signed-off-by: Jameson Graef Rollins <jrollins@finestructure.net>
---
 test/search-output |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/search-output b/test/search-output
index 02cf602..3c875cd 100755
--- a/test/search-output
+++ b/test/search-output
@@ -304,12 +304,12 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "santize output for quoted-printable line-breaks in author and subject"
+test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
 add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
 	headers'"
-notmuch search id:"$gen_msg_id" >OUTPUT
+notmuch search id:"$gen_msg_id" | notmuch_search_sanitize >OUTPUT
 cat <<EOF >EXPECTED
-thread:000000000000001b   2001-01-05 [1/1] Notmuch Test Suite; two line? subject headers (inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; two line? subject headers (inbox unread)
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-- 
1.7.4.4

[PATCH 19/25] emacs: fix notmuch-show-part-button to not include newline

[Jameson Graef Rollins]

This makes the button cleaner, so that it doesn't include the entire
rest of the line that the button is on.
---
 emacs/notmuch-show.el |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 6c498ab..a25ad99 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -280,18 +280,23 @@ message at DEPTH in the current thread."
   'face 'message-mml)
 
 (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
-  (insert-button
-   (concat "[ "
-	   (if name (concat name ": ") "")
-	   declared-type
-	   (if (not (string-equal declared-type content-type))
-	       (concat " (as " content-type ")")
-	     "")
-	   (or comment "")
-	   " ]\n")
-   :type 'notmuch-show-part-button-type
-   :notmuch-part nth
-   :notmuch-filename name))
+  (let ((button))
+    (setq button
+	  (insert-button
+	   (concat "[ "
+		   (if name (concat name ": ") "")
+		   declared-type
+		   (if (not (string-equal declared-type content-type))
+		       (concat " (as " content-type ")")
+		     "")
+		   (or comment "")
+		   " ]")
+	   :type 'notmuch-show-part-button-type
+	   :notmuch-part nth
+	   :notmuch-filename name))
+    (insert "\n")
+    ;; return button
+    button))
 
 ;; Functions handling particular MIME parts.
 
-- 
1.7.4.4

[PATCH 20/25] emacs: Don't always prompt for the "From" address when replying

[Jameson Graef Rollins]

From: Thomas Jost <schnouki@schnouki.net>

When replying, the From: address is already filled in by notmuch reply, so most
of the time there is no need to prompt the user for it.
---
 emacs/notmuch-mua.el |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 7c05a81..556d2bf 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -155,7 +155,10 @@ name and addresses configured in the notmuch configuration file."
   :type '(repeat string))
 
 (defcustom notmuch-always-prompt-for-sender nil
-  "Always prompt for the From: address when composing a new message."
+  "Always prompt for the From: address when composing or forwarding a message.
+
+This is not taken into account when replying to a message, because in that case
+the From: header is already filled in by notmuch."
   :group 'notmuch
   :type 'boolean)
 
@@ -203,7 +206,7 @@ the From: address first."
   "Invoke the notmuch reply window."
   (interactive "P")
   (let ((sender
-	 (when (or prompt-for-sender notmuch-always-prompt-for-sender)
+	 (when prompt-for-sender
 	   (notmuch-mua-prompt-for-sender))))
     (notmuch-mua-reply query-string sender)))
 
-- 
1.7.4.4

[PATCH 21/25] emacs: Cleaner interface when prompting for sender address

[Jameson Graef Rollins]

From: Thomas Jost <schnouki@schnouki.net>

Most of the time, every entry in the list of identities has the same user name
part. It can then be filled in automatically, and the user can only be prompted
for the email address, which makes the interface much cleaner.
---
 emacs/notmuch-mua.el |   33 +++++++++++++++++++++++----------
 1 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 556d2bf..274c5da 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -162,20 +162,33 @@ the From: header is already filled in by notmuch."
   :group 'notmuch
   :type 'boolean)
 
-(defun notmuch-mua-sender-collection ()
-  (if notmuch-identities
-      notmuch-identities
-    (mapcar (lambda (address)
-	      (concat (notmuch-user-name) " <" address ">"))
-	    (cons (notmuch-user-primary-email) (notmuch-user-other-email)))))
-
 (defvar notmuch-mua-sender-history nil)
 
 (defun notmuch-mua-prompt-for-sender ()
   (interactive)
-  (let ((collection (notmuch-mua-sender-collection)))
-    (ido-completing-read "Send mail From: " collection
-			 nil 'confirm nil 'notmuch-mua-sender-history (car collection))))
+  (let (name addresses one-name-only)
+    ;; If notmuch-identities is non-nil, check if there is a fixed user name.
+    (if notmuch-identities
+	(let ((components (mapcar 'mail-extract-address-components notmuch-identities)))
+	  (setq name          (caar components)
+		addresses     (mapcar 'cadr components)
+		one-name-only (eval
+			       (cons 'and
+				     (mapcar (lambda (identity)
+					       (string-equal name (car identity)))
+					     components)))))
+      ;; If notmuch-identities is nil, use values from the notmuch configuration file.
+      (setq name          (notmuch-user-name)
+	    addresses     (cons (notmuch-user-primary-email) (notmuch-user-other-email))
+	    one-name-only t))
+    ;; Now prompt the user, either for an email address only or for a full identity.
+    (if one-name-only
+	(let ((address
+	       (ido-completing-read (concat "Sender address for " name ": ") addresses
+				    nil nil nil 'notmuch-mua-sender-history (car addresses))))
+	  (concat name " <" address ">"))
+      (ido-completing-read "Send mail From: " notmuch-identities
+			   nil nil nil 'notmuch-mua-sender-history (car notmuch-identities)))))
 
 (defun notmuch-mua-new-mail (&optional prompt-for-sender)
   "Invoke the notmuch mail composition window.
-- 
1.7.4.4

[PATCH 22/25] test: fix test_expect_equal_file test to copy instead of mv test files

[Jameson Graef Rollins]

This is a pretty important fix to this test, so that it can be used to
test against existing files, without deleting them in the case of test
failure.
---
 test/test-lib.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 6a8cb98..a59d1c1 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -477,8 +477,8 @@ test_expect_equal_file ()
 			test_ok_ "$test_subtest_name"
 		else
 			testname=$this_test.$test_count
-			mv "$output" $testname.output
-			mv "$expected" $testname.expected
+			cp "$output" $testname.output
+			cp "$expected" $testname.expected
 			test_failure_ "$test_subtest_name" "$(diff -u $testname.expected $testname.output)"
 		fi
     fi
-- 
1.7.4.4

[PATCH 23/25] test: update emacs test to use test_expect_equal_file

[Jameson Graef Rollins]

This is a much cleaner way to do the emacs tests, since we're actually
comparing output against existing files with expected output.  We also
won't miss any trailing newlines this way.

And speaking of which, one of the expected output files was actually
missing a trailing blank line that was actually in one of the original
messages, so this was fixed.
---
 test/emacs                                         |   80 +++++++++----------
 .../notmuch-show-thread-maildir-storage            |    1 +
 2 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/test/emacs b/test/emacs
index 042f1cb..f14249e 100755
--- a/test/emacs
+++ b/test/emacs
@@ -7,39 +7,33 @@ EXPECTED=../emacs.expected-output
 add_email_corpus
 
 test_begin_subtest "Basic notmuch-hello view in emacs"
-output=$(test_emacs '(notmuch-hello) (princ (buffer-string))')
-expected=$(cat $EXPECTED/notmuch-hello)
-test_expect_equal "$output" "$expected"
+test_emacs '(notmuch-hello) (princ (buffer-string))' >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello
 
 test_begin_subtest "Saved search with 0 results"
-output=$(test_emacs '(setq notmuch-show-empty-saved-searches t) (setq notmuch-saved-searches '\''(("inbox" . "tag:inbox") ("unread" . "tag:unread") ("empty" . "tag:doesnotexist"))) (notmuch-hello) (princ (buffer-string))')
-expected=$(cat $EXPECTED/notmuch-hello-with-empty)
-test_expect_equal "$output" "$expected"
+test_emacs '(setq notmuch-show-empty-saved-searches t) (setq notmuch-saved-searches '\''(("inbox" . "tag:inbox") ("unread" . "tag:unread") ("empty" . "tag:doesnotexist"))) (notmuch-hello) (princ (buffer-string))' >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-with-empty
 
 test_begin_subtest "No saved searches displayed (all with 0 results)"
-output=$(test_emacs '(setq notmuch-saved-searches '\''(("empty" . "tag:doesnotexist"))) (notmuch-hello) (princ (buffer-string))')
-expected=$(cat $EXPECTED/notmuch-hello-no-saved-searches)
-test_expect_equal "$output" "$expected"
+test_emacs '(setq notmuch-saved-searches '\''(("empty" . "tag:doesnotexist"))) (notmuch-hello) (princ (buffer-string))' >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
 
 test_begin_subtest "Basic notmuch-search view in emacs"
-output=$(test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) (princ (buffer-string))')
-expected=$(cat $EXPECTED/notmuch-search-tag-inbox)
-test_expect_equal "$output" "$expected"
+test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) (princ (buffer-string))' >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-search-tag-inbox
 
 test_begin_subtest "Navigation of notmuch-hello to search results"
-output=$(test_emacs '(notmuch-hello) (goto-char (point-min)) (re-search-forward "inbox") (widget-button-press (point)) (notmuch-test-wait) (princ (buffer-string))')
-expected=$(cat $EXPECTED/notmuch-hello-view-inbox)
-test_expect_equal "$output" "$expected"
+test_emacs '(notmuch-hello) (goto-char (point-min)) (re-search-forward "inbox") (widget-button-press (point)) (notmuch-test-wait) (princ (buffer-string))' >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-view-inbox
 
 test_begin_subtest "Basic notmuch-show view in emacs"
 maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
-output=$(test_emacs "(notmuch-show \"$maildir_storage_thread\") (princ (buffer-string))")
-expected=$(cat $EXPECTED/notmuch-show-thread-maildir-storage)
-test_expect_equal "$output" "$expected"
+test_emacs "(notmuch-show \"$maildir_storage_thread\") (princ (buffer-string))" >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "Navigation of notmuch-search to thread view"
-output=$(test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) (goto-char (point-min)) (re-search-forward "Working with Maildir") (notmuch-search-show-thread) (notmuch-test-wait) (princ (buffer-string))')
-test_expect_equal "$output" "$expected"
+test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) (goto-char (point-min)) (re-search-forward "Working with Maildir") (notmuch-search-show-thread) (notmuch-test-wait) (princ (buffer-string))' >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "Add tag from search view"
 os_x_darwin_thread=$(notmuch search --output=threads id:ddd65cda0911171950o4eea4389v86de9525e46052d3@mail.gmail.com)
@@ -80,10 +74,11 @@ smtp_dummy_pid=$!
 test_emacs "(setq message-send-mail-function 'message-smtpmail-send-it) (setq smtpmail-smtp-server \"localhost\") (setq smtpmail-smtp-service \"25025\") (notmuch-hello) (notmuch-mua-mail) (message-goto-to) (insert \"user@example.com\nDate: Fri, 29 Mar 1974 10:00:00 -0000\") (message-goto-subject) (insert \"Testing message sent via SMTP\") (message-goto-body) (insert \"This is a test that messages are sent via SMTP\") (message-send-and-exit)" >/dev/null 2>&1
 wait ${smtp_dummy_pid}
 
-output=$(sed \
+sed \
     -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \
-    -e s',^Message-ID: <.*>$,Message-ID: <XXX>,' < sent_message)
-test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
+    -e s',^Message-ID: <.*>$,Message-ID: <XXX>,' < sent_message >OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: user@example.com
 Subject: Testing message sent via SMTP
 Date: Fri, 29 Mar 1974 10:00:00 -0000
@@ -92,7 +87,9 @@ Message-ID: <XXX>
 MIME-Version: 1.0
 Content-Type: text/plain; charset=us-ascii
 
-This is a test that messages are sent via SMTP"
+This is a test that messages are sent via SMTP
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Verify that sent messages are saved/searchable (via FCC)"
 notmuch new > /dev/null
@@ -102,37 +99,36 @@ test_expect_equal "$output" "thread:XXX   1974-03-29 [1/1] Notmuch Test Suite; T
 test_begin_subtest "Reply within emacs"
 # We sed away everything before the ^From in the output to avoid getting
 # confused by messages such as "Parsing /home/cworth/.mailrc... done"
-output=$(test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"") (notmuch-test-wait) (notmuch-search-reply-to-thread) (princ (buffer-string))' | sed -ne '/^From/,$ p' | sed -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/')
-test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
+test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"") (notmuch-test-wait) (notmuch-search-reply-to-thread) (princ (buffer-string))' | sed -ne '/^From/,$ p' | sed -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' >OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: user@example.com
 Subject: Re: Testing message sent via SMTP
 In-Reply-To: <XXX>
 Fcc: $(pwd)/mail/sent
 --text follows this line--
 On Fri, 29 Mar 1974 10:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
-> This is a test that messages are sent via SMTP"
+> This is a test that messages are sent via SMTP
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Save attachment from within emacs"
 echo "./attachment" | test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com") (notmuch-show-save-attachments)' > /dev/null 2>&1
-output=$(cat attachment)
-expected=$(cat $EXPECTED/attachment)
-test_expect_equal "$output" "$expected"
+test_expect_equal_file attachment $EXPECTED/attachment
 
 test_begin_subtest "View raw message within emacs"
-expected=$(cat $EXPECTED/raw-message-cf0c4d-52ad0a)
-first_line=$(echo "$expected" | head -n1)
-output=$(test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com") (notmuch-show-view-raw-message) (princ (buffer-string))' | sed -ne "/$first_line/,\$ p")
-test_expect_equal "$output" "$expected"
+first_line=$(head -n1 $EXPECTED/raw-message-cf0c4d-52ad0a)
+test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com") (notmuch-show-view-raw-message) (princ (buffer-string))' | sed -ne "/$first_line/,\$ p" >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/raw-message-cf0c4d-52ad0a
 
 test_begin_subtest "Hiding/showing signature in notmuch-show view"
 maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
-output=$(test_emacs "(notmuch-show \"$maildir_storage_thread\")
-		     (search-forward \"Click/Enter to show.\")
-		     (button-activate (button-at (point)))
-		     (search-backward \"Click/Enter to hide.\")
-		     (button-activate (button-at (point)))
-		     (princ (buffer-string))")
-expected=$(cat $EXPECTED/notmuch-show-thread-maildir-storage)
-test_expect_equal "$output" "$expected"
+test_emacs "(notmuch-show \"$maildir_storage_thread\")
+	    (search-forward \"Click/Enter to show.\")
+	    (button-activate (button-at (point)))
+	    (search-backward \"Click/Enter to hide.\")
+	    (button-activate (button-at (point)))
+	    (princ (buffer-string))" >OUTPUT
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_done
diff --git a/test/emacs.expected-output/notmuch-show-thread-maildir-storage b/test/emacs.expected-output/notmuch-show-thread-maildir-storage
index 241b5b7..8ba64b2 100644
--- a/test/emacs.expected-output/notmuch-show-thread-maildir-storage
+++ b/test/emacs.expected-output/notmuch-show-thread-maildir-storage
@@ -212,3 +212,4 @@ http://notmuchmail.org/mailman/listinfo/notmuch
  Happy hacking,
 
  -Carl
+
-- 
1.7.4.4

[PATCH 24/25] test: modify multipart test to use test_expect_equal_file

[Jameson Graef Rollins]

Again, this is a much cleaner and more thorough test, and in fact
exposes a bug in the format=text output, that will be fixed the next
commit.  Because of this, some of the multipart tests currently fail.
---
 test/multipart |  189 ++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 121 insertions(+), 68 deletions(-)

diff --git a/test/multipart b/test/multipart
index 62efd84..6d194a5 100755
--- a/test/multipart
+++ b/test/multipart
@@ -59,9 +59,9 @@ EOF
 notmuch new > /dev/null
 
 test_begin_subtest "--format=text --part=0, full message"
-output=$(notmuch show --format=text --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-"\fmessage{ id:87liy5ap00.fsf@yoom.home.cworth.org depth:0 match:1 filename:${MAIL_DIR}/multipart
+notmuch show --format=text --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+\fmessage{ id:87liy5ap00.fsf@yoom.home.cworth.org depth:0 match:1 filename:${MAIL_DIR}/multipart
 \fheader{
 Carl Worth <cworth@cworth.org> (2001-01-05) (attachment inbox signed unread)
 Subject: Multipart message
@@ -92,12 +92,23 @@ Non-text part: application/pgp-signature
 \fpart}
 \fpart}
 \fbody}
-\fmessage}"
+\fmessage}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "--format=text --part=1, message body"
-output=$(notmuch show --format=text --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-"\fpart{ ID: 1, Content-type: multipart/signed
+test_begin_subtest "--format=text --part=0, full message"
+notmuch show --format=text --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+\fmessage{ id:87liy5ap00.fsf@yoom.home.cworth.org depth:0 match:1 filename:${MAIL_DIR}/multipart
+\fheader{
+Carl Worth <cworth@cworth.org> (2001-01-05) (attachment inbox signed unread)
+Subject: Multipart message
+From: Carl Worth <cworth@cworth.org>
+To: cworth@cworth.org
+Date: Tue, 05 Jan 2001 15:43:57 -0000
+\fheader}
+\fbody{
+\fpart{ ID: 1, Content-type: multipart/signed
 \fpart{ ID: 2, Content-type: multipart/mixed
 \fpart{ ID: 3, Content-type: message/rfc822
 \fpart{ ID: 4, Content-type: text/html
@@ -117,12 +128,16 @@ And this message is signed.
 \fpart{ ID: 7, Content-type: application/pgp-signature
 Non-text part: application/pgp-signature
 \fpart}
-\fpart}"
+\fpart}
+\fbody}
+\fmessage}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=text --part=2, multipart/mixed"
-output=$(notmuch show --format=text --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-"\fpart{ ID: 2, Content-type: multipart/mixed
+notmuch show --format=text --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+\fpart{ ID: 2, Content-type: multipart/mixed
 \fpart{ ID: 3, Content-type: message/rfc822
 \fpart{ ID: 4, Content-type: text/html
 Non-text part: text/html
@@ -137,47 +152,59 @@ And this message is signed.
 
 -Carl
 \fpart}
-\fpart}"
+\fpart}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=text --part=3, rfc822 multipart"
-output=$(notmuch show --format=text --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-"\fpart{ ID: 3, Content-type: message/rfc822
+notmuch show --format=text --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+\fpart{ ID: 3, Content-type: message/rfc822
 \fpart{ ID: 4, Content-type: text/html
 Non-text part: text/html
 \fpart}
-\fpart}"
+\fpart}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=text --part=4, html part"
-output=$(notmuch show --format=text --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-"\fpart{ ID: 4, Content-type: text/html
+notmuch show --format=text --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+\fpart{ ID: 4, Content-type: text/html
 Non-text part: text/html
-\fpart}"
+\fpart}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=text --part=5, inline attachement"
-output=$(notmuch show --format=text --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-"\fattachment{ ID: 5, Content-type: text/plain
+notmuch show --format=text --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+\fattachment{ ID: 5, Content-type: text/plain
 Attachment: attachment (text/plain)
 This is a text attachment.
-\fattachment}"
+\fattachment}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=text --part=6, plain text part"
-output=$(notmuch show --format=text --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-"\fpart{ ID: 6, Content-type: text/plain
+notmuch show --format=text --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+\fpart{ ID: 6, Content-type: text/plain
 And this message is signed.
 
 -Carl
-\fpart}"
+\fpart}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=text --part=7, pgp signature (unverified)"
-output=$(notmuch show --format=text --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-"\fpart{ ID: 7, Content-type: application/pgp-signature
+notmuch show --format=text --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+\fpart{ ID: 7, Content-type: application/pgp-signature
 Non-text part: application/pgp-signature
-\fpart}"
+\fpart}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_expect_success \
     "--format=text --part=8, no part, expect error" \
@@ -197,11 +224,13 @@ test_begin_subtest "--format=json --part=2, multipart/mixed"
 output=$(notmuch show --format=json --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org')
 test_expect_equal "$output" \
 '{"id": 2, "content-type": "multipart/mixed", "content": [{"id": 3, "content-type": "message/rfc822", "content": [{"id": 4, "content-type": "text/html"}]}, {"id": 5, "content-type": "text/plain", "filename": "attachment", "content": "This is a text attachment.\n"}, {"id": 6, "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}]}'
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=3, rfc822 multipart"
 output=$(notmuch show --format=json --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org')
 test_expect_equal "$output" \
 '{"id": 3, "content-type": "message/rfc822", "content": [{"id": 4, "content-type": "text/html"}]}'
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=4, html part"
 output=$(notmuch show --format=json --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org')
@@ -228,17 +257,19 @@ test_expect_success \
     "notmuch show --format=json --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "--format=raw"
-output=$(notmuch show --format=raw 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" "$(cat "${MAIL_DIR}"/multipart)"
+notmuch show --format=raw 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+test_expect_equal_file OUTPUT "${MAIL_DIR}"/multipart
 
 test_begin_subtest "--format=raw --part=0, full message"
-output=$(notmuch show --format=raw --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" "$(cat "${MAIL_DIR}"/multipart)"
+notmuch show --format=raw --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+test_expect_equal_file OUTPUT "${MAIL_DIR}"/multipart
 
 test_begin_subtest "--format=raw --part=1, message body"
-output=$(notmuch show --format=raw --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-'<p>This is an imbeded message, with a single html part.</p>
+notmuch show --format=raw --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+# output should *not* include newline
+echo >>OUTPUT
+cat <<EOF >EXPECTED
+<p>This is an imbeded message, with a single html part.</p>
 This is a text attachment.
 And this message is signed.
 
@@ -249,65 +280,86 @@ Version: GnuPG v1.4.11 (GNU/Linux)
 iEYEARECAAYFAk3SA/gACgkQ6JDdNq8qSWj0sACghqVJEQJUs3yV8zbTzhgnSIcD
 W6cAmQE4dcYrx/LPLtYLZm1jsGauE5hE
 =zkga
------END PGP SIGNATURE-----'
+-----END PGP SIGNATURE-----
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=raw --part=2, multipart/mixed"
-output=$(notmuch show --format=raw --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-'<p>This is an imbeded message, with a single html part.</p>
+notmuch show --format=raw --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+<p>This is an imbeded message, with a single html part.</p>
 This is a text attachment.
 And this message is signed.
 
--Carl'
+-Carl
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=raw --part=3, rfc822 multipart"
-output=$(notmuch show --format=raw --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-'<p>This is an imbeded message, with a single html part.</p>'
+notmuch show --format=raw --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+<p>This is an imbeded message, with a single html part.</p>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=raw --part=4, html part"
-output=$(notmuch show --format=raw --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-'<p>This is an imbeded message, with a single html part.</p>'
+notmuch show --format=raw --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+<p>This is an imbeded message, with a single html part.</p>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=raw --part=5, inline attachment"
-output=$(notmuch show --format=raw --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-'This is a text attachment.'
+notmuch show --format=raw --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+This is a text attachment.
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=raw --part=6, plain text part"
-output=$(notmuch show --format=raw --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-'And this message is signed.
+notmuch show --format=raw --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+And this message is signed.
 
--Carl'
+-Carl
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=raw --part=7, pgp signature (unverified)"
-output=$(notmuch show --format=raw --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" \
-'-----BEGIN PGP SIGNATURE-----
+notmuch show --format=raw --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+# output should *not* include newline
+echo >>OUTPUT
+cat <<EOF >EXPECTED
+-----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.11 (GNU/Linux)
 
 iEYEARECAAYFAk3SA/gACgkQ6JDdNq8qSWj0sACghqVJEQJUs3yV8zbTzhgnSIcD
 W6cAmQE4dcYrx/LPLtYLZm1jsGauE5hE
 =zkga
------END PGP SIGNATURE-----'
+-----END PGP SIGNATURE-----
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_expect_success \
     "--format=raw --part=8, no part, expect error" \
     "notmuch show --format=raw --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "--format=mbox"
-output=$(notmuch show --format=mbox 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" "$(printf "From cworth@cworth.org Fri Jan  5 15:43:57 2001\n"; cat "${MAIL_DIR}"/multipart)"
+notmuch show --format=mbox 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+printf "From cworth@cworth.org Fri Jan  5 15:43:57 2001\n" >EXPECTED
+cat "${MAIL_DIR}"/multipart >>EXPECTED
+# mbox output is expected to include a blank line
+echo >>EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
 
 test_expect_success \
     "--format=mbox --part=1, incompatible, expect error" \
     "! notmuch show --format=mbox --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "'notmuch reply' to a multipart message"
-output=$(notmuch reply 'id:87liy5ap00.fsf@yoom.home.cworth.org')
-test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
+notmuch reply 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
 Subject: Re: Multipart message
 To: Carl Worth <cworth@cworth.org>, cworth@cworth.org
 In-Reply-To: <87liy5ap00.fsf@yoom.home.cworth.org>
@@ -322,7 +374,8 @@ Non-text part: text/html
 > And this message is signed.
 > 
 > -Carl
-Non-text part: application/pgp-signature"
-
+Non-text part: application/pgp-signature
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_done
-- 
1.7.4.4

[PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

[Jameson Graef Rollins]

The declaration of the GMimeStream pointer to stdout in
format_part_content_text was somehow preventing subsequent printf
calls from outputting to stdout if the output was redirected to a
file.  Scoping the declaration to the actual use of the stream pointer
works around this problem.
---
 notmuch-show.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index e90f07e..61c6f49 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -447,7 +447,6 @@ format_part_content_text (GMimeObject *part)
 {
     GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part);
     GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
-    GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
 
     printf (", Content-type: %s\n", g_mime_content_type_to_string (content_type));
 
@@ -462,6 +461,7 @@ format_part_content_text (GMimeObject *part)
     if (g_mime_content_type_is_type (content_type, "text", "*") &&
 	!g_mime_content_type_is_type (content_type, "text", "html"))
     {
+	GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
 	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
 	show_part_content (part, stream_stdout);
 	g_object_unref(stream_stdout);
-- 
1.7.4.4

Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]

On Sat, 28 May 2011 14:52:00 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> The declaration of the GMimeStream pointer to stdout in
> format_part_content_text was somehow preventing subsequent printf
> calls from outputting to stdout if the output was redirected to a
> file.  Scoping the declaration to the actual use of the stream pointer
> works around this problem.

This commit message sounds like we don't actually understand the problem
being fixed here.

I'd like to investigate this more. Perhaps with a test case?

Otherwise, the patches up to this point in the thread have all either
been pushed or I've asked for some additional information (perhaps
that's just this patch and the "old style fcc dirs" patch?).

I'll continue to work through my patch queue as contained in email
messages. I'm finding it easier to do that (and just piping the patches
I like to "git am") as opposed to working through a release-candidate
branch in git, (and having to keep rebasing/postponing it after I reject
some patch in the series).

I'm actually a bit surprised to see myself preferring patches in
email. When Linus first wrote git, I couldn't understand why the Linux
community kept to such a consistent culture of sending patches via
email. It seemed so backwards to do these awkward machinations (git
format-patch, git send-email, SMTP, MUA, git am), and risk all the
problems of email clients corrupting patches, etc.—especially when git
has such clean mechanisms for reliably moving patches around (git push,
git pull).

Call me converted. Email really is the right way to do collaborative
code development.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]

On Fri, 03 Jun 2011 12:56:50 -0700, Carl Worth <cworth@cworth.org> wrote:
> On Sat, 28 May 2011 14:52:00 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > The declaration of the GMimeStream pointer to stdout in
> > format_part_content_text was somehow preventing subsequent printf
> > calls from outputting to stdout if the output was redirected to a
> > file.  Scoping the declaration to the actual use of the stream pointer
> > works around this problem.
> 
> This commit message sounds like we don't actually understand the problem
> being fixed here.

Well actually it's only meant to sound like the committer doesn't
understand the problem!

> I'd like to investigate this more. Perhaps with a test case?

The current tests are how I found the problem!  Without this patch at
least the multipart tests will fail.  I don't see how another test will
add anything.

Carl, if you (or anyone else) understands what the issue is, then please
go ahead and modify the commit message.  I don't understand things
enough myself to do any better.  Clearly there is some strange
interaction with things that try to use stdout after
g_mime_stream_file_new() has already grabbed it.

I really wouldn't block on this, though, since the patch does fix an
actual bug.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

On Fri, 03 Jun 2011 14:26:48 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Well actually it's only meant to sound like the committer doesn't
> understand the problem!

Heh, OK.

> > I'd like to investigate this more. Perhaps with a test case?
> 
> The current tests are how I found the problem!  Without this patch at
> least the multipart tests will fail.  I don't see how another test will
> add anything.

Ah, in my review I'd managed to get this commit detached from the
original previous commit that introduced the test failures. It's funny
that while you were replying I was reviewing *that* commit and thinking,
"why are all these tests failing now just because they changed to
test_expect_equal_file"?

> Carl, if you (or anyone else) understands what the issue is, then please
> go ahead and modify the commit message.

Done.

>                                         I don't understand things
> enough myself to do any better.  Clearly there is some strange
> interaction with things that try to use stdout after
> g_mime_stream_file_new() has already grabbed it.

g_mime_stream_file_new is a bad citizen, API-wise. It closes files that
it didn't open, (by default).

> I really wouldn't block on this, though, since the patch does fix an
> actual bug.

Not blocked. All pushed. New commit message below for reference.

-Carl

commit d5b4d950245605b84c56ce991fa3c59a073a70e5
Author: Jameson Graef Rollins <jrollins@finestructure.net>
Date:   Sat May 28 14:52:00 2011 -0700

    show: Avoid inadvertently closing stdout
    
    GMime has a nasty habit of taking ownership by default of any FILE*
    handed to it va g_mime_stream_file_new. Specifically it will close the
    FILE* when the stream is destroyed---even though GMime didn't open the
    file itself.
    
    To avoid this bad behavior, we have to carefully set_owner(FALSE)
    after calling g_mime_stream_file_new. In the format_part_content_text
    function, since commit d92146d3a6809f8ad940302af49cd99a0820665e we've
    been calling g_mime_stream_file_new unconditionally, but only calling
    g_mime_stream_file_set_owner(FALSE) conditionally.
    
    This led to the FILE* being closed early when notmuch show output was
    redirected to a file.
    
    Fixing this fixes the test-suite cases that broke with the previous
    commit, (which added redirected "notmuch show" calls to the test suite
    to expose this bug).
    
    Edited-by: Carl Worth <cworth@cworth.org> with a new commit message to
    explain the bug and fix.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1408 bytes --]

On Fri, 03 Jun 2011 15:38:29 -0700, Carl Worth <cworth@cworth.org> wrote:
> commit d5b4d950245605b84c56ce991fa3c59a073a70e5
> Author: Jameson Graef Rollins <jrollins@finestructure.net>
> Date:   Sat May 28 14:52:00 2011 -0700
> 
>     show: Avoid inadvertently closing stdout
>     
>     GMime has a nasty habit of taking ownership by default of any FILE*
>     handed to it va g_mime_stream_file_new. Specifically it will close the
>     FILE* when the stream is destroyed---even though GMime didn't open the
>     file itself.
>     
>     To avoid this bad behavior, we have to carefully set_owner(FALSE)
>     after calling g_mime_stream_file_new. In the format_part_content_text
>     function, since commit d92146d3a6809f8ad940302af49cd99a0820665e we've
>     been calling g_mime_stream_file_new unconditionally, but only calling
>     g_mime_stream_file_set_owner(FALSE) conditionally.
>     
>     This led to the FILE* being closed early when notmuch show output was
>     redirected to a file.
>     
>     Fixing this fixes the test-suite cases that broke with the previous
>     commit, (which added redirected "notmuch show" calls to the test suite
>     to expose this bug).
>     
>     Edited-by: Carl Worth <cworth@cworth.org> with a new commit message to
>     explain the bug and fix.

Now I sound like I know what I'm talking about!  Thanks, Carl.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1376 bytes --]

On Fri, 03 Jun 2011 12:56:50 -0700, Carl Worth <cworth@cworth.org> wrote:
> Otherwise, the patches up to this point in the thread have all either
> been pushed or I've asked for some additional information (perhaps
> that's just this patch and the "old style fcc dirs" patch?).

Great!  That's really great news, Carl.  Thank you very much.  I think
at this point the only thing we need for 0.6 are:

* Austin's atomicity patches
* maybe fix the frc822 handling issue

Can we set a target date for 0.6 release?  So we'll all start feeling
really bad if we miss it?

> I'm actually a bit surprised to see myself preferring patches in
> email. When Linus first wrote git, I couldn't understand why the Linux
> community kept to such a consistent culture of sending patches via
> email. It seemed so backwards to do these awkward machinations (git
> format-patch, git send-email, SMTP, MUA, git am), and risk all the
> problems of email clients corrupting patches, etc.—especially when git
> has such clean mechanisms for reliably moving patches around (git push,
> git pull).

Yeah, I'm with you that I'm surprised by liking this method as well.  I
think it only really works once most of the bulk of things are already
working, but for bug fixes and feature additions it really works well.
It's nice to have comments for patches on list.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 17/25] avoid segfault when calling sanitize_string() on NULL

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On Sat, 28 May 2011 14:51:52 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> +    if (NULL == str)
> +	return NULL;

I haven't been blocking patches because of this, but can I please ask
everyone to not use the above style?

I understand that the above style is intended to generate a compiler
error in the case of the programmer mistyping '=' where '==' was
intended.

But I just can't stand this style.

It looks so unnatural to me to read "if some_value is some_variable"
instead of the natural "if some_variable is some_value".

Also, gcc is kind enough to warn ("suggest parentheses around assignment
used as truth value") in the case of "if (str = NULL)" anyway, so
there's no actual benefit to the unnatural style.

I really do want our code to be readable, and I think that little things
do make a difference.

Thanks for your attention, (and thanks for your patience if I seem off
my rocker).

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]

On Sat, 28 May 2011 14:51:49 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
Hi Jamie,

I've pushed the next few patches up to this point, (with only one
functional change---I fixed a new test case to correctly use
notmuch_search_sanitize to avoid spurious failures unmatching thread ID
values).

This patch, however, isn't ready. The big problem is in this commit
message:

> From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
> 
> In notmuch 0.5 notmuch-fcc-dirs style changed.  The previous code
> did not correctly identify an old configuration and, as a
> consequence, broke new configurations.

There are several things there that are too vague, ("previous code",
"old configuration", "new configurations"). What kind of configuration
is broken? How does the change here help?

It would be easier to understand the code if there were a corresponding
test case for it. I'd even be willing to help write a test case, but
there are not enough specifics in that commit message to even tell me
what to test.

>  	  ((and (listp notmuch-fcc-dirs)
> -		(= 1 (length (car notmuch-fcc-dirs))))
> +		(stringp (car notmuch-fcc-dirs)))

So the "old configuration" was a single string? And this has been
inadvertently broken since 0.5?

>  	   ;; Old style - no longer works.
>  	   (error "Invalid `notmuch-fcc-dirs' setting (old style)"))

Yikes. That vague phrasing ("old style") is already in the code in error
messages as well.

Dmitry, can you help me know what's going on here? (Preferably by
sending a newer commit with a more thorough commit message.)

Thanks,

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Dmitry Kurochkin]

Hi Carl.

On Wed, 01 Jun 2011 22:10:07 -0700, Carl Worth <cworth@cworth.org> wrote:
> On Sat, 28 May 2011 14:51:49 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hi Jamie,
> 
> I've pushed the next few patches up to this point, (with only one
> functional change---I fixed a new test case to correctly use
> notmuch_search_sanitize to avoid spurious failures unmatching thread ID
> values).
> 
> This patch, however, isn't ready. The big problem is in this commit
> message:
> 
> > From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
> > 
> > In notmuch 0.5 notmuch-fcc-dirs style changed.  The previous code
> > did not correctly identify an old configuration and, as a
> > consequence, broke new configurations.
> 
> There are several things there that are too vague, ("previous code",
> "old configuration", "new configurations").

Well, it says that changes are in notmuch 0.5.  So "old" and "previous"
refer to pre-0.5 (i.e. 0.4) and "new" refers to 0.5.

> What kind of configuration
> is broken?

Any configuration when `notmuch-fcc-dirs' is a list.  That variable has
a nice documentation.

> How does the change here help?
> 

It fixes detection of old-style pre-0.5 setting for `notmuch-fcc-dirs'.

> It would be easier to understand the code if there were a corresponding
> test case for it. I'd even be willing to help write a test case, but
> there are not enough specifics in that commit message to even tell me
> what to test.
> 

I do not think we need a test for this fix.  What we need are tests for
FCC functionality when notmuch-fcc-dirs is a list.

> >  	  ((and (listp notmuch-fcc-dirs)
> > -		(= 1 (length (car notmuch-fcc-dirs))))
> > +		(stringp (car notmuch-fcc-dirs)))
> 
> So the "old configuration" was a single string?

No, it is a list with a string as the first element.  May I refer you to
"git show 0.4:emacs/notmuch-maildir-fcc.el" and current
emacs/notmuch-maildir-fcc.el?  It has good documentation with examples
for notmuch-fcc-dirs.  Also NEWS for 0.5 describe this change.

> And this has been
> inadvertently broken since 0.5?
> 

Old configuration format was changed in 0.5 in an incompatible way.
There is a check for the unsupported old-style configuration.  But the
check is broken and results in an error when running with a valid
new-style configuration.

> >  	   ;; Old style - no longer works.
> >  	   (error "Invalid `notmuch-fcc-dirs' setting (old style)"))
> 
> Yikes. That vague phrasing ("old style") is already in the code in error
> messages as well.
> 

Yes.

> Dmitry, can you help me know what's going on here?

I hope above helps.

> (Preferably by
> sending a newer commit with a more thorough commit message.)
> 

I am not sure what you expect from the commit message here.  IMO it is
enough for this small bugfix and those who interested can always refer
to documentation for details.

Regards,
  Dmitry

> Thanks,
> 
> -Carl
Non-text part: application/pgp-signature
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 2153 bytes --]

On Thu, 02 Jun 2011 10:49:57 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Well, it says that changes are in notmuch 0.5.  So "old" and "previous"
> refer to pre-0.5 (i.e. 0.4) and "new" refers to 0.5.

Sure, but I happen to ahve already forgotten the details of how the
variable could be configured in 0.4 and in 0.5. More importantly, anyone
in the future reading the commit log is much more likely not to
remember.

> Any configuration when `notmuch-fcc-dirs' is a list.  That variable has
> a nice documentation.

Again, I'd like our commit messages to be self-contained. They are much
more useful if the describe the change being made without assuming to
much outside knowledge.

> > It would be easier to understand the code if there were a corresponding
> > test case for it.
...
> I do not think we need a test for this fix.  What we need are tests for
> FCC functionality when notmuch-fcc-dirs is a list.

Yes!

> Old configuration format was changed in 0.5 in an incompatible way.
> There is a check for the unsupported old-style configuration.  But the
> check is broken and results in an error when running with a valid
> new-style configuration.

This is actually what I meant by "corresponding test case". If the bug
here is that a "new-style configuration" doesn't work , (and I still
don't like that wording---don't say "new style"---explain what it
actually *is*), then yes, we need a test case showing that bug.

> I am not sure what you expect from the commit message here.  IMO it is
> enough for this small bugfix and those who interested can always refer
> to documentation for details.

The commit message should provide a self-contained description of the
change. It should be along the lines of:

	When fcc-dirs is set to
	<some-particular-datatype-that-should-work> notmuch was
	incorecctly detecting this as the
	<old-style-that-is-no-longer-supported> and generating an error
	message. Fix the test so that this configuration now works.

Where the <phrases> above should be replaced with actual descriptions,
not relative pointers to information like "old style" or "new style".

Does that make sense?

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Dmitry Kurochkin]

On Fri, 03 Jun 2011 13:05:00 -0700, Carl Worth <cworth@cworth.org> wrote:
Non-text part: multipart/signed
> On Thu, 02 Jun 2011 10:49:57 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Well, it says that changes are in notmuch 0.5.  So "old" and "previous"
> > refer to pre-0.5 (i.e. 0.4) and "new" refers to 0.5.
> 
> Sure, but I happen to ahve already forgotten the details of how the
> variable could be configured in 0.4 and in 0.5. More importantly, anyone
> in the future reading the commit log is much more likely not to
> remember.
> 
> > Any configuration when `notmuch-fcc-dirs' is a list.  That variable has
> > a nice documentation.
> 
> Again, I'd like our commit messages to be self-contained. They are much
> more useful if the describe the change being made without assuming to
> much outside knowledge.
> 
> > > It would be easier to understand the code if there were a corresponding
> > > test case for it.
> ...
> > I do not think we need a test for this fix.  What we need are tests for
> > FCC functionality when notmuch-fcc-dirs is a list.
> 
> Yes!
> 
> > Old configuration format was changed in 0.5 in an incompatible way.
> > There is a check for the unsupported old-style configuration.  But the
> > check is broken and results in an error when running with a valid
> > new-style configuration.
> 
> This is actually what I meant by "corresponding test case". If the bug
> here is that a "new-style configuration" doesn't work , (and I still
> don't like that wording---don't say "new style"---explain what it
> actually *is*), then yes, we need a test case showing that bug.
> 
> > I am not sure what you expect from the commit message here.  IMO it is
> > enough for this small bugfix and those who interested can always refer
> > to documentation for details.
> 
> The commit message should provide a self-contained description of the
> change. It should be along the lines of:
> 
> 	When fcc-dirs is set to
> 	<some-particular-datatype-that-should-work> notmuch was
> 	incorecctly detecting this as the
> 	<old-style-that-is-no-longer-supported> and generating an error
> 	message. Fix the test so that this configuration now works.
> 
> Where the <phrases> above should be replaced with actual descriptions,
> not relative pointers to information like "old style" or "new style".
> 
> Does that make sense?
> 

"notmuch was incorecctly detecting this as the ..." is not right.  It is
a wrong-type-argument lisp error (evaluating (length '(a . b))).  How
about:

  Fix wrong-type-argument lisp error in `notmuch-fcc-header-setup' when
  `notmuch-fcc-dirs' is set to a list.  The error was in the
  `notmuch-fcc-dirs' format check which was changed in an incompatible
  way from 0.4 to 0.5.

Regards,
  Dmitry

> -Carl
Non-text part: application/pgp-signature

Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

On Sat, 04 Jun 2011 00:22:04 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> "notmuch was incorecctly detecting this as the ..." is not right.

Thanks. I was sure my commit message wasn't right yet. That was my
point---I didn't know what the right message would be! ;-)

> It is a wrong-type-argument lisp error (evaluating (length '(a . b))).  How
> about:
> 
>   Fix wrong-type-argument lisp error in `notmuch-fcc-header-setup' when
>   `notmuch-fcc-dirs' is set to a list.  The error was in the
>   `notmuch-fcc-dirs' format check which was changed in an incompatible
>   way from 0.4 to 0.5.

Perfect! This way, if I want to investigate this patch before committing
it, I now have the information I need to test it.

Thanks,

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On Sat, 04 Jun 2011 00:22:04 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Fri, 03 Jun 2011 13:05:00 -0700, Carl Worth <cworth@cworth.org> wrote:
> > > I do not think we need a test for this fix.  What we need are tests for
> > > FCC functionality when notmuch-fcc-dirs is a list.
> > 
> > Yes!

I've written these now. And they do test this fix. What they show is
that a legitimate setting (of notmuch-fcc-dirs as a list) was resulting
in an error rather than working. That's a nasty little bug, (and poor
coverage from our test suite before.

>   Fix wrong-type-argument lisp error in `notmuch-fcc-header-setup' when
>   `notmuch-fcc-dirs' is set to a list.  The error was in the
>   `notmuch-fcc-dirs' format check which was changed in an incompatible
>   way from 0.4 to 0.5.

Thanks for the fixed wording. I've now pushed out the fix (along with
the tests).

With all the talk of "old style" vs. "new style" I was thinking that the
bug only affected people with the old-style FCC setting. The bug is much
worse than that, (preventing people from using the new list-based
style).

Anyway, thanks for the patch, Dmitry. And thanks for pushing me to take
another look.

David, I suggest including this fix (and its test) in the release
branch.

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Dmitry Kurochkin]

On Thu, 23 Jun 2011 15:22:46 -0700, Carl Worth <cworth@cworth.org> wrote:
> On Sat, 04 Jun 2011 00:22:04 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > On Fri, 03 Jun 2011 13:05:00 -0700, Carl Worth <cworth@cworth.org> wrote:
> > > > I do not think we need a test for this fix.  What we need are tests for
> > > > FCC functionality when notmuch-fcc-dirs is a list.
> > > 
> > > Yes!
> 
> I've written these now. And they do test this fix. What they show is
> that a legitimate setting (of notmuch-fcc-dirs as a list) was resulting
> in an error rather than working. That's a nasty little bug, (and poor
> coverage from our test suite before.
> 
> >   Fix wrong-type-argument lisp error in `notmuch-fcc-header-setup' when
> >   `notmuch-fcc-dirs' is set to a list.  The error was in the
> >   `notmuch-fcc-dirs' format check which was changed in an incompatible
> >   way from 0.4 to 0.5.
> 
> Thanks for the fixed wording. I've now pushed out the fix (along with
> the tests).
> 
> With all the talk of "old style" vs. "new style" I was thinking that the
> bug only affected people with the old-style FCC setting. The bug is much
> worse than that, (preventing people from using the new list-based
> style).
> 
> Anyway, thanks for the patch, Dmitry. And thanks for pushing me to take
> another look.
> 

Well, I should have prepared a better commit message from the
beginning.  Then no pushing might have been needed :)

Regards,
  Dmitry

> David, I suggest including this fix (and its test) in the release
> branch.
> 
> -Carl
> 
> -- 
> carl.d.worth@intel.com

Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

On Fri, 24 Jun 2011 02:58:23 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Well, I should have prepared a better commit message from the
> beginning.  Then no pushing might have been needed :)

That, or a test case, (which would have clued me in to read the correct
interpretation of the original commit message).

Regardless, the fix is in place now, and that's what matters.

Thanks again,

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 07/25] test: remove json test for search null result, since it's being more properly tested in search-output

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 851 bytes --]

On Sat, 28 May 2011 14:51:42 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> The test in json was inferior to the one in search-output, since it
> wasn't properly testing for the presence of a trailing newline.

Hi Jamie,

I've pushed the series up to this point now, (along with the vim
improvements that weren't in your original mailing of the series).

I really appreciate the missing-newline bug-fix along with all the
improved testing for that. Thanks! I re-ordered that portion of the
patch series a tiny bit to put the bug fix immediately after the commit
that makes the tests start failing. I also reworded a few commit
messages slightly.

But I didn't make any code changes so it's trivial enough for me to keep
using "git rebase" to keep your series on top of my modified master.

I'll keep posting as I'm pushing.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 01/25] fix check for libdir in ldconfig paths

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 2813 bytes --]

On Sat, 28 May 2011 14:51:36 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> The configure script tries to check that the libdir is included in the
> ldconfig paths, and then sets the RPATH compiler flag if it's not.
> This check was broken, and was always setting RPATH.  This fixes the
> path check.

Thanks for the patch. I was intrigued about what was actually broken
here, so tracked down the problem and fixed it with an alternate patch
(see below).

-Carl

commit 574f408816e636f677b14384c18cc2e388991411
Author: Carl Worth <cworth@cworth.org>
Date:   Wed Jun 1 13:02:58 2011 -0700

    configure: Fix detection of libdir in ldconfig configuration
    
    Ever since commit b4b5e9ce4dac62111ec11da6d22b7e618056801f the
    detection of libdir in the ldconfig configuration has been broken,
    resulting in RPATH being set when not needed and not wanted.
    
    The cause was a change from an IFS setting of:
    
    	IFS="$(printf '\n\t')"
    
    to a new setting of:
    
    	IFS="$(printf '\n')"
    
    That looks desirable since we want to split the output of ldconfig
    based on newlines, (and not split on any filename that might have an
    embedded tab in it). The subtle bug in the above is that the shell
    trims any trailing newlines when performing command substitution so
    the final statement above was equivalent to:
    
    	IFS=''
    
    which prevented any splitting of the ldconfig output at all.
    
    Fix this by avoiding command substitution and just using a literal
    newline in the source file for setting this variable.

diff --git a/configure b/configure
index bbf30cd..cf525c9 100755
--- a/configure
+++ b/configure
@@ -355,17 +355,30 @@ elif [ $uname = "Linux" ] ; then
     printf "Linux\n"
     platform=LINUX
     linker_resolves_library_dependencies=1
+
+    printf "Checking for $libdir_expanded in ldconfig... "
     ldconfig_paths=$(/sbin/ldconfig -N -X -v 2>/dev/null | sed -n -e 's,^\(/.*\):\( (.*)\)\?$,\1,p')
     # Separate ldconfig_paths only on newline (not on any potential
-    # embedded space characters in any filenames).
+    # embedded space characters in any filenames). Note, we use a
+    # literal newline in the source here rather than something like:
+    #
+    #	IFS=$(printf '\n')
+    #
+    # because the shell's command substitution deletes any trailing newlines.
     OLD_IFS=$IFS
-    IFS="$(printf '\n')"
+    IFS="
+"
     for path in $ldconfig_paths; do
 	if [ "$path" = "$libdir_expanded" ]; then
 	    libdir_in_ldconfig=1
 	fi
     done
     IFS=$OLD_IFS
+    if [ "$libdir_in_ldconfig" = '0' ]; then
+	printf "No (will set RPATH)\n"
+    else
+	printf "Yes\n"
+    fi
 else
     printf "Unknown.\n"
     cat <<EOF

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 01/25] fix check for libdir in ldconfig paths

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 294 bytes --]

On Wed, 01 Jun 2011 16:22:19 -0700, Carl Worth <cworth@cworth.org> wrote:
> Thanks for the patch. I was intrigued about what was actually broken
> here, so tracked down the problem and fixed it with an alternate patch
> (see below).

Cool.  Yeah, that issue took a while to track down.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6 redux

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

On Sat, 28 May 2011 14:51:35 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> So what follows is a patch series for a bunch of miscellaneous patches
> that should be included in 0.6.  Most of them were originally part of
> the release-candiate/0.6 branch, and they are here rebased on top of
> notmuch/master at cb8418784c21155ffea79cce8409a7ea3c546937, which
> includes all the multipart and crypto rework.

I forgot to mention that the other two things that were in the original
release-candidate/0.6 that really need to be included before 0.6 is
released are

* Austin Clements's atomicity patches
* Felipe Contreras's vim patches

both of which have been sent to the list previously, although I believe
that Austin might have a couple extra patches in his atomic-new-v3
branch.

Austin: speaking of which, would you mind rebasing that patch series
against notmuch/master at cb8418784c21155ffea79cce8409a7ea3c546937 and
sending that to the list again?  That might help push Carl to merge that
stuff sooner.  I think the conflicts are very minor.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6 redux

[Austin Clements]

On Sat, May 28, 2011 at 5:58 PM, Jameson Graef Rollins
<jrollins@finestructure.net> wrote:
> Austin: speaking of which, would you mind rebasing that patch series
> against notmuch/master at cb8418784c21155ffea79cce8409a7ea3c546937 and
> sending that to the list again?  That might help push Carl to merge that
> stuff sooner.  I think the conflicts are very minor.

Done.  The only conflicts were the usual suspects (lists in the test
suite).  I pulled in the GDB dependency in the test suite and the
corresponding tweaks to test-lib that were on the list previously
(id:"1305206080-17461-1-git-send-email-amdragon@mit.edu" and
id:"1305206110-17511-1-git-send-email-amdragon@mit.edu").  It's on
for-review/atomic-new-v4 at the usual,
  http://awakening.csail.mit.edu/git/notmuch.git

Re: release-candidate/0.6 redux

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

Hi, folks.  I have pushed a new version of the release-candidate/0.6
branch to my repo [0].  It is all reworked on top of notmuch/master [1],
and includes:

* the miscellaneous fixes/improvements patch series starting at
  id:"1306619520-25730-2-git-send-email-jrollins@finestructure.net"
* the notmuch-show refresh patches
* Austin's atomicity patches
* Felipe's vim patches
* updates to the debian packaging

cworth: do you want to review this for the new release?  We're very close!
One last push and we can get this thing out.

bremner: do you want to reconcile this with what you've already pushed
to Debian experimental, and then push another version?

jamie.

[0] git://finestructure.net/notmuch [release-candidate/0.6]
[1] cb8418784c21155ffea79cce8409a7ea3c546937

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6 redux

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]

On Tue, 31 May 2011 11:43:31 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hi, folks.  I have pushed a new version of the release-candidate/0.6
> branch to my repo [0].  It is all reworked on top of notmuch/master [1],
> and includes:
...
> cworth: do you want to review this for the new release?  We're very close!
> One last push and we can get this thing out.

I've been going through it, but trying to focus more on the emails I
have queued up (which is currently 58 messages).

From a quick rebase of your release-candidate branch and a comparison
with what I have queued it looks like only the following commits are
left on your branch and not in my email queue:

    emacs: update notmuch-crypto-process-mime config variable documentation.
    bump debian standards version
    Increment library version to 1.4.0
    Increment notmuch version to 0.6
    debian: update list of symbols for libnotmuch
    debian: separate build dependency entries on to separate lines
    debian: update changelog for 0.6

That's mostly just the release bookkeeping, which is good---that means
I've been through most of the "meat" of the patches on your branch.

I'll now work through what I still have in my queue (Austin's atomicity
work is the only big piece) and then look at the above, at which point
we should be there. (And yes, I know I sound like I'm talking along the
lines of a feature-based release rather than a time-based release.)

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: release-candidate/0.6 redux

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 893 bytes --]

On Fri, 03 Jun 2011 18:27:42 -0700, Carl Worth <cworth@cworth.org> wrote:
> From a quick rebase of your release-candidate branch and a comparison
> with what I have queued it looks like only the following commits are
> left on your branch and not in my email queue:
> 
>     emacs: update notmuch-crypto-process-mime config variable documentation.

Hey, Carl.  This is actually a five-patch series that starts at:

 emacs: add notmuch-show-refresh-view function
  id:"1306627784-3401-1-git-send-email-jrollins@finestructure.net"

You should also look at the following two patch series to fix the
message/rfc822 part handling:

 Do not attept to output part raw if part is not GMimePart.
  id:"1307120466-4980-1-git-send-email-jrollins@finestructure.net"

 improving message/rfc822 part handling
  id:"1307320169-29905-1-git-send-email-jrollins@finestructure.net"

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: release-candidate/0.6 redux

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On Sun, 05 Jun 2011 17:35:40 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Fri, 03 Jun 2011 18:27:42 -0700, Carl Worth <cworth@cworth.org> wrote:
> > From a quick rebase of your release-candidate branch and a comparison
> > with what I have queued it looks like only the following commits are
> > left on your branch and not in my email queue:
> > 
> >     emacs: update notmuch-crypto-process-mime config variable documentation.
> 
> Hey, Carl.  This is actually a five-patch series that starts at:
> 
>  emacs: add notmuch-show-refresh-view function
>   id:"1306627784-3401-1-git-send-email-jrollins@finestructure.net"

OK, good. That is actually part of my queue.

> You should also look at the following two patch series to fix the
> message/rfc822 part handling:
> 
>  Do not attept to output part raw if part is not GMimePart.
>   id:"1307120466-4980-1-git-send-email-jrollins@finestructure.net"
> 
>  improving message/rfc822 part handling
>   id:"1307320169-29905-1-git-send-email-jrollins@finestructure.net"

OK. These are also already queued.

Hopefully, someone will provide me with a good way to publish my queue
soon, ("notmuch search --output=html" ?), and then communication like
this will be a bit easier. ;-)

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

tag sharing [was: Re: release-candidate/0.6 redux]

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

On Mon, 06 Jun 2011 05:17:27 -0700, Carl Worth <cworth@cworth.org> wrote:
> Hopefully, someone will provide me with a good way to publish my queue
> soon, ("notmuch search --output=html" ?), and then communication like
> this will be a bit easier. ;-)

I've been thinking about this more and it really seems we need a way to
just share tags.  What if we had a way to export all the tags for a set
of messages as a notmuch dump file, that could just be piped into
notmuch to modify tags?  This would be a great way for lots of people to
keep tags synced on a set of messages.

The main difficulty (seems to me) would be the sharing of -tags.  You
wouldn't want the absence of a tag on a message in the tag file to mean
that the tag should be removed.  So we would need to represent both
+tags and -tags in the dump file [0].

It seems a little crazy, but would it be possible to store -tags in the
database somehow?

jamie.

[0] This would actually help with tags applied when the message is
indexed as well, like "signed" and "encrypted", since it would allow for
tagging messages that were indexed before the "signed"/"encrypted"
tagging was a feature.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: tag sharing [was: Re: release-candidate/0.6 redux]

[Jesse Rosenthal]

On Mon, 06 Jun 2011 09:28:13 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> I've been thinking about this more and it really seems we need a way to
> just share tags.  What if we had a way to export all the tags for a set
> of messages as a notmuch dump file, that could just be piped into
> notmuch to modify tags?  This would be a great way for lots of people to
> keep tags synced on a set of messages.

After a conversation with David last year about bug-tracking, I worked
up a rough python-based prototype of this. It worked in terms of
namespaces, so Carl could associate the namespace "public" with a list
of tags he publishes to a http-accessable location. And you could
associate the namespace "cworth" with those same tags.

He types `whatevercommand push public` and all tags "public.*" go to an
config-associated URL.

You type `whatevercommand pull cworth` and the tags come down from that
URL as "cworth.*".

So what he has as "public.to-push" comes down to your notmuch as
"cworth.to-push".

It's available here:

    http://commonmeasure.org/~jkr/git/nm-remote.git

See these emails for more (but note that the repo url has changed to the
above).
    
    id:"m1k4rkkchy.fsf@watt.gilman.jhu.edu"        
    id:"m1hbmokbxj.fsf@watt.gilman.jhu.edu"

There were some details and inherent ambiguities about conflict
resolution, and the above emails explain how I dealt with them. 

Note also that it uses python's configparser, which will overwrite your
config -- which means it'll get rid of your comments. So if you use any
of the config-writing commands, make sure you back up your config first.

Best,
Jesse

Re: tag sharing [was: Re: release-candidate/0.6 redux]

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

On Mon, 06 Jun 2011 13:20:00 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wrote:
> After a conversation with David last year about bug-tracking, I worked
> up a rough python-based prototype of this. It worked in terms of
> namespaces, so Carl could associate the namespace "public" with a list
> of tags he publishes to a http-accessable location. And you could
> associate the namespace "cworth" with those same tags.

This sounds very cool, Jesse.  Very git-like.  I like it.

> You type `whatevercommand pull cworth` and the tags come down from that
> URL as "cworth.*".
> 
> So what he has as "public.to-push" comes down to your notmuch as
> "cworth.to-push".

Did you guys try to address the issue of tag removal at all?  I've been
trying to decide if this is something we need to worry about or not.
For instance, if cworth pushed a tag ".needs-review", you would probably
want to have that tag removed if cworth removed it.  I guess
alternatively he could just push the tag ".reviewed" to nullify the
meaning of the previous one.  I'm not sure that would work in all cases,
though.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: tag sharing [was: Re: release-candidate/0.6 redux]

[Jesse Rosenthal]

On Wed, 08 Jun 2011 10:46:57 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Did you guys try to address the issue of tag removal at all?  I've been
> trying to decide if this is something we need to worry about or not.
> For instance, if cworth pushed a tag ".needs-review", you would probably
> want to have that tag removed if cworth removed it.  I guess
> alternatively he could just push the tag ".reviewed" to nullify the
> meaning of the previous one.  I'm not sure that would work in all cases,
> though.

Yes, if he deletes the tag "public.needs-review" in his notmuch, and
then pushes it, it will delete the tag "cworth.needs-review" from yours.

A couple points about this:

1. If you added your own "needs-review" tag to things that have a
"cworth.needs-review" tag, that wouldn't be deleted. In other words,
cworth's actions will only affect that namespace. Now, if you delete it,
and push it, and Carl pulls back from you (meaning he trusts you), then
it would be deleted on his. When he pushes next, it will be deleted on
everyone who pulls from him.

I'm sure there are resolution paradoxes here. It only had a ten-minute
trial run one day on IRC with me and someone else.

2. The history is available. If you run `nm-remote history msg-id`, it
will show you its history of being added and deleted. I think it will
show by whom it was added or deleted as well, but I don't quite remember
(I hacked this up over a year ago).

Re: tag sharing

[David Bremner]

On Wed, 08 Jun 2011 10:46:57 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:

> On Mon, 06 Jun 2011 13:20:00 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wrote:
> > After a conversation with David last year about bug-tracking, I worked
> > up a rough python-based prototype of this. It worked in terms of
> > namespaces, so Carl could associate the namespace "public" with a list
> > of tags he publishes to a http-accessable location. And you could
> > associate the namespace "cworth" with those same tags.
> 
> This sounds very cool, Jesse.  Very git-like.  I like it.
> 

Warning: git fanboi-ism ahead.

I like the concept too. But I worry a bit that we are re-inventing
things that could be done by git.  What do people think about a similar
namespace import export tool where we rely on git for a transport layer,
and for merging, conflict-resolution, and history tracking. Of course
there are details to be settled, but before I try to work them out,
would anyone like to convince me the idea is fundementally wrong headed?
Jesse, do you remember why you decided to roll your own?  I agree it is
subjective what is easier, but I think we have enough people comfortable
with git that collaborative maintenence of a bug-tag git repo would be
pretty easy.

d

Re: tag sharing

[Jesse Rosenthal]

On Thu, 06 Oct 2011 09:56:43 -0300, David Bremner <david@tethera.net> wrote:
> Jesse, do you remember why you decided to roll your own?

Only reason I can remember (a year and a half later) of is that it
seemed like the basic illustration of concept would be a saturday
morning's project. In retrospect, I think the main issue was that I was
trying to figure out how history would be kept. By using git, though,
your idea would be that we get history for free, right?

I think my format was:

    <TIMESTAMP>    <MESSAGID>     <TAG>
    12345          foo@bar        to-fix
    13456          bar@foo        feature-request

But using git would get us timestamps for free too, I guess.  And if we
get rid of timestamps, then we just have dumps. So what we would really
just need is a thin namespacing layer in some scripting language on top
of git? I.e., all we need something to just keep different dumps in
different namespaces, so that when I pull your dump, the tags come in as
`bremner/to-fix` (or whatever our separator is), and then when I dump
all `bremner` tags, it removes the namespace and puts it in
`bremner.tagfile`.

One rather obvious thing that I discovered: processing dumps and
restores from the ui is pretty slow, because you have to output a huge
file, and then find the tags in your preferred namespace. So using
either the library or the bindings would probably speed things up quite
a bit, by only dumping the tags you care about. Which means it couldn't
just be a shell script: it would have to be c/ruby/python.

Best,
Jesse

Re: tag sharing

[David Bremner]

[-- Attachment #1: Type: text/plain, Size: 1610 bytes --]

On Thu, 06 Oct 2011 09:21:48 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wrote:

> morning's project. In retrospect, I think the main issue was that I was
> trying to figure out how history would be kept. By using git, though,
> your idea would be that we get history for free, right?

Yeah. Of course, one would have to decide if the history of the current
dump file format is intelligible, or if some alternative format should
be used.

> of git? I.e., all we need something to just keep different dumps in
> different namespaces, so that when I pull your dump, the tags come in as
> `bremner/to-fix` (or whatever our separator is), and then when I dump
> all `bremner` tags, it removes the namespace and puts it in
> `bremner.tagfile`.

something like that sounds plausible. Currently the query parser doesn't
handle searches like "tag:bremner/to-fix" very well, because it
helpfully splits at '/' (aiui; maybe somebody else can explain it
better).  "notmuch search "tag:notmuch*" does not seem to work either,
so my first idea to kludge around the problem by using
e.g. "notmuchPushed" doesn't seem to work either.

> So using
> either the library or the bindings would probably speed things up quite
> a bit, by only dumping the tags you care about. Which means it couldn't
> just be a shell script: it would have to be c/ruby/python.

Without looking at the code, I'd guess it wouldn't be too hard to have
notmuch-dump take a standard search term; it's just as I mentioned I
don't see an elegant way of doing what we want with a standard search
term. Of course I'd be happy to be proved wrong.

David




[-- Attachment #2: Type: application/pgp-signature, Size: 315 bytes --]

Re: tag sharing

[Jesse Rosenthal]

On Thu, 06 Oct 2011 11:18:51 -0300, David Bremner <david@tethera.net> wrote:
> something like that sounds plausible. Currently the query parser doesn't
> handle searches like "tag:bremner/to-fix" very well, because it
> helpfully splits at '/' (aiui; maybe somebody else can explain it
> better).  "notmuch search "tag:notmuch*" does not seem to work either,
> so my first idea to kludge around the problem by using
> e.g. "notmuchPushed" doesn't seem to work either.

A period works: `tag:bremner.to-fix`. That would fit with java/python
conventions. Double-colons also seem to work, for what it's worth:
`tag:bremner::to-fix`

Re: tag sharing

[David Bremner]

On Thu, 06 Oct 2011 15:49:10 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wrote:
> 
> A period works: `tag:bremner.to-fix`. That would fit with java/python
> conventions. Double-colons also seem to work, for what it's worth:
> `tag:bremner::to-fix`

Right, I was confused and/or unclear. What doesn't work is searches for
the whole namespace "notmuch search tag:bremner.*" will return nothing,
even though "notmuch search tag:bremner.to-fix" does.

d

Re: tag sharing

[Jesse Rosenthal]

On Thu, 06 Oct 2011 17:23:21 -0300, David Bremner <david@tethera.net> wrote:
> What doesn't work is searches for the whole namespace "notmuch search
> tag:bremner.*" will return nothing, even though "notmuch search
> tag:bremner.to-fix" does.

A simple shell way to do this would be

    notmuch search-tags | grep "^bremner\." | xargs -I {} notmuch search tag:{}

That's pretty quick over a fair amount of messages. It would be quicker,
I imagine, if it did the grepping inside of a program, but this is still
way down there in the subsecond range.

NB. I don't have that many tags, so maybe the grepping step would bog
down a bit if I had more. But I doubt someone would have that much more
than a few hundred.

Re: tag sharing

[David Bremner]

On Thu, 06 Oct 2011 16:40:00 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wrote:
> On Thu, 06 Oct 2011 17:23:21 -0300, David Bremner <david@tethera.net> wrote:
> > What doesn't work is searches for the whole namespace "notmuch search
> > tag:bremner.*" will return nothing, even though "notmuch search
> > tag:bremner.to-fix" does.
> 
> A simple shell way to do this would be
> 
>     notmuch search-tags | grep "^bremner\." | xargs -I {} notmuch search tag:{}
> 

Ah thanks, that is not so bad. It turns out to be literally a one line
change to add a query argument for notmuch-dump, _except_ that notmuch
dump already uses the first command line argument as an output file
name. I think I will start a seperate thread about that. If you want to
test, this works OK, except the output is put in a file named after the
query ;).

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 7e7bc17..341207a 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -40,7 +40,7 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     if (notmuch == NULL)
 	return 1;
 
-    query = notmuch_query_create (notmuch, "");
+    query = notmuch_query_create (notmuch, argc > 0 ? argv[0] : "");
     if (query == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return 1;

Re: tag sharing

[Jesse Rosenthal]

On Thu, 06 Oct 2011 11:18:51 -0300, David Bremner <david@tethera.net> wrote:
Non-text part: multipart/signed
> On Thu, 06 Oct 2011 09:21:48 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wrote:
> > morning's project. In retrospect, I think the main issue was that I was
> > trying to figure out how history would be kept. By using git, though,
> > your idea would be that we get history for free, right?
> 
> Yeah. Of course, one would have to decide if the history of the current
> dump file format is intelligible, or if some alternative format should
> be used.

I've managed to reconstruct a bit of my thinking. I think it could work
with the current format if we went as follows. I also think we could use
git, but I'm not sure that wouldn't be overkill. Some of the steps are
in extreme slow motion, just to make sure we're on the same page. (I've
also replaced the real msg-ids to cut down on noise).

  1. David dumps the targeted messages (tags in namespace "my"), so we
     get a file like this:
      
      msg-id-1@example.1 (my.foo inbox)
      msg-id-2@example.2 (answered my.to-do inbox signed)

  2. David strips out everything but tags in the namespace and removes
     the namespace:

      msg-id-1@example.1 (foo)
      msg-id-2@example.2 (to-do)

  3. David makes it available, Jesse pulls.

  4. Jesse dumps the namespace he has associated with David ("bremner"):

      msg-id-3@example.3 (bremner.bar)
      msg-id-4@example.4 (bremner.bar unread)
      msg-id-1@example.1 (bremner.foo inbox)
      msg-id-5@example.5 (bremner.wishlist inbox)

  5. Jesse strips the tags:

      msg-id-3@example.3 (bar)
      msg-id-4@example.4 (bar)
      msg-id-1@example.1 (foo)
      msg-id-5@example.5 (wishlist)

  6. Jesse replaces this list with David's:

      msg-id-1@example.1 (foo)
      msg-id-2@example.2 (to-do)

  7. Jesse removes all tags with the "bremner" namespace from his
     database.

  8. Jesse adds the tags from the list in step (6), with the appropriate
     namespace, to his list of tags. The messages of interest now look like:

      msg-id-3@example.3 ()
      msg-id-4@example.4 (unread)
      msg-id-1@example.1 (bremner.foo inbox)
      msg-id-5@example.5 (inbox)
      msg-id-2@example.2 (bremner.to-do someothertag)

  9. Jesse runs `notmuch restore`

The reason I went through this slowly is to highlight the fact that the
only thing that needs to be recorded in diffs somehow (in git, let's
say) are the files in steps (5) and (6). So if we integrated this with
git, there would be something along the lines of a commit (on Jesse's
computer) in step (5) and then another commit (again on Jesse's
computer) in step (6). 

Note that we don't actually need (5) to do tag-sharing. The only reason
it's there is for keeping history.

I'm pretty sure that we don't need a commit on David's computer in step
(1), because what the history would record is when the tag-sharing
utility changed your tags, not when you did.

Note also that all we would really be asking git to do here is keep an
ordered collection of diffs of a single file. Honest question: since
this is the barest possible form of version control, is git necessary?
Some of the features, like commit messages, don't seem to fit well with
this model; and others, like branching, seem pretty useless. What's the
value added over just keeping a (compressed?) collection of diffs for
each namespace?

Best,
Jesse

Re: tag sharing

[Jesse Rosenthal]

On Fri, 07 Oct 2011 07:36:30 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wrote:
> What's the value added over just keeping a (compressed?) collection of
> diffs for each namespace?

Okay -- please don't bother answering this part. It was early in the
morning, and I forgot some of the obvious advantages of real version
control (we can roll back to a previous moment, etc). We still don't
need most of what git offers, but if it's the best tool for keeping
track of the move from the file in step (5) to the file in step (6),
then sure, let's use it.