* [PATCH v2 0/3] emacs: show: lazy handling of hidden parts @ 2013-05-26 7:57 Mark Walters 2013-05-26 7:57 ` [PATCH v2 1/3] emacs: show: fake wash parts are handled at insert-bodypart level Mark Walters ` (3 more replies) 0 siblings, 4 replies; 7+ messages in thread From: Mark Walters @ 2013-05-26 7:57 UTC (permalink / raw) To: notmuch This is a slightly tweaked version of id:1367672478-12247-1-git-send-email-markwalters1009@gmail.com minus the first two patches which have already been pushed. The second patch of the previous series obsoleted the handler notmuch-show-insert-part-inline-patch-fake-part so we remove that and we update the commit message of the second patch in this series to but otherwise patches 2 and 3 of this are identical to patches 3 and 4 of the previous series. There is one change that inadvertently slipped into the second patch of the previous series: it changed the (fake) name of the fake inline patch parts from inline-patch-fake-part to "inline patch". (I think this was when I was testing whether I could do away with the notmuch-show-insert-part-inline-patch-fake-part handler but I forgot to revert it after). The only place it shows to a user is in the part button title: it will say something like [ 0001-emacs-show-separate-out-handling-of-application-octe.patch: inline patch (as text/x-diff) ] instead of [ 0001-emacs-show-separate-out-handling-of-application-octe.patch: inline-patch-fake-part (as text/x-diff) ] but perhaps it should be reverted anyway. Sorry for messing that bit up Best wishes Mark Mark Walters (3): emacs: show: fake wash parts are handled at insert-bodypart 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 | 121 +++++++++++++++++++++++++++++-------------------- 1 files changed, 71 insertions(+), 50 deletions(-) -- 1.7.9.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] emacs: show: fake wash parts are handled at insert-bodypart level 2013-05-26 7:57 [PATCH v2 0/3] emacs: show: lazy handling of hidden parts Mark Walters @ 2013-05-26 7:57 ` Mark Walters 2013-05-26 7:57 ` [PATCH v2 2/3] emacs: show: move the insertion of the header button to the top level Mark Walters ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Mark Walters @ 2013-05-26 7:57 UTC (permalink / raw) To: notmuch Earlier patches have moved the handling of wash fake inline patch parts to insert-bodypart so we can drop the function notmuch-show-insert-part-inline-patch-fake-part --- emacs/notmuch-show.el | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index d56154e..51bda31 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -813,10 +813,6 @@ message at DEPTH in the current thread." nil)) nil)))) -;; 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 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 ;; invisibile parts code. In particular w3m sets up a keymap which -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] emacs: show: move the insertion of the header button to the top level 2013-05-26 7:57 [PATCH v2 0/3] emacs: show: lazy handling of hidden parts Mark Walters 2013-05-26 7:57 ` [PATCH v2 1/3] emacs: show: fake wash parts are handled at insert-bodypart level Mark Walters @ 2013-05-26 7:57 ` Mark Walters 2013-05-30 22:30 ` Austin Clements 2013-05-26 7:57 ` [PATCH v2 3/3] emacs: show: implement lazy hidden part handling Mark Walters 2013-05-31 4:16 ` [PATCH v2 0/3] emacs: show: lazy handling of hidden parts Adam Wolfe Gordon 3 siblings, 1 reply; 7+ messages in thread From: Mark Walters @ 2013-05-26 7:57 UTC (permalink / raw) To: notmuch 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. --- emacs/notmuch-show.el | 87 +++++++++++++++++++++++------------------------- 1 files changed, 42 insertions(+), 45 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 51bda31..591ad56 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -576,8 +576,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))) @@ -654,8 +653,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))) @@ -674,16 +672,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))) @@ -696,20 +693,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-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.")) (let ((inner-parts (plist-get part :content)) (start (point))) @@ -722,8 +718,7 @@ message at DEPTH in the current thread." (indent-rigidly start (point) 1))) t) -(defun notmuch-show-insert-part-multipart/* (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/* (msg part content-type nth depth declared-type button) (let ((inner-parts (plist-get part :content)) (start (point))) ;; Show all of the parts. @@ -735,8 +730,7 @@ message at DEPTH in the current thread." (indent-rigidly start (point) 1))) t) -(defun notmuch-show-insert-part-message/rfc822 (msg part content-type nth depth declared-type) - (notmuch-show-insert-part-header nth declared-type content-type nil) +(defun notmuch-show-insert-part-message/rfc822 (msg part content-type nth depth declared-type button) (let* ((message (car (plist-get part :content))) (body (car (plist-get message :body))) (start (point))) @@ -757,7 +751,7 @@ message at DEPTH in the current thread." (indent-rigidly start (point) 1))) t) -(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type) +(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type button) (let ((start (point))) ;; If this text/plain part is not the first part in the message, ;; insert a header to make this clear. @@ -770,8 +764,7 @@ message at DEPTH in the current thread." (run-hook-with-args 'notmuch-show-insert-text/plain-hook msg depth)))) t) -(defun notmuch-show-insert-part-text/calendar (msg part content-type nth depth declared-type) - (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)) +(defun notmuch-show-insert-part-text/calendar (msg part content-type nth depth declared-type button) (insert (with-temp-buffer (insert (notmuch-get-bodypart-content msg part nth notmuch-show-process-crypto)) ;; notmuch-get-bodypart-content provides "raw", non-converted @@ -794,8 +787,8 @@ message at DEPTH in the current thread." t) ;; For backwards compatibility. -(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-text/x-vcalendar (msg part content-type nth depth declared-type button) + (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type button)) (defun notmuch-show-get-mime-type-of-application/octet-stream (part) ;; If we can deduce a MIME type from the filename of the attachment, @@ -813,7 +806,7 @@ message at DEPTH in the current thread." nil)) nil)))) -(defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type) +(defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type button) ;; text/html handler to work around bugs in renderers and our ;; invisibile parts code. In particular w3m sets up a keymap which ;; "leaks" outside the invisible region and causes strange effects @@ -821,11 +814,10 @@ message at DEPTH in the current thread." ;; 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))) + (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type button))) -(defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type) +(defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type button) ;; This handler _must_ succeed - it is the handler of last resort. - (notmuch-show-insert-part-header nth content-type declared-type (plist-get part :filename)) (notmuch-mm-display-part-inline msg part nth content-type notmuch-show-process-crypto) t) @@ -848,13 +840,13 @@ message at DEPTH in the current thread." ;; \f -(defun notmuch-show-insert-bodypart-internal (msg part content-type nth depth declared-type) +(defun notmuch-show-insert-bodypart-internal (msg part content-type nth depth declared-type button) (let ((handlers (notmuch-show-handlers-for content-type))) ;; Run the content handlers until one of them returns a non-nil ;; value. (while (and handlers (not (condition-case err - (funcall (car handlers) msg part content-type nth depth declared-type) + (funcall (car handlers) msg part content-type nth depth declared-type button) (error (progn (insert "!!! Bodypart insert error: ") (insert (error-message-string err)) @@ -882,6 +874,9 @@ 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." + + ;; We handle text/plain specially as its code does things before + ;; inserting a part button (and does not always insert a part button). (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)) @@ -889,9 +884,11 @@ If HIDE is non-nil then initially hide this part." "text/x-diff") content-type)) (nth (plist-get part :id)) - (beg (point))) + (beg (point)) + (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) + (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button) ;; 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] emacs: show: move the insertion of the header button to the top level 2013-05-26 7:57 ` [PATCH v2 2/3] emacs: show: move the insertion of the header button to the top level Mark Walters @ 2013-05-30 22:30 ` Austin Clements 0 siblings, 0 replies; 7+ messages in thread From: Austin Clements @ 2013-05-30 22:30 UTC (permalink / raw) To: Mark Walters, notmuch On Sun, 26 May 2013, Mark Walters <markwalters1009@gmail.com> wrote: > 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. > --- > emacs/notmuch-show.el | 87 +++++++++++++++++++++++------------------------- > 1 files changed, 42 insertions(+), 45 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 51bda31..591ad56 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -576,8 +576,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))) > @@ -654,8 +653,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))) > > @@ -674,16 +672,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))) > @@ -696,20 +693,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-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.")) > > (let ((inner-parts (plist-get part :content)) > (start (point))) > @@ -722,8 +718,7 @@ message at DEPTH in the current thread." > (indent-rigidly start (point) 1))) > t) > > -(defun notmuch-show-insert-part-multipart/* (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/* (msg part content-type nth depth declared-type button) > (let ((inner-parts (plist-get part :content)) > (start (point))) > ;; Show all of the parts. > @@ -735,8 +730,7 @@ message at DEPTH in the current thread." > (indent-rigidly start (point) 1))) > t) > > -(defun notmuch-show-insert-part-message/rfc822 (msg part content-type nth depth declared-type) > - (notmuch-show-insert-part-header nth declared-type content-type nil) > +(defun notmuch-show-insert-part-message/rfc822 (msg part content-type nth depth declared-type button) > (let* ((message (car (plist-get part :content))) > (body (car (plist-get message :body))) > (start (point))) > @@ -757,7 +751,7 @@ message at DEPTH in the current thread." > (indent-rigidly start (point) 1))) > t) > > -(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type) > +(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type button) > (let ((start (point))) > ;; If this text/plain part is not the first part in the message, > ;; insert a header to make this clear. I know you're trying to minimize side-effects, but I think this will also complicate maintenance. I'd rather see this call to notmuch-show-insert-part-header removed and have the only call and all special-case logic be in notmuch-show-insert-bodypart. As it is, reasoning about exactly when a part button is inserted requires understanding the conjunction of two widely separated parts of the code. > @@ -770,8 +764,7 @@ message at DEPTH in the current thread." > (run-hook-with-args 'notmuch-show-insert-text/plain-hook msg depth)))) > t) > > -(defun notmuch-show-insert-part-text/calendar (msg part content-type nth depth declared-type) > - (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)) > +(defun notmuch-show-insert-part-text/calendar (msg part content-type nth depth declared-type button) > (insert (with-temp-buffer > (insert (notmuch-get-bodypart-content msg part nth notmuch-show-process-crypto)) > ;; notmuch-get-bodypart-content provides "raw", non-converted > @@ -794,8 +787,8 @@ message at DEPTH in the current thread." > t) > > ;; For backwards compatibility. > -(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-text/x-vcalendar (msg part content-type nth depth declared-type button) > + (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type button)) > > (defun notmuch-show-get-mime-type-of-application/octet-stream (part) > ;; If we can deduce a MIME type from the filename of the attachment, > @@ -813,7 +806,7 @@ message at DEPTH in the current thread." > nil)) > nil)))) > > -(defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type) > +(defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type button) > ;; text/html handler to work around bugs in renderers and our > ;; invisibile parts code. In particular w3m sets up a keymap which > ;; "leaks" outside the invisible region and causes strange effects > @@ -821,11 +814,10 @@ message at DEPTH in the current thread." > ;; 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))) > + (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type button))) > > -(defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type) > +(defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type button) > ;; This handler _must_ succeed - it is the handler of last resort. > - (notmuch-show-insert-part-header nth content-type declared-type (plist-get part :filename)) > (notmuch-mm-display-part-inline msg part nth content-type notmuch-show-process-crypto) > t) > > @@ -848,13 +840,13 @@ message at DEPTH in the current thread." > > ;; \f > > -(defun notmuch-show-insert-bodypart-internal (msg part content-type nth depth declared-type) > +(defun notmuch-show-insert-bodypart-internal (msg part content-type nth depth declared-type button) > (let ((handlers (notmuch-show-handlers-for content-type))) > ;; Run the content handlers until one of them returns a non-nil > ;; value. > (while (and handlers > (not (condition-case err > - (funcall (car handlers) msg part content-type nth depth declared-type) > + (funcall (car handlers) msg part content-type nth depth declared-type button) > (error (progn > (insert "!!! Bodypart insert error: ") > (insert (error-message-string err)) > @@ -882,6 +874,9 @@ 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." > + > + ;; We handle text/plain specially as its code does things before > + ;; inserting a part button (and does not always insert a part button). I think this comment would be clearer right above the button binding (or maybe the separate diff hunks just make it more confusing than it is). > (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)) > @@ -889,9 +884,11 @@ If HIDE is non-nil then initially hide this part." > "text/x-diff") > content-type)) > (nth (plist-get part :id)) > - (beg (point))) > + (beg (point)) > + (button (unless (string= mime-type "text/plain") > + (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))) If you take my suggestion above, this would become something like (unless (and (string= mime-type "text/plain) (<= nth 1)) ...) or maybe clearer (when (or (not (string= mime-type "text/plain")) (> nth 1)) ...) > > - (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type) > + (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button) > ;; 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] emacs: show: implement lazy hidden part handling 2013-05-26 7:57 [PATCH v2 0/3] emacs: show: lazy handling of hidden parts Mark Walters 2013-05-26 7:57 ` [PATCH v2 1/3] emacs: show: fake wash parts are handled at insert-bodypart level Mark Walters 2013-05-26 7:57 ` [PATCH v2 2/3] emacs: show: move the insertion of the header button to the top level Mark Walters @ 2013-05-26 7:57 ` Mark Walters 2013-05-30 23:28 ` Austin Clements 2013-05-31 4:16 ` [PATCH v2 0/3] emacs: show: lazy handling of hidden parts Adam Wolfe Gordon 3 siblings, 1 reply; 7+ messages in thread From: Mark Walters @ 2013-05-26 7:57 UTC (permalink / raw) To: notmuch 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 591ad56..94faa9b 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -560,6 +560,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)) @@ -570,7 +571,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)) @@ -854,6 +859,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)) @@ -888,7 +911,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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] emacs: show: implement lazy hidden part handling 2013-05-26 7:57 ` [PATCH v2 3/3] emacs: show: implement lazy hidden part handling Mark Walters @ 2013-05-30 23:28 ` Austin Clements 0 siblings, 0 replies; 7+ messages in thread From: Austin Clements @ 2013-05-30 23:28 UTC (permalink / raw) To: Mark Walters, notmuch On Sun, 26 May 2013, Mark Walters <markwalters1009@gmail.com> wrote: > 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 591ad56..94faa9b 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -560,6 +560,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)) > @@ -570,7 +571,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)) > @@ -854,6 +859,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))) Is there a reason 'button' couldn't be included in parts-args when parts-args was originally constructed below? (If so, then this probably deserves a comment.) > + (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)) > @@ -888,7 +911,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") The insert is worth a comment. Or fixing if it's not too difficult. > + (button-put button :notmuch-lazy-part > + (list msg part mime-type nth depth content-type))) > + To summarize the IRC discussion for the record, this changes the behavior of lazily inserted parts that we can't render. Currently the only way we know if we can render a part is by attempting to render it. As a result, lazily inserted parts that we can't render that would previously have offered to save the part when the part button was selected will now attempt to render the part and simply not display anything. > ;; 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] emacs: show: lazy handling of hidden parts 2013-05-26 7:57 [PATCH v2 0/3] emacs: show: lazy handling of hidden parts Mark Walters ` (2 preceding siblings ...) 2013-05-26 7:57 ` [PATCH v2 3/3] emacs: show: implement lazy hidden part handling Mark Walters @ 2013-05-31 4:16 ` Adam Wolfe Gordon 3 siblings, 0 replies; 7+ messages in thread From: Adam Wolfe Gordon @ 2013-05-31 4:16 UTC (permalink / raw) To: Mark Walters; +Cc: Notmuch Mail On Sun, May 26, 2013 at 1:57 AM, Mark Walters <markwalters1009@gmail.com> wrote: > This is a slightly tweaked version of > id:1367672478-12247-1-git-send-email-markwalters1009@gmail.com minus > the first two patches which have already been pushed. From a quick read-through, this series looks good to me. I applied the patches to my build today and they improve the performance of show substantially. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-05-31 4:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-26 7:57 [PATCH v2 0/3] emacs: show: lazy handling of hidden parts Mark Walters 2013-05-26 7:57 ` [PATCH v2 1/3] emacs: show: fake wash parts are handled at insert-bodypart level Mark Walters 2013-05-26 7:57 ` [PATCH v2 2/3] emacs: show: move the insertion of the header button to the top level Mark Walters 2013-05-30 22:30 ` Austin Clements 2013-05-26 7:57 ` [PATCH v2 3/3] emacs: show: implement lazy hidden part handling Mark Walters 2013-05-30 23:28 ` Austin Clements 2013-05-31 4:16 ` [PATCH v2 0/3] emacs: show: lazy handling of hidden parts Adam Wolfe Gordon
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).