Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
authorDavid Edmondson <dme@dme.org>
Tue, 20 Jan 2015 08:44:11 +0000 (08:44 +0000)
committerW. Trevor King <wking@tremily.us>
Sat, 20 Aug 2016 21:47:38 +0000 (14:47 -0700)
7a/7ea84a6547164a2a5ed70728bf99ce487824d9 [new file with mode: 0644]

diff --git a/7a/7ea84a6547164a2a5ed70728bf99ce487824d9 b/7a/7ea84a6547164a2a5ed70728bf99ce487824d9
new file mode 100644 (file)
index 0000000..f53a8d9
--- /dev/null
@@ -0,0 +1,266 @@
+Return-Path: <dme@dme.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id 42B9E431FC9\r
+       for <notmuch@notmuchmail.org>; Tue, 20 Jan 2015 00:44:19 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: 1.739\r
+X-Spam-Level: *\r
+X-Spam-Status: No, score=1.739 tagged_above=-999 required=5\r
+       tests=[DNS_FROM_AHBL_RHSBL=2.438, RCVD_IN_DNSWL_LOW=-0.7,\r
+       UNPARSEABLE_RELAY=0.001] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id KvUQ-JTjsma4 for <notmuch@notmuchmail.org>;\r
+       Tue, 20 Jan 2015 00:44:15 -0800 (PST)\r
+Received: from mail-wg0-f51.google.com (mail-wg0-f51.google.com\r
+ [74.125.82.51])       (using TLSv1 with cipher RC4-SHA (128/128 bits))        (No client\r
+ certificate requested)        by olra.theworths.org (Postfix) with ESMTPS id\r
+ 66070431FC3   for <notmuch@notmuchmail.org>; Tue, 20 Jan 2015 00:44:15 -0800\r
+ (PST)\r
+Received: by mail-wg0-f51.google.com with SMTP id l18so13780922wgh.10\r
+       for <notmuch@notmuchmail.org>; Tue, 20 Jan 2015 00:44:14 -0800 (PST)\r
+X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
+       d=1e100.net; s=20130820;\r
+       h=x-gm-message-state:to:subject:in-reply-to:references:user-agent\r
+       :from:date:message-id:mime-version:content-type;\r
+       bh=G6872WYckl/3/cqunl8SnD7gKU9pmM0MS/bvCKAj2jo=;\r
+       b=YNa/Aqw5V0yniV+E2Q/Sy/6sq1f7/Uy89vY/L4AT688u4bzefVAQ9wQSnomQL7I+6T\r
+       4n6y/yFb4lj1nYcuCfISSMev2q6ZMXTo2BE5k3bpeQ4faqAhpgtcO3rXz9tdRfVVOksP\r
+       NbnpSJUUEweKf6VZd02TaLIE0JoQkBe15gOJH8k9oz6zT1Q/b9AWPIxIqez44JJE3dOZ\r
+       DjJ/T4SoPb50MIRKxkmftKw1xLxKwtVu6GsuJPvCByQO0InDYXMjZDzLJGFChKrO0KtN\r
+       1tQ/J7V8bmncXPOBg3u7h3zKqggRXvl3kiEqC/6NK8ceXpvXzSF4R8J+z8lbyBnGReur\r
+       lPhw==\r
+X-Gm-Message-State:\r
+ ALoCoQmG8I/65CYOjSJoufn/m/wpsEFkWzeDAmtZzGFy+AQmkSP9wXnFOKuemOS2ui/z44+gmFsk\r
+X-Received: by 10.180.182.72 with SMTP id ec8mr3919573wic.53.1421743454278;\r
+       Tue, 20 Jan 2015 00:44:14 -0800 (PST)\r
+Received: from disaster-area.hh.sledj.net\r
+       ([2a01:348:1a2:1:ea39:35ff:fe2c:a227])\r
+       by mx.google.com with ESMTPSA id\r
+       a12sm19587936wjs.10.2015.01.20.00.44.12\r
+       (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\r
+       Tue, 20 Jan 2015 00:44:13 -0800 (PST)\r
+Received: from localhost (30000@localhost [local]);\r
+       by localhost (OpenSMTPD) with ESMTPA id d9281c57;\r
+       Tue, 20 Jan 2015 08:44:11 +0000 (UTC)\r
+To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the\r
+       message\r
+In-Reply-To: <87h9vmk010.fsf@qmul.ac.uk>\r
+References: <87sixdujkv.fsf@qmul.ac.uk>\r
+       <1399897769-26809-1-git-send-email-dme@dme.org>\r
+       <1399897769-26809-9-git-send-email-dme@dme.org>\r
+       <87h9vmk010.fsf@qmul.ac.uk>\r
+User-Agent: none\r
+From: David Edmondson <dme@dme.org>\r
+Date: Tue, 20 Jan 2015 08:44:11 +0000\r
+Message-ID: <cunr3up4zac.fsf@gargravarr.hh.sledj.net>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Tue, 20 Jan 2015 08:44:19 -0000\r
+\r
+On Mon, Jan 19 2015, Mark Walters <markwalters1009@gmail.com> wrote:\r
+> On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:\r
+>> Whether to insert part headers should depend on the details of the\r
+>> message being cited.\r
+>\r
+> Hi\r
+>\r
+> Overall I like this series and it does fix two annoying bugs (not being\r
+> able to reply to ref822 messages and (correctly) including parts\r
+> which have application/octet-stream but are actually text parts).\r
+>\r
+> The one problem is getting the right choice for part headers in the\r
+> reply text. I think getting this wrong will irritate users even if the\r
+> overall result is better.\r
+>\r
+> My guess at the correct logic is:\r
+> 1) omit the part header for any empty part: (ie a part we don't display\r
+> such as a pdf file).\r
+\r
+This seems wrong to me (i.e. it's not what I would want ;-).\r
+\r
+Showing the part header in the reply acknowledges that it was part of\r
+the message that I'm replying to.\r
+\r
+Even more, consider a message:\r
+\r
+  Hi, attached are the two PDF documents that we discussed.\r
+  \r
+  This is the version with Fred's suggested comments:\r
+\r
+  [ application/pdf: document 1 ]\r
+\r
+  This is the version with my proposed alternative edits and much more\r
+  content added:\r
+\r
+  [ application/pdf: document 2 ]\r
+\r
+The part headers form a significant part of the content. In a reply I'd\r
+like to see them, so that I can add comments appropriately. (I realise\r
+that commenting 'in-line' is out of fashion in many places now, but for\r
+more complex discussions I still prefer it.)\r
+\r
+I still like the original rule that I proposed: the reply should include\r
+whatever is in the 'show' buffer, modulo content that was elided due to\r
+washing.\r
+\r
+> 2) omit multipart/* part headers\r
+> 3) include all other part headers\r
+> 4) except omit the first part header (perhaps only in the case it is text/plain)\r
+>\r
+> My reasoning for each is\r
+> 1) there is no point in saying we had a part which we are omitting.\r
+> 2) all the subparts of multipart/* will get there own header which\r
+> should be sufficient.\r
+> 3) we want to keep the parts distinguished\r
+> 4) except we don't need to do that with the first part.\r
+>\r
+> Note for 4) it would be good to have a multipart/alternative with\r
+> subparts text/plain and text/html just give the text/plain with no part\r
+> header.\r
+>\r
+> I include a patch below which does all of these apart from 4) as I\r
+> couldn't see a clean way of implementing it. Any suggestions?\r
+>\r
+> It should apply on top of patch 6 or 7 instead of 8. The key change is\r
+> that it always puts in a button and then deletes it if unwanted: this\r
+> makes doing 1) above easy. \r
+>\r
+> It does break some tests, nothing unexpected except an interaction with\r
+> the way we wash text/plain parts: we remove leading blank lines from the\r
+> first text/plain part (because it doesn't have a button) but not from\r
+> subsequent ones (because they do). Because this code always has the\r
+> second case it doesn't remove a leading blank line of the first part.\r
+>\r
+> Best wishes\r
+>\r
+> Mark\r
+>\r
+>\r
+> From 8f198b38e76e050ae8d20d866748c41ccf79f3d4 Mon Sep 17 00:00:00 2001\r
+> From: Mark Walters <markwalters1009@gmail.com>\r
+> Date: Mon, 19 Jan 2015 14:39:25 +0000\r
+> Subject: [PATCH] emacs show/reply modify part handling\r
+>\r
+> Modify the part handling so that we always insert the button and\r
+> delete it afterwards if not wanted. The advantage is that we can\r
+> decide whether to keep the part button based on what the insertion\r
+> code does. In particular the reply code can omit the button for all\r
+> parts with no displayable content.\r
+> ---\r
+>  emacs/notmuch-mua.el  |  5 +++--\r
+>  emacs/notmuch-show.el | 39 +++++++++++++++++++++++++--------------\r
+>  2 files changed, 28 insertions(+), 16 deletions(-)\r
+>\r
+> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el\r
+> index 0ca9eed..6060f33 100644\r
+> --- a/emacs/notmuch-mua.el\r
+> +++ b/emacs/notmuch-mua.el\r
+> @@ -29,6 +29,7 @@\r
+>  (eval-when-compile (require 'cl))\r
+>  \r
+>  (declare-function notmuch-show-insert-body "notmuch-show" (msg body depth))\r
+> +(declare-function notmuch-show-insert-header-p-reply "notmuch-show" (part empty-part))\r
+>  \r
+>  ;;\r
+>  \r
+> @@ -223,8 +224,8 @@ Note that these functions use `mail-citation-hook' if that is non-nil."\r
+>                    ;; citations, etc. in the original message before\r
+>                    ;; quoting.\r
+>                    ((notmuch-show-insert-text/plain-hook nil)\r
+> -                   ;; Don't insert part buttons.\r
+> -                   (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-never))\r
+> +                   ;; Insert part buttons appropriate for a reply.\r
+> +                   (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-reply))\r
+>                  (notmuch-show-insert-body original (plist-get original :body) 0)\r
+>                  (buffer-substring-no-properties (point-min) (point-max)))))\r
+>  \r
+> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
+> index 4a0899f..2cdb5a8 100644\r
+> --- a/emacs/notmuch-show.el\r
+> +++ b/emacs/notmuch-show.el\r
+> @@ -909,16 +909,24 @@ message at DEPTH in the current thread."\r
+>           "text/x-diff")\r
+>      content-type)))\r
+>  \r
+> -(defun notmuch-show-insert-header-p-smart (part)\r
+> +(defun notmuch-show-insert-header-p-smart (part empty-part)\r
+>    "Return non-NIL if a header button should be inserted for this part."\r
+>    (let ((mime-type (notmuch-show-mime-type part)))\r
+>      (not (and (string= mime-type "text/plain")\r
+>            (<= (plist-get part :id) 1)))))\r
+>  \r
+> -(defun notmuch-show-insert-header-p-always (part)\r
+> +(defun notmuch-show-insert-header-p-reply (part empty-part)\r
+> +  "Return non-NIL if a header button should be inserted for this part."\r
+> +  (let ((mime-type (notmuch-show-mime-type part)))\r
+> +    (not (or empty-part\r
+> +         (notmuch-match-content-type mime-type "multipart/*")\r
+> +         (and (string= mime-type "text/plain")\r
+> +              (<= (plist-get part :id) 1))))))\r
+> +\r
+> +(defun notmuch-show-insert-header-p-always (part empty-part)\r
+>    t)\r
+>  \r
+> -(defun notmuch-show-insert-header-p-never (part)\r
+> +(defun notmuch-show-insert-header-p-never (part empty-part)\r
+>    nil)\r
+>  \r
+>  (defun notmuch-show-insert-bodypart (msg part depth &optional hide)\r
+> @@ -936,8 +944,8 @@ is t, hide the part initially and show the button."\r
+>       (show-part (not (equal hide t)))\r
+>       ;; We omit the part button for the first (or only) part if\r
+>       ;; this is text/plain.\r
+> -     (button (when (funcall notmuch-show-insert-header-p-function part)\r
+> -               (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))\r
+> +     (button-beg (point))\r
+> +     (button (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))\r
+>       (content-beg (point)))\r
+>  \r
+>      ;; Store the computed mime-type for later use (e.g. by attachment handlers).\r
+> @@ -952,15 +960,18 @@ is t, hide the part initially and show the button."\r
+>      ;; Some of the body part handlers leave point somewhere up in the\r
+>      ;; part, so we make sure that we're down at the end.\r
+>      (goto-char (point-max))\r
+> -    ;; Ensure that the part ends with a carriage return.\r
+> -    (unless (bolp)\r
+> -      (insert "\n"))\r
+> -    ;; We do not create the overlay for hidden (lazy) parts until\r
+> -    ;; they are inserted.\r
+> -    (if show-part\r
+> -    (notmuch-show-create-part-overlays button content-beg (point))\r
+> -      (save-excursion\r
+> -    (notmuch-show-toggle-part-invisibility button)))\r
+> +    (let ((empty-part (equal (point) content-beg)))\r
+> +      (if (not (funcall notmuch-show-insert-header-p-function part empty-part))\r
+> +      (delete-region button-beg content-beg)\r
+> +    ;; Ensure that the part ends with a carriage return.\r
+> +    (unless (bolp)\r
+> +      (insert "\n"))\r
+> +    ;; We do not create the overlay for hidden (lazy) parts until\r
+> +    ;; they are inserted.\r
+> +    (if show-part\r
+> +        (notmuch-show-create-part-overlays button content-beg (point))\r
+> +      (save-excursion\r
+> +        (notmuch-show-toggle-part-invisibility button)))))\r
+>      (notmuch-show-record-part-information part beg (point))))\r
+>  \r
+>  (defun notmuch-show-insert-body (msg body depth)\r
+> -- \r
+> 2.1.3\r