[PATCH 1/3] emacs: Tidy `notmuch-show-insert-part-header'.

[PATCH 1/3] emacs: Tidy `notmuch-show-insert-part-header'.

[David Edmondson]

---
 emacs/notmuch-show.el |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 03c1f6b..f62f8ac 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -286,20 +286,18 @@ message at DEPTH in the current thread."
   'face 'message-mml)
 
 (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
-  (let ((button))
-    (setq button
-	  (insert-button
-	   (concat "[ "
-		   (if name (concat name ": ") "")
-		   declared-type
-		   (if (not (string-equal declared-type content-type))
-		       (concat " (as " content-type ")")
-		     "")
-		   (or comment "")
-		   " ]")
-	   :type 'notmuch-show-part-button-type
-	   :notmuch-part nth
-	   :notmuch-filename name))
+  (let ((button (insert-button
+		 (concat "[ "
+			 (if name (concat name ": ") "")
+			 declared-type
+			 (if (not (string-equal declared-type content-type))
+			     (concat " (as " content-type ")")
+			   "")
+			 (or comment "")
+			 " ]")
+		 :type 'notmuch-show-part-button-type
+		 :notmuch-part nth
+		 :notmuch-filename name)))
     (insert "\n")
     ;; return button
     button))
-- 
1.7.8.3

[PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

Instead, allow the caller to specify some parameters for the
button. Rework `notmuch-show-insert-part-multipart/signed' and
`notmuch-show-insert-part-multipart/encrypted' accordingly, moving
most of the code into a common
`notmuch-show-insert-part-multipart/signed-or-encrypted' to reduce
duplication.
---

The buttons inserted for encrypted parts are slightly different now -
previously the logic was that if a part was encrypted it would have
the signature status inserted only if the encryption status was
specified. Now the signature status will be inserted even without
encryption status. My reading of the documentation says that this is
correct, but I'm no expert. Comments?

 emacs/notmuch-show.el |   88 +++++++++++++++++++++---------------------------
 1 files changed, 39 insertions(+), 49 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f62f8ac..97e2a15 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -285,22 +285,23 @@ message at DEPTH in the current thread."
   'follow-link t
   'face 'message-mml)
 
-(defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
-  (let ((button (insert-button
-		 (concat "[ "
-			 (if name (concat name ": ") "")
-			 declared-type
-			 (if (not (string-equal declared-type content-type))
-			     (concat " (as " content-type ")")
-			   "")
-			 (or comment "")
-			 " ]")
-		 :type 'notmuch-show-part-button-type
-		 :notmuch-part nth
-		 :notmuch-filename name)))
-    (insert "\n")
-    ;; return button
-    button))
+(defun notmuch-show-insert-part-header (nth content-type declared-type
+					    &optional name comment
+					    &rest button-parameters)
+  (apply #'insert-button
+	 (concat "[ "
+		 (if name (concat name ": ") "")
+		 declared-type
+		 (if (not (string-equal declared-type content-type))
+		     (concat " (as " content-type ")")
+		   "")
+		 (or comment "")
+		 " ]")
+	 :type 'notmuch-show-part-button-type
+	 :notmuch-part nth
+	 :notmuch-filename name
+	 button-parameters)
+  (insert "\n"))
 
 ;; Functions handling particular MIME parts.
 
@@ -458,42 +459,31 @@ current buffer, if possible."
   t)
 
 (defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add signature status button if sigstatus provided
-    (if (plist-member part :sigstatus)
-	(let* ((from (notmuch-show-get-header :From msg))
-	       (sigstatus (car (plist-get part :sigstatus))))
-	  (notmuch-crypto-insert-sigstatus-button sigstatus from))
-      ;; if we're not adding sigstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")))
-
-  (let ((inner-parts (plist-get part :content))
-	(start (point)))
-    ;; Show all of the parts.
-    (mapc (lambda (inner-part)
-	    (notmuch-show-insert-bodypart msg inner-part depth))
-	  inner-parts)
-
-    (when notmuch-show-indent-multipart
-      (indent-rigidly start (point) 1)))
-  t)
+  (notmuch-show-insert-part-multipart/signed-or-encrypted msg part content-type nth depth declared-type
+							 (plist-get part :sigstatus)
+							 nil))
 
 (defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add encryption status button if encstatus specified
-    (if (plist-member part :encstatus)
-	(let ((encstatus (car (plist-get part :encstatus))))
-	  (notmuch-crypto-insert-encstatus-button encstatus)
-	  ;; add signature status button if sigstatus specified
-	  (if (plist-member part :sigstatus)
-	      (let* ((from (notmuch-show-get-header :From msg))
-		     (sigstatus (car (plist-get part :sigstatus))))
-		(notmuch-crypto-insert-sigstatus-button sigstatus from))))
-      ;; if we're not adding encstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")))
+  (notmuch-show-insert-part-multipart/signed-or-encrypted msg part content-type nth depth declared-type
+							 (plist-get part :sigstatus)
+							 (plist-get part :encstatus)))
 
+(defun notmuch-show-insert-part-multipart/signed-or-encrypted (msg part content-type nth depth declared-type sigstatus-tuple encstatus-tuple)
+  (if (or encstatus-tuple sigstatus-tuple)
+      (progn
+	(notmuch-show-insert-part-header nth declared-type content-type
+					 nil nil 'face 'notmuch-crypto-part-header)
+	(if encstatus-tuple
+	    (notmuch-crypto-insert-encstatus-button (car encstatus-tuple)))
+	(if sigstatus-tuple
+	    (notmuch-crypto-insert-sigstatus-button (car sigstatus-tuple)
+						    (notmuch-show-get-header :From msg))))
+    ;; If we're not adding status buttons, tell the user how they can
+    ;; enable them.
+    (notmuch-show-insert-part-header nth declared-type content-type
+				     nil nil 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))
+
+  ;; Insert the enclosed parts.
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
     ;; Show all of the parts.
-- 
1.7.8.3

Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Fri, 20 Jan 2012 09:43:31 +0000, David Edmondson <dme@dme.org> wrote:
> The buttons inserted for encrypted parts are slightly different now -
> previously the logic was that if a part was encrypted it would have
> the signature status inserted only if the encryption status was
> specified. Now the signature status will be inserted even without
> encryption status. My reading of the documentation says that this is
> correct, but I'm no expert. Comments?

I need some more time to review this patch, but I don't think this is
correct.  The signature status of encrypted messages (or even if the
message was signed at all) comes from the decryption process.  They're
not independent.  If the decryption fails it's unknown if there was a
signature or not.

I need to look closer at this, though.  Hopefully this weekend.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On Fri, 20 Jan 2012 09:43:31 +0000, David Edmondson <dme@dme.org> wrote:
> Instead, allow the caller to specify some parameters for the
> button. Rework `notmuch-show-insert-part-multipart/signed' and
> `notmuch-show-insert-part-multipart/encrypted' accordingly, moving
> most of the code into a common
> `notmuch-show-insert-part-multipart/signed-or-encrypted' to reduce
> duplication.

Hi, David.  A couple of issues with this patch:

This patch seems to include multiple distinct changes.  There is a
change to notmuch-show-insert-part-header, but a seemingly unrelated
change to the insertion of signed/encrypted part buttons.  They should
be in separate patches.

I'm also not sure I understand why the proposed changes to the
signed/encrypted button insertion functions are necessary or desired.
Was there a problem with the logic as it was?  What is gained by having
one function filled with special casing to handle two things, rather
than having two distinct functions?

Finally, this patch throws out all the changes from the previous patch,
making the previous patch superfluous.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On Sun, 22 Jan 2012 13:38:09 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> This patch seems to include multiple distinct changes.  There is a
> change to notmuch-show-insert-part-header, but a seemingly unrelated
> change to the insertion of signed/encrypted part buttons.  They should
> be in separate patches.

I can separate them.

> I'm also not sure I understand why the proposed changes to the
> signed/encrypted button insertion functions are necessary or desired.
> Was there a problem with the logic as it was?  What is gained by
> having one function filled with special casing to handle two things,
> rather than having two distinct functions?

There was no problem with the logic. The code in the two functions was
almost identical, so I'd like to make any future changes in just one
place.

You didn't actually answer my question - is the logic in the new
function correct?

> Finally, this patch throws out all the changes from the previous patch,
> making the previous patch superfluous.

I'll merge the first patch into the later (and presumably get accused of
submitting patches which include multiple distinct changes :-)).

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

On Mon, 23 Jan 2012 08:16:03 +0000, David Edmondson <dme@dme.org> wrote:
> There was no problem with the logic. The code in the two functions was
> almost identical, so I'd like to make any future changes in just one
> place.
>
> You didn't actually answer my question - is the logic in the new
> function correct?

Honestly I didn't look too closely yet since I'm not convinced we need
the change at all.  I would prefer to keep the functions separate.  In
my opinion, enough special casing would be required that it wouldn't be
worth it, and it would make the code less clear.

> I'll merge the first patch into the later (and presumably get accused of
> submitting patches which include multiple distinct changes :-)).

But if you're removing all the code anyway, it's not a distinct change.
It's still just a replacement.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

On Mon, 23 Jan 2012 00:52:26 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Mon, 23 Jan 2012 08:16:03 +0000, David Edmondson <dme@dme.org> wrote:
> > There was no problem with the logic. The code in the two functions was
> > almost identical, so I'd like to make any future changes in just one
> > place.
> >
> > You didn't actually answer my question - is the logic in the new
> > function correct?
> 
> Honestly I didn't look too closely yet since I'm not convinced we need
> the change at all.  I would prefer to keep the functions separate.  In
> my opinion, enough special casing would be required that it wouldn't be
> worth it, and it would make the code less clear.

Okay.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

Instead, allow the caller to specify some parameters for the
button. Rework `notmuch-show-insert-part-multipart/signed' and
`notmuch-show-insert-part-multipart/encrypted' accordingly.
---

Removed the merge of multipart/signed and multipart/encrypted.

 emacs/notmuch-show.el |   84 +++++++++++++++++++++++++-----------------------
 1 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 03c1f6b..9144484 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -285,24 +285,23 @@ message at DEPTH in the current thread."
   'follow-link t
   'face 'message-mml)
 
-(defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment)
-  (let ((button))
-    (setq button
-	  (insert-button
-	   (concat "[ "
-		   (if name (concat name ": ") "")
-		   declared-type
-		   (if (not (string-equal declared-type content-type))
-		       (concat " (as " content-type ")")
-		     "")
-		   (or comment "")
-		   " ]")
-	   :type 'notmuch-show-part-button-type
-	   :notmuch-part nth
-	   :notmuch-filename name))
-    (insert "\n")
-    ;; return button
-    button))
+(defun notmuch-show-insert-part-header (nth content-type declared-type
+					    &optional name comment
+					    &rest button-parameters)
+  (apply #'insert-button
+	 (concat "[ "
+		 (if name (concat name ": ") "")
+		 declared-type
+		 (if (not (string-equal declared-type content-type))
+		     (concat " (as " content-type ")")
+		   "")
+		 (or comment "")
+		 " ]")
+	 :type 'notmuch-show-part-button-type
+	 :notmuch-part nth
+	 :notmuch-filename name
+	 button-parameters)
+  (insert "\n"))
 
 ;; Functions handling particular MIME parts.
 
@@ -460,15 +459,18 @@ current buffer, if possible."
   t)
 
 (defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add signature status button if sigstatus provided
-    (if (plist-member part :sigstatus)
-	(let* ((from (notmuch-show-get-header :From msg))
-	       (sigstatus (car (plist-get part :sigstatus))))
-	  (notmuch-crypto-insert-sigstatus-button sigstatus from))
-      ;; if we're not adding sigstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")))
+  ;; Add signature status button if sigstatus provided.
+  (if (plist-member part :sigstatus)
+      (let ((from (notmuch-show-get-header :From msg))
+	    (sigstatus (car (plist-get part :sigstatus))))
+	(notmuch-show-insert-part-header nth declared-type content-type nil nil 
+					 'face 'notmuch-crypto-part-header)
+	(notmuch-crypto-insert-sigstatus-button sigstatus from))
+
+    ;; If we're not adding sigstatus, tell the user how to enable it.
+    (notmuch-show-insert-part-header nth declared-type content-type nil nil 
+				     'face 'notmuch-crypto-part-header
+				     'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
@@ -482,19 +484,21 @@ current buffer, if possible."
   t)
 
 (defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add encryption status button if encstatus specified
-    (if (plist-member part :encstatus)
-	(let ((encstatus (car (plist-get part :encstatus))))
-	  (notmuch-crypto-insert-encstatus-button encstatus)
-	  ;; add signature status button if sigstatus specified
-	  (if (plist-member part :sigstatus)
-	      (let* ((from (notmuch-show-get-header :From msg))
-		     (sigstatus (car (plist-get part :sigstatus))))
-		(notmuch-crypto-insert-sigstatus-button sigstatus from))))
-      ;; if we're not adding encstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")))
+  ;; Add encryption status button if encstatus provided.
+  (if (plist-member part :encstatus)
+      (let ((encstatus (car (plist-get part :encstatus))))
+	(notmuch-show-insert-part-header nth declared-type content-type nil nil 
+					 'face 'notmuch-crypto-part-header)
+	(notmuch-crypto-insert-encstatus-button encstatus)
+	;; add signature status button if sigstatus specified
+	(if (plist-member part :sigstatus)
+	    (let* ((from (notmuch-show-get-header :From msg))
+		   (sigstatus (car (plist-get part :sigstatus))))
+	      (notmuch-crypto-insert-sigstatus-button sigstatus from))))
+    ;; If we're not adding encstatus, tell the user how to enable it.
+    (notmuch-show-insert-part-header nth declared-type content-type nil nil 
+				     'face 'notmuch-crypto-part-header
+				     'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
-- 
1.7.8.3

[PATCH 2/3 v2] emacs: Optionally hide some part headers.

[David Edmondson]

Add a regexp, `notmuch-show-part-headers-hidden' and if the
content-type of a part matches, don't show the part header.
---
 emacs/notmuch-show.el |   41 +++++++++++++++++++++++++++--------------
 1 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 9144484..39f35ed 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -110,6 +110,12 @@ indentation."
   :group 'notmuch
   :type 'boolean)
 
+(defcustom notmuch-show-part-headers-hidden nil
+  "Headers for parts whose content-type matches this regexp will
+not be shown."
+  :group 'notmuch
+  :type 'regexp)
+
 (defmacro with-current-notmuch-show-message (&rest body)
   "Evaluate body with current buffer set to the text of current message"
   `(save-excursion
@@ -285,23 +291,30 @@ message at DEPTH in the current thread."
   'follow-link t
   'face 'message-mml)
 
+(defun notmuch-show-hidden-part-header (content-type)
+  "Return non-nil if a part header should be hidden for
+CONTENT-TYPE parts."
+  (and notmuch-show-part-headers-hidden
+       (string-match notmuch-show-part-headers-hidden content-type)))
+
 (defun notmuch-show-insert-part-header (nth content-type declared-type
 					    &optional name comment
 					    &rest button-parameters)
-  (apply #'insert-button
-	 (concat "[ "
-		 (if name (concat name ": ") "")
-		 declared-type
-		 (if (not (string-equal declared-type content-type))
-		     (concat " (as " content-type ")")
-		   "")
-		 (or comment "")
-		 " ]")
-	 :type 'notmuch-show-part-button-type
-	 :notmuch-part nth
-	 :notmuch-filename name
-	 button-parameters)
-  (insert "\n"))
+  (unless (notmuch-show-hidden-part-header content-type)
+    (apply #'insert-button
+	   (concat "[ "
+		   (if name (concat name ": ") "")
+		   declared-type
+		   (if (not (string-equal declared-type content-type))
+		       (concat " (as " content-type ")")
+		     "")
+		   (or comment "")
+		   " ]")
+	   :type 'notmuch-show-part-button-type
+	   :notmuch-part nth
+	   :notmuch-filename name
+	   button-parameters)
+    (insert "\n")))
 
 ;; Functions handling particular MIME parts.
 
-- 
1.7.8.3

[PATCH 3/3 v2] emacs: Don't insert a part header if it's the first part and text/*.

[David Edmondson]

Previously this logic applied only to text/plain. Allow it for other
text/* parts as well.
---

This was not included in version 1 of the patch set.

 emacs/notmuch-show.el |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 39f35ed..55e4e34 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -300,7 +300,9 @@ CONTENT-TYPE parts."
 (defun notmuch-show-insert-part-header (nth content-type declared-type
 					    &optional name comment
 					    &rest button-parameters)
-  (unless (notmuch-show-hidden-part-header content-type)
+  (unless (or (notmuch-show-hidden-part-header content-type)
+	      (and (= nth 1)
+		   (string-match "text/*" content-type)))
     (apply #'insert-button
 	   (concat "[ "
 		   (if name (concat name ": ") "")
@@ -561,10 +563,7 @@ current buffer, if possible."
 
 (defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type)
   (let ((start (point)))
-    ;; If this text/plain part is not the first part in the message,
-    ;; insert a header to make this clear.
-    (if (> nth 1)
-	(notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)))
+    (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))
     (insert (notmuch-show-get-bodypart-content msg part nth))
     (save-excursion
       (save-restriction
-- 
1.7.8.3

Re: [PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 181 bytes --]

v2 of these patches need to be merged with Mark's part saving
stuff. I've done this, but I'll hold off posting the merged version
until this one is reviewed, unless anyone objects.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 3/3] emacs: Optionally hide some part headers.

[David Edmondson]

Add a regexp, `notmuch-show-part-headers-hidden' and if the
content-type of a part matches, don't show the part header.
---
 emacs/notmuch-show.el |   41 +++++++++++++++++++++++++++--------------
 1 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 97e2a15..fa826f7 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -110,6 +110,12 @@ indentation."
   :group 'notmuch
   :type 'boolean)
 
+(defcustom notmuch-show-part-headers-hidden nil
+  "Headers for parts whose content-type matches this regexp will
+not be shown."
+  :group 'notmuch
+  :type 'regexp)
+
 (defmacro with-current-notmuch-show-message (&rest body)
   "Evaluate body with current buffer set to the text of current message"
   `(save-excursion
@@ -285,23 +291,30 @@ message at DEPTH in the current thread."
   'follow-link t
   'face 'message-mml)
 
+(defun notmuch-show-hidden-part-header (content-type)
+  "Return non-nil if a part header should be hidden for
+CONTENT-TYPE parts."
+  (and notmuch-show-part-headers-hidden
+       (string-match notmuch-show-part-headers-hidden content-type)))
+
 (defun notmuch-show-insert-part-header (nth content-type declared-type
 					    &optional name comment
 					    &rest button-parameters)
-  (apply #'insert-button
-	 (concat "[ "
-		 (if name (concat name ": ") "")
-		 declared-type
-		 (if (not (string-equal declared-type content-type))
-		     (concat " (as " content-type ")")
-		   "")
-		 (or comment "")
-		 " ]")
-	 :type 'notmuch-show-part-button-type
-	 :notmuch-part nth
-	 :notmuch-filename name
-	 button-parameters)
-  (insert "\n"))
+  (unless (notmuch-show-hidden-part-header content-type)
+    (apply #'insert-button
+	   (concat "[ "
+		   (if name (concat name ": ") "")
+		   declared-type
+		   (if (not (string-equal declared-type content-type))
+		       (concat " (as " content-type ")")
+		     "")
+		   (or comment "")
+		   " ]")
+	   :type 'notmuch-show-part-button-type
+	   :notmuch-part nth
+	   :notmuch-filename name
+	   button-parameters)
+    (insert "\n")))
 
 ;; Functions handling particular MIME parts.
 
-- 
1.7.8.3

optional hiding of some part headers v3

[David Edmondson]

Rebased on top of the recent button related changes from Mark.

[PATCH 1/3] emacs: Don't return the button from
[PATCH 2/3] emacs: Optionally hide some part headers.
[PATCH 3/3] emacs: Don't insert a part header if it's the first part

[PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

Instead, allow the caller to specify some parameters for the
button. Rework `notmuch-show-insert-part-multipart/signed' and
`notmuch-show-insert-part-multipart/encrypted' accordingly.
---
 emacs/notmuch-show.el |   86 +++++++++++++++++++++++++-----------------------
 1 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e6a5b31..d0e0d38 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -355,25 +355,24 @@ message at DEPTH in the current thread."
   "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
-	  (insert-button
-	   (concat "[ "
-		   (if name (concat name ": ") "")
-		   declared-type
-		   (if (not (string-equal declared-type content-type))
-		       (concat " (as " content-type ")")
-		     "")
-		   (or comment "")
-		   " ]")
-	   :type 'notmuch-show-part-button-type
-	   :notmuch-part nth
-	   :notmuch-filename name
-	   :notmuch-content-type content-type))
-    (insert "\n")
-    ;; return button
-    button))
+(defun notmuch-show-insert-part-header (nth content-type declared-type
+					    &optional name comment
+					    &rest button-parameters)
+  (apply #'insert-button
+	 (concat "[ "
+		 (if name (concat name ": ") "")
+		 declared-type
+		 (if (not (string-equal declared-type content-type))
+		     (concat " (as " content-type ")")
+		   "")
+		 (or comment "")
+		 " ]")
+	 :type 'notmuch-show-part-button-type
+	 :notmuch-part nth
+	 :notmuch-filename name
+	 :notmuch-content-type content-type
+	 button-parameters)
+  (insert "\n"))
 
 ;; Functions handling particular MIME parts.
 
@@ -559,15 +558,18 @@ current buffer, if possible."
   t)
 
 (defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add signature status button if sigstatus provided
-    (if (plist-member part :sigstatus)
-	(let* ((from (notmuch-show-get-header :From msg))
-	       (sigstatus (car (plist-get part :sigstatus))))
-	  (notmuch-crypto-insert-sigstatus-button sigstatus from))
-      ;; if we're not adding sigstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")))
+  ;; Add signature status button if sigstatus provided.
+  (if (plist-member part :sigstatus)
+      (let ((from (notmuch-show-get-header :From msg))
+	    (sigstatus (car (plist-get part :sigstatus))))
+	(notmuch-show-insert-part-header nth declared-type content-type nil nil 
+					 'face 'notmuch-crypto-part-header)
+	(notmuch-crypto-insert-sigstatus-button sigstatus from))
+
+    ;; If we're not adding sigstatus, tell the user how to enable it.
+    (notmuch-show-insert-part-header nth declared-type content-type nil nil 
+				     'face 'notmuch-crypto-part-header
+				     'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
@@ -581,19 +583,21 @@ current buffer, if possible."
   t)
 
 (defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add encryption status button if encstatus specified
-    (if (plist-member part :encstatus)
-	(let ((encstatus (car (plist-get part :encstatus))))
-	  (notmuch-crypto-insert-encstatus-button encstatus)
-	  ;; add signature status button if sigstatus specified
-	  (if (plist-member part :sigstatus)
-	      (let* ((from (notmuch-show-get-header :From msg))
-		     (sigstatus (car (plist-get part :sigstatus))))
-		(notmuch-crypto-insert-sigstatus-button sigstatus from))))
-      ;; if we're not adding encstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")))
+  ;; Add encryption status button if encstatus provided.
+  (if (plist-member part :encstatus)
+      (let ((encstatus (car (plist-get part :encstatus))))
+	(notmuch-show-insert-part-header nth declared-type content-type nil nil 
+					 'face 'notmuch-crypto-part-header)
+	(notmuch-crypto-insert-encstatus-button encstatus)
+	;; add signature status button if sigstatus specified
+	(if (plist-member part :sigstatus)
+	    (let* ((from (notmuch-show-get-header :From msg))
+		   (sigstatus (car (plist-get part :sigstatus))))
+	      (notmuch-crypto-insert-sigstatus-button sigstatus from))))
+    ;; If we're not adding encstatus, tell the user how to enable it.
+    (notmuch-show-insert-part-header nth declared-type content-type nil nil 
+				     'face 'notmuch-crypto-part-header
+				     'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
-- 
1.7.8.3

Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

On Tue, 24 Jan 2012 12:53:38 +0000, David Edmondson <dme@dme.org> wrote:
> Instead, allow the caller to specify some parameters for the
> button. Rework `notmuch-show-insert-part-multipart/signed' and
> `notmuch-show-insert-part-multipart/encrypted' accordingly.

Hi, David.  I was thinking about this, and it seems to me that returning
the button itself is useful.  I can imagine in the future that it might
be useful to be able to modify the button after you've created.  Maybe
it's inconvenient to specify all button parameters at creation time.

Is there a reason it's really necessary to make this change?  Can't
callers just ignore the returned button if they don't care about it
further?  I can see that maybe it's nice to be able to specify
parameters at creation time, but I'm not sure why that requires throwing
out the returned object as well.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

On Tue, 24 Jan 2012 10:46:57 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Tue, 24 Jan 2012 12:53:38 +0000, David Edmondson <dme@dme.org> wrote:
> > Instead, allow the caller to specify some parameters for the
> > button. Rework `notmuch-show-insert-part-multipart/signed' and
> > `notmuch-show-insert-part-multipart/encrypted' accordingly.
> 
> Hi, David.  I was thinking about this, and it seems to me that returning
> the button itself is useful.  I can imagine in the future that it might
> be useful to be able to modify the button after you've created.  Maybe
> it's inconvenient to specify all button parameters at creation time.
> 
> Is there a reason it's really necessary to make this change?  Can't
> callers just ignore the returned button if they don't care about it
> further?  I can see that maybe it's nice to be able to specify
> parameters at creation time, but I'm not sure why that requires throwing
> out the returned object as well.

Patches 2 and 3 in that series can result in the button not being
inserted.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Tue, 24 Jan 2012 19:25:19 +0000, David Edmondson <dme@dme.org> wrote:
> On Tue, 24 Jan 2012 10:46:57 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > Is there a reason it's really necessary to make this change?  Can't
> > callers just ignore the returned button if they don't care about it
> > further?  I can see that maybe it's nice to be able to specify
> > parameters at creation time, but I'm not sure why that requires throwing
> > out the returned object as well.
> 
> Patches 2 and 3 in that series can result in the button not being
> inserted.

Can patches 2 and 3 be rewritten so they are compatible with the button
being returned by the button creation function?

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

On Tue, 24 Jan 2012 11:52:09 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Tue, 24 Jan 2012 19:25:19 +0000, David Edmondson <dme@dme.org> wrote:
> > On Tue, 24 Jan 2012 10:46:57 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > > Is there a reason it's really necessary to make this change?  Can't
> > > callers just ignore the returned button if they don't care about it
> > > further?  I can see that maybe it's nice to be able to specify
> > > parameters at creation time, but I'm not sure why that requires throwing
> > > out the returned object as well.
> > 
> > Patches 2 and 3 in that series can result in the button not being
> > inserted.
> 
> Can patches 2 and 3 be rewritten so they are compatible with the button
> being returned by the button creation function?

No. The whole purpose of 2 and 3 is that they don't insert a button in
some circumstances. If no button is inserted then there is no button to
return.

The changes to `notmuch-show-insert-part-multipart/signed' and
`notmuch-show-insert-part-multipart/encrypted' in patch 1 could be
re-written to anticipate that a button might not be returned, but the
resulting code was more complex that that I sent.

> > > I can see that maybe it's nice to be able to specify parameters at
> > > creation time, but I'm not sure why that requires throwing out the
> > > returned object as well.

Do you have a specific use case for this?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[Tomi Ollila]

On Tue, 24 Jan 2012 12:53:38 +0000, David Edmondson <dme@dme.org> wrote:
> Instead, allow the caller to specify some parameters for the
> button. Rework `notmuch-show-insert-part-multipart/signed' and
> `notmuch-show-insert-part-multipart/encrypted' accordingly.
> ---

LGTM.

[PATCH 2/3] emacs: Optionally hide some part headers.

[David Edmondson]

Add a regexp, `notmuch-show-part-headers-hidden' and if the
content-type of a part matches, don't show the part header.
---
 emacs/notmuch-show.el |   43 ++++++++++++++++++++++++++++---------------
 1 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index d0e0d38..a0a2873 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -123,6 +123,12 @@ indentation."
 		 (const :tag "View interactively"
 			notmuch-show-interactively-view-part)))
 
+(defcustom notmuch-show-part-headers-hidden nil
+  "Headers for parts whose content-type matches this regexp will
+not be shown."
+  :group 'notmuch
+  :type 'regexp)
+
 (defmacro with-current-notmuch-show-message (&rest body)
   "Evaluate body with current buffer set to the text of current message"
   `(save-excursion
@@ -355,24 +361,31 @@ message at DEPTH in the current thread."
   "Submap for button commands")
 (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
 
+(defun notmuch-show-hidden-part-header (content-type)
+  "Return non-nil if a part header should be hidden for
+CONTENT-TYPE parts."
+  (and notmuch-show-part-headers-hidden
+       (string-match notmuch-show-part-headers-hidden content-type)))
+
 (defun notmuch-show-insert-part-header (nth content-type declared-type
 					    &optional name comment
 					    &rest button-parameters)
-  (apply #'insert-button
-	 (concat "[ "
-		 (if name (concat name ": ") "")
-		 declared-type
-		 (if (not (string-equal declared-type content-type))
-		     (concat " (as " content-type ")")
-		   "")
-		 (or comment "")
-		 " ]")
-	 :type 'notmuch-show-part-button-type
-	 :notmuch-part nth
-	 :notmuch-filename name
-	 :notmuch-content-type content-type
-	 button-parameters)
-  (insert "\n"))
+  (unless (notmuch-show-hidden-part-header content-type)
+    (apply #'insert-button
+	   (concat "[ "
+		   (if name (concat name ": ") "")
+		   declared-type
+		   (if (not (string-equal declared-type content-type))
+		       (concat " (as " content-type ")")
+		     "")
+		   (or comment "")
+		   " ]")
+	   :type 'notmuch-show-part-button-type
+	   :notmuch-part nth
+	   :notmuch-filename name
+	   :notmuch-content-type content-type
+	   button-parameters)
+    (insert "\n")))
 
 ;; Functions handling particular MIME parts.
 
-- 
1.7.8.3

Re: [PATCH 2/3] emacs: Optionally hide some part headers.

[Tomi Ollila]

On Tue, 24 Jan 2012 12:53:39 +0000, David Edmondson <dme@dme.org> wrote:
> Add a regexp, `notmuch-show-part-headers-hidden' and if the
> content-type of a part matches, don't show the part header.
> ---

LGTM... (I wonder ow easy is it to write regexps in custimization
interface)

[PATCH 3/3] emacs: Don't insert a part header if it's the first part and text/*.

[David Edmondson]

Previously this logic applied only to text/plain. Allow it for other
text/* parts as well.
---
 emacs/notmuch-show.el |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index a0a2873..23bf1f4 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -370,7 +370,9 @@ CONTENT-TYPE parts."
 (defun notmuch-show-insert-part-header (nth content-type declared-type
 					    &optional name comment
 					    &rest button-parameters)
-  (unless (notmuch-show-hidden-part-header content-type)
+  (unless (or (notmuch-show-hidden-part-header content-type)
+	      (and (= nth 1)
+		   (string-match "text/*" content-type)))
     (apply #'insert-button
 	   (concat "[ "
 		   (if name (concat name ": ") "")
@@ -660,10 +662,7 @@ current buffer, if possible."
 
 (defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type)
   (let ((start (point)))
-    ;; If this text/plain part is not the first part in the message,
-    ;; insert a header to make this clear.
-    (if (> nth 1)
-	(notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)))
+    (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))
     (insert (notmuch-show-get-bodypart-content msg part nth))
     (save-excursion
       (save-restriction
-- 
1.7.8.3

Re: [PATCH 3/3] emacs: Don't insert a part header if it's the first part and text/*.

[Tomi Ollila]

On Tue, 24 Jan 2012 12:53:40 +0000, David Edmondson <dme@dme.org> wrote:
> Previously this logic applied only to text/plain. Allow it for other
> text/* parts as well.
> ---

LGTM.

[PATCH v3 0/3] part header code tidying and options

[David Edmondson]

v3:
- simple rebase.

The previous version of this had 'LGTM' for each of the three
parts. jrollins asked some questions, all of which were answered.

David Edmondson (3):
  emacs: Don't return the button from
    `notmuch-show-insert-part-header'.
  emacs: Optionally hide some part headers.
  emacs: Don't insert a part header if it's the first part and text/*.

 emacs/notmuch-show.el |   84 +++++++++++++++++++++++++++++--------------------
 1 files changed, 50 insertions(+), 34 deletions(-)

-- 
1.7.8.3

[PATCH v2] emacs: Add more processing of displayed headers.

[David Edmondson]

Wrap headers to the width of the window and indent continuations.
---
 emacs/notmuch-show.el |   43 ++++++++++++++++++++++++++++++++++++-------
 1 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 7469e2e..a589d37 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -68,9 +68,16 @@ any given message."
   :type 'boolean
   :group 'notmuch-show)
 
-(defvar notmuch-show-markup-headers-hook '(notmuch-show-colour-headers)
+(defcustom notmuch-show-markup-headers-hook '(notmuch-show-colour-headers
+					      notmuch-show-fill-headers
+					      notmuch-show-indent-continuations)
   "A list of functions called to decorate the headers listed in
-`notmuch-message-headers'.")
+`notmuch-message-headers'."
+  :type 'hook
+  :options '(notmuch-show-colour-headers
+	     notmuch-show-fill-headers
+	     notmuch-show-indent-continuations)
+  :group 'notmuch-show)
 
 (defcustom notmuch-show-hook '(notmuch-show-turn-on-visual-line-mode)
   "Functions called after populating a `notmuch-show' buffer."
@@ -269,13 +276,35 @@ operation on the contents of the current buffer."
     (overlay-put (make-overlay (point) (re-search-forward ".*$"))
 		 'face face)))
 
-(defun notmuch-show-colour-headers ()
+(defun notmuch-show-colour-headers (depth)
   "Apply some colouring to the current headers."
   (goto-char (point-min))
   (while (looking-at "^[A-Za-z][-A-Za-z0-9]*:")
     (notmuch-show-fontify-header)
     (forward-line)))
 
+(defun notmuch-show-fill-headers (depth)
+  "Wrap the text of the current headers."
+
+  ;; '-5' to allow for the indentation code.
+  (let ((fill-column (- (window-width) depth 5)))
+    (goto-char (point-min))
+    (while (not (eobp))
+      (let ((start (point)))
+	(end-of-line)
+	;; We're left at the start of the next line, so there's no need
+	;; to move forward after filling.
+	(fill-region-as-paragraph start (point))))))
+
+(defun notmuch-show-indent-continuations (depth)
+  "Indent any continuation lines."
+  (goto-char (point-min))
+  (while (not (eobp))
+    (if (not (looking-at "^[A-Za-z][-A-Za-z0-9]*:"))
+	;; Four spaces tends to work well with 'To' and 'Cc' headers.
+	(insert "    "))
+    (forward-line)))
+
 (defun notmuch-show-spaces-n (n)
   "Return a string comprised of `n' spaces."
   (make-string n ? ))
@@ -366,7 +395,7 @@ message at DEPTH in the current thread."
   "Insert a single header."
   (insert header ": " header-value "\n"))
 
-(defun notmuch-show-insert-headers (headers)
+(defun notmuch-show-insert-headers (headers depth)
   "Insert the headers of the current message."
   (let ((start (point)))
     (mapc (lambda (header)
@@ -379,7 +408,7 @@ message at DEPTH in the current thread."
     (save-excursion
       (save-restriction
 	(narrow-to-region start (point-max))
-	(run-hooks 'notmuch-show-markup-headers-hook)))))
+	(run-hook-with-args 'notmuch-show-markup-headers-hook depth)))))
 
 (define-button-type 'notmuch-show-part-button-type
   'action 'notmuch-show-part-button-default
@@ -671,7 +700,7 @@ current buffer, if possible."
     ;; Override `notmuch-message-headers' to force `From' to be
     ;; displayed.
     (let ((notmuch-message-headers '("From" "Subject" "To" "Cc" "Date")))
-      (notmuch-show-insert-headers (plist-get message :headers)))
+      (notmuch-show-insert-headers (plist-get message :headers) 0))
 
     ;; Blank line after headers to be compatible with the normal
     ;; message display.
@@ -864,7 +893,7 @@ current buffer, if possible."
     ;; Set `headers-start' to point after the 'Subject:' header to be
     ;; compatible with the existing implementation. This just sets it
     ;; to after the first header.
-    (notmuch-show-insert-headers headers)
+    (notmuch-show-insert-headers headers depth)
     (save-excursion
       (goto-char content-start)
       ;; If the subject of this message is the same as that of the
-- 
1.7.8.3

Re: [PATCH v2] emacs: Add more processing of displayed headers.

[David Bremner]

On Mon,  6 Feb 2012 15:39:05 +0000, David Edmondson <dme@dme.org> wrote:
> Wrap headers to the width of the window and indent continuations.

Hi David;

Not sure what this patch is doing in the middle of this thread?
Is it meant to be part of this series?

d

Re: [PATCH v2] emacs: Add more processing of displayed headers.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

* david@tethera.net [2012-02-14 Tue 12:30]
> On Mon,  6 Feb 2012 15:39:05 +0000, David Edmondson <dme@dme.org> wrote:
>> Wrap headers to the width of the window and indent continuations.
>
> Hi David;
>
> Not sure what this patch is doing in the middle of this thread?
> Is it meant to be part of this series?

No, it's not related to those others. Mistaken 'In-Reply-To'.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH v2] emacs: Add more processing of displayed headers.

[Ethan Glasser-Camp]

Hi! Just going through the patch queue.

This is definitely a nice effect, but I'm not sure of the approach. It
doesn't indent the message's tags, and it doesn't work when you resize the
window. (You can get some very ugly wrapping if you put your mind to
it.)

Is there no better way to do this using visual-line-mode? I know that
the rest of notmuch uses hard filling, but that's no reason to make a
bad situation worse. It looks like you can put wrap-prefix text
properties all over, as done in the adaptive-wrap package:

http://elpa.gnu.org/packages/adaptive-wrap-0.1.el

Slightly more nit-picky comments below.

David Edmondson <dme@dme.org> writes:

> -(defvar notmuch-show-markup-headers-hook '(notmuch-show-colour-headers)
> +(defcustom notmuch-show-markup-headers-hook '(notmuch-show-colour-headers
> +					      notmuch-show-fill-headers
> +					      notmuch-show-indent-continuations)
>    "A list of functions called to decorate the headers listed in
> -`notmuch-message-headers'.")
> +`notmuch-message-headers'."
> +  :type 'hook
> +  :options '(notmuch-show-colour-headers
> +	     notmuch-show-fill-headers
> +	     notmuch-show-indent-continuations)
> +  :group 'notmuch-show)

This hook is not normal because it takes an argument, and so should have
a name ending in -hooks or -functions. Also, since it's a defcustom now,
it should probably have a better explanation of how it works, that it
takes an argument, and what that argument means.

It seems extremely dicey to me that you can put
notmuch-show-indent-continuations in this list before, or without,
notmuch-show-fill-headers.

> +(defun notmuch-show-fill-headers (depth)
> +  "Wrap the text of the current headers."
> +
> +  ;; '-5' to allow for the indentation code.
> +  (let ((fill-column (- (window-width) depth 5)))

It took me a little while to figure out what this meant. How about,
"underfill by 5 so that inserting indentation doesn't cause more
wrapping"? Is it possible to be smart enough to let

> +(defun notmuch-show-indent-continuations (depth)
> +  "Indent any continuation lines."
> +  (goto-char (point-min))
> +  (while (not (eobp))
> +    (if (not (looking-at "^[A-Za-z][-A-Za-z0-9]*:"))
> +	;; Four spaces tends to work well with 'To' and 'Cc' headers.
> +	(insert "    "))
> +    (forward-line)))

I'm not crazy about this but I'm not sure I can say why exactly. Why
can't we just run indent-rigidly over the whole thing?

The comment isn't terribly useful. Only those headers? "Tends to work
well"?

>      ;; Override `notmuch-message-headers' to force `From' to be
>      ;; displayed.
>      (let ((notmuch-message-headers '("From" "Subject" "To" "Cc" "Date")))
> -      (notmuch-show-insert-headers (plist-get message :headers)))
> +      (notmuch-show-insert-headers (plist-get message :headers) 0))

This took me a long while to figure out, especially because it looks
like the depth is being passed normally to notmuch-show-insert-bodypart,
just below. A comment like "depth = 0 because we reindent below" would
have been really helpful. A good test message is
id:"87ocabvp0y.fsf@wsheee.2x.cz".

Ethan

[PATCH v3 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.

[David Edmondson]

Instead, allow the caller to specify some parameters for the
button. Rework `notmuch-show-insert-part-multipart/signed' and
`notmuch-show-insert-part-multipart/encrypted' accordingly.
---
 emacs/notmuch-show.el |   86 +++++++++++++++++++++++++-----------------------
 1 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 7469e2e..079d7cb 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -397,25 +397,24 @@ message at DEPTH in the current thread."
   "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
-	  (insert-button
-	   (concat "[ "
-		   (if name (concat name ": ") "")
-		   declared-type
-		   (if (not (string-equal declared-type content-type))
-		       (concat " (as " content-type ")")
-		     "")
-		   (or comment "")
-		   " ]")
-	   :type 'notmuch-show-part-button-type
-	   :notmuch-part nth
-	   :notmuch-filename name
-	   :notmuch-content-type content-type))
-    (insert "\n")
-    ;; return button
-    button))
+(defun notmuch-show-insert-part-header (nth content-type declared-type
+					    &optional name comment
+					    &rest button-parameters)
+  (apply #'insert-button
+	 (concat "[ "
+		 (if name (concat name ": ") "")
+		 declared-type
+		 (if (not (string-equal declared-type content-type))
+		     (concat " (as " content-type ")")
+		   "")
+		 (or comment "")
+		 " ]")
+	 :type 'notmuch-show-part-button-type
+	 :notmuch-part nth
+	 :notmuch-filename name
+	 :notmuch-content-type content-type
+	 button-parameters)
+  (insert "\n"))
 
 ;; Functions handling particular MIME parts.
 
@@ -602,15 +601,18 @@ current buffer, if possible."
   t)
 
 (defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add signature status button if sigstatus provided
-    (if (plist-member part :sigstatus)
-	(let* ((from (notmuch-show-get-header :From msg))
-	       (sigstatus (car (plist-get part :sigstatus))))
-	  (notmuch-crypto-insert-sigstatus-button sigstatus from))
-      ;; if we're not adding sigstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")))
+  ;; Add signature status button if sigstatus provided.
+  (if (plist-member part :sigstatus)
+      (let ((from (notmuch-show-get-header :From msg))
+	    (sigstatus (car (plist-get part :sigstatus))))
+	(notmuch-show-insert-part-header nth declared-type content-type nil nil 
+					 'face 'notmuch-crypto-part-header)
+	(notmuch-crypto-insert-sigstatus-button sigstatus from))
+
+    ;; If we're not adding sigstatus, tell the user how to enable it.
+    (notmuch-show-insert-part-header nth declared-type content-type nil nil 
+				     'face 'notmuch-crypto-part-header
+				     'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
@@ -624,19 +626,21 @@ current buffer, if possible."
   t)
 
 (defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add encryption status button if encstatus specified
-    (if (plist-member part :encstatus)
-	(let ((encstatus (car (plist-get part :encstatus))))
-	  (notmuch-crypto-insert-encstatus-button encstatus)
-	  ;; add signature status button if sigstatus specified
-	  (if (plist-member part :sigstatus)
-	      (let* ((from (notmuch-show-get-header :From msg))
-		     (sigstatus (car (plist-get part :sigstatus))))
-		(notmuch-crypto-insert-sigstatus-button sigstatus from))))
-      ;; if we're not adding encstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")))
+  ;; Add encryption status button if encstatus provided.
+  (if (plist-member part :encstatus)
+      (let ((encstatus (car (plist-get part :encstatus))))
+	(notmuch-show-insert-part-header nth declared-type content-type nil nil 
+					 'face 'notmuch-crypto-part-header)
+	(notmuch-crypto-insert-encstatus-button encstatus)
+	;; add signature status button if sigstatus specified
+	(if (plist-member part :sigstatus)
+	    (let* ((from (notmuch-show-get-header :From msg))
+		   (sigstatus (car (plist-get part :sigstatus))))
+	      (notmuch-crypto-insert-sigstatus-button sigstatus from))))
+    ;; If we're not adding encstatus, tell the user how to enable it.
+    (notmuch-show-insert-part-header nth declared-type content-type nil nil 
+				     'face 'notmuch-crypto-part-header
+				     'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
-- 
1.7.8.3

[PATCH v3 2/3] emacs: Optionally hide some part headers.

[David Edmondson]

Add a regexp, `notmuch-show-part-headers-hidden' and if the
content-type of a part matches, don't show the part header.
---
 emacs/notmuch-show.el |   43 ++++++++++++++++++++++++++++---------------
 1 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 079d7cb..ce79762 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -125,6 +125,12 @@ indentation."
 		 (const :tag "View interactively"
 			notmuch-show-interactively-view-part)))
 
+(defcustom notmuch-show-part-headers-hidden nil
+  "Headers for parts whose content-type matches this regexp will
+not be shown."
+  :group 'notmuch
+  :type 'regexp)
+
 (defmacro with-current-notmuch-show-message (&rest body)
   "Evaluate body with current buffer set to the text of current message"
   `(save-excursion
@@ -397,24 +403,31 @@ message at DEPTH in the current thread."
   "Submap for button commands")
 (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
 
+(defun notmuch-show-hidden-part-header (content-type)
+  "Return non-nil if a part header should be hidden for
+CONTENT-TYPE parts."
+  (and notmuch-show-part-headers-hidden
+       (string-match notmuch-show-part-headers-hidden content-type)))
+
 (defun notmuch-show-insert-part-header (nth content-type declared-type
 					    &optional name comment
 					    &rest button-parameters)
-  (apply #'insert-button
-	 (concat "[ "
-		 (if name (concat name ": ") "")
-		 declared-type
-		 (if (not (string-equal declared-type content-type))
-		     (concat " (as " content-type ")")
-		   "")
-		 (or comment "")
-		 " ]")
-	 :type 'notmuch-show-part-button-type
-	 :notmuch-part nth
-	 :notmuch-filename name
-	 :notmuch-content-type content-type
-	 button-parameters)
-  (insert "\n"))
+  (unless (notmuch-show-hidden-part-header content-type)
+    (apply #'insert-button
+	   (concat "[ "
+		   (if name (concat name ": ") "")
+		   declared-type
+		   (if (not (string-equal declared-type content-type))
+		       (concat " (as " content-type ")")
+		     "")
+		   (or comment "")
+		   " ]")
+	   :type 'notmuch-show-part-button-type
+	   :notmuch-part nth
+	   :notmuch-filename name
+	   :notmuch-content-type content-type
+	   button-parameters)
+    (insert "\n")))
 
 ;; Functions handling particular MIME parts.
 
-- 
1.7.8.3

Re: [PATCH v3 2/3] emacs: Optionally hide some part headers.

[Mark Walters]

On Mon,  6 Feb 2012 15:39:07 +0000, David Edmondson <dme@dme.org> wrote:
> Add a regexp, `notmuch-show-part-headers-hidden' and if the
> content-type of a part matches, don't show the part header.
> ---
>  emacs/notmuch-show.el |   43 ++++++++++++++++++++++++++++---------------
>  1 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 079d7cb..ce79762 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -125,6 +125,12 @@ indentation."
>  		 (const :tag "View interactively"
>  			notmuch-show-interactively-view-part)))
>  
> +(defcustom notmuch-show-part-headers-hidden nil
> +  "Headers for parts whose content-type matches this regexp will
> +not be shown."
> +  :group 'notmuch
> +  :type 'regexp)
> +

Hi 

I found this confusing for two reasons: first I thought the entire
content-type would have to match the regular expression but content-type
only needs to contain the regular expression. Secondly the value I typed
into the defcustom box seemed to need to be in quotes or I got an error
message.

Best wishes

Mark

>  (defmacro with-current-notmuch-show-message (&rest body)
>    "Evaluate body with current buffer set to the text of current message"
>    `(save-excursion
> @@ -397,24 +403,31 @@ message at DEPTH in the current thread."
>    "Submap for button commands")
>  (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
>  
> +(defun notmuch-show-hidden-part-header (content-type)
> +  "Return non-nil if a part header should be hidden for
> +CONTENT-TYPE parts."
> +  (and notmuch-show-part-headers-hidden
> +       (string-match notmuch-show-part-headers-hidden content-type)))
> +
>  (defun notmuch-show-insert-part-header (nth content-type declared-type
>  					    &optional name comment
>  					    &rest button-parameters)
> -  (apply #'insert-button
> -	 (concat "[ "
> -		 (if name (concat name ": ") "")
> -		 declared-type
> -		 (if (not (string-equal declared-type content-type))
> -		     (concat " (as " content-type ")")
> -		   "")
> -		 (or comment "")
> -		 " ]")
> -	 :type 'notmuch-show-part-button-type
> -	 :notmuch-part nth
> -	 :notmuch-filename name
> -	 :notmuch-content-type content-type
> -	 button-parameters)
> -  (insert "\n"))
> +  (unless (notmuch-show-hidden-part-header content-type)
> +    (apply #'insert-button
> +	   (concat "[ "
> +		   (if name (concat name ": ") "")
> +		   declared-type
> +		   (if (not (string-equal declared-type content-type))
> +		       (concat " (as " content-type ")")
> +		     "")
> +		   (or comment "")
> +		   " ]")
> +	   :type 'notmuch-show-part-button-type
> +	   :notmuch-part nth
> +	   :notmuch-filename name
> +	   :notmuch-content-type content-type
> +	   button-parameters)
> +    (insert "\n")))
>  
>  ;; Functions handling particular MIME parts.
>  
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/*.

[David Edmondson]

Previously this logic applied only to text/plain. Allow it for other
text/* parts as well.
---
 emacs/notmuch-show.el |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index ce79762..c60e613 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -412,7 +412,9 @@ CONTENT-TYPE parts."
 (defun notmuch-show-insert-part-header (nth content-type declared-type
 					    &optional name comment
 					    &rest button-parameters)
-  (unless (notmuch-show-hidden-part-header content-type)
+  (unless (or (notmuch-show-hidden-part-header content-type)
+	      (and (= nth 1)
+		   (string-match "text/*" content-type)))
     (apply #'insert-button
 	   (concat "[ "
 		   (if name (concat name ": ") "")
@@ -703,10 +705,7 @@ current buffer, if possible."
 
 (defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type)
   (let ((start (point)))
-    ;; If this text/plain part is not the first part in the message,
-    ;; insert a header to make this clear.
-    (if (> nth 1)
-	(notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)))
+    (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))
     (insert (notmuch-show-get-bodypart-content msg part nth))
     (save-excursion
       (save-restriction
-- 
1.7.8.3

Re: [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/*.

[Mark Walters]

On Mon,  6 Feb 2012 15:39:08 +0000, David Edmondson <dme@dme.org> wrote:
> Previously this logic applied only to text/plain. Allow it for other
> text/* parts as well.

What is the reason for treating the first part differently? 

Personally, I would prefer to keep it as now since pressing v on the
[text/html] `views' the message in my mailcap preferred html viewer
which can be convenient.

Thanks

Mark


> ---
>  emacs/notmuch-show.el |    9 ++++-----
>  1 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index ce79762..c60e613 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -412,7 +412,9 @@ CONTENT-TYPE parts."
>  (defun notmuch-show-insert-part-header (nth content-type declared-type
>  					    &optional name comment
>  					    &rest button-parameters)
> -  (unless (notmuch-show-hidden-part-header content-type)
> +  (unless (or (notmuch-show-hidden-part-header content-type)
> +	      (and (= nth 1)
> +		   (string-match "text/*" content-type)))
>      (apply #'insert-button
>  	   (concat "[ "
>  		   (if name (concat name ": ") "")
> @@ -703,10 +705,7 @@ current buffer, if possible."
>  
>  (defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type)
>    (let ((start (point)))
> -    ;; If this text/plain part is not the first part in the message,
> -    ;; insert a header to make this clear.
> -    (if (> nth 1)
> -	(notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)))
> +    (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))
>      (insert (notmuch-show-get-bodypart-content msg part nth))
>      (save-excursion
>        (save-restriction
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/*.

[Mark Walters]

On Sun, 12 Feb 2012 09:24:09 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> On Mon,  6 Feb 2012 15:39:08 +0000, David Edmondson <dme@dme.org> wrote:
> > Previously this logic applied only to text/plain. Allow it for other
> > text/* parts as well.
> 
> What is the reason for treating the first part differently? 
> 
> Personally, I would prefer to keep it as now since pressing v on the
> [text/html] `views' the message in my mailcap preferred html viewer
> which can be convenient.

Actually I experimented with this and if I remove the special case
totally (i.e., for text/plain as well) I get a button which lets me save
the text part of the email fixing the problem I mentioned in
id:"87y5tl3xz0.fsf@qmul.ac.uk"

I wonder if it would be possible to always include the button but
possibly at the end rather than the start of the message? Or possibly
add a view-mime-part-structure function which showed all the buttons.

Best wishes

Mark

> 
> Thanks
> 
> Mark
> 
> 
> > ---
> >  emacs/notmuch-show.el |    9 ++++-----
> >  1 files changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> > index ce79762..c60e613 100644
> > --- a/emacs/notmuch-show.el
> > +++ b/emacs/notmuch-show.el
> > @@ -412,7 +412,9 @@ CONTENT-TYPE parts."
> >  (defun notmuch-show-insert-part-header (nth content-type declared-type
> >  					    &optional name comment
> >  					    &rest button-parameters)
> > -  (unless (notmuch-show-hidden-part-header content-type)
> > +  (unless (or (notmuch-show-hidden-part-header content-type)
> > +	      (and (= nth 1)
> > +		   (string-match "text/*" content-type)))
> >      (apply #'insert-button
> >  	   (concat "[ "
> >  		   (if name (concat name ": ") "")
> > @@ -703,10 +705,7 @@ current buffer, if possible."
> >  
> >  (defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type)
> >    (let ((start (point)))
> > -    ;; If this text/plain part is not the first part in the message,
> > -    ;; insert a header to make this clear.
> > -    (if (> nth 1)
> > -	(notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)))
> > +    (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))
> >      (insert (notmuch-show-get-bodypart-content msg part nth))
> >      (save-excursion
> >        (save-restriction
> > -- 
> > 1.7.8.3
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch