[PATCH] sprinters: bugfix when NULL passed for a string.

2012-08-07 Thread Austin Clements
LGTM.

This won't commute with [0], since that introduces broken tests that are
fixed by this patch.

I think we should remove the fields in the JSON header object for
missing headers (except perhaps From and Date, if those really are
mandatory headers), but I think we should do that after the freeze,
since it might affect frontends.  It probably makes sense to add an
Emacs test or two to the tests in [0].

[0] id:"1344389313-7886-1-git-send-email-amdragon at mit.edu"

On Tue, 07 Aug 2012, Mark Walters  wrote:
> The string function in a sprinter may be called with a NULL string
> pointer (eg if a header is absent). This causes a segfault. We fix
> this by checking for a null pointer in the string functions and update
> the sprinter documentation.
>
> At the moment some output when format=text is done directly rather than
> via an sprinter: in that case a null pointer is passed to printf or
> similar and a "(null)" appears in the output. That behaviour is not
> changed in this patch.
> ---
>
> This could really do with some tests (it is the second time this type of
> bug has occurred). To be considered as a message by notmuch new a file
> needs at least one of a From: Subject: or To: header. Thus we should
> have three messages each of which just contains that single header (and
> nothing else) and check that search and show work as expected. 
>
>
>
>  sprinter-json.c |2 ++
>  sprinter-text.c |2 ++
>  sprinter.h  |4 +++-
>  3 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/sprinter-json.c b/sprinter-json.c
> index c9b6835..0a07790 100644
> --- a/sprinter-json.c
> +++ b/sprinter-json.c
> @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, 
> size_t len)
>  static void
>  json_string (struct sprinter *sp, const char *val)
>  {
> +if (val == NULL)
> + val = "";
>  json_string_len (sp, val, strlen (val));
>  }
>  
> diff --git a/sprinter-text.c b/sprinter-text.c
> index dfa54b5..10343be 100644
> --- a/sprinter-text.c
> +++ b/sprinter-text.c
> @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, 
> size_t len)
>  static void
>  text_string (struct sprinter *sp, const char *val)
>  {
> +if (val == NULL)
> + val = "";
>  text_string_len (sp, val, strlen (val));
>  }
>  
> diff --git a/sprinter.h b/sprinter.h
> index 5f43175..912a526 100644
> --- a/sprinter.h
> +++ b/sprinter.h
> @@ -27,7 +27,9 @@ typedef struct sprinter {
>   * a list or map, followed or preceded by separators).  For string
>   * and string_len, the char * must be UTF-8 encoded.  string_len
>   * allows non-terminated strings and strings with embedded NULs
> - * (though the handling of the latter is format-dependent).
> + * (though the handling of the latter is format-dependent). For
> + * string (but not string_len) the string pointer passed may be
> + * NULL.
>   */
>  void (*string) (struct sprinter *, const char *);
>  void (*string_len) (struct sprinter *, const char *, size_t);
> -- 
> 1.7.9.1
>
>
> H
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2] test: Add test for messages with missing headers

2012-08-07 Thread Austin Clements
Currently the JSON tests for search and show are broken because
notmuch attempts to dereference a NULL pointer.
---
This version fixes the "Show: text" test so that it sanitize its
output and doesn't hard-code my test paths.

 test/missing-headers |  162 ++
 test/notmuch-test|1 +
 2 files changed, 163 insertions(+)
 create mode 100755 test/missing-headers

diff --git a/test/missing-headers b/test/missing-headers
new file mode 100755
index 000..e79f922
--- /dev/null
+++ b/test/missing-headers
@@ -0,0 +1,162 @@
+#!/usr/bin/env bash
+test_description='messages with missing headers'
+. ./test-lib.sh
+
+# Notmuch requires at least one of from, subject, or to or it will
+# ignore the file.  Generate two messages so that together they cover
+# all possible missing headers.  We also give one of the messages a
+# date to ensure stable result ordering.
+
+cat < "${MAIL_DIR}/msg-2"
+To: Notmuch Test Suite 
+Date: Fri, 05 Jan 2001 15:43:57 +
+
+Body
+EOF
+
+cat < "${MAIL_DIR}/msg-1"
+From: Notmuch Test Suite 
+
+Body
+EOF
+
+NOTMUCH_NEW
+
+test_begin_subtest "Search: text"
+output=$(notmuch search '*' | notmuch_search_sanitize)
+test_expect_equal "$output" "\
+thread:XXX   2001-01-05 [1/1] (null);  (inbox unread)
+thread:XXX   1970-01-01 [1/1] Notmuch Test Suite;  (inbox unread)"
+
+test_begin_subtest "Search: json"
+test_subtest_known_broken
+output=$(notmuch search --format=json '*' | notmuch_search_sanitize)
+test_expect_equal_json "$output" '
+[
+{
+"authors": "",
+"date_relative": "2001-01-05",
+"matched": 1,
+"subject": "",
+"tags": [
+"inbox",
+"unread"
+],
+"thread": "XXX",
+"timestamp": 978709437,
+"total": 1
+},
+{
+"authors": "Notmuch Test Suite",
+"date_relative": "1970-01-01",
+"matched": 1,
+"subject": "",
+"tags": [
+"inbox",
+"unread"
+],
+"thread": "XXX",
+"timestamp": 0,
+"total": 1
+}
+]'
+
+test_begin_subtest "Show: text"
+output=$(notmuch show '*' | notmuch_show_sanitize)
+test_expect_equal "$output" "\
+message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 
match:1 excluded:0 filename:/XXX/mail/msg-2
+header{
+ (2001-01-05) (inbox unread)
+Subject: (null)
+From: (null)
+To: Notmuch Test Suite 
+Date: Fri, 05 Jan 2001 15:43:57 +
+header}
+body{
+part{ ID: 1, Content-type: text/plain
+Body
+part}
+body}
+message}
+message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 
match:1 excluded:0 filename:/XXX/mail/msg-1
+header{
+Notmuch Test Suite  (1970-01-01) (inbox unread)
+Subject: (null)
+From: Notmuch Test Suite 
+Date: Thu, 01 Jan 1970 00:00:00 +
+header}
+body{
+part{ ID: 1, Content-type: text/plain
+Body
+part}
+body}
+message}"
+
+test_begin_subtest "Show: json"
+test_subtest_known_broken
+output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize)
+test_expect_equal_json "$output" '
+[
+[
+[
+{
+"body": [
+{
+"content": "Body\n",
+"content-type": "text/plain",
+"id": 1
+}
+],
+"date_relative": "2001-01-05",
+"excluded": false,
+"filename": "Y",
+"headers": {
+"Date": "Fri, 05 Jan 2001 15:43:57 +",
+"From": "",
+"Subject": "",
+"To": "Notmuch Test Suite "
+},
+"id": "X",
+"match": true,
+"tags": [
+"inbox",
+"unread"
+],
+"timestamp": 978709437
+},
+[]
+]
+],
+[
+[
+{
+"body": [
+{
+"content": "Body\n",
+"content-type": "text/plain",
+"id": 1
+}
+],
+"date_relative": "1970-01-01",
+"excluded": false,
+"filename": "Y",
+"headers": {
+"Date": "Thu, 01 Jan 1970 00:00:00 +",
+"From": "Notmuch Test Suite ",
+"Subject": ""
+},
+"id": "X",
+"match": true,
+"tags": [
+"inbox",
+"unread"
+],
+"timestamp": 0
+},
+[]
+]
+]
+]'
+
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index ea39dfc..cc732c3 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -59,6 +59,7 @@ TESTS="
   

[PATCH] sprinters: bugfix when NULL passed for a string.

2012-08-07 Thread Mark Walters

The string function in a sprinter may be called with a NULL string
pointer (eg if a header is absent). This causes a segfault. We fix
this by checking for a null pointer in the string functions and update
the sprinter documentation.

At the moment some output when format=text is done directly rather than
via an sprinter: in that case a null pointer is passed to printf or
similar and a "(null)" appears in the output. That behaviour is not
changed in this patch.
---

This could really do with some tests (it is the second time this type of
bug has occurred). To be considered as a message by notmuch new a file
needs at least one of a From: Subject: or To: header. Thus we should
have three messages each of which just contains that single header (and
nothing else) and check that search and show work as expected. 



 sprinter-json.c |2 ++
 sprinter-text.c |2 ++
 sprinter.h  |4 +++-
 3 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/sprinter-json.c b/sprinter-json.c
index c9b6835..0a07790 100644
--- a/sprinter-json.c
+++ b/sprinter-json.c
@@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, 
size_t len)
 static void
 json_string (struct sprinter *sp, const char *val)
 {
+if (val == NULL)
+   val = "";
 json_string_len (sp, val, strlen (val));
 }

diff --git a/sprinter-text.c b/sprinter-text.c
index dfa54b5..10343be 100644
--- a/sprinter-text.c
+++ b/sprinter-text.c
@@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, size_t 
len)
 static void
 text_string (struct sprinter *sp, const char *val)
 {
+if (val == NULL)
+   val = "";
 text_string_len (sp, val, strlen (val));
 }

diff --git a/sprinter.h b/sprinter.h
index 5f43175..912a526 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -27,7 +27,9 @@ typedef struct sprinter {
  * a list or map, followed or preceded by separators).  For string
  * and string_len, the char * must be UTF-8 encoded.  string_len
  * allows non-terminated strings and strings with embedded NULs
- * (though the handling of the latter is format-dependent).
+ * (though the handling of the latter is format-dependent). For
+ * string (but not string_len) the string pointer passed may be
+ * NULL.
  */
 void (*string) (struct sprinter *, const char *);
 void (*string_len) (struct sprinter *, const char *, size_t);
-- 
1.7.9.1


H


[PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Mark Walters  wrote:

> On Tue, 07 Aug 2012, Michal Nazarewicz  wrote:

[ ... ]

>> Mark Walters  writes:
>>> As an alternative approach would allowing a list of tags (or even tag
>>> changes) to apply when a message is "read" do what you want and be more
>>> flexible?
>>
>> Something like the following (not tested)?
>
> Yes this was what I had in mind. I like this (both the symmetry with
> reply tags and the added flexibility) but I will let others
> comment. (There is one small bug in your draft: see below)

I like the idea -- and the draft!

>
> Best wishes
>
> Mark

Tomi

>
>> From: Michal Nazarewicz 
>> Date: Mon, 6 Aug 2012 15:31:20 +0200
>> Subject: [PATCH] notmuch-show: add notmuch-show-mark-read-tags option
>>
>> The `notmuch-show-mark-read-tags' lists tags that are to be applied when
>> message is read.  By default, the only value is "-unread" which will remove
>> the unread tag.  Among other uses, this variable can be used to stop
>> notmuch-show from modifying tags when message is shown (by setting the
>> variable to an empty list).
>> ---
>>  emacs/notmuch-show.el |   12 ++--
>>  1 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index dcfc190..92a4beb 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -183,6 +183,13 @@ provided with an MLA argument nor `completing-read' 
>> input."
>>   notmuch-show-stash-mlarchive-link-alist))
>>:group 'notmuch-show)
>>  
>> +(defcustom notmuch-show-mark-read-tags '("-unread")
>> +  "List of tags to apply when message is read, ie. shown in notmuch-show
>> +buffer."
>> +  :type '(repeat string)
>> +  :group 'notmuch-show)
>> +
>> +
>>  (defmacro with-current-notmuch-show-message ( body)
>>"Evaluate body with current buffer set to the text of current message"
>>`(save-excursion
>> @@ -1383,8 +1390,9 @@ current thread."
>>(notmuch-show-get-prop :headers-visible))
>>  
>>  (defun notmuch-show-mark-read ()
>> -  "Mark the current message as read."
>> -  (notmuch-show-tag-message "-unread"))
>> +  "Apply `notmuch-show-mark-read-tags' to the message."
>> +  (when notmuch-show-mark-read-tags
>> +(apply notmuch-show-tag-message notmuch-show-mark-read-tags)))
>
>  This needs to be (apply 'notmuch-show-tag-message ...)
>>  
>>  ;; Functions for getting attributes of several messages in the current
>>  ;; thread.
>>
>> -- 
>> Best regards, _ _
>> .o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
>> ..o | Computer Science,  Micha? ?mina86? Nazarewicz(o o)
>> ooo +--ooO--(_)--Ooo--
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] emacs: notmuch search bugfix

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Mark Walters wrote:

> The recent change to use json for notmuch-search.el introduced a bug
> in the code for keeping position on refresh. The problem is a
> comparison between (plist-get result :thread) and a thread-id returned
> by notmuch-search-find-thread-id: the latter is prefixed with
> "thread:"
>
> We fix this by adding an option to notmuch-search-find-thread-id to
> return the bare thread-id. It appears that notmuch-search-refresh-view
> is the only caller of notmuch-search that supplies a thread-id so this
> change should be safe (but could theoretically break users .emacs
> functions).
> ---
>
> This is a different version of the bugfix suggested in the parent to
> this thread. I think it is nicer but there is a slight risk of breaking
> users .emacs lisp functions.
>
> In general I think the thread: prefix should be present where the term
> is "really" a search term, but absent when it is must be a thread-id,
> and this fits with that idea.
>
> With 0.14 freeze pretty close it might be best to apply this to master
> and the simpler no breakage option (in id:"87boinpqfg.fsf at qmul.ac.uk")
> to the branch.

LGTM.

IMHO you're too shy with this patch to be included in 0.14 :D
-- at least I cannot imagine a case this would fail...

My change would have been:
  (when thread (if bare thread (concat "thread:" thread)))

or even (which changes irrelevant part):
  (if thread (if bare thread (concat "thread:" thread)))

Anyway, this is good as it is "in the grand scheme of things".

Tomi

>
> Best wishes 
>
> Mark
>
>  emacs/notmuch.el |   10 ++
>  1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index d2d82a9..0fff58a 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -475,10 +475,12 @@ BEG."
>   (push (plist-get (notmuch-search-get-result pos) property) output)))
>  output))
>  
> -(defun notmuch-search-find-thread-id ()
> -  "Return the thread for the current thread"
> +(defun notmuch-search-find-thread-id ( bare)
> +  "Return the thread for the current thread
> +
> +If BARE is set then do not prefix with \"thread:\""
>(let ((thread (plist-get (notmuch-search-get-result) :thread)))
> -(when thread (concat "thread:" thread
> +(when thread (concat (when (not bare) "thread:") thread
>  
>  (defun notmuch-search-find-thread-id-region (beg end)
>"Return a list of threads for the current region"
> @@ -993,7 +995,7 @@ same relative position within the new buffer."
>(interactive)
>(let ((target-line (line-number-at-pos))
>   (oldest-first notmuch-search-oldest-first)
> - (target-thread (notmuch-search-find-thread-id))
> + (target-thread (notmuch-search-find-thread-id t))
>   (query notmuch-search-query-string)
>   (continuation notmuch-search-continuation))
>  (notmuch-kill-this-buffer)
> -- 
> 1.7.9.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Mark Walters  wrote:

> On Mon, 06 Aug 2012, Michal Nazarewicz  wrote:
>> From: Michal Nazarewicz 
>>
>> Setting `notmuch-show-auto-mark-read' to nil stops notmuch-show from marking
>> the message as read (by removing the unread tag).  Inteded for people who
>> like to mark messages read explicitly.
>> ---
>>  emacs/notmuch-show.el |   16 +---
>>  1 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index d318430..85a17b1 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -183,6 +183,14 @@ provided with an MLA argument nor `completing-read' 
>> input."
>>   notmuch-show-stash-mlarchive-link-alist))
>>:group 'notmuch-show)
>>  
>> +(defcustom notmuch-show-auto-mark-read t
>> +  "Whether to automatically mark message as read when it is shown.  If
>> +nil, message needs to be marked as read manually for instance by
>> +removing the unread tag."
>> +  :type 'boolean
>> +  :group 'notmuch-show)
>> +
>> +
>>  (defmacro with-current-notmuch-show-message ( body)
>>"Evaluate body with current buffer set to the text of current message"
>>`(save-excursion
>> @@ -1374,9 +1382,11 @@ current thread."
>>"Are the headers of the current message visible?"
>>(notmuch-show-get-prop :headers-visible))
>>  
>> -(defun notmuch-show-mark-read ()
>> -  "Mark the current message as read."
>> -  (notmuch-show-tag-message "-unread"))
>> +(defun notmuch-show-mark-read ( force)
>> +  "Mark the current message as read if FORCE or
>> +`notmuch-show-auto-mark-read' is non-nil."
>> +  (when (or force notmuch-show-auto-mark-read)
>> +(notmuch-show-tag-message "-unread")))
>
>
> As an alternative approach would allowing a list of tags (or even tag
> changes) to apply when a message is "read" do what you want and be more
> flexible?

That's a great idea -- better than mine (just a variable which informs
which tag is to be removed) .. something like:

(defcustom notmuch-message-read-tag-changes '("-unread") "docstring" ...)

Then one can have e.g.

'() -- for no tag changes
'("-inbox" "-unread" "+read") 
'("-postilaatikko" "-lukematon" "+luettu") -- the above in finnish.


Tomi


> I am thinking of something roughly analogous to
> notmuch-message-replied-tags. I can imagine some people would like to
> remove the inbox tag automatically for example. (And since we sync with
> maildir flags the exact tag name does matter).
>
> I agree with Austin that the current unread marking is a little
> weird/unpredictable (eg notmuch-show-next-message marks a message read
> even if it is closed) but I don't think fixing that helps your use.


>
> Best wishes 
>
> Mark
>
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] sprinters: bugfix when NULL passed for a string.

2012-08-07 Thread Jameson Graef Rollins
On Tue, Aug 07 2012, Mark Walters  wrote:
> The string function in a sprinter may be called with a NULL string
> pointer (eg if a header is absent). This causes a segfault. We fix
> this by checking for a null pointer in the string functions and update
> the sprinter documentation.
>
> At the moment some output when format=text is done directly rather than
> via an sprinter: in that case a null pointer is passed to printf or
> similar and a "(null)" appears in the output. That behaviour is not
> changed in this patch.
> ---
>
> This could really do with some tests (it is the second time this type of
> bug has occurred). To be considered as a message by notmuch new a file
> needs at least one of a From: Subject: or To: header. Thus we should
> have three messages each of which just contains that single header (and
> nothing else) and check that search and show work as expected. 

Hey, Mark.  Thanks for working on this.

I was wondering if we should distinguish between the header being
absent, and having a null value.  It looks like the idea here is to
output an empty string for the value in all of these cases.  But should
we output the field at all if the actual header isn't there?  In other
words, I can imagine three scenarios:

Header: value
Header:--> "Header": ""
no header 

At the moment these would be output as:

"Header": "value"
"Header": ""
"Header": ""

Where as I could imagine we could instead do:

"Header": "value"
"Header": ""
no output

Maybe that would be too complicated or break the output spec to much?
If it's too complicated to do the later, then I'm fine with this
solution as is.

I definitely agree we need tests for this.

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


[PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Michal Nazarewicz
>> From: Michal Nazarewicz 
>> @@ -1383,8 +1390,9 @@ current thread."
>>(notmuch-show-get-prop :headers-visible))
>>  
>>  (defun notmuch-show-mark-read ()
>> -  "Mark the current message as read."
>> -  (notmuch-show-tag-message "-unread"))
>> +  "Apply `notmuch-show-mark-read-tags' to the message."
>> +  (when notmuch-show-mark-read-tags
>> +(apply notmuch-show-tag-message notmuch-show-mark-read-tags)))

Mark Walters  writes:
>  This needs to be (apply 'notmuch-show-tag-message ...)

Yes, it would appear you are right. :)

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Micha? ?mina86? Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120807/5a60dc84/attachment-0001.pgp>


[PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Michal Nazarewicz
> On Mon, 06 Aug 2012, Michal Nazarewicz  wrote:
>> @@ -1374,9 +1382,11 @@ current thread."
>>"Are the headers of the current message visible?"
>>(notmuch-show-get-prop :headers-visible))
>>  
>> -(defun notmuch-show-mark-read ()
>> -  "Mark the current message as read."
>> -  (notmuch-show-tag-message "-unread"))
>> +(defun notmuch-show-mark-read ( force)
>> +  "Mark the current message as read if FORCE or
>> +`notmuch-show-auto-mark-read' is non-nil."
>> +  (when (or force notmuch-show-auto-mark-read)
>> +(notmuch-show-tag-message "-unread")))

Mark Walters  writes:
> As an alternative approach would allowing a list of tags (or even tag
> changes) to apply when a message is "read" do what you want and be more
> flexible?

Something like the following (not tested)?

From: Michal Nazarewicz <min...@mina86.com>
Date: Mon, 6 Aug 2012 15:31:20 +0200
Subject: [PATCH] notmuch-show: add notmuch-show-mark-read-tags option

The `notmuch-show-mark-read-tags' lists tags that are to be applied when
message is read.  By default, the only value is "-unread" which will remove
the unread tag.  Among other uses, this variable can be used to stop
notmuch-show from modifying tags when message is shown (by setting the
variable to an empty list).
---
 emacs/notmuch-show.el |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index dcfc190..92a4beb 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -183,6 +183,13 @@ provided with an MLA argument nor `completing-read' input."
 notmuch-show-stash-mlarchive-link-alist))
   :group 'notmuch-show)

+(defcustom notmuch-show-mark-read-tags '("-unread")
+  "List of tags to apply when message is read, ie. shown in notmuch-show
+buffer."
+  :type '(repeat string)
+  :group 'notmuch-show)
+
+
 (defmacro with-current-notmuch-show-message ( body)
   "Evaluate body with current buffer set to the text of current message"
   `(save-excursion
@@ -1383,8 +1390,9 @@ current thread."
   (notmuch-show-get-prop :headers-visible))

 (defun notmuch-show-mark-read ()
-  "Mark the current message as read."
-  (notmuch-show-tag-message "-unread"))
+  "Apply `notmuch-show-mark-read-tags' to the message."
+  (when notmuch-show-mark-read-tags
+(apply notmuch-show-tag-message notmuch-show-mark-read-tags)))

 ;; Functions for getting attributes of several messages in the current
 ;; thread.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Micha? ?mina86? Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120807/657154a7/attachment.pgp>


[PATCH v2] emacs: notmuch search bugfix

2012-08-07 Thread Mark Walters

The recent change to use json for notmuch-search.el introduced a bug
in the code for keeping position on refresh. The problem is a
comparison between (plist-get result :thread) and a thread-id returned
by notmuch-search-find-thread-id: the latter is prefixed with
"thread:"

We fix this by adding an option to notmuch-search-find-thread-id to
return the bare thread-id. It appears that notmuch-search-refresh-view
is the only caller of notmuch-search that supplies a thread-id so this
change should be safe (but could theoretically break users .emacs
functions).
---

I think this implements all of Austin's suggestions, and it adds a short
remark to NEWS (since in unlikely cases it could break users' functions)
and updates the docstring for notmuch-search. 

Best wishes

Mark

 NEWS |3 +++
 emacs/notmuch.el |   12 +++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 761b2c1..9916c91 100644
--- a/NEWS
+++ b/NEWS
@@ -55,6 +55,9 @@ user-specified formatting
   you've customized this variable, you may have to change your date
   format from `"%s "` to `"%12s "`.

+The thread-id for the `target-thread` argument for `notmuch-search` should
+now be supplied without the "thread:" prefix.
+
 Notmuch 0.13.2 (2012-06-02)
 ===

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index d2d82a9..7b61e9b 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -475,10 +475,12 @@ BEG."
(push (plist-get (notmuch-search-get-result pos) property) output)))
 output))

-(defun notmuch-search-find-thread-id ()
-  "Return the thread for the current thread"
+(defun notmuch-search-find-thread-id ( bare)
+  "Return the thread for the current thread
+
+If BARE is set then do not prefix with \"thread:\""
   (let ((thread (plist-get (notmuch-search-get-result) :thread)))
-(when thread (concat "thread:" thread
+(when thread (concat (unless bare "thread:") thread

 (defun notmuch-search-find-thread-id-region (beg end)
   "Return a list of threads for the current region"
@@ -936,7 +938,7 @@ If `query' is nil, it is read interactively from the 
minibuffer.
 Other optional parameters are used as follows:

   oldest-first: A Boolean controlling the sort order of returned threads
-  target-thread: A thread ID (with the thread: prefix) that will be made
+  target-thread: A thread ID (without the thread: prefix) that will be made
  current if it appears in the search results.
   target-line: The line number to move to if the target thread does not
appear in the search results."
@@ -993,7 +995,7 @@ same relative position within the new buffer."
   (interactive)
   (let ((target-line (line-number-at-pos))
(oldest-first notmuch-search-oldest-first)
-   (target-thread (notmuch-search-find-thread-id))
+   (target-thread (notmuch-search-find-thread-id 'bare))
(query notmuch-search-query-string)
(continuation notmuch-search-continuation))
 (notmuch-kill-this-buffer)
-- 
1.7.9.1



‘class Xapian::Database’ has no member named ‘close’

2012-08-07 Thread Michal Nazarewicz
Hi guys,

when I'm trying to build notmuch on Ubuntu Lucid, I'm getting the
following error:

lib/database.cc: In function ?void notmuch_database_close(notmuch_database_t*)?:
lib/database.cc:767: error: ?class Xapian::Database? has no member named ?close?

I'm solving that by:

diff --git a/lib/database.cc b/lib/database.cc
index 761dc1a..fd78135 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -764,7 +764,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
  * close it.  Thus, we explicitly close it here. */
 if (notmuch->xapian_db != NULL) {
try {
-   notmuch->xapian_db->close();
+   /* notmuch->xapian_db->close(); */
} catch (const Xapian::Error ) {
/* do nothing */
}

which does not seem to break anything.  The Xapian packages installed on
my machine:

$ dpkg -l |grep xapian
ii  apt-xapian-index  0.25ubuntu2  maintenance tools for a Xapian index of 
Debi
ii  libxapian-dev 1.0.18-1 Development files for Xapian search 
engine l
ii  libxapian15   1.0.18-1 Search engine library
ii  python-xapian 1.0.17-1ubuntu1  Xapian search engine interface for Python

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Micha? ?mina86? Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120807/95f32f6e/attachment-0001.pgp>


[PATCH] test: Add test for messages with missing headers

2012-08-07 Thread Austin Clements
Currently the JSON tests for search and show are broken because
notmuch attempts to dereference a NULL pointer.
---
Things to bikeshed:

* Should we include From and Subject in the headers object when there
  are no from or subject headers?  Currently the schema says that
  everything but those two and "Date" is optional (indeed, "To" is
  missing from the second message) but that was just post facto
  standardization.

* How should we format expected JSON in the test suite, now that we
  can format it however we want?  Here I've just pasted in the result
  of python -mjson.tool.  While that was very easy and the result is
  quite readable, it's the antithesis of compact and the keys have
  been alphabetized.

 test/missing-headers |  162 ++
 test/notmuch-test|1 +
 2 files changed, 163 insertions(+)
 create mode 100755 test/missing-headers

diff --git a/test/missing-headers b/test/missing-headers
new file mode 100755
index 000..744c04e
--- /dev/null
+++ b/test/missing-headers
@@ -0,0 +1,162 @@
+#!/usr/bin/env bash
+test_description='messages with missing headers'
+. ./test-lib.sh
+
+# Notmuch requires at least one of from, subject, or to or it will
+# ignore the file.  Generate two messages so that together they cover
+# all possible missing headers.  We also give one of the messages a
+# date to ensure stable result ordering.
+
+cat < "${MAIL_DIR}/msg-2"
+To: Notmuch Test Suite 
+Date: Fri, 05 Jan 2001 15:43:57 +
+
+Body
+EOF
+
+cat < "${MAIL_DIR}/msg-1"
+From: Notmuch Test Suite 
+
+Body
+EOF
+
+NOTMUCH_NEW
+
+test_begin_subtest "Search: text"
+output=$(notmuch search '*' | notmuch_search_sanitize)
+test_expect_equal "$output" "\
+thread:XXX   2001-01-05 [1/1] (null);  (inbox unread)
+thread:XXX   1970-01-01 [1/1] Notmuch Test Suite;  (inbox unread)"
+
+test_begin_subtest "Search: json"
+test_subtest_known_broken
+output=$(notmuch search --format=json '*' | notmuch_search_sanitize)
+test_expect_equal_json "$output" '
+[
+{
+"authors": "",
+"date_relative": "2001-01-05",
+"matched": 1,
+"subject": "",
+"tags": [
+"inbox",
+"unread"
+],
+"thread": "XXX",
+"timestamp": 978709437,
+"total": 1
+},
+{
+"authors": "Notmuch Test Suite",
+"date_relative": "1970-01-01",
+"matched": 1,
+"subject": "",
+"tags": [
+"inbox",
+"unread"
+],
+"thread": "XXX",
+"timestamp": 0,
+"total": 1
+}
+]'
+
+test_begin_subtest "Show: text"
+output=$(notmuch show '*')
+test_expect_equal "$output" "\
+message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 
match:1 excluded:0 filename:/tmp/nmtest/tmp.missing-headers/mail/msg-2
+header{
+ (2001-01-05) (inbox unread)
+Subject: (null)
+From: (null)
+To: Notmuch Test Suite 
+Date: Fri, 05 Jan 2001 15:43:57 +
+header}
+body{
+part{ ID: 1, Content-type: text/plain
+Body
+part}
+body}
+message}
+message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 
match:1 excluded:0 filename:/tmp/nmtest/tmp.missing-headers/mail/msg-1
+header{
+Notmuch Test Suite  (1970-01-01) (inbox unread)
+Subject: (null)
+From: Notmuch Test Suite 
+Date: Thu, 01 Jan 1970 00:00:00 +
+header}
+body{
+part{ ID: 1, Content-type: text/plain
+Body
+part}
+body}
+message}"
+
+test_begin_subtest "Show: json"
+test_subtest_known_broken
+output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize)
+test_expect_equal_json "$output" '
+[
+[
+[
+{
+"body": [
+{
+"content": "Body\n",
+"content-type": "text/plain",
+"id": 1
+}
+],
+"date_relative": "2001-01-05",
+"excluded": false,
+"filename": "Y",
+"headers": {
+"Date": "Fri, 05 Jan 2001 15:43:57 +",
+"From": "",
+"Subject": "",
+"To": "Notmuch Test Suite "
+},
+"id": "X",
+"match": true,
+"tags": [
+"inbox",
+"unread"
+],
+"timestamp": 978709437
+},
+[]
+]
+],
+[
+[
+{
+"body": [
+{
+"content": "Body\n",
+"content-type": "text/plain",
+"id": 1
+}
+],
+"date_relative": "1970-01-01",
+"excluded": false,
+"filename": "Y",
+"headers": {
+"Date": "Thu, 01 Jan 1970 00:00:00 +",
+"From": 

[PATCH] cli: Remove now-unused json.c

2012-08-07 Thread Mark Walters

+1

Mark

On Tue, 07 Aug 2012, Austin Clements  wrote:
> The string buffer quoting functions in json.c have been superseded by
> the new sprinter interface and are no longer used.  Remove them.
> ---
>  Makefile.local |1 -
>  json.c |  109 
> 
>  2 files changed, 110 deletions(-)
>  delete mode 100644 json.c
>
> diff --git a/Makefile.local b/Makefile.local
> index b3b960c..de984ab 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -294,7 +294,6 @@ notmuch_client_srcs = \
>   query-string.c  \
>   mime-node.c \
>   crypto.c\
> - json.c
>  
>  notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
>  
> diff --git a/json.c b/json.c
> deleted file mode 100644
> index 817fc83..000
> --- a/json.c
> +++ /dev/null
> @@ -1,109 +0,0 @@
> -/* notmuch - Not much of an email program, (just index and search)
> - *
> - * Copyright ? 2009 Dave Gamble
> - * Copyright ? 2009 Scott Robinson
> - *
> - * This program is free software: you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation, either version 3 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program.  If not, see http://www.gnu.org/licenses/ .
> - *
> - * Authors: Dave Gamble
> - *  Scott Robinson 
> - *
> - */
> -
> -#include "notmuch-client.h"
> -
> -/* This function was derived from the print_string_ptr function of
> - * cJSON (http://cjson.sourceforge.net/) and is used by permission of
> - * the following license:
> - *
> - * Copyright (c) 2009 Dave Gamble
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> - * of this software and associated documentation files (the "Software"), to 
> deal
> - * in the Software without restriction, including without limitation the 
> rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -
> -char *
> -json_quote_chararray(const void *ctx, const char *str, const size_t len)
> -{
> -const char *ptr;
> -char *ptr2;
> -char *out;
> -size_t loop;
> -size_t required;
> -
> -for (loop = 0, required = 0, ptr = str;
> -  loop < len;
> -  loop++, required++, ptr++) {
> - if ((unsigned char)(*ptr) < 32 || *ptr == '\"' || *ptr == '\\')
> - required++;
> -}
> -
> -/*
> - * + 3 for:
> - * - leading quotation mark,
> - * - trailing quotation mark,
> - * - trailing NULL.
> - */
> -out = talloc_array (ctx, char, required + 3);
> -
> -ptr = str;
> -ptr2 = out;
> -
> -*ptr2++ = '\"';
> -for (loop = 0; loop < len; loop++) {
> - if ((unsigned char)(*ptr) > 31 && *ptr != '\"' && *ptr != '\\') {
> - *ptr2++ = *ptr++;
> - } else {
> - *ptr2++ = '\\';
> - switch (*ptr++) {
> - case '\"':  *ptr2++ = '\"'; break;
> - case '\\':  *ptr2++ = '\\'; break;
> - case '\b':  *ptr2++ = 'b';  break;
> - case '\f':  *ptr2++ = 'f';  break;
> - case '\n':  *ptr2++ = 'n';  break;
> - case '\r':  *ptr2++ = 'r';  break;
> - case '\t':  *ptr2++ = 't';  break;
> - default: ptr2--;break;
> - }
> - }
> -}
> -*ptr2++ = '\"';
> -*ptr2++ = '\0';
> -
> -return out;
> -}
> -
> -char *
> -json_quote_str(const void *ctx, const char *str)
> -{
> -if (str == NULL)
> - str = "";
> -
> -return (json_quote_chararray (ctx, str, strlen (str)));
> -}
> -- 
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> 

[PATCH] reply: Convert JSON format to use sprinter

2012-08-07 Thread Mark Walters


On Tue, 07 Aug 2012, Austin Clements  wrote:
> Almost all of reply was already being formatted using the sprinter.
> This patch converts the top-level dictionary to use the sprinter
> interface.
> ---
>
> One last sprinter piece that had slipped through the cracks.

Looks good to me +1

Mark

>
>  notmuch-reply.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index fa6665f..e60a264 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -614,20 +614,20 @@ notmuch_reply_format_json(void *ctx,
>   return 1;
>  
>  sp = sprinter_json_create (ctx, stdout);
> +sp->begin_map (sp);
>  
>  /* The headers of the reply message we've created */
> -printf ("{\"reply-headers\": ");
> +sp->map_key (sp, "reply-headers");
>  format_headers_json (sp, reply, TRUE);
>  g_object_unref (G_OBJECT (reply));
>  reply = NULL;
>  
>  /* Start the original */
> -printf (", \"original\": ");
> -
> +sp->map_key (sp, "original");
>  format_part_json (ctx, sp, node, TRUE, TRUE);
>  
>  /* End */
> -printf ("}\n");
> +sp->end (sp);
>  notmuch_message_destroy (message);
>  
>  return 0;
> -- 
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Mark Walters
On Tue, 07 Aug 2012, Michal Nazarewicz  wrote:
>> On Mon, 06 Aug 2012, Michal Nazarewicz  wrote:
>>> @@ -1374,9 +1382,11 @@ current thread."
>>>"Are the headers of the current message visible?"
>>>(notmuch-show-get-prop :headers-visible))
>>>  
>>> -(defun notmuch-show-mark-read ()
>>> -  "Mark the current message as read."
>>> -  (notmuch-show-tag-message "-unread"))
>>> +(defun notmuch-show-mark-read ( force)
>>> +  "Mark the current message as read if FORCE or
>>> +`notmuch-show-auto-mark-read' is non-nil."
>>> +  (when (or force notmuch-show-auto-mark-read)
>>> +(notmuch-show-tag-message "-unread")))
>
> Mark Walters  writes:
>> As an alternative approach would allowing a list of tags (or even tag
>> changes) to apply when a message is "read" do what you want and be more
>> flexible?
>
> Something like the following (not tested)?

Yes this was what I had in mind. I like this (both the symmetry with
reply tags and the added flexibility) but I will let others
comment. (There is one small bug in your draft: see below)

Best wishes

Mark

> From: Michal Nazarewicz 
> Date: Mon, 6 Aug 2012 15:31:20 +0200
> Subject: [PATCH] notmuch-show: add notmuch-show-mark-read-tags option
>
> The `notmuch-show-mark-read-tags' lists tags that are to be applied when
> message is read.  By default, the only value is "-unread" which will remove
> the unread tag.  Among other uses, this variable can be used to stop
> notmuch-show from modifying tags when message is shown (by setting the
> variable to an empty list).
> ---
>  emacs/notmuch-show.el |   12 ++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index dcfc190..92a4beb 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -183,6 +183,13 @@ provided with an MLA argument nor `completing-read' 
> input."
>notmuch-show-stash-mlarchive-link-alist))
>:group 'notmuch-show)
>  
> +(defcustom notmuch-show-mark-read-tags '("-unread")
> +  "List of tags to apply when message is read, ie. shown in notmuch-show
> +buffer."
> +  :type '(repeat string)
> +  :group 'notmuch-show)
> +
> +
>  (defmacro with-current-notmuch-show-message ( body)
>"Evaluate body with current buffer set to the text of current message"
>`(save-excursion
> @@ -1383,8 +1390,9 @@ current thread."
>(notmuch-show-get-prop :headers-visible))
>  
>  (defun notmuch-show-mark-read ()
> -  "Mark the current message as read."
> -  (notmuch-show-tag-message "-unread"))
> +  "Apply `notmuch-show-mark-read-tags' to the message."
> +  (when notmuch-show-mark-read-tags
> +(apply notmuch-show-tag-message notmuch-show-mark-read-tags)))

 This needs to be (apply 'notmuch-show-tag-message ...)
>  
>  ;; Functions for getting attributes of several messages in the current
>  ;; thread.
>
> -- 
> Best regards, _ _
> .o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
> ..o | Computer Science,  Micha? ?mina86? Nazarewicz(o o)
> ooo +--ooO--(_)--Ooo--


[PATCH] emacs: notmuch search bugfix

2012-08-07 Thread Mark Walters

The recent change to use json for notmuch-search.el introduced a bug
in the code for keeping position on refresh. The problem is a
comparison between (plist-get result :thread) and a thread-id returned
by notmuch-search-find-thread-id: the latter is prefixed with
"thread:"

We fix this by adding an option to notmuch-search-find-thread-id to
return the bare thread-id. It appears that notmuch-search-refresh-view
is the only caller of notmuch-search that supplies a thread-id so this
change should be safe (but could theoretically break users .emacs
functions).
---

This is a different version of the bugfix suggested in the parent to
this thread. I think it is nicer but there is a slight risk of breaking
users .emacs lisp functions.

In general I think the thread: prefix should be present where the term
is "really" a search term, but absent when it is must be a thread-id,
and this fits with that idea.

With 0.14 freeze pretty close it might be best to apply this to master
and the simpler no breakage option (in id:"87boinpqfg.fsf at qmul.ac.uk")
to the branch.

Best wishes 

Mark

 emacs/notmuch.el |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index d2d82a9..0fff58a 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -475,10 +475,12 @@ BEG."
(push (plist-get (notmuch-search-get-result pos) property) output)))
 output))

-(defun notmuch-search-find-thread-id ()
-  "Return the thread for the current thread"
+(defun notmuch-search-find-thread-id ( bare)
+  "Return the thread for the current thread
+
+If BARE is set then do not prefix with \"thread:\""
   (let ((thread (plist-get (notmuch-search-get-result) :thread)))
-(when thread (concat "thread:" thread
+(when thread (concat (when (not bare) "thread:") thread

 (defun notmuch-search-find-thread-id-region (beg end)
   "Return a list of threads for the current region"
@@ -993,7 +995,7 @@ same relative position within the new buffer."
   (interactive)
   (let ((target-line (line-number-at-pos))
(oldest-first notmuch-search-oldest-first)
-   (target-thread (notmuch-search-find-thread-id))
+   (target-thread (notmuch-search-find-thread-id t))
(query notmuch-search-query-string)
(continuation notmuch-search-continuation))
 (notmuch-kill-this-buffer)
-- 
1.7.9.1



[PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Mark Walters
On Mon, 06 Aug 2012, Michal Nazarewicz  wrote:
> From: Michal Nazarewicz 
>
> Setting `notmuch-show-auto-mark-read' to nil stops notmuch-show from marking
> the message as read (by removing the unread tag).  Inteded for people who
> like to mark messages read explicitly.
> ---
>  emacs/notmuch-show.el |   16 +---
>  1 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index d318430..85a17b1 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -183,6 +183,14 @@ provided with an MLA argument nor `completing-read' 
> input."
>notmuch-show-stash-mlarchive-link-alist))
>:group 'notmuch-show)
>  
> +(defcustom notmuch-show-auto-mark-read t
> +  "Whether to automatically mark message as read when it is shown.  If
> +nil, message needs to be marked as read manually for instance by
> +removing the unread tag."
> +  :type 'boolean
> +  :group 'notmuch-show)
> +
> +
>  (defmacro with-current-notmuch-show-message ( body)
>"Evaluate body with current buffer set to the text of current message"
>`(save-excursion
> @@ -1374,9 +1382,11 @@ current thread."
>"Are the headers of the current message visible?"
>(notmuch-show-get-prop :headers-visible))
>  
> -(defun notmuch-show-mark-read ()
> -  "Mark the current message as read."
> -  (notmuch-show-tag-message "-unread"))
> +(defun notmuch-show-mark-read ( force)
> +  "Mark the current message as read if FORCE or
> +`notmuch-show-auto-mark-read' is non-nil."
> +  (when (or force notmuch-show-auto-mark-read)
> +(notmuch-show-tag-message "-unread")))


As an alternative approach would allowing a list of tags (or even tag
changes) to apply when a message is "read" do what you want and be more
flexible? I am thinking of something roughly analogous to
notmuch-message-replied-tags. I can imagine some people would like to
remove the inbox tag automatically for example. (And since we sync with
maildir flags the exact tag name does matter).


I agree with Austin that the current unread marking is a little
weird/unpredictable (eg notmuch-show-next-message marks a message read
even if it is closed) but I don't think fixing that helps your use.

Best wishes 

Mark




[PATCH] cli: Remove now-unused json.c

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Austin Clements  wrote:

> The string buffer quoting functions in json.c have been superseded by
> the new sprinter interface and are no longer used.  Remove them.
> ---

+1

Tomi


>  Makefile.local |1 -
>  json.c |  109 
> 
>  2 files changed, 110 deletions(-)
>  delete mode 100644 json.c
>
> diff --git a/Makefile.local b/Makefile.local
> index b3b960c..de984ab 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -294,7 +294,6 @@ notmuch_client_srcs = \
>   query-string.c  \
>   mime-node.c \
>   crypto.c\
> - json.c
>  
>  notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
>  
> diff --git a/json.c b/json.c
> deleted file mode 100644
> index 817fc83..000
> --- a/json.c
> +++ /dev/null
> @@ -1,109 +0,0 @@
> -/* notmuch - Not much of an email program, (just index and search)
> - *
> - * Copyright ? 2009 Dave Gamble
> - * Copyright ? 2009 Scott Robinson
> - *
> - * This program is free software: you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation, either version 3 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program.  If not, see http://www.gnu.org/licenses/ .
> - *
> - * Authors: Dave Gamble
> - *  Scott Robinson 
> - *
> - */
> -
> -#include "notmuch-client.h"
> -
> -/* This function was derived from the print_string_ptr function of
> - * cJSON (http://cjson.sourceforge.net/) and is used by permission of
> - * the following license:
> - *
> - * Copyright (c) 2009 Dave Gamble
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> - * of this software and associated documentation files (the "Software"), to 
> deal
> - * in the Software without restriction, including without limitation the 
> rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -
> -char *
> -json_quote_chararray(const void *ctx, const char *str, const size_t len)
> -{
> -const char *ptr;
> -char *ptr2;
> -char *out;
> -size_t loop;
> -size_t required;
> -
> -for (loop = 0, required = 0, ptr = str;
> -  loop < len;
> -  loop++, required++, ptr++) {
> - if ((unsigned char)(*ptr) < 32 || *ptr == '\"' || *ptr == '\\')
> - required++;
> -}
> -
> -/*
> - * + 3 for:
> - * - leading quotation mark,
> - * - trailing quotation mark,
> - * - trailing NULL.
> - */
> -out = talloc_array (ctx, char, required + 3);
> -
> -ptr = str;
> -ptr2 = out;
> -
> -*ptr2++ = '\"';
> -for (loop = 0; loop < len; loop++) {
> - if ((unsigned char)(*ptr) > 31 && *ptr != '\"' && *ptr != '\\') {
> - *ptr2++ = *ptr++;
> - } else {
> - *ptr2++ = '\\';
> - switch (*ptr++) {
> - case '\"':  *ptr2++ = '\"'; break;
> - case '\\':  *ptr2++ = '\\'; break;
> - case '\b':  *ptr2++ = 'b';  break;
> - case '\f':  *ptr2++ = 'f';  break;
> - case '\n':  *ptr2++ = 'n';  break;
> - case '\r':  *ptr2++ = 'r';  break;
> - case '\t':  *ptr2++ = 't';  break;
> - default: ptr2--;break;
> - }
> - }
> -}
> -*ptr2++ = '\"';
> -*ptr2++ = '\0';
> -
> -return out;
> -}
> -
> -char *
> -json_quote_str(const void *ctx, const char *str)
> -{
> -if (str == NULL)
> - str = "";
> -
> -return (json_quote_chararray (ctx, str, strlen (str)));
> -}
> -- 
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> 

[PATCH] reply: Convert JSON format to use sprinter

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Austin Clements  wrote:

> Almost all of reply was already being formatted using the sprinter.
> This patch converts the top-level dictionary to use the sprinter
> interface.
> ---

LGTM.

Tomi


>
> One last sprinter piece that had slipped through the cracks.
>
>  notmuch-reply.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index fa6665f..e60a264 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -614,20 +614,20 @@ notmuch_reply_format_json(void *ctx,
>   return 1;
>  
>  sp = sprinter_json_create (ctx, stdout);
> +sp->begin_map (sp);
>  
>  /* The headers of the reply message we've created */
> -printf ("{\"reply-headers\": ");
> +sp->map_key (sp, "reply-headers");
>  format_headers_json (sp, reply, TRUE);
>  g_object_unref (G_OBJECT (reply));
>  reply = NULL;
>  
>  /* Start the original */
> -printf (", \"original\": ");
> -
> +sp->map_key (sp, "original");
>  format_part_json (ctx, sp, node, TRUE, TRUE);
>  
>  /* End */
> -printf ("}\n");
> +sp->end (sp);
>  notmuch_message_destroy (message);
>  
>  return 0;
> -- 
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2] emacs: notmuch search bugfix

2012-08-07 Thread Austin Clements
On Tue, 07 Aug 2012, Mark Walters  wrote:
> The recent change to use json for notmuch-search.el introduced a bug
> in the code for keeping position on refresh. The problem is a
> comparison between (plist-get result :thread) and a thread-id returned
> by notmuch-search-find-thread-id: the latter is prefixed with
> "thread:"
>
> We fix this by adding an option to notmuch-search-find-thread-id to
> return the bare thread-id. It appears that notmuch-search-refresh-view
> is the only caller of notmuch-search that supplies a thread-id so this
> change should be safe (but could theoretically break users .emacs
> functions).
> ---
>
> I think this implements all of Austin's suggestions, and it adds a short
> remark to NEWS (since in unlikely cases it could break users' functions)
> and updates the docstring for notmuch-search. 

LGTM.


[PATCH] emacs: notmuch search bugfix

2012-08-07 Thread Austin Clements
Quoth Mark Walters on Aug 07 at  4:40 pm:
> 
> The recent change to use json for notmuch-search.el introduced a bug
> in the code for keeping position on refresh. The problem is a
> comparison between (plist-get result :thread) and a thread-id returned
> by notmuch-search-find-thread-id: the latter is prefixed with
> "thread:"
> 
> We fix this by adding an option to notmuch-search-find-thread-id to
> return the bare thread-id. It appears that notmuch-search-refresh-view
> is the only caller of notmuch-search that supplies a thread-id so this
> change should be safe (but could theoretically break users .emacs
> functions).
> ---
> 
> This is a different version of the bugfix suggested in the parent to
> this thread. I think it is nicer but there is a slight risk of breaking
> users .emacs lisp functions.
> 
> In general I think the thread: prefix should be present where the term
> is "really" a search term, but absent when it is must be a thread-id,
> and this fits with that idea.

Agreed.

> With 0.14 freeze pretty close it might be best to apply this to master
> and the simpler no breakage option (in id:"87boinpqfg.fsf at qmul.ac.uk")
> to the branch.
> 
> Best wishes 
> 
> Mark
> 
>  emacs/notmuch.el |   10 ++
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index d2d82a9..0fff58a 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -475,10 +475,12 @@ BEG."
>   (push (plist-get (notmuch-search-get-result pos) property) output)))
>  output))
>  
> -(defun notmuch-search-find-thread-id ()
> -  "Return the thread for the current thread"
> +(defun notmuch-search-find-thread-id ( bare)
> +  "Return the thread for the current thread
> +
> +If BARE is set then do not prefix with \"thread:\""
>(let ((thread (plist-get (notmuch-search-get-result) :thread)))
> -(when thread (concat "thread:" thread
> +(when thread (concat (when (not bare) "thread:") thread

(unless bare "thread:")?

>  
>  (defun notmuch-search-find-thread-id-region (beg end)
>"Return a list of threads for the current region"
> @@ -993,7 +995,7 @@ same relative position within the new buffer."
>(interactive)
>(let ((target-line (line-number-at-pos))
>   (oldest-first notmuch-search-oldest-first)
> - (target-thread (notmuch-search-find-thread-id))
> + (target-thread (notmuch-search-find-thread-id t))

Perhaps pass 'bare instead of t to make this self-documenting?

>   (query notmuch-search-query-string)
>   (continuation notmuch-search-continuation))
>  (notmuch-kill-this-buffer)


Vim plugins

2012-08-07 Thread Anton Khirnov

On Tue, 7 Aug 2012 09:35:02 +0100 (BST), Sepp Tannhuber  wrote:
> That's strange. I suppose there must be a function called
> ? NMVimpy
> in one of your scripts. But
> ? grep -R NMVimpy *
> in your notmuch directory does not find anything. I am completely at a loss.
> 

vim/plugin/notmuch-vimpy.vim, line 817

-- 
Anton Khirnov


[PATCH 1/4] show: indicate length of omitted body content (json)

2012-08-07 Thread Austin Clements
Quoting Peter Wang :
> On Mon, 6 Aug 2012 12:47:10 -0400, Austin Clements  
> wrote:
>> What's the overall goal of adding this?  Are you planning to add size
>> information to one of the frontends?
>
> Yes, to my frontend.
>
>>> > diff --git a/devel/schemata b/devel/schemata
>> > index 9cb25f5..3df2764 100644
>> > --- a/devel/schemata
>> > +++ b/devel/schemata
>> > @@ -69,7 +69,10 @@ part = {
>> >  # A leaf part's body content is optional, but may be included if
>> >  # it can be correctly encoded as a string.  Consumers should use
>> >  # this in preference to fetching the part content separately.
>> > -content?:   string
>> > +content?:   string,
>> > +# If a leaf part's body content is not included, the content-length
>> > +# may be included instead.
>>
>> You mentioned elsewhere that the content-length returned is an
>> estimate.  If that's the case, this comment should say as much.  Is it
>> actually the case, though?  g_mime_part_get_content_object is
>> remarkably poorly documented for such an important function, but based
>> on format_part_raw, it seems like the content-length your code returns
>> will be exactly the number of bytes returned by the raw format for a
>> leaf part.
>
> It's the exact length of the _encoded_ content.  If the transfer
> encoding is base64, multiplying by 3/4 will get a close estimate of the
> decoded content length.  I assume quoted-printable encoding would only
> be used if the content is mostly ASCII, so the encoded length can serve
> as the estimated decoded length then.

Ah, I see.  format_part_raw misled me; apparently the
g_mime_data_wrapper_write_to_stream is key there, since *that* decodes
the transfer encoding of the data wrapper's underlying, raw stream.

In that case, the comment could either mention that this is the length
of the transfer encoded content or it could say it's an approximation
of the decoded length.  The advantage of only claiming the latter is
that it would leave open the possibility of, say, multiplying by .75
for base64 transfer encoding to get a better decoded estimate (your
assumption about quoted-printable sounds completely reasonable).
Alternatively, we could add the transfer encoding in the future and
let the caller do such approximations.

>> > diff --git a/notmuch-show.c b/notmuch-show.c
>> > index 3556293..5c54257 100644
>> > --- a/notmuch-show.c
>> > +++ b/notmuch-show.c
>> > @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t 
>> *sp, mime_node_t *node,
>> >sp->map_key (sp, "content");
>> >sp->string_len (sp, (char *) part_content->data, part_content->len);
>> >g_object_unref (stream_memory);
>> > +  } else {
>> > +  GMimeDataWrapper *wrapper = g_mime_part_get_content_object 
>> (GMIME_PART (node->part));
>> > +  GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
>> > +  ssize_t length = g_mime_stream_length (stream);
>> > +  if (length >= 0) {
>> > +  sp->map_key (sp, "content-length");
>> > +  sp->integer (sp, length);
>> > +  }
>>
>> Do wrapper or stream need to be g_object_unref'd?
>
> No.
>
>> Any idea what the performance overhead of this is?  I'm just curious.
>> It might be approximately nothing, since GMime's parser is eager.
>
> The start and end bounds of the stream are already known so there's
> approximately nothing for g_mime_stream_length to do.  The other
> functions simply return field values.

Sounds good.

> I'll drop the changes for text output.
>
> Peter



Vim plugins

2012-08-07 Thread Sepp Tannhuber
That's strange. I suppose there must be a function called
? NMVimpy
in one of your scripts. But
? grep -R NMVimpy *
in your notmuch directory does not find anything. I am completely at a loss.


- Urspr?ngliche Message -
Von: Anton Khirnov 
An: Sepp Tannhuber ; "notmuch at notmuchmail.org" 

CC: 
Gesendet: 6:42 Dienstag, 7.August 2012
Betreff: Re: Vim plugins


On Mon, 6 Aug 2012 23:29:08 +0100 (BST), Sepp Tannhuber  wrote:
> Hi Anton,
> 
> thanks for answering. Finally I found it. My next problem is that I have 
> absolutely no idea how to use it.
> I followed the instructions I have found in this mailing list and copied the 
> syntax files into my ~/.vim/syntax
> and the two plugin files?into my ~/.vim/plugin folders.
> 
> Then I called
> ? ? vi -c 'NMVimpy()'
> The result is
> ? ? E492: Not an editor command: NMVimpy()
> 
> I tried
> ? ? vi -c ':nm_vimpy()'
> as well. And vi answers
> ? ? No mapping found
> 

I invoke it as vim -c :NMVimpy

> Is there a README file or any other kind of documentation for your script?
> 

I wish. Patches welcome ;)

-- 
Anton Khirnov



[PATCH] cli: Remove now-unused json.c

2012-08-07 Thread Austin Clements
The string buffer quoting functions in json.c have been superseded by
the new sprinter interface and are no longer used.  Remove them.
---
 Makefile.local |1 -
 json.c |  109 
 2 files changed, 110 deletions(-)
 delete mode 100644 json.c

diff --git a/Makefile.local b/Makefile.local
index b3b960c..de984ab 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -294,7 +294,6 @@ notmuch_client_srcs =   \
query-string.c  \
mime-node.c \
crypto.c\
-   json.c

 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)

diff --git a/json.c b/json.c
deleted file mode 100644
index 817fc83..000
--- a/json.c
+++ /dev/null
@@ -1,109 +0,0 @@
-/* notmuch - Not much of an email program, (just index and search)
- *
- * Copyright ? 2009 Dave Gamble
- * Copyright ? 2009 Scott Robinson
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see http://www.gnu.org/licenses/ .
- *
- * Authors: Dave Gamble
- *  Scott Robinson 
- *
- */
-
-#include "notmuch-client.h"
-
-/* This function was derived from the print_string_ptr function of
- * cJSON (http://cjson.sourceforge.net/) and is used by permission of
- * the following license:
- *
- * Copyright (c) 2009 Dave Gamble
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-char *
-json_quote_chararray(const void *ctx, const char *str, const size_t len)
-{
-const char *ptr;
-char *ptr2;
-char *out;
-size_t loop;
-size_t required;
-
-for (loop = 0, required = 0, ptr = str;
-loop < len;
-loop++, required++, ptr++) {
-   if ((unsigned char)(*ptr) < 32 || *ptr == '\"' || *ptr == '\\')
-   required++;
-}
-
-/*
- * + 3 for:
- * - leading quotation mark,
- * - trailing quotation mark,
- * - trailing NULL.
- */
-out = talloc_array (ctx, char, required + 3);
-
-ptr = str;
-ptr2 = out;
-
-*ptr2++ = '\"';
-for (loop = 0; loop < len; loop++) {
-   if ((unsigned char)(*ptr) > 31 && *ptr != '\"' && *ptr != '\\') {
-   *ptr2++ = *ptr++;
-   } else {
-   *ptr2++ = '\\';
-   switch (*ptr++) {
-   case '\"':  *ptr2++ = '\"'; break;
-   case '\\':  *ptr2++ = '\\'; break;
-   case '\b':  *ptr2++ = 'b';  break;
-   case '\f':  *ptr2++ = 'f';  break;
-   case '\n':  *ptr2++ = 'n';  break;
-   case '\r':  *ptr2++ = 'r';  break;
-   case '\t':  *ptr2++ = 't';  break;
-   default: ptr2--;break;
-   }
-   }
-}
-*ptr2++ = '\"';
-*ptr2++ = '\0';
-
-return out;
-}
-
-char *
-json_quote_str(const void *ctx, const char *str)
-{
-if (str == NULL)
-   str = "";
-
-return (json_quote_chararray (ctx, str, strlen (str)));
-}
-- 
1.7.10



Segmentation fault in notmuch search --format=json

2012-08-07 Thread Mark Walters
On Tue, 07 Aug 2012, Ben Gamari  wrote:
> It seems some messages trigger a segmentation fault in
> `do_search_threads()`. It appears the problem occurs (at least) when
> `authors` is NULL.

Hi thanks for the bug report and detailed debugging. I think I can see
the problem and there is a test patch to fix it below, and this does
appear to be a regression.

In json.c the function json_quote_str explicitly checks/allows for a
NULL pointer passed as a string and pretends it is just an empty
string. That behaviour was lost in the move to structured formatters.

A simple fix is to put this check for a null pointer in json_string in
sprinter-json.c which is what this patch does.

Incidentally this is the second time this bug has appeared: 

commit cacefbf3d6dd5bce0b60b3cdfce29bfa371dfaea
Author: David Edmondson 
Date:   Tue Apr 6 08:24:00 2010 +0100

json: Avoid calling strlen(NULL)

MIME parts may have no filename, which previously resulted 
in calling
strlen(NULL).

so it really might be worth having a test for it!

Finally, I think nothing in json.c is used anymore so perhaps it
 could be removed.


diff --git a/sprinter-json.c b/sprinter-json.c
index c9b6835..0a07790 100644
--- a/sprinter-json.c
+++ b/sprinter-json.c
@@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, 
size_t len)
 static void
 json_string (struct sprinter *sp, const char *val)
 {
+if (val == NULL)
+   val = "";
 json_string_len (sp, val, strlen (val));
 }




Vim plugins

2012-08-07 Thread Anton Khirnov

On Mon, 6 Aug 2012 23:29:08 +0100 (BST), Sepp Tannhuber  wrote:
> Hi Anton,
> 
> thanks for answering. Finally I found it. My next problem is that I have 
> absolutely no idea how to use it.
> I followed the instructions I have found in this mailing list and copied the 
> syntax files into my ~/.vim/syntax
> and the two plugin files?into my ~/.vim/plugin folders.
> 
> Then I called
> ? ? vi -c 'NMVimpy()'
> The result is
> ? ? E492: Not an editor command: NMVimpy()
> 
> I tried
> ? ? vi -c ':nm_vimpy()'
> as well. And vi answers
> ? ? No mapping found
> 

I invoke it as vim -c :NMVimpy

> Is there a README file or any other kind of documentation for your script?
> 

I wish. Patches welcome ;)

-- 
Anton Khirnov


Segmentation fault in notmuch search --format=json

2012-08-07 Thread Ben Gamari
It seems some messages trigger a segmentation fault in
`do_search_threads()`. It appears the problem occurs (at least) when
`authors` is NULL.

Program received signal SIGSEGV, Segmentation fault.
0x00415aa3 in json_string (sp=0x646c70, val=0x0) at 
sprinter-json.c:121
121 json_string_len (sp, val, strlen (val));
(gdb) bt
#0  0x00415aa3 in json_string (sp=0x646c70, val=0x0)
at sprinter-json.c:121
#1  0x0041084b in do_search_threads (format=0x646c70, 
query=0x64bb70, 
sort=NOTMUCH_SORT_NEWEST_FIRST, output=OUTPUT_SUMMARY, offset=0, 
limit=-1)
at notmuch-search.c:131
#2  0x00411361 in notmuch_search_command (ctx=0x6361b0, argc=3, 
argv=0x7fffdfb0) at notmuch-search.c:405
#3  0x00409e22 in main (argc=4, argv=0x7fffdfa8) at 
notmuch.c:294
(gdb) up
#1  0x0041084b in do_search_threads (format=0x646c70, 
query=0x64bb70, 
sort=NOTMUCH_SORT_NEWEST_FIRST, output=OUTPUT_SUMMARY, offset=0, 
limit=-1)
at notmuch-search.c:131
131 format->string (format, authors);
(gdb) list
126 format->map_key (format, "matched");
127 format->integer (format, matched);
128 format->map_key (format, "total");
129 format->integer (format, total);
130 format->map_key (format, "authors");
131 format->string (format, authors);
132 format->map_key (format, "subject");
133 format->string (format, subject);
134 }
135 
(gdb) 

It seems that the message in question was produced by a script I use to
feed RSS feeds into notmuch so while I wouldn't doubt that there are few
cases where `authors` should be NULL, it would be nice if notmuch
handled this case with a bit more grace.

Cheers,

- Ben



Segmentation fault in notmuch search --format=json

2012-08-07 Thread Jameson Graef Rollins
On Tue, Aug 07 2012, Mark Walters  wrote:
> On Tue, 07 Aug 2012, Ben Gamari  wrote:
>> It seems some messages trigger a segmentation fault in
>> `do_search_threads()`. It appears the problem occurs (at least) when
>> `authors` is NULL.
>
> Hi thanks for the bug report and detailed debugging. I think I can see
> the problem and there is a test patch to fix it below, and this does
> appear to be a regression.
>
> In json.c the function json_quote_str explicitly checks/allows for a
> NULL pointer passed as a string and pretends it is just an empty
> string. That behaviour was lost in the move to structured formatters.
>
> A simple fix is to put this check for a null pointer in json_string in
> sprinter-json.c which is what this patch does.

Thanks Mark!  I was experiencing the same problem and this fixed the
issue before I even got a chance to respond.  This seems like a fine
solution.

> Incidentally this is the second time this bug has appeared: 
>
> commit cacefbf3d6dd5bce0b60b3cdfce29bfa371dfaea
> Author: David Edmondson 
> Date:   Tue Apr 6 08:24:00 2010 +0100
>
> json: Avoid calling strlen(NULL)
> 
> MIME parts may have no filename, which previously 
> resulted in calling
> strlen(NULL).
>
> so it really might be worth having a test for it!

Indeed!  I think the problematic email in this case was one with no
subject.

> Finally, I think nothing in json.c is used anymore so perhaps it
>  could be removed.

Agreed.

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


Small bug in json-search changes

2012-08-07 Thread Mark Walters

Hi

I have found a small bug in the recent changes to notmuch search to use
the JSON output. If you refresh the search buffer "point" does not stay
on the same thread. 

I think the problem is that notmuch-search-refresh-view calls notmuch
search with target-thread set to notmuch-search-find-thread-id which
returns the thread-id with "thread:" prefixed, whereas
notmuch-search-show-result checks for a match with (plist-get result
:thread) which is the thread id with no prefix.

The patch below fixes this but I am not sure it is the nicest fix.

Best wishes

Mark

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index d2d82a9..a53d5a0 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -798,7 +798,7 @@ non-authors is found, assume that all of the authors match."
(insert "\n")
(notmuch-search-color-line beg (point) (plist-get result :tags))
(put-text-property beg (point) 'notmuch-search-result result))
-  (when (string= (plist-get result :thread) notmuch-search-target-thread)
+  (when (string= (concat "thread:" (plist-get result :thread)) 
notmuch-search-target-thread)
(setq notmuch-search-target-thread "found")
(goto-char beg)




Vim plugins

2012-08-07 Thread Sepp Tannhuber
Hi Anton,

thanks for answering. Finally I found it. My next problem is that I have 
absolutely no idea how to use it.
I followed the instructions I have found in this mailing list and copied the 
syntax files into my ~/.vim/syntax
and the two plugin files?into my ~/.vim/plugin folders.

Then I called
? ? vi -c 'NMVimpy()'
The result is
? ? E492: Not an editor command: NMVimpy()

I tried
? ? vi -c ':nm_vimpy()'
as well. And vi answers
? ? No mapping found

Is there a README file or any other kind of documentation for your script?

Best regards
Joseph



Re: Segmentation fault in notmuch search --format=json

2012-08-07 Thread Mark Walters
On Tue, 07 Aug 2012, Ben Gamari bgamari.f...@gmail.com wrote:
 It seems some messages trigger a segmentation fault in
 `do_search_threads()`. It appears the problem occurs (at least) when
 `authors` is NULL.

Hi thanks for the bug report and detailed debugging. I think I can see
the problem and there is a test patch to fix it below, and this does
appear to be a regression.

In json.c the function json_quote_str explicitly checks/allows for a
NULL pointer passed as a string and pretends it is just an empty
string. That behaviour was lost in the move to structured formatters.

A simple fix is to put this check for a null pointer in json_string in
sprinter-json.c which is what this patch does.

Incidentally this is the second time this bug has appeared: 

commit cacefbf3d6dd5bce0b60b3cdfce29bfa371dfaea
Author: David Edmondson d...@dme.org
Date:   Tue Apr 6 08:24:00 2010 +0100

json: Avoid calling strlen(NULL)

MIME parts may have no filename, which previously resulted 
in calling
strlen(NULL).

so it really might be worth having a test for it!

Finally, I think nothing in json.c is used anymore so perhaps it
 could be removed.


diff --git a/sprinter-json.c b/sprinter-json.c
index c9b6835..0a07790 100644
--- a/sprinter-json.c
+++ b/sprinter-json.c
@@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, 
size_t len)
 static void
 json_string (struct sprinter *sp, const char *val)
 {
+if (val == NULL)
+   val = ;
 json_string_len (sp, val, strlen (val));
 }
 

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


Re: Segmentation fault in notmuch search --format=json

2012-08-07 Thread Jameson Graef Rollins
On Tue, Aug 07 2012, Mark Walters markwalters1...@gmail.com wrote:
 On Tue, 07 Aug 2012, Ben Gamari bgamari.f...@gmail.com wrote:
 It seems some messages trigger a segmentation fault in
 `do_search_threads()`. It appears the problem occurs (at least) when
 `authors` is NULL.

 Hi thanks for the bug report and detailed debugging. I think I can see
 the problem and there is a test patch to fix it below, and this does
 appear to be a regression.

 In json.c the function json_quote_str explicitly checks/allows for a
 NULL pointer passed as a string and pretends it is just an empty
 string. That behaviour was lost in the move to structured formatters.

 A simple fix is to put this check for a null pointer in json_string in
 sprinter-json.c which is what this patch does.

Thanks Mark!  I was experiencing the same problem and this fixed the
issue before I even got a chance to respond.  This seems like a fine
solution.

 Incidentally this is the second time this bug has appeared: 

 commit cacefbf3d6dd5bce0b60b3cdfce29bfa371dfaea
 Author: David Edmondson d...@dme.org
 Date:   Tue Apr 6 08:24:00 2010 +0100

 json: Avoid calling strlen(NULL)
 
 MIME parts may have no filename, which previously 
 resulted in calling
 strlen(NULL).

 so it really might be worth having a test for it!

Indeed!  I think the problematic email in this case was one with no
subject.

 Finally, I think nothing in json.c is used anymore so perhaps it
  could be removed.

Agreed.

jamie.


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


Re: Vim plugins

2012-08-07 Thread Sepp Tannhuber
That's strange. I suppose there must be a function called
  NMVimpy
in one of your scripts. But
  grep -R NMVimpy *
in your notmuch directory does not find anything. I am completely at a loss.


- Ursprüngliche Message -
Von: Anton Khirnov an...@khirnov.net
An: Sepp Tannhuber sepp.tannhu...@yahoo.de; notmuch@notmuchmail.org 
notmuch@notmuchmail.org
CC: 
Gesendet: 6:42 Dienstag, 7.August 2012
Betreff: Re: Vim plugins


On Mon, 6 Aug 2012 23:29:08 +0100 (BST), Sepp Tannhuber 
sepp.tannhu...@yahoo.de wrote:
 Hi Anton,
 
 thanks for answering. Finally I found it. My next problem is that I have 
 absolutely no idea how to use it.
 I followed the instructions I have found in this mailing list and copied the 
 syntax files into my ~/.vim/syntax
 and the two plugin files into my ~/.vim/plugin folders.
 
 Then I called
     vi -c 'NMVimpy()'
 The result is
     E492: Not an editor command: NMVimpy()
 
 I tried
     vi -c ':nm_vimpy()'
 as well. And vi answers
     No mapping found
 

I invoke it as vim -c :NMVimpy

 Is there a README file or any other kind of documentation for your script?
 

I wish. Patches welcome ;)

-- 
Anton Khirnov

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


Re: Vim plugins

2012-08-07 Thread Anton Khirnov

On Tue, 7 Aug 2012 09:35:02 +0100 (BST), Sepp Tannhuber 
sepp.tannhu...@yahoo.de wrote:
 That's strange. I suppose there must be a function called
   NMVimpy
 in one of your scripts. But
   grep -R NMVimpy *
 in your notmuch directory does not find anything. I am completely at a loss.
 

vim/plugin/notmuch-vimpy.vim, line 817

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


Re: Segmentation fault in notmuch search --format=json

2012-08-07 Thread Austin Clements
Quoth Mark Walters on Aug 07 at  8:07 am:
 On Tue, 07 Aug 2012, Ben Gamari bgamari.f...@gmail.com wrote:
  It seems some messages trigger a segmentation fault in
  `do_search_threads()`. It appears the problem occurs (at least) when
  `authors` is NULL.
 
 Hi thanks for the bug report and detailed debugging. I think I can see
 the problem and there is a test patch to fix it below, and this does
 appear to be a regression.
 
 In json.c the function json_quote_str explicitly checks/allows for a
 NULL pointer passed as a string and pretends it is just an empty
 string. That behaviour was lost in the move to structured formatters.
 
 A simple fix is to put this check for a null pointer in json_string in
 sprinter-json.c which is what this patch does.
 
 Incidentally this is the second time this bug has appeared: 
 
 commit cacefbf3d6dd5bce0b60b3cdfce29bfa371dfaea
 Author: David Edmondson d...@dme.org
 Date:   Tue Apr 6 08:24:00 2010 +0100
 
 json: Avoid calling strlen(NULL)
 
 MIME parts may have no filename, which previously 
 resulted in calling
 strlen(NULL).
 
 so it really might be worth having a test for it!
 
 Finally, I think nothing in json.c is used anymore so perhaps it
  could be removed.

LGTM.  We'll want to do something similar for text_string and, of
course, update the sprinter doc comments.

 diff --git a/sprinter-json.c b/sprinter-json.c
 index c9b6835..0a07790 100644
 --- a/sprinter-json.c
 +++ b/sprinter-json.c
 @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, 
 size_t len)
  static void
  json_string (struct sprinter *sp, const char *val)
  {
 +if (val == NULL)
 + val = ;
  json_string_len (sp, val, strlen (val));
  }
  
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] reply: Convert JSON format to use sprinter

2012-08-07 Thread Austin Clements
Almost all of reply was already being formatted using the sprinter.
This patch converts the top-level dictionary to use the sprinter
interface.
---

One last sprinter piece that had slipped through the cracks.

 notmuch-reply.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index fa6665f..e60a264 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -614,20 +614,20 @@ notmuch_reply_format_json(void *ctx,
return 1;
 
 sp = sprinter_json_create (ctx, stdout);
+sp-begin_map (sp);
 
 /* The headers of the reply message we've created */
-printf ({\reply-headers\: );
+sp-map_key (sp, reply-headers);
 format_headers_json (sp, reply, TRUE);
 g_object_unref (G_OBJECT (reply));
 reply = NULL;
 
 /* Start the original */
-printf (, \original\: );
-
+sp-map_key (sp, original);
 format_part_json (ctx, sp, node, TRUE, TRUE);
 
 /* End */
-printf (}\n);
+sp-end (sp);
 notmuch_message_destroy (message);
 
 return 0;
-- 
1.7.10

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


[PATCH] cli: Remove now-unused json.c

2012-08-07 Thread Austin Clements
The string buffer quoting functions in json.c have been superseded by
the new sprinter interface and are no longer used.  Remove them.
---
 Makefile.local |1 -
 json.c |  109 
 2 files changed, 110 deletions(-)
 delete mode 100644 json.c

diff --git a/Makefile.local b/Makefile.local
index b3b960c..de984ab 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -294,7 +294,6 @@ notmuch_client_srcs =   \
query-string.c  \
mime-node.c \
crypto.c\
-   json.c
 
 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
 
diff --git a/json.c b/json.c
deleted file mode 100644
index 817fc83..000
--- a/json.c
+++ /dev/null
@@ -1,109 +0,0 @@
-/* notmuch - Not much of an email program, (just index and search)
- *
- * Copyright © 2009 Dave Gamble
- * Copyright © 2009 Scott Robinson
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see http://www.gnu.org/licenses/ .
- *
- * Authors: Dave Gamble
- *  Scott Robinson sc...@quadhome.com
- *
- */
-
-#include notmuch-client.h
-
-/* This function was derived from the print_string_ptr function of
- * cJSON (http://cjson.sourceforge.net/) and is used by permission of
- * the following license:
- *
- * Copyright (c) 2009 Dave Gamble
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the Software), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-char *
-json_quote_chararray(const void *ctx, const char *str, const size_t len)
-{
-const char *ptr;
-char *ptr2;
-char *out;
-size_t loop;
-size_t required;
-
-for (loop = 0, required = 0, ptr = str;
-loop  len;
-loop++, required++, ptr++) {
-   if ((unsigned char)(*ptr)  32 || *ptr == '\' || *ptr == '\\')
-   required++;
-}
-
-/*
- * + 3 for:
- * - leading quotation mark,
- * - trailing quotation mark,
- * - trailing NULL.
- */
-out = talloc_array (ctx, char, required + 3);
-
-ptr = str;
-ptr2 = out;
-
-*ptr2++ = '\';
-for (loop = 0; loop  len; loop++) {
-   if ((unsigned char)(*ptr)  31  *ptr != '\'  *ptr != '\\') {
-   *ptr2++ = *ptr++;
-   } else {
-   *ptr2++ = '\\';
-   switch (*ptr++) {
-   case '\':  *ptr2++ = '\'; break;
-   case '\\':  *ptr2++ = '\\'; break;
-   case '\b':  *ptr2++ = 'b';  break;
-   case '\f':  *ptr2++ = 'f';  break;
-   case '\n':  *ptr2++ = 'n';  break;
-   case '\r':  *ptr2++ = 'r';  break;
-   case '\t':  *ptr2++ = 't';  break;
-   default: ptr2--;break;
-   }
-   }
-}
-*ptr2++ = '\';
-*ptr2++ = '\0';
-
-return out;
-}
-
-char *
-json_quote_str(const void *ctx, const char *str)
-{
-if (str == NULL)
-   str = ;
-
-return (json_quote_chararray (ctx, str, strlen (str)));
-}
-- 
1.7.10

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


Re: [PATCH] reply: Convert JSON format to use sprinter

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Austin Clements amdra...@mit.edu wrote:

 Almost all of reply was already being formatted using the sprinter.
 This patch converts the top-level dictionary to use the sprinter
 interface.
 ---

LGTM.

Tomi



 One last sprinter piece that had slipped through the cracks.

  notmuch-reply.c |8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/notmuch-reply.c b/notmuch-reply.c
 index fa6665f..e60a264 100644
 --- a/notmuch-reply.c
 +++ b/notmuch-reply.c
 @@ -614,20 +614,20 @@ notmuch_reply_format_json(void *ctx,
   return 1;
  
  sp = sprinter_json_create (ctx, stdout);
 +sp-begin_map (sp);
  
  /* The headers of the reply message we've created */
 -printf ({\reply-headers\: );
 +sp-map_key (sp, reply-headers);
  format_headers_json (sp, reply, TRUE);
  g_object_unref (G_OBJECT (reply));
  reply = NULL;
  
  /* Start the original */
 -printf (, \original\: );
 -
 +sp-map_key (sp, original);
  format_part_json (ctx, sp, node, TRUE, TRUE);
  
  /* End */
 -printf (}\n);
 +sp-end (sp);
  notmuch_message_destroy (message);
  
  return 0;
 -- 
 1.7.10

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


Re: [PATCH] cli: Remove now-unused json.c

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Austin Clements amdra...@mit.edu wrote:

 The string buffer quoting functions in json.c have been superseded by
 the new sprinter interface and are no longer used.  Remove them.
 ---

+1

Tomi


  Makefile.local |1 -
  json.c |  109 
 
  2 files changed, 110 deletions(-)
  delete mode 100644 json.c

 diff --git a/Makefile.local b/Makefile.local
 index b3b960c..de984ab 100644
 --- a/Makefile.local
 +++ b/Makefile.local
 @@ -294,7 +294,6 @@ notmuch_client_srcs = \
   query-string.c  \
   mime-node.c \
   crypto.c\
 - json.c
  
  notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
  
 diff --git a/json.c b/json.c
 deleted file mode 100644
 index 817fc83..000
 --- a/json.c
 +++ /dev/null
 @@ -1,109 +0,0 @@
 -/* notmuch - Not much of an email program, (just index and search)
 - *
 - * Copyright © 2009 Dave Gamble
 - * Copyright © 2009 Scott Robinson
 - *
 - * This program is free software: you can redistribute it and/or modify
 - * it under the terms of the GNU General Public License as published by
 - * the Free Software Foundation, either version 3 of the License, or
 - * (at your option) any later version.
 - *
 - * This program is distributed in the hope that it will be useful,
 - * but WITHOUT ANY WARRANTY; without even the implied warranty of
 - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 - * GNU General Public License for more details.
 - *
 - * You should have received a copy of the GNU General Public License
 - * along with this program.  If not, see http://www.gnu.org/licenses/ .
 - *
 - * Authors: Dave Gamble
 - *  Scott Robinson sc...@quadhome.com
 - *
 - */
 -
 -#include notmuch-client.h
 -
 -/* This function was derived from the print_string_ptr function of
 - * cJSON (http://cjson.sourceforge.net/) and is used by permission of
 - * the following license:
 - *
 - * Copyright (c) 2009 Dave Gamble
 - *
 - * Permission is hereby granted, free of charge, to any person obtaining a 
 copy
 - * of this software and associated documentation files (the Software), to 
 deal
 - * in the Software without restriction, including without limitation the 
 rights
 - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 - * copies of the Software, and to permit persons to whom the Software is
 - * furnished to do so, subject to the following conditions:
 - *
 - * The above copyright notice and this permission notice shall be included in
 - * all copies or substantial portions of the Software.
 - *
 - * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
 THE
 - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
 FROM,
 - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 - * THE SOFTWARE.
 - */
 -
 -char *
 -json_quote_chararray(const void *ctx, const char *str, const size_t len)
 -{
 -const char *ptr;
 -char *ptr2;
 -char *out;
 -size_t loop;
 -size_t required;
 -
 -for (loop = 0, required = 0, ptr = str;
 -  loop  len;
 -  loop++, required++, ptr++) {
 - if ((unsigned char)(*ptr)  32 || *ptr == '\' || *ptr == '\\')
 - required++;
 -}
 -
 -/*
 - * + 3 for:
 - * - leading quotation mark,
 - * - trailing quotation mark,
 - * - trailing NULL.
 - */
 -out = talloc_array (ctx, char, required + 3);
 -
 -ptr = str;
 -ptr2 = out;
 -
 -*ptr2++ = '\';
 -for (loop = 0; loop  len; loop++) {
 - if ((unsigned char)(*ptr)  31  *ptr != '\'  *ptr != '\\') {
 - *ptr2++ = *ptr++;
 - } else {
 - *ptr2++ = '\\';
 - switch (*ptr++) {
 - case '\':  *ptr2++ = '\'; break;
 - case '\\':  *ptr2++ = '\\'; break;
 - case '\b':  *ptr2++ = 'b';  break;
 - case '\f':  *ptr2++ = 'f';  break;
 - case '\n':  *ptr2++ = 'n';  break;
 - case '\r':  *ptr2++ = 'r';  break;
 - case '\t':  *ptr2++ = 't';  break;
 - default: ptr2--;break;
 - }
 - }
 -}
 -*ptr2++ = '\';
 -*ptr2++ = '\0';
 -
 -return out;
 -}
 -
 -char *
 -json_quote_str(const void *ctx, const char *str)
 -{
 -if (str == NULL)
 - str = ;
 -
 -return (json_quote_chararray (ctx, str, strlen (str)));
 -}
 -- 
 1.7.10

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

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

2012-08-07 Thread Peter Wang
On Mon, 6 Aug 2012 12:47:10 -0400, Austin Clements amdra...@mit.edu wrote:
 What's the overall goal of adding this?  Are you planning to add size
 information to one of the frontends?

Yes, to my frontend.

  diff --git a/devel/schemata b/devel/schemata
  index 9cb25f5..3df2764 100644
  --- a/devel/schemata
  +++ b/devel/schemata
  @@ -69,7 +69,10 @@ part = {
   # A leaf part's body content is optional, but may be included if
   # it can be correctly encoded as a string.  Consumers should use
   # this in preference to fetching the part content separately.
  -content?:   string
  +content?:   string,
  +# If a leaf part's body content is not included, the content-length
  +# may be included instead.
 
 You mentioned elsewhere that the content-length returned is an
 estimate.  If that's the case, this comment should say as much.  Is it
 actually the case, though?  g_mime_part_get_content_object is
 remarkably poorly documented for such an important function, but based
 on format_part_raw, it seems like the content-length your code returns
 will be exactly the number of bytes returned by the raw format for a
 leaf part.

It's the exact length of the _encoded_ content.  If the transfer
encoding is base64, multiplying by 3/4 will get a close estimate of the
decoded content length.  I assume quoted-printable encoding would only
be used if the content is mostly ASCII, so the encoded length can serve
as the estimated decoded length then.

  diff --git a/notmuch-show.c b/notmuch-show.c
  index 3556293..5c54257 100644
  --- a/notmuch-show.c
  +++ b/notmuch-show.c
  @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t *sp, 
  mime_node_t *node,
  sp-map_key (sp, content);
  sp-string_len (sp, (char *) part_content-data, part_content-len);
  g_object_unref (stream_memory);
  +   } else {
  +   GMimeDataWrapper *wrapper = g_mime_part_get_content_object 
  (GMIME_PART (node-part));
  +   GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
  +   ssize_t length = g_mime_stream_length (stream);
  +   if (length = 0) {
  +   sp-map_key (sp, content-length);
  +   sp-integer (sp, length);
  +   }
 
 Do wrapper or stream need to be g_object_unref'd?

No.

 Any idea what the performance overhead of this is?  I'm just curious.
 It might be approximately nothing, since GMime's parser is eager.

The start and end bounds of the stream are already known so there's
approximately nothing for g_mime_stream_length to do.  The other
functions simply return field values.

I'll drop the changes for text output.

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


Re: [PATCH 1/4] show: indicate length of omitted body content (json)

2012-08-07 Thread Austin Clements

Quoting Peter Wang noval...@gmail.com:

On Mon, 6 Aug 2012 12:47:10 -0400, Austin Clements amdra...@mit.edu wrote:

What's the overall goal of adding this?  Are you planning to add size
information to one of the frontends?


Yes, to my frontend.


 diff --git a/devel/schemata b/devel/schemata

 index 9cb25f5..3df2764 100644
 --- a/devel/schemata
 +++ b/devel/schemata
 @@ -69,7 +69,10 @@ part = {
  # A leaf part's body content is optional, but may be included if
  # it can be correctly encoded as a string.  Consumers should use
  # this in preference to fetching the part content separately.
 -content?:   string
 +content?:   string,
 +# If a leaf part's body content is not included, the content-length
 +# may be included instead.

You mentioned elsewhere that the content-length returned is an
estimate.  If that's the case, this comment should say as much.  Is it
actually the case, though?  g_mime_part_get_content_object is
remarkably poorly documented for such an important function, but based
on format_part_raw, it seems like the content-length your code returns
will be exactly the number of bytes returned by the raw format for a
leaf part.


It's the exact length of the _encoded_ content.  If the transfer
encoding is base64, multiplying by 3/4 will get a close estimate of the
decoded content length.  I assume quoted-printable encoding would only
be used if the content is mostly ASCII, so the encoded length can serve
as the estimated decoded length then.


Ah, I see.  format_part_raw misled me; apparently the
g_mime_data_wrapper_write_to_stream is key there, since *that* decodes
the transfer encoding of the data wrapper's underlying, raw stream.

In that case, the comment could either mention that this is the length
of the transfer encoded content or it could say it's an approximation
of the decoded length.  The advantage of only claiming the latter is
that it would leave open the possibility of, say, multiplying by .75
for base64 transfer encoding to get a better decoded estimate (your
assumption about quoted-printable sounds completely reasonable).
Alternatively, we could add the transfer encoding in the future and
let the caller do such approximations.


 diff --git a/notmuch-show.c b/notmuch-show.c
 index 3556293..5c54257 100644
 --- a/notmuch-show.c
 +++ b/notmuch-show.c
 @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t 
*sp, mime_node_t *node,

sp-map_key (sp, content);
sp-string_len (sp, (char *) part_content-data, part_content-len);
g_object_unref (stream_memory);
 +  } else {
 +	GMimeDataWrapper *wrapper = g_mime_part_get_content_object 
(GMIME_PART (node-part));

 +  GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
 +  ssize_t length = g_mime_stream_length (stream);
 +  if (length = 0) {
 +  sp-map_key (sp, content-length);
 +  sp-integer (sp, length);
 +  }

Do wrapper or stream need to be g_object_unref'd?


No.


Any idea what the performance overhead of this is?  I'm just curious.
It might be approximately nothing, since GMime's parser is eager.


The start and end bounds of the stream are already known so there's
approximately nothing for g_mime_stream_length to do.  The other
functions simply return field values.


Sounds good.


I'll drop the changes for text output.

Peter


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


Re: [PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Mark Walters
On Mon, 06 Aug 2012, Michal Nazarewicz m...@google.com wrote:
 From: Michal Nazarewicz min...@mina86.com

 Setting `notmuch-show-auto-mark-read' to nil stops notmuch-show from marking
 the message as read (by removing the unread tag).  Inteded for people who
 like to mark messages read explicitly.
 ---
  emacs/notmuch-show.el |   16 +---
  1 files changed, 13 insertions(+), 3 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index d318430..85a17b1 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -183,6 +183,14 @@ provided with an MLA argument nor `completing-read' 
 input.
notmuch-show-stash-mlarchive-link-alist))
:group 'notmuch-show)
  
 +(defcustom notmuch-show-auto-mark-read t
 +  Whether to automatically mark message as read when it is shown.  If
 +nil, message needs to be marked as read manually for instance by
 +removing the unread tag.
 +  :type 'boolean
 +  :group 'notmuch-show)
 +
 +
  (defmacro with-current-notmuch-show-message (rest body)
Evaluate body with current buffer set to the text of current message
`(save-excursion
 @@ -1374,9 +1382,11 @@ current thread.
Are the headers of the current message visible?
(notmuch-show-get-prop :headers-visible))
  
 -(defun notmuch-show-mark-read ()
 -  Mark the current message as read.
 -  (notmuch-show-tag-message -unread))
 +(defun notmuch-show-mark-read (optional force)
 +  Mark the current message as read if FORCE or
 +`notmuch-show-auto-mark-read' is non-nil.
 +  (when (or force notmuch-show-auto-mark-read)
 +(notmuch-show-tag-message -unread)))


As an alternative approach would allowing a list of tags (or even tag
changes) to apply when a message is read do what you want and be more
flexible? I am thinking of something roughly analogous to
notmuch-message-replied-tags. I can imagine some people would like to
remove the inbox tag automatically for example. (And since we sync with
maildir flags the exact tag name does matter).


I agree with Austin that the current unread marking is a little
weird/unpredictable (eg notmuch-show-next-message marks a message read
even if it is closed) but I don't think fixing that helps your use.

Best wishes 

Mark


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


‘class Xapian::Database’ has no member named ‘close’

2012-08-07 Thread Michal Nazarewicz
Hi guys,

when I'm trying to build notmuch on Ubuntu Lucid, I'm getting the
following error:

lib/database.cc: In function ‘void notmuch_database_close(notmuch_database_t*)’:
lib/database.cc:767: error: ‘class Xapian::Database’ has no member named ‘close’

I'm solving that by:

diff --git a/lib/database.cc b/lib/database.cc
index 761dc1a..fd78135 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -764,7 +764,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
  * close it.  Thus, we explicitly close it here. */
 if (notmuch-xapian_db != NULL) {
try {
-   notmuch-xapian_db-close();
+   /* notmuch-xapian_db-close(); */
} catch (const Xapian::Error error) {
/* do nothing */
}

which does not seem to break anything.  The Xapian packages installed on
my machine:

$ dpkg -l |grep xapian
ii  apt-xapian-index  0.25ubuntu2  maintenance tools for a Xapian index of 
Debi
ii  libxapian-dev 1.0.18-1 Development files for Xapian search 
engine l
ii  libxapian15   1.0.18-1 Search engine library
ii  python-xapian 1.0.17-1ubuntu1  Xapian search engine interface for Python

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

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


Re: [PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Michal Nazarewicz
 On Mon, 06 Aug 2012, Michal Nazarewicz m...@google.com wrote:
 @@ -1374,9 +1382,11 @@ current thread.
Are the headers of the current message visible?
(notmuch-show-get-prop :headers-visible))
  
 -(defun notmuch-show-mark-read ()
 -  Mark the current message as read.
 -  (notmuch-show-tag-message -unread))
 +(defun notmuch-show-mark-read (optional force)
 +  Mark the current message as read if FORCE or
 +`notmuch-show-auto-mark-read' is non-nil.
 +  (when (or force notmuch-show-auto-mark-read)
 +(notmuch-show-tag-message -unread)))

Mark Walters markwalters1...@gmail.com writes:
 As an alternative approach would allowing a list of tags (or even tag
 changes) to apply when a message is read do what you want and be more
 flexible?

Something like the following (not tested)?

From: Michal Nazarewicz min...@mina86.com
Date: Mon, 6 Aug 2012 15:31:20 +0200
Subject: [PATCH] notmuch-show: add notmuch-show-mark-read-tags option

The `notmuch-show-mark-read-tags' lists tags that are to be applied when
message is read.  By default, the only value is -unread which will remove
the unread tag.  Among other uses, this variable can be used to stop
notmuch-show from modifying tags when message is shown (by setting the
variable to an empty list).
---
 emacs/notmuch-show.el |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index dcfc190..92a4beb 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -183,6 +183,13 @@ provided with an MLA argument nor `completing-read' input.
 notmuch-show-stash-mlarchive-link-alist))
   :group 'notmuch-show)
 
+(defcustom notmuch-show-mark-read-tags '(-unread)
+  List of tags to apply when message is read, ie. shown in notmuch-show
+buffer.
+  :type '(repeat string)
+  :group 'notmuch-show)
+
+
 (defmacro with-current-notmuch-show-message (rest body)
   Evaluate body with current buffer set to the text of current message
   `(save-excursion
@@ -1383,8 +1390,9 @@ current thread.
   (notmuch-show-get-prop :headers-visible))
 
 (defun notmuch-show-mark-read ()
-  Mark the current message as read.
-  (notmuch-show-tag-message -unread))
+  Apply `notmuch-show-mark-read-tags' to the message.
+  (when notmuch-show-mark-read-tags
+(apply notmuch-show-tag-message notmuch-show-mark-read-tags)))
 
 ;; Functions for getting attributes of several messages in the current
 ;; thread.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

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


[PATCH] emacs: notmuch search bugfix

2012-08-07 Thread Mark Walters

The recent change to use json for notmuch-search.el introduced a bug
in the code for keeping position on refresh. The problem is a
comparison between (plist-get result :thread) and a thread-id returned
by notmuch-search-find-thread-id: the latter is prefixed with
thread:

We fix this by adding an option to notmuch-search-find-thread-id to
return the bare thread-id. It appears that notmuch-search-refresh-view
is the only caller of notmuch-search that supplies a thread-id so this
change should be safe (but could theoretically break users .emacs
functions).
---

This is a different version of the bugfix suggested in the parent to
this thread. I think it is nicer but there is a slight risk of breaking
users .emacs lisp functions.

In general I think the thread: prefix should be present where the term
is really a search term, but absent when it is must be a thread-id,
and this fits with that idea.

With 0.14 freeze pretty close it might be best to apply this to master
and the simpler no breakage option (in id:87boinpqfg@qmul.ac.uk)
to the branch.

Best wishes 

Mark

 emacs/notmuch.el |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index d2d82a9..0fff58a 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -475,10 +475,12 @@ BEG.
(push (plist-get (notmuch-search-get-result pos) property) output)))
 output))
 
-(defun notmuch-search-find-thread-id ()
-  Return the thread for the current thread
+(defun notmuch-search-find-thread-id (optional bare)
+  Return the thread for the current thread
+
+If BARE is set then do not prefix with \thread:\
   (let ((thread (plist-get (notmuch-search-get-result) :thread)))
-(when thread (concat thread: thread
+(when thread (concat (when (not bare) thread:) thread
 
 (defun notmuch-search-find-thread-id-region (beg end)
   Return a list of threads for the current region
@@ -993,7 +995,7 @@ same relative position within the new buffer.
   (interactive)
   (let ((target-line (line-number-at-pos))
(oldest-first notmuch-search-oldest-first)
-   (target-thread (notmuch-search-find-thread-id))
+   (target-thread (notmuch-search-find-thread-id t))
(query notmuch-search-query-string)
(continuation notmuch-search-continuation))
 (notmuch-kill-this-buffer)
-- 
1.7.9.1

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


Re: [PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Mark Walters
On Tue, 07 Aug 2012, Michal Nazarewicz m...@google.com wrote:
 On Mon, 06 Aug 2012, Michal Nazarewicz m...@google.com wrote:
 @@ -1374,9 +1382,11 @@ current thread.
Are the headers of the current message visible?
(notmuch-show-get-prop :headers-visible))
  
 -(defun notmuch-show-mark-read ()
 -  Mark the current message as read.
 -  (notmuch-show-tag-message -unread))
 +(defun notmuch-show-mark-read (optional force)
 +  Mark the current message as read if FORCE or
 +`notmuch-show-auto-mark-read' is non-nil.
 +  (when (or force notmuch-show-auto-mark-read)
 +(notmuch-show-tag-message -unread)))

 Mark Walters markwalters1...@gmail.com writes:
 As an alternative approach would allowing a list of tags (or even tag
 changes) to apply when a message is read do what you want and be more
 flexible?

 Something like the following (not tested)?

Yes this was what I had in mind. I like this (both the symmetry with
reply tags and the added flexibility) but I will let others
comment. (There is one small bug in your draft: see below)

Best wishes

Mark

 From: Michal Nazarewicz min...@mina86.com
 Date: Mon, 6 Aug 2012 15:31:20 +0200
 Subject: [PATCH] notmuch-show: add notmuch-show-mark-read-tags option

 The `notmuch-show-mark-read-tags' lists tags that are to be applied when
 message is read.  By default, the only value is -unread which will remove
 the unread tag.  Among other uses, this variable can be used to stop
 notmuch-show from modifying tags when message is shown (by setting the
 variable to an empty list).
 ---
  emacs/notmuch-show.el |   12 ++--
  1 files changed, 10 insertions(+), 2 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index dcfc190..92a4beb 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -183,6 +183,13 @@ provided with an MLA argument nor `completing-read' 
 input.
notmuch-show-stash-mlarchive-link-alist))
:group 'notmuch-show)
  
 +(defcustom notmuch-show-mark-read-tags '(-unread)
 +  List of tags to apply when message is read, ie. shown in notmuch-show
 +buffer.
 +  :type '(repeat string)
 +  :group 'notmuch-show)
 +
 +
  (defmacro with-current-notmuch-show-message (rest body)
Evaluate body with current buffer set to the text of current message
`(save-excursion
 @@ -1383,8 +1390,9 @@ current thread.
(notmuch-show-get-prop :headers-visible))
  
  (defun notmuch-show-mark-read ()
 -  Mark the current message as read.
 -  (notmuch-show-tag-message -unread))
 +  Apply `notmuch-show-mark-read-tags' to the message.
 +  (when notmuch-show-mark-read-tags
 +(apply notmuch-show-tag-message notmuch-show-mark-read-tags)))

 This needs to be (apply 'notmuch-show-tag-message ...)
  
  ;; Functions for getting attributes of several messages in the current
  ;; thread.

 -- 
 Best regards, _ _
 .o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
 ..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
 ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Mark Walters markwalters1...@gmail.com wrote:

 On Mon, 06 Aug 2012, Michal Nazarewicz m...@google.com wrote:
 From: Michal Nazarewicz min...@mina86.com

 Setting `notmuch-show-auto-mark-read' to nil stops notmuch-show from marking
 the message as read (by removing the unread tag).  Inteded for people who
 like to mark messages read explicitly.
 ---
  emacs/notmuch-show.el |   16 +---
  1 files changed, 13 insertions(+), 3 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index d318430..85a17b1 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -183,6 +183,14 @@ provided with an MLA argument nor `completing-read' 
 input.
   notmuch-show-stash-mlarchive-link-alist))
:group 'notmuch-show)
  
 +(defcustom notmuch-show-auto-mark-read t
 +  Whether to automatically mark message as read when it is shown.  If
 +nil, message needs to be marked as read manually for instance by
 +removing the unread tag.
 +  :type 'boolean
 +  :group 'notmuch-show)
 +
 +
  (defmacro with-current-notmuch-show-message (rest body)
Evaluate body with current buffer set to the text of current message
`(save-excursion
 @@ -1374,9 +1382,11 @@ current thread.
Are the headers of the current message visible?
(notmuch-show-get-prop :headers-visible))
  
 -(defun notmuch-show-mark-read ()
 -  Mark the current message as read.
 -  (notmuch-show-tag-message -unread))
 +(defun notmuch-show-mark-read (optional force)
 +  Mark the current message as read if FORCE or
 +`notmuch-show-auto-mark-read' is non-nil.
 +  (when (or force notmuch-show-auto-mark-read)
 +(notmuch-show-tag-message -unread)))


 As an alternative approach would allowing a list of tags (or even tag
 changes) to apply when a message is read do what you want and be more
 flexible?

That's a great idea -- better than mine (just a variable which informs
which tag is to be removed) .. something like:

(defcustom notmuch-message-read-tag-changes '(-unread) docstring ...)

Then one can have e.g.

'() -- for no tag changes
'(-inbox -unread +read) 
'(-postilaatikko -lukematon +luettu) -- the above in finnish.


Tomi


 I am thinking of something roughly analogous to
 notmuch-message-replied-tags. I can imagine some people would like to
 remove the inbox tag automatically for example. (And since we sync with
 maildir flags the exact tag name does matter).

 I agree with Austin that the current unread marking is a little
 weird/unpredictable (eg notmuch-show-next-message marks a message read
 even if it is closed) but I don't think fixing that helps your use.



 Best wishes 

 Mark


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


Re: [PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Michal Nazarewicz
 From: Michal Nazarewicz min...@mina86.com
 @@ -1383,8 +1390,9 @@ current thread.
(notmuch-show-get-prop :headers-visible))
  
  (defun notmuch-show-mark-read ()
 -  Mark the current message as read.
 -  (notmuch-show-tag-message -unread))
 +  Apply `notmuch-show-mark-read-tags' to the message.
 +  (when notmuch-show-mark-read-tags
 +(apply notmuch-show-tag-message notmuch-show-mark-read-tags)))

Mark Walters markwalters1...@gmail.com writes:
  This needs to be (apply 'notmuch-show-tag-message ...)

Yes, it would appear you are right. :)

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

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


Re: [PATCH] reply: Convert JSON format to use sprinter

2012-08-07 Thread Mark Walters


On Tue, 07 Aug 2012, Austin Clements amdra...@mit.edu wrote:
 Almost all of reply was already being formatted using the sprinter.
 This patch converts the top-level dictionary to use the sprinter
 interface.
 ---

 One last sprinter piece that had slipped through the cracks.

Looks good to me +1

Mark


  notmuch-reply.c |8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/notmuch-reply.c b/notmuch-reply.c
 index fa6665f..e60a264 100644
 --- a/notmuch-reply.c
 +++ b/notmuch-reply.c
 @@ -614,20 +614,20 @@ notmuch_reply_format_json(void *ctx,
   return 1;
  
  sp = sprinter_json_create (ctx, stdout);
 +sp-begin_map (sp);
  
  /* The headers of the reply message we've created */
 -printf ({\reply-headers\: );
 +sp-map_key (sp, reply-headers);
  format_headers_json (sp, reply, TRUE);
  g_object_unref (G_OBJECT (reply));
  reply = NULL;
  
  /* Start the original */
 -printf (, \original\: );
 -
 +sp-map_key (sp, original);
  format_part_json (ctx, sp, node, TRUE, TRUE);
  
  /* End */
 -printf (}\n);
 +sp-end (sp);
  notmuch_message_destroy (message);
  
  return 0;
 -- 
 1.7.10

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


Re: [PATCH] cli: Remove now-unused json.c

2012-08-07 Thread Mark Walters

+1

Mark

On Tue, 07 Aug 2012, Austin Clements amdra...@mit.edu wrote:
 The string buffer quoting functions in json.c have been superseded by
 the new sprinter interface and are no longer used.  Remove them.
 ---
  Makefile.local |1 -
  json.c |  109 
 
  2 files changed, 110 deletions(-)
  delete mode 100644 json.c

 diff --git a/Makefile.local b/Makefile.local
 index b3b960c..de984ab 100644
 --- a/Makefile.local
 +++ b/Makefile.local
 @@ -294,7 +294,6 @@ notmuch_client_srcs = \
   query-string.c  \
   mime-node.c \
   crypto.c\
 - json.c
  
  notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
  
 diff --git a/json.c b/json.c
 deleted file mode 100644
 index 817fc83..000
 --- a/json.c
 +++ /dev/null
 @@ -1,109 +0,0 @@
 -/* notmuch - Not much of an email program, (just index and search)
 - *
 - * Copyright © 2009 Dave Gamble
 - * Copyright © 2009 Scott Robinson
 - *
 - * This program is free software: you can redistribute it and/or modify
 - * it under the terms of the GNU General Public License as published by
 - * the Free Software Foundation, either version 3 of the License, or
 - * (at your option) any later version.
 - *
 - * This program is distributed in the hope that it will be useful,
 - * but WITHOUT ANY WARRANTY; without even the implied warranty of
 - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 - * GNU General Public License for more details.
 - *
 - * You should have received a copy of the GNU General Public License
 - * along with this program.  If not, see http://www.gnu.org/licenses/ .
 - *
 - * Authors: Dave Gamble
 - *  Scott Robinson sc...@quadhome.com
 - *
 - */
 -
 -#include notmuch-client.h
 -
 -/* This function was derived from the print_string_ptr function of
 - * cJSON (http://cjson.sourceforge.net/) and is used by permission of
 - * the following license:
 - *
 - * Copyright (c) 2009 Dave Gamble
 - *
 - * Permission is hereby granted, free of charge, to any person obtaining a 
 copy
 - * of this software and associated documentation files (the Software), to 
 deal
 - * in the Software without restriction, including without limitation the 
 rights
 - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 - * copies of the Software, and to permit persons to whom the Software is
 - * furnished to do so, subject to the following conditions:
 - *
 - * The above copyright notice and this permission notice shall be included in
 - * all copies or substantial portions of the Software.
 - *
 - * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
 THE
 - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
 FROM,
 - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 - * THE SOFTWARE.
 - */
 -
 -char *
 -json_quote_chararray(const void *ctx, const char *str, const size_t len)
 -{
 -const char *ptr;
 -char *ptr2;
 -char *out;
 -size_t loop;
 -size_t required;
 -
 -for (loop = 0, required = 0, ptr = str;
 -  loop  len;
 -  loop++, required++, ptr++) {
 - if ((unsigned char)(*ptr)  32 || *ptr == '\' || *ptr == '\\')
 - required++;
 -}
 -
 -/*
 - * + 3 for:
 - * - leading quotation mark,
 - * - trailing quotation mark,
 - * - trailing NULL.
 - */
 -out = talloc_array (ctx, char, required + 3);
 -
 -ptr = str;
 -ptr2 = out;
 -
 -*ptr2++ = '\';
 -for (loop = 0; loop  len; loop++) {
 - if ((unsigned char)(*ptr)  31  *ptr != '\'  *ptr != '\\') {
 - *ptr2++ = *ptr++;
 - } else {
 - *ptr2++ = '\\';
 - switch (*ptr++) {
 - case '\':  *ptr2++ = '\'; break;
 - case '\\':  *ptr2++ = '\\'; break;
 - case '\b':  *ptr2++ = 'b';  break;
 - case '\f':  *ptr2++ = 'f';  break;
 - case '\n':  *ptr2++ = 'n';  break;
 - case '\r':  *ptr2++ = 'r';  break;
 - case '\t':  *ptr2++ = 't';  break;
 - default: ptr2--;break;
 - }
 - }
 -}
 -*ptr2++ = '\';
 -*ptr2++ = '\0';
 -
 -return out;
 -}
 -
 -char *
 -json_quote_str(const void *ctx, const char *str)
 -{
 -if (str == NULL)
 - str = ;
 -
 -return (json_quote_chararray (ctx, str, strlen (str)));
 -}
 -- 
 1.7.10

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

Re: [PATCH] emacs: notmuch search bugfix

2012-08-07 Thread Austin Clements
Quoth Mark Walters on Aug 07 at  4:40 pm:
 
 The recent change to use json for notmuch-search.el introduced a bug
 in the code for keeping position on refresh. The problem is a
 comparison between (plist-get result :thread) and a thread-id returned
 by notmuch-search-find-thread-id: the latter is prefixed with
 thread:
 
 We fix this by adding an option to notmuch-search-find-thread-id to
 return the bare thread-id. It appears that notmuch-search-refresh-view
 is the only caller of notmuch-search that supplies a thread-id so this
 change should be safe (but could theoretically break users .emacs
 functions).
 ---
 
 This is a different version of the bugfix suggested in the parent to
 this thread. I think it is nicer but there is a slight risk of breaking
 users .emacs lisp functions.
 
 In general I think the thread: prefix should be present where the term
 is really a search term, but absent when it is must be a thread-id,
 and this fits with that idea.

Agreed.

 With 0.14 freeze pretty close it might be best to apply this to master
 and the simpler no breakage option (in id:87boinpqfg@qmul.ac.uk)
 to the branch.
 
 Best wishes 
 
 Mark
 
  emacs/notmuch.el |   10 ++
  1 files changed, 6 insertions(+), 4 deletions(-)
 
 diff --git a/emacs/notmuch.el b/emacs/notmuch.el
 index d2d82a9..0fff58a 100644
 --- a/emacs/notmuch.el
 +++ b/emacs/notmuch.el
 @@ -475,10 +475,12 @@ BEG.
   (push (plist-get (notmuch-search-get-result pos) property) output)))
  output))
  
 -(defun notmuch-search-find-thread-id ()
 -  Return the thread for the current thread
 +(defun notmuch-search-find-thread-id (optional bare)
 +  Return the thread for the current thread
 +
 +If BARE is set then do not prefix with \thread:\
(let ((thread (plist-get (notmuch-search-get-result) :thread)))
 -(when thread (concat thread: thread
 +(when thread (concat (when (not bare) thread:) thread

(unless bare thread:)?

  
  (defun notmuch-search-find-thread-id-region (beg end)
Return a list of threads for the current region
 @@ -993,7 +995,7 @@ same relative position within the new buffer.
(interactive)
(let ((target-line (line-number-at-pos))
   (oldest-first notmuch-search-oldest-first)
 - (target-thread (notmuch-search-find-thread-id))
 + (target-thread (notmuch-search-find-thread-id t))

Perhaps pass 'bare instead of t to make this self-documenting?

   (query notmuch-search-query-string)
   (continuation notmuch-search-continuation))
  (notmuch-kill-this-buffer)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] emacs: notmuch search bugfix

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Mark Walters wrote:

 The recent change to use json for notmuch-search.el introduced a bug
 in the code for keeping position on refresh. The problem is a
 comparison between (plist-get result :thread) and a thread-id returned
 by notmuch-search-find-thread-id: the latter is prefixed with
 thread:

 We fix this by adding an option to notmuch-search-find-thread-id to
 return the bare thread-id. It appears that notmuch-search-refresh-view
 is the only caller of notmuch-search that supplies a thread-id so this
 change should be safe (but could theoretically break users .emacs
 functions).
 ---

 This is a different version of the bugfix suggested in the parent to
 this thread. I think it is nicer but there is a slight risk of breaking
 users .emacs lisp functions.

 In general I think the thread: prefix should be present where the term
 is really a search term, but absent when it is must be a thread-id,
 and this fits with that idea.

 With 0.14 freeze pretty close it might be best to apply this to master
 and the simpler no breakage option (in id:87boinpqfg@qmul.ac.uk)
 to the branch.

LGTM.

IMHO you're too shy with this patch to be included in 0.14 :D
-- at least I cannot imagine a case this would fail...

My change would have been:
  (when thread (if bare thread (concat thread: thread)))

or even (which changes irrelevant part):
  (if thread (if bare thread (concat thread: thread)))

Anyway, this is good as it is in the grand scheme of things.

Tomi


 Best wishes 

 Mark

  emacs/notmuch.el |   10 ++
  1 files changed, 6 insertions(+), 4 deletions(-)

 diff --git a/emacs/notmuch.el b/emacs/notmuch.el
 index d2d82a9..0fff58a 100644
 --- a/emacs/notmuch.el
 +++ b/emacs/notmuch.el
 @@ -475,10 +475,12 @@ BEG.
   (push (plist-get (notmuch-search-get-result pos) property) output)))
  output))
  
 -(defun notmuch-search-find-thread-id ()
 -  Return the thread for the current thread
 +(defun notmuch-search-find-thread-id (optional bare)
 +  Return the thread for the current thread
 +
 +If BARE is set then do not prefix with \thread:\
(let ((thread (plist-get (notmuch-search-get-result) :thread)))
 -(when thread (concat thread: thread
 +(when thread (concat (when (not bare) thread:) thread
  
  (defun notmuch-search-find-thread-id-region (beg end)
Return a list of threads for the current region
 @@ -993,7 +995,7 @@ same relative position within the new buffer.
(interactive)
(let ((target-line (line-number-at-pos))
   (oldest-first notmuch-search-oldest-first)
 - (target-thread (notmuch-search-find-thread-id))
 + (target-thread (notmuch-search-find-thread-id t))
   (query notmuch-search-query-string)
   (continuation notmuch-search-continuation))
  (notmuch-kill-this-buffer)
 -- 
 1.7.9.1

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


[PATCH v2] emacs: notmuch search bugfix

2012-08-07 Thread Mark Walters

The recent change to use json for notmuch-search.el introduced a bug
in the code for keeping position on refresh. The problem is a
comparison between (plist-get result :thread) and a thread-id returned
by notmuch-search-find-thread-id: the latter is prefixed with
thread:

We fix this by adding an option to notmuch-search-find-thread-id to
return the bare thread-id. It appears that notmuch-search-refresh-view
is the only caller of notmuch-search that supplies a thread-id so this
change should be safe (but could theoretically break users .emacs
functions).
---

I think this implements all of Austin's suggestions, and it adds a short
remark to NEWS (since in unlikely cases it could break users' functions)
and updates the docstring for notmuch-search. 

Best wishes

Mark

 NEWS |3 +++
 emacs/notmuch.el |   12 +++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 761b2c1..9916c91 100644
--- a/NEWS
+++ b/NEWS
@@ -55,6 +55,9 @@ user-specified formatting
   you've customized this variable, you may have to change your date
   format from `%s ` to `%12s `.
 
+The thread-id for the `target-thread` argument for `notmuch-search` should
+now be supplied without the thread: prefix.
+
 Notmuch 0.13.2 (2012-06-02)
 ===
 
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index d2d82a9..7b61e9b 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -475,10 +475,12 @@ BEG.
(push (plist-get (notmuch-search-get-result pos) property) output)))
 output))
 
-(defun notmuch-search-find-thread-id ()
-  Return the thread for the current thread
+(defun notmuch-search-find-thread-id (optional bare)
+  Return the thread for the current thread
+
+If BARE is set then do not prefix with \thread:\
   (let ((thread (plist-get (notmuch-search-get-result) :thread)))
-(when thread (concat thread: thread
+(when thread (concat (unless bare thread:) thread
 
 (defun notmuch-search-find-thread-id-region (beg end)
   Return a list of threads for the current region
@@ -936,7 +938,7 @@ If `query' is nil, it is read interactively from the 
minibuffer.
 Other optional parameters are used as follows:
 
   oldest-first: A Boolean controlling the sort order of returned threads
-  target-thread: A thread ID (with the thread: prefix) that will be made
+  target-thread: A thread ID (without the thread: prefix) that will be made
  current if it appears in the search results.
   target-line: The line number to move to if the target thread does not
appear in the search results.
@@ -993,7 +995,7 @@ same relative position within the new buffer.
   (interactive)
   (let ((target-line (line-number-at-pos))
(oldest-first notmuch-search-oldest-first)
-   (target-thread (notmuch-search-find-thread-id))
+   (target-thread (notmuch-search-find-thread-id 'bare))
(query notmuch-search-query-string)
(continuation notmuch-search-continuation))
 (notmuch-kill-this-buffer)
-- 
1.7.9.1

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


Re: [PATCH] notmuch-show: add notmuch-show-auto-mark-read option

2012-08-07 Thread Tomi Ollila
On Tue, Aug 07 2012, Mark Walters markwalters1...@gmail.com wrote:

 On Tue, 07 Aug 2012, Michal Nazarewicz m...@google.com wrote:

[ ... ]

 Mark Walters markwalters1...@gmail.com writes:
 As an alternative approach would allowing a list of tags (or even tag
 changes) to apply when a message is read do what you want and be more
 flexible?

 Something like the following (not tested)?

 Yes this was what I had in mind. I like this (both the symmetry with
 reply tags and the added flexibility) but I will let others
 comment. (There is one small bug in your draft: see below)

I like the idea -- and the draft!


 Best wishes

 Mark

Tomi


 From: Michal Nazarewicz min...@mina86.com
 Date: Mon, 6 Aug 2012 15:31:20 +0200
 Subject: [PATCH] notmuch-show: add notmuch-show-mark-read-tags option

 The `notmuch-show-mark-read-tags' lists tags that are to be applied when
 message is read.  By default, the only value is -unread which will remove
 the unread tag.  Among other uses, this variable can be used to stop
 notmuch-show from modifying tags when message is shown (by setting the
 variable to an empty list).
 ---
  emacs/notmuch-show.el |   12 ++--
  1 files changed, 10 insertions(+), 2 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index dcfc190..92a4beb 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -183,6 +183,13 @@ provided with an MLA argument nor `completing-read' 
 input.
   notmuch-show-stash-mlarchive-link-alist))
:group 'notmuch-show)
  
 +(defcustom notmuch-show-mark-read-tags '(-unread)
 +  List of tags to apply when message is read, ie. shown in notmuch-show
 +buffer.
 +  :type '(repeat string)
 +  :group 'notmuch-show)
 +
 +
  (defmacro with-current-notmuch-show-message (rest body)
Evaluate body with current buffer set to the text of current message
`(save-excursion
 @@ -1383,8 +1390,9 @@ current thread.
(notmuch-show-get-prop :headers-visible))
  
  (defun notmuch-show-mark-read ()
 -  Mark the current message as read.
 -  (notmuch-show-tag-message -unread))
 +  Apply `notmuch-show-mark-read-tags' to the message.
 +  (when notmuch-show-mark-read-tags
 +(apply notmuch-show-tag-message notmuch-show-mark-read-tags)))

  This needs to be (apply 'notmuch-show-tag-message ...)
  
  ;; Functions for getting attributes of several messages in the current
  ;; thread.

 -- 
 Best regards, _ _
 .o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
 ..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
 ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--
 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: Add test for messages with missing headers

2012-08-07 Thread Austin Clements
Currently the JSON tests for search and show are broken because
notmuch attempts to dereference a NULL pointer.
---
Things to bikeshed:

* Should we include From and Subject in the headers object when there
  are no from or subject headers?  Currently the schema says that
  everything but those two and Date is optional (indeed, To is
  missing from the second message) but that was just post facto
  standardization.

* How should we format expected JSON in the test suite, now that we
  can format it however we want?  Here I've just pasted in the result
  of python -mjson.tool.  While that was very easy and the result is
  quite readable, it's the antithesis of compact and the keys have
  been alphabetized.

 test/missing-headers |  162 ++
 test/notmuch-test|1 +
 2 files changed, 163 insertions(+)
 create mode 100755 test/missing-headers

diff --git a/test/missing-headers b/test/missing-headers
new file mode 100755
index 000..744c04e
--- /dev/null
+++ b/test/missing-headers
@@ -0,0 +1,162 @@
+#!/usr/bin/env bash
+test_description='messages with missing headers'
+. ./test-lib.sh
+
+# Notmuch requires at least one of from, subject, or to or it will
+# ignore the file.  Generate two messages so that together they cover
+# all possible missing headers.  We also give one of the messages a
+# date to ensure stable result ordering.
+
+cat EOF  ${MAIL_DIR}/msg-2
+To: Notmuch Test Suite test_su...@notmuchmail.org
+Date: Fri, 05 Jan 2001 15:43:57 +
+
+Body
+EOF
+
+cat EOF  ${MAIL_DIR}/msg-1
+From: Notmuch Test Suite test_su...@notmuchmail.org
+
+Body
+EOF
+
+NOTMUCH_NEW
+
+test_begin_subtest Search: text
+output=$(notmuch search '*' | notmuch_search_sanitize)
+test_expect_equal $output \
+thread:XXX   2001-01-05 [1/1] (null);  (inbox unread)
+thread:XXX   1970-01-01 [1/1] Notmuch Test Suite;  (inbox unread)
+
+test_begin_subtest Search: json
+test_subtest_known_broken
+output=$(notmuch search --format=json '*' | notmuch_search_sanitize)
+test_expect_equal_json $output '
+[
+{
+authors: ,
+date_relative: 2001-01-05,
+matched: 1,
+subject: ,
+tags: [
+inbox,
+unread
+],
+thread: XXX,
+timestamp: 978709437,
+total: 1
+},
+{
+authors: Notmuch Test Suite,
+date_relative: 1970-01-01,
+matched: 1,
+subject: ,
+tags: [
+inbox,
+unread
+],
+thread: XXX,
+timestamp: 0,
+total: 1
+}
+]'
+
+test_begin_subtest Show: text
+output=$(notmuch show '*')
+test_expect_equal $output \
+message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 
match:1 excluded:0 filename:/tmp/nmtest/tmp.missing-headers/mail/msg-2
+header{
+ (2001-01-05) (inbox unread)
+Subject: (null)
+From: (null)
+To: Notmuch Test Suite test_su...@notmuchmail.org
+Date: Fri, 05 Jan 2001 15:43:57 +
+header}
+body{
+part{ ID: 1, Content-type: text/plain
+Body
+part}
+body}
+message}
+message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 
match:1 excluded:0 filename:/tmp/nmtest/tmp.missing-headers/mail/msg-1
+header{
+Notmuch Test Suite test_su...@notmuchmail.org (1970-01-01) (inbox unread)
+Subject: (null)
+From: Notmuch Test Suite test_su...@notmuchmail.org
+Date: Thu, 01 Jan 1970 00:00:00 +
+header}
+body{
+part{ ID: 1, Content-type: text/plain
+Body
+part}
+body}
+message}
+
+test_begin_subtest Show: json
+test_subtest_known_broken
+output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize)
+test_expect_equal_json $output '
+[
+[
+[
+{
+body: [
+{
+content: Body\n,
+content-type: text/plain,
+id: 1
+}
+],
+date_relative: 2001-01-05,
+excluded: false,
+filename: Y,
+headers: {
+Date: Fri, 05 Jan 2001 15:43:57 +,
+From: ,
+Subject: ,
+To: Notmuch Test Suite test_su...@notmuchmail.org
+},
+id: X,
+match: true,
+tags: [
+inbox,
+unread
+],
+timestamp: 978709437
+},
+[]
+]
+],
+[
+[
+{
+body: [
+{
+content: Body\n,
+content-type: text/plain,
+id: 1
+}
+],
+date_relative: 1970-01-01,
+excluded: false,
+filename: Y,
+headers: {
+Date: Thu, 01 Jan 1970 00:00:00 +,
+From: Notmuch 

Re: [PATCH] test: Add test for messages with missing headers

2012-08-07 Thread Mark Walters

Hi 

This generally looks good but there is a bug in the test from a
hardcoded /tmp/ path (see below). And as confirmation with the patch in
id:874noe1o0r@qmul.ac.uk the tests (modulo the test bug) pass.

On Tue, 07 Aug 2012, Austin Clements amdra...@mit.edu wrote:
 Currently the JSON tests for search and show are broken because
 notmuch attempts to dereference a NULL pointer.
 ---
 Things to bikeshed:

 * Should we include From and Subject in the headers object when there
   are no from or subject headers?  Currently the schema says that
   everything but those two and Date is optional (indeed, To is
   missing from the second message) but that was just post facto
   standardization.

I think Date and From are compulsory in an email but the others are not
(but I am unsure which Date and which From so that may be unhelpful).
If I am correct it might be sensible to always include those two.

 * How should we format expected JSON in the test suite, now that we
   can format it however we want?  Here I've just pasted in the result
   of python -mjson.tool.  While that was very easy and the result is
   quite readable, it's the antithesis of compact and the keys have
   been alphabetized.

I like this: making the tests readable is a big plus.

  test/missing-headers |  162 
 ++
  test/notmuch-test|1 +
  2 files changed, 163 insertions(+)
  create mode 100755 test/missing-headers

 diff --git a/test/missing-headers b/test/missing-headers
 new file mode 100755
 index 000..744c04e
 --- /dev/null
 +++ b/test/missing-headers
 @@ -0,0 +1,162 @@
 +#!/usr/bin/env bash
 +test_description='messages with missing headers'
 +. ./test-lib.sh
 +
 +# Notmuch requires at least one of from, subject, or to or it will
 +# ignore the file.  Generate two messages so that together they cover
 +# all possible missing headers.  We also give one of the messages a
 +# date to ensure stable result ordering.
 +
 +cat EOF  ${MAIL_DIR}/msg-2
 +To: Notmuch Test Suite test_su...@notmuchmail.org
 +Date: Fri, 05 Jan 2001 15:43:57 +
 +
 +Body
 +EOF
 +
 +cat EOF  ${MAIL_DIR}/msg-1
 +From: Notmuch Test Suite test_su...@notmuchmail.org
 +
 +Body
 +EOF
 +
 +NOTMUCH_NEW
 +
 +test_begin_subtest Search: text
 +output=$(notmuch search '*' | notmuch_search_sanitize)
 +test_expect_equal $output \
 +thread:XXX   2001-01-05 [1/1] (null);  (inbox unread)
 +thread:XXX   1970-01-01 [1/1] Notmuch Test Suite;  (inbox unread)
 +
 +test_begin_subtest Search: json
 +test_subtest_known_broken
 +output=$(notmuch search --format=json '*' | notmuch_search_sanitize)
 +test_expect_equal_json $output '
 +[
 +{
 +authors: ,
 +date_relative: 2001-01-05,
 +matched: 1,
 +subject: ,
 +tags: [
 +inbox,
 +unread
 +],
 +thread: XXX,
 +timestamp: 978709437,
 +total: 1
 +},
 +{
 +authors: Notmuch Test Suite,
 +date_relative: 1970-01-01,
 +matched: 1,
 +subject: ,
 +tags: [
 +inbox,
 +unread
 +],
 +thread: XXX,
 +timestamp: 0,
 +total: 1
 +}
 +]'
 +
 +test_begin_subtest Show: text
 +output=$(notmuch show '*')
 +test_expect_equal $output \
 +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 
 match:1 excluded:0 filename:/tmp/nmtest/tmp.missing-headers/mail/msg-2

The filename above has not been sanitised so contains your tmp path.

Best wishes

Mark

 +header{
 + (2001-01-05) (inbox unread)
 +Subject: (null)
 +From: (null)
 +To: Notmuch Test Suite test_su...@notmuchmail.org
 +Date: Fri, 05 Jan 2001 15:43:57 +
 +header}
 +body{
 +part{ ID: 1, Content-type: text/plain
 +Body
 +part}
 +body}
 +message}
 +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 
 match:1 excluded:0 filename:/tmp/nmtest/tmp.missing-headers/mail/msg-1
 +header{
 +Notmuch Test Suite test_su...@notmuchmail.org (1970-01-01) (inbox unread)
 +Subject: (null)
 +From: Notmuch Test Suite test_su...@notmuchmail.org
 +Date: Thu, 01 Jan 1970 00:00:00 +
 +header}
 +body{
 +part{ ID: 1, Content-type: text/plain
 +Body
 +part}
 +body}
 +message}
 +
 +test_begin_subtest Show: json
 +test_subtest_known_broken
 +output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize)
 +test_expect_equal_json $output '
 +[
 +[
 +[
 +{
 +body: [
 +{
 +content: Body\n,
 +content-type: text/plain,
 +id: 1
 +}
 +],
 +date_relative: 2001-01-05,
 +excluded: false,
 +filename: Y,
 +headers: {
 +Date: Fri, 05 Jan 2001 15:43:57 +,
 +From: ,
 +Subject: ,
 +To: Notmuch Test Suite 

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

2012-08-07 Thread Mark Walters

I like this (and agree with Austin and you that text and json can
diverge). I just hacked something together which uses this and makes the
emacs front-end display the content-length on part buttons and as
someone who uses notmuch over ssh that is nice.

I have two minor queries: do you think omitted text/html parts should
also have the length given? Or even should it always be sent? But I am
happy with it as is.

I haven't checked the test patch (or the text ones as they are being
dropped).

Best wishes

Mark

On Sun, 05 Aug 2012, Peter Wang noval...@gmail.com wrote:
 If a leaf part's body content is omitted, return the content length in
 --format=json output.  This information may be used by the consumer,
 e.g. to decide whether to download a large attachment over a slow link.
 ---
  devel/schemata |5 -
  notmuch-show.c |8 
  2 files changed, 12 insertions(+), 1 deletions(-)

 diff --git a/devel/schemata b/devel/schemata
 index 9cb25f5..3df2764 100644
 --- a/devel/schemata
 +++ b/devel/schemata
 @@ -69,7 +69,10 @@ part = {
  # A leaf part's body content is optional, but may be included if
  # it can be correctly encoded as a string.  Consumers should use
  # this in preference to fetching the part content separately.
 -content?:   string
 +content?:   string,
 +# If a leaf part's body content is not included, the content-length
 +# may be included instead.
 +content-length?: int
  }
  
  # The headers of a message or part (format_headers_json with reply = FALSE)
 diff --git a/notmuch-show.c b/notmuch-show.c
 index 3556293..5c54257 100644
 --- a/notmuch-show.c
 +++ b/notmuch-show.c
 @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t *sp, 
 mime_node_t *node,
   sp-map_key (sp, content);
   sp-string_len (sp, (char *) part_content-data, part_content-len);
   g_object_unref (stream_memory);
 + } else {
 + GMimeDataWrapper *wrapper = g_mime_part_get_content_object 
 (GMIME_PART (node-part));
 + GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
 + ssize_t length = g_mime_stream_length (stream);
 + if (length = 0) {
 + sp-map_key (sp, content-length);
 + sp-integer (sp, length);
 + }
   }
  } else if (GMIME_IS_MULTIPART (node-part)) {
   sp-map_key (sp, content);
 -- 
 1.7.4.4

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


[PATCH v2] test: Add test for messages with missing headers

2012-08-07 Thread Austin Clements
Currently the JSON tests for search and show are broken because
notmuch attempts to dereference a NULL pointer.
---
This version fixes the Show: text test so that it sanitize its
output and doesn't hard-code my test paths.

 test/missing-headers |  162 ++
 test/notmuch-test|1 +
 2 files changed, 163 insertions(+)
 create mode 100755 test/missing-headers

diff --git a/test/missing-headers b/test/missing-headers
new file mode 100755
index 000..e79f922
--- /dev/null
+++ b/test/missing-headers
@@ -0,0 +1,162 @@
+#!/usr/bin/env bash
+test_description='messages with missing headers'
+. ./test-lib.sh
+
+# Notmuch requires at least one of from, subject, or to or it will
+# ignore the file.  Generate two messages so that together they cover
+# all possible missing headers.  We also give one of the messages a
+# date to ensure stable result ordering.
+
+cat EOF  ${MAIL_DIR}/msg-2
+To: Notmuch Test Suite test_su...@notmuchmail.org
+Date: Fri, 05 Jan 2001 15:43:57 +
+
+Body
+EOF
+
+cat EOF  ${MAIL_DIR}/msg-1
+From: Notmuch Test Suite test_su...@notmuchmail.org
+
+Body
+EOF
+
+NOTMUCH_NEW
+
+test_begin_subtest Search: text
+output=$(notmuch search '*' | notmuch_search_sanitize)
+test_expect_equal $output \
+thread:XXX   2001-01-05 [1/1] (null);  (inbox unread)
+thread:XXX   1970-01-01 [1/1] Notmuch Test Suite;  (inbox unread)
+
+test_begin_subtest Search: json
+test_subtest_known_broken
+output=$(notmuch search --format=json '*' | notmuch_search_sanitize)
+test_expect_equal_json $output '
+[
+{
+authors: ,
+date_relative: 2001-01-05,
+matched: 1,
+subject: ,
+tags: [
+inbox,
+unread
+],
+thread: XXX,
+timestamp: 978709437,
+total: 1
+},
+{
+authors: Notmuch Test Suite,
+date_relative: 1970-01-01,
+matched: 1,
+subject: ,
+tags: [
+inbox,
+unread
+],
+thread: XXX,
+timestamp: 0,
+total: 1
+}
+]'
+
+test_begin_subtest Show: text
+output=$(notmuch show '*' | notmuch_show_sanitize)
+test_expect_equal $output \
+message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 
match:1 excluded:0 filename:/XXX/mail/msg-2
+header{
+ (2001-01-05) (inbox unread)
+Subject: (null)
+From: (null)
+To: Notmuch Test Suite test_su...@notmuchmail.org
+Date: Fri, 05 Jan 2001 15:43:57 +
+header}
+body{
+part{ ID: 1, Content-type: text/plain
+Body
+part}
+body}
+message}
+message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 
match:1 excluded:0 filename:/XXX/mail/msg-1
+header{
+Notmuch Test Suite test_su...@notmuchmail.org (1970-01-01) (inbox unread)
+Subject: (null)
+From: Notmuch Test Suite test_su...@notmuchmail.org
+Date: Thu, 01 Jan 1970 00:00:00 +
+header}
+body{
+part{ ID: 1, Content-type: text/plain
+Body
+part}
+body}
+message}
+
+test_begin_subtest Show: json
+test_subtest_known_broken
+output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize)
+test_expect_equal_json $output '
+[
+[
+[
+{
+body: [
+{
+content: Body\n,
+content-type: text/plain,
+id: 1
+}
+],
+date_relative: 2001-01-05,
+excluded: false,
+filename: Y,
+headers: {
+Date: Fri, 05 Jan 2001 15:43:57 +,
+From: ,
+Subject: ,
+To: Notmuch Test Suite test_su...@notmuchmail.org
+},
+id: X,
+match: true,
+tags: [
+inbox,
+unread
+],
+timestamp: 978709437
+},
+[]
+]
+],
+[
+[
+{
+body: [
+{
+content: Body\n,
+content-type: text/plain,
+id: 1
+}
+],
+date_relative: 1970-01-01,
+excluded: false,
+filename: Y,
+headers: {
+Date: Thu, 01 Jan 1970 00:00:00 +,
+From: Notmuch Test Suite test_su...@notmuchmail.org,
+Subject: 
+},
+id: X,
+match: true,
+tags: [
+inbox,
+unread
+],
+timestamp: 0
+},
+[]
+]
+]
+]'
+
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index ea39dfc..cc732c3 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -59,6 +59,7 @@ TESTS=
   

Re: [PATCH] sprinters: bugfix when NULL passed for a string.

2012-08-07 Thread Jameson Graef Rollins
On Tue, Aug 07 2012, Mark Walters markwalters1...@gmail.com wrote:
 The string function in a sprinter may be called with a NULL string
 pointer (eg if a header is absent). This causes a segfault. We fix
 this by checking for a null pointer in the string functions and update
 the sprinter documentation.

 At the moment some output when format=text is done directly rather than
 via an sprinter: in that case a null pointer is passed to printf or
 similar and a (null) appears in the output. That behaviour is not
 changed in this patch.
 ---

 This could really do with some tests (it is the second time this type of
 bug has occurred). To be considered as a message by notmuch new a file
 needs at least one of a From: Subject: or To: header. Thus we should
 have three messages each of which just contains that single header (and
 nothing else) and check that search and show work as expected. 

Hey, Mark.  Thanks for working on this.

I was wondering if we should distinguish between the header being
absent, and having a null value.  It looks like the idea here is to
output an empty string for the value in all of these cases.  But should
we output the field at all if the actual header isn't there?  In other
words, I can imagine three scenarios:

Header: value
Header:-- Header: 
no header 

At the moment these would be output as:

Header: value
Header: 
Header: 

Where as I could imagine we could instead do:

Header: value
Header: 
no output

Maybe that would be too complicated or break the output spec to much?
If it's too complicated to do the later, then I'm fine with this
solution as is.

I definitely agree we need tests for this.

jamie.


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


Re: [PATCH] sprinters: bugfix when NULL passed for a string.

2012-08-07 Thread Austin Clements
LGTM.

This won't commute with [0], since that introduces broken tests that are
fixed by this patch.

I think we should remove the fields in the JSON header object for
missing headers (except perhaps From and Date, if those really are
mandatory headers), but I think we should do that after the freeze,
since it might affect frontends.  It probably makes sense to add an
Emacs test or two to the tests in [0].

[0] id:1344389313-7886-1-git-send-email-amdra...@mit.edu

On Tue, 07 Aug 2012, Mark Walters markwalters1...@gmail.com wrote:
 The string function in a sprinter may be called with a NULL string
 pointer (eg if a header is absent). This causes a segfault. We fix
 this by checking for a null pointer in the string functions and update
 the sprinter documentation.

 At the moment some output when format=text is done directly rather than
 via an sprinter: in that case a null pointer is passed to printf or
 similar and a (null) appears in the output. That behaviour is not
 changed in this patch.
 ---

 This could really do with some tests (it is the second time this type of
 bug has occurred). To be considered as a message by notmuch new a file
 needs at least one of a From: Subject: or To: header. Thus we should
 have three messages each of which just contains that single header (and
 nothing else) and check that search and show work as expected. 



  sprinter-json.c |2 ++
  sprinter-text.c |2 ++
  sprinter.h  |4 +++-
  3 files changed, 7 insertions(+), 1 deletions(-)

 diff --git a/sprinter-json.c b/sprinter-json.c
 index c9b6835..0a07790 100644
 --- a/sprinter-json.c
 +++ b/sprinter-json.c
 @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, 
 size_t len)
  static void
  json_string (struct sprinter *sp, const char *val)
  {
 +if (val == NULL)
 + val = ;
  json_string_len (sp, val, strlen (val));
  }
  
 diff --git a/sprinter-text.c b/sprinter-text.c
 index dfa54b5..10343be 100644
 --- a/sprinter-text.c
 +++ b/sprinter-text.c
 @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, 
 size_t len)
  static void
  text_string (struct sprinter *sp, const char *val)
  {
 +if (val == NULL)
 + val = ;
  text_string_len (sp, val, strlen (val));
  }
  
 diff --git a/sprinter.h b/sprinter.h
 index 5f43175..912a526 100644
 --- a/sprinter.h
 +++ b/sprinter.h
 @@ -27,7 +27,9 @@ typedef struct sprinter {
   * a list or map, followed or preceded by separators).  For string
   * and string_len, the char * must be UTF-8 encoded.  string_len
   * allows non-terminated strings and strings with embedded NULs
 - * (though the handling of the latter is format-dependent).
 + * (though the handling of the latter is format-dependent). For
 + * string (but not string_len) the string pointer passed may be
 + * NULL.
   */
  void (*string) (struct sprinter *, const char *);
  void (*string_len) (struct sprinter *, const char *, size_t);
 -- 
 1.7.9.1


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