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

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

Date: Mon, 16 Jan 2012 11:31:16 -0800

To: Mark Walters, notmuch@notmuchmail.org

Cc:

From: Jameson Graef Rollins


On Sun, 15 Jan 2012 12:16:36 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> Define a keymap for attachment buttons to allow multiple actions.
> Define 3 possible actions:
>     save attachment: exactly as currently,
>     view attachment: uses mailcap entry,
>     view attachment with user chosen program

Great improvement, Mark!  Thanks for this.  I've been wanting this kind
of functionality for a while, actually, and this is a really great
implementation.  It works like a charm, and the code looks good to me,
modulo a couple small comments below.

> 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.

> One implementation detail: the view attachment function forces all
> attachments to be "displayed" using mailcap even if emacs could
> display them itself. Thus, for example, text/html appears in a browser
> and text/plain asks whether to save (on a standard debian setup)

I think this is good.

> +(defvar notmuch-show-part-button-default-action 'notmuch-show-part-save-action) 

There's a white space at the end of this line, which produces the
following git warning:

  Applying: Make buttons for attachments allow viewing as well as saving
  /home/jrollins/src/notmuch/git/.git/rebase-apply/patch:96: trailing whitespace.
  (defvar notmuch-show-part-button-default-action 'notmuch-show-part-save-action)
  warning: 1 line adds whitespace errors.

So if you go with (an improved version of) my defcustom suggestion above
you can kill two birds with one stone:

-(defvar notmuch-show-part-button-default-action 'notmuch-show-part-save-action) 
+(defcustom notmuch-show-part-button-default-action '(notmuch-show-part-but=
ton-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))

jamie.
part-000.sig (application/pgp-signature)

Thread: