From: Mark Walters Date: Mon, 19 Jan 2015 20:06:51 +0000 (+0000) Subject: Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=1ac354a2953e73eda55dfa3e9a9a91f6455ce6b7;p=notmuch-archives.git Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message --- diff --git a/5c/7217a4a4dbe540bbd96e0b880b194806cd0eb1 b/5c/7217a4a4dbe540bbd96e0b880b194806cd0eb1 new file mode 100644 index 000000000..d42a4fd2d --- /dev/null +++ b/5c/7217a4a4dbe540bbd96e0b880b194806cd0eb1 @@ -0,0 +1,241 @@ +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 285DD431FBF + for ; Mon, 19 Jan 2015 12:06:45 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 1.34 +X-Spam-Level: * +X-Spam-Status: No, score=1.34 tagged_above=-999 required=5 + tests=[DKIM_ADSP_CUSTOM_MED=0.001, DNS_FROM_AHBL_RHSBL=2.438, + FREEMAIL_FROM=0.001, NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] + 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 tCxYbcCPum0r for ; + Mon, 19 Jan 2015 12:06:42 -0800 (PST) +Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 8BF8F431FB6 + for ; Mon, 19 Jan 2015 12:06:41 -0800 (PST) +Received: from smtp.qmul.ac.uk ([138.37.6.40]) + by mail2.qmul.ac.uk with esmtp (Exim 4.71) + (envelope-from ) + id 1YDIan-00089h-8F; Mon, 19 Jan 2015 20:06:35 +0000 +Received: from 5751dfa2.skybroadband.com ([87.81.223.162] helo=localhost) + by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71) + (envelope-from ) + id 1YDIam-0006iC-US; Mon, 19 Jan 2015 20:06:33 +0000 +From: Mark Walters +To: David Edmondson , notmuch@notmuchmail.org +Subject: Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the + message +In-Reply-To: <1399897769-26809-9-git-send-email-dme@dme.org> +References: <87sixdujkv.fsf@qmul.ac.uk> + <1399897769-26809-1-git-send-email-dme@dme.org> + <1399897769-26809-9-git-send-email-dme@dme.org> +User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/24.4.1 + (x86_64-pc-linux-gnu) +Date: Mon, 19 Jan 2015 20:06:51 +0000 +Message-ID: <87h9vmk010.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain +X-Sender-Host-Address: 87.81.223.162 +X-QM-Geographic: According to ripencc, + this message was delivered by a machine in Britain (UK) (GB). +X-QM-SPAM-Info: Sender has good ham record. :) +X-QM-Body-MD5: 4265275bf0d9b37ac3af025d02a6e314 (of first 20000 bytes) +X-SpamAssassin-Score: -0.1 +X-SpamAssassin-SpamBar: / +X-SpamAssassin-Report: The QM spam filters have analysed this message to + determine if it is + spam. We require at least 5.0 points to mark a message as spam. + This message scored -0.1 points. + Summary of the scoring: + * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail + provider * (markwalters1009[at]gmail.com) + * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay + * domain + * -0.1 AWL AWL: Adjusted score from AWL reputation of From: address +X-QM-Scan-Virus: ClamAV says the message is clean +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: Mon, 19 Jan 2015 20:06:45 -0000 + + +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). +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 + +