From 48d8b2057a8936427dda0dcf2e3aa2e9a9d0d506 Mon Sep 17 00:00:00 2001 From: David Edmondson Date: Tue, 20 Jan 2015 08:44:11 +0000 Subject: [PATCH] Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message --- 7a/7ea84a6547164a2a5ed70728bf99ce487824d9 | 266 ++++++++++++++++++++++ 1 file changed, 266 insertions(+) create mode 100644 7a/7ea84a6547164a2a5ed70728bf99ce487824d9 diff --git a/7a/7ea84a6547164a2a5ed70728bf99ce487824d9 b/7a/7ea84a6547164a2a5ed70728bf99ce487824d9 new file mode 100644 index 000000000..f53a8d9cd --- /dev/null +++ b/7a/7ea84a6547164a2a5ed70728bf99ce487824d9 @@ -0,0 +1,266 @@ +Return-Path: +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 42B9E431FC9 + for ; Tue, 20 Jan 2015 00:44:19 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 1.739 +X-Spam-Level: * +X-Spam-Status: No, score=1.739 tagged_above=-999 required=5 + tests=[DNS_FROM_AHBL_RHSBL=2.438, RCVD_IN_DNSWL_LOW=-0.7, + UNPARSEABLE_RELAY=0.001] 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 KvUQ-JTjsma4 for ; + Tue, 20 Jan 2015 00:44:15 -0800 (PST) +Received: from mail-wg0-f51.google.com (mail-wg0-f51.google.com + [74.125.82.51]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client + certificate requested) by olra.theworths.org (Postfix) with ESMTPS id + 66070431FC3 for ; Tue, 20 Jan 2015 00:44:15 -0800 + (PST) +Received: by mail-wg0-f51.google.com with SMTP id l18so13780922wgh.10 + for ; Tue, 20 Jan 2015 00:44:14 -0800 (PST) +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=1e100.net; s=20130820; + h=x-gm-message-state:to:subject:in-reply-to:references:user-agent + :from:date:message-id:mime-version:content-type; + bh=G6872WYckl/3/cqunl8SnD7gKU9pmM0MS/bvCKAj2jo=; + b=YNa/Aqw5V0yniV+E2Q/Sy/6sq1f7/Uy89vY/L4AT688u4bzefVAQ9wQSnomQL7I+6T + 4n6y/yFb4lj1nYcuCfISSMev2q6ZMXTo2BE5k3bpeQ4faqAhpgtcO3rXz9tdRfVVOksP + NbnpSJUUEweKf6VZd02TaLIE0JoQkBe15gOJH8k9oz6zT1Q/b9AWPIxIqez44JJE3dOZ + DjJ/T4SoPb50MIRKxkmftKw1xLxKwtVu6GsuJPvCByQO0InDYXMjZDzLJGFChKrO0KtN + 1tQ/J7V8bmncXPOBg3u7h3zKqggRXvl3kiEqC/6NK8ceXpvXzSF4R8J+z8lbyBnGReur + lPhw== +X-Gm-Message-State: + ALoCoQmG8I/65CYOjSJoufn/m/wpsEFkWzeDAmtZzGFy+AQmkSP9wXnFOKuemOS2ui/z44+gmFsk +X-Received: by 10.180.182.72 with SMTP id ec8mr3919573wic.53.1421743454278; + Tue, 20 Jan 2015 00:44:14 -0800 (PST) +Received: from disaster-area.hh.sledj.net + ([2a01:348:1a2:1:ea39:35ff:fe2c:a227]) + by mx.google.com with ESMTPSA id + a12sm19587936wjs.10.2015.01.20.00.44.12 + (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); + Tue, 20 Jan 2015 00:44:13 -0800 (PST) +Received: from localhost (30000@localhost [local]); + by localhost (OpenSMTPD) with ESMTPA id d9281c57; + Tue, 20 Jan 2015 08:44:11 +0000 (UTC) +To: Mark Walters , notmuch@notmuchmail.org +Subject: Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the + message +In-Reply-To: <87h9vmk010.fsf@qmul.ac.uk> +References: <87sixdujkv.fsf@qmul.ac.uk> + <1399897769-26809-1-git-send-email-dme@dme.org> + <1399897769-26809-9-git-send-email-dme@dme.org> + <87h9vmk010.fsf@qmul.ac.uk> +User-Agent: none +From: David Edmondson +Date: Tue, 20 Jan 2015 08:44:11 +0000 +Message-ID: +MIME-Version: 1.0 +Content-Type: text/plain +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Tue, 20 Jan 2015 08:44:19 -0000 + +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 +0000 +> 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 show the button." +> (show-part (not (equal hide t))) +> ;; We omit the part button for the first (or only) part if +> ;; this is text/plain. +> - (button (when (funcall notmuch-show-insert-header-p-function part) +> - (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))) +> + (button-beg (point)) +> + (button (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))) +> (content-beg (point))) +> +> ;; Store the computed mime-type for later use (e.g. by attachment handlers). +> @@ -952,15 +960,18 @@ is t, hide the part initially and show the 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. +> (goto-char (point-max)) +> - ;; Ensure that the part ends with a carriage return. +> - (unless (bolp) +> - (insert "\n")) +> - ;; We do not create the overlay for hidden (lazy) parts until +> - ;; they are inserted. +> - (if show-part +> - (notmuch-show-create-part-overlays button content-beg (point)) +> - (save-excursion +> - (notmuch-show-toggle-part-invisibility button))) +> + (let ((empty-part (equal (point) content-beg))) +> + (if (not (funcall notmuch-show-insert-header-p-function part empty-part)) +> + (delete-region button-beg content-beg) +> + ;; Ensure that the part ends with a carriage return. +> + (unless (bolp) +> + (insert "\n")) +> + ;; We do not create the overlay for hidden (lazy) parts until +> + ;; they are inserted. +> + (if show-part +> + (notmuch-show-create-part-overlays button content-beg (point)) +> + (save-excursion +> + (notmuch-show-toggle-part-invisibility button))))) +> (notmuch-show-record-part-information part beg (point)))) +> +> (defun notmuch-show-insert-body (msg body depth) +> -- +> 2.1.3 -- 2.26.2