[PATCH v3 0/5] Use invisibility to toggle display of all parts including multipart

[PATCH v3 0/5] Use invisibility to toggle display of all parts including multipart

[Mark Walters]

This is version 3 of this set: version 2 is at
id:1355389924-3718-1-git-send-email-markwalters1009@gmail.com

This fixes the things Austin pointed out in the previous version.

Diff from v2 is below the diffstat.

Best wishes 

Mark

Mark Walters (5):
  emacs: show: modify insert-part-header to save the button text
  emacs: show: add overlays for each part
  emacs: show: add invisibility button action
  emacs: wash: fix fake-diff part to include msg parameter
  emacs: show: set default show-all-multipart/alternatives to nil

 emacs/notmuch-show.el |  113 +++++++++++++++++++++++++++++++++++++-----------
 emacs/notmuch-wash.el |    2 +-
 2 files changed, 88 insertions(+), 27 deletions(-)



diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index c43dd32..457f84a 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -478,7 +478,6 @@ message at DEPTH in the current thread."
     (define-key map "v" 'notmuch-show-part-button-view)
     (define-key map "o" 'notmuch-show-part-button-interactively-view)
     (define-key map "|" 'notmuch-show-part-button-pipe)
-    (define-key map "t" 'notmuch-show-toggle-invisible-part-action)
     map)
   "Submap for button commands")
 (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
@@ -556,7 +555,7 @@ message at DEPTH in the current thread."
       (mm-pipe-part handle))))
 
 ;; This is taken from notmuch-wash: maybe it should be unified?
-(defun notmuch-show-toggle-invisible-part-action (&optional button)
+(defun notmuch-show-toggle-part-invisibility (&optional button)
   (interactive)
   (let* ((button (or button (button-at (point))))
 	 (overlay (button-get button 'overlay))
@@ -861,7 +860,7 @@ message at DEPTH in the current thread."
       (setq handlers (cdr handlers))))
   t)
 
-(defun notmuch-show-insert-part-overlays (msg beg end hide)
+(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))
 	 (part-beg (and button (1+ (button-end button)))))
@@ -875,7 +874,6 @@ message at DEPTH in the current thread."
 
 	(overlay-put overlay 'invisible (list invis-spec message-invis-spec))
 	(overlay-put overlay 'priority 10)
-	(overlay-put overlay 'type "part")
 	;; Now we have to add invis-spec to every overlay this
 	;; overlay contains, otherwise these inner overlays will
 	;; override this one.
@@ -892,7 +890,7 @@ message at DEPTH in the current thread."
       ;; button label right.
       (save-excursion
 	(when hide
-	  (notmuch-show-toggle-invisible-part-action button))))))
+	  (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.
@@ -909,7 +907,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-insert-part-overlays msg beg (point) hide)))
+    (notmuch-show-create-part-overlays msg beg (point) hide)))
 
 (defun notmuch-show-insert-body (msg body depth)
   "Insert the body BODY at depth DEPTH in the current thread."
@@ -2023,7 +2021,7 @@ the user (see `notmuch-show-stash-mlarchive-link-alist')."
   (interactive)
   (let ((button (or button (button-at (point)))))
     (if (button-get button 'invisibility-spec)
-	(notmuch-show-toggle-invisible-part-action button)
+	(notmuch-show-toggle-part-invisibility button)
       (notmuch-show-part-button-internal button notmuch-show-part-button-default-action))))
 
 (defun notmuch-show-part-button-save (&optional button)

[PATCH v3 1/5] emacs: show: modify insert-part-header to save the button text

[Mark Walters]

This just make notmuch-show-insert-part-header save the basic button
text for parts as an attribute. This makes it simpler for the button
action (added in a later patch) to reword the label as appropriate (eg
append "(not shown)" or not as appropriate).
---
 emacs/notmuch-show.el |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index d7fa10e..06f7ca5 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -483,17 +483,17 @@ message at DEPTH in the current thread."
 (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
 
 (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
-  (let ((button))
+  (let ((button)
+	(base-label (concat (when name (concat name ": "))
+			    declared-type
+			    (unless (string-equal declared-type content-type)
+			      (concat " (as " content-type ")"))
+			    comment)))
+
     (setq button
 	  (insert-button
-	   (concat "[ "
-		   (if name (concat name ": ") "")
-		   declared-type
-		   (if (not (string-equal declared-type content-type))
-		       (concat " (as " content-type ")")
-		     "")
-		   (or comment "")
-		   " ]")
+	   (concat "[ " base-label " ]")
+	   :base-label base-label
 	   :type 'notmuch-show-part-button-type
 	   :notmuch-part nth
 	   :notmuch-filename name
-- 
1.7.9.1

[PATCH v3 2/5] emacs: show: add overlays for each part

[Mark Walters]

This makes notmuch-show-insert-bodypart add an overlay for any
non-trivial part with a button header (currently the first text/plain
part does not have a button). At this point the overlay is available
to the button but there is no action using it yet.

In addition the argument HIDE is passed down to
notmuch-show-insert-part-overlays to request that the part be hidden
by default but this is not acted on yet.
---
 emacs/notmuch-show.el |   60 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 06f7ca5..b65c339 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -567,11 +567,10 @@ message at DEPTH in the current thread."
     ;; but it's not clear that this is the wrong thing to do - which
     ;; should be chosen if there are more than one that match?
     (mapc (lambda (inner-part)
-	    (let ((inner-type (plist-get inner-part :content-type)))
-	      (if (or notmuch-show-all-multipart/alternative-parts
-		      (string= chosen-type inner-type))
-		  (notmuch-show-insert-bodypart msg inner-part depth)
-		(notmuch-show-insert-part-header (plist-get inner-part :id) inner-type inner-type nil " (not shown)"))))
+	    (let* ((inner-type (plist-get inner-part :content-type))
+		  (hide (not (or notmuch-show-all-multipart/alternative-parts
+			   (string= chosen-type inner-type)))))
+	      (notmuch-show-insert-bodypart msg inner-part depth hide)))
 	  inner-parts)
 
     (when notmuch-show-indent-multipart
@@ -839,17 +838,48 @@ message at DEPTH in the current thread."
       (setq handlers (cdr handlers))))
   t)
 
-(defun notmuch-show-insert-bodypart (msg part depth)
-  "Insert the body part PART at depth DEPTH in the current thread."
+(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))
+	 (part-beg (and button (1+ (button-end button)))))
+
+    ;; If the part contains no text we do not make it toggleable.
+    (when (and button (/= part-beg end))
+      (let ((base-label (button-get button :base-label))
+	    (overlay (make-overlay part-beg end))
+	    (message-invis-spec (plist-get msg :message-invis-spec))
+	    (invis-spec (make-symbol "notmuch-part-region")))
+
+	(overlay-put overlay 'invisible (list invis-spec message-invis-spec))
+	(overlay-put overlay 'priority 10)
+	;; Now we have to add invis-spec to every overlay this
+	;; overlay contains, otherwise these inner overlays will
+	;; override this one.
+	(dolist (inner (overlays-in part-beg end))
+	  (when (and (>= (overlay-start inner) part-beg)
+			   (<= (overlay-end inner) end))
+		  (overlay-put inner 'invisible
+			       (cons invis-spec (overlay-get inner 'invisible)))))
+
+	(button-put button 'invisibility-spec invis-spec)
+	(button-put button 'overlay overlay)))))
+
+(defun notmuch-show-insert-bodypart (msg part depth &optional hide)
+  "Insert the body part PART at depth DEPTH in the current thread.
+
+If HIDE is non-nil then initially hide this part."
   (let ((content-type (downcase (plist-get part :content-type)))
-	(nth (plist-get part :id)))
-    (notmuch-show-insert-bodypart-internal msg part content-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))
-  ;; Ensure that the part ends with a carriage return.
-  (unless (bolp)
-    (insert "\n")))
+	(nth (plist-get part :id))
+	(beg (point)))
+
+    (notmuch-show-insert-bodypart-internal msg part content-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))
+    ;; Ensure that the part ends with a carriage return.
+    (unless (bolp)
+      (insert "\n"))
+    (notmuch-show-create-part-overlays msg beg (point) hide)))
 
 (defun notmuch-show-insert-body (msg body depth)
   "Insert the body BODY at depth DEPTH in the current thread."
-- 
1.7.9.1

[PATCH v3 3/5] emacs: show: add invisibility button action

[Mark Walters]

This adds a button action to show hidden parts. In this version "t"
toggles the visibility of a part. In addition "RET" on a non-shown
part shows it.

The button is used to hide parts when appropriate (eg text/html in
multipart/alternative).
---
 emacs/notmuch-show.el |   35 +++++++++++++++++++++++++++++++++--
 1 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index b65c339..b11d65d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -554,6 +554,28 @@ message at DEPTH in the current thread."
     (let ((handle (mm-make-handle (current-buffer) (list content-type))))
       (mm-pipe-part handle))))
 
+;; 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))
+	 (invis-spec (button-get button 'invisibility-spec))
+	 (show (invisible-p invis-spec)))
+    (when overlay
+      (if show
+	  (remove-from-invisibility-spec invis-spec)
+	(add-to-invisibility-spec invis-spec))
+      (let* ((new-start (button-start button))
+	     (button-label (button-get button :base-label))
+	     (old-point (point))
+	     (inhibit-read-only t))
+	(goto-char new-start)
+	(insert "[ " button-label (if show " ]" " (hidden) ]"))
+	(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))))))))
+
 (defun notmuch-show-multipart/*-to-list (part)
   (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
 	  (plist-get part :content)))
@@ -862,7 +884,13 @@ message at DEPTH in the current thread."
 			       (cons invis-spec (overlay-get inner 'invisible)))))
 
 	(button-put button 'invisibility-spec invis-spec)
-	(button-put button 'overlay overlay)))))
+	(button-put button 'overlay overlay))
+
+      ;; 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.
@@ -1991,7 +2019,10 @@ the user (see `notmuch-show-stash-mlarchive-link-alist')."
 
 (defun notmuch-show-part-button-default (&optional button)
   (interactive)
-  (notmuch-show-part-button-internal button notmuch-show-part-button-default-action))
+  (let ((button (or button (button-at (point)))))
+    (if (button-get button 'invisibility-spec)
+	(notmuch-show-toggle-part-invisibility button)
+      (notmuch-show-part-button-internal button notmuch-show-part-button-default-action))))
 
 (defun notmuch-show-part-button-save (&optional button)
   (interactive)
-- 
1.7.9.1

Re: [PATCH v3 3/5] emacs: show: add invisibility button action

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> This adds a button action to show hidden parts. In this version "t"
> toggles the visibility of a part. In addition "RET" on a non-shown
> part shows it.

After applying this patch (and the two before it), I get 

emacs-show: Testing emacs notmuch-show view
 FAIL   Hiding Original Message region at beginning of a message
	--- emacs-show.1.OUTPUT	2012-12-17 12:55:01.000000000 +0000
	+++ emacs-show.1.EXPECTED	2012-12-17 12:55:01.000000000 +0000
	@@ -4,5 +4,3 @@
	 Date: Fri, 05 Jan 2001 15:43:57 +0000
	 
	 [ 2-line hidden original message. Click/Enter to show. ]
	------Original Message-----
	-Text here.
nil

I believe this failure exists after applying the full series; I'm just
double checking now.

The problem seems to be specific to emacs24; I'm running 24.2.1

d

[PATCH v3 4/5] emacs: wash: fix fake-diff part to include msg parameter

[Mark Walters]

The fake-diff part from notmuch wash did not pass the msg parameter to
notmuch-show-insert-bodypart. Previously this did not matter, but is
now needed to get the overlays right for invisibility.
---
 emacs/notmuch-wash.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 7d003a2..b2affba 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -378,7 +378,7 @@ for error."
 			       (plist-get
 				(plist-get msg :headers) :Subject))))
 	(delete-region (point-min) (point-max))
-	(notmuch-show-insert-bodypart nil part depth)))))
+	(notmuch-show-insert-bodypart msg part depth)))))
 
 ;;
 
-- 
1.7.9.1

[PATCH v3 5/5] emacs: show: set default show-all-multipart/alternatives to nil

[Mark Walters]

Now that the invisibility display of parts is present we no longer
need to force the display of all multipart/alternatives: users can
toggle them for themselves when needed.
---
 emacs/notmuch-show.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index b11d65d..457f84a 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -94,7 +94,7 @@ visible for any given message."
   :group 'notmuch-hooks)
 
 ;; Mostly useful for debugging.
-(defcustom notmuch-show-all-multipart/alternative-parts t
+(defcustom notmuch-show-all-multipart/alternative-parts nil
   "Should all parts of multipart/alternative parts be shown?"
   :type 'boolean
   :group 'notmuch-show)
-- 
1.7.9.1

Re: [PATCH v3 0/5] Use invisibility to toggle display of all parts including multipart

[Austin Clements]

Quoth Mark Walters on Dec 15 at  8:15 am:
> This is version 3 of this set: version 2 is at
> id:1355389924-3718-1-git-send-email-markwalters1009@gmail.com
> 
> This fixes the things Austin pointed out in the previous version.
> 
> Diff from v2 is below the diffstat.
> 
> Best wishes 
> 
> Mark

LGTM.