Return-Path: <markwalters1009@gmail.com>
X-Original-To: notmuch@notmuchmail.org
Delivered-To: notmuch@notmuchmail.org
Received: from localhost (localhost [127.0.0.1])
	by olra.theworths.org (Postfix) with ESMTP id 1F93D431FBD
	for <notmuch@notmuchmail.org>; Mon, 12 May 2014 15:30:42 -0700 (PDT)
X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
X-Spam-Flag: NO
X-Spam-Score: 0.201
X-Spam-Level: 
X-Spam-Status: No, score=0.201 tagged_above=-999 required=5
	tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,
	FREEMAIL_ENVFROM_END_DIGIT=1, FREEMAIL_FROM=0.001,
	RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled
Received: from olra.theworths.org ([127.0.0.1])
	by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id 5AOvAhbf1rHh for <notmuch@notmuchmail.org>;
	Mon, 12 May 2014 15:30:34 -0700 (PDT)
Received: from mail-we0-f179.google.com (mail-we0-f179.google.com
	[74.125.82.179]) (using TLSv1 with cipher RC4-SHA (128/128 bits))
	(No client certificate requested)
	by olra.theworths.org (Postfix) with ESMTPS id DBDD7431FAF
	for <notmuch@notmuchmail.org>; Mon, 12 May 2014 15:30:33 -0700 (PDT)
Received: by mail-we0-f179.google.com with SMTP id q59so7424529wes.24
	for <notmuch@notmuchmail.org>; Mon, 12 May 2014 15:30:32 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;
	h=from:to:subject:in-reply-to:references:user-agent:date:message-id
	:mime-version:content-type;
	bh=IAwrBxB0lehoNuzsY1r/2BYt05z4YErfjvibABCoivY=;
	b=dltqxc9KAJfXInhsQXrYn4+kc9N59JgXGLzpXNDIIk33t7XwMYcxNti9+eJB612E9U
	P7AaojkUhGcEqMeFrMgM+WuKU62U+z3RlDvEoNNalHSEvmFcWgN5COnnjFD/BW46yyMo
	bktvYnHFmy4gECKDsp5gFqy/Y5xgjRW3XYwMM1FJGWqpFjjsCArBNoDfSSX4wXRFnD8t
	iB26ge1lEAzTE4xWVeE012V/jsXboPKgTx8XYeOk0aLcmELxtRjgBZea4LfPRD4HgLGQ
	7XVmx/Z4LQIXAsS+XwxNzpTdXZ4LNiZIhcoqY688qj9lmKGWdYu7QomgKD2Alt9urZW7
	jLqA==
X-Received: by 10.180.92.103 with SMTP id cl7mr2870880wib.26.1399933832780;
	Mon, 12 May 2014 15:30:32 -0700 (PDT)
Received: from localhost (5751dfa2.skybroadband.com. [87.81.223.162])
	by mx.google.com with ESMTPSA id c12sm3550893wib.13.2014.05.12.15.30.31
	for <multiple recipients>
	(version=TLSv1.2 cipher=RC4-SHA bits=128/128);
	Mon, 12 May 2014 15:30:32 -0700 (PDT)
From: Mark Walters <markwalters1009@gmail.com>
To: David Edmondson <dme@dme.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH v3 4/9] emacs/mua: Generate improved cited text for
 replies
In-Reply-To: <1399897769-26809-5-git-send-email-dme@dme.org>
References: <87sixdujkv.fsf@qmul.ac.uk>
	<1399897769-26809-1-git-send-email-dme@dme.org>
	<1399897769-26809-5-git-send-email-dme@dme.org>
User-Agent: Notmuch/0.15.2+615~g78e3a93 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Mon, 12 May 2014 23:30:29 +0100
Message-ID: <87egzyk4u2.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-BeenThere: notmuch@notmuchmail.org
X-Mailman-Version: 2.1.13
Precedence: list
List-Id: "Use and development of the notmuch mail system."
	<notmuch.notmuchmail.org>
List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
List-Archive: <http://notmuchmail.org/pipermail/notmuch>
List-Post: <mailto:notmuch@notmuchmail.org>
List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=subscribe>
X-List-Received-Date: Mon, 12 May 2014 22:30:42 -0000


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