unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 1/3] emacs: Tidy `notmuch-show-insert-part-header'.
@ 2012-01-20  9:43 David Edmondson
  2012-01-20  9:43 ` [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
                   ` (3 more replies)
  0 siblings, 4 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-20  9:43 UTC (permalink / raw)
  To: notmuch

---
 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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-20  9:43 [PATCH 1/3] emacs: Tidy `notmuch-show-insert-part-header' David Edmondson
@ 2012-01-20  9:43 ` David Edmondson
  2012-01-20 20:20   ` Jameson Graef Rollins
  2012-01-22 21:38   ` Jameson Graef Rollins
  2012-01-20  9:43 ` [PATCH 3/3] emacs: Optionally hide some part headers David Edmondson
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-20  9:43 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 3/3] emacs: Optionally hide some part headers.
  2012-01-20  9:43 [PATCH 1/3] emacs: Tidy `notmuch-show-insert-part-header' David Edmondson
  2012-01-20  9:43 ` [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
@ 2012-01-20  9:43 ` David Edmondson
  2012-01-24 12:53 ` optional hiding of some part headers v3 David Edmondson
  2012-02-06 15:39 ` [PATCH v3 0/3] part header code tidying and options David Edmondson
  3 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-20  9:43 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-20  9:43 ` [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
@ 2012-01-20 20:20   ` Jameson Graef Rollins
  2012-01-22 21:38   ` Jameson Graef Rollins
  1 sibling, 0 replies; 34+ messages in thread
From: Jameson Graef Rollins @ 2012-01-20 20:20 UTC (permalink / raw)
  To: David Edmondson, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-20  9:43 ` [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
  2012-01-20 20:20   ` Jameson Graef Rollins
@ 2012-01-22 21:38   ` Jameson Graef Rollins
  2012-01-23  8:16     ` David Edmondson
  1 sibling, 1 reply; 34+ messages in thread
From: Jameson Graef Rollins @ 2012-01-22 21:38 UTC (permalink / raw)
  To: David Edmondson, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-22 21:38   ` Jameson Graef Rollins
@ 2012-01-23  8:16     ` David Edmondson
  2012-01-23  8:52       ` Jameson Graef Rollins
  0 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2012-01-23  8:16 UTC (permalink / raw)
  To: Jameson Graef Rollins; +Cc: notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-23  8:16     ` David Edmondson
@ 2012-01-23  8:52       ` Jameson Graef Rollins
  2012-01-23  9:12         ` David Edmondson
  0 siblings, 1 reply; 34+ messages in thread
From: Jameson Graef Rollins @ 2012-01-23  8:52 UTC (permalink / raw)
  To: David Edmondson; +Cc: notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-23  8:52       ` Jameson Graef Rollins
@ 2012-01-23  9:12         ` David Edmondson
  2012-01-23 10:33           ` [PATCH 1/3 v2] " David Edmondson
  0 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2012-01-23  9:12 UTC (permalink / raw)
  To: Jameson Graef Rollins; +Cc: notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-23  9:12         ` David Edmondson
@ 2012-01-23 10:33           ` David Edmondson
  2012-01-23 10:33             ` [PATCH 2/3 v2] emacs: Optionally hide some part headers David Edmondson
                               ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-23 10:33 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/3 v2] emacs: Optionally hide some part headers.
  2012-01-23 10:33           ` [PATCH 1/3 v2] " David Edmondson
@ 2012-01-23 10:33             ` David Edmondson
  2012-01-23 10:33             ` [PATCH 3/3 v2] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
  2012-01-23 11:32             ` [PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
  2 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-23 10:33 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 3/3 v2] emacs: Don't insert a part header if it's the first part and text/*.
  2012-01-23 10:33           ` [PATCH 1/3 v2] " David Edmondson
  2012-01-23 10:33             ` [PATCH 2/3 v2] emacs: Optionally hide some part headers David Edmondson
@ 2012-01-23 10:33             ` David Edmondson
  2012-01-23 11:32             ` [PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
  2 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-23 10:33 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-23 10:33           ` [PATCH 1/3 v2] " David Edmondson
  2012-01-23 10:33             ` [PATCH 2/3 v2] emacs: Optionally hide some part headers David Edmondson
  2012-01-23 10:33             ` [PATCH 3/3 v2] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
@ 2012-01-23 11:32             ` David Edmondson
  2 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-23 11:32 UTC (permalink / raw)
  To: notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* optional hiding of some part headers v3
  2012-01-20  9:43 [PATCH 1/3] emacs: Tidy `notmuch-show-insert-part-header' David Edmondson
  2012-01-20  9:43 ` [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
  2012-01-20  9:43 ` [PATCH 3/3] emacs: Optionally hide some part headers David Edmondson
@ 2012-01-24 12:53 ` David Edmondson
  2012-01-24 12:53   ` [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
                     ` (2 more replies)
  2012-02-06 15:39 ` [PATCH v3 0/3] part header code tidying and options David Edmondson
  3 siblings, 3 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-24 12:53 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-24 12:53 ` optional hiding of some part headers v3 David Edmondson
@ 2012-01-24 12:53   ` David Edmondson
  2012-01-24 18:46     ` Jameson Graef Rollins
  2012-01-24 19:33     ` Tomi Ollila
  2012-01-24 12:53   ` [PATCH 2/3] emacs: Optionally hide some part headers David Edmondson
  2012-01-24 12:53   ` [PATCH 3/3] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
  2 siblings, 2 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-24 12:53 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/3] emacs: Optionally hide some part headers.
  2012-01-24 12:53 ` optional hiding of some part headers v3 David Edmondson
  2012-01-24 12:53   ` [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
@ 2012-01-24 12:53   ` David Edmondson
  2012-01-24 19:34     ` Tomi Ollila
  2012-01-24 12:53   ` [PATCH 3/3] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
  2 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2012-01-24 12:53 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 3/3] emacs: Don't insert a part header if it's the first part and text/*.
  2012-01-24 12:53 ` optional hiding of some part headers v3 David Edmondson
  2012-01-24 12:53   ` [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
  2012-01-24 12:53   ` [PATCH 2/3] emacs: Optionally hide some part headers David Edmondson
@ 2012-01-24 12:53   ` David Edmondson
  2012-01-24 19:46     ` Tomi Ollila
  2 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2012-01-24 12:53 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-24 12:53   ` [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
@ 2012-01-24 18:46     ` Jameson Graef Rollins
  2012-01-24 19:25       ` David Edmondson
  2012-01-24 19:33     ` Tomi Ollila
  1 sibling, 1 reply; 34+ messages in thread
From: Jameson Graef Rollins @ 2012-01-24 18:46 UTC (permalink / raw)
  To: David Edmondson, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-24 18:46     ` Jameson Graef Rollins
@ 2012-01-24 19:25       ` David Edmondson
  2012-01-24 19:52         ` Jameson Graef Rollins
  0 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2012-01-24 19:25 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-24 12:53   ` [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
  2012-01-24 18:46     ` Jameson Graef Rollins
@ 2012-01-24 19:33     ` Tomi Ollila
  1 sibling, 0 replies; 34+ messages in thread
From: Tomi Ollila @ 2012-01-24 19:33 UTC (permalink / raw)
  To: David Edmondson, notmuch

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.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/3] emacs: Optionally hide some part headers.
  2012-01-24 12:53   ` [PATCH 2/3] emacs: Optionally hide some part headers David Edmondson
@ 2012-01-24 19:34     ` Tomi Ollila
  0 siblings, 0 replies; 34+ messages in thread
From: Tomi Ollila @ 2012-01-24 19:34 UTC (permalink / raw)
  To: David Edmondson, notmuch

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)

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/3] emacs: Don't insert a part header if it's the first part and text/*.
  2012-01-24 12:53   ` [PATCH 3/3] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
@ 2012-01-24 19:46     ` Tomi Ollila
  0 siblings, 0 replies; 34+ messages in thread
From: Tomi Ollila @ 2012-01-24 19:46 UTC (permalink / raw)
  To: David Edmondson, notmuch

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.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-24 19:25       ` David Edmondson
@ 2012-01-24 19:52         ` Jameson Graef Rollins
  2012-01-25  6:15           ` David Edmondson
  0 siblings, 1 reply; 34+ messages in thread
From: Jameson Graef Rollins @ 2012-01-24 19:52 UTC (permalink / raw)
  To: David Edmondson, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-01-24 19:52         ` Jameson Graef Rollins
@ 2012-01-25  6:15           ` David Edmondson
  0 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2012-01-25  6:15 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 0/3] part header code tidying and options
  2012-01-20  9:43 [PATCH 1/3] emacs: Tidy `notmuch-show-insert-part-header' David Edmondson
                   ` (2 preceding siblings ...)
  2012-01-24 12:53 ` optional hiding of some part headers v3 David Edmondson
@ 2012-02-06 15:39 ` David Edmondson
  2012-02-06 15:39   ` [PATCH v2] emacs: Add more processing of displayed headers David Edmondson
                     ` (3 more replies)
  3 siblings, 4 replies; 34+ messages in thread
From: David Edmondson @ 2012-02-06 15:39 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2] emacs: Add more processing of displayed headers.
  2012-02-06 15:39 ` [PATCH v3 0/3] part header code tidying and options David Edmondson
@ 2012-02-06 15:39   ` David Edmondson
  2012-02-14 12:30     ` David Bremner
  2012-10-12 19:11     ` Ethan Glasser-Camp
  2012-02-06 15:39   ` [PATCH v3 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 34+ messages in thread
From: David Edmondson @ 2012-02-06 15:39 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
  2012-02-06 15:39 ` [PATCH v3 0/3] part header code tidying and options David Edmondson
  2012-02-06 15:39   ` [PATCH v2] emacs: Add more processing of displayed headers David Edmondson
@ 2012-02-06 15:39   ` David Edmondson
  2012-02-06 15:39   ` [PATCH v3 2/3] emacs: Optionally hide some part headers David Edmondson
  2012-02-06 15:39   ` [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
  3 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2012-02-06 15:39 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 2/3] emacs: Optionally hide some part headers.
  2012-02-06 15:39 ` [PATCH v3 0/3] part header code tidying and options David Edmondson
  2012-02-06 15:39   ` [PATCH v2] emacs: Add more processing of displayed headers David Edmondson
  2012-02-06 15:39   ` [PATCH v3 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
@ 2012-02-06 15:39   ` David Edmondson
  2012-02-12  9:21     ` Mark Walters
  2012-02-06 15:39   ` [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
  3 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2012-02-06 15:39 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/*.
  2012-02-06 15:39 ` [PATCH v3 0/3] part header code tidying and options David Edmondson
                     ` (2 preceding siblings ...)
  2012-02-06 15:39   ` [PATCH v3 2/3] emacs: Optionally hide some part headers David Edmondson
@ 2012-02-06 15:39   ` David Edmondson
  2012-02-12  9:24     ` Mark Walters
  3 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2012-02-06 15:39 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/3] emacs: Optionally hide some part headers.
  2012-02-06 15:39   ` [PATCH v3 2/3] emacs: Optionally hide some part headers David Edmondson
@ 2012-02-12  9:21     ` Mark Walters
  0 siblings, 0 replies; 34+ messages in thread
From: Mark Walters @ 2012-02-12  9:21 UTC (permalink / raw)
  To: David Edmondson, notmuch

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/*.
  2012-02-06 15:39   ` [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
@ 2012-02-12  9:24     ` Mark Walters
  2012-02-12  9:39       ` Mark Walters
  0 siblings, 1 reply; 34+ messages in thread
From: Mark Walters @ 2012-02-12  9:24 UTC (permalink / raw)
  To: David Edmondson, notmuch

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/*.
  2012-02-12  9:24     ` Mark Walters
@ 2012-02-12  9:39       ` Mark Walters
  0 siblings, 0 replies; 34+ messages in thread
From: Mark Walters @ 2012-02-12  9:39 UTC (permalink / raw)
  To: David Edmondson, notmuch


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2] emacs: Add more processing of displayed headers.
  2012-02-06 15:39   ` [PATCH v2] emacs: Add more processing of displayed headers David Edmondson
@ 2012-02-14 12:30     ` David Bremner
  2012-02-14 13:28       ` David Edmondson
  2012-10-12 19:11     ` Ethan Glasser-Camp
  1 sibling, 1 reply; 34+ messages in thread
From: David Bremner @ 2012-02-14 12:30 UTC (permalink / raw)
  To: David Edmondson, notmuch

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2] emacs: Add more processing of displayed headers.
  2012-02-14 12:30     ` David Bremner
@ 2012-02-14 13:28       ` David Edmondson
  0 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2012-02-14 13:28 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2] emacs: Add more processing of displayed headers.
  2012-02-06 15:39   ` [PATCH v2] emacs: Add more processing of displayed headers David Edmondson
  2012-02-14 12:30     ` David Bremner
@ 2012-10-12 19:11     ` Ethan Glasser-Camp
  1 sibling, 0 replies; 34+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-12 19:11 UTC (permalink / raw)
  To: David Edmondson, notmuch

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2012-10-12 19:11 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-20  9:43 [PATCH 1/3] emacs: Tidy `notmuch-show-insert-part-header' David Edmondson
2012-01-20  9:43 ` [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
2012-01-20 20:20   ` Jameson Graef Rollins
2012-01-22 21:38   ` Jameson Graef Rollins
2012-01-23  8:16     ` David Edmondson
2012-01-23  8:52       ` Jameson Graef Rollins
2012-01-23  9:12         ` David Edmondson
2012-01-23 10:33           ` [PATCH 1/3 v2] " David Edmondson
2012-01-23 10:33             ` [PATCH 2/3 v2] emacs: Optionally hide some part headers David Edmondson
2012-01-23 10:33             ` [PATCH 3/3 v2] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
2012-01-23 11:32             ` [PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
2012-01-20  9:43 ` [PATCH 3/3] emacs: Optionally hide some part headers David Edmondson
2012-01-24 12:53 ` optional hiding of some part headers v3 David Edmondson
2012-01-24 12:53   ` [PATCH 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
2012-01-24 18:46     ` Jameson Graef Rollins
2012-01-24 19:25       ` David Edmondson
2012-01-24 19:52         ` Jameson Graef Rollins
2012-01-25  6:15           ` David Edmondson
2012-01-24 19:33     ` Tomi Ollila
2012-01-24 12:53   ` [PATCH 2/3] emacs: Optionally hide some part headers David Edmondson
2012-01-24 19:34     ` Tomi Ollila
2012-01-24 12:53   ` [PATCH 3/3] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
2012-01-24 19:46     ` Tomi Ollila
2012-02-06 15:39 ` [PATCH v3 0/3] part header code tidying and options David Edmondson
2012-02-06 15:39   ` [PATCH v2] emacs: Add more processing of displayed headers David Edmondson
2012-02-14 12:30     ` David Bremner
2012-02-14 13:28       ` David Edmondson
2012-10-12 19:11     ` Ethan Glasser-Camp
2012-02-06 15:39   ` [PATCH v3 1/3] emacs: Don't return the button from `notmuch-show-insert-part-header' David Edmondson
2012-02-06 15:39   ` [PATCH v3 2/3] emacs: Optionally hide some part headers David Edmondson
2012-02-12  9:21     ` Mark Walters
2012-02-06 15:39   ` [PATCH v3 3/3] emacs: Don't insert a part header if it's the first part and text/* David Edmondson
2012-02-12  9:24     ` Mark Walters
2012-02-12  9:39       ` Mark Walters

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).