[PATCH 2/2] lib/database.cc: change how the parent of a message is calculated
LGTM
On Wed, 06 Mar 2013, Aaron Ecay wrote:
> Presently, the code which finds the parent of a message as it is being
> added to the database assumes that the first Message-ID-like substring
> of the In-Reply-To header is the parent Message ID. Some mail clients,
> however, put stuff other than the Message-ID of the parent in the
> In-Reply-To header, such as the email address of the sender of the
> parent. This can fool notmuch.
>
> The updated algorithm prefers the last Message ID in the References
> header. The References header lists messages oldest-first, so the last
> Message ID is the parent (RFC2822, p. 24). The References header is
> also less likely to be in a non-standard
> syntax (http://cr.yp.to/immhf/thread.html,
> http://www.jwz.org/doc/threading.html). In case the References header
> is not to be found, fall back to the old behavior.
>
> V2 of this patch, incorporating feedback from Jani and (indirectly)
> Austin.
> ---
> lib/database.cc | 48 +---
> test/thread-replies | 4
> 2 files changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 91d4329..52ed618 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -501,8 +501,10 @@ _parse_message_id (void *ctx, const char *message_id,
> const char **next)
> * 'message_id' in the result (to avoid mass confusion when a single
> * message references itself cyclically---and yes, mail messages are
> * not infrequent in the wild that do this---don't ask me why).
> -*/
> -static void
> + *
> + * Return the last reference parsed, if it is not equal to message_id.
> + */
> +static char *
> parse_references (void *ctx,
> const char *message_id,
> GHashTable *hash,
> @@ -511,7 +513,7 @@ parse_references (void *ctx,
> char *ref;
>
> if (refs == NULL || *refs == '\0')
> - return;
> + return NULL;
>
> while (*refs) {
> ref = _parse_message_id (ctx, refs, &refs);
> @@ -519,6 +521,17 @@ parse_references (void *ctx,
> if (ref && strcmp (ref, message_id))
> g_hash_table_insert (hash, ref, NULL);
> }
> +
> +/* The return value of this function is used to add a parent
> + * reference to the database. We should avoid making a message
> + * its own parent, thus the following check.
> + */
> +
> +if (ref && strcmp(ref, message_id)) {
> + return ref;
> +} else {
> + return NULL;
> +}
> }
>
> notmuch_status_t
> @@ -1510,28 +1523,33 @@ _notmuch_database_link_message_to_parents
> (notmuch_database_t *notmuch,
> {
> GHashTable *parents = NULL;
> const char *refs, *in_reply_to, *in_reply_to_message_id;
> +const char *last_ref_message_id, *this_message_id;
> GList *l, *keys = NULL;
> notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
>
> parents = g_hash_table_new_full (g_str_hash, g_str_equal,
>_my_talloc_free_for_g_hash, NULL);
> +this_message_id = notmuch_message_get_message_id (message);
>
> refs = notmuch_message_file_get_header (message_file, "references");
> -parse_references (message, notmuch_message_get_message_id (message),
> - parents, refs);
> +last_ref_message_id = parse_references (message,
> + this_message_id,
> + parents, refs);
>
> in_reply_to = notmuch_message_file_get_header (message_file,
> "in-reply-to");
> -parse_references (message, notmuch_message_get_message_id (message),
> - parents, in_reply_to);
> -
> -/* Carefully avoid adding any self-referential in-reply-to term. */
> -in_reply_to_message_id = _parse_message_id (message, in_reply_to, NULL);
> -if (in_reply_to_message_id &&
> - strcmp (in_reply_to_message_id,
> - notmuch_message_get_message_id (message)))
> -{
> +in_reply_to_message_id = parse_references (message,
> +this_message_id,
> +parents, in_reply_to);
> +
> +/* For the parent of this message, use the last message ID of the
> + * References header, if available. If not, fall back to the
> + * first message ID in the In-Reply-To header. */
> +if (last_ref_message_id) {
> +_notmuch_message_add_term (message, "replyto",
> + last_ref_message_id);
> +} else if (in_reply_to_message_id) {
> _notmuch_message_add_term (message, "replyto",
> - _parse_message_id (message, in_reply_to, NULL));
> + in_reply_to_message_id);
> }
>
> keys = g_hash_table_get_keys (parents);
> diff --git a/test/thread-replies b/test/thread-replies
> index a902691..28c2b1f 100755
> --- a/test/thread-replies
> +++ b/test/thread-replies
> @@ -11,7 +11,6 @@ constructed prope
[PATCH 1/2] test: add tests for the handling of References and In-Reply-To headers
On Wed, 06 Mar 2013, Aaron Ecay wrote:
> These tests are known_broken, the following commit fixes them.
> ---
>
> Thanks to David and Tomi for pointing out test_expect_equal_json. In
> the process of implementing that, I discovered
> notmuch_json_show_sanitize, which I had also not been using. This
> patch fixes the test to use both these conveniences. The second patch
> is not changed substantively, but I am resending it for tidiness.
>
> test/thread-replies | 144
>
> 1 file changed, 144 insertions(+)
> create mode 100755 test/thread-replies
>
> diff --git a/test/thread-replies b/test/thread-replies
> new file mode 100755
> index 000..a902691
> --- /dev/null
> +++ b/test/thread-replies
> @@ -0,0 +1,144 @@
> +#!/usr/bin/env bash
> +#
> +# Copyright (c) 2013 Aaron Ecay
> +#
> +
> +test_description='test of proper handling of in-reply-to and references
> headers
> +
> +This test makes sure that the thread structure in the notmuch database is
> +constructed properly, even in the presence of non-RFC-compliant headers'
Nitpick, most other tests have one line test descriptions. The second
paragraph could be left as comment.
> +
> +. ./test-lib.sh
> +
> +test_begin_subtest "Use References when In-Reply-To is broken"
> +test_subtest_known_broken
> +add_message '[id]="foo at one.com"' \
> +'[subject]=one'
> +add_message '[in-reply-to]="mumble"' \
> +'[references]=""' \
> +'[subject]="Re: one"'
> +output=$(notmuch show --format=json 'subject:one' |
> notmuch_json_show_sanitize)
> +expected='[[[{"id": "foo at one.com",
> + "match": true,
> + "excluded": false,
> + "filename":
> "/home/aecay/development/notmuch/notmuch-git/src/notmuch/test/tmp.thread-replies/mail/msg-001",
Please replace the path with:
"filename": "Y",
Ditto below for all of them.
> + "timestamp": 978709437,
> + "date_relative": "2001-01-05",
> + "tags": ["inbox", "unread"],
> + "headers": {"Subject": "one",
> + "From": "Notmuch Test Suite ",
> + "To": "Notmuch Test Suite ",
> + "Date": "Fri, 05 Jan 2001 15:43:57 +"},
> + "body": [{"id": 1,
> + "content-type": "text/plain",
> + "content": "This is just a test message (#1)\n"}]},
> + [[{"id": "msg-002 at notmuch-test-suite",
> + "match": true, "excluded": false,
> + "filename":
> "/home/aecay/development/notmuch/notmuch-git/src/notmuch/test/tmp.thread-replies/mail/msg-002",
> + "timestamp": 978709437, "date_relative": "2001-01-05",
> + "tags": ["inbox", "unread"], "headers": {"Subject": "Re: one",
> + "From": "Notmuch Test Suite ",
> + "To": "Notmuch Test Suite ",
> + "Date": "Fri, 05 Jan 2001 15:43:57 +"},
> + "body": [{"id": 1, "content-type": "text/plain",
> + "content": "This is just a test message (#2)\n"}]}, []]'
> +expected=`echo "$expected" | notmuch_json_show_sanitize`
> +test_expect_equal_json "$output" "$expected"
> +
> +test_begin_subtest "Prefer References to In-Reply-To"
> +test_subtest_known_broken
> +add_message '[id]="foo at two.com"' \
> +'[subject]=two'
> +add_message '[in-reply-to]=""' \
> +'[references]=""' \
> +'[subject]="Re: two"'
> +output=$(notmuch show --format=json 'subject:two' |
> notmuch_json_show_sanitize)
> +expected='[[[{"id": "foo at two.com",
> + "match": true, "excluded": false,
> + "filename":
> "/home/aecay/development/notmuch/notmuch-git/src/notmuch/test/tmp.thread-replies/mail/msg-003",
> + "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["inbox",
> "unread"],
> + "headers": {"Subject": "two",
> + "From": "Notmuch Test Suite ",
> + "To": "Notmuch Test Suite ",
> + "Date": "Fri, 05 Jan 2001 15:43:57 +"},
> + "body": [{"id": 1, "content-type": "text/plain",
> + "content": "This is just a test message (#3)\n"}]},
> + [[{"id": "msg-004 at notmuch-test-suite", "match": true, "excluded": false,
> + "filename":
> "/home/aecay/development/notmuch/notmuch-git/src/notmuch/test/tmp.thread-replies/mail/msg-004",
> + "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["inbox",
> "unread"],
> + "headers": {"Subject": "Re: two",
> + "From": "Notmuch Test Suite ",
> + "To": "Notmuch Test Suite ",
> + "Date": "Fri, 05 Jan 2001 15:43:57 +"},
> + "body": [{"id": 1,
> + "content-type": "text/plain", "content": "This is just a test message
> (#4)\n"}]},
> + []]'
> +expected=`echo "$expected" | notmuch_json_show_sanitize`
> +test_expect_equal_json "$output" "$expected"
> +
> +test_begin_subtest "Use In-Reply-To when no References"
> +test_subtest_known_broken
As David said, this is not broken currently, as In-Reply-To is used
unconditionally. Just drop the broken annotation, the test itself is
useful.
> +add_message '[id]="foo at three.com"' \
> +'[subject]="three"'
> +add_message '[in-reply-to]=""' \
> +'[subject]="Re: three"'
> +output=$(notmuch show --format=json 'subject:three' |
> notmuch_json_show_sanitize)
> +expected='[[[{"id": "foo at three.com", "match": true, "excluded": false,
> + "filename":
> "/home/
Re: [PATCH 2/2] emacs: hello: allow deleting individual searches in the history
These patches both look good to me (and the lisp all looks fine) modulo a trivial style comment and a little bikeshedding. For the bikeshedding I would prefer that the first patch was a y-or-n-p (as Jani suggested) and that the second didn't query at all (the dataloss potential from deleting a single search from the history is pretty small) On Fri, 03 May 2013, Servilio Afre Puentes wrote: > This commit adds an extra button at the end of the search entries that > allows deleting that individual search from the history. A short > confirmation («y» or «n») is made before taking action. > --- > emacs/notmuch-hello.el | 26 -- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el > index aa063b7..9fbbdc9 100644 > --- a/emacs/notmuch-hello.el > +++ b/emacs/notmuch-hello.el > @@ -286,6 +286,16 @@ afterwards.") > (message "Saved '%s' as '%s'." search name) > (notmuch-hello-update))) > > +(defun notmuch-hello-delete-search-from-history (widget) > + (interactive) > + (let ((search (widget-value > + (symbol-value > + (widget-get widget :notmuch-saved-search-widget) > +(setq notmuch-search-history (delete search > + notmuch-search-history)) > +(notmuch-hello-update) > +)) > + Trivial formatting/style point: notmuch goes for putting all the braces after the final function. Best wishes Mark > (defun notmuch-hello-longest-label (searches-alist) >(or (loop for elem in searches-alist > maximize (length (car elem))) > @@ -624,7 +634,12 @@ Complete list of currently available key bindings: > ;; `[save]' button. 6 > ;; for the `[save]' > ;; button. > - 1 6)) > + 1 6 > + ;; 1 for the space > + ;; before the `[del]' > + ;; button. 5 for the > + ;; `[del]' button. > + 1 5)) > :action (lambda (widget &rest ignore) > (notmuch-hello-search (widget-value > widget))) > search)) > @@ -633,7 +648,14 @@ Complete list of currently available key bindings: >:notify (lambda (widget &rest ignore) > (notmuch-hello-add-saved-search widget)) >:notmuch-saved-search-widget widget-symbol > - "save")) > + "save") > + (widget-insert " ") > + (widget-create 'push-button > + :notify (lambda (widget &rest ignore) > +(when (y-or-n-p "Are you sure you want > to delete this search? ") > + > (notmuch-hello-delete-search-from-history widget))) > + :notmuch-saved-search-widget widget-symbol > + "del")) > (widget-insert "\n")) >(indent-rigidly start (point) notmuch-hello-indent)) > nil)) > -- > 1.8.2.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
Re: [PATCH REBASE] emacs: add show view bindings to move to previous/next thread
Hi
This seems like a useful addition to me. I have a couple of comments and
a little bikeshedding below.
On Thu, 02 May 2013, Jani Nikula wrote:
> We have most of the plumbing in place, add the bindings M-n and M-p.
> ---
> emacs/notmuch-show.el | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index face2a0..7f6ea65 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -39,6 +39,7 @@
>
> (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
> (declare-function notmuch-search-next-thread "notmuch" nil)
> +(declare-function notmuch-search-previous-thread "notmuch" nil)
> (declare-function notmuch-search-show-thread "notmuch" nil)
>
> (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
> @@ -1267,6 +1268,8 @@ reset based on the original query."
> (define-key map "P" 'notmuch-show-previous-message)
> (define-key map "n" 'notmuch-show-next-open-message)
> (define-key map "p" 'notmuch-show-previous-open-message)
> + (define-key map (kbd "M-n") 'notmuch-show-next-thread-show)
> + (define-key map (kbd "M-p") 'notmuch-show-previous-thread-show)
These seem sensible bindings.
> (define-key map (kbd "DEL") 'notmuch-show-rewind)
> (define-key map " " 'notmuch-show-advance-and-archive)
> (define-key map (kbd "M-RET") 'notmuch-show-open-or-close-all)
> @@ -1839,6 +1842,27 @@ argument, hide all of the messages."
>(if show-next
> (notmuch-search-show-thread)
>
> +(defun notmuch-show-previous-thread (&optional show-previous)
> + "Move to the next item in the search results, if any."
^^
Should be previous item.
> + (interactive "P")
> + (let ((parent-buffer notmuch-show-parent-buffer))
> +(notmuch-kill-this-buffer)
> +(when (buffer-live-p parent-buffer)
> + (switch-to-buffer parent-buffer)
> + (notmuch-search-previous-thread)
> + (if show-previous
> + (notmuch-search-show-thread)
> +
If you change this to
(if (and (notmuch-search-previous-thread) show-previous)
(notmuch-search-show-thread)
then if you apply it to the first thread it jumps back to the search menu
which is
consistent with the next-thread version.
I would have a slight preference for adding another optional argument
notmuch-show-next-thread (&optional show-message previous)
where if PREVIOUS is set then it would go back otherwise forward. But I
with the duplicated version too.
Best wishes
Mark
> +(defun notmuch-show-next-thread-show ()
> + "Show the next thread in the search results, if any."
> + (interactive)
> + (notmuch-show-next-thread t))
> +
> +(defun notmuch-show-previous-thread-show ()
> + "Show the previous thread in the search results, if any."
> + (interactive)
> + (notmuch-show-previous-thread t))
> +
> (defun notmuch-show-archive-thread (&optional unarchive)
>"Archive each message in thread.
>
> --
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 5/6] python: Add bindings for notmuch_thread_get_messages
Austin Clements writes: > --- > bindings/python/notmuch/thread.py | 27 ++- Justus OKed this privately. d
[PATCH 4/4] emacs: show: implement lazy hidden part handling
This adds the actual code to do the lazy insertion of hidden parts. We use a memory inefficient but simple method: when we come to insert the part if it is hidden we just store all of the arguments to the part insertion function as a button property. This means when we want to show the part we can just resume where we left off. The only slight subtlety/hack is that to simplify the handling of the invisibility overlay (for the hiding unhiding later) we do insert some dummy text which we remove when we show the part. --- emacs/notmuch-show.el | 32 ++-- 1 files changed, 30 insertions(+), 2 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 2c48b24..a14a135 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -559,6 +559,7 @@ message at DEPTH in the current thread." (overlay (button-get button 'overlay))) (when overlay (let* ((show (overlay-get overlay 'invisible)) +(lazy-part (button-get button :notmuch-lazy-part)) (new-start (button-start button)) (button-label (button-get button :base-label)) (old-point (point)) @@ -569,7 +570,11 @@ message at DEPTH in the current thread." (let ((old-end (button-end button))) (move-overlay button new-start (point)) (delete-region (point) old-end)) - (goto-char (min old-point (1- (button-end button + (goto-char (min old-point (1- (button-end button + (when (and show lazy-part) + (save-excursion + (button-put button :notmuch-lazy-part nil) + (notmuch-show-lazy-part lazy-part button))) (defun notmuch-show-multipart/*-to-list (part) (mapcar (lambda (inner-part) (plist-get inner-part :content-type)) @@ -857,6 +862,24 @@ message at DEPTH in the current thread." (setq handlers (cdr handlers t) +(defun notmuch-show-lazy-part (part-args button) + (interactive) + ;; We have to save the depth as we can't find the depth when narrowed + (let ((inhibit-read-only t) + (overlay (button-get button 'overlay)) + (depth (notmuch-show-get-depth))) +(save-restriction + (narrow-to-region (overlay-start overlay) (1- (overlay-end overlay))) + (delete-region (overlay-start overlay) (1- (overlay-end overlay))) + (goto-char (overlay-start overlay)) + (apply #'notmuch-show-insert-bodypart-internal (nconc part-args (list button))) + (indent-rigidly (overlay-start overlay) + (1- (overlay-end overlay)) + depth)) +;; We deferred deleting this character to simplify handling of the +;; overlay: all of the above takes place inside the overlay. +(delete-region (1- (overlay-end overlay)) (overlay-end overlay + (defun notmuch-show-create-part-overlays (msg beg end hide) "Add an overlay to the part between BEG and END" (let* ((button (button-at beg)) @@ -891,7 +914,12 @@ If HIDE is non-nil then initially hide this part." (button (unless (string= mime-type "text/plain") (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename) -(notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button) +(if (not hide) +(notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button) + (insert "lazy part") + (button-put button :notmuch-lazy-part + (list msg part mime-type nth depth content-type))) + ;; Some of the body part handlers leave point somewhere up in the ;; part, so we make sure that we're down at the end. (goto-char (point-max)) -- 1.7.9.1
[PATCH 3/4] emacs: show: move the insertion of the header button to the top level
Previously each of the part insertion handlers inserted the part button themselves. Move this up into notmuch-show-insert-bodypart. Since a small number of the handlers modify the button (the encryption/signature ones) we need to pass the header button as an argument into the individual part insertion handlers. The patch is large but mostly simple. The only things of note are that we let the text/plain handler insert part buttons itself (as it does not always insert one and it applies motmuch-wash to the whole part including the button. Also this is by far the most important case so this reduces the risk of annoying side effects. The handler for inline-patch-fake-part is called from notmuch wash not from notmuch-show-insert-bodypart so that needs to insert the header button itself (rather than relying on notmuch-show-insert-part-*/*). --- emacs/notmuch-show.el | 91 +++- 1 files changed, 44 insertions(+), 47 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index f984143..2c48b24 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -575,8 +575,7 @@ message at DEPTH in the current thread." (mapcar (lambda (inner-part) (plist-get inner-part :content-type)) (plist-get part :content))) -(defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type) - (notmuch-show-insert-part-header nth declared-type content-type nil) +(defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type button) (let ((chosen-type (car (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part (inner-parts (plist-get part :content)) (start (point))) @@ -653,8 +652,7 @@ message at DEPTH in the current thread." content-type) nil))) -(defun notmuch-show-insert-part-multipart/related (msg part content-type nth depth declared-type) - (notmuch-show-insert-part-header nth declared-type content-type nil) +(defun notmuch-show-insert-part-multipart/related (msg part content-type nth depth declared-type button) (let ((inner-parts (plist-get part :content)) (start (point))) @@ -673,16 +671,15 @@ message at DEPTH in the current thread." (indent-rigidly start (point) 1))) t) -(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type) - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil))) -(button-put button 'face 'notmuch-crypto-part-header) -;; add signature status button if sigstatus provided -(if (plist-member part :sigstatus) - (let* ((from (notmuch-show-get-header :From msg)) - (sigstatus (car (plist-get part :sigstatus - (notmuch-crypto-insert-sigstatus-button sigstatus from)) - ;; if we're not adding sigstatus, tell the user how they can get it - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))) +(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type button) + (button-put button 'face 'notmuch-crypto-part-header) + ;; add signature status button if sigstatus provided + (if (plist-member part :sigstatus) + (let* ((from (notmuch-show-get-header :From msg)) +(sigstatus (car (plist-get part :sigstatus + (notmuch-crypto-insert-sigstatus-button sigstatus from)) +;; if we're not adding sigstatus, tell the user how they can get it +(button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")) (let ((inner-parts (plist-get part :content)) (start (point))) @@ -695,20 +692,19 @@ message at DEPTH in the current thread." (indent-rigidly start (point) 1))) t) -(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type) - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil))) -(button-put button 'face 'notmuch-crypto-part-header) -;; add encryption status button if encstatus specified -(if (plist-member part :encstatus) - (let ((encstatus (car (plist-get part :encstatus - (notmuch-crypto-insert-encstatus-button encstatus) - ;; add signature status button if sigstatus specified - (if (plist-member part :sigstatus) - (let* ((from (notmuch-show-get-header :From msg)) -(sigstatus (car (plist-get part :sigstatus - (notmuch-crypto-insert-sigstatus-button sigstatus from - ;; if we're not adding encstatus, tell the user how they can get it - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))) +(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type button) + (button-put button 'face 'notmuch-crypto-part-head
[PATCH 2/4] emacs: show: handle inline patch fake parts at top level
The inline patch fake part handler also modifies the content-type so handle this in notmuch-show-insert-bodypart too. --- emacs/notmuch-show.el |4 +++- emacs/notmuch-wash.el |2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 3b9a2ad..f984143 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -814,7 +814,7 @@ message at DEPTH in the current thread." ;; Handler for wash generated inline patch fake parts. (defun notmuch-show-insert-part-inline-patch-fake-part (msg part content-type nth depth declared-type) - (notmuch-show-insert-part-*/* msg part "text/x-diff" nth depth "inline patch")) + (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type)) (defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type) ;; text/html handler to work around bugs in renderers and our @@ -888,6 +888,8 @@ If HIDE is non-nil then initially hide this part." (let* ((content-type (downcase (plist-get part :content-type))) (mime-type (or (and (string= content-type "application/octet-stream") (notmuch-show-get-mime-type-of-application/octet-stream part)) + (and (string= content-type "inline patch") +"text/x-diff") content-type)) (nth (plist-get part :id)) (beg (point))) diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el index 80c475c..8a68819 100644 --- a/emacs/notmuch-wash.el +++ b/emacs/notmuch-wash.el @@ -365,7 +365,7 @@ for error." (setq patch-end (match-beginning 0))) (save-restriction (narrow-to-region patch-start patch-end) - (setq part (plist-put part :content-type "inline-patch-fake-part")) + (setq part (plist-put part :content-type "inline patch")) (setq part (plist-put part :content (buffer-string))) (setq part (plist-put part :id -1)) (setq part (plist-put part :filename -- 1.7.9.1
[PATCH 1/4] emacs:show: separate out handling of application/octet-stream
Currently mime parts are basically handled based on their mime-type with the exception of application/octet-stream parts. Deal with these parts at the top level (notmuch-show-insert-bodypart). This is needed later in the series as we need to put in a part button for each part (which means knowing its mime type) while deferring the actual insertion of the part. --- emacs/notmuch-show.el | 19 +++ 1 files changed, 11 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index face2a0..3b9a2ad 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -796,9 +796,9 @@ message at DEPTH in the current thread." (defun notmuch-show-insert-part-text/x-vcalendar (msg part content-type nth depth declared-type) (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type)) -(defun notmuch-show-insert-part-application/octet-stream (msg part content-type nth depth declared-type) +(defun notmuch-show-get-mime-type-of-application/octet-stream (part) ;; If we can deduce a MIME type from the filename of the attachment, - ;; do so and pass it on to the handler for that type. + ;; we return that. (if (plist-get part :filename) (let ((extension (file-name-extension (plist-get part :filename))) mime-type) @@ -808,7 +808,7 @@ message at DEPTH in the current thread." (setq mime-type (mailcap-extension-to-mime extension)) (if (and mime-type (not (string-equal mime-type "application/octet-stream"))) - (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type) + mime-type nil)) nil @@ -885,11 +885,14 @@ message at DEPTH in the current thread." "Insert the body part PART at depth DEPTH in the current thread. If HIDE is non-nil then initially hide this part." - (let ((content-type (downcase (plist-get part :content-type))) - (nth (plist-get part :id)) - (beg (point))) - -(notmuch-show-insert-bodypart-internal msg part content-type nth depth content-type) + (let* ((content-type (downcase (plist-get part :content-type))) +(mime-type (or (and (string= content-type "application/octet-stream") + (notmuch-show-get-mime-type-of-application/octet-stream part)) + content-type)) +(nth (plist-get part :id)) +(beg (point))) + +(notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type) ;; Some of the body part handlers leave point somewhere up in the ;; part, so we make sure that we're down at the end. (goto-char (point-max)) -- 1.7.9.1
[PATCH 0/4] emacs: show: lazy handling of hidden parts
This is a much better version of the WIP patch at id:1367628568-11656-1-git-send-email-markwalters1009 at gmail.com There was some discussion on irc about the new invisibility handling making large threads of messages with html parts slow to appear. This is caused by the new code rendering all of these parts and then hiding them. (I think this is exacerbated by the text/html having to fetch the part as a separate request: it is not in the notmuch show output). This code makes the rendering of all hidden parts lazy (ie it occurs when the part is shown). This should make the common case of hidden parts which are never viewed much faster. The code is relatively simple: we store all the arguments to the part insertion handler on the part button and then give them to the part handler when needed. This is not very memory efficient (we already store the whole message so we could extract it all again) but I think it is unlikely to be a problem in practice. The patch series looks very large but almost everything of interest happens in the final patch: the rest is code rearrangement. In particular patch 3/4 is entirely code rearrangement (and since it changes some indentation looks like it changes much more than it actually does). Testing is always helpful but there are two particular things that would be very useful: first, if anyone who has found it slow can see if this fixes it and secondly if anyone with encryption setup could test that the encryption buttons all work and look correct (I don't have encryption set up). In my small amount of testing it seems to work and all tests pass. Best wishes Mark Mark Walters (4): emacs:show: separate out handling of application/octet-stream emacs: show: handle inline patch fake parts at top level emacs: show: move the insertion of the header button to the top level emacs: show: implement lazy hidden part handling emacs/notmuch-show.el | 136 ++--- emacs/notmuch-wash.el |2 +- 2 files changed, 84 insertions(+), 54 deletions(-) -- 1.7.9.1 *** BLURB HERE *** Mark Walters (4): emacs:show: separate out handling of application/octet-stream emacs: show: handle inline patch fake parts at top level emacs: show: move the insertion of the header button to the top level emacs: show: implement lazy hidden part handling emacs/notmuch-show.el | 136 ++--- emacs/notmuch-wash.el |2 +- 2 files changed, 84 insertions(+), 54 deletions(-) -- 1.7.9.1
[PATCH 0/2] Enhancements to notmuch-hello search history
Jani Nikula writes: > On Fri, 03 May 2013, Servilio Afre Puentes wrote: >> Two patches that enhance the notmuch-hello search history UI. Though >> minor I find them very helpful. > > Both seem to work as advertised; I did not look at the code much. A > minor bikeshed is that I think y-or-n-p would suffice in patch 1/2. I know, but as it clears the whole history I decided to go with a confirmation that required more attention to diminish the chance of accident, and the action is taken not so oftenly (at least for me, and my guess is that it is the same for most) so it stills balances out positively in the usability. Servilio
[PATCH 0/2] Enhancements to notmuch-hello search history
On Fri, 03 May 2013, Servilio Afre Puentes wrote: > Two patches that enhance the notmuch-hello search history UI. Though > minor I find them very helpful. Both seem to work as advertised; I did not look at the code much. A minor bikeshed is that I think y-or-n-p would suffice in patch 1/2. BR, Jani.
Re: [PATCH 5/6] python: Add bindings for notmuch_thread_get_messages
Austin Clements writes: > --- > bindings/python/notmuch/thread.py | 27 ++- Justus OKed this privately. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] lib/database.cc: change how the parent of a message is calculated
Aaron Ecay writes:
> +if (ref && strcmp(ref, message_id)) {
> + return ref;
> +} else {
> + return NULL;
> +}
As a nit, there should be a space after strcmp. But if nobody has any
other comments, I could amend that.
d
[PATCH 1/2] test: add tests for the handling of References and In-Reply-To headers
Aaron Ecay writes: > These tests are known_broken, the following commit fixes them. > --- Sorry for the extra mails, but this patch also makes test/basic fail because thread-replies is not added to test/notmuch-test d
[PATCH 1/2] test: add tests for the handling of References and In-Reply-To headers
Aaron Ecay writes: > These tests are known_broken, the following commit fixes them. > --- On current master, I get FIXED Use In-Reply-To when no References d
Re: [PATCH 2/2] lib/database.cc: change how the parent of a message is calculated
LGTM
On Wed, 06 Mar 2013, Aaron Ecay wrote:
> Presently, the code which finds the parent of a message as it is being
> added to the database assumes that the first Message-ID-like substring
> of the In-Reply-To header is the parent Message ID. Some mail clients,
> however, put stuff other than the Message-ID of the parent in the
> In-Reply-To header, such as the email address of the sender of the
> parent. This can fool notmuch.
>
> The updated algorithm prefers the last Message ID in the References
> header. The References header lists messages oldest-first, so the last
> Message ID is the parent (RFC2822, p. 24). The References header is
> also less likely to be in a non-standard
> syntax (http://cr.yp.to/immhf/thread.html,
> http://www.jwz.org/doc/threading.html). In case the References header
> is not to be found, fall back to the old behavior.
>
> V2 of this patch, incorporating feedback from Jani and (indirectly)
> Austin.
> ---
> lib/database.cc | 48 +---
> test/thread-replies | 4
> 2 files changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 91d4329..52ed618 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -501,8 +501,10 @@ _parse_message_id (void *ctx, const char *message_id,
> const char **next)
> * 'message_id' in the result (to avoid mass confusion when a single
> * message references itself cyclically---and yes, mail messages are
> * not infrequent in the wild that do this---don't ask me why).
> -*/
> -static void
> + *
> + * Return the last reference parsed, if it is not equal to message_id.
> + */
> +static char *
> parse_references (void *ctx,
> const char *message_id,
> GHashTable *hash,
> @@ -511,7 +513,7 @@ parse_references (void *ctx,
> char *ref;
>
> if (refs == NULL || *refs == '\0')
> - return;
> + return NULL;
>
> while (*refs) {
> ref = _parse_message_id (ctx, refs, &refs);
> @@ -519,6 +521,17 @@ parse_references (void *ctx,
> if (ref && strcmp (ref, message_id))
> g_hash_table_insert (hash, ref, NULL);
> }
> +
> +/* The return value of this function is used to add a parent
> + * reference to the database. We should avoid making a message
> + * its own parent, thus the following check.
> + */
> +
> +if (ref && strcmp(ref, message_id)) {
> + return ref;
> +} else {
> + return NULL;
> +}
> }
>
> notmuch_status_t
> @@ -1510,28 +1523,33 @@ _notmuch_database_link_message_to_parents
> (notmuch_database_t *notmuch,
> {
> GHashTable *parents = NULL;
> const char *refs, *in_reply_to, *in_reply_to_message_id;
> +const char *last_ref_message_id, *this_message_id;
> GList *l, *keys = NULL;
> notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
>
> parents = g_hash_table_new_full (g_str_hash, g_str_equal,
>_my_talloc_free_for_g_hash, NULL);
> +this_message_id = notmuch_message_get_message_id (message);
>
> refs = notmuch_message_file_get_header (message_file, "references");
> -parse_references (message, notmuch_message_get_message_id (message),
> - parents, refs);
> +last_ref_message_id = parse_references (message,
> + this_message_id,
> + parents, refs);
>
> in_reply_to = notmuch_message_file_get_header (message_file,
> "in-reply-to");
> -parse_references (message, notmuch_message_get_message_id (message),
> - parents, in_reply_to);
> -
> -/* Carefully avoid adding any self-referential in-reply-to term. */
> -in_reply_to_message_id = _parse_message_id (message, in_reply_to, NULL);
> -if (in_reply_to_message_id &&
> - strcmp (in_reply_to_message_id,
> - notmuch_message_get_message_id (message)))
> -{
> +in_reply_to_message_id = parse_references (message,
> +this_message_id,
> +parents, in_reply_to);
> +
> +/* For the parent of this message, use the last message ID of the
> + * References header, if available. If not, fall back to the
> + * first message ID in the In-Reply-To header. */
> +if (last_ref_message_id) {
> +_notmuch_message_add_term (message, "replyto",
> + last_ref_message_id);
> +} else if (in_reply_to_message_id) {
> _notmuch_message_add_term (message, "replyto",
> - _parse_message_id (message, in_reply_to, NULL));
> + in_reply_to_message_id);
> }
>
> keys = g_hash_table_get_keys (parents);
> diff --git a/test/thread-replies b/test/thread-replies
> index a902691..28c2b1f 100755
> --- a/test/thread-replies
> +++ b/test/thread-replies
> @@ -11,7 +11,6 @@ constructed prope
Re: [PATCH 1/2] test: add tests for the handling of References and In-Reply-To headers
On Wed, 06 Mar 2013, Aaron Ecay wrote:
> These tests are known_broken, the following commit fixes them.
> ---
>
> Thanks to David and Tomi for pointing out test_expect_equal_json. In
> the process of implementing that, I discovered
> notmuch_json_show_sanitize, which I had also not been using. This
> patch fixes the test to use both these conveniences. The second patch
> is not changed substantively, but I am resending it for tidiness.
>
> test/thread-replies | 144
>
> 1 file changed, 144 insertions(+)
> create mode 100755 test/thread-replies
>
> diff --git a/test/thread-replies b/test/thread-replies
> new file mode 100755
> index 000..a902691
> --- /dev/null
> +++ b/test/thread-replies
> @@ -0,0 +1,144 @@
> +#!/usr/bin/env bash
> +#
> +# Copyright (c) 2013 Aaron Ecay
> +#
> +
> +test_description='test of proper handling of in-reply-to and references
> headers
> +
> +This test makes sure that the thread structure in the notmuch database is
> +constructed properly, even in the presence of non-RFC-compliant headers'
Nitpick, most other tests have one line test descriptions. The second
paragraph could be left as comment.
> +
> +. ./test-lib.sh
> +
> +test_begin_subtest "Use References when In-Reply-To is broken"
> +test_subtest_known_broken
> +add_message '[id]="f...@one.com"' \
> +'[subject]=one'
> +add_message '[in-reply-to]="mumble"' \
> +'[references]=""' \
> +'[subject]="Re: one"'
> +output=$(notmuch show --format=json 'subject:one' |
> notmuch_json_show_sanitize)
> +expected='[[[{"id": "f...@one.com",
> + "match": true,
> + "excluded": false,
> + "filename":
> "/home/aecay/development/notmuch/notmuch-git/src/notmuch/test/tmp.thread-replies/mail/msg-001",
Please replace the path with:
"filename": "Y",
Ditto below for all of them.
> + "timestamp": 978709437,
> + "date_relative": "2001-01-05",
> + "tags": ["inbox", "unread"],
> + "headers": {"Subject": "one",
> + "From": "Notmuch Test Suite ",
> + "To": "Notmuch Test Suite ",
> + "Date": "Fri, 05 Jan 2001 15:43:57 +"},
> + "body": [{"id": 1,
> + "content-type": "text/plain",
> + "content": "This is just a test message (#1)\n"}]},
> + [[{"id": "msg-002@notmuch-test-suite",
> + "match": true, "excluded": false,
> + "filename":
> "/home/aecay/development/notmuch/notmuch-git/src/notmuch/test/tmp.thread-replies/mail/msg-002",
> + "timestamp": 978709437, "date_relative": "2001-01-05",
> + "tags": ["inbox", "unread"], "headers": {"Subject": "Re: one",
> + "From": "Notmuch Test Suite ",
> + "To": "Notmuch Test Suite ",
> + "Date": "Fri, 05 Jan 2001 15:43:57 +"},
> + "body": [{"id": 1, "content-type": "text/plain",
> + "content": "This is just a test message (#2)\n"}]}, []]'
> +expected=`echo "$expected" | notmuch_json_show_sanitize`
> +test_expect_equal_json "$output" "$expected"
> +
> +test_begin_subtest "Prefer References to In-Reply-To"
> +test_subtest_known_broken
> +add_message '[id]="f...@two.com"' \
> +'[subject]=two'
> +add_message '[in-reply-to]=""' \
> +'[references]=""' \
> +'[subject]="Re: two"'
> +output=$(notmuch show --format=json 'subject:two' |
> notmuch_json_show_sanitize)
> +expected='[[[{"id": "f...@two.com",
> + "match": true, "excluded": false,
> + "filename":
> "/home/aecay/development/notmuch/notmuch-git/src/notmuch/test/tmp.thread-replies/mail/msg-003",
> + "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["inbox",
> "unread"],
> + "headers": {"Subject": "two",
> + "From": "Notmuch Test Suite ",
> + "To": "Notmuch Test Suite ",
> + "Date": "Fri, 05 Jan 2001 15:43:57 +"},
> + "body": [{"id": 1, "content-type": "text/plain",
> + "content": "This is just a test message (#3)\n"}]},
> + [[{"id": "msg-004@notmuch-test-suite", "match": true, "excluded": false,
> + "filename":
> "/home/aecay/development/notmuch/notmuch-git/src/notmuch/test/tmp.thread-replies/mail/msg-004",
> + "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["inbox",
> "unread"],
> + "headers": {"Subject": "Re: two",
> + "From": "Notmuch Test Suite ",
> + "To": "Notmuch Test Suite ",
> + "Date": "Fri, 05 Jan 2001 15:43:57 +"},
> + "body": [{"id": 1,
> + "content-type": "text/plain", "content": "This is just a test message
> (#4)\n"}]},
> + []]'
> +expected=`echo "$expected" | notmuch_json_show_sanitize`
> +test_expect_equal_json "$output" "$expected"
> +
> +test_begin_subtest "Use In-Reply-To when no References"
> +test_subtest_known_broken
As David said, this is not broken currently, as In-Reply-To is used
unconditionally. Just drop the broken annotation, the test itself is
useful.
> +add_message '[id]="f...@three.com"' \
> +'[subject]="three"'
> +add_message '[in-reply-to]=""' \
> +'[subject]="Re: three"'
> +output=$(notmuch show --format=json 'subject:three' |
> notmuch_json_show_sanitize)
> +expected='[[[{"id": "f...@three.com", "match": true, "excluded": false,
> + "filename":
> "/home/aecay/development/
Re: [PATCH 0/2] Enhancements to notmuch-hello search history
Jani Nikula writes: > On Fri, 03 May 2013, Servilio Afre Puentes wrote: >> Two patches that enhance the notmuch-hello search history UI. Though >> minor I find them very helpful. > > Both seem to work as advertised; I did not look at the code much. A > minor bikeshed is that I think y-or-n-p would suffice in patch 1/2. I know, but as it clears the whole history I decided to go with a confirmation that required more attention to diminish the chance of accident, and the action is taken not so oftenly (at least for me, and my guess is that it is the same for most) so it stills balances out positively in the usability. Servilio ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2] lib/database.cc: change how the parent of a message is calculated
Aaron Ecay writes:
> +if (ref && strcmp(ref, message_id)) {
> + return ref;
> +} else {
> + return NULL;
> +}
As a nit, there should be a space after strcmp. But if nobody has any
other comments, I could amend that.
d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] test: add tests for the handling of References and In-Reply-To headers
Aaron Ecay writes: > These tests are known_broken, the following commit fixes them. > --- Sorry for the extra mails, but this patch also makes test/basic fail because thread-replies is not added to test/notmuch-test d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] test: add tests for the handling of References and In-Reply-To headers
Aaron Ecay writes: > These tests are known_broken, the following commit fixes them. > --- On current master, I get FIXED Use In-Reply-To when no References d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] emacs: show: implement lazy hidden part handling
This adds the actual code to do the lazy insertion of hidden parts. We use a memory inefficient but simple method: when we come to insert the part if it is hidden we just store all of the arguments to the part insertion function as a button property. This means when we want to show the part we can just resume where we left off. The only slight subtlety/hack is that to simplify the handling of the invisibility overlay (for the hiding unhiding later) we do insert some dummy text which we remove when we show the part. --- emacs/notmuch-show.el | 32 ++-- 1 files changed, 30 insertions(+), 2 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 2c48b24..a14a135 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -559,6 +559,7 @@ message at DEPTH in the current thread." (overlay (button-get button 'overlay))) (when overlay (let* ((show (overlay-get overlay 'invisible)) +(lazy-part (button-get button :notmuch-lazy-part)) (new-start (button-start button)) (button-label (button-get button :base-label)) (old-point (point)) @@ -569,7 +570,11 @@ message at DEPTH in the current thread." (let ((old-end (button-end button))) (move-overlay button new-start (point)) (delete-region (point) old-end)) - (goto-char (min old-point (1- (button-end button + (goto-char (min old-point (1- (button-end button + (when (and show lazy-part) + (save-excursion + (button-put button :notmuch-lazy-part nil) + (notmuch-show-lazy-part lazy-part button))) (defun notmuch-show-multipart/*-to-list (part) (mapcar (lambda (inner-part) (plist-get inner-part :content-type)) @@ -857,6 +862,24 @@ message at DEPTH in the current thread." (setq handlers (cdr handlers t) +(defun notmuch-show-lazy-part (part-args button) + (interactive) + ;; We have to save the depth as we can't find the depth when narrowed + (let ((inhibit-read-only t) + (overlay (button-get button 'overlay)) + (depth (notmuch-show-get-depth))) +(save-restriction + (narrow-to-region (overlay-start overlay) (1- (overlay-end overlay))) + (delete-region (overlay-start overlay) (1- (overlay-end overlay))) + (goto-char (overlay-start overlay)) + (apply #'notmuch-show-insert-bodypart-internal (nconc part-args (list button))) + (indent-rigidly (overlay-start overlay) + (1- (overlay-end overlay)) + depth)) +;; We deferred deleting this character to simplify handling of the +;; overlay: all of the above takes place inside the overlay. +(delete-region (1- (overlay-end overlay)) (overlay-end overlay + (defun notmuch-show-create-part-overlays (msg beg end hide) "Add an overlay to the part between BEG and END" (let* ((button (button-at beg)) @@ -891,7 +914,12 @@ If HIDE is non-nil then initially hide this part." (button (unless (string= mime-type "text/plain") (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename) -(notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button) +(if (not hide) +(notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button) + (insert "lazy part") + (button-put button :notmuch-lazy-part + (list msg part mime-type nth depth content-type))) + ;; Some of the body part handlers leave point somewhere up in the ;; part, so we make sure that we're down at the end. (goto-char (point-max)) -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/4] emacs: show: move the insertion of the header button to the top level
Previously each of the part insertion handlers inserted the part button themselves. Move this up into notmuch-show-insert-bodypart. Since a small number of the handlers modify the button (the encryption/signature ones) we need to pass the header button as an argument into the individual part insertion handlers. The patch is large but mostly simple. The only things of note are that we let the text/plain handler insert part buttons itself (as it does not always insert one and it applies motmuch-wash to the whole part including the button. Also this is by far the most important case so this reduces the risk of annoying side effects. The handler for inline-patch-fake-part is called from notmuch wash not from notmuch-show-insert-bodypart so that needs to insert the header button itself (rather than relying on notmuch-show-insert-part-*/*). --- emacs/notmuch-show.el | 91 +++- 1 files changed, 44 insertions(+), 47 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index f984143..2c48b24 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -575,8 +575,7 @@ message at DEPTH in the current thread." (mapcar (lambda (inner-part) (plist-get inner-part :content-type)) (plist-get part :content))) -(defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type) - (notmuch-show-insert-part-header nth declared-type content-type nil) +(defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type button) (let ((chosen-type (car (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part (inner-parts (plist-get part :content)) (start (point))) @@ -653,8 +652,7 @@ message at DEPTH in the current thread." content-type) nil))) -(defun notmuch-show-insert-part-multipart/related (msg part content-type nth depth declared-type) - (notmuch-show-insert-part-header nth declared-type content-type nil) +(defun notmuch-show-insert-part-multipart/related (msg part content-type nth depth declared-type button) (let ((inner-parts (plist-get part :content)) (start (point))) @@ -673,16 +671,15 @@ message at DEPTH in the current thread." (indent-rigidly start (point) 1))) t) -(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type) - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil))) -(button-put button 'face 'notmuch-crypto-part-header) -;; add signature status button if sigstatus provided -(if (plist-member part :sigstatus) - (let* ((from (notmuch-show-get-header :From msg)) - (sigstatus (car (plist-get part :sigstatus - (notmuch-crypto-insert-sigstatus-button sigstatus from)) - ;; if we're not adding sigstatus, tell the user how they can get it - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))) +(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type button) + (button-put button 'face 'notmuch-crypto-part-header) + ;; add signature status button if sigstatus provided + (if (plist-member part :sigstatus) + (let* ((from (notmuch-show-get-header :From msg)) +(sigstatus (car (plist-get part :sigstatus + (notmuch-crypto-insert-sigstatus-button sigstatus from)) +;; if we're not adding sigstatus, tell the user how they can get it +(button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")) (let ((inner-parts (plist-get part :content)) (start (point))) @@ -695,20 +692,19 @@ message at DEPTH in the current thread." (indent-rigidly start (point) 1))) t) -(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type) - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil))) -(button-put button 'face 'notmuch-crypto-part-header) -;; add encryption status button if encstatus specified -(if (plist-member part :encstatus) - (let ((encstatus (car (plist-get part :encstatus - (notmuch-crypto-insert-encstatus-button encstatus) - ;; add signature status button if sigstatus specified - (if (plist-member part :sigstatus) - (let* ((from (notmuch-show-get-header :From msg)) -(sigstatus (car (plist-get part :sigstatus - (notmuch-crypto-insert-sigstatus-button sigstatus from - ;; if we're not adding encstatus, tell the user how they can get it - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))) +(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type button) + (button-put button 'face 'notmuch-crypto-par
[PATCH 2/4] emacs: show: handle inline patch fake parts at top level
The inline patch fake part handler also modifies the content-type so handle this in notmuch-show-insert-bodypart too. --- emacs/notmuch-show.el |4 +++- emacs/notmuch-wash.el |2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 3b9a2ad..f984143 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -814,7 +814,7 @@ message at DEPTH in the current thread." ;; Handler for wash generated inline patch fake parts. (defun notmuch-show-insert-part-inline-patch-fake-part (msg part content-type nth depth declared-type) - (notmuch-show-insert-part-*/* msg part "text/x-diff" nth depth "inline patch")) + (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type)) (defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type) ;; text/html handler to work around bugs in renderers and our @@ -888,6 +888,8 @@ If HIDE is non-nil then initially hide this part." (let* ((content-type (downcase (plist-get part :content-type))) (mime-type (or (and (string= content-type "application/octet-stream") (notmuch-show-get-mime-type-of-application/octet-stream part)) + (and (string= content-type "inline patch") +"text/x-diff") content-type)) (nth (plist-get part :id)) (beg (point))) diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el index 80c475c..8a68819 100644 --- a/emacs/notmuch-wash.el +++ b/emacs/notmuch-wash.el @@ -365,7 +365,7 @@ for error." (setq patch-end (match-beginning 0))) (save-restriction (narrow-to-region patch-start patch-end) - (setq part (plist-put part :content-type "inline-patch-fake-part")) + (setq part (plist-put part :content-type "inline patch")) (setq part (plist-put part :content (buffer-string))) (setq part (plist-put part :id -1)) (setq part (plist-put part :filename -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/4] emacs:show: separate out handling of application/octet-stream
Currently mime parts are basically handled based on their mime-type with the exception of application/octet-stream parts. Deal with these parts at the top level (notmuch-show-insert-bodypart). This is needed later in the series as we need to put in a part button for each part (which means knowing its mime type) while deferring the actual insertion of the part. --- emacs/notmuch-show.el | 19 +++ 1 files changed, 11 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index face2a0..3b9a2ad 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -796,9 +796,9 @@ message at DEPTH in the current thread." (defun notmuch-show-insert-part-text/x-vcalendar (msg part content-type nth depth declared-type) (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type)) -(defun notmuch-show-insert-part-application/octet-stream (msg part content-type nth depth declared-type) +(defun notmuch-show-get-mime-type-of-application/octet-stream (part) ;; If we can deduce a MIME type from the filename of the attachment, - ;; do so and pass it on to the handler for that type. + ;; we return that. (if (plist-get part :filename) (let ((extension (file-name-extension (plist-get part :filename))) mime-type) @@ -808,7 +808,7 @@ message at DEPTH in the current thread." (setq mime-type (mailcap-extension-to-mime extension)) (if (and mime-type (not (string-equal mime-type "application/octet-stream"))) - (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type) + mime-type nil)) nil @@ -885,11 +885,14 @@ message at DEPTH in the current thread." "Insert the body part PART at depth DEPTH in the current thread. If HIDE is non-nil then initially hide this part." - (let ((content-type (downcase (plist-get part :content-type))) - (nth (plist-get part :id)) - (beg (point))) - -(notmuch-show-insert-bodypart-internal msg part content-type nth depth content-type) + (let* ((content-type (downcase (plist-get part :content-type))) +(mime-type (or (and (string= content-type "application/octet-stream") + (notmuch-show-get-mime-type-of-application/octet-stream part)) + content-type)) +(nth (plist-get part :id)) +(beg (point))) + +(notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type) ;; Some of the body part handlers leave point somewhere up in the ;; part, so we make sure that we're down at the end. (goto-char (point-max)) -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 0/4] emacs: show: lazy handling of hidden parts
This is a much better version of the WIP patch at id:1367628568-11656-1-git-send-email-markwalters1...@gmail.com There was some discussion on irc about the new invisibility handling making large threads of messages with html parts slow to appear. This is caused by the new code rendering all of these parts and then hiding them. (I think this is exacerbated by the text/html having to fetch the part as a separate request: it is not in the notmuch show output). This code makes the rendering of all hidden parts lazy (ie it occurs when the part is shown). This should make the common case of hidden parts which are never viewed much faster. The code is relatively simple: we store all the arguments to the part insertion handler on the part button and then give them to the part handler when needed. This is not very memory efficient (we already store the whole message so we could extract it all again) but I think it is unlikely to be a problem in practice. The patch series looks very large but almost everything of interest happens in the final patch: the rest is code rearrangement. In particular patch 3/4 is entirely code rearrangement (and since it changes some indentation looks like it changes much more than it actually does). Testing is always helpful but there are two particular things that would be very useful: first, if anyone who has found it slow can see if this fixes it and secondly if anyone with encryption setup could test that the encryption buttons all work and look correct (I don't have encryption set up). In my small amount of testing it seems to work and all tests pass. Best wishes Mark Mark Walters (4): emacs:show: separate out handling of application/octet-stream emacs: show: handle inline patch fake parts at top level emacs: show: move the insertion of the header button to the top level emacs: show: implement lazy hidden part handling emacs/notmuch-show.el | 136 ++--- emacs/notmuch-wash.el |2 +- 2 files changed, 84 insertions(+), 54 deletions(-) -- 1.7.9.1 *** BLURB HERE *** Mark Walters (4): emacs:show: separate out handling of application/octet-stream emacs: show: handle inline patch fake parts at top level emacs: show: move the insertion of the header button to the top level emacs: show: implement lazy hidden part handling emacs/notmuch-show.el | 136 ++--- emacs/notmuch-wash.el |2 +- 2 files changed, 84 insertions(+), 54 deletions(-) -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 0/2] Enhancements to notmuch-hello search history
On Fri, 03 May 2013, Servilio Afre Puentes wrote: > Two patches that enhance the notmuch-hello search history UI. Though > minor I find them very helpful. Both seem to work as advertised; I did not look at the code much. A minor bikeshed is that I think y-or-n-p would suffice in patch 1/2. BR, Jani. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[WIP] emacs: show: lazily insert html parts
The current code renders all html parts by default and then hides them. This is quite slow so this is an attempt to make it only render them when the user chooses to show them. In my testing this code seems to work but has some rough edges. However, it may be useful to people in its current form (and I am not sure when I will have enough time to do more). There are various problems with the current approach and implementation: It special cases text/html (rather than lazily rendering all initially hidden parts). If you toggle some multipart parts (the whole multipart part not the subpart) then you can end up with the html part claiming to be visible but not rendered. However toggling the html part twice fixes this. (If I didn't special case text/html this might just work.) It may get the depth wrong by 1 if the text/html is part of a multipart part. I think it adds an extra blank line at the end of the newly rendered html part. If I fix this then the following message can get messed up (this newly render part can end up inside the following header overlay). Maybe someone more familiar with overlays can see how to fix this easily. It just saves all the arguments which would have been passed to the html rendering function to the part-button and then calls the rendering function with them later. Given that we store the entire message json this may not be necessary. On the other hand this does make the code relatively simple. Any feedback on whether it works, much better ways of doing things etc gratefully received! Best wishes Mark --- emacs/notmuch-show.el | 27 --- 1 files changed, 24 insertions(+), 3 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index face2a0..6749bc9 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -559,6 +559,7 @@ message at DEPTH in the current thread." (overlay (button-get button 'overlay))) (when overlay (let* ((show (overlay-get overlay 'invisible)) +(unparsed-html (button-get button :notmuch-html-properties)) (new-start (button-start button)) (button-label (button-get button :base-label)) (old-point (point)) @@ -569,7 +570,11 @@ message at DEPTH in the current thread." (let ((old-end (button-end button))) (move-overlay button new-start (point)) (delete-region (point) old-end)) - (goto-char (min old-point (1- (button-end button + (goto-char (min old-point (1- (button-end button + (when (and show unparsed-html) + (save-excursion + (button-put button :notmuch-html-properties nil) + (notmuch-show-lazy-html-part unparsed-html overlay))) (defun notmuch-show-multipart/*-to-list (part) (mapcar (lambda (inner-part) (plist-get inner-part :content-type)) @@ -823,8 +828,24 @@ message at DEPTH in the current thread." ;; in notmuch. We set mm-inline-text-html-with-w3m-keymap to nil to ;; tell w3m not to set a keymap (so the normal notmuch-show-mode-map ;; remains). - (let ((mm-inline-text-html-with-w3m-keymap nil)) -(notmuch-show-insert-part-*/* msg part content-type nth depth declared-type))) + (let ((button (notmuch-show-insert-part-header nth content-type declared-type (plist-get part :filename +(button-put button :notmuch-html-properties + (list msg part nth content-type notmuch-show-process-crypto))) + ;; We need to insert something or the part toggle gets suppressed. + (save-excursion + (insert "html")) + t) + +(defun notmuch-show-lazy-html-part (html overlay) + (interactive) + (let ((mm-inline-text-html-with-w3m-keymap nil) + (inhibit-read-only t)) +(delete-region (overlay-start overlay) (1- (overlay-end overlay))) +(apply #'notmuch-mm-display-part-inline html)) + (indent-rigidly (overlay-start overlay) + (1- (overlay-end overlay)) + (notmuch-show-get-depth))) + (defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type) ;; This handler _must_ succeed - it is the handler of last resort. -- 1.7.9.1
