From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>
Subject: Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as
	saving
In-Reply-To: <20120117230255.GW16740@mit.edu>
References: <1326629796-11436-1-git-send-email-markwalters1009@gmail.com>
	<1326629796-11436-2-git-send-email-markwalters1009@gmail.com>
	<87wr8r5trv.fsf@servo.finestructure.net>
	<87lip7fhkc.fsf@qmul.ac.uk> <20120117022330.GE16740@mit.edu>
	<8739beitq4.fsf@qmul.ac.uk> <20120117202603.GP16740@mit.edu>
	<871uqy3vy4.fsf@qmul.ac.uk> <20120117210158.GS16740@mit.edu>
	<87obu2q80o.fsf@qmul.ac.uk> <20120117230255.GW16740@mit.edu>
User-Agent: Notmuch/0.11~rc2+73~g1ea2b60 (http://notmuchmail.org) Emacs/23.2.1
	(i486-pc-linux-gnu)
Date: Tue, 17 Jan 2012 23:42:52 +0000
Message-ID: <87lip5rj43.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: notmuch@notmuchmail.org
Precedence: list

> 
> Oops, actually there was a bug in that macro.  It should have been
> 
> (defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
>   (declare (indent 2))
>   (let ((process-crypto (make-symbol "process-crypto")))
>     `(let ((,process-crypto notmuch-show-process-crypto))
>        (with-temp-buffer
>          (setq notmuch-show-process-crypto ,process-crypto)
>          ;; Always acquires the part via `notmuch part', even if it is
>          ;; available in the JSON output.
>          (insert (notmuch-show-get-bodypart-internal ,message-id ,nth))
>          ,@body))))
> 
> The only difference is on the "insert" line.  Sorry about that.

Fixed.

[Snip excellent explanation of defmacro]

Thanks for the excellent explanation!

> > Finally, I have discovered one bug/misfeature. If you try to "view" an
> > attachment then it will offer to save it but will not offer a
> > filename. If you try and save it (or use the default action) it will
> > offer a filename as now. As far as I can see this is not fixable if I
> > use mm-display-part: however, I could include a slight tweaked version,
> > notmuch-show-mm-display-part say, which would fix this corner
> > case. (Essentially, it would call notmuch-show-save-part if it failed to
> > find a handler rather than mailcap-save-binary-file.) However, this is
> > about 50 lines of lisp so I am not sure it is worth it.
> 
> Hmm.  This is probably worth fixing, but probably in a separate patch.
> Duplicating mm-display-part is probably not the way to go.  It think
> it will work to pass t as the no-default argument to mm-display-part
> and check the return value, which should be 'inline if it was able to
> handle it internally or 'external if it found an external helper.  I'm
> pretty sure it will never fall in to mailcap-save-binary-file in that
> case.  If that doesn't work, you could flet mailcap-save-binary-file
> around the call to mm-display-part.

I had tried passing t to mm-display-part and that didn't work as I
expected. I will experiment some more and try your flet suggestion but I
think that will be a separate patch.

I will send a potential final version of this patch as a reply to this
email. Many thanks for all the guidance and help!

Best wishes

Mark