Date: Mon, 16 Jan 2012 21:23:30 -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: <20120117022330.GE16740@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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87lip7fhkc.fsf@qmul.ac.uk>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Mark Walters on Jan 16 at  9:44 pm:
> On Mon, 16 Jan 2012 11:31:16 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > > Keymap on a button is: s for save, v for view and o for view with
> > > other program. Default (i.e. enter or mouse button) is save but is
> > > easily configurable e.g. set to view with
> > > (setq notmuch-show-part-button-default-action 'notmuch-show-part-view-action)
> > 
> > Actually, this should really be a defcustom.  Maybe something like this:
> > 
> > (defcustom notmuch-show-part-button-default-action 'notmuch-show-part-button-save
> >   "Default part header button action (on ENTER or mouse click)."
> >   :group 'notmuch
> >   :type '(choice (function :tag "Save part"
> >                           :value notmuch-show-part-button-save)
> >                 (function :tag "View part"
> >                           :value notmuch-show-part-button-view)
> >                 (function :tag "View interactively"
> >                           :value notmuch-show-part-button-interactively-view))
> > 
> > Unfortunately this isn't quite working right, since it's not setting the
> > default properly, but if someone can help me figure out what I'm doing
> > wrong, I think this is at least the right idea.
> 
> This did not work for me, nor did Austin's suggestion but the following
> does
> +(defcustom notmuch-show-part-button-default-action 'notmuch-show-part-save-action
> +  "Default part header button action (on ENTER or mouse click)."
> +  :group 'notmuch
> +  :type '(choice (const :tag "Save part"
> +                       notmuch-show-part-save-action)
> +                (const :tag "View part"
> +                       notmuch-show-part-view-action)
> +                (const :tag "View interactively"
> +                       notmuch-show-part-interactively-view-action)))
> 
> 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.