From: Jani Nikula <jani@nikula.org>
To: Jameson Graef Rollins <jrollins@finestructure.net>,
	David Edmondson <dme@dme.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 1/2] emacs: create patch filename from subject for
	inline patch fake parts
In-Reply-To: <8739c7jcfo.fsf@servo.finestructure.net>
References: <1321657368-13872-1-git-send-email-jani@nikula.org>
	<cover.1324849534.git.jani@nikula.org>
	<aa55383e8609a2e0d376744ff3e982fd072c58d6.1324849534.git.jani@nikula.org>
	<cun62h3tu51.fsf@hotblack-desiato.hh.sledj.net>
	<87wr9ja5bp.fsf@nikula.org>
	<8739c7jcfo.fsf@servo.finestructure.net>
User-Agent: Notmuch/0.10.2+131~gb184771 (http://notmuchmail.org) Emacs/23.3.1
	(i686-pc-linux-gnu)
Date: Mon, 26 Dec 2011 23:52:23 +0200
Message-ID: <87mxafq9uw.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Mon, 26 Dec 2011 12:38:35 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Mon, 26 Dec 2011 14:24:42 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > Clicking on the button for the part saves the wrong thing, though,
> > > because it's not a real MIME part. That looks a bit awkward to fix, so
> > > perhaps you could still prefix the name with "inline: " to indicate that
> > > it's odd?
> > 
> > I'm not sure I follow you here. Could you elaborate what you mean by
> > "the wrong thing", please?
> 
> Yes, this is a problem/bug with the current behavior of these fake
> inline parts, actually.  The button should behave as those it is
> representing an actual MIME part of the message.  However, clicking the
> button saves the entire part that the fake part is derived from, rather
> than just the data in the fake part.  This is countereintuitive and
> confusing.

Okay, thanks for the explanation. This might be beyond my elisp
skills. And IMHO out of the scope of the patch anyway.

> > I don't think the user would want to have an "inline: " prefix in the
> > filename that would have to be removed every time. I think that kind of
> > defeats the purpose here. Or where exactly are you proposing to have the
> > prefix?
> 
> It's confusing to have this fake part button look just like a real part
> button, without any indication that it's really a fake part, and then to
> behave differently as well.  The button needs some indication that it
> does not represent a real mime part of the message.  Giving the fake
> part a proper filename is fine, but it needs some other indicator that
> the part is fake and that the button is not going to behave as expected.

Below is an idea I came up with, utilizing the content-type
vs. declared-type distinction. I think it's really simple and neat, but
I hope not too magical.

Picking up an example mail from the list, inline patches would show up
like this:

[ 0001-emacs-unify-search-mechanisms.patch: inline patch (as text/x-diff) ]


BR,
Jani.


>From b1847714f7368247fbc5c93767f59d8269eadc1c Mon Sep 17 00:00:00 2001
From: Jani Nikula <jani@nikula.org>
Date: Mon, 26 Dec 2011 23:31:41 +0200
Subject: [PATCH] emacs: add inline patch fake parts through a special handler

Add wash generated inline patch fake parts through a special
"inline-patch-fake-part" handler to distinguish them from real MIME
parts. The fake parts are described as "inline patch (as text/x-diff)".

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 emacs/notmuch-show.el |    4 ++++
 emacs/notmuch-wash.el |    2 +-
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index eee4da9..6ef3f90 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -585,6 +585,10 @@ current buffer, if possible."
 		nil))
 	  nil))))
 
+;; Handler for wash generated inline patch fake parts.
+(defun notmuch-show-insert-part-inline-patch-fake-part (msg part content-type nth depth declared-type)
+  (notmuch-show-insert-part-*/* msg part "text/x-diff" nth depth "inline patch"))
+
 (defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type)
   ;; This handler _must_ succeed - it is the handler of last resort.
   (notmuch-show-insert-part-header nth content-type declared-type (plist-get part :filename))
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 7d037f5..de51c88 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -351,7 +351,7 @@ for error."
 	      (setq patch-end (match-beginning 0)))
 	  (save-restriction
 	    (narrow-to-region patch-start patch-end)
-	    (setq part (plist-put part :content-type "text/x-diff"))
+	    (setq part (plist-put part :content-type "inline-patch-fake-part"))
 	    (setq part (plist-put part :content (buffer-string)))
 	    (setq part (plist-put part :id -1))
 	    (setq part (plist-put part :filename
-- 
1.7.5.4