[PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
On Mon, Jan 19 2015, Mark Walters wrote: > On Mon, 12 May 2014, David Edmondson wrote: >> Whether to insert part headers should depend on the details of the >> message being cited. > > Hi > > Overall I like this series and it does fix two annoying bugs (not being > able to reply to ref822 messages and (correctly) including parts > which have application/octet-stream but are actually text parts). > > The one problem is getting the right choice for part headers in the > reply text. I think getting this wrong will irritate users even if the > overall result is better. > > My guess at the correct logic is: > 1) omit the part header for any empty part: (ie a part we don't display > such as a pdf file). This seems wrong to me (i.e. it's not what I would want ;-). Showing the part header in the reply acknowledges that it was part of the message that I'm replying to. Even more, consider a message: Hi, attached are the two PDF documents that we discussed. This is the version with Fred's suggested comments: [ application/pdf: document 1 ] This is the version with my proposed alternative edits and much more content added: [ application/pdf: document 2 ] The part headers form a significant part of the content. In a reply I'd like to see them, so that I can add comments appropriately. (I realise that commenting 'in-line' is out of fashion in many places now, but for more complex discussions I still prefer it.) I still like the original rule that I proposed: the reply should include whatever is in the 'show' buffer, modulo content that was elided due to washing. > 2) omit multipart/* part headers > 3) include all other part headers > 4) except omit the first part header (perhaps only in the case it is > text/plain) > > My reasoning for each is > 1) there is no point in saying we had a part which we are omitting. > 2) all the subparts of multipart/* will get there own header which > should be sufficient. > 3) we want to keep the parts distinguished > 4) except we don't need to do that with the first part. > > Note for 4) it would be good to have a multipart/alternative with > subparts text/plain and text/html just give the text/plain with no part > header. > > I include a patch below which does all of these apart from 4) as I > couldn't see a clean way of implementing it. Any suggestions? > > It should apply on top of patch 6 or 7 instead of 8. The key change is > that it always puts in a button and then deletes it if unwanted: this > makes doing 1) above easy. > > It does break some tests, nothing unexpected except an interaction with > the way we wash text/plain parts: we remove leading blank lines from the > first text/plain part (because it doesn't have a button) but not from > subsequent ones (because they do). Because this code always has the > second case it doesn't remove a leading blank line of the first part. > > Best wishes > > Mark > > > From 8f198b38e76e050ae8d20d866748c41ccf79f3d4 Mon Sep 17 00:00:00 2001 > From: Mark Walters > Date: Mon, 19 Jan 2015 14:39:25 + > Subject: [PATCH] emacs show/reply modify part handling > > Modify the part handling so that we always insert the button and > delete it afterwards if not wanted. The advantage is that we can > decide whether to keep the part button based on what the insertion > code does. In particular the reply code can omit the button for all > parts with no displayable content. > --- > emacs/notmuch-mua.el | 5 +++-- > emacs/notmuch-show.el | 39 +-- > 2 files changed, 28 insertions(+), 16 deletions(-) > > diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el > index 0ca9eed..6060f33 100644 > --- a/emacs/notmuch-mua.el > +++ b/emacs/notmuch-mua.el > @@ -29,6 +29,7 @@ > (eval-when-compile (require 'cl)) > > (declare-function notmuch-show-insert-body "notmuch-show" (msg body depth)) > +(declare-function notmuch-show-insert-header-p-reply "notmuch-show" (part > empty-part)) > > ;; > > @@ -223,8 +224,8 @@ Note that these functions use `mail-citation-hook' if > that is non-nil." > ;; citations, etc. in the original message before > ;; quoting. > ((notmuch-show-insert-text/plain-hook nil) > -;; Don't insert part buttons. > -(notmuch-show-insert-header-p-function > #'notmuch-show-insert-header-p-never)) > +;; Insert part buttons appropriate for a reply. > +(notmuch-show-insert-header-p-function > #'notmuch-show-insert-header-p-reply)) > (notmuch-show-insert-body original (plist-get original > :body) 0) > (buffer-substring-no-properties (point-min) (point-max) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 4a0899f..2cdb5a8 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -909,16 +909,24 @@ message at DEPTH in the current thread." >"text/x-diff") >
Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
On Mon, Jan 19 2015, Mark Walters wrote: > On Mon, 12 May 2014, David Edmondson wrote: >> Whether to insert part headers should depend on the details of the >> message being cited. > > Hi > > Overall I like this series and it does fix two annoying bugs (not being > able to reply to ref822 messages and (correctly) including parts > which have application/octet-stream but are actually text parts). > > The one problem is getting the right choice for part headers in the > reply text. I think getting this wrong will irritate users even if the > overall result is better. > > My guess at the correct logic is: > 1) omit the part header for any empty part: (ie a part we don't display > such as a pdf file). This seems wrong to me (i.e. it's not what I would want ;-). Showing the part header in the reply acknowledges that it was part of the message that I'm replying to. Even more, consider a message: Hi, attached are the two PDF documents that we discussed. This is the version with Fred's suggested comments: [ application/pdf: document 1 ] This is the version with my proposed alternative edits and much more content added: [ application/pdf: document 2 ] The part headers form a significant part of the content. In a reply I'd like to see them, so that I can add comments appropriately. (I realise that commenting 'in-line' is out of fashion in many places now, but for more complex discussions I still prefer it.) I still like the original rule that I proposed: the reply should include whatever is in the 'show' buffer, modulo content that was elided due to washing. > 2) omit multipart/* part headers > 3) include all other part headers > 4) except omit the first part header (perhaps only in the case it is > text/plain) > > My reasoning for each is > 1) there is no point in saying we had a part which we are omitting. > 2) all the subparts of multipart/* will get there own header which > should be sufficient. > 3) we want to keep the parts distinguished > 4) except we don't need to do that with the first part. > > Note for 4) it would be good to have a multipart/alternative with > subparts text/plain and text/html just give the text/plain with no part > header. > > I include a patch below which does all of these apart from 4) as I > couldn't see a clean way of implementing it. Any suggestions? > > It should apply on top of patch 6 or 7 instead of 8. The key change is > that it always puts in a button and then deletes it if unwanted: this > makes doing 1) above easy. > > It does break some tests, nothing unexpected except an interaction with > the way we wash text/plain parts: we remove leading blank lines from the > first text/plain part (because it doesn't have a button) but not from > subsequent ones (because they do). Because this code always has the > second case it doesn't remove a leading blank line of the first part. > > Best wishes > > Mark > > > From 8f198b38e76e050ae8d20d866748c41ccf79f3d4 Mon Sep 17 00:00:00 2001 > From: Mark Walters > Date: Mon, 19 Jan 2015 14:39:25 + > Subject: [PATCH] emacs show/reply modify part handling > > Modify the part handling so that we always insert the button and > delete it afterwards if not wanted. The advantage is that we can > decide whether to keep the part button based on what the insertion > code does. In particular the reply code can omit the button for all > parts with no displayable content. > --- > emacs/notmuch-mua.el | 5 +++-- > emacs/notmuch-show.el | 39 +-- > 2 files changed, 28 insertions(+), 16 deletions(-) > > diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el > index 0ca9eed..6060f33 100644 > --- a/emacs/notmuch-mua.el > +++ b/emacs/notmuch-mua.el > @@ -29,6 +29,7 @@ > (eval-when-compile (require 'cl)) > > (declare-function notmuch-show-insert-body "notmuch-show" (msg body depth)) > +(declare-function notmuch-show-insert-header-p-reply "notmuch-show" (part > empty-part)) > > ;; > > @@ -223,8 +224,8 @@ Note that these functions use `mail-citation-hook' if > that is non-nil." > ;; citations, etc. in the original message before > ;; quoting. > ((notmuch-show-insert-text/plain-hook nil) > -;; Don't insert part buttons. > -(notmuch-show-insert-header-p-function > #'notmuch-show-insert-header-p-never)) > +;; Insert part buttons appropriate for a reply. > +(notmuch-show-insert-header-p-function > #'notmuch-show-insert-header-p-reply)) > (notmuch-show-insert-body original (plist-get original > :body) 0) > (buffer-substring-no-properties (point-min) (point-max) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 4a0899f..2cdb5a8 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -909,16 +909,24 @@ message at DEPTH in the current thread." >"text/x-diff")
[PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
On Mon, 12 May 2014, David Edmondson wrote: > Whether to insert part headers should depend on the details of the > message being cited. Hi Overall I like this series and it does fix two annoying bugs (not being able to reply to ref822 messages and (correctly) including parts which have application/octet-stream but are actually text parts). The one problem is getting the right choice for part headers in the reply text. I think getting this wrong will irritate users even if the overall result is better. My guess at the correct logic is: 1) omit the part header for any empty part: (ie a part we don't display such as a pdf file). 2) omit multipart/* part headers 3) include all other part headers 4) except omit the first part header (perhaps only in the case it is text/plain) My reasoning for each is 1) there is no point in saying we had a part which we are omitting. 2) all the subparts of multipart/* will get there own header which should be sufficient. 3) we want to keep the parts distinguished 4) except we don't need to do that with the first part. Note for 4) it would be good to have a multipart/alternative with subparts text/plain and text/html just give the text/plain with no part header. I include a patch below which does all of these apart from 4) as I couldn't see a clean way of implementing it. Any suggestions? It should apply on top of patch 6 or 7 instead of 8. The key change is that it always puts in a button and then deletes it if unwanted: this makes doing 1) above easy. It does break some tests, nothing unexpected except an interaction with the way we wash text/plain parts: we remove leading blank lines from the first text/plain part (because it doesn't have a button) but not from subsequent ones (because they do). Because this code always has the second case it doesn't remove a leading blank line of the first part. Best wishes Mark >From 8f198b38e76e050ae8d20d866748c41ccf79f3d4 Mon Sep 17 00:00:00 2001 From: Mark Walters Date: Mon, 19 Jan 2015 14:39:25 + Subject: [PATCH] emacs show/reply modify part handling Modify the part handling so that we always insert the button and delete it afterwards if not wanted. The advantage is that we can decide whether to keep the part button based on what the insertion code does. In particular the reply code can omit the button for all parts with no displayable content. --- emacs/notmuch-mua.el | 5 +++-- emacs/notmuch-show.el | 39 +-- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el index 0ca9eed..6060f33 100644 --- a/emacs/notmuch-mua.el +++ b/emacs/notmuch-mua.el @@ -29,6 +29,7 @@ (eval-when-compile (require 'cl)) (declare-function notmuch-show-insert-body "notmuch-show" (msg body depth)) +(declare-function notmuch-show-insert-header-p-reply "notmuch-show" (part empty-part)) ;; @@ -223,8 +224,8 @@ Note that these functions use `mail-citation-hook' if that is non-nil." ;; citations, etc. in the original message before ;; quoting. ((notmuch-show-insert-text/plain-hook nil) - ;; Don't insert part buttons. - (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-never)) + ;; Insert part buttons appropriate for a reply. + (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-reply)) (notmuch-show-insert-body original (plist-get original :body) 0) (buffer-substring-no-properties (point-min) (point-max) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 4a0899f..2cdb5a8 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -909,16 +909,24 @@ message at DEPTH in the current thread." "text/x-diff") content-type))) -(defun notmuch-show-insert-header-p-smart (part) +(defun notmuch-show-insert-header-p-smart (part empty-part) "Return non-NIL if a header button should be inserted for this part." (let ((mime-type (notmuch-show-mime-type part))) (not (and (string= mime-type "text/plain") (<= (plist-get part :id) 1) -(defun notmuch-show-insert-header-p-always (part) +(defun notmuch-show-insert-header-p-reply (part empty-part) + "Return non-NIL if a header button should be inserted for this part." + (let ((mime-type (notmuch-show-mime-type part))) +(not (or empty-part +(notmuch-match-content-type mime-type "multipart/*") +(and (string= mime-type "text/plain") + (<= (plist-get part :id) 1)) + +(defun notmuch-show-insert-header-p-always (part empty-part) t) -(defun notmuch-show-insert-header-p-never (part) +(defun notmuch-show-insert-header-p-never (part empty-part) nil) (defun notmuch-show-insert-bodypart (msg part depth &optional hide) @@ -936,8 +944,8 @@ is t, hide the part initially and
Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
On Mon, 12 May 2014, David Edmondson wrote: > Whether to insert part headers should depend on the details of the > message being cited. Hi Overall I like this series and it does fix two annoying bugs (not being able to reply to ref822 messages and (correctly) including parts which have application/octet-stream but are actually text parts). The one problem is getting the right choice for part headers in the reply text. I think getting this wrong will irritate users even if the overall result is better. My guess at the correct logic is: 1) omit the part header for any empty part: (ie a part we don't display such as a pdf file). 2) omit multipart/* part headers 3) include all other part headers 4) except omit the first part header (perhaps only in the case it is text/plain) My reasoning for each is 1) there is no point in saying we had a part which we are omitting. 2) all the subparts of multipart/* will get there own header which should be sufficient. 3) we want to keep the parts distinguished 4) except we don't need to do that with the first part. Note for 4) it would be good to have a multipart/alternative with subparts text/plain and text/html just give the text/plain with no part header. I include a patch below which does all of these apart from 4) as I couldn't see a clean way of implementing it. Any suggestions? It should apply on top of patch 6 or 7 instead of 8. The key change is that it always puts in a button and then deletes it if unwanted: this makes doing 1) above easy. It does break some tests, nothing unexpected except an interaction with the way we wash text/plain parts: we remove leading blank lines from the first text/plain part (because it doesn't have a button) but not from subsequent ones (because they do). Because this code always has the second case it doesn't remove a leading blank line of the first part. Best wishes Mark >From 8f198b38e76e050ae8d20d866748c41ccf79f3d4 Mon Sep 17 00:00:00 2001 From: Mark Walters Date: Mon, 19 Jan 2015 14:39:25 + Subject: [PATCH] emacs show/reply modify part handling Modify the part handling so that we always insert the button and delete it afterwards if not wanted. The advantage is that we can decide whether to keep the part button based on what the insertion code does. In particular the reply code can omit the button for all parts with no displayable content. --- emacs/notmuch-mua.el | 5 +++-- emacs/notmuch-show.el | 39 +-- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el index 0ca9eed..6060f33 100644 --- a/emacs/notmuch-mua.el +++ b/emacs/notmuch-mua.el @@ -29,6 +29,7 @@ (eval-when-compile (require 'cl)) (declare-function notmuch-show-insert-body "notmuch-show" (msg body depth)) +(declare-function notmuch-show-insert-header-p-reply "notmuch-show" (part empty-part)) ;; @@ -223,8 +224,8 @@ Note that these functions use `mail-citation-hook' if that is non-nil." ;; citations, etc. in the original message before ;; quoting. ((notmuch-show-insert-text/plain-hook nil) - ;; Don't insert part buttons. - (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-never)) + ;; Insert part buttons appropriate for a reply. + (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-reply)) (notmuch-show-insert-body original (plist-get original :body) 0) (buffer-substring-no-properties (point-min) (point-max) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 4a0899f..2cdb5a8 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -909,16 +909,24 @@ message at DEPTH in the current thread." "text/x-diff") content-type))) -(defun notmuch-show-insert-header-p-smart (part) +(defun notmuch-show-insert-header-p-smart (part empty-part) "Return non-NIL if a header button should be inserted for this part." (let ((mime-type (notmuch-show-mime-type part))) (not (and (string= mime-type "text/plain") (<= (plist-get part :id) 1) -(defun notmuch-show-insert-header-p-always (part) +(defun notmuch-show-insert-header-p-reply (part empty-part) + "Return non-NIL if a header button should be inserted for this part." + (let ((mime-type (notmuch-show-mime-type part))) +(not (or empty-part +(notmuch-match-content-type mime-type "multipart/*") +(and (string= mime-type "text/plain") + (<= (plist-get part :id) 1)) + +(defun notmuch-show-insert-header-p-always (part empty-part) t) -(defun notmuch-show-insert-header-p-never (part) +(defun notmuch-show-insert-header-p-never (part empty-part) nil) (defun notmuch-show-insert-bodypart (msg part depth &optional hide) @@ -936,8 +944,8 @@ is t, hide the part initia
[PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
Whether to insert part headers should depend on the details of the
message being cited.
---
emacs/notmuch-mua.el | 45 +++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index c800c38..239cc1a 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -132,6 +132,47 @@ list."
(funcall original-func header references)
(unless (bolp) (insert "\n")))
+(defun notmuch-mua-reply-determine-part-function (message)
+ "Determine the part header rendering function to use when
+citing MESSAGE."
+
+ (let* ((body (plist-get message :body))
+(first-part (car body)))
+(cond
+ ;; If there are multiple top-level parts, we need the
+ ;; headers. Will this ever happen?
+ ((> (length body) 1)
+ #'notmuch-show-insert-header-p-always)
+
+ ;; If the type of the part is multipart/mixed, we need to see the
+ ;; part headers.
+ ((notmuch-match-content-type (plist-get first-part :content-type)
"multipart/mixed")
+ #'notmuch-show-insert-header-p-always)
+
+ ;; If it is multipart/alternative the renderer will choose a
+ ;; default part and render it. There's no need to show the
+ ;; alternative part buttons, as they are not active.
+ ((notmuch-match-content-type (plist-get first-part :content-type)
"multipart/alternative")
+ #'notmuch-show-insert-header-p-never)
+
+ ;; If it's a multipart/signed with a single text/* part and a
+ ;; signature, we don't need to see the headers.
+ ((let ((inner-content (plist-get first-part :content)))
+ (and (notmuch-match-content-type (plist-get first-part :content-type)
"multipart/signed")
+(eq (length inner-content) 2)
+(notmuch-match-content-type (plist-get (nth 0 inner-content)
:content-type) "text/*")
+(notmuch-match-content-type (plist-get (nth 1 inner-content)
:content-type) "application/pgp-signature")))
+ #'notmuch-show-insert-header-p-never)
+
+ ;; If the type of the part is text/*, we don't need to see the
+ ;; part headers.
+ ((notmuch-match-content-type (plist-get first-part :content-type)
"text/*")
+ #'notmuch-show-insert-header-p-never)
+
+ ;; Otherwise insert the part headers.
+ (t
+ #'notmuch-show-insert-header-p-always
+
(defun notmuch-mua-reply (query-string &optional sender reply-all)
(let ((args '("reply" "--format=sexp" "--format-version=1"))
(process-crypto notmuch-show-process-crypto)
@@ -206,8 +247,8 @@ list."
;; citations, etc. in the original message before
;; quoting.
((notmuch-show-insert-text/plain-hook nil)
- ;; Don't insert part buttons.
- (notmuch-show-insert-header-p-function
#'notmuch-show-insert-header-p-never))
+ ;; Determine how to insert part headers.
+ (notmuch-show-insert-header-p-function
(notmuch-mua-reply-determine-part-function original)))
(notmuch-show-insert-body original (plist-get original
:body) 0)
(buffer-substring-no-properties (point-min) (point-max)
--
2.0.0.rc0
[PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
Whether to insert part headers should depend on the details of the
message being cited.
---
emacs/notmuch-mua.el | 45 +++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index c800c38..239cc1a 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -132,6 +132,47 @@ list."
(funcall original-func header references)
(unless (bolp) (insert "\n")))
+(defun notmuch-mua-reply-determine-part-function (message)
+ "Determine the part header rendering function to use when
+citing MESSAGE."
+
+ (let* ((body (plist-get message :body))
+(first-part (car body)))
+(cond
+ ;; If there are multiple top-level parts, we need the
+ ;; headers. Will this ever happen?
+ ((> (length body) 1)
+ #'notmuch-show-insert-header-p-always)
+
+ ;; If the type of the part is multipart/mixed, we need to see the
+ ;; part headers.
+ ((notmuch-match-content-type (plist-get first-part :content-type)
"multipart/mixed")
+ #'notmuch-show-insert-header-p-always)
+
+ ;; If it is multipart/alternative the renderer will choose a
+ ;; default part and render it. There's no need to show the
+ ;; alternative part buttons, as they are not active.
+ ((notmuch-match-content-type (plist-get first-part :content-type)
"multipart/alternative")
+ #'notmuch-show-insert-header-p-never)
+
+ ;; If it's a multipart/signed with a single text/* part and a
+ ;; signature, we don't need to see the headers.
+ ((let ((inner-content (plist-get first-part :content)))
+ (and (notmuch-match-content-type (plist-get first-part :content-type)
"multipart/signed")
+(eq (length inner-content) 2)
+(notmuch-match-content-type (plist-get (nth 0 inner-content)
:content-type) "text/*")
+(notmuch-match-content-type (plist-get (nth 1 inner-content)
:content-type) "application/pgp-signature")))
+ #'notmuch-show-insert-header-p-never)
+
+ ;; If the type of the part is text/*, we don't need to see the
+ ;; part headers.
+ ((notmuch-match-content-type (plist-get first-part :content-type)
"text/*")
+ #'notmuch-show-insert-header-p-never)
+
+ ;; Otherwise insert the part headers.
+ (t
+ #'notmuch-show-insert-header-p-always
+
(defun notmuch-mua-reply (query-string &optional sender reply-all)
(let ((args '("reply" "--format=sexp" "--format-version=1"))
(process-crypto notmuch-show-process-crypto)
@@ -206,8 +247,8 @@ list."
;; citations, etc. in the original message before
;; quoting.
((notmuch-show-insert-text/plain-hook nil)
- ;; Don't insert part buttons.
- (notmuch-show-insert-header-p-function
#'notmuch-show-insert-header-p-never))
+ ;; Determine how to insert part headers.
+ (notmuch-show-insert-header-p-function
(notmuch-mua-reply-determine-part-function original)))
(notmuch-show-insert-body original (plist-get original
:body) 0)
(buffer-substring-no-properties (point-min) (point-max)
--
2.0.0.rc0
___
notmuch mailing list
[email protected]
http://notmuchmail.org/mailman/listinfo/notmuch
