[PATCH v5 0/6] emacs: show: lazy handling of hidden parts

[PATCH v5 0/6] emacs: show: lazy handling of hidden parts

[Mark Walters]

This is version 5 of this patch set. Version 4 is at
id:1370074547-24677-1-git-send-email-markwalters1009@gmail.com.

This version fixes the two bugs pointed out in Austin's review
id:20130610023952.GC22196@mit.edu.

I decided to add the :notmuch-part text property separately from
create-overlays as they have slightly different regions (one includes
the button one does not) and are applied at slightly different times
(we do not create overlays for lazy parts).

I was not sure whether we created two overlays for hidden lazy parts
in v4 (it might not have done as the part was empty and we do not
create overlays for empty parts) but that is stupidly fragile. Thus we
explicitly do not create overlays for lazy parts until they are
inserted.

The diff from v4 is below.

Best wishes

Mark


Mark Walters (6):
  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: pass button to create-overlays
  emacs: show: modify the way hidden state is recorded.
  emacs: show move addition of :notmuch-part to separate function
  emacs: show: implement lazy hidden part handling

 emacs/notmuch-show.el |  241 ++++++++++++++++++++++++++++++-------------------
 1 files changed, 146 insertions(+), 95 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index b963859..d030ea3 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -817,6 +817,27 @@ message at DEPTH in the current thread."
     ;; Return true if we created an overlay.
     t))
 
+(defun notmuch-show-record-part-information (part beg end)
+  "Store PART as a text property from BEG to END"
+
+  ;; Record part information.  Since we already inserted subparts,
+  ;; don't override existing :notmuch-part properties.
+  (notmuch-map-text-property beg end :notmuch-part
+			     (lambda (v) (or v part)))
+  ;; Make :notmuch-part front sticky and rear non-sticky so it stays
+  ;; applied to the beginning of each line when we indent the
+  ;; message.  Since we're operating on arbitrary renderer output,
+  ;; watch out for sticky specs of t, which means all properties are
+  ;; front-sticky/rear-nonsticky.
+  (notmuch-map-text-property beg end 'front-sticky
+			     (lambda (v) (if (listp v)
+					     (pushnew :notmuch-part v)
+					   v)))
+  (notmuch-map-text-property beg end 'rear-nonsticky
+			     (lambda (v) (if (listp v)
+					     (pushnew :notmuch-part v)
+					   v))))
+
 (defun notmuch-show-lazy-part (part-args button)
   ;; Insert the lazy part after the button for the part. We would just
   ;; move to the start of the new line following the button and insert
@@ -843,6 +864,9 @@ message at DEPTH in the current thread."
 	(indent-rigidly part-beg part-end depth))
       (goto-char part-end)
       (delete-char 1)
+      (notmuch-show-record-part-information (second part-args)
+					    (button-start button)
+					    part-end)
       ;; Create the overlay. If the lazy-part turned out to be empty/not
       ;; showable this returns nil.
       (notmuch-show-create-part-overlays button part-beg part-end))))
@@ -876,27 +900,13 @@ If HIDE is non-nil then initially hide this part."
     ;; Ensure that the part ends with a carriage return.
     (unless (bolp)
       (insert "\n"))
-    (notmuch-show-create-part-overlays button content-beg (point))
-    (when hide
+    ;; We do not create the overlay for hidden (lazy) parts until
+    ;; they are inserted.
+    (if (not hide)
+	(notmuch-show-create-part-overlays button content-beg (point))
       (save-excursion
 	(notmuch-show-toggle-part-invisibility button)))
-    ;; Record part information.  Since we already inserted subparts,
-    ;; don't override existing :notmuch-part properties.
-    (notmuch-map-text-property beg (point) :notmuch-part
-			       (lambda (v) (or v part)))
-    ;; Make :notmuch-part front sticky and rear non-sticky so it stays
-    ;; applied to the beginning of each line when we indent the
-    ;; message.  Since we're operating on arbitrary renderer output,
-    ;; watch out for sticky specs of t, which means all properties are
-    ;; front-sticky/rear-nonsticky.
-    (notmuch-map-text-property beg (point) 'front-sticky
-			       (lambda (v) (if (listp v)
-					       (pushnew :notmuch-part v)
-					     v)))
-    (notmuch-map-text-property beg (point) 'rear-nonsticky
-			       (lambda (v) (if (listp v)
-					       (pushnew :notmuch-part v)
-					     v)))))
+    (notmuch-show-record-part-information part beg (point))))
 
 (defun notmuch-show-insert-body (msg body depth)
   "Insert the body BODY at depth DEPTH in the current thread."


-- 
1.7.9.1

[PATCH v5 1/6] emacs: show: fake wash parts are handled at insert-bodypart level

[Mark Walters]

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 83bb9ad..209cd4a 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -751,10 +751,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

[PATCH v5 2/6] emacs: show: move the insertion of the header button to the top level

[Mark Walters]

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. However, the declared-type argument was only used for the
text for the part buttons we can now omit it.

The patch is large but mostly simple. The only things of note are that
we let the text/plain handler applies notmuch-wash to the whole part
including the part button. In particular, notmuch-wash removes leading
blank lines from a text/plain part, but since the button is counted as
part of the part this does not happen with text/plain buttons that
have a button. This is probably a bug in notmuch-wash but changing it
does make several tests fail (that rely on this blank line) so, for
the moment, keep the old behaviour.
---
 emacs/notmuch-show.el |   97 ++++++++++++++++++++++++-------------------------
 1 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 209cd4a..c69456a 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -514,8 +514,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 button)
   (let ((chosen-type (car (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part))))
 	(inner-parts (plist-get part :content))
 	(start (point)))
@@ -592,8 +591,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 button)
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
 
@@ -612,16 +610,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 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)))
@@ -634,20 +631,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 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)))
@@ -660,8 +656,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 button)
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
     ;; Show all of the parts.
@@ -673,8 +668,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 button)
   (let* ((message (car (plist-get part :content)))
 	 (body (car (plist-get message :body)))
 	 (start (point)))
@@ -695,12 +689,13 @@ 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)
-  (let ((start (point)))
-    ;; If this text/plain part is not the first part in the message,
-    ;; insert a header to make this clear.
-    (if (> nth 1)
-	(notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)))
+(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth button)
+  ;; For backward compatibility we want to apply the text/plain hook
+  ;; to the whole of the part including the part button if there is
+  ;; one.
+  (let ((start (if button
+		   (button-start button)
+		 (point))))
     (insert (notmuch-get-bodypart-content msg part nth notmuch-show-process-crypto))
     (save-excursion
       (save-restriction
@@ -708,8 +703,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 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
@@ -732,8 +726,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 button)
+  (notmuch-show-insert-part-text/calendar msg part content-type nth depth 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,
@@ -751,7 +745,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 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
@@ -759,11 +753,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 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 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)
 
@@ -786,13 +779,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 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 button)
 		       (error (progn
 				(insert "!!! Bodypart insert error: ")
 				(insert (error-message-string err))
@@ -820,6 +813,7 @@ 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)))
 	 (mime-type (or (and (string= content-type "application/octet-stream")
 			     (notmuch-show-get-mime-type-of-application/octet-stream part))
@@ -827,9 +821,12 @@ If HIDE is non-nil then initially hide this part."
 			     "text/x-diff")
 			content-type))
 	 (nth (plist-get part :id))
-	 (beg (point)))
+	 (beg (point))
+	 ;; We omit the part button for the first (or only) part if this is text/plain.
+	 (button (unless (and (string= mime-type "text/plain") (<= nth 1))
+		   (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 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

[PATCH v5 3/6] emacs: show: pass button to create-overlays

[Mark Walters]

Now that the bodypart code has the button we can pass that to
create-overlays and simplify that.
---
 emacs/notmuch-show.el |   31 +++++++++++++++----------------
 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index c69456a..0a0451b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -793,21 +793,19 @@ message at DEPTH in the current thread."
       (setq handlers (cdr handlers))))
   t)
 
-(defun notmuch-show-create-part-overlays (msg beg end hide)
+(defun notmuch-show-create-part-overlays (button beg end hide)
   "Add an overlay to the part between BEG and END"
-  (let* ((button (button-at beg))
-	 (part-beg (and button (1+ (button-end button)))))
-
-    ;; If the part contains no text we do not make it toggleable. We
-    ;; also need to check that the button is a genuine part button not
-    ;; a notmuch-wash button.
-    (when (and button (/= part-beg end) (button-get button :base-label))
-      (button-put button 'overlay (make-overlay part-beg end))
-      ;; We toggle the button for hidden parts as that gets the
-      ;; button label right.
-      (save-excursion
-	(when hide
-	  (notmuch-show-toggle-part-invisibility button))))))
+
+  ;; If there is no button (i.e., the part is text/plain and the first
+  ;; part) or if the part has no content then we don't make the part
+  ;; toggleable.
+  (when (and button (/= beg end))
+    (button-put button 'overlay (make-overlay beg end))
+    ;; We toggle the button for hidden parts as that gets the
+    ;; button label right.
+    (save-excursion
+      (when hide
+	(notmuch-show-toggle-part-invisibility button)))))
 
 (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
   "Insert the body part PART at depth DEPTH in the current thread.
@@ -824,7 +822,8 @@ If HIDE is non-nil then initially hide this part."
 	 (beg (point))
 	 ;; We omit the part button for the first (or only) part if this is text/plain.
 	 (button (unless (and (string= mime-type "text/plain") (<= nth 1))
-		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))))
+		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
+	 (content-beg (point)))
 
     (notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
     ;; Some of the body part handlers leave point somewhere up in the
@@ -833,7 +832,7 @@ If HIDE is non-nil then initially hide this part."
     ;; Ensure that the part ends with a carriage return.
     (unless (bolp)
       (insert "\n"))
-    (notmuch-show-create-part-overlays msg beg (point) hide)
+    (notmuch-show-create-part-overlays button content-beg (point) hide)
     ;; Record part information.  Since we already inserted subparts,
     ;; don't override existing :notmuch-part properties.
     (notmuch-map-text-property beg (point) :notmuch-part
-- 
1.7.9.1

[PATCH v5 4/6] emacs: show: modify the way hidden state is recorded.

[Mark Walters]

Previously, whether a part was hidden or shown was recorded in the
invisibility/visibility of the part overlay. Since we are going to
have lazily rendered parts with no overlay store the hidden/shown
state in the part button itself.

Additionally, in preparation for the invisible part handling move the
actual hiding of the hidden parts to insert-bodypart from
create-part-overlays.

Finally, we will need to know whether a part-insertion has done
anything (it won't if the invisible part cannot be displayed by emacs)
so we slightly rejig the code order in
notmuch-show-toggle-part-invisibility to make it easier for the
function to set an appropriate return value.
---
 emacs/notmuch-show.el |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0a0451b..43aa88b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -482,7 +482,8 @@ message at DEPTH in the current thread."
 	  (insert-button
 	   (concat "[ " base-label " ]")
 	   :base-label base-label
-	   :type 'notmuch-show-part-button-type))
+	   :type 'notmuch-show-part-button-type
+	   :notmuch-part-hidden nil))
     (insert "\n")
     ;; return button
     button))
@@ -493,20 +494,22 @@ message at DEPTH in the current thread."
   (let* ((button (or button (button-at (point))))
 	 (overlay (button-get button 'overlay)))
     (when overlay
-      (let* ((show (overlay-get overlay 'invisible))
+      (let* ((show (button-get button :notmuch-part-hidden))
 	     (new-start (button-start button))
 	     (button-label (button-get button :base-label))
 	     (old-point (point))
 	     (properties (text-properties-at (point)))
 	     (inhibit-read-only t))
-	(overlay-put overlay 'invisible (not show))
+	;; Toggle the button itself.
+	(button-put button :notmuch-part-hidden (not show))
 	(goto-char new-start)
 	(insert "[ " button-label (if show " ]" " (hidden) ]"))
 	(set-text-properties new-start (point) properties)
 	(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))))
+	(overlay-put overlay 'invisible (not show))))))
 
 ;; MIME part renderers
 
@@ -793,7 +796,7 @@ message at DEPTH in the current thread."
       (setq handlers (cdr handlers))))
   t)
 
-(defun notmuch-show-create-part-overlays (button beg end hide)
+(defun notmuch-show-create-part-overlays (button beg end)
   "Add an overlay to the part between BEG and END"
 
   ;; If there is no button (i.e., the part is text/plain and the first
@@ -801,11 +804,8 @@ message at DEPTH in the current thread."
   ;; toggleable.
   (when (and button (/= beg end))
     (button-put button 'overlay (make-overlay beg end))
-    ;; We toggle the button for hidden parts as that gets the
-    ;; button label right.
-    (save-excursion
-      (when hide
-	(notmuch-show-toggle-part-invisibility button)))))
+    ;; Return true if we created an overlay.
+    t))
 
 (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
   "Insert the body part PART at depth DEPTH in the current thread.
@@ -832,7 +832,10 @@ If HIDE is non-nil then initially hide this part."
     ;; Ensure that the part ends with a carriage return.
     (unless (bolp)
       (insert "\n"))
-    (notmuch-show-create-part-overlays button content-beg (point) hide)
+    (notmuch-show-create-part-overlays button content-beg (point))
+    (when hide
+      (save-excursion
+	(notmuch-show-toggle-part-invisibility button)))
     ;; Record part information.  Since we already inserted subparts,
     ;; don't override existing :notmuch-part properties.
     (notmuch-map-text-property beg (point) :notmuch-part
-- 
1.7.9.1

[PATCH v5 5/6] emacs: show move addition of :notmuch-part to separate function

[Mark Walters]

This separates out the adding of the :notmuch-part text property to a
separate function to simplify calling from the lazy part insertion
code.
---
 emacs/notmuch-show.el |   39 ++++++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 43aa88b..6ec70c9 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -807,6 +807,27 @@ message at DEPTH in the current thread."
     ;; Return true if we created an overlay.
     t))
 
+(defun notmuch-show-record-part-information (part beg end)
+  "Store PART as a text property from BEG to END"
+
+  ;; Record part information.  Since we already inserted subparts,
+  ;; don't override existing :notmuch-part properties.
+  (notmuch-map-text-property beg end :notmuch-part
+			     (lambda (v) (or v part)))
+  ;; Make :notmuch-part front sticky and rear non-sticky so it stays
+  ;; applied to the beginning of each line when we indent the
+  ;; message.  Since we're operating on arbitrary renderer output,
+  ;; watch out for sticky specs of t, which means all properties are
+  ;; front-sticky/rear-nonsticky.
+  (notmuch-map-text-property beg end 'front-sticky
+			     (lambda (v) (if (listp v)
+					     (pushnew :notmuch-part v)
+					   v)))
+  (notmuch-map-text-property beg end 'rear-nonsticky
+			     (lambda (v) (if (listp v)
+					     (pushnew :notmuch-part v)
+					   v))))
+
 (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
   "Insert the body part PART at depth DEPTH in the current thread.
 
@@ -836,23 +857,7 @@ If HIDE is non-nil then initially hide this part."
     (when hide
       (save-excursion
 	(notmuch-show-toggle-part-invisibility button)))
-    ;; Record part information.  Since we already inserted subparts,
-    ;; don't override existing :notmuch-part properties.
-    (notmuch-map-text-property beg (point) :notmuch-part
-			       (lambda (v) (or v part)))
-    ;; Make :notmuch-part front sticky and rear non-sticky so it stays
-    ;; applied to the beginning of each line when we indent the
-    ;; message.  Since we're operating on arbitrary renderer output,
-    ;; watch out for sticky specs of t, which means all properties are
-    ;; front-sticky/rear-nonsticky.
-    (notmuch-map-text-property beg (point) 'front-sticky
-			       (lambda (v) (if (listp v)
-					       (pushnew :notmuch-part v)
-					     v)))
-    (notmuch-map-text-property beg (point) 'rear-nonsticky
-			       (lambda (v) (if (listp v)
-					       (pushnew :notmuch-part v)
-					     v)))))
+    (notmuch-show-record-part-information part beg (point))))
 
 (defun notmuch-show-insert-body (msg body depth)
   "Insert the body BODY at depth DEPTH in the current thread."
-- 
1.7.9.1

[PATCH v5 6/6] emacs: show: implement lazy hidden part handling

[Mark Walters]

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.

One thing is that we can't tell if a lazy part will produce text until
we try to render it so when unhiding a part we check to see if it
rendered; if not we invoke the default part handler (e.g. an external
viewer).

Also, we would like to insert the lazy part at the start of the line
after the part button. But if this line has some text properties
(e.g. the colours for a following message header) then the lazy part
gets these properties. Thus we start at the end of the part button
line, insert a newline, insert the lazy part, and then delete the
extra newline at the end of the part.
---
 emacs/notmuch-show.el |   69 ++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 6ec70c9..d030ea3 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -488,12 +488,14 @@ message at DEPTH in the current thread."
     ;; return button
     button))
 
-;; This is taken from notmuch-wash: maybe it should be unified?
 (defun notmuch-show-toggle-part-invisibility (&optional button)
   (interactive)
   (let* ((button (or button (button-at (point))))
-	 (overlay (button-get button 'overlay)))
-    (when overlay
+	 (overlay (button-get button 'overlay))
+	 (lazy-part (button-get button :notmuch-lazy-part)))
+    ;; We have a part to toggle if there is an overlay or if there is a lazy part.
+    ;; If neither is present we cannot toggle the part so we just return nil.
+    (when (or overlay lazy-part)
       (let* ((show (button-get button :notmuch-part-hidden))
 	     (new-start (button-start button))
 	     (button-label (button-get button :base-label))
@@ -509,7 +511,15 @@ message at DEPTH in the current thread."
 	  (move-overlay button new-start (point))
 	  (delete-region (point) old-end))
 	(goto-char (min old-point (1- (button-end button))))
-	(overlay-put overlay 'invisible (not show))))))
+	;; Return nil if there is a lazy-part, it is empty, and we are
+	;; trying to show it.  In all other cases return t.
+	(if lazy-part
+	    (when show
+	      (button-put button :notmuch-lazy-part nil)
+	      (notmuch-show-lazy-part lazy-part button))
+	  ;; else there must be an overlay.
+	  (overlay-put overlay 'invisible (not show))
+	  t)))))
 
 ;; MIME part renderers
 
@@ -828,6 +838,39 @@ message at DEPTH in the current thread."
 					     (pushnew :notmuch-part v)
 					   v))))
 
+(defun notmuch-show-lazy-part (part-args button)
+  ;; Insert the lazy part after the button for the part. We would just
+  ;; move to the start of the new line following the button and insert
+  ;; the part but that point might have text properties (eg colours
+  ;; from a message header etc) so instead we start from the last
+  ;; character of the button by adding a newline and finish by
+  ;; removing the extra newline from the end of the part.
+  (save-excursion
+    (goto-char (button-end button))
+    (insert "\n")
+    (let* ((inhibit-read-only t)
+	   ;; We need to use markers for the start and end of the part
+	   ;; because the part insertion functions do not guarantee
+	   ;; to leave point at the end of the part.
+	   (part-beg (copy-marker (point) nil))
+	   (part-end (copy-marker (point) t))
+	   ;; We have to save the depth as we can't find the depth
+	   ;; when narrowed.
+	   (depth (notmuch-show-get-depth)))
+      (save-restriction
+	(narrow-to-region part-beg part-end)
+	(delete-region part-beg part-end)
+	(apply #'notmuch-show-insert-bodypart-internal part-args)
+	(indent-rigidly part-beg part-end depth))
+      (goto-char part-end)
+      (delete-char 1)
+      (notmuch-show-record-part-information (second part-args)
+					    (button-start button)
+					    part-end)
+      ;; Create the overlay. If the lazy-part turned out to be empty/not
+      ;; showable this returns nil.
+      (notmuch-show-create-part-overlays button part-beg part-end))))
+
 (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
   "Insert the body part PART at depth DEPTH in the current thread.
 
@@ -846,15 +889,21 @@ If HIDE is non-nil then initially hide this part."
 		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
 	 (content-beg (point)))
 
-    (notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
+    (if (not hide)
+        (notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
+      (button-put button :notmuch-lazy-part
+                  (list msg part mime-type nth depth 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))
     ;; Ensure that the part ends with a carriage return.
     (unless (bolp)
       (insert "\n"))
-    (notmuch-show-create-part-overlays button content-beg (point))
-    (when hide
+    ;; We do not create the overlay for hidden (lazy) parts until
+    ;; they are inserted.
+    (if (not hide)
+	(notmuch-show-create-part-overlays button content-beg (point))
       (save-excursion
 	(notmuch-show-toggle-part-invisibility button)))
     (notmuch-show-record-part-information part beg (point))))
@@ -2019,8 +2068,10 @@ is destroyed when FN returns."
 (defun notmuch-show-part-button-default (&optional button)
   (interactive)
   (let ((button (or button (button-at (point)))))
-    (if (button-get button 'overlay)
-	(notmuch-show-toggle-part-invisibility button)
+    ;; Try to toggle the part, if that fails then call the default
+    ;; action. The toggle fails if the part has no emacs renderable
+    ;; content.
+    (unless (notmuch-show-toggle-part-invisibility button)
       (call-interactively notmuch-show-part-button-default-action))))
 
 (defun notmuch-show-save-part ()
-- 
1.7.9.1

Re: [PATCH v5 0/6] emacs: show: lazy handling of hidden parts

[Austin Clements]

LGTM.

Quoth Mark Walters on Jun 10 at  5:57 am:
> This is version 5 of this patch set. Version 4 is at
> id:1370074547-24677-1-git-send-email-markwalters1009@gmail.com.
> 
> This version fixes the two bugs pointed out in Austin's review
> id:20130610023952.GC22196@mit.edu.
> 
> I decided to add the :notmuch-part text property separately from
> create-overlays as they have slightly different regions (one includes
> the button one does not) and are applied at slightly different times
> (we do not create overlays for lazy parts).
> 
> I was not sure whether we created two overlays for hidden lazy parts
> in v4 (it might not have done as the part was empty and we do not
> create overlays for empty parts) but that is stupidly fragile. Thus we
> explicitly do not create overlays for lazy parts until they are
> inserted.
> 
> The diff from v4 is below.
> 
> Best wishes
> 
> Mark

Re: [PATCH v5 0/6] emacs: show: lazy handling of hidden parts

[Tomi Ollila]

On Mon, Jun 10 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> This is version 5 of this patch set. Version 4 is at
> id:1370074547-24677-1-git-send-email-markwalters1009@gmail.com.
>
> This version fixes the two bugs pointed out in Austin's review
> id:20130610023952.GC22196@mit.edu.
>
> I decided to add the :notmuch-part text property separately from
> create-overlays as they have slightly different regions (one includes
> the button one does not) and are applied at slightly different times
> (we do not create overlays for lazy parts).
>
> I was not sure whether we created two overlays for hidden lazy parts
> in v4 (it might not have done as the part was empty and we do not
> create overlays for empty parts) but that is stupidly fragile. Thus we
> explicitly do not create overlays for lazy parts until they are
> inserted.
>
> The diff from v4 is below.

As far as I understood the code looked good. I also checked lazy
rendering by using w3m-standalone as mm-text-html-renderer and by
writing a wrapper w3m which appended command line to a file before
running w3m and then `tail -f`d the file...

... and w3m was run at the time [text/html (hidden)] was pressed.

So, +1

> Best wishes
>
> Mark

Tomi

PS: anyone interested the command lines were:

-dump -T text/html -I UTF-8 -O UTF-8
-dump -T text/html -I UTF-8 -O UTF-8
-dump -T text/html -I iso-8859-1 -O iso-8859-1

(based on original encoding, emacs then did conversions when needed)

> Mark Walters (6):
>   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: pass button to create-overlays
>   emacs: show: modify the way hidden state is recorded.
>   emacs: show move addition of :notmuch-part to separate function
>   emacs: show: implement lazy hidden part handling
>
>  emacs/notmuch-show.el |  241 ++++++++++++++++++++++++++++++-------------------
>  1 files changed, 146 insertions(+), 95 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index b963859..d030ea3 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -817,6 +817,27 @@ message at DEPTH in the current thread."
>      ;; Return true if we created an overlay.
>      t))
>  
> +(defun notmuch-show-record-part-information (part beg end)
> +  "Store PART as a text property from BEG to END"
> +
> +  ;; Record part information.  Since we already inserted subparts,
> +  ;; don't override existing :notmuch-part properties.
> +  (notmuch-map-text-property beg end :notmuch-part
> +			     (lambda (v) (or v part)))
> +  ;; Make :notmuch-part front sticky and rear non-sticky so it stays
> +  ;; applied to the beginning of each line when we indent the
> +  ;; message.  Since we're operating on arbitrary renderer output,
> +  ;; watch out for sticky specs of t, which means all properties are
> +  ;; front-sticky/rear-nonsticky.
> +  (notmuch-map-text-property beg end 'front-sticky
> +			     (lambda (v) (if (listp v)
> +					     (pushnew :notmuch-part v)
> +					   v)))
> +  (notmuch-map-text-property beg end 'rear-nonsticky
> +			     (lambda (v) (if (listp v)
> +					     (pushnew :notmuch-part v)
> +					   v))))
> +
>  (defun notmuch-show-lazy-part (part-args button)
>    ;; Insert the lazy part after the button for the part. We would just
>    ;; move to the start of the new line following the button and insert
> @@ -843,6 +864,9 @@ message at DEPTH in the current thread."
>  	(indent-rigidly part-beg part-end depth))
>        (goto-char part-end)
>        (delete-char 1)
> +      (notmuch-show-record-part-information (second part-args)
> +					    (button-start button)
> +					    part-end)
>        ;; Create the overlay. If the lazy-part turned out to be empty/not
>        ;; showable this returns nil.
>        (notmuch-show-create-part-overlays button part-beg part-end))))
> @@ -876,27 +900,13 @@ If HIDE is non-nil then initially hide this part."
>      ;; Ensure that the part ends with a carriage return.
>      (unless (bolp)
>        (insert "\n"))
> -    (notmuch-show-create-part-overlays button content-beg (point))
> -    (when hide
> +    ;; We do not create the overlay for hidden (lazy) parts until
> +    ;; they are inserted.
> +    (if (not hide)
> +	(notmuch-show-create-part-overlays button content-beg (point))
>        (save-excursion
>  	(notmuch-show-toggle-part-invisibility button)))
> -    ;; Record part information.  Since we already inserted subparts,
> -    ;; don't override existing :notmuch-part properties.
> -    (notmuch-map-text-property beg (point) :notmuch-part
> -			       (lambda (v) (or v part)))
> -    ;; Make :notmuch-part front sticky and rear non-sticky so it stays
> -    ;; applied to the beginning of each line when we indent the
> -    ;; message.  Since we're operating on arbitrary renderer output,
> -    ;; watch out for sticky specs of t, which means all properties are
> -    ;; front-sticky/rear-nonsticky.
> -    (notmuch-map-text-property beg (point) 'front-sticky
> -			       (lambda (v) (if (listp v)
> -					       (pushnew :notmuch-part v)
> -					     v)))
> -    (notmuch-map-text-property beg (point) 'rear-nonsticky
> -			       (lambda (v) (if (listp v)
> -					       (pushnew :notmuch-part v)
> -					     v)))))
> +    (notmuch-show-record-part-information part beg (point))))
>  
>  (defun notmuch-show-insert-body (msg body depth)
>    "Insert the body BODY at depth DEPTH in the current thread."
>
>
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v5 0/6] emacs: show: lazy handling of hidden parts

[Adam Wolfe Gordon]

On Sun, Jun 9, 2013 at 10:57 PM, Mark Walters <markwalters1009@gmail.com> wrote:
> This is version 5 of this patch set. Version 4 is at
> id:1370074547-24677-1-git-send-email-markwalters1009@gmail.com.

I've been using version 4 for a few days, and works great. The version
of the code I reviewed LGTM.

Since this series provides a big performance improvement for my usage,
I'd like to see it pushed soon :-).

-- Adam

Re: [PATCH v5 0/6] emacs: show: lazy handling of hidden parts

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> This is version 5 of this patch set. Version 4 is at
> id:1370074547-24677-1-git-send-email-markwalters1009@gmail.com.
>

pushed, 

d