unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: add function to toggle display of all multipart/alternative parts
@ 2012-06-15 15:55 Jani Nikula
  2012-06-18 21:26 ` Jameson Graef Rollins
  2012-06-19 20:14 ` [PATCH v2] " Jani Nikula
  0 siblings, 2 replies; 23+ messages in thread
From: Jani Nikula @ 2012-06-15 15:55 UTC (permalink / raw)
  To: notmuch

Add function notmuch-show-toggle-multipart-alternative to make
notmuch-show-all-multipart/alternative-parts buffer local, toggle its
value in the buffer, and redisplay the thread with either all or
preferred alternative parts expanded.

A small wrinkle is that in order to make the variable survive
notmuch-show-refresh-view (which is required for expanding/collapsing
the parts, but ends up calling kill-all-local-variables through
notmuch-show-mode) it is necessary to give it the permanent-local
property.
---
 emacs/notmuch-show.el |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 36cad93..9c177d9 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -99,6 +99,17 @@ any given message."
   :type 'boolean
   :group 'notmuch-show)
 
+(defun notmuch-show-toggle-multipart-alternative ()
+  "Toggle `notmuch-show-all-multipart/alternative-parts' in the buffer."
+  (interactive)
+  (unless (local-variable-p 'notmuch-show-all-multipart/alternative-parts)
+    (make-local-variable 'notmuch-show-all-multipart/alternative-parts)
+    ;; Make it permanently local to survive notmuch-show-refresh-view.
+    (put 'notmuch-show-all-multipart/alternative-parts 'permanent-local t))
+  (setq notmuch-show-all-multipart/alternative-parts
+	(not notmuch-show-all-multipart/alternative-parts))
+  (notmuch-show-refresh-view))
+
 (defcustom notmuch-show-indent-messages-width 1
   "Width of message indentation in threads.
 
-- 
1.7.9.5

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

* Re: [PATCH] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-15 15:55 [PATCH] emacs: add function to toggle display of all multipart/alternative parts Jani Nikula
@ 2012-06-18 21:26 ` Jameson Graef Rollins
  2012-06-19  6:56   ` Mark Walters
  2012-06-19  6:57   ` [PATCH] " Jani Nikula
  2012-06-19 20:14 ` [PATCH v2] " Jani Nikula
  1 sibling, 2 replies; 23+ messages in thread
From: Jameson Graef Rollins @ 2012-06-18 21:26 UTC (permalink / raw)
  To: Jani Nikula, notmuch

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

On Fri, Jun 15 2012, Jani Nikula <jani@nikula.org> wrote:
> Add function notmuch-show-toggle-multipart-alternative to make
> notmuch-show-all-multipart/alternative-parts buffer local, toggle its
> value in the buffer, and redisplay the thread with either all or
> preferred alternative parts expanded.
>
> A small wrinkle is that in order to make the variable survive
> notmuch-show-refresh-view (which is required for expanding/collapsing
> the parts, but ends up calling kill-all-local-variables through
> notmuch-show-mode) it is necessary to give it the permanent-local
> property.

This is a very cool feature.  I haven't looked closely at this but can't
this mechanism work the same as the other show-mode toggles, like crypto
or indenting?  I wouldn't think it would require any more extra variable
stuff than what is used for those other toggles.

Also, the *really* sweet thing would be if the toggle actually cycled
through display of the alternatives, only displaying one at a time.
That would be very swank.

David Edmondson put together the other show-mode toggling stuff, so he
may be able to help with this as well.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

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

* Re: [PATCH] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-18 21:26 ` Jameson Graef Rollins
@ 2012-06-19  6:56   ` Mark Walters
  2012-08-09 20:52     ` Jameson Graef Rollins
  2012-06-19  6:57   ` [PATCH] " Jani Nikula
  1 sibling, 1 reply; 23+ messages in thread
From: Mark Walters @ 2012-06-19  6:56 UTC (permalink / raw)
  To: Jameson Graef Rollins, Jani Nikula, notmuch

On Mon, 18 Jun 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Fri, Jun 15 2012, Jani Nikula <jani@nikula.org> wrote:
>> Add function notmuch-show-toggle-multipart-alternative to make
>> notmuch-show-all-multipart/alternative-parts buffer local, toggle its
>> value in the buffer, and redisplay the thread with either all or
>> preferred alternative parts expanded.
>>
>> A small wrinkle is that in order to make the variable survive
>> notmuch-show-refresh-view (which is required for expanding/collapsing
>> the parts, but ends up calling kill-all-local-variables through
>> notmuch-show-mode) it is necessary to give it the permanent-local
>> property.
>
> This is a very cool feature.  I haven't looked closely at this but can't
> this mechanism work the same as the other show-mode toggles, like crypto
> or indenting?  I wouldn't think it would require any more extra variable
> stuff than what is used for those other toggles.
>
> Also, the *really* sweet thing would be if the toggle actually cycled
> through display of the alternatives, only displaying one at a time.
> That would be very swank.
>
> David Edmondson put together the other show-mode toggling stuff, so he
> may be able to help with this as well.

Hi

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)

Best wishes

Mark

From 4164456d225fca3c62bacfdc08a21cf4b32b5067 Mon Sep 17 00:00:00 2001
From: Mark Walters <markwalters1009@gmail.com>
Date: Tue, 19 Jun 2012 07:50:17 +0100
Subject: [PATCH] first attempt at cycling multipart

---
 emacs/notmuch-show.el |   29 ++++++++++++++++++++++++++---
 1 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0f54259..65a89fe 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -155,6 +155,10 @@ indentation."
 (make-variable-buffer-local 'notmuch-show-indent-content)
 (put 'notmuch-show-indent-content 'permanent-local t)
 
+(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)
+
 (defcustom notmuch-show-stash-mlarchive-link-alist
   '(("Gmane" . "http://mid.gmane.org/")
     ("MARC" . "http://marc.info/?i=")
@@ -537,9 +541,17 @@ 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 (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
+	    (lax-plist-put notmuch-show-message-multipart/alternative (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?
@@ -943,6 +955,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 t))
+	 (next-part (1+ (or (lax-plist-get notmuch-show-message-multipart/alternative msg-id) 0))))
+    (setq notmuch-show-message-multipart/alternative
+	(lax-plist-put notmuch-show-message-multipart/alternative 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)
@@ -1155,6 +1177,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

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

* Re: [PATCH] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-18 21:26 ` Jameson Graef Rollins
  2012-06-19  6:56   ` Mark Walters
@ 2012-06-19  6:57   ` Jani Nikula
  1 sibling, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2012-06-19  6:57 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch

On Mon, 18 Jun 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Fri, Jun 15 2012, Jani Nikula <jani@nikula.org> wrote:
>> Add function notmuch-show-toggle-multipart-alternative to make
>> notmuch-show-all-multipart/alternative-parts buffer local, toggle its
>> value in the buffer, and redisplay the thread with either all or
>> preferred alternative parts expanded.
>
> This is a very cool feature.  I haven't looked closely at this but can't
> this mechanism work the same as the other show-mode toggles, like crypto
> or indenting?  I wouldn't think it would require any more extra variable
> stuff than what is used for those other toggles.

I don't know how I missed those - thanks for pointing them out to
me. They seem to do the extra variable stuff at the top level, alongside
the defvar, rather than in the toggle function like I do. I should
rearrange this toggle to be in line with the others.

> Also, the *really* sweet thing would be if the toggle actually cycled
> through display of the alternatives, only displaying one at a time.
> That would be very swank.

It is easy to agree with that. At a glance, it looks like it needs a
bunch more work than the simple toggle in this patch, though. One
difficulty is choosing the parts to cycle, as we may have multiple
messages with different sets of multipart/alternative in the same show
buffer. And the cycle would be buffer local, not "message local".

I think it would be possible to have show/hide buttons for the parts,
and the buttons could add/remove the part type from a buffer local list
of alternative parts to show, and then refresh view. But that too would
affect all messages in the buffer, not just one part of one message.

In short, with my limited elisp skills and even more limited time, I
don't see a nice solution this. That doesn't say anything about the
possible existence of a nice solution, though. ;)


BR,
Jani.

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

* [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-15 15:55 [PATCH] emacs: add function to toggle display of all multipart/alternative parts Jani Nikula
  2012-06-18 21:26 ` Jameson Graef Rollins
@ 2012-06-19 20:14 ` Jani Nikula
  2012-06-19 20:25   ` Jani Nikula
                     ` (2 more replies)
  1 sibling, 3 replies; 23+ messages in thread
From: Jani Nikula @ 2012-06-19 20:14 UTC (permalink / raw)
  To: notmuch

Add function notmuch-show-toggle-multipart-alternative to toggle the
value of notmuch-show-all-multipart/alternative-parts variable in the
buffer, and redisplay the thread with either all or preferred
alternative parts expanded.

A small wrinkle is that in order to make the variable survive
notmuch-show-refresh-view (which is required for expanding/collapsing
the parts, but ends up calling kill-all-local-variables through
notmuch-show-mode) it is necessary to give it the permanent-local
property.
---
 emacs/notmuch-show.el |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 36cad93..4d3f03f 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -98,6 +98,18 @@ any given message."
   "Should all parts of multipart/alternative parts be shown?"
   :type 'boolean
   :group 'notmuch-show)
+(make-variable-buffer-local 'notmuch-show-all-multipart/alternative-parts)
+(put 'notmuch-show-all-multipart/alternative-parts 'permanent-local t)
+
+(defun notmuch-show-toggle-multipart-alternative ()
+  "Toggle the display of all multipart/alternative parts."
+  (interactive)
+  (setq notmuch-show-all-multipart/alternative-parts
+	(not notmuch-show-all-multipart/alternative-parts))
+  (message (if notmuch-show-all-multipart/alternative-parts
+	       "Showing all multipart/alternative parts."
+	     "Showing preferred multipart/alternative part."))
+  (notmuch-show-refresh-view))
 
 (defcustom notmuch-show-indent-messages-width 1
   "Width of message indentation in threads.
-- 
1.7.9.5

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-19 20:14 ` [PATCH v2] " Jani Nikula
@ 2012-06-19 20:25   ` Jani Nikula
  2012-06-22 22:34   ` David Bremner
  2012-06-24 18:56   ` Mark Walters
  2 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2012-06-19 20:25 UTC (permalink / raw)
  To: notmuch

On Tue, 19 Jun 2012, Jani Nikula <jani@nikula.org> wrote:
> Add function notmuch-show-toggle-multipart-alternative to toggle the
> value of notmuch-show-all-multipart/alternative-parts variable in the
> buffer, and redisplay the thread with either all or preferred
> alternative parts expanded.

I see that Mark has some draft stuff for a more desirable approach, but
decided to send this anyway, as this is fairly small and ready. The v2
is now in line with the existing show mode toggles.

BR,
Jani.

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-19 20:14 ` [PATCH v2] " Jani Nikula
  2012-06-19 20:25   ` Jani Nikula
@ 2012-06-22 22:34   ` David Bremner
  2012-06-24  7:10     ` Jani Nikula
  2012-06-24 18:56   ` Mark Walters
  2 siblings, 1 reply; 23+ messages in thread
From: David Bremner @ 2012-06-22 22:34 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

>
> A small wrinkle is that in order to make the variable survive
> notmuch-show-refresh-view (which is required for expanding/collapsing
> the parts, but ends up calling kill-all-local-variables through
> notmuch-show-mode) it is necessary to give it the permanent-local
> property.

The code looks simple enough; should we apply this patch while we wait
for something fancier? I don't really know how to evaluate the
permanent-local bit.

d

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-22 22:34   ` David Bremner
@ 2012-06-24  7:10     ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2012-06-24  7:10 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

On Jun 23, 2012 1:34 AM, "David Bremner" <david@tethera.net> wrote:
>
> Jani Nikula <jani@nikula.org> writes:
>
> >
> > A small wrinkle is that in order to make the variable survive
> > notmuch-show-refresh-view (which is required for expanding/collapsing
> > the parts, but ends up calling kill-all-local-variables through
> > notmuch-show-mode) it is necessary to give it the permanent-local
> > property.
>
> The code looks simple enough; should we apply this patch while we wait
> for something fancier?

Applying this doesn't make the fancy stuff harder to do at all. And this
doesn't have a key binding, on purpose, so we're not committed to that
either. But then I'm biased...

> I don't really know how to evaluate the
> permanent-local bit.

All the other show mode toggles use that too. It's probably less surprising
like this than the permanently local only as needed approach in v1.

>
> d

[-- Attachment #2: Type: text/html, Size: 1187 bytes --]

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-19 20:14 ` [PATCH v2] " Jani Nikula
  2012-06-19 20:25   ` Jani Nikula
  2012-06-22 22:34   ` David Bremner
@ 2012-06-24 18:56   ` Mark Walters
  2012-10-19 16:14     ` [PATCH] Add NEWS item for multipart/alternative toggle Ethan Glasser-Camp
  2 siblings, 1 reply; 23+ messages in thread
From: Mark Walters @ 2012-06-24 18:56 UTC (permalink / raw)
  To: Jani Nikula, notmuch


On Tue, 19 Jun 2012, Jani Nikula <jani@nikula.org> wrote:
> Add function notmuch-show-toggle-multipart-alternative to toggle the
> value of notmuch-show-all-multipart/alternative-parts variable in the
> buffer, and redisplay the thread with either all or preferred
> alternative parts expanded.
>
> A small wrinkle is that in order to make the variable survive
> notmuch-show-refresh-view (which is required for expanding/collapsing
> the parts, but ends up calling kill-all-local-variables through
> notmuch-show-mode) it is necessary to give it the permanent-local
> property.

Hi 

This patch looks good to me with one small concern: I set the
variable notmuch-show-all-multipart/alternative-parts in my an emacs file
loaded from my .emacs file rather than through customize using 
(setq notmuch-show-all-multipart/alternative-parts nil)

This no longer works because of the buffer local property (ie I see all
parts): I need to use 
(setq-default notmuch-show-all-multipart/alternative-parts nil)

This "change" might be worth flagging up in the news file.

I do not think the existence of my semi-patch
id:"87pq8vokmp.fsf@qmul.ac.uk" should hold up this patch. My patch is
only a draft and I definitely do not know enough lisp to be able to
write a correct version (with correct quoting of message-ids etc).

Best wishes

Mark



> ---
>  emacs/notmuch-show.el |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 36cad93..4d3f03f 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -98,6 +98,18 @@ any given message."
>    "Should all parts of multipart/alternative parts be shown?"
>    :type 'boolean
>    :group 'notmuch-show)
> +(make-variable-buffer-local 'notmuch-show-all-multipart/alternative-parts)
> +(put 'notmuch-show-all-multipart/alternative-parts 'permanent-local t)
> +
> +(defun notmuch-show-toggle-multipart-alternative ()
> +  "Toggle the display of all multipart/alternative parts."
> +  (interactive)
> +  (setq notmuch-show-all-multipart/alternative-parts
> +	(not notmuch-show-all-multipart/alternative-parts))
> +  (message (if notmuch-show-all-multipart/alternative-parts
> +	       "Showing all multipart/alternative parts."
> +	     "Showing preferred multipart/alternative part."))
> +  (notmuch-show-refresh-view))
>  
>  (defcustom notmuch-show-indent-messages-width 1
>    "Width of message indentation in threads.
> -- 
> 1.7.9.5
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH] emacs: add function to toggle display of all multipart/alternative parts
  2012-06-19  6:56   ` Mark Walters
@ 2012-08-09 20:52     ` Jameson Graef Rollins
  2012-08-10  7:10       ` [PATCH v2] " Mark Walters
  0 siblings, 1 reply; 23+ messages in thread
From: Jameson Graef Rollins @ 2012-08-09 20:52 UTC (permalink / raw)
  To: Mark Walters, Jani Nikula, notmuch

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

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

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

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

* [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-08-09 20:52     ` Jameson Graef Rollins
@ 2012-08-10  7:10       ` Mark Walters
  2012-08-10 14:53         ` Jani Nikula
                           ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Mark Walters @ 2012-08-10  7:10 UTC (permalink / raw)
  To: Jameson Graef Rollins, Jani Nikula, notmuch


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

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-08-10  7:10       ` [PATCH v2] " Mark Walters
@ 2012-08-10 14:53         ` Jani Nikula
  2012-08-10 16:18           ` Jameson Graef Rollins
  2012-08-12 17:39         ` [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative Jameson Graef Rollins
                           ` (2 subsequent siblings)
  3 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2012-08-10 14:53 UTC (permalink / raw)
  To: Mark Walters, Jameson Graef Rollins, notmuch

On Fri, 10 Aug 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> 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.

How would this work together with something like [1] (rationale in [2])?

[1] id:"ab777cf0fa83778d3399ac52094df9230738819d.1328798471.git.jani@nikula.org"
[2] id:"cover.1328719309.git.jani@nikula.org"

If you introduce a mechanism to store the state, could it be extended to
store the state of each individual part? That, in turn, could be used to
add support for expanding/collapsing each alternative part through the
buttons (e.g. [ text/html (not shown) ]). Each button could toggle the
state of the part, and refresh buffer.

I guess basically the above are related. If you stored a list of parts
to display per each message id, the initial list could be created based
on customized regexps, the buttons could be used for toggling each
individual part (adding/removing the type from the list), and you could
have a function that would cycle the list to your heart's content.

BR,
Jani.



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

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-08-10 14:53         ` Jani Nikula
@ 2012-08-10 16:18           ` Jameson Graef Rollins
  2012-08-10 16:39             ` Jani Nikula
  0 siblings, 1 reply; 23+ messages in thread
From: Jameson Graef Rollins @ 2012-08-10 16:18 UTC (permalink / raw)
  To: Jani Nikula, Mark Walters, notmuch

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]

On Fri, Aug 10 2012, Jani Nikula <jani@nikula.org> wrote:
> How would this work together with something like [1] (rationale in [2])?
>
> [1] id:"ab777cf0fa83778d3399ac52094df9230738819d.1328798471.git.jani@nikula.org"
> [2] id:"cover.1328719309.git.jani@nikula.org"
>
> If you introduce a mechanism to store the state, could it be extended to
> store the state of each individual part? That, in turn, could be used to
> add support for expanding/collapsing each alternative part through the
> buttons (e.g. [ text/html (not shown) ]). Each button could toggle the
> state of the part, and refresh buffer.

Hey, Jani.  Are these patches needed if we have Mark's patch?  I would
prefer to see Mark's solution.  Since alternative parts are supposed to
be just that, alternative, it seems to me that a solution that would
cycle through display of these parts is really what we want.  Is there a
strong need to show multiple alternative parts at the exact same time?

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-08-10 16:18           ` Jameson Graef Rollins
@ 2012-08-10 16:39             ` Jani Nikula
  2012-08-10 16:58               ` Mark Walters
  0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2012-08-10 16:39 UTC (permalink / raw)
  To: Jameson Graef Rollins; +Cc: Notmuch Mail

[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]

On Aug 10, 2012 7:18 PM, "Jameson Graef Rollins" <jrollins@finestructure.net>
wrote:
>
> On Fri, Aug 10 2012, Jani Nikula <jani@nikula.org> wrote:
> > How would this work together with something like [1] (rationale in [2])?
> >
> > [1] id:"
ab777cf0fa83778d3399ac52094df9230738819d.1328798471.git.jani@nikula.org"
> > [2] id:"cover.1328719309.git.jani@nikula.org"
> >
> > If you introduce a mechanism to store the state, could it be extended to
> > store the state of each individual part? That, in turn, could be used to
> > add support for expanding/collapsing each alternative part through the
> > buttons (e.g. [ text/html (not shown) ]). Each button could toggle the
> > state of the part, and refresh buffer.
>
> Hey, Jani.  Are these patches needed if we have Mark's patch?  I would
> prefer to see Mark's solution.  Since alternative parts are supposed to
> be just that, alternative, it seems to me that a solution that would
> cycle through display of these parts is really what we want.  Is there a
> strong need to show multiple alternative parts at the exact same time?

Thanks to broken Microsoft mail clients, I get plenty of invitations that
have text/plain and text/calendar alternative parts with information
complimenting each other. I usually need to see both (luckily the included
html part I can ignore) and it's helpful if I can see them at the same
time. In a perfect world neither you or me would need any of this
functionality...

I suppose cycling through the alternative parts is, in a sense, correct for
the reasons you state, we have the code here to do just that, and I can
always cook up something for myself. Let's go with this, then, to move
forward.

BR,
Jani.

[-- Attachment #2: Type: text/html, Size: 2211 bytes --]

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-08-10 16:39             ` Jani Nikula
@ 2012-08-10 16:58               ` Mark Walters
  2012-08-10 17:49                 ` Jameson Graef Rollins
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Walters @ 2012-08-10 16:58 UTC (permalink / raw)
  To: Jani Nikula, Jameson Graef Rollins; +Cc: Notmuch Mail


On Fri, 10 Aug 2012, Jani Nikula <jani@nikula.org> wrote:
> On Aug 10, 2012 7:18 PM, "Jameson Graef Rollins" <jrollins@finestructure.net>
> wrote:
>>
>> On Fri, Aug 10 2012, Jani Nikula <jani@nikula.org> wrote:
>> > How would this work together with something like [1] (rationale in [2])?
>> >
>> > [1] id:"
> ab777cf0fa83778d3399ac52094df9230738819d.1328798471.git.jani@nikula.org"
>> > [2] id:"cover.1328719309.git.jani@nikula.org"
>> >
>> > If you introduce a mechanism to store the state, could it be extended to
>> > store the state of each individual part? That, in turn, could be used to
>> > add support for expanding/collapsing each alternative part through the
>> > buttons (e.g. [ text/html (not shown) ]). Each button could toggle the
>> > state of the part, and refresh buffer.
>>
>> Hey, Jani.  Are these patches needed if we have Mark's patch?  I would
>> prefer to see Mark's solution.  Since alternative parts are supposed to
>> be just that, alternative, it seems to me that a solution that would
>> cycle through display of these parts is really what we want.  Is there a
>> strong need to show multiple alternative parts at the exact same time?
>
> Thanks to broken Microsoft mail clients, I get plenty of invitations that
> have text/plain and text/calendar alternative parts with information
> complimenting each other. I usually need to see both (luckily the included
> html part I can ignore) and it's helpful if I can see them at the same
> time. In a perfect world neither you or me would need any of this
> functionality...
>
> I suppose cycling through the alternative parts is, in a sense, correct for
> the reasons you state, we have the code here to do just that, and I can
> always cook up something for myself. Let's go with this, then, to move
> forward.

Hi

I am not sure I agree: I think maybe toggling parts is better. Either
the parts contain the same information and then the current behaviour is
probably fine, or they are not in which case we might want to see both
at once

Best wishes

Mark

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-08-10 16:58               ` Mark Walters
@ 2012-08-10 17:49                 ` Jameson Graef Rollins
  0 siblings, 0 replies; 23+ messages in thread
From: Jameson Graef Rollins @ 2012-08-10 17:49 UTC (permalink / raw)
  To: Mark Walters, Jani Nikula; +Cc: Notmuch Mail

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Fri, Aug 10 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> I am not sure I agree: I think maybe toggling parts is better. Either
> the parts contain the same information and then the current behaviour is
> probably fine, or they are not in which case we might want to see both
> at once

If the case we're talking about is the alternative part having
additional information, then why is cycling not sufficient?  Shouldn't I
be able to cycle to a part that shows all the information I'm interested
in?  I assume that even in the most broken mailers the alternatives
still show mostly redundant information.

I really like the cycling behavior, but If this solution is really not
enough then how about we just start off with the alternate parts
collapsed, and you can click on them to open them.  I don't really like
the idea of having special code to handle specific alternative parts.
That seems overly complicated to me.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

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

* [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative
  2012-08-10  7:10       ` [PATCH v2] " Mark Walters
  2012-08-10 14:53         ` Jani Nikula
@ 2012-08-12 17:39         ` Jameson Graef Rollins
  2012-08-12 17:39           ` [PATCH 2/2] test: move all emacs show tests to emacs-show test script Jameson Graef Rollins
  2012-10-20  2:31           ` [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative Ethan Glasser-Camp
  2012-10-09 22:21         ` [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts Jeremy Nickurak
  2012-10-19 16:36         ` Ethan Glasser-Camp
  3 siblings, 2 replies; 23+ messages in thread
From: Jameson Graef Rollins @ 2012-08-12 17:39 UTC (permalink / raw)
  To: Notmuch Mail

Fairly simple test that looks at a diff of test-emacs output from each
case.
---
As I mentioned, I really like this toggle view solution, so here's a
test for it.

 test/emacs-show |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/test/emacs-show b/test/emacs-show
index e9a714f..695f929 100755
--- a/test/emacs-show
+++ b/test/emacs-show
@@ -36,4 +36,51 @@ test_begin_subtest "Bare subject #3"
 output=$(test_emacs '(notmuch-show-strip-re "the cure: fix the regexp")')
 test_expect_equal "$output" '"the cure: fix the regexp"'
 
+test_begin_subtest "Toggle display multipart/alternative"
+cat <<EOF > ${MAIL_DIR}/embedded_message
+From: Carl Worth <cworth@cworth.org>
+To: cworth@cworth.org
+Subject: html message
+Date: Fri, 05 Jan 2001 15:42:57 +0000
+User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
+Message-ID: <87liy5ap01.fsf@yoom.home.cworth.org>
+MIME-Version: 1.0
+Content-Type: multipart/alternative; boundary="==-=-=="
+
+--==-=-==
+Content-Type: text/html
+
+<p>This is the text/html part of a multipart/alternative.</p>
+
+--==-=-==
+Content-Type: text/plain
+
+This is the text/plain part of a multipart/alternative.
+
+--==-=-==--
+EOF
+notmuch new > /dev/null
+test_emacs "(let ((notmuch-show-all-multipart/alternative-parts nil))
+              (notmuch-show \"id:87liy5ap01.fsf@yoom.home.cworth.org\")
+	      (test-output))"
+mv OUTPUT{,.text}
+test_emacs "(let ((notmuch-show-all-multipart/alternative-parts nil))
+              (notmuch-show \"id:87liy5ap01.fsf@yoom.home.cworth.org\")
+              (notmuch-show-cycle-message-multipart)
+	      (test-output))"
+mv OUTPUT{,.html}
+diff OUTPUT.{text,html} >OUTPUT.diff
+cat <<EOF >EXPECTED.diff
+7,9c7,10
+< [ text/html (not shown) ]
+< [ text/plain ]
+< This is the text/plain part of a multipart/alternative.
+---
+> [ text/html ]
+> This is the text/html part of a multipart/alternative.
+> 
+> [ text/plain (not shown) ]
+EOF
+test_expect_equal_file {OUTPUT,EXPECTED}.diff
+
 test_done
-- 
1.7.10.4

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

* [PATCH 2/2] test: move all emacs show tests to emacs-show test script
  2012-08-12 17:39         ` [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative Jameson Graef Rollins
@ 2012-08-12 17:39           ` Jameson Graef Rollins
  2012-08-31 19:44             ` David Bremner
  2012-10-20  2:31           ` [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative Ethan Glasser-Camp
  1 sibling, 1 reply; 23+ messages in thread
From: Jameson Graef Rollins @ 2012-08-12 17:39 UTC (permalink / raw)
  To: Notmuch Mail

No functional change.

Most notmuch-show mode tests were in the emacs script, while some were
in the emacs-show script.  This moves all the notmuch-show mode tests
to the emacs-show script, to make things a little more consistent.
---
This patch is not necessary, but I think it makes the emacs tests
clearer, by not having tests for show-mode split across multiple test
scripts.

 test/emacs      |  397 +----------------------------------------------------
 test/emacs-show |  409 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 410 insertions(+), 396 deletions(-)

diff --git a/test/emacs b/test/emacs
index afe35ba..9cb5795 100755
--- a/test/emacs
+++ b/test/emacs
@@ -54,49 +54,6 @@ test_emacs '(notmuch-hello)
 	    (test-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-view-inbox
 
-test_begin_subtest "Basic notmuch-show view in emacs"
-maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
-test_emacs "(notmuch-show \"$maildir_storage_thread\")
-	    (test-output)"
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
-
-test_begin_subtest "Basic notmuch-show view in emacs default indentation"
-maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
-test_emacs "(let ((notmuch-show-indent-messages-width 1))
-	      (notmuch-show \"$maildir_storage_thread\")
-	      (test-output))"
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
-
-test_begin_subtest "Basic notmuch-show view in emacs without indentation"
-maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
-test_emacs "(let ((notmuch-show-indent-messages-width 0))
-	      (notmuch-show \"$maildir_storage_thread\")
-	      (test-output))"
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage-without-indentation
-
-test_begin_subtest "Basic notmuch-show view in emacs with fourfold indentation"
-maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
-test_emacs "(let ((notmuch-show-indent-messages-width 4))
-	      (notmuch-show \"$maildir_storage_thread\")
-	      (test-output))"
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage-with-fourfold-indentation
-
-test_begin_subtest "notmuch-show for message with invalid From"
-add_message "[subject]=\"message-with-invalid-from\"" \
-	    "[from]=\"\\\"Invalid \\\" From\\\" <test_suite@notmuchmail.org>\""
-thread=$(notmuch search --output=threads subject:message-with-invalid-from)
-test_emacs "(notmuch-show \"$thread\")
-	    (test-output)"
-cat <<EOF >EXPECTED
-"Invalid " (2001-01-05) (inbox)
-Subject: message-with-invalid-from
-To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 05 Jan 2001 15:43:57 +0000
-
-This is just a test message (#1)
-EOF
-test_expect_equal_file OUTPUT EXPECTED
-
 test_begin_subtest "Navigation of notmuch-search to thread view"
 test_emacs '(notmuch-search "tag:inbox")
 	    (notmuch-test-wait)
@@ -122,18 +79,6 @@ test_emacs "(notmuch-search \"$os_x_darwin_thread\")
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
-test_begin_subtest "Add tag from notmuch-show view"
-test_emacs "(notmuch-show \"$os_x_darwin_thread\")
-	    (execute-kbd-macro \"+tag-from-show-view\")"
-output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-show-view unread)"
-
-test_begin_subtest "Remove tag from notmuch-show view"
-test_emacs "(notmuch-show \"$os_x_darwin_thread\")
-	    (execute-kbd-macro \"-tag-from-show-view\")"
-output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
-
 test_begin_subtest "Message with .. in Message-Id:"
 add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
 test_emacs '(notmuch-search "id:\"123..456@example\"")
@@ -351,248 +296,7 @@ Sender <sender@example.com> writes:
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "Reply within emacs to a multipart/mixed message"
-test_emacs '(let ((message-hidden-headers ''()))
-	    (notmuch-show "id:20091118002059.067214ed@hikari")
-		(notmuch-show-reply)
-		(test-output))'
-sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' OUTPUT
-cat <<EOF >EXPECTED
-From: Notmuch Test Suite <test_suite@notmuchmail.org>
-To: Adrian Perez de Castro <aperez@igalia.com>, notmuch@notmuchmail.org
-Subject: Re: [notmuch] Introducing myself
-In-Reply-To: <20091118002059.067214ed@hikari>
-Fcc: ${MAIL_DIR}/sent
-References: <20091118002059.067214ed@hikari>
-User-Agent: Notmuch/XXX Emacs/XXX
---text follows this line--
-Adrian Perez de Castro <aperez@igalia.com> writes:
-
-> Hello to all,
->
-> I have just heard about Not Much today in some random Linux-related news
-> site (LWN?), my name is Adrian Perez and I work as systems administrator
-> (although I can do some code as well :P). I have always thought that the
-> ideas behind Sup were great, but after some time using it, I got tired of
-> the oddities that it has. I also do not like doing things like having to
-> install Ruby just for reading and sorting mails. Some time ago I thought
-> about doing something like Not Much and in fact I played a bit with the
-> Python+Xapian and the Python+Whoosh combinations, because I find relaxing
-> to code things in Python when I am not working and also it is installed
-> by default on most distribution. I got to have some mailboxes indexed and
-> basic searching working a couple of months ago. Lately I have been very
-> busy and had no time for coding, and them... boom! Not Much appears -- and
-> it is almost exactly what I was trying to do, but faster. I have been
-> playing a bit with Not Much today, and I think it has potential.
->
-> Also, I would like to share one idea I had in mind, that you might find
-> interesting: One thing I have found very annoying is having to re-tag my
-> mail when the indexes get b0rked (it happened a couple of times to me while
-> using Sup), so I was planning to mails as read/unread and adding the tags
-> not just to the index, but to the mail text itself, e.g. by adding a
-> "X-Tags" header field or by reusing the "Keywords" one. This way, the index
-> could be totally recreated by re-reading the mail directories, and this
-> would also allow to a tools like OfflineIMAP [1] to get the mails into a
-> local maildir, tagging and indexing the mails with the e-mail reader and
-> then syncing back the messages with the "X-Tags" header to the IMAP server.
-> This would allow to use the mail reader from a different computer and still
-> have everything tagged finely.
->
-> Best regards,
->
->
-> ---
-> [1] http://software.complete.org/software/projects/show/offlineimap
->
-> -- 
-> Adrian Perez de Castro <aperez@igalia.com>
-> Igalia - Free Software Engineering
-> _______________________________________________
-> notmuch mailing list
-> notmuch@notmuchmail.org
-> http://notmuchmail.org/mailman/listinfo/notmuch
-EOF
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Reply within emacs to a multipart/alternative message"
-test_emacs '(let ((message-hidden-headers ''()))
-	    (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
-		(notmuch-show-reply)
-		(test-output))'
-sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' OUTPUT
-cat <<EOF >EXPECTED
-From: Notmuch Test Suite <test_suite@notmuchmail.org>
-To: Alex Botero-Lowry <alex.boterolowry@gmail.com>, notmuch@notmuchmail.org
-Subject: Re: [notmuch] preliminary FreeBSD support
-In-Reply-To: <cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com>
-Fcc: ${MAIL_DIR}/sent
-References: <cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com>
-User-Agent: Notmuch/XXX Emacs/XXX
---text follows this line--
-Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
-
-> I saw the announcement this morning, and was very excited, as I had been
-> hoping sup would be turned into a library,
-> since I like the concept more than the UI (I'd rather an emacs interface).
->
-> I did a preliminary compile which worked out fine, but
-> sysconf(_SC_SC_GETPW_R_SIZE_MAX) returns -1 on
-> FreeBSD, so notmuch_config_open segfaulted.
->
-> Attached is a patch that supplies a default buffer size of 64 in cases where
-> -1 is returned.
->
-> http://www.opengroup.org/austin/docs/austin_328.txt - seems to indicate this
-> is acceptable behavior,
-> and http://mail-index.netbsd.org/pkgsrc-bugs/2006/06/07/msg016808.htmlspecifically
-> uses 64 as the
-> buffer size.
-> _______________________________________________
-> notmuch mailing list
-> notmuch@notmuchmail.org
-> http://notmuchmail.org/mailman/listinfo/notmuch
-EOF
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Reply within emacs to an html-only message"
-add_message '[content-type]="text/html"' \
-	    '[body]="Hi,<br />This is an <b>HTML</b> test message.<br /><br />OK?"'
-test_emacs "(let ((message-hidden-headers '()) (mm-text-html-renderer 'html2text))
-	    (notmuch-show \"id:${gen_msg_id}\")
-	    (notmuch-show-reply)
-	    (test-output))"
-sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' OUTPUT
-cat <<EOF >EXPECTED
-From: Notmuch Test Suite <test_suite@notmuchmail.org>
-To: 
-Subject: Re: Reply within emacs to an html-only message
-In-Reply-To: <${gen_msg_id}>
-Fcc: ${MAIL_DIR}/sent
-References: <${gen_msg_id}>
-User-Agent: Notmuch/XXX Emacs/XXX
---text follows this line--
-Notmuch Test Suite <test_suite@notmuchmail.org> writes:
-
-> Hi,This is an HTML test message.OK?
-EOF
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Quote MML tags in reply"
-message_id='test-emacs-mml-quoting@message.id'
-add_message [id]="$message_id" \
-	    "[subject]='$test_subtest_name'" \
-	    '[body]="<#part disposition=inline>"'
-test_emacs "(let ((message-hidden-headers '()))
-	      (notmuch-show \"id:$message_id\")
-	      (notmuch-show-reply)
-	      (test-output))"
-sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' OUTPUT
-cat <<EOF >EXPECTED
-From: Notmuch Test Suite <test_suite@notmuchmail.org>
-To: 
-Subject: Re: Quote MML tags in reply
-In-Reply-To: <test-emacs-mml-quoting@message.id>
-Fcc: ${MAIL_DIR}/sent
-References: <test-emacs-mml-quoting@message.id>
-User-Agent: Notmuch/XXX Emacs/XXX
---text follows this line--
-Notmuch Test Suite <test_suite@notmuchmail.org> writes:
-
-> <#!part disposition=inline>
-EOF
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
-# save as archive to test that Emacs does not re-compress .gz
-test_emacs '(let ((standard-input "\"attachment1.gz\""))
-	      (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
-	      (notmuch-show-save-attachments))'
-test_expect_equal_file attachment1.gz "$EXPECTED/attachment"
-
-test_begin_subtest "Save attachment from within emacs using notmuch-show-save-part"
-# save as archive to test that Emacs does not re-compress .gz
-test_emacs '(let ((standard-input "\"attachment2.gz\""))
-	      (notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5))'
-test_expect_equal_file attachment2.gz "$EXPECTED/attachment"
-
-test_begin_subtest "View raw message within emacs"
-test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
-	    (notmuch-show-view-raw-message)
-	    (test-output)'
-test_expect_equal_file OUTPUT $EXPECTED/raw-message-cf0c4d-52ad0a
-
-test_begin_subtest "Hiding/showing signature in notmuch-show view"
-maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
-test_emacs "(notmuch-show \"$maildir_storage_thread\")
-	    (search-forward \"Click/Enter to show.\")
-	    (button-activate (button-at (point)))
-	    (search-backward \"Click/Enter to hide.\")
-	    (button-activate (button-at (point)))
-	    (test-output)"
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
-
-test_begin_subtest "Detection and hiding of top-post quoting of message"
-add_message '[subject]="The problem with top-posting"' \
-	    [id]=top-post-target \
-	    '[body]="A: Because it messes up the order in which people normally read text.
-Q: Why is top-posting such a bad thing?
-A: Top-posting.
-Q: What is the most annoying thing in e-mail?"'
-add_message '[from]="Top Poster <top@poster.com>"' \
-	    [in-reply-to]=top-post-target \
-	    [references]=top-post-target \
-	    '[subject]="Re: The problem with top-posting"' \
-	    '[body]="Thanks for the advice! I will be sure to put it to good use.
-
--Top Poster
-
------ Original Message -----
-From: Notmuch Test Suite <test_suite@notmuchmail.org>
-To: Notmuch Test Suite <test_suite@notmuchmai.org>
-Sent: Fri, 05 Jan 2001 15:43:57 +0000
-Subject: The problem with top-posting
-
-Q: Why is top-posting such a bad thing?
-A: Top-posting.
-Q: What is the most annoying thing in e-mail?"'
-test_emacs "(notmuch-show \"top-posting\")
-	    (test-visible-output)"
-echo "Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
-Subject: The problem with top-posting
-To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 05 Jan 2001 15:43:57 +0000
-
-A: Because it messes up the order in which people normally read text.
-Q: Why is top-posting such a bad thing?
-A: Top-posting.
-Q: What is the most annoying thing in e-mail?
-Top Poster <top@poster.com> (2001-01-05) (inbox unread)
-Subject: Re: The problem with top-posting
-To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 05 Jan 2001 15:43:57 +0000
-
-Thanks for the advice! I will be sure to put it to good use.
-
--Top Poster
-
-[ 9-line hidden original message. Click/Enter to show. ]" > EXPECTED
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Hiding message in notmuch-show view"
-test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-	    (notmuch-show-toggle-message)
-	    (test-visible-output)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
-
-test_begin_subtest "Hiding message with visible citation in notmuch-show view"
-test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-	    (search-forward "Click/Enter to show.")
-	    (button-activate (button-at (point)))
-	    (notmuch-show-toggle-message)
-	    (test-visible-output)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
-
-test_begin_subtest "Stashing in notmuch-show"
+test_begin_subtest "Stashing in notmuch-search"
 add_message '[date]="Sat, 01 Jan 2000 12:00:00 -0000"' \
     '[from]="Some One <someone@somewhere.org>"' \
     '[to]="Some One Else <notsomeone@somewhere.org>"' \
@@ -601,44 +305,6 @@ add_message '[date]="Sat, 01 Jan 2000 12:00:00 -0000"' \
     '[id]="bought"' \
     '[body]="Unable to stash body. Where did you get it in the first place?!?"'
 notmuch tag +stashtest id:${gen_msg_id}
-test_emacs '(notmuch-show "id:\"bought\"")
-	(notmuch-show-stash-date)
-	(notmuch-show-stash-from)
-	(notmuch-show-stash-to)
-	(notmuch-show-stash-cc)
-	(notmuch-show-stash-subject)
-	(notmuch-show-stash-message-id)
-	(notmuch-show-stash-message-id-stripped)
-	(notmuch-show-stash-tags)
-	(notmuch-show-stash-filename)
-	(notmuch-show-stash-mlarchive-link "Gmane")
-	(notmuch-show-stash-mlarchive-link "MARC")
-	(notmuch-show-stash-mlarchive-link "Mail Archive, The")
-	(switch-to-buffer
-	  (generate-new-buffer "*test-stashing*"))
-	(dotimes (i 12)
-	  (yank)
-	  (insert "\n")
-	  (rotate-yank-pointer 1))
-	(reverse-region (point-min) (point-max))
-	    (test-output)'
-cat <<EOF >EXPECTED
-Sat, 01 Jan 2000 12:00:00 +0000
-Some One <someone@somewhere.org>
-Some One Else <notsomeone@somewhere.org>
-Notmuch <notmuch@notmuchmail.org>
-Stash my stashables
-id:"bought"
-bought
-inbox,stashtest
-${gen_msg_filename}
-http://mid.gmane.org/bought
-http://marc.info/?i=bought
-http://mail-archive.com/search?l=mid&q=bought
-EOF
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Stashing in notmuch-search"
 test_emacs '(notmuch-search "id:\"bought\"")
 	(notmuch-test-wait)
 	(notmuch-search-stash-thread-id)
@@ -649,67 +315,6 @@ test_emacs '(notmuch-search "id:\"bought\"")
 sed -i -e 's/^thread:.*$/thread:XXX/' OUTPUT
 test_expect_equal "$(cat OUTPUT)" "thread:XXX"
 
-test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature'
-message1='id:20091118010116.GC25380@dottiness.seas.harvard.edu'
-message2='id:1258491078-29658-1-git-send-email-dottedmag@dottedmag.net'
-test_emacs "(notmuch-search \"$message1 or $message2\")
-	    (notmuch-test-wait)
-	    (notmuch-search-show-thread)
-	    (goto-char (point-max))
-	    (redisplay)
-	    (notmuch-show-advance-and-archive)
-	    (test-output)"
-test_emacs "(notmuch-show \"$message2\")
-	    (test-output \"EXPECTED\")"
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Refresh show buffer"
-test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-	    (test-visible-output "EXPECTED")
-	    (notmuch-show-refresh-view)
-	    (test-visible-output)'
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Refresh modified show buffer"
-test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-	    (notmuch-show-toggle-message)
-	    (notmuch-show-next-message)
-	    (notmuch-show-toggle-message)
-	    (test-visible-output "EXPECTED")
-	    (notmuch-show-refresh-view)
-	    (test-visible-output)'
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Do not call notmuch for non-inlinable application/mpeg parts"
-id='message-with-application/mpeg-attachment@notmuchmail.org'
-emacs_deliver_message \
-    'Message with application/mpeg attachment' \
-    '' \
-    "(message-goto-eoh)
-     (insert \"Message-ID: <$id>\n\")
-     (message-goto-body)
-     (mml-insert-part \"application/mpeg\")
-     (insert \"a fake mp3 file\")"
-notmuch_counter_reset
-test_emacs "(let ((notmuch-command \"$notmuch_counter_command\"))
-	      (notmuch-show \"id:$id\"))"
-test_expect_equal $(notmuch_counter_value) 1
-
-test_begin_subtest "Do not call notmuch for non-inlinable audio/mpeg parts"
-id='message-with-audio/mpeg-attachment@notmuchmail.org'
-emacs_deliver_message \
-    'Message with audio/mpeg attachment' \
-    '' \
-    "(message-goto-eoh)
-     (insert \"Message-ID: <$id>\n\")
-     (message-goto-body)
-     (mml-insert-part \"audio/mpeg\")
-     (insert \"a fake mp3 file\")"
-notmuch_counter_reset
-test_emacs "(let ((notmuch-command \"$notmuch_counter_command\"))
-	      (notmuch-show \"id:$id\"))"
-test_expect_equal $(notmuch_counter_value) 1
-
 test_begin_subtest "notmuch-hello-mode hook is called"
 counter=$(test_emacs \
     '(let ((notmuch-hello-mode-hook-counter 0))
diff --git a/test/emacs-show b/test/emacs-show
index 695f929..923a808 100755
--- a/test/emacs-show
+++ b/test/emacs-show
@@ -3,6 +3,67 @@
 test_description="emacs notmuch-show view"
 . test-lib.sh
 
+EXPECTED=$TEST_DIRECTORY/emacs.expected-output
+
+add_email_corpus
+
+test_begin_subtest "Basic notmuch-show view"
+maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
+test_emacs "(notmuch-show \"$maildir_storage_thread\")
+	    (test-output)"
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
+
+test_begin_subtest "Basic notmuch-show view default indentation"
+maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
+test_emacs "(let ((notmuch-show-indent-messages-width 1))
+	      (notmuch-show \"$maildir_storage_thread\")
+	      (test-output))"
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
+
+test_begin_subtest "Basic notmuch-show view without indentation"
+maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
+test_emacs "(let ((notmuch-show-indent-messages-width 0))
+	      (notmuch-show \"$maildir_storage_thread\")
+	      (test-output))"
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage-without-indentation
+
+test_begin_subtest "Basic notmuch-show view with fourfold indentation"
+maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
+test_emacs "(let ((notmuch-show-indent-messages-width 4))
+	      (notmuch-show \"$maildir_storage_thread\")
+	      (test-output))"
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage-with-fourfold-indentation
+
+test_begin_subtest "Message with invalid From"
+add_message "[subject]=\"message-with-invalid-from\"" \
+	    "[from]=\"\\\"Invalid \\\" From\\\" <test_suite@notmuchmail.org>\""
+thread=$(notmuch search --output=threads subject:message-with-invalid-from)
+test_emacs "(notmuch-show \"$thread\")
+	    (test-output)"
+cat <<EOF >EXPECTED
+"Invalid " (2001-01-05) (inbox)
+Subject: message-with-invalid-from
+To: Notmuch Test Suite <test_suite@notmuchmail.org>
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+
+This is just a test message (#1)
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+os_x_darwin_thread=$(notmuch search --output=threads id:ddd65cda0911171950o4eea4389v86de9525e46052d3@mail.gmail.com)
+
+test_begin_subtest "Add tag"
+test_emacs "(notmuch-show \"$os_x_darwin_thread\")
+	    (execute-kbd-macro \"+tag-from-show-view\")"
+output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-show-view unread)"
+
+test_begin_subtest "Remove tag"
+test_emacs "(notmuch-show \"$os_x_darwin_thread\")
+	    (execute-kbd-macro \"-tag-from-show-view\")"
+output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
+
 test_begin_subtest "Hiding Original Message region at beginning of a message"
 message_id='OriginalMessageHiding.1@notmuchmail.org'
 add_message \
@@ -24,6 +85,172 @@ test_emacs "(notmuch-show \"id:$message_id\")
 	    (test-visible-output)"
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Do not call notmuch for non-inlinable application/mpeg parts"
+id='message-with-application/mpeg-attachment@notmuchmail.org'
+emacs_deliver_message \
+    'Message with application/mpeg attachment' \
+    '' \
+    "(message-goto-eoh)
+     (insert \"Message-ID: <$id>\n\")
+     (message-goto-body)
+     (mml-insert-part \"application/mpeg\")
+     (insert \"a fake mp3 file\")"
+notmuch_counter_reset
+test_emacs "(let ((notmuch-command \"$notmuch_counter_command\"))
+	      (notmuch-show \"id:$id\"))"
+test_expect_equal $(notmuch_counter_value) 1
+
+test_begin_subtest "Do not call notmuch for non-inlinable audio/mpeg parts"
+id='message-with-audio/mpeg-attachment@notmuchmail.org'
+emacs_deliver_message \
+    'Message with audio/mpeg attachment' \
+    '' \
+    "(message-goto-eoh)
+     (insert \"Message-ID: <$id>\n\")
+     (message-goto-body)
+     (mml-insert-part \"audio/mpeg\")
+     (insert \"a fake mp3 file\")"
+notmuch_counter_reset
+test_emacs "(let ((notmuch-command \"$notmuch_counter_command\"))
+	      (notmuch-show \"id:$id\"))"
+test_expect_equal $(notmuch_counter_value) 1
+
+test_begin_subtest "Save attachment using notmuch-show-save-attachments"
+# save as archive to test that Emacs does not re-compress .gz
+test_emacs '(let ((standard-input "\"attachment1.gz\""))
+	      (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
+	      (notmuch-show-save-attachments))'
+test_expect_equal_file attachment1.gz "$EXPECTED/attachment"
+
+test_begin_subtest "Save attachment using notmuch-show-save-part"
+# save as archive to test that Emacs does not re-compress .gz
+test_emacs '(let ((standard-input "\"attachment2.gz\""))
+	      (notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5))'
+test_expect_equal_file attachment2.gz "$EXPECTED/attachment"
+
+test_begin_subtest "View raw message"
+test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
+	    (notmuch-show-view-raw-message)
+	    (test-output)'
+test_expect_equal_file OUTPUT $EXPECTED/raw-message-cf0c4d-52ad0a
+
+test_begin_subtest "Hide/show signature"
+maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
+test_emacs "(notmuch-show \"$maildir_storage_thread\")
+	    (search-forward \"Click/Enter to show.\")
+	    (button-activate (button-at (point)))
+	    (search-backward \"Click/Enter to hide.\")
+	    (button-activate (button-at (point)))
+	    (test-output)"
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
+
+test_begin_subtest "Detection and hiding of top-post quoting of message"
+add_message '[subject]="The problem with top-posting"' \
+	    [id]=top-post-target \
+	    '[body]="A: Because it messes up the order in which people normally read text.
+Q: Why is top-posting such a bad thing?
+A: Top-posting.
+Q: What is the most annoying thing in e-mail?"'
+add_message '[from]="Top Poster <top@poster.com>"' \
+	    [in-reply-to]=top-post-target \
+	    [references]=top-post-target \
+	    '[subject]="Re: The problem with top-posting"' \
+	    '[body]="Thanks for the advice! I will be sure to put it to good use.
+
+-Top Poster
+
+----- Original Message -----
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: Notmuch Test Suite <test_suite@notmuchmai.org>
+Sent: Fri, 05 Jan 2001 15:43:57 +0000
+Subject: The problem with top-posting
+
+Q: Why is top-posting such a bad thing?
+A: Top-posting.
+Q: What is the most annoying thing in e-mail?"'
+test_emacs "(notmuch-show \"top-posting\")
+	    (test-visible-output)"
+echo "Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
+Subject: The problem with top-posting
+To: Notmuch Test Suite <test_suite@notmuchmail.org>
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+
+A: Because it messes up the order in which people normally read text.
+Q: Why is top-posting such a bad thing?
+A: Top-posting.
+Q: What is the most annoying thing in e-mail?
+Top Poster <top@poster.com> (2001-01-05) (inbox unread)
+Subject: Re: The problem with top-posting
+To: Notmuch Test Suite <test_suite@notmuchmail.org>
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+
+Thanks for the advice! I will be sure to put it to good use.
+
+-Top Poster
+
+[ 9-line hidden original message. Click/Enter to show. ]" > EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Hiding message"
+test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	    (notmuch-show-toggle-message)
+	    (test-visible-output)'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
+
+test_begin_subtest "Hiding message with visible citation"
+test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	    (search-forward "Click/Enter to show.")
+	    (button-activate (button-at (point)))
+	    (notmuch-show-toggle-message)
+	    (test-visible-output)'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
+
+test_begin_subtest "Stash commands"
+add_message '[date]="Sat, 01 Jan 2000 12:00:00 -0000"' \
+    '[from]="Some One <someone@somewhere.org>"' \
+    '[to]="Some One Else <notsomeone@somewhere.org>"' \
+    '[cc]="Notmuch <notmuch@notmuchmail.org>"' \
+    '[subject]="Stash my stashables"' \
+    '[id]="bought"' \
+    '[body]="Unable to stash body. Where did you get it in the first place?!?"'
+notmuch tag +stashtest id:${gen_msg_id}
+test_emacs '(notmuch-show "id:\"bought\"")
+	(notmuch-show-stash-date)
+	(notmuch-show-stash-from)
+	(notmuch-show-stash-to)
+	(notmuch-show-stash-cc)
+	(notmuch-show-stash-subject)
+	(notmuch-show-stash-message-id)
+	(notmuch-show-stash-message-id-stripped)
+	(notmuch-show-stash-tags)
+	(notmuch-show-stash-filename)
+	(notmuch-show-stash-mlarchive-link "Gmane")
+	(notmuch-show-stash-mlarchive-link "MARC")
+	(notmuch-show-stash-mlarchive-link "Mail Archive, The")
+	(switch-to-buffer
+	  (generate-new-buffer "*test-stashing*"))
+	(dotimes (i 12)
+	  (yank)
+	  (insert "\n")
+	  (rotate-yank-pointer 1))
+	(reverse-region (point-min) (point-max))
+	    (test-output)'
+cat <<EOF >EXPECTED
+Sat, 01 Jan 2000 12:00:00 +0000
+Some One <someone@somewhere.org>
+Some One Else <notsomeone@somewhere.org>
+Notmuch <notmuch@notmuchmail.org>
+Stash my stashables
+id:"bought"
+bought
+inbox,stashtest
+${gen_msg_filename}
+http://mid.gmane.org/bought
+http://marc.info/?i=bought
+http://mail-archive.com/search?l=mid&q=bought
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Bare subject #1"
 output=$(test_emacs '(notmuch-show-strip-re "Re: subject")')
 test_expect_equal "$output" '"subject"'
@@ -36,6 +263,37 @@ test_begin_subtest "Bare subject #3"
 output=$(test_emacs '(notmuch-show-strip-re "the cure: fix the regexp")')
 test_expect_equal "$output" '"the cure: fix the regexp"'
 
+test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature'
+message1='id:20091118010116.GC25380@dottiness.seas.harvard.edu'
+message2='id:1258491078-29658-1-git-send-email-dottedmag@dottedmag.net'
+test_emacs "(notmuch-search \"$message1 or $message2\")
+	    (notmuch-test-wait)
+	    (notmuch-search-show-thread)
+	    (goto-char (point-max))
+	    (redisplay)
+	    (notmuch-show-advance-and-archive)
+	    (test-output)"
+test_emacs "(notmuch-show \"$message2\")
+	    (test-output \"EXPECTED\")"
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Refresh show buffer"
+test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	    (test-visible-output "EXPECTED")
+	    (notmuch-show-refresh-view)
+	    (test-visible-output)'
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Refresh modified show buffer"
+test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	    (notmuch-show-toggle-message)
+	    (notmuch-show-next-message)
+	    (notmuch-show-toggle-message)
+	    (test-visible-output "EXPECTED")
+	    (notmuch-show-refresh-view)
+	    (test-visible-output)'
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Toggle display multipart/alternative"
 cat <<EOF > ${MAIL_DIR}/embedded_message
 From: Carl Worth <cworth@cworth.org>
@@ -83,4 +341,155 @@ cat <<EOF >EXPECTED.diff
 EOF
 test_expect_equal_file {OUTPUT,EXPECTED}.diff
 
+test_begin_subtest "Reply within emacs to a multipart/mixed message"
+test_emacs '(let ((message-hidden-headers ''()))
+	    (notmuch-show "id:20091118002059.067214ed@hikari")
+		(notmuch-show-reply)
+		(test-output))'
+sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: Adrian Perez de Castro <aperez@igalia.com>, notmuch@notmuchmail.org
+Subject: Re: [notmuch] Introducing myself
+In-Reply-To: <20091118002059.067214ed@hikari>
+Fcc: ${MAIL_DIR}/sent
+References: <20091118002059.067214ed@hikari>
+User-Agent: Notmuch/XXX Emacs/XXX
+--text follows this line--
+Adrian Perez de Castro <aperez@igalia.com> writes:
+
+> Hello to all,
+>
+> I have just heard about Not Much today in some random Linux-related news
+> site (LWN?), my name is Adrian Perez and I work as systems administrator
+> (although I can do some code as well :P). I have always thought that the
+> ideas behind Sup were great, but after some time using it, I got tired of
+> the oddities that it has. I also do not like doing things like having to
+> install Ruby just for reading and sorting mails. Some time ago I thought
+> about doing something like Not Much and in fact I played a bit with the
+> Python+Xapian and the Python+Whoosh combinations, because I find relaxing
+> to code things in Python when I am not working and also it is installed
+> by default on most distribution. I got to have some mailboxes indexed and
+> basic searching working a couple of months ago. Lately I have been very
+> busy and had no time for coding, and them... boom! Not Much appears -- and
+> it is almost exactly what I was trying to do, but faster. I have been
+> playing a bit with Not Much today, and I think it has potential.
+>
+> Also, I would like to share one idea I had in mind, that you might find
+> interesting: One thing I have found very annoying is having to re-tag my
+> mail when the indexes get b0rked (it happened a couple of times to me while
+> using Sup), so I was planning to mails as read/unread and adding the tags
+> not just to the index, but to the mail text itself, e.g. by adding a
+> "X-Tags" header field or by reusing the "Keywords" one. This way, the index
+> could be totally recreated by re-reading the mail directories, and this
+> would also allow to a tools like OfflineIMAP [1] to get the mails into a
+> local maildir, tagging and indexing the mails with the e-mail reader and
+> then syncing back the messages with the "X-Tags" header to the IMAP server.
+> This would allow to use the mail reader from a different computer and still
+> have everything tagged finely.
+>
+> Best regards,
+>
+>
+> ---
+> [1] http://software.complete.org/software/projects/show/offlineimap
+>
+> -- 
+> Adrian Perez de Castro <aperez@igalia.com>
+> Igalia - Free Software Engineering
+> _______________________________________________
+> notmuch mailing list
+> notmuch@notmuchmail.org
+> http://notmuchmail.org/mailman/listinfo/notmuch
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Reply within emacs to a multipart/alternative message"
+test_emacs '(let ((message-hidden-headers ''()))
+	    (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
+		(notmuch-show-reply)
+		(test-output))'
+sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: Alex Botero-Lowry <alex.boterolowry@gmail.com>, notmuch@notmuchmail.org
+Subject: Re: [notmuch] preliminary FreeBSD support
+In-Reply-To: <cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com>
+Fcc: ${MAIL_DIR}/sent
+References: <cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com>
+User-Agent: Notmuch/XXX Emacs/XXX
+--text follows this line--
+Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
+
+> I saw the announcement this morning, and was very excited, as I had been
+> hoping sup would be turned into a library,
+> since I like the concept more than the UI (I'd rather an emacs interface).
+>
+> I did a preliminary compile which worked out fine, but
+> sysconf(_SC_SC_GETPW_R_SIZE_MAX) returns -1 on
+> FreeBSD, so notmuch_config_open segfaulted.
+>
+> Attached is a patch that supplies a default buffer size of 64 in cases where
+> -1 is returned.
+>
+> http://www.opengroup.org/austin/docs/austin_328.txt - seems to indicate this
+> is acceptable behavior,
+> and http://mail-index.netbsd.org/pkgsrc-bugs/2006/06/07/msg016808.htmlspecifically
+> uses 64 as the
+> buffer size.
+> _______________________________________________
+> notmuch mailing list
+> notmuch@notmuchmail.org
+> http://notmuchmail.org/mailman/listinfo/notmuch
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Reply within emacs to an html-only message"
+add_message '[content-type]="text/html"' \
+	    '[body]="Hi,<br />This is an <b>HTML</b> test message.<br /><br />OK?"'
+test_emacs "(let ((message-hidden-headers '()) (mm-text-html-renderer 'html2text))
+	    (notmuch-show \"id:${gen_msg_id}\")
+	    (notmuch-show-reply)
+	    (test-output))"
+sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: 
+Subject: Re: Reply within emacs to an html-only message
+In-Reply-To: <${gen_msg_id}>
+Fcc: ${MAIL_DIR}/sent
+References: <${gen_msg_id}>
+User-Agent: Notmuch/XXX Emacs/XXX
+--text follows this line--
+Notmuch Test Suite <test_suite@notmuchmail.org> writes:
+
+> Hi,This is an HTML test message.OK?
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Quote MML tags in reply"
+message_id='test-emacs-mml-quoting@message.id'
+add_message [id]="$message_id" \
+	    "[subject]='$test_subtest_name'" \
+	    '[body]="<#part disposition=inline>"'
+test_emacs "(let ((message-hidden-headers '()))
+	      (notmuch-show \"id:$message_id\")
+	      (notmuch-show-reply)
+	      (test-output))"
+sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: 
+Subject: Re: Quote MML tags in reply
+In-Reply-To: <test-emacs-mml-quoting@message.id>
+Fcc: ${MAIL_DIR}/sent
+References: <test-emacs-mml-quoting@message.id>
+User-Agent: Notmuch/XXX Emacs/XXX
+--text follows this line--
+Notmuch Test Suite <test_suite@notmuchmail.org> writes:
+
+> <#!part disposition=inline>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_done
-- 
1.7.10.4

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

* Re: [PATCH 2/2] test: move all emacs show tests to emacs-show test script
  2012-08-12 17:39           ` [PATCH 2/2] test: move all emacs show tests to emacs-show test script Jameson Graef Rollins
@ 2012-08-31 19:44             ` David Bremner
  0 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2012-08-31 19:44 UTC (permalink / raw)
  To: Jameson Graef Rollins, Notmuch Mail

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> No functional change.
>
> Most notmuch-show mode tests were in the emacs script, while some were
> in the emacs-show script.  This moves all the notmuch-show mode tests
> to the emacs-show script, to make things a little more consistent.

This seems harmless enough, and independent of the the other patch in
the series, but I broke it with one fixes for emacs24. Interested in
rebasing it?

d

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-08-10  7:10       ` [PATCH v2] " Mark Walters
  2012-08-10 14:53         ` Jani Nikula
  2012-08-12 17:39         ` [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative Jameson Graef Rollins
@ 2012-10-09 22:21         ` Jeremy Nickurak
  2012-10-19 16:36         ` Ethan Glasser-Camp
  3 siblings, 0 replies; 23+ messages in thread
From: Jeremy Nickurak @ 2012-10-09 22:21 UTC (permalink / raw)
  To: notmuch

On Fri, Aug 10, 2012 at 1:10 AM, Mark Walters <markwalters1009@gmail.com> wrote:
>
> 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'.

I've started using this, and quite like it. Only thing missing I can
see would be a customizable way to say which types should be
preferred.

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

* [PATCH] Add NEWS item for multipart/alternative toggle
  2012-06-24 18:56   ` Mark Walters
@ 2012-10-19 16:14     ` Ethan Glasser-Camp
  0 siblings, 0 replies; 23+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-19 16:14 UTC (permalink / raw)
  To: notmuch; +Cc: Ethan Glasser-Camp

Users who relied on notmuch-show-all-multipart/alternative-parts
might need to know that it is now buffer-local.

Signed-off-by: Ethan Glasser-Camp <ethan@betacantrips.com>
---
Hi! I'm trying to figure out the status of this patch series, which
seems to have fallen through the cracks. It looks like Jani's solution
exists and works, though it isn't perfect. I would like to get Jani's
solution applied until someone who is sufficiently motivated decides to
work on fancy cycle-multipart-message things. It seems like the last
hurdle is that Mark would like Jani to put the buffer-localization of
notmuch-show-all-multipart/alternative-parts in NEWS. Here's that.

However, it also seems like there was a disagreement about design
direction -- should multiple alternative parts be toggled so you can
see them all at once, or cycled so you can see one at a time? See
id:"87d32yadl5.fsf@qmul.ac.uk". I'm for the code that works now which
is Jani's patch.

 NEWS |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/NEWS b/NEWS
index 2b50ba3..fc9b50a 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,14 @@
+Emacs Interface
+---------------
+
+Display of multipart messages
+
+  Displaying multipart/alternative parts has been made a toggle, the
+  same way as indentation and message decryption.
+  notmuch-show-all-multipart/alternative-parts is now buffer-local, so
+  if you have set it using setq, you will have to update that to use
+  setq-default.
+
 Notmuch 0.14 (2012-08-20)
 =========================
 
-- 
1.7.9.5

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

* Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts
  2012-08-10  7:10       ` [PATCH v2] " Mark Walters
                           ` (2 preceding siblings ...)
  2012-10-09 22:21         ` [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts Jeremy Nickurak
@ 2012-10-19 16:36         ` Ethan Glasser-Camp
  3 siblings, 0 replies; 23+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-19 16:36 UTC (permalink / raw)
  To: Mark Walters, Jameson Graef Rollins, Jani Nikula, notmuch

Mark Walters <markwalters1009@gmail.com> writes:

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

For what it's worth, I don't feel that this code is horrible. It seems
like there remain design decisions to be made about how notmuch show
"ought" to handle multipart/alternatives, but I can at least comment on
this code.

First, the use of a plist looks fine to me because most of the time it's
going to have length 0. At most it will have one entry per message -- a
few hundred. So I'm not worried about efficiency concerns.

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

Changing let to let* makes me the slightest bit uneasy, although I'm not
sure I could explain why.

It would be nice if you could wrap the manipulation of
notmuch-show-message-multipart/alternative-display-part in functions,
ideally with names that are shorter than the variable they
manipulate. Specifically, I think the definition of chosen-nth (which is
almost repeated below) could be its own function, something like
(notmuch-show-message-current-multipart msg), which could take a msg-id
or a plist and do the plist-get and id-to-query that you do here.

> +    ;; 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."

This docstring is broken.

> +  (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)))

Maybe move the reset-and-go-back-to-zero behavior to this function
instead of in the display function. This opens you up to weird
situations if one of the multipart/alternatives should disappear from a
message or if some other function should change the alternative on
display for a given message, but both of these seem unlikely to me..

Ethan

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

* Re: [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative
  2012-08-12 17:39         ` [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative Jameson Graef Rollins
  2012-08-12 17:39           ` [PATCH 2/2] test: move all emacs show tests to emacs-show test script Jameson Graef Rollins
@ 2012-10-20  2:31           ` Ethan Glasser-Camp
  1 sibling, 0 replies; 23+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-20  2:31 UTC (permalink / raw)
  To: Jameson Graef Rollins, Notmuch Mail

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> +diff OUTPUT.{text,html} >OUTPUT.diff
> +cat <<EOF >EXPECTED.diff
> +7,9c7,10
> +< [ text/html (not shown) ]
> +< [ text/plain ]
> +< This is the text/plain part of a multipart/alternative.
> +---
> +> [ text/html ]
> +> This is the text/html part of a multipart/alternative.
> +> 
> +> [ text/plain (not shown) ]
> +EOF

Hmm, old-school diff. I guess this (instead of unified diff) is so that
if the diff isn't like this, then the diff of the diffs are clearly
readable? :)

git am complains at me because this patch introduces trailing whitespace
(to mimic, of course, the output of diff which will have trailing
whitespace as well). I would love for you to write this file using some
other technique which would not require putting trailing whitespace in
the test file.

Additionally the patch is stale, perhaps because my patch that moved some tests
to emacs-show was just pushed too.

Ethan

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

end of thread, other threads:[~2012-10-20  2:31 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-15 15:55 [PATCH] emacs: add function to toggle display of all multipart/alternative parts Jani Nikula
2012-06-18 21:26 ` Jameson Graef Rollins
2012-06-19  6:56   ` Mark Walters
2012-08-09 20:52     ` Jameson Graef Rollins
2012-08-10  7:10       ` [PATCH v2] " Mark Walters
2012-08-10 14:53         ` Jani Nikula
2012-08-10 16:18           ` Jameson Graef Rollins
2012-08-10 16:39             ` Jani Nikula
2012-08-10 16:58               ` Mark Walters
2012-08-10 17:49                 ` Jameson Graef Rollins
2012-08-12 17:39         ` [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative Jameson Graef Rollins
2012-08-12 17:39           ` [PATCH 2/2] test: move all emacs show tests to emacs-show test script Jameson Graef Rollins
2012-08-31 19:44             ` David Bremner
2012-10-20  2:31           ` [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative Ethan Glasser-Camp
2012-10-09 22:21         ` [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts Jeremy Nickurak
2012-10-19 16:36         ` Ethan Glasser-Camp
2012-06-19  6:57   ` [PATCH] " Jani Nikula
2012-06-19 20:14 ` [PATCH v2] " Jani Nikula
2012-06-19 20:25   ` Jani Nikula
2012-06-22 22:34   ` David Bremner
2012-06-24  7:10     ` Jani Nikula
2012-06-24 18:56   ` Mark Walters
2012-10-19 16:14     ` [PATCH] Add NEWS item for multipart/alternative toggle Ethan Glasser-Camp

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