[PATCH v2] emacs: Improve the cited message included in replies

[David Edmondson]

On Mon, May 12 2014, Mark Walters wrote:
>>> Secondly, the existing code only includes text sub-parts of the
>>> message. I would think your version might include any sub-parts show is
>>> configured to display, including, say images. (However, in my testing
>>> images didn't seem to be included: I am not sure why.)
>>
>> Eek.
>
> I phrased this badly. I don't want images included: I just couldn't see
> why they weren't with your current patch (as they are displayed in show
> mode)

`buffer-substring-no-properties' grabs the text of the rendered buffer
without any properties, and images are inserted using properties.

You could replace it with `buffer-substring' to see the images in the
reply buffer (but see previous comments about what happens when you
send).

> I guess my concern with adding to the show code is that is already
> more complicated than I would like (invisibility, hidden parts, lazy
> parts etc).

One complex piece of code that knows how to iterate over messages
display the parts seems enough - why have two?
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 310 bytes
Desc: not available
URL: 

[PATCH v2] emacs: Improve the cited message included in replies

[Mark Walters]

On Mon, 12 May 2014, David Edmondson  wrote:
> On Sat, May 10 2014, Mark Walters wrote:
>> On Thu, 08 May 2014, David Edmondson  wrote:
>>> emacs: Improve the cited message included in replies
>>>
>>> v2:
>>> - Don't run the text/plain hooks when generating the message to quote.
>>>
>>
>> In principle I like this approach: keeping show and reply closely linked
>> seems good.
>>
>> At the moment, as you say, the tests don't all pass. The first reason is
>> that this puts in buttons for the parts. Stopping that happening is not
>> completely trivial as we need to make sure that the instruction gets
>> passed down to sub-parts of multiparts etc. (You could argue that the
>> 'no-button option to notmuch-show-insert-bodypart is buggy as it only
>> stops the top level button for the part)
>
> That seems straightforward. I was sure that there was a variable listing
> part types that didn't require buttons (which could be let-bound), but I
> must have been imagining it.
>
> Inserting the button text for the trivial case (single text/plain part)
> is already special-cased in the show code. In the case where only a
> single part is being shown (e.g. multipart/alternative with text/plain
> and text/html with the text/html hidden) it would make sense _not_ to
> show the button text. For more complex messages (e.g. multipart/mixed
> with text/plain and message/rfc822, where the message/rfc822 contains
> multiple parts), showing the button text seems useful to allow the
> different sub-sections of the reply to be distinguished.

Yes that is true: I hadn't thought of that.

> Perhaps there is an approach based on the complexity of the quoted
> message that should determine whether the button text is inserted (which
> might also apply (in modified form?) in normal message display)? Normal
> message display has to allow for some interaction with the parts
> (show/hide, etc.), which doesn't apply to citation.
>
>> Secondly, the existing code only includes text sub-parts of the
>> message. I would think your version might include any sub-parts show is
>> configured to display, including, say images. (However, in my testing
>> images didn't seem to be included: I am not sure why.)
>
> Eek.

I phrased this badly. I don't want images included: I just couldn't see
why they weren't with your current patch (as they are displayed in show
mode)

>
> Inserting the images in the reply buffer itself would not be
> difficult. What should happen when the user hits 'send'? We currently
> don't have a composition mode that would allow us to generate useful
> output in that case. Adding one feels like a lot of work. In many cases
> it would be necessary to transform the message into HTML to properly
> represent the content.
>
> MML (the markup used in `message-mode') is really not designed for
> something this complex.
>
>> I can't tell how much work it is to modify show to take account of these
>> things, so am not sure if this is the best approach, or just adding
>> something to deal with rfc822 to our existing reply code is easier.
>
> The approach in this patch mostly involves removing code - adding
> special case code to notmuch-mua.el to support message/rfc822 involves
> _adding_ a bunch more complex code (I tried that first).

I guess my concern with adding to the show code is that is already more
complicated than I would like (invisibility, hidden parts, lazy parts
etc). But I am very definitely interested in seeing what a more finished
version of your patch would look like.

Best wishes

Mark

[PATCH v2] emacs: Improve the cited message included in replies

[David Edmondson]

On Sat, May 10 2014, Mark Walters wrote:
> On Thu, 08 May 2014, David Edmondson  wrote:
>> emacs: Improve the cited message included in replies
>>
>> v2:
>> - Don't run the text/plain hooks when generating the message to quote.
>>
>
> In principle I like this approach: keeping show and reply closely linked
> seems good.
>
> At the moment, as you say, the tests don't all pass. The first reason is
> that this puts in buttons for the parts. Stopping that happening is not
> completely trivial as we need to make sure that the instruction gets
> passed down to sub-parts of multiparts etc. (You could argue that the
> 'no-button option to notmuch-show-insert-bodypart is buggy as it only
> stops the top level button for the part)

That seems straightforward. I was sure that there was a variable listing
part types that didn't require buttons (which could be let-bound), but I
must have been imagining it.

Inserting the button text for the trivial case (single text/plain part)
is already special-cased in the show code. In the case where only a
single part is being shown (e.g. multipart/alternative with text/plain
and text/html with the text/html hidden) it would make sense _not_ to
show the button text. For more complex messages (e.g. multipart/mixed
with text/plain and message/rfc822, where the message/rfc822 contains
multiple parts), showing the button text seems useful to allow the
different sub-sections of the reply to be distinguished.

Perhaps there is an approach based on the complexity of the quoted
message that should determine whether the button text is inserted (which
might also apply (in modified form?) in normal message display)? Normal
message display has to allow for some interaction with the parts
(show/hide, etc.), which doesn't apply to citation.

> Secondly, the existing code only includes text sub-parts of the
> message. I would think your version might include any sub-parts show is
> configured to display, including, say images. (However, in my testing
> images didn't seem to be included: I am not sure why.)

Eek.

Inserting the images in the reply buffer itself would not be
difficult. What should happen when the user hits 'send'? We currently
don't have a composition mode that would allow us to generate useful
output in that case. Adding one feels like a lot of work. In many cases
it would be necessary to transform the message into HTML to properly
represent the content.

MML (the markup used in `message-mode') is really not designed for
something this complex.

> I can't tell how much work it is to modify show to take account of these
> things, so am not sure if this is the best approach, or just adding
> something to deal with rfc822 to our existing reply code is easier.

The approach in this patch mostly involves removing code - adding
special case code to notmuch-mua.el to support message/rfc822 involves
_adding_ a bunch more complex code (I tried that first).
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 310 bytes
Desc: not available
URL: 

Re: [PATCH v2] emacs: Improve the cited message included in replies

[David Edmondson]

On Mon, May 12 2014, Mark Walters wrote:
>>> Secondly, the existing code only includes text sub-parts of the
>>> message. I would think your version might include any sub-parts show is
>>> configured to display, including, say images. (However, in my testing
>>> images didn't seem to be included: I am not sure why.)
>>
>> Eek.
>
> I phrased this badly. I don't want images included: I just couldn't see
> why they weren't with your current patch (as they are displayed in show
> mode)

`buffer-substring-no-properties' grabs the text of the rendered buffer
without any properties, and images are inserted using properties.

You could replace it with `buffer-substring' to see the images in the
reply buffer (but see previous comments about what happens when you
send).

> I guess my concern with adding to the show code is that is already
> more complicated than I would like (invisibility, hidden parts, lazy
> parts etc).

One complex piece of code that knows how to iterate over messages
display the parts seems enough - why have two?


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2] emacs: Improve the cited message included in replies

[Mark Walters]

On Mon, 12 May 2014, David Edmondson  wrote:
> On Sat, May 10 2014, Mark Walters wrote:
>> On Thu, 08 May 2014, David Edmondson  wrote:
>>> emacs: Improve the cited message included in replies
>>>
>>> v2:
>>> - Don't run the text/plain hooks when generating the message to quote.
>>>
>>
>> In principle I like this approach: keeping show and reply closely linked
>> seems good.
>>
>> At the moment, as you say, the tests don't all pass. The first reason is
>> that this puts in buttons for the parts. Stopping that happening is not
>> completely trivial as we need to make sure that the instruction gets
>> passed down to sub-parts of multiparts etc. (You could argue that the
>> 'no-button option to notmuch-show-insert-bodypart is buggy as it only
>> stops the top level button for the part)
>
> That seems straightforward. I was sure that there was a variable listing
> part types that didn't require buttons (which could be let-bound), but I
> must have been imagining it.
>
> Inserting the button text for the trivial case (single text/plain part)
> is already special-cased in the show code. In the case where only a
> single part is being shown (e.g. multipart/alternative with text/plain
> and text/html with the text/html hidden) it would make sense _not_ to
> show the button text. For more complex messages (e.g. multipart/mixed
> with text/plain and message/rfc822, where the message/rfc822 contains
> multiple parts), showing the button text seems useful to allow the
> different sub-sections of the reply to be distinguished.

Yes that is true: I hadn't thought of that.

> Perhaps there is an approach based on the complexity of the quoted
> message that should determine whether the button text is inserted (which
> might also apply (in modified form?) in normal message display)? Normal
> message display has to allow for some interaction with the parts
> (show/hide, etc.), which doesn't apply to citation.
>
>> Secondly, the existing code only includes text sub-parts of the
>> message. I would think your version might include any sub-parts show is
>> configured to display, including, say images. (However, in my testing
>> images didn't seem to be included: I am not sure why.)
>
> Eek.

I phrased this badly. I don't want images included: I just couldn't see
why they weren't with your current patch (as they are displayed in show
mode)

>
> Inserting the images in the reply buffer itself would not be
> difficult. What should happen when the user hits 'send'? We currently
> don't have a composition mode that would allow us to generate useful
> output in that case. Adding one feels like a lot of work. In many cases
> it would be necessary to transform the message into HTML to properly
> represent the content.
>
> MML (the markup used in `message-mode') is really not designed for
> something this complex.
>
>> I can't tell how much work it is to modify show to take account of these
>> things, so am not sure if this is the best approach, or just adding
>> something to deal with rfc822 to our existing reply code is easier.
>
> The approach in this patch mostly involves removing code - adding
> special case code to notmuch-mua.el to support message/rfc822 involves
> _adding_ a bunch more complex code (I tried that first).

I guess my concern with adding to the show code is that is already more
complicated than I would like (invisibility, hidden parts, lazy parts
etc). But I am very definitely interested in seeing what a more finished
version of your patch would look like.

Best wishes

Mark

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2] emacs: Improve the cited message included in replies

[David Edmondson]

On Sat, May 10 2014, Mark Walters wrote:
> On Thu, 08 May 2014, David Edmondson  wrote:
>> emacs: Improve the cited message included in replies
>>
>> v2:
>> - Don't run the text/plain hooks when generating the message to quote.
>>
>
> In principle I like this approach: keeping show and reply closely linked
> seems good.
>
> At the moment, as you say, the tests don't all pass. The first reason is
> that this puts in buttons for the parts. Stopping that happening is not
> completely trivial as we need to make sure that the instruction gets
> passed down to sub-parts of multiparts etc. (You could argue that the
> 'no-button option to notmuch-show-insert-bodypart is buggy as it only
> stops the top level button for the part)

That seems straightforward. I was sure that there was a variable listing
part types that didn't require buttons (which could be let-bound), but I
must have been imagining it.

Inserting the button text for the trivial case (single text/plain part)
is already special-cased in the show code. In the case where only a
single part is being shown (e.g. multipart/alternative with text/plain
and text/html with the text/html hidden) it would make sense _not_ to
show the button text. For more complex messages (e.g. multipart/mixed
with text/plain and message/rfc822, where the message/rfc822 contains
multiple parts), showing the button text seems useful to allow the
different sub-sections of the reply to be distinguished.

Perhaps there is an approach based on the complexity of the quoted
message that should determine whether the button text is inserted (which
might also apply (in modified form?) in normal message display)? Normal
message display has to allow for some interaction with the parts
(show/hide, etc.), which doesn't apply to citation.

> Secondly, the existing code only includes text sub-parts of the
> message. I would think your version might include any sub-parts show is
> configured to display, including, say images. (However, in my testing
> images didn't seem to be included: I am not sure why.)

Eek.

Inserting the images in the reply buffer itself would not be
difficult. What should happen when the user hits 'send'? We currently
don't have a composition mode that would allow us to generate useful
output in that case. Adding one feels like a lot of work. In many cases
it would be necessary to transform the message into HTML to properly
represent the content.

MML (the markup used in `message-mode') is really not designed for
something this complex.

> I can't tell how much work it is to modify show to take account of these
> things, so am not sure if this is the best approach, or just adding
> something to deal with rfc822 to our existing reply code is easier.

The approach in this patch mostly involves removing code - adding
special case code to notmuch-mua.el to support message/rfc822 involves
_adding_ a bunch more complex code (I tried that first).


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2] emacs: Improve the cited message included in replies

[Mark Walters]

On Thu, 08 May 2014, David Edmondson  wrote:
> emacs: Improve the cited message included in replies
>
> v2:
> - Don't run the text/plain hooks when generating the message to quote.
>

In principle I like this approach: keeping show and reply closely linked
seems good.

At the moment, as you say, the tests don't all pass. The first reason is
that this puts in buttons for the parts. Stopping that happening is not
completely trivial as we need to make sure that the instruction gets
passed down to sub-parts of multiparts etc. (You could argue that the
'no-button option to notmuch-show-insert-bodypart is buggy as it only
stops the top level button for the part)

Secondly, the existing code only includes text sub-parts of the
message. I would think your version might include any sub-parts show is
configured to display, including, say images. (However, in my testing
images didn't seem to be included: I am not sure why.)

I can't tell how much work it is to modify show to take account of these
things, so am not sure if this is the best approach, or just adding
something to deal with rfc822 to our existing reply code is easier.

Best wishes

Mark

Re: [PATCH v2] emacs: Improve the cited message included in replies

[Mark Walters]

On Thu, 08 May 2014, David Edmondson  wrote:
> emacs: Improve the cited message included in replies
>
> v2:
> - Don't run the text/plain hooks when generating the message to quote.
>

In principle I like this approach: keeping show and reply closely linked
seems good.

At the moment, as you say, the tests don't all pass. The first reason is
that this puts in buttons for the parts. Stopping that happening is not
completely trivial as we need to make sure that the instruction gets
passed down to sub-parts of multiparts etc. (You could argue that the
'no-button option to notmuch-show-insert-bodypart is buggy as it only
stops the top level button for the part)

Secondly, the existing code only includes text sub-parts of the
message. I would think your version might include any sub-parts show is
configured to display, including, say images. (However, in my testing
images didn't seem to be included: I am not sure why.)

I can't tell how much work it is to modify show to take account of these
things, so am not sure if this is the best approach, or just adding
something to deal with rfc822 to our existing reply code is easier.

Best wishes

Mark

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2] emacs: Improve the cited message included in replies

[David Edmondson]

emacs: Improve the cited message included in replies

v2:
- Don't run the text/plain hooks when generating the message to quote.


David Edmondson (1):
  emacs/notmuch-mua: Generate improved cited text for replies

 emacs/notmuch-mua.el | 38 --
 1 file changed, 8 insertions(+), 30 deletions(-)

-- 
2.0.0.rc0

[PATCH v2] emacs: Improve the cited message included in replies

[David Edmondson]

emacs: Improve the cited message included in replies

v2:
- Don't run the text/plain hooks when generating the message to quote.


David Edmondson (1):
  emacs/notmuch-mua: Generate improved cited text for replies

 emacs/notmuch-mua.el | 38 --
 1 file changed, 8 insertions(+), 30 deletions(-)

-- 
2.0.0.rc0

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch