Re: [PATCH v3 4/9] emacs/mua: Generate improved cited text for replies

Subject: Re: [PATCH v3 4/9] emacs/mua: Generate improved cited text for replies

Date: Mon, 12 May 2014 23:30:29 +0100

To: David Edmondson, notmuch@notmuchmail.org

Cc:

From: Mark Walters


On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> Use the message display code to generate message text to cite in
> replies.

So this is the key change. I am trying to work out what the actual
changes are here: in your commit message for the test update 7/9 you say
that the old code only output the first part. My impression of the
deleted code is that that is not the case (but I don't grok cl very
well).

I think the test change is because in show we do some content-type
guessing of application/octet-stream which the below doesn't do.

But we may also have some things with mm-inlined-types as mentioned in
my earlier reply. Anyway if you can point out any other cases where it
is changed that would be great.

If you go for a function deciding which parts to include then it might
be possible to have a midpoint where we are the same as before, and then
tweak the function to get whatever behaviour we think is best. That
might make it easy to see what is tidying/unification and what is
enhancement.

Incidentally, thank you for splitting this series so finely: I did find
that made it a lot easier to review.

Best wishes

Mark


> ---
>  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 95e4a4d..09c922f 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))
>  
>  ;;
>  
> @@ -123,31 +123,6 @@ list."
>  	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,
> @@ -225,10 +200,13 @@ list."
>  	(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.0.0.rc0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Thread: