unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v5 0/7] emacs: Improve the cited message included in replies
@ 2016-02-14 18:39 Mark Walters
  2016-02-14 18:39 ` [PATCH v5 1/7] emacs/show: Re-arrange determination if a part header is necessary Mark Walters
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Mark Walters @ 2016-02-14 18:39 UTC (permalink / raw)
  To: notmuch

This is a slight update and a rebase of this patch set (previous
version at
id:1446894276-7814-1-git-send-email-markwalters1009@gmail.com )

The only change from last time (apart from fixing rebasing to master)
is the rewording of couple of minor comments inline with dme's review
of the previous version.

Although I am submitting it is primarily dme's series, with minor
updates from me.

One particular motivation for this series is that it fixes a long
standing bug we have that replying to a message with an rfc822 part
completely omits that part. It also fixes the bug whereby we don't
include application/octet-stream parts which are actually text/plain
(and thus are displayed in show mode).

This series makes the reply code use the same code as the show code so
everything works as expected: the reply buffer looks essentially the
same as the show buffer.

There is one slight difference: the user might want different part
headers displayed when replying; both because the audience is
different (a non-notmuch using recipient) and because the buttons
don't "work" (you can't click on them to show or view a part).

Dme and I disagree on which of these we would like to see so make that
customisable.

The key change is patch 3 which switches how reply works. Also note
that patch 2 is almost all whitespace change as the changes modify the
indentation.

Best wishes

Mark


David Edmondson (6):
  emacs/show: Re-arrange determination if a part header is necessary
  emacs/show: Accommodate the lack of part header buttons
  emacs/mua: Generate improved cited text for replies
  emacs/show: Remove the 'no-buttons option of
    `notmuch-show-insert-bodypart'
  emacs/show: Make the insertion of part headers overridable.
  emacs/mua: Let user specify which parts get a header in citations.

Mark Walters (1):
  test: fix the tests for the new reply code

 emacs/notmuch-mua.el  |  60 ++++++++++-----------
 emacs/notmuch-show.el | 144 +++++++++++++++++++++++++++++++-------------------
 test/T310-emacs.sh    |  32 +++++++++++
 test/test-lib.el      |   4 ++
 4 files changed, 156 insertions(+), 84 deletions(-)

-- 
2.1.4

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

* [PATCH v5 1/7] emacs/show: Re-arrange determination if a part header is necessary
  2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
@ 2016-02-14 18:39 ` Mark Walters
  2016-02-14 18:39 ` [PATCH v5 2/7] emacs/show: Accommodate the lack of part header buttons Mark Walters
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Mark Walters @ 2016-02-14 18:39 UTC (permalink / raw)
  To: notmuch

From: David Edmondson <dme@dme.org>

Move the determination of whether a part header is required to a
distinct function.
---
 emacs/notmuch-show.el | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 3345878..70cdb9d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -927,6 +927,22 @@ will return nil if the CID is unknown or cannot be retrieved."
       ;; showable this returns nil.
       (notmuch-show-create-part-overlays button part-beg part-end))))
 
+(defun notmuch-show-mime-type (part)
+  "Return the correct mime-type to use for PART."
+  (let ((content-type (downcase (plist-get part :content-type))))
+    (or (and (string= content-type "application/octet-stream")
+	     (notmuch-show-get-mime-type-of-application/octet-stream part))
+	(and (string= content-type "inline patch")
+	     "text/x-diff")
+	content-type)))
+
+(defun notmuch-show-insert-header-p (part hide)
+  "Return non-NIL if a header button should be inserted for this part."
+  ;; Show all part buttons except for the first part if it is text/plain.
+  (let ((mime-type (notmuch-show-mime-type part)))
+    (not (and (string= mime-type "text/plain")
+	      (<= (plist-get part :id) 1)))))
+
 (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
   "Insert the body part PART at depth DEPTH in the current thread.
 
@@ -937,20 +953,16 @@ is t, hide the part initially and show the button. If HIDE is
 useful for quoting in replies)."
 
   (let* ((content-type (downcase (plist-get part :content-type)))
-	 (mime-type (or (and (string= content-type "application/octet-stream")
-			     (notmuch-show-get-mime-type-of-application/octet-stream part))
-			(and (string= content-type "inline patch")
-			     "text/x-diff")
-			content-type))
+	 (mime-type (notmuch-show-mime-type part))
 	 (nth (plist-get part :id))
 	 (long (and (notmuch-match-content-type mime-type "text/*")
 		    (> notmuch-show-max-text-part-size 0)
 		    (> (length (plist-get part :content)) notmuch-show-max-text-part-size)))
 	 (beg (point))
-	 ;; We omit the part button for the first (or only) part if
-	 ;; this is text/plain, or HIDE is 'no-buttons.
-	 (button (unless (or (equal hide 'no-buttons)
-			     (and (string= mime-type "text/plain") (<= nth 1)))
+	 ;; We show the part button if notmuch-show-insert-header-p
+	 ;; says to, unless HIDE is 'no-buttons.
+	 (button (when (and (not (equal hide 'no-buttons))
+			    (notmuch-show-insert-header-p part hide))
 		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
 	 ;; Hide the part initially if HIDE is t, or if it is too long
 	 ;; and we have a button to allow toggling (thus reply which
-- 
2.1.4

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

* [PATCH v5 2/7] emacs/show: Accommodate the lack of part header buttons
  2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
  2016-02-14 18:39 ` [PATCH v5 1/7] emacs/show: Re-arrange determination if a part header is necessary Mark Walters
@ 2016-02-14 18:39 ` Mark Walters
  2016-02-14 18:39 ` [PATCH v5 3/7] emacs/mua: Generate improved cited text for replies Mark Walters
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Mark Walters @ 2016-02-14 18:39 UTC (permalink / raw)
  To: notmuch

From: David Edmondson <dme@dme.org>

Various pieces of code assumed (reasonably) that part header buttons
are present. Modify them to avoid problems if no part headers were
inserted.
---
 emacs/notmuch-show.el | 88 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 40 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 70cdb9d..37ed74d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -507,36 +507,37 @@ message at DEPTH in the current thread."
 
 (defun notmuch-show-toggle-part-invisibility (&optional button)
   (interactive)
-  (let* ((button (or button (button-at (point))))
-	 (overlay (button-get button 'overlay))
-	 (lazy-part (button-get button :notmuch-lazy-part)))
-    ;; We have a part to toggle if there is an overlay or if there is a lazy part.
-    ;; If neither is present we cannot toggle the part so we just return nil.
-    (when (or overlay lazy-part)
-      (let* ((show (button-get button :notmuch-part-hidden))
-	     (new-start (button-start button))
-	     (button-label (button-get button :base-label))
-	     (old-point (point))
-	     (properties (text-properties-at (button-start button)))
-	     (inhibit-read-only t))
-	;; Toggle the button itself.
-	(button-put button :notmuch-part-hidden (not show))
-	(goto-char new-start)
-	(insert "[ " button-label (if show " ]" " (hidden) ]"))
-	(set-text-properties new-start (point) properties)
-	(let ((old-end (button-end button)))
-	  (move-overlay button new-start (point))
-	  (delete-region (point) old-end))
-	(goto-char (min old-point (1- (button-end button))))
-	;; Return nil if there is a lazy-part, it is empty, and we are
-	;; trying to show it.  In all other cases return t.
-	(if lazy-part
-	    (when show
-	      (button-put button :notmuch-lazy-part nil)
-	      (notmuch-show-lazy-part lazy-part button))
-	  ;; else there must be an overlay.
-	  (overlay-put overlay 'invisible (not show))
-	  t)))))
+  (let ((button (or button (button-at (point)))))
+    (when button
+      (let ((overlay (button-get button 'overlay))
+	    (lazy-part (button-get button :notmuch-lazy-part)))
+	;; We have a part to toggle if there is an overlay or if there is a lazy part.
+	;; If neither is present we cannot toggle the part so we just return nil.
+	(when (or overlay lazy-part)
+	  (let* ((show (button-get button :notmuch-part-hidden))
+		 (new-start (button-start button))
+		 (button-label (button-get button :base-label))
+		 (old-point (point))
+		 (properties (text-properties-at (button-start button)))
+		 (inhibit-read-only t))
+	    ;; Toggle the button itself.
+	    (button-put button :notmuch-part-hidden (not show))
+	    (goto-char new-start)
+	    (insert "[ " button-label (if show " ]" " (hidden) ]"))
+	    (set-text-properties new-start (point) properties)
+	    (let ((old-end (button-end button)))
+	      (move-overlay button new-start (point))
+	      (delete-region (point) old-end))
+	    (goto-char (min old-point (1- (button-end button))))
+	    ;; Return nil if there is a lazy-part, it is empty, and we are
+	    ;; trying to show it.  In all other cases return t.
+	    (if lazy-part
+		(when show
+		  (button-put button :notmuch-lazy-part nil)
+		  (notmuch-show-lazy-part lazy-part button))
+	      ;; else there must be an overlay.
+	      (overlay-put overlay 'invisible (not show))
+	      t)))))))
 
 ;; Part content ID handling
 
@@ -645,14 +646,17 @@ will return nil if the CID is unknown or cannot be retrieved."
   t)
 
 (defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth button)
-  (button-put button 'face 'notmuch-crypto-part-header)
-  ;; add signature status button if sigstatus provided
+  (when button
+    (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."))
+    ;; If we're not adding the signature status, tell the user how
+    ;; they can get it.
+    (when button
+      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
@@ -666,17 +670,20 @@ will return nil if the CID is unknown or cannot be retrieved."
   t)
 
 (defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth button)
-  (button-put button 'face 'notmuch-crypto-part-header)
-  ;; add encryption status button if encstatus specified
+  (when button
+    (button-put button 'face 'notmuch-crypto-part-header))
+  ;; Add encryption status button if encryption status is 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
+	;; Add signature status button if signature status is
+	;; 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
+    ;; If we're not adding the encryption status, 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))
@@ -976,8 +983,9 @@ useful for quoting in replies)."
 
     (if show-part
         (notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
-      (button-put button :notmuch-lazy-part
-                  (list msg part mime-type nth depth button)))
+      (when button
+	(button-put button :notmuch-lazy-part
+		    (list msg part mime-type nth depth button))))
 
     ;; Some of the body part handlers leave point somewhere up in the
     ;; part, so we make sure that we're down at the end.
-- 
2.1.4

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

* [PATCH v5 3/7] emacs/mua: Generate improved cited text for replies
  2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
  2016-02-14 18:39 ` [PATCH v5 1/7] emacs/show: Re-arrange determination if a part header is necessary Mark Walters
  2016-02-14 18:39 ` [PATCH v5 2/7] emacs/show: Accommodate the lack of part header buttons Mark Walters
@ 2016-02-14 18:39 ` Mark Walters
  2016-02-20 13:15   ` David Bremner
  2016-02-14 18:39 ` [PATCH v5 4/7] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart' Mark Walters
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Mark Walters @ 2016-02-14 18:39 UTC (permalink / raw)
  To: notmuch

From: David Edmondson <dme@dme.org>

Use the message display code to generate message text to cite in
replies.
---
 emacs/notmuch-mua.el | 38 ++++++++------------------------------
 1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index d4fad7b..a386d43 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -28,7 +28,7 @@
 
 (eval-when-compile (require 'cl))
 
-(declare-function notmuch-show-insert-bodypart "notmuch-show" (msg part depth &optional hide))
+(declare-function notmuch-show-insert-body "notmuch-show" (msg body depth))
 (declare-function notmuch-fcc-header-setup "notmuch-maildir-fcc" ())
 (declare-function notmuch-fcc-handler "notmuch-maildir-fcc" (destdir))
 
@@ -144,31 +144,6 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
 	else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
 	  do (notmuch-mua-reply-crypto (plist-get part :content))))
 
-(defun notmuch-mua-get-quotable-parts (parts)
-  (loop for part in parts
-	if (notmuch-match-content-type (plist-get part :content-type) "multipart/alternative")
-	  collect (let* ((subparts (plist-get part :content))
-			(types (mapcar (lambda (part) (plist-get part :content-type)) subparts))
-			(chosen-type (car (notmuch-multipart/alternative-choose types))))
-		   (loop for part in (reverse subparts)
-			 if (notmuch-match-content-type (plist-get part :content-type) chosen-type)
-			 return part))
-	else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
-	  append (notmuch-mua-get-quotable-parts (plist-get part :content))
-	else if (notmuch-match-content-type (plist-get part :content-type) "text/*")
-	  collect part))
-
-(defun notmuch-mua-insert-quotable-part (message part)
-  ;; We don't want text properties leaking from the show renderer into
-  ;; the reply so we use a temp buffer. Also we don't want hooks, such
-  ;; as notmuch-wash-*, to be run on the quotable part so we set
-  ;; notmuch-show-insert-text/plain-hook to nil.
-  (insert (with-temp-buffer
-	    (let ((notmuch-show-insert-text/plain-hook nil))
-	      ;; Show the part but do not add buttons.
-	      (notmuch-show-insert-bodypart message part 0 'no-buttons))
-	    (buffer-substring-no-properties (point-min) (point-max)))))
-
 ;; There is a bug in emacs 23's message.el that results in a newline
 ;; not being inserted after the References header, so the next header
 ;; is concatenated to the end of it. This function fixes the problem,
@@ -247,10 +222,13 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
 	(insert "From: " from "\n")
 	(insert "Date: " date "\n\n")
 
-	;; Get the parts of the original message that should be quoted; this includes
-	;; all the text parts, except the non-preferred ones in a multipart/alternative.
-	(let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get original :body))))
-	  (mapc (apply-partially 'notmuch-mua-insert-quotable-part original) quotable-parts))
+	(insert (with-temp-buffer
+		  ;; Don't attempt to clean up messages, excerpt
+		  ;; citations, etc. in the original message before
+		  ;; quoting.
+		  (let ((notmuch-show-insert-text/plain-hook nil))
+		    (notmuch-show-insert-body original (plist-get original :body) 0)
+		    (buffer-substring-no-properties (point-min) (point-max)))))
 
 	(set-mark (point))
 	(goto-char start)
-- 
2.1.4

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

* [PATCH v5 4/7] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart'
  2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
                   ` (2 preceding siblings ...)
  2016-02-14 18:39 ` [PATCH v5 3/7] emacs/mua: Generate improved cited text for replies Mark Walters
@ 2016-02-14 18:39 ` Mark Walters
  2016-02-14 18:39 ` [PATCH v5 5/7] emacs/show: Make the insertion of part headers overridable Mark Walters
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Mark Walters @ 2016-02-14 18:39 UTC (permalink / raw)
  To: notmuch

From: David Edmondson <dme@dme.org>

No code uses the 'no-buttons argument to
`notmuch-show-insert-bodypart', so remove it.
---
 emacs/notmuch-show.el | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 37ed74d..f0bbff0 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -955,9 +955,7 @@ will return nil if the CID is unknown or cannot be retrieved."
 
 HIDE determines whether to show or hide the part and the button
 as follows: If HIDE is nil, show the part and the button. If HIDE
-is t, hide the part initially and show the button. If HIDE is
-'no-buttons, show the part but do not add any buttons (this is
-useful for quoting in replies)."
+is t, hide the part initially and show the button."
 
   (let* ((content-type (downcase (plist-get part :content-type)))
 	 (mime-type (notmuch-show-mime-type part))
@@ -966,14 +964,10 @@ useful for quoting in replies)."
 		    (> notmuch-show-max-text-part-size 0)
 		    (> (length (plist-get part :content)) notmuch-show-max-text-part-size)))
 	 (beg (point))
-	 ;; We show the part button if notmuch-show-insert-header-p
-	 ;; says to, unless HIDE is 'no-buttons.
-	 (button (when (and (not (equal hide 'no-buttons))
-			    (notmuch-show-insert-header-p part hide))
+	 (button (when (notmuch-show-insert-header-p part hide)
 		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
 	 ;; Hide the part initially if HIDE is t, or if it is too long
-	 ;; and we have a button to allow toggling (thus reply which
-	 ;; uses 'no-buttons automatically includes long parts)
+	 ;; and we have a button to allow toggling.
 	 (show-part (not (or (equal hide t)
 			     (and long button))))
 	 (content-beg (point)))
-- 
2.1.4

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

* [PATCH v5 5/7] emacs/show: Make the insertion of part headers overridable.
  2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
                   ` (3 preceding siblings ...)
  2016-02-14 18:39 ` [PATCH v5 4/7] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart' Mark Walters
@ 2016-02-14 18:39 ` Mark Walters
  2016-02-14 18:39 ` [PATCH v5 6/7] emacs/mua: Let user specify which parts get a header in citations Mark Walters
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Mark Walters @ 2016-02-14 18:39 UTC (permalink / raw)
  To: notmuch

From: David Edmondson <dme@dme.org>

This allows callers of notmuch-show-insert-bodypart to use a `let'
binding to override the default function for specifying when part
headers should be inserted.
---
 emacs/notmuch-show.el | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f0bbff0..5654d11 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -943,8 +943,15 @@ will return nil if the CID is unknown or cannot be retrieved."
 	     "text/x-diff")
 	content-type)))
 
+;; The following variable can be overridden by let bindings.
+(defvar notmuch-show-insert-header-p-function 'notmuch-show-insert-header-p
+  "Specify which function decides which part headers get inserted.
+
+The function should take two parameters, PART and HIDE, and
+should return non-NIL if a header button should be inserted for
+this part.")
+
 (defun notmuch-show-insert-header-p (part hide)
-  "Return non-NIL if a header button should be inserted for this part."
   ;; Show all part buttons except for the first part if it is text/plain.
   (let ((mime-type (notmuch-show-mime-type part)))
     (not (and (string= mime-type "text/plain")
@@ -964,7 +971,9 @@ is t, hide the part initially and show the button."
 		    (> notmuch-show-max-text-part-size 0)
 		    (> (length (plist-get part :content)) notmuch-show-max-text-part-size)))
 	 (beg (point))
-	 (button (when (notmuch-show-insert-header-p part hide)
+	 ;; This default header-p function omits the part button for
+	 ;; the first (or only) part if this is text/plain.
+	 (button (when (funcall notmuch-show-insert-header-p-function part hide)
 		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
 	 ;; Hide the part initially if HIDE is t, or if it is too long
 	 ;; and we have a button to allow toggling.
-- 
2.1.4

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

* [PATCH v5 6/7] emacs/mua: Let user specify which parts get a header in citations.
  2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
                   ` (4 preceding siblings ...)
  2016-02-14 18:39 ` [PATCH v5 5/7] emacs/show: Make the insertion of part headers overridable Mark Walters
@ 2016-02-14 18:39 ` Mark Walters
  2016-02-14 18:39 ` [PATCH v5 7/7] test: fix the tests for the new reply code Mark Walters
  2016-02-14 19:45 ` [PATCH v5 0/7] emacs: Improve the cited message included in replies Tomi Ollila
  7 siblings, 0 replies; 11+ messages in thread
From: Mark Walters @ 2016-02-14 18:39 UTC (permalink / raw)
  To: notmuch

From: David Edmondson <dme@dme.org>

Add a customizable function specifying which parts get a header when
replying, and give some sensible possiblities. These are,

1) all parts except multipart/*. (Subparts of a multipart part do
receive a header button.)

2) only included text/* parts.

3) Exactly as in the show buffer.

4) None at all. This means the reply contains a mish-mash of all the
original message's parts.
---
 emacs/notmuch-mua.el  | 30 ++++++++++++++++++++++++++----
 emacs/notmuch-show.el | 13 +++++++++++++
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index a386d43..fcb3e95 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -93,6 +93,23 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
   :link '(custom-manual "(message)Insertion Variables")
   :group 'notmuch-reply)
 
+(defcustom notmuch-mua-reply-insert-header-p-function
+  'notmuch-show-reply-insert-header-p-trimmed
+  "Function to decide which parts get a header when replying.
+
+This function specifies which parts of a mime message with
+mutiple parts get a header."
+  :type '(radio (const :tag "All except multipart/* and hidden parts"
+			       notmuch-show-reply-insert-header-p-trimmed)
+		(const :tag "Only for included text parts"
+			       notmuch-show-reply-insert-header-p-minimal)
+		(const :tag "Exactly as in show view"
+			       notmuch-show-insert-header-p)
+		(const :tag "No part headers"
+			       notmuch-show-reply-insert-header-p-never)
+		(function :tag "Other"))
+  :group 'notmuch-reply)
+
 ;;
 
 (defun notmuch-mua-get-switch-function ()
@@ -223,10 +240,15 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
 	(insert "Date: " date "\n\n")
 
 	(insert (with-temp-buffer
-		  ;; Don't attempt to clean up messages, excerpt
-		  ;; citations, etc. in the original message before
-		  ;; quoting.
-		  (let ((notmuch-show-insert-text/plain-hook nil))
+		  (let
+		      ;; Don't attempt to clean up messages, excerpt
+		      ;; citations, etc. in the original message before
+		      ;; quoting.
+		      ((notmuch-show-insert-text/plain-hook nil)
+		       ;; Don't omit long parts.
+		       (notmuch-show-max-text-part-size 0)
+		       ;; Insert headers for parts as appropriate for replying.
+		       (notmuch-show-insert-header-p-function notmuch-mua-reply-insert-header-p-function))
 		    (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 5654d11..ec58cac 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -957,6 +957,19 @@ this part.")
     (not (and (string= mime-type "text/plain")
 	      (<= (plist-get part :id) 1)))))
 
+(defun notmuch-show-reply-insert-header-p-never (part hide)
+  nil)
+
+(defun notmuch-show-reply-insert-header-p-trimmed (part hide)
+  (let ((mime-type (notmuch-show-mime-type part)))
+    (and (not (notmuch-match-content-type mime-type "multipart/*"))
+	 (not hide))))
+
+(defun notmuch-show-reply-insert-header-p-minimal (part hide)
+  (let ((mime-type (notmuch-show-mime-type part)))
+    (and (notmuch-match-content-type mime-type "text/*")
+	 (not hide))))
+
 (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
   "Insert the body part PART at depth DEPTH in the current thread.
 
-- 
2.1.4

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

* [PATCH v5 7/7] test: fix the tests for the new reply code
  2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
                   ` (5 preceding siblings ...)
  2016-02-14 18:39 ` [PATCH v5 6/7] emacs/mua: Let user specify which parts get a header in citations Mark Walters
@ 2016-02-14 18:39 ` Mark Walters
  2016-02-14 19:45 ` [PATCH v5 0/7] emacs: Improve the cited message included in replies Tomi Ollila
  7 siblings, 0 replies; 11+ messages in thread
From: Mark Walters @ 2016-02-14 18:39 UTC (permalink / raw)
  To: notmuch

This sets the part-insertion code to never insert part headers (as we
didn't before).

With that change there is only one failing test: this test has a text
part (an email message) listed as application/octet-stream. Notmuch
show displays this part, but the reply code omitted it as it had type
application/octet-stream. The new code correctly includes it. Thus
update the expected output to match.
---
 test/T310-emacs.sh | 32 ++++++++++++++++++++++++++++++++
 test/test-lib.el   |  4 ++++
 2 files changed, 36 insertions(+)

diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 61bc369..22ca71c 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -473,6 +473,38 @@ Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
 > and http://mail-index.netbsd.org/pkgsrc-bugs/2006/06/07/msg016808.htmlspecifically
 > uses 64 as the
 > buffer size.
+> From e3bc4bbd7b9d0d086816ab5f8f2d6ffea1dd3ea4 Mon Sep 17 00:00:00 2001
+> From: Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+> Date: Tue, 17 Nov 2009 11:30:39 -0800
+> Subject: [PATCH] Deal with situation where sysconf(_SC_GETPW_R_SIZE_MAX) returns -1
+>
+> ---
+>  notmuch-config.c |    2 ++
+>  1 files changed, 2 insertions(+), 0 deletions(-)
+>
+> diff --git a/notmuch-config.c b/notmuch-config.c
+> index 248149c..e7220d8 100644
+> --- a/notmuch-config.c
+> +++ b/notmuch-config.c
+> @@ -77,6 +77,7 @@ static char *
+>  get_name_from_passwd_file (void *ctx)
+>  {
+>      long pw_buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
+> +    if (pw_buf_size == -1) pw_buf_size = 64;
+>      char *pw_buf = talloc_zero_size (ctx, pw_buf_size);
+>      struct passwd passwd, *ignored;
+>      char *name;
+> @@ -101,6 +102,7 @@ static char *
+>  get_username_from_passwd_file (void *ctx)
+>  {
+>      long pw_buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
+> +    if (pw_buf_size == -1) pw_buf_size = 64;
+>      char *pw_buf = talloc_zero_size (ctx, pw_buf_size);
+>      struct passwd passwd, *ignored;
+>      char *name;
+> -- 
+> 1.6.5.2
+>
 > _______________________________________________
 > notmuch mailing list
 > notmuch@notmuchmail.org
diff --git a/test/test-lib.el b/test/test-lib.el
index 596a705..02e020c 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -184,6 +184,10 @@ nothing."
 (setq notmuch-tag-deleted-formats
       '((".*" nil)))
 
+;; For historical reasonse we don't print part headers when replying
+;; in the tests suite
+(setq notmuch-mua-reply-insert-header-p-function 'notmuch-show-reply-insert-header-p-never)
+
 ;; force a common html renderer, to avoid test variations between
 ;; environments
 
-- 
2.1.4

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

* Re: [PATCH v5 0/7] emacs: Improve the cited message included in replies
  2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
                   ` (6 preceding siblings ...)
  2016-02-14 18:39 ` [PATCH v5 7/7] test: fix the tests for the new reply code Mark Walters
@ 2016-02-14 19:45 ` Tomi Ollila
  2016-02-17 10:51   ` David Edmondson
  7 siblings, 1 reply; 11+ messages in thread
From: Tomi Ollila @ 2016-02-14 19:45 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sun, Feb 14 2016, Mark Walters <markwalters1009@gmail.com> wrote:

> This is a slight update and a rebase of this patch set (previous
> version at
> id:1446894276-7814-1-git-send-email-markwalters1009@gmail.com )
>
> The only change from last time (apart from fixing rebasing to master)
> is the rewording of couple of minor comments inline with dme's review
> of the previous version.
>
> Although I am submitting it is primarily dme's series, with minor
> updates from me.
>
> One particular motivation for this series is that it fixes a long
> standing bug we have that replying to a message with an rfc822 part
> completely omits that part. It also fixes the bug whereby we don't
> include application/octet-stream parts which are actually text/plain
> (and thus are displayed in show mode).
>
> This series makes the reply code use the same code as the show code so
> everything works as expected: the reply buffer looks essentially the
> same as the show buffer.
>
> There is one slight difference: the user might want different part
> headers displayed when replying; both because the audience is
> different (a non-notmuch using recipient) and because the buttons
> don't "work" (you can't click on them to show or view a part).
>
> Dme and I disagree on which of these we would like to see so make that
> customisable.

Does *NOT* look bad, works and (relevant) tests pass.

We'd need at least David to verify that Mark got his (David's that is)
patches properly rebased...

Tomi

>
> The key change is patch 3 which switches how reply works. Also note
> that patch 2 is almost all whitespace change as the changes modify the
> indentation.
>
> Best wishes
>
> Mark
>
>
> David Edmondson (6):
>   emacs/show: Re-arrange determination if a part header is necessary
>   emacs/show: Accommodate the lack of part header buttons
>   emacs/mua: Generate improved cited text for replies
>   emacs/show: Remove the 'no-buttons option of
>     `notmuch-show-insert-bodypart'
>   emacs/show: Make the insertion of part headers overridable.
>   emacs/mua: Let user specify which parts get a header in citations.
>
> Mark Walters (1):
>   test: fix the tests for the new reply code
>
>  emacs/notmuch-mua.el  |  60 ++++++++++-----------
>  emacs/notmuch-show.el | 144 +++++++++++++++++++++++++++++++-------------------
>  test/T310-emacs.sh    |  32 +++++++++++
>  test/test-lib.el      |   4 ++
>  4 files changed, 156 insertions(+), 84 deletions(-)
>
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH v5 0/7] emacs: Improve the cited message included in replies
  2016-02-14 19:45 ` [PATCH v5 0/7] emacs: Improve the cited message included in replies Tomi Ollila
@ 2016-02-17 10:51   ` David Edmondson
  0 siblings, 0 replies; 11+ messages in thread
From: David Edmondson @ 2016-02-17 10:51 UTC (permalink / raw)
  To: Tomi Ollila, Mark Walters, notmuch

On Sun, Feb 14 2016, Tomi Ollila wrote:
> We'd need at least David to verify that Mark got his (David's that is)
> patches properly rebased...

I think that I already said "okay!" for these.

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

* Re: [PATCH v5 3/7] emacs/mua: Generate improved cited text for replies
  2016-02-14 18:39 ` [PATCH v5 3/7] emacs/mua: Generate improved cited text for replies Mark Walters
@ 2016-02-20 13:15   ` David Bremner
  0 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2016-02-20 13:15 UTC (permalink / raw)
  To: Mark Walters, notmuch

Mark Walters <markwalters1009@gmail.com> writes:

> From: David Edmondson <dme@dme.org>
>
> Use the message display code to generate message text to cite in
> replies.

I think some tests need updating for this change. I get 3 failures in
T310-emacs. If I read the output correctly, the problem is inserting new
button text into the output. If someone is touching T310 note that most
of the file comparisons seem to be "OUTPUT EXPECTED" rather than
"EXPECTED OUTPUT".

d

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

end of thread, other threads:[~2016-02-20 13:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-14 18:39 [PATCH v5 0/7] emacs: Improve the cited message included in replies Mark Walters
2016-02-14 18:39 ` [PATCH v5 1/7] emacs/show: Re-arrange determination if a part header is necessary Mark Walters
2016-02-14 18:39 ` [PATCH v5 2/7] emacs/show: Accommodate the lack of part header buttons Mark Walters
2016-02-14 18:39 ` [PATCH v5 3/7] emacs/mua: Generate improved cited text for replies Mark Walters
2016-02-20 13:15   ` David Bremner
2016-02-14 18:39 ` [PATCH v5 4/7] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart' Mark Walters
2016-02-14 18:39 ` [PATCH v5 5/7] emacs/show: Make the insertion of part headers overridable Mark Walters
2016-02-14 18:39 ` [PATCH v5 6/7] emacs/mua: Let user specify which parts get a header in citations Mark Walters
2016-02-14 18:39 ` [PATCH v5 7/7] test: fix the tests for the new reply code Mark Walters
2016-02-14 19:45 ` [PATCH v5 0/7] emacs: Improve the cited message included in replies Tomi Ollila
2016-02-17 10:51   ` David Edmondson

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