31/dbfe8b5896d33ac6abdd438bb31d420dd7b1e7

   1 Return-Path: <m.walters@qmul.ac.uk>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 657B542119E\r
   6         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 14:27:10 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -1.098\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
  12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
  13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id uJ4B5moORjZM for <notmuch@notmuchmail.org>;\r
  17         Tue, 17 Jan 2012 14:27:09 -0800 (PST)\r
  18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id F32E1421192\r
  22         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 14:27:08 -0800 (PST)\r
  23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
  24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
  25         (envelope-from <m.walters@qmul.ac.uk>)\r
  26         id 1RnHUd-0003mj-Pq; Tue, 17 Jan 2012 22:27:04 +0000\r
  27 Received: from 94-192-233-223.zone6.bethere.co.uk ([94.192.233.223]\r
  28         helo=localhost)\r
  29         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)\r
  30         (envelope-from <m.walters@qmul.ac.uk>)\r
  31         id 1RnHUd-000572-7i; Tue, 17 Jan 2012 22:27:03 +0000\r
  32 From: Mark Walters <markwalters1009@gmail.com>\r
  33 To: Austin Clements <amdragon@MIT.EDU>\r
  34 Subject: Re: [PATCH 1/1] Make buttons for attachments allow viewing as well as\r
  35         saving\r
  36 In-Reply-To: <20120117210158.GS16740@mit.edu>\r
  37 References: <1326629796-11436-1-git-send-email-markwalters1009@gmail.com>\r
  38         <1326629796-11436-2-git-send-email-markwalters1009@gmail.com>\r
  39         <87wr8r5trv.fsf@servo.finestructure.net>\r
  40         <87lip7fhkc.fsf@qmul.ac.uk> <20120117022330.GE16740@mit.edu>\r
  41         <8739beitq4.fsf@qmul.ac.uk> <20120117202603.GP16740@mit.edu>\r
  42         <871uqy3vy4.fsf@qmul.ac.uk> <20120117210158.GS16740@mit.edu>\r
  43 User-Agent: Notmuch/0.11~rc2+73~g1ea2b60 (http://notmuchmail.org) Emacs/23.2.1\r
  44         (i486-pc-linux-gnu)\r
  45 Date: Tue, 17 Jan 2012 22:27:51 +0000\r
  46 Message-ID: <87obu2q80o.fsf@qmul.ac.uk>\r
  47 MIME-Version: 1.0\r
  48 Content-Type: text/plain; charset=us-ascii\r
  49 X-Sender-Host-Address: 94.192.233.223\r
  50 X-QM-SPAM-Info: Sender has good ham record.  :)\r
  51 X-QM-Body-MD5: 27229b3f5b3316ce06f4fcf69b487582 (of first 20000 bytes)\r
  52 X-SpamAssassin-Score: -1.8\r
  53 X-SpamAssassin-SpamBar: -\r
  54 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
  55         determine if it is\r
  56         spam. We require at least 5.0 points to mark a message as spam.\r
  57         This message scored -1.8 points.\r
  58         Summary of the scoring: \r
  59         * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,\r
  60         *      medium trust\r
  61         *      [138.37.6.40 listed in list.dnswl.org]\r
  62         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
  63         provider *      (markwalters1009[at]gmail.com)\r
  64         * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay\r
  65         *      domain\r
  66         *  0.6 AWL AWL: From: address is in the auto white-list\r
  67 X-QM-Scan-Virus: ClamAV says the message is clean\r
  68 Cc: notmuch@notmuchmail.org\r
  69 X-BeenThere: notmuch@notmuchmail.org\r
  70 X-Mailman-Version: 2.1.13\r
  71 Precedence: list\r
  72 List-Id: "Use and development of the notmuch mail system."\r
  73         <notmuch.notmuchmail.org>\r
  74 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  75         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  76 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  77 List-Post: <mailto:notmuch@notmuchmail.org>\r
  78 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  79 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  80         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  81 X-List-Received-Date: Tue, 17 Jan 2012 22:27:10 -0000\r
  82 \r
  83 \r
  84 > In general, yes, I think so.  A few comments on your draft below.\r
  85 \r
  86 Ok I include a newer version which I am fairly happy with but I do have\r
  87 some queries.\r
  88 \r
  89 > > +(defvar notmuch-show-part-button-map\r
  90 > > +  (let ((map (make-sparse-keymap)))\r
  91 > > +       (set-keymap-parent map button-map)\r
  92 > > +       (define-key map "s" 'notmuch-show-part-button-save)\r
  93 > > +       (define-key map "v" 'notmuch-show-part-button-view)\r
  94 > > +       (define-key map "o" 'notmuch-show-part-button-interactively-view)\r
  95 > > +    map)\r
  96 > > +  "Submap for button commands")\r
  97 > > +(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)\r
  98 > \r
  99 > I don't think this fset is necessary.  Actually, I've never seen this\r
 100 > outside of the notmuch code.  It looks like it does appear in code\r
 101 > shipped with Emacs, but only in a handful of places.  All of those\r
 102 > places look like very old code, so maybe this was necessary once upon\r
 103 > a time?\r
 104 \r
 105 I have no idea on this: at the moment I have left it in as fset for\r
 106 keymaps seems to occur throughout notmuch (I have the fset because I\r
 107 copied it from somewhere).\r
 108 \r
 109 > (defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)\r
 110 >   (declare (indent 2))\r
 111 >   (let ((process-crypto (make-symbol "process-crypto")))\r
 112 >     `(let ((,process-crypto notmuch-show-process-crypto))\r
 113 >        (with-temp-buffer\r
 114 >          (setq notmuch-show-process-crypto ,process-crypto)\r
 115 >          ;; Always acquires the part via `notmuch part', even if it is\r
 116 >          ;; available in the JSON output.\r
 117 >          (insert (notmuch-show-get-bodypart-internal message-id nth))\r
 118 >          ,@body))))\r
 119 \r
 120 I have followed the macro approach: since notmuch-show-save-part also\r
 121 uses it (which doesn't appear in the diff as it was unchanged). I have\r
 122 made all three functions use notmuch-with-temp-part-buffer. However, I\r
 123 used the macro exactly as you wrote it (and it seems to work) but I\r
 124 moderately understand why but could not justify it to someone! \r
 125 \r
 126 > (defun notmuch-show-interactively-view-part (message-id nth content-type)\r
 127 >   (notmuch-with-temp-part-buffer message-id nth\r
 128 >     (let ((handle (mm-make-handle (current-buffer) (list content-type))))\r
 129 >       (mm-interactively-view-part handle)))))\r
 130 \r
 131 Emacs wants to indent the (let line level with message-id in the line\r
 132 above which looks odd (and makes the lines too long). Do I overrule\r
 133 emacs, or put message-id and nth onto a separate line or is there\r
 134 something better?\r
 135 \r
 136 Also note that, because of the unification with notmuch-show-save-part\r
 137 all three functions have to have the four arguments message-id, nth,\r
 138 filename and content-type (even though currently each individual\r
 139 function only uses three of them). However see below for another comment\r
 140 on this.\r
 141 \r
 142 > > +(defcustom notmuch-show-part-button-default-action 'notmuch-show-part-button-save\r
 143 > > +  "Default part header button action (on ENTER or mouse click)."\r
 144 > > +  :group 'notmuch\r
 145 > > +  :type '(choice (const :tag "Save part"\r
 146 > > +                   notmuch-show-part-button-save)\r
 147 > > +            (const :tag "View part"\r
 148 > > +                   notmuch-show-part-button-view)\r
 149 > > +            (const :tag "View interactively"\r
 150 > > +                   notmuch-show-part-button-interactively-view)))\r
 151 > \r
 152 > You probably want this to be the handler function, rather than the\r
 153 > button function, since the interface to the button function is rather\r
 154 > awkward.  That is, if someone wanted to plug in their own action, they\r
 155 > would want to define it in terms of the high-level handler interface\r
 156 > that you use above, rather than the low-level\r
 157 > button-with-magic-properties interface that Emacs forces you to use\r
 158 > below.\r
 159 \r
 160 I have done this.\r
 161 \r
 162 > This duplication is much worse, but also less necessary.\r
 163 > \r
 164 > (defun notmuch-show-part-button-interactively-view (&optional button)\r
 165 >   (interactive)\r
 166 >   (notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))\r
 167 > \r
 168 > (defun notmuch-show-part-button-internal (button handler)\r
 169 >   (let ((button (or button (button-at (point)))))\r
 170 >     (if button\r
 171 >       (let ((nth (button-get button :notmuch-part)))\r
 172 >         (if nth\r
 173 >             (funcall handler (notmuch-show-get-message-id) nth\r
 174 >                              (button-get button :notmuch-content-type))\r
 175 >           (message "Not a valid part (is it a fake part?)."))))))\r
 176 \r
 177 Yes this is much nicer and I have done this too (modulo the extra\r
 178 argument mentioned above).\r
 179 \r
 180 Finally, I have discovered one bug/misfeature. If you try to "view" an\r
 181 attachment then it will offer to save it but will not offer a\r
 182 filename. If you try and save it (or use the default action) it will\r
 183 offer a filename as now. As far as I can see this is not fixable if I\r
 184 use mm-display-part: however, I could include a slight tweaked version,\r
 185 notmuch-show-mm-display-part say, which would fix this corner\r
 186 case. (Essentially, it would call notmuch-show-save-part if it failed to\r
 187 find a handler rather than mailcap-save-binary-file.) However, this is\r
 188 about 50 lines of lisp so I am not sure it is worth it.\r
 189 \r
 190 Best wishes\r
 191 \r
 192 Mark\r
 193 \r
 194 >From bda4bb7637fb7d09c50f95b6b76fd42a377e0dde Mon Sep 17 00:00:00 2001\r
 195 From: Mark Walters <markwalters1009@gmail.com>\r
 196 Date: Sat, 14 Jan 2012 18:04:22 +0000\r
 197 Subject: [PATCH] Make buttons for attachments allow viewing as well as saving\r
 198 \r
 199 Define a keymap for attachment buttons to allow multiple actions.\r
 200 Define 3 possible actions:\r
 201     save attachment: exactly as currently,\r
 202     view attachment: uses mailcap entry,\r
 203     view attachment with user chosen program\r
 204 \r
 205 Keymap on a button is: s for save, v for view and o for view with\r
 206 other program. Default (i.e. enter or mouse button) is save but this\r
 207 is configurable in notmuch customize.\r
 208 \r
 209 One implementation detail: the view attachment function forces all\r
 210 attachments to be "displayed" using mailcap even if emacs could\r
 211 display them itself. Thus, for example, text/html appears in a browser\r
 212 and text/plain asks whether to save (on a standard debian setup)\r
 213 ---\r
 214  emacs/notmuch-show.el |  105 +++++++++++++++++++++++++++++++++++++-----------\r
 215  1 files changed, 81 insertions(+), 24 deletions(-)\r
 216 \r
 217 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
 218 index 03c1f6b..2e4fecd 100644\r
 219 --- a/emacs/notmuch-show.el\r
 220 +++ b/emacs/notmuch-show.el\r
 221 @@ -281,10 +281,21 @@ message at DEPTH in the current thread."\r
 222         (run-hooks 'notmuch-show-markup-headers-hook)))))\r
 223  \r
 224  (define-button-type 'notmuch-show-part-button-type\r
 225 -  'action 'notmuch-show-part-button-action\r
 226 +  'action 'notmuch-show-part-button-default\r
 227 +  'keymap 'notmuch-show-part-button-map\r
 228    'follow-link t\r
 229    'face 'message-mml)\r
 230  \r
 231 +(defvar notmuch-show-part-button-map\r
 232 +  (let ((map (make-sparse-keymap)))\r
 233 +       (set-keymap-parent map button-map)\r
 234 +       (define-key map "s" 'notmuch-show-part-button-save)\r
 235 +       (define-key map "v" 'notmuch-show-part-button-view)\r
 236 +       (define-key map "o" 'notmuch-show-part-button-interactively-view)\r
 237 +    map)\r
 238 +  "Submap for button commands")\r
 239 +(fset 'notmuch-show-part-button-map notmuch-show-part-button-map)\r
 240 +\r
 241  (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)\r
 242    (let ((button))\r
 243      (setq button\r
 244 @@ -299,29 +310,48 @@ message at DEPTH in the current thread."\r
 245                    " ]")\r
 246            :type 'notmuch-show-part-button-type\r
 247            :notmuch-part nth\r
 248 -          :notmuch-filename name))\r
 249 +          :notmuch-filename name\r
 250 +          :notmuch-content-type content-type))\r
 251      (insert "\n")\r
 252      ;; return button\r
 253      button))\r
 254  \r
 255  ;; Functions handling particular MIME parts.\r
 256  \r
 257 -(defun notmuch-show-save-part (message-id nth &optional filename)\r
 258 -  (let ((process-crypto notmuch-show-process-crypto))\r
 259 -    (with-temp-buffer\r
 260 -      (setq notmuch-show-process-crypto process-crypto)\r
 261 -      ;; Always acquires the part via `notmuch part', even if it is\r
 262 -      ;; available in the JSON output.\r
 263 -      (insert (notmuch-show-get-bodypart-internal message-id nth))\r
 264 -      (let ((file (read-file-name\r
 265 -                  "Filename to save as: "\r
 266 -                  (or mailcap-download-directory "~/")\r
 267 -                  nil nil\r
 268 -                  filename)))\r
 269 -       ;; Don't re-compress .gz & al.  Arguably we should make\r
 270 -       ;; `file-name-handler-alist' nil, but that would chop\r
 271 -       ;; ange-ftp, which is reasonable to use here.\r
 272 -       (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))\r
 273 +(defmacro notmuch-with-temp-part-buffer (message-id nth &rest body)\r
 274 +  (declare (indent 2))\r
 275 +  (let ((process-crypto (make-symbol "process-crypto")))\r
 276 +    `(let ((,process-crypto notmuch-show-process-crypto))\r
 277 +       (with-temp-buffer\r
 278 +        (setq notmuch-show-process-crypto ,process-crypto)\r
 279 +        ;; Always acquires the part via `notmuch part', even if it is\r
 280 +        ;; available in the JSON output.\r
 281 +        (insert (notmuch-show-get-bodypart-internal message-id nth))\r
 282 +        ,@body))))\r
 283 +\r
 284 +(defun notmuch-show-save-part (message-id nth &optional filename content-type)\r
 285 +  (notmuch-with-temp-part-buffer message-id nth\r
 286 +     (let ((file (read-file-name\r
 287 +                 "Filename to save as: "\r
 288 +                 (or mailcap-download-directory "~/")\r
 289 +                 nil nil\r
 290 +                 filename)))\r
 291 +       ;; Don't re-compress .gz & al.  Arguably we should make\r
 292 +       ;; `file-name-handler-alist' nil, but that would chop\r
 293 +       ;; ange-ftp, which is reasonable to use here.\r
 294 +       (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t))))\r
 295 +\r
 296 +(defun notmuch-show-view-part (message-id nth &optional filename content-type )\r
 297 +  (notmuch-with-temp-part-buffer message-id nth\r
 298 +   ;; set mm-inlined-types to nil to force an external viewer\r
 299 +    (let ((handle (mm-make-handle (current-buffer) (list content-type)))\r
 300 +         (mm-inlined-types nil))\r
 301 +      (mm-display-part handle t))))\r
 302 +\r
 303 +(defun notmuch-show-interactively-view-part (message-id nth &optional filename content-type)\r
 304 +  (notmuch-with-temp-part-buffer message-id nth\r
 305 +   (let ((handle (mm-make-handle (current-buffer) (list content-type))))\r
 306 +     (mm-interactively-view-part handle))))\r
 307  \r
 308  (defun notmuch-show-mm-display-part-inline (msg part nth content-type)\r
 309    "Use the mm-decode/mm-view functions to display a part in the\r
 310 @@ -1502,13 +1532,40 @@ buffer."\r
 311  \r
 312  ;; Commands typically bound to buttons.\r
 313  \r
 314 -(defun notmuch-show-part-button-action (button)\r
 315 -  (let ((nth (button-get button :notmuch-part)))\r
 316 -    (if nth\r
 317 -       (notmuch-show-save-part (notmuch-show-get-message-id) nth\r
 318 -                               (button-get button :notmuch-filename))\r
 319 -      (message "Not a valid part (is it a fake part?)."))))\r
 320 +(defcustom notmuch-show-part-button-default-action 'notmuch-show-save-part\r
 321 +  "Default part header button action (on ENTER or mouse click)."\r
 322 +  :group 'notmuch\r
 323 +  :type '(choice (const :tag "Save part"\r
 324 +                       notmuch-show-save-part)\r
 325 +                (const :tag "View part"\r
 326 +                       notmuch-show-view-part)\r
 327 +                (const :tag "View interactively"\r
 328 +                       notmuch-show-interactively-view-part)))\r
 329 +\r
 330 +(defun notmuch-show-part-button-default (&optional button)\r
 331 +  (interactive)\r
 332 +  (notmuch-show-part-button-internal button notmuch-show-part-button-default-action))\r
 333  \r
 334 +(defun notmuch-show-part-button-save (&optional button)\r
 335 +  (interactive)\r
 336 +  (notmuch-show-part-button-internal button #'notmuch-show-save-part))\r
 337 +\r
 338 +(defun notmuch-show-part-button-view (&optional button)\r
 339 +  (interactive)\r
 340 +  (notmuch-show-part-button-internal button #'notmuch-show-view-part))\r
 341 +\r
 342 +(defun notmuch-show-part-button-interactively-view (&optional button)\r
 343 +  (interactive)\r
 344 +  (notmuch-show-part-button-internal button #'notmuch-show-interactively-view-part))\r
 345 +\r
 346 +(defun notmuch-show-part-button-internal (button handler)\r
 347 +  (let ((button (or button (button-at (point)))))\r
 348 +    (if button\r
 349 +       (let ((nth (button-get button :notmuch-part)))\r
 350 +         (if nth\r
 351 +             (funcall handler (notmuch-show-get-message-id) nth\r
 352 +                      (button-get button :notmuch-filename)\r
 353 +                      (button-get button :notmuch-content-type)))))))\r
 354  ;;\r
 355  \r
 356  (provide 'notmuch-show)\r
 357 -- \r
 358 1.7.2.3\r
 359 \r