[PATCH 1/1] Make buttons for attachments allow viewing as well as saving

[Aaron Ecay]

On Tue, 17 Jan 2012 15:26:03 -0500, Austin Clements <amdragon at MIT.EDU> wrote:
> Quoth Mark Walters on Jan 17 at  9:06 am:
> > 
> > > > I wonder if the "problem" comes from me doing things in a non-lispy
> > > > fashion (I am completely new to lisp). Thus
> > > > notmuch-show-part-button-default-action is a variable that gets passed
> > > > around rather than a function.
> > > 
> > > Sorry, I should have looked at the bigger context in this patch.  I
> > > think Jameson was implying that notmuch-show-part-button-default
> > > should change to
> > > 
> > > (defun notmuch-show-part-button-default (&optional button)
> > >   (interactive)
> > >   (funcall notmuch-show-part-button-default-action button))
> > > 
> > > I would go one step further and say that each action should probably
> > > be a separate function.  That is, break notmuch-show-part-action into
> > > separate functions and simply invoke the appropriate function, rather
> > > than performing a fixed data dispatch.  This would be more flexible
> > > and Lispy.  It may be that your approach works out better, but I'd at
> > > least give this a shot.
> > 
> > I am happy to make that change. My original patch in the summer was more
> > like that:
> > id:"CALUdzSWAto+4mCUOOMk+8vFs+Pog-xUma6u-Aqx2M6-sbyQROg at mail.gmail.com"
> 
> Is this the right id?  I couldn't find it in the list archive.
> 
> > Is that more what you had in mind? (Only in broad terms: Obviously I
> > would need to add in the customization and default function etc). I
> > decided that I didn't like the code duplication (but I am completely new
> > to lisp) which is why I changed it for this submission.
> 
> Yes, I wondered about this, too.  It seems like at worst the
> notmuch-show-process-crypto stuff would be duplicated.  This might be
> little enough that it's not worth worrying about, or it might be worth
> introducing something like
> 
> (defun notmuch-with-temp-part-buffer (message-id nth action)
>   (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))
>       (funcall action))))
> 
> You could also do this as a macro, but that definitely seems like
> overkill.

It seems to me that a macro is in fact the best solution.  If you do it
with a function, you need two defuns per action: one to do the actual
work:
(defun notmuch-show-part-button-whatever-worker ()
  ;; do stuff...
)
and one that says:
(defun notmuch-show-part-button-whatever ()
  (notmuch-with-temp-part-buffer
   id part-number #'notmuch-show-part-button-whatever-worker))

It would be the latter function that the key would be bound to.  If a
macro is used, the split between the worker and glue fns can be
abandoned, and only one function is needed:
(defun notmuch-show-part-button-whatever ()
  (notmuch-with-temp-part-buffer
  ;; do stuff...
  ))

A further advantage is if interactive arguments (e.g. C-u prefix) are
needed for the function, there is no need to thread them through as
arguments of notmuch-with-temp-part-buffer.

-- 
Aaron Ecay