From: Mark Walters Date: Tue, 13 May 2014 12:05:35 +0000 (+0100) Subject: Re: [PATCH v3 0/9] emacs: Improve the cited message included in replies X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=47f1f15c1611e8423bd2d818a96dd5b2cbb7d833;p=notmuch-archives.git Re: [PATCH v3 0/9] emacs: Improve the cited message included in replies --- diff --git a/00/bfa253bd0ae35bdb8b30ee6734492780c168b2 b/00/bfa253bd0ae35bdb8b30ee6734492780c168b2 new file mode 100644 index 000000000..2ee9a50b6 --- /dev/null +++ b/00/bfa253bd0ae35bdb8b30ee6734492780c168b2 @@ -0,0 +1,198 @@ +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 15E54431FC2 + for ; Tue, 13 May 2014 05:05:49 -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 r6kD6k3nXhCH for ; + Tue, 13 May 2014 05:05:40 -0700 (PDT) +Received: from mail-wi0-f182.google.com (mail-wi0-f182.google.com + [209.85.212.182]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id A4BFC431FBD + for ; Tue, 13 May 2014 05:05:40 -0700 (PDT) +Received: by mail-wi0-f182.google.com with SMTP id r20so460412wiv.3 + for ; Tue, 13 May 2014 05:05:39 -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=R8ogC98n158X26oLlc63Rb84tVT0Mx/FLv6hRrVGOjs=; + b=clIlwUULTjT8EJTnwNFE52fe6dl7ARC8KTshyvSBzhXmBj+TKWuP7XBulkw9REdqzY + FzOQjXMiAIyqEnlX2Ffo7nJ87C/ycqF2i4MbRZD8cUAc3VwawRNbuuou38m0IQrSoHwH + KlL+3JsftAMqXDKn2XrKZhZvcBPB0ZPrW2LPL+K8YoOtM4YLGCylUfCpdBeTZ974iTyM + DVs2+GSQWDEQQqX9+VdP6q/HPF433Tq3RXlK3IrrqO8w/hAsdhkRYeJM8vlvEl+NQykt + SM4xpzZpisVH/Xumlkg3J6It1Gj2qRCLTONUNniDLOWsiNHiLGPlxC3g9MiKkBu4GIrQ + UQAA== +X-Received: by 10.180.212.107 with SMTP id nj11mr21082267wic.40.1399982739332; + Tue, 13 May 2014 05:05:39 -0700 (PDT) +Received: from localhost (5751dfa2.skybroadband.com. [87.81.223.162]) + by mx.google.com with ESMTPSA id + ed6sm21566629wib.20.2014.05.13.05.05.38 for + (version=TLSv1.2 cipher=RC4-SHA bits=128/128); + Tue, 13 May 2014 05:05:38 -0700 (PDT) +From: Mark Walters +To: David Edmondson , notmuch@notmuchmail.org +Subject: Re: [PATCH v3 0/9] emacs: Improve the cited message included in + replies +In-Reply-To: +References: <87sixdujkv.fsf@qmul.ac.uk> + <1399897769-26809-1-git-send-email-dme@dme.org> + <87vbtak5uz.fsf@qmul.ac.uk> + +User-Agent: Notmuch/0.15.2+615~g78e3a93 (http://notmuchmail.org) Emacs/23.4.1 + (x86_64-pc-linux-gnu) +Date: Tue, 13 May 2014 13:05:35 +0100 +Message-ID: <87y4y5j33k.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." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Tue, 13 May 2014 12:05:49 -0000 + + +On Tue, 13 May 2014, David Edmondson wrote: +> Firstly, I don't think that the code resulting from this patch series is +> beyond improvement - the intention was really only that it be better +> than the current implementation. +> +> On Mon, May 12 2014, Mark Walters wrote: +>> On Mon, 12 May 2014, David Edmondson wrote: +>>> emacs: Improve the cited message included in replies +>>> +>>> I tried to do things in small increments to make it easier to review. +>>> +>>> v2: +>>> - Don't run the text/plain hooks when generating the message to quote. +>>> +>>> v3: +>>> - Remove the 'no-button code, as it's no longer used. +>>> - Control the insertion of part headers using a function. +>>> - Fix the tests. +>> +>> I think I broadly like this series. I haven't thought through all the +>> ramifications yet so this is just some first thoughts. I will also send +>> some comments on individual patches. +> +> Thanks! +> +>> In notmuch-show we go to notmuch-show-insert-part-*/* to +>> notmuch-mm-display-part-inline and then leave the decision to inline to +>> mm-inlined-types. I think this means that, by default, we will not +>> inline signatures amongst other things. +> +> The rule is essentially: whatever text would be shown when the message +> is displayed for reading, without any of the washing. + +Yes I think that is a good rule. That means that essentially all my +comments end up being more a (future) feature request that we make +choosing which parts to view in notmuch-show more configurable. + +I do have a slight worry about whether there are any cases that people +will want parts shown in their emacs-show but not in any reply but I +can't think of any. + +>> So at the least I think we should decide whether we want to override +>> mm-inlined-types. +> +> I'm not really clear on the benefits of this. Could you explain? + +This was just the above slight worry. + +>> Alternatively, and in my view preferably, we could have a function or +>> variable of our own which says which parts to include. Indeed, if do +>> it with a function we might be able to make an option to reply to mean +>> "include parts currently shown in the notmuch-show buffer" which might +>> be nice. +> +> That seems over complicated to me. The rule (above) from this series +> is easy to understand and work with. Other mechanisms could be +> implemented later, of course. + +Yes, as you say, leave this for later (if ever). +> +>> There is a related question and possible bug that we might be able to +>> do something about at the same time: should we include text parts in the +>> reply if they have content-disposition attachment? I have been caught +>> about by this on one occasion replying to a message with a 500K log file +>> attached (and notmuch-show/wash becomes very slow with a 500K +>> message!) +> +> This is really a question of what happens in `show' mode. If it is +> currently displaying text parts with content-disposition attachment, +> that sounds like a bug that should be fixed (which would mean that the +> cited message wouldn't include that part either). + +Yes I agree. + +>> Finally, I am not sure whether I like having buttons in the reply. My +>> instinct is against, but they do add context. +> +> The last patch in the series is an example of trying to do the right +> thing - show the part headers when they are necessary for proper +> understanding, but elide them in all other cases. +> +> The mechanism used to do this is pretty crude in the patch. One could +> imagine a better implementation that examines the depth of the part +> tree, etc. + +OK I will try and review these soon. + +Best wishes + +Mark + +> +>> Best wishes +>> +>> Mark +>> +>> +>> +>> +>>> +>>> +>>> David Edmondson (9): +>>> emacs/show: Re-arrange determination if a part header is necessary +>>> emacs/show: Allow the user to decide when part headers should be +>>> inserted +>>> emacs/show: Accommodate the lack of part header buttons +>>> emacs/mua: Generate improved cited text for replies +>>> emacs/show: Remove the 'no-buttons option of +>>> `notmuch-show-insert-bodypart' +>>> emacs/mua: Don't insert part headers in citations +>>> test: Update the test output to accord with the reply changes +>>> emacs/mua: Insert part headers depending on the message +>>> test: Update the test output to accord with more reply changes +>>> +>>> emacs/notmuch-mua.el | 82 +++++++++++++++++++----------- +>>> emacs/notmuch-show.el | 135 +++++++++++++++++++++++++++++++------------------- +>>> test/T310-emacs.sh | 44 ++++++++++++++++ +>>> 3 files changed, 180 insertions(+), 81 deletions(-) +>>> +>>> -- +>>> 2.0.0.rc0 +>>> +>>> _______________________________________________ +>>> notmuch mailing list +>>> notmuch@notmuchmail.org +>>> http://notmuchmail.org/mailman/listinfo/notmuch