Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
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-sbyq...@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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
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-sbyq...@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. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
I am happy to make that change. My original patch in the summer was more like that: id:caludzswato+4mcuoomk+8vfs+pog-xuma6u-aqx2m6-sbyq...@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 markwalters1...@gmail.com Date: Sat, 14 Jan 2012 18:04:22 + 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) +
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
On Tue, 17 Jan 2012 15:26:03 -0500, Austin Clements amdra...@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-sbyq...@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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
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-sbyq...@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 markwalters1...@gmail.com Date: Sat, 14 Jan 2012 18:04:22 + 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
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
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
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
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)
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
Quoth Jameson Graef Rollins on Jan 16 at 11:31 am: On Sun, 15 Jan 2012 12:16:36 +, Mark Walters markwalters1...@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))) ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
(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-markwalters1...@gmail.com) On Mon, 16 Jan 2012 11:31:16 -0800, Jameson Graef Rollins jroll...@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-markwalters1...@gmail.com Best wishes Mark ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as saving
Quoth Mark Walters on Jan 16 at 9:44 pm: On Mon, 16 Jan 2012 11:31:16 -0800, Jameson Graef Rollins jroll...@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. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
