From: Mark Walters <markwalters1009@gmail.com>
To: Jameson Graef Rollins <jrollins@finestructure.net>,
	Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org
Subject: [PATCH v2] emacs: add function to toggle display of all
	multipart/alternative parts
In-Reply-To: <87ipcrvlcw.fsf@servo.finestructure.net>
References: <1339775751-18002-1-git-send-email-jani@nikula.org>
	<87zk80gvmb.fsf@servo.finestructure.net>
	<87pq8vokmp.fsf@qmul.ac.uk>
	<87ipcrvlcw.fsf@servo.finestructure.net>
User-Agent: Notmuch/0.13.2+96~g634443c (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Fri, 10 Aug 2012 08:10:22 +0100
Message-ID: <87r4rf9q8x.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


Some messages are sent as multipart/alternative but the alternatives
contain different information. This allows the user to cycle which
part to view. By default this is bound to 'W'.
---

On Thu, 09 Aug 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Mon, Jun 18 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>> I agree with this cycling approach but I think it needs to be per
>> message rather than per buffer. I attach a rather hacky attempt at this
>> below: on minimal testing it seems to work. But the lisp is really a bit
>> gross. In particular  I have no idea if I should be escaping the message
>> ids (so this could break in unfortunate/insecure ways)
>
> Thanks to broken Apple mail clients, I'm getting more and more messages
> that have attachments hidden in multipart/"alternatives" to text/plain
> parts.  So I would really like to revive this patch.
>
> I just tested it and it still applies to current master, and actually
> seems to work great.  'W' cycles through which part is displayed in the
> current message.  Pretty much exactly what I want.
>
> Mark seems to think this patch is less than ideal.  One issue is that
> it's trying to store a setting for a single displayed message in a
> variable of full buffer scope.  So he's storing a list of message ids
> there:
>
>> +(defvar notmuch-show-message-multipart/alternative nil)
>> +(make-variable-buffer-local 'notmuch-show-message-multipart/alternative)
>> +(put 'notmuch-show-message-multipart/alternative 'permanent-local t)
> ...
>> +	    (lax-plist-put notmuch-show-message-multipart/alternative (plist-get msg :id) 0)))
>
> I can see that might get a little hairy.  Can any elisp experts out
> there think of a better way to do this?  (actually, this is making me
> again want a show mode that only displays one message at a time (which I
> guess means I need to try pick again)).

This version at least uses the notmuch escaping for message-id which
makes me a bit happier: it probably doesn't have any nasty security
flaws. I do still feel that the lisp is a bit ugly though.

Incidentally, Austin suggested I might be able to use text-properties
rather than this big list. Unfortunately, I use
notmuch-show-refresh-view to do the redisplay and that deletes all
text-properties.

Note this is not very well tested as I have very few
multipart/alternative messages.

Best wishes

Mark


 emacs/notmuch-show.el |   31 ++++++++++++++++++++++++++++---
 1 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index dcfc190..dee6b85 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -154,6 +154,10 @@ indentation."
 (make-variable-buffer-local 'notmuch-show-indent-content)
 (put 'notmuch-show-indent-content 'permanent-local t)
 
+(defvar notmuch-show-message-multipart/alternative-display-part nil)
+(make-variable-buffer-local 'notmuch-show-message-multipart/alternative-display-part)
+(put 'notmuch-show-message-multipart/alternative-display-part 'permanent-local t)
+
 (defcustom notmuch-show-stash-mlarchive-link-alist
   '(("Gmane" . "http://mid.gmane.org/")
     ("MARC" . "http://marc.info/?i=")
@@ -536,9 +540,19 @@ message at DEPTH in the current thread."
 
 (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)
-  (let ((chosen-type (car (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part))))
-	(inner-parts (plist-get part :content))
-	(start (point)))
+  (let* ((chosen-nth (or (lax-plist-get notmuch-show-message-multipart/alternative-display-part
+					(notmuch-id-to-query (plist-get msg :id))) 0))
+	 (chosen-type (nth chosen-nth
+			  (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part))))
+	 (inner-parts (plist-get part :content))
+	 (start (point)))
+    ;; If we have run out of possible content-types restart from the beginning
+    (unless chosen-type
+      (setq chosen-type (car (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part))))
+      (setq notmuch-show-message-multipart/alternative-display-part
+	    (lax-plist-put notmuch-show-message-multipart/alternative-display-part
+			   (notmuch-id-to-query (plist-get msg :id)) 0)))
+
     ;; This inserts all parts of the chosen type rather than just one,
     ;; 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?
@@ -942,6 +956,16 @@ message at DEPTH in the current thread."
 	     "Not processing cryptographic MIME parts."))
   (notmuch-show-refresh-view))
 
+(defun notmuch-show-cycle-message-multipart ()
+  "Cycle which part to display of a multipart messageToggle the display of non-matching messages."
+  (interactive)
+  (let* ((msg-id (notmuch-show-get-message-id))
+	 (next-part (1+ (or (lax-plist-get notmuch-show-message-multipart/alternative-display-part msg-id) 0))))
+    (setq notmuch-show-message-multipart/alternative-display-part
+	(lax-plist-put notmuch-show-message-multipart/alternative-display-part msg-id next-part))
+    (message "Cycling multipart/alternative for current message")
+    (notmuch-show-refresh-view)))
+
 (defun notmuch-show-toggle-elide-non-matching ()
   "Toggle the display of non-matching messages."
   (interactive)
@@ -1151,6 +1175,7 @@ reset based on the original query."
 	(define-key map "R" 'notmuch-show-reply)
 	(define-key map "|" 'notmuch-show-pipe-message)
 	(define-key map "w" 'notmuch-show-save-attachments)
+	(define-key map "W" 'notmuch-show-cycle-message-multipart)
 	(define-key map "V" 'notmuch-show-view-raw-message)
 	(define-key map "v" 'notmuch-show-view-all-mime-parts)
 	(define-key map "c" 'notmuch-show-stash-map)
-- 
1.7.9.1