[PATCH] test: Add test for messages with missing headers
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.fsf at qmul.ac.uk" the tests (modulo the test bug) pass. On Tue, 07 Aug 2012, Austin Clements 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 < "${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 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 > +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 +", > +
[PATCH 1/4] show: indicate length of omitted body content (json)
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. > > 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
Re: [PATCH v2] emacs: notmuch search bugfix
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). > --- LGTM. Tomi > > 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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] sprinters: bugfix when NULL passed for a string.
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
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=" emacs-address-clea
[PATCH] sprinters: bugfix when NULL passed for a string.
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
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 (&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 +--ooO--(_)--Ooo-- > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] sprinters: bugfix when NULL passed for a string.
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 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
[PATCH] emacs: notmuch search bugfix
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 (&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 at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] notmuch-show: add notmuch-show-auto-mark-read option
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 (&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 at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] sprinters: bugfix when NULL passed for a string.
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. pgpwRxLzAxTqr.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] sprinters: bugfix when NULL passed for a string.
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 v2] test: Add test for messages with missing headers
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=" emacs-address-clea
[PATCH] notmuch-show: add notmuch-show-auto-mark-read option
>> 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
> 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 (&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 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 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 +--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
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
‘class Xapian::Database’ has no member named ‘close’
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 +--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
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": "N
[PATCH] cli: Remove now-unused json.c
+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 > http://notmuchmail.or
[PATCH] reply: Convert JSON format to use sprinter
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
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 (&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 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 (&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 +--ooO--(_)--Ooo--
[PATCH] emacs: notmuch search bugfix
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 (&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
[PATCH] notmuch-show: add notmuch-show-auto-mark-read option
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 (&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
[PATCH] cli: Remove now-unused json.c
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 > http://notmuchmail.
[PATCH] reply: Convert JSON format to use sprinter
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
Re: [PATCH 1/4] show: indicate length of omitted body content (json)
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 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
Re: [PATCH] test: Add test for messages with missing headers
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 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 < "${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 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 > +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 +", > +
[PATCH v2] emacs: notmuch search bugfix
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] test: Add test for messages with missing headers
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": "N
[PATCH] sprinters: bugfix when NULL passed for a string.
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
[PATCH] emacs: notmuch search bugfix
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 (&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)
Vim plugins
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
Re: [PATCH v2] emacs: notmuch search bugfix
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. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/4] show: indicate length of omitted body content (json)
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
Re: [PATCH] notmuch-show: add notmuch-show-auto-mark-read option
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 (&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 +--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
Vim plugins
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 v2] emacs: notmuch search bugfix
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] emacs: notmuch search bugfix
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] cli: Remove now-unused json.c
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
Re: [PATCH] emacs: notmuch search bugfix
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] cli: Remove now-unused json.c
+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@notmuchmail.org > http://notmuchmail.org/m
Re: [PATCH] reply: Convert JSON format to use sprinter
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@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
>> 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-- pgpDSdT99fBDM.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] reply: Convert JSON format to use sprinter
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
Re: [PATCH] notmuch-show: add notmuch-show-auto-mark-read option
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 (&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
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 (&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 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 (&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 +--ooO--(_)--Ooo-- ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Segmentation fault in notmuch search --format=json
Quoth Mark Walters on Aug 07 at 8:07 am: > 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. 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)); > } >
[PATCH] emacs: notmuch search bugfix
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
> 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 (&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 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 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 +--ooO--(_)--Ooo-- pgpsZ7bYwHCbd.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
‘class Xapian::Database’ has no member named ‘close’
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 +--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
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 (&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
Segmentation fault in notmuch search --format=json
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)); }
Re: [PATCH 1/4] show: indicate length of omitted body content (json)
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
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
Re: [PATCH 1/4] show: indicate length of omitted body content (json)
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. > > 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] cli: Remove now-unused json.c
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@notmuchmail.org > http://notmuchmail.org
Re: [PATCH] reply: Convert JSON format to use sprinter
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@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] cli: Remove now-unused json.c
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@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] reply: Convert JSON format to use sprinter
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
Re: Segmentation fault in notmuch search --format=json
Quoth Mark Walters on Aug 07 at 8:07 am: > 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. 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
Re: Vim plugins
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Segmentation fault in notmuch search --format=json
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
Re: Vim plugins
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@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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: Segmentation fault in notmuch search --format=json
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. pgp2YNSAKxWh8.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Segmentation fault in notmuch search --format=json
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>
[PATCH 1/4] show: indicate length of omitted body content (json)
On Sun, 05 Aug 2012 14:37:02 -0700, Jameson Graef Rollins wrote: > On Sun, Aug 05 2012, Peter Wang wrote: > > 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 > > Hey, Peter. Something somewhere, and probably at least here in the > schemata, should mention what the uids are (b? kB? KiB? YiB?) I thought content-length was a MIME header, but it's not. Anyway, it's the length of the encoded content in bytes. GMime doesn't provide an easy way to return the length of the decoded content. I actually only need an _estimate_ of the decoded size to present to the user. Strictly speaking, that requires knowledge of the transfer encoding. Previously I planned on guessing it from the content-type, but I think it's better to return the transfer encoding as well (if any). Alternatively, notmuch could put in more effort to return the exact length of the decoded content. But it's a waste of time if no consumers will use it. Peter
Small bug in json-search changes
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)
Re: Segmentation fault in notmuch search --format=json
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)); } ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch