* [PATCH 0/1] emacs: allow viewing as well saving individual attachments
@ 2012-01-15 12:16 Mark Walters
2012-01-15 12:16 ` [PATCH 1/1] Make buttons for attachments allow viewing as well as saving Mark Walters
2012-01-16 9:22 ` [PATCH 0/1] emacs: allow viewing as well saving individual attachments David Edmondson
0 siblings, 2 replies; 22+ messages in thread
From: Mark Walters @ 2012-01-15 12:16 UTC (permalink / raw)
To: notmuch
Hello
This is a resubmission of (a tidied version) of
id:"87mxehqhbl.fsf@r102.config".
I have modified the emacs interface for handling attachments by adding
a keymap to the attachment button. For example pressing v when on an
attachment button views the attachment (using the mailcap method) and
pressing s saves the attachment. I find this makes it a lot easier
when dealing with message with lots of attachments.
Other comments:
"Viewing" a text/html button opens the part in the mailcap
defined html viewer.
"Viewing" a part with no mailcap entry just offers to save it.
In this version I make the button default to saving (as currently) but
this is easily customizable.
I have also mapped the key "o" (other/open with) on a button
to open with user chosen program. This could be split out into a
separate patch if preferred.
I have been using the previous version since I wrote it in the summer
and not had any problems. This tidied version is less well tested but
the logic should be the same; it seems to work correctly in all cases
I can think to test.
Best wishes
Mark
Mark Walters (1):
Make buttons for attachments allow viewing as well as saving
emacs/notmuch-show.el | 81 ++++++++++++++++++++++++++++++++++++++----------
1 files changed, 64 insertions(+), 17 deletions(-)
--
1.7.2.3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-15 12:16 [PATCH 0/1] emacs: allow viewing as well saving individual attachments Mark Walters
@ 2012-01-15 12:16 ` Mark Walters
2012-01-16 19:31 ` Jameson Graef Rollins
2012-01-16 9:22 ` [PATCH 0/1] emacs: allow viewing as well saving individual attachments David Edmondson
1 sibling, 1 reply; 22+ messages in thread
From: Mark Walters @ 2012-01-15 12:16 UTC (permalink / raw)
To: notmuch
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
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)
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)
---
emacs/notmuch-show.el | 81 ++++++++++++++++++++++++++++++++++++++----------
1 files changed, 64 insertions(+), 17 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 03c1f6b..a1c0e63 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -281,10 +281,21 @@ message at DEPTH in the current thread."
(run-hooks 'notmuch-show-markup-headers-hook)))))
(define-button-type 'notmuch-show-part-button-type
- 'action 'notmuch-show-part-button-action
+ 'action 'notmuch-show-part-button-default
+ 'keymap 'notmuch-show-part-button-map
'follow-link t
'face 'message-mml)
+(defvar notmuch-show-part-button-map
+ (let ((map (make-sparse-keymap)))
+ (set-keymap-parent map button-map)
+ (define-key map "s" 'notmuch-show-part-button-save)
+ (define-key map "v" 'notmuch-show-part-button-view)
+ (define-key map "o" 'notmuch-show-part-button-interactively-view)
+ map)
+ "Submap for button commands")
+(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
+
(defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
(let ((button))
(setq button
@@ -299,29 +310,43 @@ message at DEPTH in the current thread."
" ]")
:type 'notmuch-show-part-button-type
:notmuch-part nth
- :notmuch-filename name))
+ :notmuch-filename name
+ :notmuch-content-type content-type))
(insert "\n")
;; return button
button))
;; Functions handling particular MIME parts.
+;; this function is kept for the tests and any external users
(defun notmuch-show-save-part (message-id nth &optional filename)
+ (notmuch-show-part-action 'notmuch-show-part-save-action message-id nth nil filename))
+
+(defun notmuch-show-part-action (action message-id nth content-type &optional filename)
(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))
- (let ((file (read-file-name
- "Filename to save as: "
- (or mailcap-download-directory "~/")
- nil nil
- filename)))
- ;; Don't re-compress .gz & al. Arguably we should make
- ;; `file-name-handler-alist' nil, but that would chop
- ;; ange-ftp, which is reasonable to use here.
- (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
+ (cond ((eq action 'notmuch-show-part-save-action)
+ (let ((file (read-file-name
+ "Filename to save as: "
+ (or mailcap-download-directory "~/")
+ nil nil
+ filename)))
+ ;; Don't re-compress .gz & al. Arguably we should make
+ ;; `file-name-handler-alist' nil, but that would chop
+ ;; ange-ftp, which is reasonable to use here.
+ (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))
+ ((eq action 'notmuch-show-part-view-action)
+ ;; set mm-inlined-types to nil to force an external viewer
+ (let ((handle (mm-make-handle (current-buffer) (list content-type)))
+ (mm-inlined-types nil))
+ (mm-display-part handle)))
+ ((eq action 'notmuch-show-part-interactively-view-action)
+ (let ((handle (mm-make-handle (current-buffer) (list content-type))))
+ (mm-interactively-view-part handle)))))))
(defun notmuch-show-mm-display-part-inline (msg part nth content-type)
"Use the mm-decode/mm-view functions to display a part in the
@@ -1502,12 +1527,34 @@ buffer."
;; Commands typically bound to buttons.
-(defun notmuch-show-part-button-action (button)
- (let ((nth (button-get button :notmuch-part)))
- (if nth
- (notmuch-show-save-part (notmuch-show-get-message-id) nth
- (button-get button :notmuch-filename))
- (message "Not a valid part (is it a fake part?)."))))
+(defvar notmuch-show-part-button-default-action 'notmuch-show-part-save-action)
+
+(defun notmuch-show-part-button-default (&optional button)
+ (interactive)
+ (notmuch-show-part-button notmuch-show-part-button-default-action button))
+
+(defun notmuch-show-part-button-save (&optional button)
+ (interactive)
+ (notmuch-show-part-button 'notmuch-show-part-save-action button))
+
+(defun notmuch-show-part-button-view (&optional button)
+ (interactive)
+ (notmuch-show-part-button 'notmuch-show-part-view-action button))
+
+(defun notmuch-show-part-button-interactively-view (&optional button)
+ (interactive)
+ (notmuch-show-part-button 'notmuch-show-part-interactively-view-action button))
+
+(defun notmuch-show-part-button (action &optional button)
+ (interactive)
+ (let ((button (or button (button-at (point)))))
+ (if button
+ (let ((nth (button-get button :notmuch-part)))
+ (if nth
+ (notmuch-show-part-action action (notmuch-show-get-message-id) nth
+ (button-get button :notmuch-content-type)
+ (button-get button :notmuch-filename))
+ (message "Not a valid part (is it a fake part?)."))))))
;;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] emacs: allow viewing as well saving individual attachments
2012-01-15 12:16 [PATCH 0/1] emacs: allow viewing as well saving individual attachments Mark Walters
2012-01-15 12:16 ` [PATCH 1/1] Make buttons for attachments allow viewing as well as saving Mark Walters
@ 2012-01-16 9:22 ` David Edmondson
1 sibling, 0 replies; 22+ messages in thread
From: David Edmondson @ 2012-01-16 9:22 UTC (permalink / raw)
To: Mark Walters, notmuch
[-- Attachment #1: Type: text/plain, Size: 330 bytes --]
On Sun, 15 Jan 2012 12:16:35 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> Mark Walters (1):
> Make buttons for attachments allow viewing as well as saving
>
> emacs/notmuch-show.el | 81 ++++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 64 insertions(+), 17 deletions(-)
Nice changes.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-15 12:16 ` [PATCH 1/1] Make buttons for attachments allow viewing as well as saving Mark Walters
@ 2012-01-16 19:31 ` Jameson Graef Rollins
2012-01-16 20:11 ` Austin Clements
2012-01-16 21:44 ` Mark Walters
0 siblings, 2 replies; 22+ messages in thread
From: Jameson Graef Rollins @ 2012-01-16 19:31 UTC (permalink / raw)
To: Mark Walters, notmuch
[-- Attachment #1: Type: text/plain, Size: 3154 bytes --]
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.
[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-16 19:31 ` Jameson Graef Rollins
@ 2012-01-16 20:11 ` Austin Clements
2012-01-16 21:44 ` Mark Walters
1 sibling, 0 replies; 22+ messages in thread
From: Austin Clements @ 2012-01-16 20:11 UTC (permalink / raw)
To: Jameson Graef Rollins; +Cc: notmuch
Quoth Jameson Graef Rollins on Jan 16 at 11:31 am:
> 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.
Jamie's defcustom doesn't work for me either (apparently it works even
less for me than it does for Jamie), but the following works for me
(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 (const :tag "Save part"
notmuch-show-part-button-save)
(const :tag "View part"
notmuch-show-part-button-view)
(const :tag "View interactively"
notmuch-show-part-button-interactively-view)))
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-16 19:31 ` Jameson Graef Rollins
2012-01-16 20:11 ` Austin Clements
@ 2012-01-16 21:44 ` Mark Walters
2012-01-17 2:23 ` Austin Clements
1 sibling, 1 reply; 22+ messages in thread
From: Mark Walters @ 2012-01-16 21:44 UTC (permalink / raw)
To: notmuch
(My apologies, owing to various errors on my part the patch has ended up
in a separate thread:
id:"1326749910-30437-1-git-send-email-markwalters1009@gmail.com")
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.
> 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:
Right I have fixed this as above and think the whitespace is ok now (it
passes git diff --check). I intended to send the patch as a reply to
this email but it has ended up here:
id:"1326749910-30437-1-git-send-email-markwalters1009@gmail.com"
Best wishes
Mark
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-16 21:44 ` Mark Walters
@ 2012-01-17 2:23 ` Austin Clements
2012-01-17 9:06 ` Mark Walters
0 siblings, 1 reply; 22+ messages in thread
From: Austin Clements @ 2012-01-17 2:23 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-17 2:23 ` Austin Clements
@ 2012-01-17 9:06 ` Mark Walters
2012-01-17 20:26 ` Austin Clements
0 siblings, 1 reply; 22+ messages in thread
From: Mark Walters @ 2012-01-17 9:06 UTC (permalink / raw)
To: Austin Clements; +Cc: notmuch
> > 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 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.
Best wishes
Mark
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-17 9:06 ` Mark Walters
@ 2012-01-17 20:26 ` Austin Clements
2012-01-17 20:39 ` Mark Walters
2012-01-17 20:44 ` [PATCH 1/1] " Aaron Ecay
0 siblings, 2 replies; 22+ messages in thread
From: Austin Clements @ 2012-01-17 20:26 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-17 20:26 ` Austin Clements
@ 2012-01-17 20:39 ` Mark Walters
2012-01-17 21:01 ` Austin Clements
2012-01-17 20:44 ` [PATCH 1/1] " Aaron Ecay
1 sibling, 1 reply; 22+ messages in thread
From: Mark Walters @ 2012-01-17 20:39 UTC (permalink / raw)
To: Austin Clements; +Cc: notmuch
> > 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.
Sorry I messed up: it should be id:"87mxehqhbl.fsf@r102.config" However
I have included my current draft along these lines. I think it is
working but I am not submitting yet: just asking if this is the right
idea.
Best wishes
Mark
From 9e52414b9871369c1cbb5c3e72d833b56bb236d4 Mon Sep 17 00:00:00 2001
From: Mark Walters <markwalters1009@gmail.com>
Date: Sat, 14 Jan 2012 18:04:22 +0000
Subject: [PATCH] Make buttons for attachments allow viewing as well as saving
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
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 this
is configurable in notmuch customize.
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)
---
emacs/notmuch-show.el | 87 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 03c1f6b..2413caa 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -281,10 +281,21 @@ message at DEPTH in the current thread."
(run-hooks 'notmuch-show-markup-headers-hook)))))
(define-button-type 'notmuch-show-part-button-type
- 'action 'notmuch-show-part-button-action
+ 'action 'notmuch-show-part-button-default
+ 'keymap 'notmuch-show-part-button-map
'follow-link t
'face 'message-mml)
+(defvar notmuch-show-part-button-map
+ (let ((map (make-sparse-keymap)))
+ (set-keymap-parent map button-map)
+ (define-key map "s" 'notmuch-show-part-button-save)
+ (define-key map "v" 'notmuch-show-part-button-view)
+ (define-key map "o" 'notmuch-show-part-button-interactively-view)
+ map)
+ "Submap for button commands")
+(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
+
(defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
(let ((button))
(setq button
@@ -299,7 +310,8 @@ message at DEPTH in the current thread."
" ]")
:type 'notmuch-show-part-button-type
:notmuch-part nth
- :notmuch-filename name))
+ :notmuch-filename name
+ :notmuch-content-type content-type))
(insert "\n")
;; return button
button))
@@ -323,6 +335,28 @@ message at DEPTH in the current thread."
;; ange-ftp, which is reasonable to use here.
(mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
+(defun notmuch-show-view-part (message-id nth content-type)
+ (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))
+ ;; set mm-inlined-types to nil to force an external viewer
+ (let ((handle (mm-make-handle (current-buffer) (list content-type)))
+ (mm-inlined-types nil))
+ (mm-display-part handle t)))))
+
+(defun notmuch-show-interactively-view-part (message-id nth content-type)
+ (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))
+ (let ((handle (mm-make-handle (current-buffer) (list content-type))))
+ (mm-interactively-view-part handle)))))
+
(defun notmuch-show-mm-display-part-inline (msg part nth content-type)
"Use the mm-decode/mm-view functions to display a part in the
current buffer, if possible."
@@ -1502,12 +1536,49 @@ buffer."
;; Commands typically bound to buttons.
-(defun notmuch-show-part-button-action (button)
- (let ((nth (button-get button :notmuch-part)))
- (if nth
- (notmuch-show-save-part (notmuch-show-get-message-id) nth
- (button-get button :notmuch-filename))
- (message "Not a valid part (is it a fake part?)."))))
+(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 (const :tag "Save part"
+ notmuch-show-part-button-save)
+ (const :tag "View part"
+ notmuch-show-part-button-view)
+ (const :tag "View interactively"
+ notmuch-show-part-button-interactively-view)))
+
+(defun notmuch-show-part-button-default (&optional button)
+ (interactive)
+ (funcall notmuch-show-part-button-default-action button))
+
+(defun notmuch-show-part-button-save (&optional button)
+ (interactive)
+ (let ((button (or button (button-at (point)))))
+ (if button
+ (let ((nth (button-get button :notmuch-part)))
+ (if nth
+ (notmuch-show-save-part (notmuch-show-get-message-id) nth
+ (button-get button :notmuch-filename))
+ (message "Not a valid part (is it a fake part?)."))))))
+
+(defun notmuch-show-part-button-view (&optional button)
+ (interactive)
+ (let ((button (or button (button-at (point)))))
+ (if button
+ (let ((nth (button-get button :notmuch-part)))
+ (if nth
+ (notmuch-show-view-part (notmuch-show-get-message-id) nth
+ (button-get button :notmuch-content-type))
+ (message "Not a valid part (is it a fake part?)."))))))
+
+(defun notmuch-show-part-button-interactively-view (&optional button)
+ (interactive)
+ (let ((button (or button (button-at (point)))))
+ (if button
+ (let ((nth (button-get button :notmuch-part)))
+ (if nth
+ (notmuch-show-interactively-view-part (notmuch-show-get-message-id) nth
+ (button-get button :notmuch-content-type))
+ (message "Not a valid part (is it a fake part?)."))))))
;;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-17 20:26 ` Austin Clements
2012-01-17 20:39 ` Mark Walters
@ 2012-01-17 20:44 ` Aaron Ecay
1 sibling, 0 replies; 22+ messages in thread
From: Aaron Ecay @ 2012-01-17 20:44 UTC (permalink / raw)
To: Austin Clements, Mark Walters; +Cc: notmuch
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-17 20:39 ` Mark Walters
@ 2012-01-17 21:01 ` Austin Clements
2012-01-17 22:27 ` Mark Walters
0 siblings, 1 reply; 22+ messages in thread
From: Austin Clements @ 2012-01-17 21:01 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
Quoth Mark Walters on Jan 17 at 8:39 pm:
> > > 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.
>
> Sorry I messed up: it should be id:"87mxehqhbl.fsf@r102.config" However
> I have included my current draft along these lines. I think it is
> working but I am not submitting yet: just asking if this is the right
> idea.
In general, yes, I think so. A few comments on your draft below.
> Best wishes
>
> Mark
>
> From 9e52414b9871369c1cbb5c3e72d833b56bb236d4 Mon Sep 17 00:00:00 2001
> From: Mark Walters <markwalters1009@gmail.com>
> Date: Sat, 14 Jan 2012 18:04:22 +0000
> Subject: [PATCH] Make buttons for attachments allow viewing as well as saving
>
> 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
>
> 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 this
> is configurable in notmuch customize.
>
> 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)
> ---
> emacs/notmuch-show.el | 87 ++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 79 insertions(+), 8 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 03c1f6b..2413caa 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -281,10 +281,21 @@ message at DEPTH in the current thread."
> (run-hooks 'notmuch-show-markup-headers-hook)))))
>
> (define-button-type 'notmuch-show-part-button-type
> - 'action 'notmuch-show-part-button-action
> + 'action 'notmuch-show-part-button-default
> + 'keymap 'notmuch-show-part-button-map
> 'follow-link t
> 'face 'message-mml)
>
> +(defvar notmuch-show-part-button-map
> + (let ((map (make-sparse-keymap)))
> + (set-keymap-parent map button-map)
> + (define-key map "s" 'notmuch-show-part-button-save)
> + (define-key map "v" 'notmuch-show-part-button-view)
> + (define-key map "o" 'notmuch-show-part-button-interactively-view)
> + map)
> + "Submap for button commands")
> +(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
I don't think this fset is necessary. Actually, I've never seen this
outside of the notmuch code. It looks like it does appear in code
shipped with Emacs, but only in a handful of places. All of those
places look like very old code, so maybe this was necessary once upon
a time?
> +
> (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
> (let ((button))
> (setq button
> @@ -299,7 +310,8 @@ message at DEPTH in the current thread."
> " ]")
> :type 'notmuch-show-part-button-type
> :notmuch-part nth
> - :notmuch-filename name))
> + :notmuch-filename name
> + :notmuch-content-type content-type))
> (insert "\n")
> ;; return button
> button))
> @@ -323,6 +335,28 @@ message at DEPTH in the current thread."
> ;; ange-ftp, which is reasonable to use here.
> (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
>
> +(defun notmuch-show-view-part (message-id nth content-type)
> + (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))
> + ;; set mm-inlined-types to nil to force an external viewer
> + (let ((handle (mm-make-handle (current-buffer) (list content-type)))
> + (mm-inlined-types nil))
> + (mm-display-part handle t)))))
> +
> +(defun notmuch-show-interactively-view-part (message-id nth content-type)
> + (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))
> + (let ((handle (mm-make-handle (current-buffer) (list content-type))))
> + (mm-interactively-view-part handle)))))
> +
Yeah. Though you're right that the duplication is a little annoying.
With the snippet I sent earlier, these could change to, e.g.,
(defun notmuch-show-interactively-view-part (message-id nth content-type)
(notmuch-with-part-temp-buffer
message-id nth
(lambda ()
(let ((handle (mm-make-handle (current-buffer) (list content-type))))
(mm-interactively-view-part handle)))))
Maybe this is better as a macro...
(defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
(declare (indent 2))
(let ((process-crypto (make-symbol "process-crypto")))
`(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))
,@body))))
(defun notmuch-show-interactively-view-part (message-id nth content-type)
(notmuch-with-temp-part-buffer message-id nth
(let ((handle (mm-make-handle (current-buffer) (list content-type))))
(mm-interactively-view-part handle)))))
Up to you. Maybe there's no really satisfying way to write this.
> (defun notmuch-show-mm-display-part-inline (msg part nth content-type)
> "Use the mm-decode/mm-view functions to display a part in the
> current buffer, if possible."
> @@ -1502,12 +1536,49 @@ buffer."
>
> ;; Commands typically bound to buttons.
>
> -(defun notmuch-show-part-button-action (button)
> - (let ((nth (button-get button :notmuch-part)))
> - (if nth
> - (notmuch-show-save-part (notmuch-show-get-message-id) nth
> - (button-get button :notmuch-filename))
> - (message "Not a valid part (is it a fake part?)."))))
> +(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 (const :tag "Save part"
> + notmuch-show-part-button-save)
> + (const :tag "View part"
> + notmuch-show-part-button-view)
> + (const :tag "View interactively"
> + notmuch-show-part-button-interactively-view)))
You probably want this to be the handler function, rather than the
button function, since the interface to the button function is rather
awkward. That is, if someone wanted to plug in their own action, they
would want to define it in terms of the high-level handler interface
that you use above, rather than the low-level
button-with-magic-properties interface that Emacs forces you to use
below.
> +
> +(defun notmuch-show-part-button-default (&optional button)
> + (interactive)
> + (funcall notmuch-show-part-button-default-action button))
> +
> +(defun notmuch-show-part-button-save (&optional button)
> + (interactive)
> + (let ((button (or button (button-at (point)))))
> + (if button
> + (let ((nth (button-get button :notmuch-part)))
> + (if nth
> + (notmuch-show-save-part (notmuch-show-get-message-id) nth
> + (button-get button :notmuch-filename))
> + (message "Not a valid part (is it a fake part?)."))))))
> +
> +(defun notmuch-show-part-button-view (&optional button)
> + (interactive)
> + (let ((button (or button (button-at (point)))))
> + (if button
> + (let ((nth (button-get button :notmuch-part)))
> + (if nth
> + (notmuch-show-view-part (notmuch-show-get-message-id) nth
> + (button-get button :notmuch-content-type))
> + (message "Not a valid part (is it a fake part?)."))))))
> +
> +(defun notmuch-show-part-button-interactively-view (&optional button)
> + (interactive)
> + (let ((button (or button (button-at (point)))))
> + (if button
> + (let ((nth (button-get button :notmuch-part)))
> + (if nth
> + (notmuch-show-interactively-view-part (notmuch-show-get-message-id) nth
> + (button-get button :notmuch-content-type))
> + (message "Not a valid part (is it a fake part?)."))))))
This duplication is much worse, but also less necessary.
(defun notmuch-show-part-button-interactively-view (&optional button)
(interactive)
(notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))
(defun notmuch-show-part-button-internal (button handler)
(let ((button (or button (button-at (point)))))
(if button
(let ((nth (button-get button :notmuch-part)))
(if nth
(funcall handler (notmuch-show-get-message-id) nth
(button-get button :notmuch-content-type))
(message "Not a valid part (is it a fake part?)."))))))
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-17 21:01 ` Austin Clements
@ 2012-01-17 22:27 ` Mark Walters
2012-01-17 23:02 ` Austin Clements
0 siblings, 1 reply; 22+ messages in thread
From: Mark Walters @ 2012-01-17 22:27 UTC (permalink / raw)
To: Austin Clements; +Cc: notmuch
> In general, yes, I think so. A few comments on your draft below.
Ok I include a newer version which I am fairly happy with but I do have
some queries.
> > +(defvar notmuch-show-part-button-map
> > + (let ((map (make-sparse-keymap)))
> > + (set-keymap-parent map button-map)
> > + (define-key map "s" 'notmuch-show-part-button-save)
> > + (define-key map "v" 'notmuch-show-part-button-view)
> > + (define-key map "o" 'notmuch-show-part-button-interactively-view)
> > + map)
> > + "Submap for button commands")
> > +(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
>
> I don't think this fset is necessary. Actually, I've never seen this
> outside of the notmuch code. It looks like it does appear in code
> shipped with Emacs, but only in a handful of places. All of those
> places look like very old code, so maybe this was necessary once upon
> a time?
I have no idea on this: at the moment I have left it in as fset for
keymaps seems to occur throughout notmuch (I have the fset because I
copied it from somewhere).
> (defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
> (declare (indent 2))
> (let ((process-crypto (make-symbol "process-crypto")))
> `(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))
> ,@body))))
I have followed the macro approach: since notmuch-show-save-part also
uses it (which doesn't appear in the diff as it was unchanged). I have
made all three functions use notmuch-with-temp-part-buffer. However, I
used the macro exactly as you wrote it (and it seems to work) but I
moderately understand why but could not justify it to someone!
> (defun notmuch-show-interactively-view-part (message-id nth content-type)
> (notmuch-with-temp-part-buffer message-id nth
> (let ((handle (mm-make-handle (current-buffer) (list content-type))))
> (mm-interactively-view-part handle)))))
Emacs wants to indent the (let line level with message-id in the line
above which looks odd (and makes the lines too long). Do I overrule
emacs, or put message-id and nth onto a separate line or is there
something better?
Also note that, because of the unification with notmuch-show-save-part
all three functions have to have the four arguments message-id, nth,
filename and content-type (even though currently each individual
function only uses three of them). However see below for another comment
on 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 (const :tag "Save part"
> > + notmuch-show-part-button-save)
> > + (const :tag "View part"
> > + notmuch-show-part-button-view)
> > + (const :tag "View interactively"
> > + notmuch-show-part-button-interactively-view)))
>
> You probably want this to be the handler function, rather than the
> button function, since the interface to the button function is rather
> awkward. That is, if someone wanted to plug in their own action, they
> would want to define it in terms of the high-level handler interface
> that you use above, rather than the low-level
> button-with-magic-properties interface that Emacs forces you to use
> below.
I have done this.
> This duplication is much worse, but also less necessary.
>
> (defun notmuch-show-part-button-interactively-view (&optional button)
> (interactive)
> (notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))
>
> (defun notmuch-show-part-button-internal (button handler)
> (let ((button (or button (button-at (point)))))
> (if button
> (let ((nth (button-get button :notmuch-part)))
> (if nth
> (funcall handler (notmuch-show-get-message-id) nth
> (button-get button :notmuch-content-type))
> (message "Not a valid part (is it a fake part?)."))))))
Yes this is much nicer and I have done this too (modulo the extra
argument mentioned above).
Finally, I have discovered one bug/misfeature. If you try to "view" an
attachment then it will offer to save it but will not offer a
filename. If you try and save it (or use the default action) it will
offer a filename as now. As far as I can see this is not fixable if I
use mm-display-part: however, I could include a slight tweaked version,
notmuch-show-mm-display-part say, which would fix this corner
case. (Essentially, it would call notmuch-show-save-part if it failed to
find a handler rather than mailcap-save-binary-file.) However, this is
about 50 lines of lisp so I am not sure it is worth it.
Best wishes
Mark
From bda4bb7637fb7d09c50f95b6b76fd42a377e0dde Mon Sep 17 00:00:00 2001
From: Mark Walters <markwalters1009@gmail.com>
Date: Sat, 14 Jan 2012 18:04:22 +0000
Subject: [PATCH] Make buttons for attachments allow viewing as well as saving
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
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 this
is configurable in notmuch customize.
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)
---
emacs/notmuch-show.el | 105 +++++++++++++++++++++++++++++++++++++-----------
1 files changed, 81 insertions(+), 24 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 03c1f6b..2e4fecd 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -281,10 +281,21 @@ message at DEPTH in the current thread."
(run-hooks 'notmuch-show-markup-headers-hook)))))
(define-button-type 'notmuch-show-part-button-type
- 'action 'notmuch-show-part-button-action
+ 'action 'notmuch-show-part-button-default
+ 'keymap 'notmuch-show-part-button-map
'follow-link t
'face 'message-mml)
+(defvar notmuch-show-part-button-map
+ (let ((map (make-sparse-keymap)))
+ (set-keymap-parent map button-map)
+ (define-key map "s" 'notmuch-show-part-button-save)
+ (define-key map "v" 'notmuch-show-part-button-view)
+ (define-key map "o" 'notmuch-show-part-button-interactively-view)
+ map)
+ "Submap for button commands")
+(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
+
(defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
(let ((button))
(setq button
@@ -299,29 +310,48 @@ message at DEPTH in the current thread."
" ]")
:type 'notmuch-show-part-button-type
:notmuch-part nth
- :notmuch-filename name))
+ :notmuch-filename name
+ :notmuch-content-type content-type))
(insert "\n")
;; return button
button))
;; Functions handling particular MIME parts.
-(defun notmuch-show-save-part (message-id nth &optional filename)
- (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))
- (let ((file (read-file-name
- "Filename to save as: "
- (or mailcap-download-directory "~/")
- nil nil
- filename)))
- ;; Don't re-compress .gz & al. Arguably we should make
- ;; `file-name-handler-alist' nil, but that would chop
- ;; ange-ftp, which is reasonable to use here.
- (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
+(defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
+ (declare (indent 2))
+ (let ((process-crypto (make-symbol "process-crypto")))
+ `(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))
+ ,@body))))
+
+(defun notmuch-show-save-part (message-id nth &optional filename content-type)
+ (notmuch-with-temp-part-buffer message-id nth
+ (let ((file (read-file-name
+ "Filename to save as: "
+ (or mailcap-download-directory "~/")
+ nil nil
+ filename)))
+ ;; Don't re-compress .gz & al. Arguably we should make
+ ;; `file-name-handler-alist' nil, but that would chop
+ ;; ange-ftp, which is reasonable to use here.
+ (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t))))
+
+(defun notmuch-show-view-part (message-id nth &optional filename content-type )
+ (notmuch-with-temp-part-buffer message-id nth
+ ;; set mm-inlined-types to nil to force an external viewer
+ (let ((handle (mm-make-handle (current-buffer) (list content-type)))
+ (mm-inlined-types nil))
+ (mm-display-part handle t))))
+
+(defun notmuch-show-interactively-view-part (message-id nth &optional filename content-type)
+ (notmuch-with-temp-part-buffer message-id nth
+ (let ((handle (mm-make-handle (current-buffer) (list content-type))))
+ (mm-interactively-view-part handle))))
(defun notmuch-show-mm-display-part-inline (msg part nth content-type)
"Use the mm-decode/mm-view functions to display a part in the
@@ -1502,13 +1532,40 @@ buffer."
;; Commands typically bound to buttons.
-(defun notmuch-show-part-button-action (button)
- (let ((nth (button-get button :notmuch-part)))
- (if nth
- (notmuch-show-save-part (notmuch-show-get-message-id) nth
- (button-get button :notmuch-filename))
- (message "Not a valid part (is it a fake part?)."))))
+(defcustom notmuch-show-part-button-default-action 'notmuch-show-save-part
+ "Default part header button action (on ENTER or mouse click)."
+ :group 'notmuch
+ :type '(choice (const :tag "Save part"
+ notmuch-show-save-part)
+ (const :tag "View part"
+ notmuch-show-view-part)
+ (const :tag "View interactively"
+ notmuch-show-interactively-view-part)))
+
+(defun notmuch-show-part-button-default (&optional button)
+ (interactive)
+ (notmuch-show-part-button-internal button notmuch-show-part-button-default-action))
+(defun notmuch-show-part-button-save (&optional button)
+ (interactive)
+ (notmuch-show-part-button-internal button #'notmuch-show-save-part))
+
+(defun notmuch-show-part-button-view (&optional button)
+ (interactive)
+ (notmuch-show-part-button-internal button #'notmuch-show-view-part))
+
+(defun notmuch-show-part-button-interactively-view (&optional button)
+ (interactive)
+ (notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))
+
+(defun notmuch-show-part-button-internal (button handler)
+ (let ((button (or button (button-at (point)))))
+ (if button
+ (let ((nth (button-get button :notmuch-part)))
+ (if nth
+ (funcall handler (notmuch-show-get-message-id) nth
+ (button-get button :notmuch-filename)
+ (button-get button :notmuch-content-type)))))))
;;
(provide 'notmuch-show)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-17 22:27 ` Mark Walters
@ 2012-01-17 23:02 ` Austin Clements
2012-01-17 23:42 ` Mark Walters
0 siblings, 1 reply; 22+ messages in thread
From: Austin Clements @ 2012-01-17 23:02 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
Quoth Mark Walters on Jan 17 at 10:27 pm:
> > (defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
> > (declare (indent 2))
> > (let ((process-crypto (make-symbol "process-crypto")))
> > `(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))
> > ,@body))))
>
> I have followed the macro approach: since notmuch-show-save-part also
> uses it (which doesn't appear in the diff as it was unchanged). I have
> made all three functions use notmuch-with-temp-part-buffer. However, I
> used the macro exactly as you wrote it (and it seems to work) but I
> moderately understand why but could not justify it to someone!
Oops, actually there was a bug in that macro. It should have been
(defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
(declare (indent 2))
(let ((process-crypto (make-symbol "process-crypto")))
`(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))
,@body))))
The only difference is on the "insert" line. Sorry about that.
I don't know how familiar you are with syntactic abstraction, so
here's a top-down explanation. A macro is like the compile-time
equivalent of a function: rather than the arguments and return being
values that result from evaluation, they are pieces of code, the body
of the macro executes at compile time instead of at run time, and the
compiler replaces the invocation of the macro with the code that the
macro returns. This is not unlike a C/C++ macro, but the
implementation of the macro is regular Lisp code that runs at compile
time and the code is represented as S-expressions rather than strings
(one beauty of Lisp is that the representation of code and data is the
same).
The above macro returns the code starting after the "`" (pronounced
quasiquote), so that's what invocations of the macro get replaced
with. Quasiquote (like quote) inhibits the evaluation of what follows
it and results in the code as data, rather than the result of
evaluating the code. Unlike quote, quasiquote lets you jump back into
the evaluator using "," and ",@", so it's great for piecing together
code from a template, which is what macros spend most of their time
doing.
The "declare" is simply a specification to emacs-lisp-mode about how
to indent uses of this macro. The "make-symbol" is necessary to avoid
conflicting with variable names that may appear in the code that uses
this macro (since the invoking code and the code returned by the macro
will be interleaved, you have to worry about variable conflicts).
> > (defun notmuch-show-interactively-view-part (message-id nth content-type)
> > (notmuch-with-temp-part-buffer message-id nth
> > (let ((handle (mm-make-handle (current-buffer) (list content-type))))
> > (mm-interactively-view-part handle)))))
>
> Emacs wants to indent the (let line level with message-id in the line
> above which looks odd (and makes the lines too long). Do I overrule
> emacs, or put message-id and nth onto a separate line or is there
> something better?
If you evaluate the defmacro, it'll pick up on that declare line at
the beginning of it and indent this correctly.
> Also note that, because of the unification with notmuch-show-save-part
> all three functions have to have the four arguments message-id, nth,
> filename and content-type (even though currently each individual
> function only uses three of them). However see below for another comment
> on this.
That makes sense.
> > > +(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 (const :tag "Save part"
> > > + notmuch-show-part-button-save)
> > > + (const :tag "View part"
> > > + notmuch-show-part-button-view)
> > > + (const :tag "View interactively"
> > > + notmuch-show-part-button-interactively-view)))
> >
> > You probably want this to be the handler function, rather than the
> > button function, since the interface to the button function is rather
> > awkward. That is, if someone wanted to plug in their own action, they
> > would want to define it in terms of the high-level handler interface
> > that you use above, rather than the low-level
> > button-with-magic-properties interface that Emacs forces you to use
> > below.
>
> I have done this.
>
> > This duplication is much worse, but also less necessary.
> >
> > (defun notmuch-show-part-button-interactively-view (&optional button)
> > (interactive)
> > (notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))
> >
> > (defun notmuch-show-part-button-internal (button handler)
> > (let ((button (or button (button-at (point)))))
> > (if button
> > (let ((nth (button-get button :notmuch-part)))
> > (if nth
> > (funcall handler (notmuch-show-get-message-id) nth
> > (button-get button :notmuch-content-type))
> > (message "Not a valid part (is it a fake part?)."))))))
>
> Yes this is much nicer and I have done this too (modulo the extra
> argument mentioned above).
>
> Finally, I have discovered one bug/misfeature. If you try to "view" an
> attachment then it will offer to save it but will not offer a
> filename. If you try and save it (or use the default action) it will
> offer a filename as now. As far as I can see this is not fixable if I
> use mm-display-part: however, I could include a slight tweaked version,
> notmuch-show-mm-display-part say, which would fix this corner
> case. (Essentially, it would call notmuch-show-save-part if it failed to
> find a handler rather than mailcap-save-binary-file.) However, this is
> about 50 lines of lisp so I am not sure it is worth it.
Hmm. This is probably worth fixing, but probably in a separate patch.
Duplicating mm-display-part is probably not the way to go. It think
it will work to pass t as the no-default argument to mm-display-part
and check the return value, which should be 'inline if it was able to
handle it internally or 'external if it found an external helper. I'm
pretty sure it will never fall in to mailcap-save-binary-file in that
case. If that doesn't work, you could flet mailcap-save-binary-file
around the call to mm-display-part.
> Best wishes
>
> Mark
>
> From bda4bb7637fb7d09c50f95b6b76fd42a377e0dde Mon Sep 17 00:00:00 2001
> From: Mark Walters <markwalters1009@gmail.com>
> Date: Sat, 14 Jan 2012 18:04:22 +0000
> Subject: [PATCH] Make buttons for attachments allow viewing as well as saving
>
> 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
>
> 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 this
> is configurable in notmuch customize.
>
> 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)
> ---
> emacs/notmuch-show.el | 105 +++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 81 insertions(+), 24 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 03c1f6b..2e4fecd 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -281,10 +281,21 @@ message at DEPTH in the current thread."
> (run-hooks 'notmuch-show-markup-headers-hook)))))
>
> (define-button-type 'notmuch-show-part-button-type
> - 'action 'notmuch-show-part-button-action
> + 'action 'notmuch-show-part-button-default
> + 'keymap 'notmuch-show-part-button-map
> 'follow-link t
> 'face 'message-mml)
>
> +(defvar notmuch-show-part-button-map
> + (let ((map (make-sparse-keymap)))
> + (set-keymap-parent map button-map)
> + (define-key map "s" 'notmuch-show-part-button-save)
> + (define-key map "v" 'notmuch-show-part-button-view)
> + (define-key map "o" 'notmuch-show-part-button-interactively-view)
> + map)
> + "Submap for button commands")
> +(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
> +
> (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
> (let ((button))
> (setq button
> @@ -299,29 +310,48 @@ message at DEPTH in the current thread."
> " ]")
> :type 'notmuch-show-part-button-type
> :notmuch-part nth
> - :notmuch-filename name))
> + :notmuch-filename name
> + :notmuch-content-type content-type))
> (insert "\n")
> ;; return button
> button))
>
> ;; Functions handling particular MIME parts.
>
> -(defun notmuch-show-save-part (message-id nth &optional filename)
> - (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))
> - (let ((file (read-file-name
> - "Filename to save as: "
> - (or mailcap-download-directory "~/")
> - nil nil
> - filename)))
> - ;; Don't re-compress .gz & al. Arguably we should make
> - ;; `file-name-handler-alist' nil, but that would chop
> - ;; ange-ftp, which is reasonable to use here.
> - (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
> +(defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
> + (declare (indent 2))
> + (let ((process-crypto (make-symbol "process-crypto")))
> + `(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))
Should be ",message-id ,nth" instead of "message-id nth" (my fault).
> + ,@body))))
> +
> +(defun notmuch-show-save-part (message-id nth &optional filename content-type)
> + (notmuch-with-temp-part-buffer message-id nth
> + (let ((file (read-file-name
> + "Filename to save as: "
> + (or mailcap-download-directory "~/")
> + nil nil
> + filename)))
> + ;; Don't re-compress .gz & al. Arguably we should make
> + ;; `file-name-handler-alist' nil, but that would chop
> + ;; ange-ftp, which is reasonable to use here.
> + (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t))))
> +
> +(defun notmuch-show-view-part (message-id nth &optional filename content-type )
> + (notmuch-with-temp-part-buffer message-id nth
> + ;; set mm-inlined-types to nil to force an external viewer
> + (let ((handle (mm-make-handle (current-buffer) (list content-type)))
> + (mm-inlined-types nil))
> + (mm-display-part handle t))))
> +
> +(defun notmuch-show-interactively-view-part (message-id nth &optional filename content-type)
> + (notmuch-with-temp-part-buffer message-id nth
> + (let ((handle (mm-make-handle (current-buffer) (list content-type))))
> + (mm-interactively-view-part handle))))
>
> (defun notmuch-show-mm-display-part-inline (msg part nth content-type)
> "Use the mm-decode/mm-view functions to display a part in the
> @@ -1502,13 +1532,40 @@ buffer."
>
> ;; Commands typically bound to buttons.
>
> -(defun notmuch-show-part-button-action (button)
> - (let ((nth (button-get button :notmuch-part)))
> - (if nth
> - (notmuch-show-save-part (notmuch-show-get-message-id) nth
> - (button-get button :notmuch-filename))
> - (message "Not a valid part (is it a fake part?)."))))
> +(defcustom notmuch-show-part-button-default-action 'notmuch-show-save-part
> + "Default part header button action (on ENTER or mouse click)."
> + :group 'notmuch
> + :type '(choice (const :tag "Save part"
> + notmuch-show-save-part)
> + (const :tag "View part"
> + notmuch-show-view-part)
> + (const :tag "View interactively"
> + notmuch-show-interactively-view-part)))
> +
> +(defun notmuch-show-part-button-default (&optional button)
> + (interactive)
> + (notmuch-show-part-button-internal button notmuch-show-part-button-default-action))
>
> +(defun notmuch-show-part-button-save (&optional button)
> + (interactive)
> + (notmuch-show-part-button-internal button #'notmuch-show-save-part))
> +
> +(defun notmuch-show-part-button-view (&optional button)
> + (interactive)
> + (notmuch-show-part-button-internal button #'notmuch-show-view-part))
> +
> +(defun notmuch-show-part-button-interactively-view (&optional button)
> + (interactive)
> + (notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))
Much better!
> +
> +(defun notmuch-show-part-button-internal (button handler)
> + (let ((button (or button (button-at (point)))))
> + (if button
> + (let ((nth (button-get button :notmuch-part)))
> + (if nth
> + (funcall handler (notmuch-show-get-message-id) nth
> + (button-get button :notmuch-filename)
> + (button-get button :notmuch-content-type)))))))
> ;;
>
> (provide 'notmuch-show)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
2012-01-17 23:02 ` Austin Clements
@ 2012-01-17 23:42 ` Mark Walters
2012-01-17 23:44 ` [PATCH v3] " Mark Walters
0 siblings, 1 reply; 22+ messages in thread
From: Mark Walters @ 2012-01-17 23:42 UTC (permalink / raw)
To: Austin Clements; +Cc: notmuch
>
> Oops, actually there was a bug in that macro. It should have been
>
> (defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
> (declare (indent 2))
> (let ((process-crypto (make-symbol "process-crypto")))
> `(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))
> ,@body))))
>
> The only difference is on the "insert" line. Sorry about that.
Fixed.
[Snip excellent explanation of defmacro]
Thanks for the excellent explanation!
> > Finally, I have discovered one bug/misfeature. If you try to "view" an
> > attachment then it will offer to save it but will not offer a
> > filename. If you try and save it (or use the default action) it will
> > offer a filename as now. As far as I can see this is not fixable if I
> > use mm-display-part: however, I could include a slight tweaked version,
> > notmuch-show-mm-display-part say, which would fix this corner
> > case. (Essentially, it would call notmuch-show-save-part if it failed to
> > find a handler rather than mailcap-save-binary-file.) However, this is
> > about 50 lines of lisp so I am not sure it is worth it.
>
> Hmm. This is probably worth fixing, but probably in a separate patch.
> Duplicating mm-display-part is probably not the way to go. It think
> it will work to pass t as the no-default argument to mm-display-part
> and check the return value, which should be 'inline if it was able to
> handle it internally or 'external if it found an external helper. I'm
> pretty sure it will never fall in to mailcap-save-binary-file in that
> case. If that doesn't work, you could flet mailcap-save-binary-file
> around the call to mm-display-part.
I had tried passing t to mm-display-part and that didn't work as I
expected. I will experiment some more and try your flet suggestion but I
think that will be a separate patch.
I will send a potential final version of this patch as a reply to this
email. Many thanks for all the guidance and help!
Best wishes
Mark
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3] Make buttons for attachments allow viewing as well as saving
2012-01-17 23:42 ` Mark Walters
@ 2012-01-17 23:44 ` Mark Walters
2012-01-17 23:53 ` Austin Clements
2012-01-18 7:51 ` Antoine Beaupré
0 siblings, 2 replies; 22+ messages in thread
From: Mark Walters @ 2012-01-17 23:44 UTC (permalink / raw)
To: notmuch
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
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 this
is configurable in notmuch customize.
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)
---
emacs/notmuch-show.el | 106 ++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 82 insertions(+), 24 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 03c1f6b..0aaaf79 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -281,10 +281,21 @@ message at DEPTH in the current thread."
(run-hooks 'notmuch-show-markup-headers-hook)))))
(define-button-type 'notmuch-show-part-button-type
- 'action 'notmuch-show-part-button-action
+ 'action 'notmuch-show-part-button-default
+ 'keymap 'notmuch-show-part-button-map
'follow-link t
'face 'message-mml)
+(defvar notmuch-show-part-button-map
+ (let ((map (make-sparse-keymap)))
+ (set-keymap-parent map button-map)
+ (define-key map "s" 'notmuch-show-part-button-save)
+ (define-key map "v" 'notmuch-show-part-button-view)
+ (define-key map "o" 'notmuch-show-part-button-interactively-view)
+ map)
+ "Submap for button commands")
+(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
+
(defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
(let ((button))
(setq button
@@ -299,29 +310,48 @@ message at DEPTH in the current thread."
" ]")
:type 'notmuch-show-part-button-type
:notmuch-part nth
- :notmuch-filename name))
+ :notmuch-filename name
+ :notmuch-content-type content-type))
(insert "\n")
;; return button
button))
;; Functions handling particular MIME parts.
-(defun notmuch-show-save-part (message-id nth &optional filename)
- (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))
- (let ((file (read-file-name
- "Filename to save as: "
- (or mailcap-download-directory "~/")
- nil nil
- filename)))
- ;; Don't re-compress .gz & al. Arguably we should make
- ;; `file-name-handler-alist' nil, but that would chop
- ;; ange-ftp, which is reasonable to use here.
- (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
+(defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
+ (declare (indent 2))
+ (let ((process-crypto (make-symbol "process-crypto")))
+ `(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))
+ ,@body))))
+
+(defun notmuch-show-save-part (message-id nth &optional filename content-type)
+ (notmuch-with-temp-part-buffer message-id nth
+ (let ((file (read-file-name
+ "Filename to save as: "
+ (or mailcap-download-directory "~/")
+ nil nil
+ filename)))
+ ;; Don't re-compress .gz & al. Arguably we should make
+ ;; `file-name-handler-alist' nil, but that would chop
+ ;; ange-ftp, which is reasonable to use here.
+ (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t))))
+
+(defun notmuch-show-view-part (message-id nth &optional filename content-type )
+ (notmuch-with-temp-part-buffer message-id nth
+ ;; set mm-inlined-types to nil to force an external viewer
+ (let ((handle (mm-make-handle (current-buffer) (list content-type)))
+ (mm-inlined-types nil))
+ (mm-display-part handle t))))
+
+(defun notmuch-show-interactively-view-part (message-id nth &optional filename content-type)
+ (notmuch-with-temp-part-buffer message-id nth
+ (let ((handle (mm-make-handle (current-buffer) (list content-type))))
+ (mm-interactively-view-part handle))))
(defun notmuch-show-mm-display-part-inline (msg part nth content-type)
"Use the mm-decode/mm-view functions to display a part in the
@@ -1502,12 +1532,40 @@ buffer."
;; Commands typically bound to buttons.
-(defun notmuch-show-part-button-action (button)
- (let ((nth (button-get button :notmuch-part)))
- (if nth
- (notmuch-show-save-part (notmuch-show-get-message-id) nth
- (button-get button :notmuch-filename))
- (message "Not a valid part (is it a fake part?)."))))
+(defcustom notmuch-show-part-button-default-action 'notmuch-show-save-part
+ "Default part header button action (on ENTER or mouse click)."
+ :group 'notmuch
+ :type '(choice (const :tag "Save part"
+ notmuch-show-save-part)
+ (const :tag "View part"
+ notmuch-show-view-part)
+ (const :tag "View interactively"
+ notmuch-show-interactively-view-part)))
+
+(defun notmuch-show-part-button-default (&optional button)
+ (interactive)
+ (notmuch-show-part-button-internal button notmuch-show-part-button-default-action))
+
+(defun notmuch-show-part-button-save (&optional button)
+ (interactive)
+ (notmuch-show-part-button-internal button #'notmuch-show-save-part))
+
+(defun notmuch-show-part-button-view (&optional button)
+ (interactive)
+ (notmuch-show-part-button-internal button #'notmuch-show-view-part))
+
+(defun notmuch-show-part-button-interactively-view (&optional button)
+ (interactive)
+ (notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))
+
+(defun notmuch-show-part-button-internal (button handler)
+ (let ((button (or button (button-at (point)))))
+ (if button
+ (let ((nth (button-get button :notmuch-part)))
+ (if nth
+ (funcall handler (notmuch-show-get-message-id) nth
+ (button-get button :notmuch-filename)
+ (button-get button :notmuch-content-type)))))))
;;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3] Make buttons for attachments allow viewing as well as saving
2012-01-17 23:44 ` [PATCH v3] " Mark Walters
@ 2012-01-17 23:53 ` Austin Clements
2012-01-18 0:40 ` Mark Walters
2012-01-18 7:51 ` Antoine Beaupré
1 sibling, 1 reply; 22+ messages in thread
From: Austin Clements @ 2012-01-17 23:53 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
Quoth Mark Walters on Jan 17 at 11:44 pm:
> 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
>
> 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 this
> is configurable in notmuch customize.
>
> 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)
Oof, sorry. Two more tweaks that I really should have caught in the
previous version. After that this gets my automatic +1.
> ---
> emacs/notmuch-show.el | 106 ++++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 82 insertions(+), 24 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 03c1f6b..0aaaf79 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -281,10 +281,21 @@ message at DEPTH in the current thread."
> (run-hooks 'notmuch-show-markup-headers-hook)))))
>
> (define-button-type 'notmuch-show-part-button-type
> - 'action 'notmuch-show-part-button-action
> + 'action 'notmuch-show-part-button-default
> + 'keymap 'notmuch-show-part-button-map
> 'follow-link t
> 'face 'message-mml)
>
> +(defvar notmuch-show-part-button-map
> + (let ((map (make-sparse-keymap)))
> + (set-keymap-parent map button-map)
> + (define-key map "s" 'notmuch-show-part-button-save)
> + (define-key map "v" 'notmuch-show-part-button-view)
> + (define-key map "o" 'notmuch-show-part-button-interactively-view)
Indentation.
> + map)
> + "Submap for button commands")
> +(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
> +
> (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
> (let ((button))
> (setq button
> @@ -299,29 +310,48 @@ message at DEPTH in the current thread."
> " ]")
> :type 'notmuch-show-part-button-type
> :notmuch-part nth
> - :notmuch-filename name))
> + :notmuch-filename name
> + :notmuch-content-type content-type))
> (insert "\n")
> ;; return button
> button))
>
> ;; Functions handling particular MIME parts.
>
> -(defun notmuch-show-save-part (message-id nth &optional filename)
> - (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))
> - (let ((file (read-file-name
> - "Filename to save as: "
> - (or mailcap-download-directory "~/")
> - nil nil
> - filename)))
> - ;; Don't re-compress .gz & al. Arguably we should make
> - ;; `file-name-handler-alist' nil, but that would chop
> - ;; ange-ftp, which is reasonable to use here.
> - (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
> +(defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)
> + (declare (indent 2))
> + (let ((process-crypto (make-symbol "process-crypto")))
> + `(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))
> + ,@body))))
> +
> +(defun notmuch-show-save-part (message-id nth &optional filename content-type)
> + (notmuch-with-temp-part-buffer message-id nth
> + (let ((file (read-file-name
> + "Filename to save as: "
> + (or mailcap-download-directory "~/")
> + nil nil
> + filename)))
> + ;; Don't re-compress .gz & al. Arguably we should make
> + ;; `file-name-handler-alist' nil, but that would chop
> + ;; ange-ftp, which is reasonable to use here.
> + (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t))))
> +
> +(defun notmuch-show-view-part (message-id nth &optional filename content-type )
> + (notmuch-with-temp-part-buffer message-id nth
> + ;; set mm-inlined-types to nil to force an external viewer
> + (let ((handle (mm-make-handle (current-buffer) (list content-type)))
> + (mm-inlined-types nil))
> + (mm-display-part handle t))))
> +
> +(defun notmuch-show-interactively-view-part (message-id nth &optional filename content-type)
> + (notmuch-with-temp-part-buffer message-id nth
> + (let ((handle (mm-make-handle (current-buffer) (list content-type))))
> + (mm-interactively-view-part handle))))
>
> (defun notmuch-show-mm-display-part-inline (msg part nth content-type)
> "Use the mm-decode/mm-view functions to display a part in the
> @@ -1502,12 +1532,40 @@ buffer."
>
> ;; Commands typically bound to buttons.
>
> -(defun notmuch-show-part-button-action (button)
> - (let ((nth (button-get button :notmuch-part)))
> - (if nth
> - (notmuch-show-save-part (notmuch-show-get-message-id) nth
> - (button-get button :notmuch-filename))
> - (message "Not a valid part (is it a fake part?)."))))
> +(defcustom notmuch-show-part-button-default-action 'notmuch-show-save-part
> + "Default part header button action (on ENTER or mouse click)."
> + :group 'notmuch
> + :type '(choice (const :tag "Save part"
> + notmuch-show-save-part)
> + (const :tag "View part"
> + notmuch-show-view-part)
> + (const :tag "View interactively"
> + notmuch-show-interactively-view-part)))
defcustoms customarily appear at the top of the file (see the rest of
the defcustoms in notmuch-show.el)
> +
> +(defun notmuch-show-part-button-default (&optional button)
> + (interactive)
> + (notmuch-show-part-button-internal button notmuch-show-part-button-default-action))
> +
> +(defun notmuch-show-part-button-save (&optional button)
> + (interactive)
> + (notmuch-show-part-button-internal button #'notmuch-show-save-part))
> +
> +(defun notmuch-show-part-button-view (&optional button)
> + (interactive)
> + (notmuch-show-part-button-internal button #'notmuch-show-view-part))
> +
> +(defun notmuch-show-part-button-interactively-view (&optional button)
> + (interactive)
> + (notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))
> +
> +(defun notmuch-show-part-button-internal (button handler)
> + (let ((button (or button (button-at (point)))))
> + (if button
> + (let ((nth (button-get button :notmuch-part)))
> + (if nth
> + (funcall handler (notmuch-show-get-message-id) nth
> + (button-get button :notmuch-filename)
> + (button-get button :notmuch-content-type)))))))
>
> ;;
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] Make buttons for attachments allow viewing as well as saving
2012-01-17 23:53 ` Austin Clements
@ 2012-01-18 0:40 ` Mark Walters
2012-01-18 1:25 ` Austin Clements
0 siblings, 1 reply; 22+ messages in thread
From: Mark Walters @ 2012-01-18 0:40 UTC (permalink / raw)
To: Austin Clements; +Cc: notmuch
> Oof, sorry. Two more tweaks that I really should have caught in the
> previous version. After that this gets my automatic +1.
Both fixed. I have also fixed the bug I mentioned (missing filename when
"view" falls back on save); I couldn't make it work with the
"no-default" option. However overriding mm-save-part with flet seems to
do the trick.
+(defun notmuch-show-view-part (message-id nth &optional filename content-type )
+ (notmuch-with-temp-part-buffer message-id nth
+ ;; set mm-inlined-types to nil to force an external viewer
+ (let ((handle (mm-make-handle (current-buffer) (list content-type)))
+ (mm-inlined-types nil))
+ ;; We override mm-save-part as notmuch-show-save-part is better
+ ;; since it offers the filename
+ (flet ((mm-save-part (&rest args) (ignore)))
+ (or (mm-display-part handle)
+ (notmuch-show-save-part message-id nth filename content-type))))))
Is that a reasonable solution?
Best wishes
Mark
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] Make buttons for attachments allow viewing as well as saving
2012-01-18 0:40 ` Mark Walters
@ 2012-01-18 1:25 ` Austin Clements
2012-01-18 10:46 ` Mark Walters
0 siblings, 1 reply; 22+ messages in thread
From: Austin Clements @ 2012-01-18 1:25 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
Quoth Mark Walters on Jan 18 at 12:40 am:
>
> > Oof, sorry. Two more tweaks that I really should have caught in the
> > previous version. After that this gets my automatic +1.
>
> Both fixed. I have also fixed the bug I mentioned (missing filename when
> "view" falls back on save); I couldn't make it work with the
> "no-default" option. However overriding mm-save-part with flet seems to
> do the trick.
Oh, indeed. I'd foolishly assumed that when mm-display-part passed
the function mailcap-save-binary-file as the method to
mm-display-external that it would actually *use* that function, but
you're right that it uses mm-save-part.
> +(defun notmuch-show-view-part (message-id nth &optional filename content-type )
> + (notmuch-with-temp-part-buffer message-id nth
> + ;; set mm-inlined-types to nil to force an external viewer
> + (let ((handle (mm-make-handle (current-buffer) (list content-type)))
> + (mm-inlined-types nil))
> + ;; We override mm-save-part as notmuch-show-save-part is better
> + ;; since it offers the filename
> + (flet ((mm-save-part (&rest args) (ignore)))
> + (or (mm-display-part handle)
> + (notmuch-show-save-part message-id nth filename content-type))))))
>
> Is that a reasonable solution?
It's *probably* safe to depend on the result of mm-display-part, but
you can avoid the question altogether by simply calling
notmuch-show-save-part from your flet mm-save-part. E.g.,
(flet ((mm-save-part (&rest args) (notmuch-show-save-part
message-id nth filename content-type)))
(mm-display-part handle))
(Yeah, flet indentation is lame.)
> Best wishes
>
> Mark
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] Make buttons for attachments allow viewing as well as saving
2012-01-17 23:44 ` [PATCH v3] " Mark Walters
2012-01-17 23:53 ` Austin Clements
@ 2012-01-18 7:51 ` Antoine Beaupré
1 sibling, 0 replies; 22+ messages in thread
From: Antoine Beaupré @ 2012-01-18 7:51 UTC (permalink / raw)
To: Mark Walters, notmuch
[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]
On Tue, 17 Jan 2012 23:44:46 +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
>
> 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 this
> is configurable in notmuch customize.
>
> 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)
This works pretty much as advertised, +1 from me, good work.
A.
--
C'est trop facile quand les guerres sont finies
D'aller gueuler que c'était la dernière
Amis bourgeois vous me faites envie
Ne voyez vous pas donc point vos cimetières?
- Jaques Brel
[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] Make buttons for attachments allow viewing as well as saving
2012-01-18 1:25 ` Austin Clements
@ 2012-01-18 10:46 ` Mark Walters
2012-01-18 19:21 ` Austin Clements
0 siblings, 1 reply; 22+ messages in thread
From: Mark Walters @ 2012-01-18 10:46 UTC (permalink / raw)
To: Austin Clements; +Cc: notmuch
> > +(defun notmuch-show-view-part (message-id nth &optional filename content-type )
> > + (notmuch-with-temp-part-buffer message-id nth
> > + ;; set mm-inlined-types to nil to force an external viewer
> > + (let ((handle (mm-make-handle (current-buffer) (list content-type)))
> > + (mm-inlined-types nil))
> > + ;; We override mm-save-part as notmuch-show-save-part is better
> > + ;; since it offers the filename
> > + (flet ((mm-save-part (&rest args) (ignore)))
> > + (or (mm-display-part handle)
> > + (notmuch-show-save-part message-id nth filename content-type))))))
> >
> > Is that a reasonable solution?
>
> It's *probably* safe to depend on the result of mm-display-part, but
> you can avoid the question altogether by simply calling
> notmuch-show-save-part from your flet mm-save-part. E.g.,
>
> (flet ((mm-save-part (&rest args) (notmuch-show-save-part
> message-id nth filename content-type)))
> (mm-display-part handle))
Unfortunately that does not work since mm-display-part has a local
variable "filename". I could copy the variables to some notmuch
prefixed variables but maybe there is some obvious "quoting" to avoid
the problem? (I can't easily check now as the gnu site is closed for the
day.)
Best wishes
Mark
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] Make buttons for attachments allow viewing as well as saving
2012-01-18 10:46 ` Mark Walters
@ 2012-01-18 19:21 ` Austin Clements
0 siblings, 0 replies; 22+ messages in thread
From: Austin Clements @ 2012-01-18 19:21 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
Quoth Mark Walters on Jan 18 at 10:46 am:
>
> > > +(defun notmuch-show-view-part (message-id nth &optional filename content-type )
> > > + (notmuch-with-temp-part-buffer message-id nth
> > > + ;; set mm-inlined-types to nil to force an external viewer
> > > + (let ((handle (mm-make-handle (current-buffer) (list content-type)))
> > > + (mm-inlined-types nil))
> > > + ;; We override mm-save-part as notmuch-show-save-part is better
> > > + ;; since it offers the filename
> > > + (flet ((mm-save-part (&rest args) (ignore)))
> > > + (or (mm-display-part handle)
> > > + (notmuch-show-save-part message-id nth filename content-type))))))
> > >
> > > Is that a reasonable solution?
> >
> > It's *probably* safe to depend on the result of mm-display-part, but
> > you can avoid the question altogether by simply calling
> > notmuch-show-save-part from your flet mm-save-part. E.g.,
> >
> > (flet ((mm-save-part (&rest args) (notmuch-show-save-part
> > message-id nth filename content-type)))
> > (mm-display-part handle))
>
> Unfortunately that does not work since mm-display-part has a local
> variable "filename". I could copy the variables to some notmuch
> prefixed variables but maybe there is some obvious "quoting" to avoid
> the problem? (I can't easily check now as the gnu site is closed for the
> day.)
Arrrg, dynamic scoping. Well, you can
;; Lexically bind everything we need in mm-save-part to prevent
;; potential dynamic shadowing.
(lexical-let ((message-id message-id)
(nth nth)
(filename filename)
(content-type content-type))
(flet ((mm-save-part (&rest args) (notmuch-show-save-part
message-id nth filename content-type)))
(mm-display-part handle)))
Or you could just, y'know, do what were doing.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2012-01-18 19:22 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-15 12:16 [PATCH 0/1] emacs: allow viewing as well saving individual attachments Mark Walters
2012-01-15 12:16 ` [PATCH 1/1] Make buttons for attachments allow viewing as well as saving Mark Walters
2012-01-16 19:31 ` Jameson Graef Rollins
2012-01-16 20:11 ` Austin Clements
2012-01-16 21:44 ` Mark Walters
2012-01-17 2:23 ` Austin Clements
2012-01-17 9:06 ` Mark Walters
2012-01-17 20:26 ` Austin Clements
2012-01-17 20:39 ` Mark Walters
2012-01-17 21:01 ` Austin Clements
2012-01-17 22:27 ` Mark Walters
2012-01-17 23:02 ` Austin Clements
2012-01-17 23:42 ` Mark Walters
2012-01-17 23:44 ` [PATCH v3] " Mark Walters
2012-01-17 23:53 ` Austin Clements
2012-01-18 0:40 ` Mark Walters
2012-01-18 1:25 ` Austin Clements
2012-01-18 10:46 ` Mark Walters
2012-01-18 19:21 ` Austin Clements
2012-01-18 7:51 ` Antoine Beaupré
2012-01-17 20:44 ` [PATCH 1/1] " Aaron Ecay
2012-01-16 9:22 ` [PATCH 0/1] emacs: allow viewing as well saving individual attachments David Edmondson
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).