unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v4 0/5] emacs: show: lazy handling of hidden parts
@ 2013-06-01  8:15 Mark Walters
  2013-06-01  8:15 ` [PATCH v4 1/5] emacs: show: fake wash parts are handled at insert-bodypart level Mark Walters
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Mark Walters @ 2013-06-01  8:15 UTC (permalink / raw)
  To: notmuch, Austin Clements, Adam Wolfe Gordon

This is version 4 of this series. Version 3 is at
id:1370024806-6616-1-git-send-email-markwalters1009@gmail.com

Changes from version 3 are: fix the whitespace changes in patch 3/5,
store both the start of the part and the start of the content in
insert-bodypart (Austin's part handler series needs one and I need the
other) and fixed the missing save-excursion in patch 4/5.

Finally, there is one more substantial change: rebasing on to Austin's
part handler (now in master). There was a little more to this than I
expected so for anyone testing please use this version.

Many thanks

Mark



Mark Walters (5):
  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: implement lazy hidden part handling

 emacs/notmuch-show.el |  189 +++++++++++++++++++++++++++++--------------------
 1 file changed, 111 insertions(+), 78 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/5] emacs: show: fake wash parts are handled at insert-bodypart level
  2013-06-01  8:15 [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Mark Walters
@ 2013-06-01  8:15 ` Mark Walters
  2013-06-01  8:15 ` [PATCH v4 2/5] emacs: show: move the insertion of the header button to the top level Mark Walters
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Mark Walters @ 2013-06-01  8:15 UTC (permalink / raw)
  To: notmuch, Austin Clements, Adam Wolfe Gordon

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 file changed, 4 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 613e666..dda5c10 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.10.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/5] emacs: show: move the insertion of the header button to the top level
  2013-06-01  8:15 [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Mark Walters
  2013-06-01  8:15 ` [PATCH v4 1/5] emacs: show: fake wash parts are handled at insert-bodypart level Mark Walters
@ 2013-06-01  8:15 ` Mark Walters
  2013-06-01  8:15 ` [PATCH v4 3/5] emacs: show: pass button to create-overlays Mark Walters
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Mark Walters @ 2013-06-01  8:15 UTC (permalink / raw)
  To: notmuch, Austin Clements, Adam Wolfe Gordon

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 file changed, 47 insertions(+), 50 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index dda5c10..9e119e8 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.10.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 3/5] emacs: show: pass button to create-overlays
  2013-06-01  8:15 [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Mark Walters
  2013-06-01  8:15 ` [PATCH v4 1/5] emacs: show: fake wash parts are handled at insert-bodypart level Mark Walters
  2013-06-01  8:15 ` [PATCH v4 2/5] emacs: show: move the insertion of the header button to the top level Mark Walters
@ 2013-06-01  8:15 ` Mark Walters
  2013-06-01  8:15 ` [PATCH v4 4/5] emacs: show: modify the way hidden state is recorded Mark Walters
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Mark Walters @ 2013-06-01  8:15 UTC (permalink / raw)
  To: notmuch, Austin Clements, Adam Wolfe Gordon

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

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 9e119e8..cdea271 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.10.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 4/5] emacs: show: modify the way hidden state is recorded.
  2013-06-01  8:15 [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Mark Walters
                   ` (2 preceding siblings ...)
  2013-06-01  8:15 ` [PATCH v4 3/5] emacs: show: pass button to create-overlays Mark Walters
@ 2013-06-01  8:15 ` Mark Walters
  2013-06-01  8:15 ` [PATCH v4 5/5] emacs: show: implement lazy hidden part handling Mark Walters
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Mark Walters @ 2013-06-01  8:15 UTC (permalink / raw)
  To: notmuch, Austin Clements, Adam Wolfe Gordon

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 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index cdea271..49f85ba 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.10.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 5/5] emacs: show: implement lazy hidden part handling
  2013-06-01  8:15 [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Mark Walters
                   ` (3 preceding siblings ...)
  2013-06-01  8:15 ` [PATCH v4 4/5] emacs: show: modify the way hidden state is recorded Mark Walters
@ 2013-06-01  8:15 ` Mark Walters
  2013-06-01 16:46 ` [PATCH v4 6/5] emacs: show: lazy-part bugfix Mark Walters
  2013-06-10  2:39 ` [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Austin Clements
  6 siblings, 0 replies; 8+ messages in thread
From: Mark Walters @ 2013-06-01  8:15 UTC (permalink / raw)
  To: notmuch, Austin Clements, Adam Wolfe Gordon

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).
---
 emacs/notmuch-show.el |   52 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 49f85ba..92f481b 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
 
@@ -807,6 +817,28 @@ message at DEPTH in the current thread."
     ;; Return true if we created an overlay.
     t))
 
+(defun notmuch-show-lazy-part (part-args button)
+  ;; Insert the lazy part after the button for the part.
+  (save-excursion
+    (goto-char (1+ (button-end button)))
+    (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))
+      ;; 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.
 
@@ -825,7 +857,11 @@ 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))
@@ -2009,8 +2045,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.10.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 6/5] emacs: show: lazy-part bugfix
  2013-06-01  8:15 [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Mark Walters
                   ` (4 preceding siblings ...)
  2013-06-01  8:15 ` [PATCH v4 5/5] emacs: show: implement lazy hidden part handling Mark Walters
@ 2013-06-01 16:46 ` Mark Walters
  2013-06-10  2:39 ` [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Austin Clements
  6 siblings, 0 replies; 8+ messages in thread
From: Mark Walters @ 2013-06-01 16:46 UTC (permalink / raw)
  To: notmuch, Austin Clements, Adam Wolfe Gordon


Previously we inserted the lazy part at the start of the line after
the part button. But if this line had 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.
---

I found a bug in the lazy-part patch set (v4): the problem can be seen
if you find a multipart/alternative text/plain text/html message with a
reply following it. Before this extra patch the lazy-part gets inserted
with the wrong text properties: in particular it gets the special
background colour that the headerline of a message gets in the show
view. I think this was why I had the "lazy part" text before but this is
probably better. 

I can fold this into the main series but I thought I would ask for
comments on this change first.

Best wishes

Mark
 


 emacs/notmuch-show.el |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 92f481b..453ff54 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -818,9 +818,15 @@ message at DEPTH in the current thread."
     t))
 
 (defun notmuch-show-lazy-part (part-args button)
-  ;; Insert the lazy part after the button for the part.
+  ;; 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 (1+ (button-end button)))
+    (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
@@ -835,6 +841,8 @@ message at DEPTH in the current thread."
 	(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)
       ;; 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))))
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 0/5] emacs: show: lazy handling of hidden parts
  2013-06-01  8:15 [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Mark Walters
                   ` (5 preceding siblings ...)
  2013-06-01 16:46 ` [PATCH v4 6/5] emacs: show: lazy-part bugfix Mark Walters
@ 2013-06-10  2:39 ` Austin Clements
  6 siblings, 0 replies; 8+ messages in thread
From: Austin Clements @ 2013-06-10  2:39 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch

Quoth Mark Walters on Jun 01 at  9:15 am:
> This is version 4 of this series. Version 3 is at
> id:1370024806-6616-1-git-send-email-markwalters1009@gmail.com
> 
> Changes from version 3 are: fix the whitespace changes in patch 3/5,
> store both the start of the part and the start of the content in
> insert-bodypart (Austin's part handler series needs one and I need the
> other) and fixed the missing save-excursion in patch 4/5.
> 
> Finally, there is one more substantial change: rebasing on to Austin's
> part handler (now in master). There was a little more to this than I
> expected so for anyone testing please use this version.

This series LGTM except for one bug (also related to my new part
handling, sorry): after expanding a lazy part, the "." submap doesn't
work within that part because it doesn't get the :notmuch-part
property applied to it.  I'm not sure what the best solution to this
is.  Maybe applying :notmuch-part should be moved to
notmuch-show-create-part-overlays?

Also, unrelated: with this series, do we wind up creating a second
overlay when a lazy part is shown?

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-06-10  2:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-01  8:15 [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Mark Walters
2013-06-01  8:15 ` [PATCH v4 1/5] emacs: show: fake wash parts are handled at insert-bodypart level Mark Walters
2013-06-01  8:15 ` [PATCH v4 2/5] emacs: show: move the insertion of the header button to the top level Mark Walters
2013-06-01  8:15 ` [PATCH v4 3/5] emacs: show: pass button to create-overlays Mark Walters
2013-06-01  8:15 ` [PATCH v4 4/5] emacs: show: modify the way hidden state is recorded Mark Walters
2013-06-01  8:15 ` [PATCH v4 5/5] emacs: show: implement lazy hidden part handling Mark Walters
2013-06-01 16:46 ` [PATCH v4 6/5] emacs: show: lazy-part bugfix Mark Walters
2013-06-10  2:39 ` [PATCH v4 0/5] emacs: show: lazy handling of hidden parts Austin Clements

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).