Date: Tue, 17 Jan 2012 15:26:03 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>
Subject: Re: [PATCH 1/1] Make buttons for attachments allow viewing as well
	as saving
Message-ID: <20120117202603.GP16740@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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8739beitq4.fsf@qmul.ac.uk>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

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.