On Tue, 17 Jan 2012 15:26:03 -0500, Austin Clements <amdragon@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@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