Re: [PATCH v3 0/9] emacs: Improve the cited message included in replies

[David Edmondson <dme@dme.org>]

[-- Attachment #1: Type: text/plain, Size: 4129 bytes --]

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 <dme@dme.org> 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.

> 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?

> 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.

> 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).

> 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.

> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 310 bytes --]