--- /dev/null
+Return-Path: <amdragon@mit.edu>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 2CB00431FC3\r
+ for <notmuch@notmuchmail.org>; Thu, 30 May 2013 15:31:15 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id 5hFv4qRtbRoU for <notmuch@notmuchmail.org>;\r
+ Thu, 30 May 2013 15:31:09 -0700 (PDT)\r
+Received: from dmz-mailsec-scanner-4.mit.edu (dmz-mailsec-scanner-4.mit.edu\r
+ [18.9.25.15])\r
+ by olra.theworths.org (Postfix) with ESMTP id 3CE8A431FD2\r
+ for <notmuch@notmuchmail.org>; Thu, 30 May 2013 15:31:09 -0700 (PDT)\r
+X-AuditID: 1209190f-b7f256d000005616-b2-51a7d327d69c\r
+Received: from mailhub-auth-2.mit.edu ( [18.7.62.36])\r
+ by dmz-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP\r
+ id 04.FA.22038.723D7A15; Thu, 30 May 2013 18:31:03 -0400 (EDT)\r
+Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
+ by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id r4UMV2Ck019853; \r
+ Thu, 30 May 2013 18:31:03 -0400\r
+Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
+ (authenticated bits=0)\r
+ (User authenticated as amdragon@ATHENA.MIT.EDU)\r
+ by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id r4UMUxNt012623\r
+ (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
+ Thu, 30 May 2013 18:31:01 -0400\r
+Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
+ (envelope-from <amdragon@mit.edu>)\r
+ id 1UiBN5-0005tg-O0; Thu, 30 May 2013 18:30:59 -0400\r
+From: Austin Clements <aclements@csail.mit.edu>\r
+To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v2 2/3] emacs: show: move the insertion of the header\r
+ button to the top level\r
+In-Reply-To: <1369555061-21361-3-git-send-email-markwalters1009@gmail.com>\r
+References: <1369555061-21361-1-git-send-email-markwalters1009@gmail.com>\r
+ <1369555061-21361-3-git-send-email-markwalters1009@gmail.com>\r
+User-Agent: Notmuch/0.15.2+83~g8bee3c4 (http://notmuchmail.org) Emacs/23.4.1\r
+ (i486-pc-linux-gnu)\r
+Date: Thu, 30 May 2013 18:30:59 -0400\r
+Message-ID: <87a9nc2j8c.fsf@awakening.csail.mit.edu>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-Brightmail-Tracker:\r
+ H4sIAAAAAAAAA+NgFnrNIsWRmVeSWpSXmKPExsUixG6noqt+eXmgwZrDKhar5/JYXL85k9mB\r
+ yWPnrLvsHs9W3WIOYIrisklJzcksSy3St0vgyji0egN7warUiiM/L7I0MD4P6mLk5JAQMJE4\r
+ MHcxM4QtJnHh3nq2LkYuDiGBfYwSL35+YYJwNjJKLGu6zwrhnGaSOHLsCDtIi5DAEkaJ1dfS\r
+ QGw2AX2JFWsnsYLYIgKuEk+/fQYbKyyQKPFxyV+wek4BL4lPx6+wQPS2M0q873UAsUUFEiRW\r
+ 3j3BCGKzCKhK7H8wDWgOBwcv0Hkb11mDhHkFBCVOznwC1sosoCVx499LpgmMArOQpGYhSS1g\r
+ ZFrFKJuSW6Wbm5iZU5yarFucnJiXl1qka6KXm1mil5pSuokRFI6ckvw7GL8dVDrEKMDBqMTD\r
+ m5G0PFCINbGsuDL3EKMkB5OSKO/PU0AhvqT8lMqMxOKM+KLSnNTiQ4wSHMxKIryd54ByvCmJ\r
+ lVWpRfkwKWkOFiVx3qspN/2FBNITS1KzU1MLUotgsjIcHEoSvPMuAjUKFqWmp1akZeaUIKSZ\r
+ ODhBhvMADT8DUsNbXJCYW5yZDpE/xajLsfn85HeMQix5+XmpUuK8h0CKBECKMkrz4ObA0sgr\r
+ RnGgt4R5l4BU8QBTENykV0BLmICWPLEGW1KSiJCSamBUv+hfonnivN/LrKjqlYZbMup/n7gS\r
+ +WzLRj1vlvUi+u75q244F8TlSYbPrEk1t1tiz2a1R81/9lz7k1ODV3/4uUTpywHvuin6tToW\r
+ ezNe28krz1Hg+anLO+PN9EiG5pYF9ziiDrFYZ6ybnqghozu1Q+rA9D2qB1a9+H2aI/E5/7vc\r
+ l+Zzr+9QYinOSDTUYi4qTgQAYrJMrf4CAAA=\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Thu, 30 May 2013 22:31:15 -0000\r
+\r
+On Sun, 26 May 2013, Mark Walters <markwalters1009@gmail.com> wrote:\r
+> Previously each of the part insertion handlers inserted the part\r
+> button themselves. Move this up into\r
+> notmuch-show-insert-bodypart. Since a small number of the handlers\r
+> modify the button (the encryption/signature ones) we need to pass the\r
+> header button as an argument into the individual part insertion\r
+> handlers.\r
+>\r
+> The patch is large but mostly simple. The only things of note are that\r
+> we let the text/plain handler insert part buttons itself (as it does\r
+> not always insert one and it applies motmuch-wash to the whole part\r
+> including the button. Also this is by far the most important case so\r
+> this reduces the risk of annoying side effects.\r
+> ---\r
+> emacs/notmuch-show.el | 87 +++++++++++++++++++++++-------------------------\r
+> 1 files changed, 42 insertions(+), 45 deletions(-)\r
+>\r
+> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
+> index 51bda31..591ad56 100644\r
+> --- a/emacs/notmuch-show.el\r
+> +++ b/emacs/notmuch-show.el\r
+> @@ -576,8 +576,7 @@ message at DEPTH in the current thread."\r
+> (mapcar (lambda (inner-part) (plist-get inner-part :content-type))\r
+> (plist-get part :content)))\r
+> \r
+> -(defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type)\r
+> - (notmuch-show-insert-part-header nth declared-type content-type nil)\r
+> +(defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type button)\r
+> (let ((chosen-type (car (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part))))\r
+> (inner-parts (plist-get part :content))\r
+> (start (point)))\r
+> @@ -654,8 +653,7 @@ message at DEPTH in the current thread."\r
+> content-type)\r
+> nil)))\r
+> \r
+> -(defun notmuch-show-insert-part-multipart/related (msg part content-type nth depth declared-type)\r
+> - (notmuch-show-insert-part-header nth declared-type content-type nil)\r
+> +(defun notmuch-show-insert-part-multipart/related (msg part content-type nth depth declared-type button)\r
+> (let ((inner-parts (plist-get part :content))\r
+> (start (point)))\r
+> \r
+> @@ -674,16 +672,15 @@ message at DEPTH in the current thread."\r
+> (indent-rigidly start (point) 1)))\r
+> t)\r
+> \r
+> -(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type)\r
+> - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))\r
+> - (button-put button 'face 'notmuch-crypto-part-header)\r
+> - ;; add signature status button if sigstatus provided\r
+> - (if (plist-member part :sigstatus)\r
+> - (let* ((from (notmuch-show-get-header :From msg))\r
+> - (sigstatus (car (plist-get part :sigstatus))))\r
+> - (notmuch-crypto-insert-sigstatus-button sigstatus from))\r
+> - ;; if we're not adding sigstatus, tell the user how they can get it\r
+> - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")))\r
+> +(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type button)\r
+> + (button-put button 'face 'notmuch-crypto-part-header)\r
+> + ;; add signature status button if sigstatus provided\r
+> + (if (plist-member part :sigstatus)\r
+> + (let* ((from (notmuch-show-get-header :From msg))\r
+> + (sigstatus (car (plist-get part :sigstatus))))\r
+> + (notmuch-crypto-insert-sigstatus-button sigstatus from))\r
+> + ;; if we're not adding sigstatus, tell the user how they can get it\r
+> + (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))\r
+> \r
+> (let ((inner-parts (plist-get part :content))\r
+> (start (point)))\r
+> @@ -696,20 +693,19 @@ message at DEPTH in the current thread."\r
+> (indent-rigidly start (point) 1)))\r
+> t)\r
+> \r
+> -(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type)\r
+> - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))\r
+> - (button-put button 'face 'notmuch-crypto-part-header)\r
+> - ;; add encryption status button if encstatus specified\r
+> - (if (plist-member part :encstatus)\r
+> - (let ((encstatus (car (plist-get part :encstatus))))\r
+> - (notmuch-crypto-insert-encstatus-button encstatus)\r
+> - ;; add signature status button if sigstatus specified\r
+> - (if (plist-member part :sigstatus)\r
+> - (let* ((from (notmuch-show-get-header :From msg))\r
+> - (sigstatus (car (plist-get part :sigstatus))))\r
+> - (notmuch-crypto-insert-sigstatus-button sigstatus from))))\r
+> - ;; if we're not adding encstatus, tell the user how they can get it\r
+> - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")))\r
+> +(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type button)\r
+> + (button-put button 'face 'notmuch-crypto-part-header)\r
+> + ;; add encryption status button if encstatus specified\r
+> + (if (plist-member part :encstatus)\r
+> + (let ((encstatus (car (plist-get part :encstatus))))\r
+> + (notmuch-crypto-insert-encstatus-button encstatus)\r
+> + ;; add signature status button if sigstatus specified\r
+> + (if (plist-member part :sigstatus)\r
+> + (let* ((from (notmuch-show-get-header :From msg))\r
+> + (sigstatus (car (plist-get part :sigstatus))))\r
+> + (notmuch-crypto-insert-sigstatus-button sigstatus from))))\r
+> + ;; if we're not adding encstatus, tell the user how they can get it\r
+> + (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))\r
+> \r
+> (let ((inner-parts (plist-get part :content))\r
+> (start (point)))\r
+> @@ -722,8 +718,7 @@ message at DEPTH in the current thread."\r
+> (indent-rigidly start (point) 1)))\r
+> t)\r
+> \r
+> -(defun notmuch-show-insert-part-multipart/* (msg part content-type nth depth declared-type)\r
+> - (notmuch-show-insert-part-header nth declared-type content-type nil)\r
+> +(defun notmuch-show-insert-part-multipart/* (msg part content-type nth depth declared-type button)\r
+> (let ((inner-parts (plist-get part :content))\r
+> (start (point)))\r
+> ;; Show all of the parts.\r
+> @@ -735,8 +730,7 @@ message at DEPTH in the current thread."\r
+> (indent-rigidly start (point) 1)))\r
+> t)\r
+> \r
+> -(defun notmuch-show-insert-part-message/rfc822 (msg part content-type nth depth declared-type)\r
+> - (notmuch-show-insert-part-header nth declared-type content-type nil)\r
+> +(defun notmuch-show-insert-part-message/rfc822 (msg part content-type nth depth declared-type button)\r
+> (let* ((message (car (plist-get part :content)))\r
+> (body (car (plist-get message :body)))\r
+> (start (point)))\r
+> @@ -757,7 +751,7 @@ message at DEPTH in the current thread."\r
+> (indent-rigidly start (point) 1)))\r
+> t)\r
+> \r
+> -(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type)\r
+> +(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type button)\r
+> (let ((start (point)))\r
+> ;; If this text/plain part is not the first part in the message,\r
+> ;; insert a header to make this clear.\r
+\r
+I know you're trying to minimize side-effects, but I think this will\r
+also complicate maintenance. I'd rather see this call to\r
+notmuch-show-insert-part-header removed and have the only call and all\r
+special-case logic be in notmuch-show-insert-bodypart. As it is,\r
+reasoning about exactly when a part button is inserted requires\r
+understanding the conjunction of two widely separated parts of the code.\r
+\r
+> @@ -770,8 +764,7 @@ message at DEPTH in the current thread."\r
+> (run-hook-with-args 'notmuch-show-insert-text/plain-hook msg depth))))\r
+> t)\r
+> \r
+> -(defun notmuch-show-insert-part-text/calendar (msg part content-type nth depth declared-type)\r
+> - (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))\r
+> +(defun notmuch-show-insert-part-text/calendar (msg part content-type nth depth declared-type button)\r
+> (insert (with-temp-buffer\r
+> (insert (notmuch-get-bodypart-content msg part nth notmuch-show-process-crypto))\r
+> ;; notmuch-get-bodypart-content provides "raw", non-converted\r
+> @@ -794,8 +787,8 @@ message at DEPTH in the current thread."\r
+> t)\r
+> \r
+> ;; For backwards compatibility.\r
+> -(defun notmuch-show-insert-part-text/x-vcalendar (msg part content-type nth depth declared-type)\r
+> - (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type))\r
+> +(defun notmuch-show-insert-part-text/x-vcalendar (msg part content-type nth depth declared-type button)\r
+> + (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type button))\r
+> \r
+> (defun notmuch-show-get-mime-type-of-application/octet-stream (part)\r
+> ;; If we can deduce a MIME type from the filename of the attachment,\r
+> @@ -813,7 +806,7 @@ message at DEPTH in the current thread."\r
+> nil))\r
+> nil))))\r
+> \r
+> -(defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type)\r
+> +(defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type button)\r
+> ;; text/html handler to work around bugs in renderers and our\r
+> ;; invisibile parts code. In particular w3m sets up a keymap which\r
+> ;; "leaks" outside the invisible region and causes strange effects\r
+> @@ -821,11 +814,10 @@ message at DEPTH in the current thread."\r
+> ;; tell w3m not to set a keymap (so the normal notmuch-show-mode-map\r
+> ;; remains).\r
+> (let ((mm-inline-text-html-with-w3m-keymap nil))\r
+> - (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type)))\r
+> + (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type button)))\r
+> \r
+> -(defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type)\r
+> +(defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type button)\r
+> ;; This handler _must_ succeed - it is the handler of last resort.\r
+> - (notmuch-show-insert-part-header nth content-type declared-type (plist-get part :filename))\r
+> (notmuch-mm-display-part-inline msg part nth content-type notmuch-show-process-crypto)\r
+> t)\r
+> \r
+> @@ -848,13 +840,13 @@ message at DEPTH in the current thread."\r
+> \r
+> ;; \f\r
+\r
+> \r
+> -(defun notmuch-show-insert-bodypart-internal (msg part content-type nth depth declared-type)\r
+> +(defun notmuch-show-insert-bodypart-internal (msg part content-type nth depth declared-type button)\r
+> (let ((handlers (notmuch-show-handlers-for content-type)))\r
+> ;; Run the content handlers until one of them returns a non-nil\r
+> ;; value.\r
+> (while (and handlers\r
+> (not (condition-case err\r
+> - (funcall (car handlers) msg part content-type nth depth declared-type)\r
+> + (funcall (car handlers) msg part content-type nth depth declared-type button)\r
+> (error (progn\r
+> (insert "!!! Bodypart insert error: ")\r
+> (insert (error-message-string err))\r
+> @@ -882,6 +874,9 @@ message at DEPTH in the current thread."\r
+> "Insert the body part PART at depth DEPTH in the current thread.\r
+> \r
+> If HIDE is non-nil then initially hide this part."\r
+> +\r
+> + ;; We handle text/plain specially as its code does things before\r
+> + ;; inserting a part button (and does not always insert a part button).\r
+\r
+I think this comment would be clearer right above the button binding (or\r
+maybe the separate diff hunks just make it more confusing than it is).\r
+\r
+> (let* ((content-type (downcase (plist-get part :content-type)))\r
+> (mime-type (or (and (string= content-type "application/octet-stream")\r
+> (notmuch-show-get-mime-type-of-application/octet-stream part))\r
+> @@ -889,9 +884,11 @@ If HIDE is non-nil then initially hide this part."\r
+> "text/x-diff")\r
+> content-type))\r
+> (nth (plist-get part :id))\r
+> - (beg (point)))\r
+> + (beg (point))\r
+> + (button (unless (string= mime-type "text/plain")\r
+> + (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))))\r
+\r
+If you take my suggestion above, this would become something like\r
+\r
+(unless (and (string= mime-type "text/plain) (<= nth 1)) ...)\r
+\r
+or maybe clearer\r
+\r
+(when (or (not (string= mime-type "text/plain")) (> nth 1)) ...)\r
+\r
+> \r
+> - (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type)\r
+> + (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button)\r
+> ;; Some of the body part handlers leave point somewhere up in the\r
+> ;; part, so we make sure that we're down at the end.\r
+> (goto-char (point-max))\r
+> -- \r
+> 1.7.9.1\r
+>\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r
projects / notmuch-archives.git / commitdiff