Frederick888 / external-editor-revived

External Editor Revived is a Thunderbird MailExtension which allows editing emails in programs such as Vim, Neovim, Emacs, etc.
GNU General Public License v3.0
110 stars 6 forks source link

Respect flowed format (missing trailing spaces) #48

Open VincentCordobes opened 2 years ago

VincentCordobes commented 2 years ago

Description

I noticed trailing spaces are removed. They are important as they mean softwrap when writing Content-Type: text/plain; format=flowed, which is the default for plain text in thunderbird. The consequence is that the text is not flowed.

Note trailing spaces are correctly preserved on quoted lines

Environment

Configuration

Editor: neovim configured for format=flowed emails shell: zsh terminal: kitty

Frederick888 commented 2 years ago

Could you share a your TB configs and a few screenshots? An example email can be handy too. The fix should be simple but I'm not sure about the expected outcome since I don't use flowed format usually.

I just reset wrap/flowed settings:

name value
mail.wrap_long_lines true
mailnews.wraplength 72
plain_text.wrap_long_lines true
mailnews.display.disable_format_flowed_support false
mailnews.send_plaintext_flowed true

And now my emails also have that format=flowed header. So IIUC this should allow recipients' email clients to rewrap my email visually, despite having hard line wraps, based on their devices' viewport widths?

I tried sending a few and resizing TB's window but failed to see any differences.

Frederick888 commented 2 years ago

The fix should be https://github.com/Frederick888/external-editor-revived/pull/49. would be nice if you can test it out (and let me know how this actually makes a difference...).

VincentCordobes commented 2 years ago

Thanks. I just tested #49 and unfortunately it does not fix it :( I feel like TB reformat the text after receiving it from external-editor-revived.

My configuration

name value
mail.wrap_long_lines true
mailnews.wraplength 72
plain_text.wrap_long_lines true
mailnews.display.disable_format_flowed_support false
mailnews.send_plaintext_flowed true

Here is an example of what is expected

This very very long line contains a trailing space right after this word 
and it should certainly be wrapped and flowed.

The message contains a trailing space after word, represented by the - character in my editor. I used mutt and vim, but the behaviour is the same when you type a long line in TB editor. The source mail should contain the trailing space.

message_in_editor

After saving it in drafts, it should be reflowed like this:

flowed_message_in_tb_wide_screen

On small screen :

flowed_message_in_tb_small_screen

Also on my phone mail app:

phone
Frederick888 commented 2 years ago

I did some test and it seems that you have to physically type into TB editor for it to preserve trailing spaces and make the email 'flow'.

If you just copy the text from e.g. the preformatted block you just shared and paste it into TB, TB'll remove trailing spaces:

image image

And the sent email won't flow:

image

So I guess the compose API I use works in the same way as how pasting from clipboard works, and I'm afraid this is not something I can fix on my end.

PS: I'll merge #49 anyway since it's still a sensible change.

Frederick888 commented 2 years ago

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1780196.

jobisoft commented 4 months ago

Hi,

I would like to discuss how this should be solved. I think I reach more people here, then in the bug.

What currently happens: Every line break in a string used in setComposeDetails(<tabId>, {plainTextBody: <string>}) is interpreted as a hard line break. Even if the line ends with a space.

If the user has enabled flowed output and a certain wrap limit, the composer should be in charge of "flowing" the text. If you use a very long string, and the user has requested flowed output, the editor will correctly flow the text from the following commands:

let [{ id }] = await browser.tabs.query({type:"messageCompose"});
await browser.compose.setComposeDetails(id, {plainTextBody: "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Duis autem vel eum iriure dolor in hendrerit in vulputate velit esse molestie consequat, vel illum dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te feugait nulla facilisi. Lorem ipsum dolor sit amet."})

What should happen if the input data already has flowed content, and maybe even the wrong wrap limit?

I see three options:

  1. Require the add-on to provide clean unflowed data (the add-on has to remove \n from the input data)
  2. Let setComposeDetails clean up the input data
  3. Let setComposeDetails clean up the input data only if the composer is set to use flowed output

Option 2 has the risk of doing something automatically, which may be undesired. Maybe the user wanted to have a space? Option 3 may have the risk of not being deterministic, because the add-on does not know if the composer is using flowed output.

Note: getComposeDetails always returns unflowed data.

jobisoft commented 4 months ago

With option 3, the composer will correctly reflow any flowed input, and keep the soft line breaks (without the extra space), if the user has disabled flowed output.

This will honor what the user typed into the external editor, something which is not possible with option 1. Except if we expose a property PLAINTEXT_FLOWED, which allows the add-on to check the current flow setting.

To minimize automatic adjustments to input data, which could have unwanted side effects for other add-ons, I currently would opt for option 1 and adding a PLAINTEXT_FLOWED property.

Frederick888 commented 4 months ago

@jobisoft Thanks for reaching out! It's getting late here where I live, and as mentioned above I don't usually use flowed format, so I'll need a little time to think it over.

But it seems that all three options involve sanitising the whole email body when flowed output is on, then leaving the flowing work to the composer. May I know, putting Thunderbird composer's behaviour aside, whether flowed format itself can support mixed formats in one email? For example,

This is a long paragraph where flowed format should apply. Pellentesque⌴
pellentesque lobortis ligula nec pellentesque. In placerat dui sit amet⌴
pretium egestas.

Now some code:

fn very_very_very_super_super_super_tremendously_surprisingly_long_long_name() -> u32 {  // don't wrap this line!
    // hello, world!
}

PS: I'll be AFK soon. No rush to get back to me :)

jobisoft commented 4 months ago

Flowed output was invented to be able to write nice looking emails and follow the rules of SMTP, which require line breaks after a certain amount of chars.

So in the early days, where people read the actual mail which came in over the wire (like on the server), they got used to this hard requirement to have line breaks and a width of 72 chars per line. On many modern clients that looks strange, because the available space is not used.

This was fixed by flowed, quoted printable or html. For html, line breaks do not matter at all, but you need an html capable client to read the mail.

With quoted printable, the message basically gets encoded. It can still be read as-is, but it is not fun.

Flowed adds soft line breaks, which are inserted by the client before the message is sent, to comply with the SMTP protocol. The receiving client can remove these soft line breaks again and use the available space to display the message (a.k.a. reflow).

So you need to switch the perspective. Not the user is adding these soft line breaks, but the client. The above message will get soft line breaks in the line which should not be wrapped. And the receiving client will remove them again (when it is showing the message, not in the source of the message).

For your add-on that means: Do not include soft line breaks in the data which you are using in setComposeDetails(). The editor will take care of that.

What could be interpreted as a bug: The editor does not understand soft line breaks when the content is added. It interprets them has hard line breaks. One could assume that the editor should remove them and reflow the text. But I do not think we can change that, as that would have a lot of consequences for other unrelated scenarios. We could clean-up the input data just for setComposeDetails(), but as I said, I do not want to temper with input data. My current proposal is that the add-on should not use soft line breaks, which are supposed to be added by the client just before sending.

What you need to ask your users: What is their goal, when they provide an input text with soft line breaks?

So after re-thinking this after writing that paragraph: Exposing PLAINTEXT_FLOWED will probably not help you in any way. Your add-on should simply ignore soft line breaks with body.replaceAll(" \n", " ");

Frederick888 commented 3 months ago

@jobisoft Thanks for the background info :)

On many modern clients that looks strange, because the available space is not used.

Personally I actually dislike long lines. I find them mentally suffocating. It's like reading a very long sentence without catching a breath.

In terminals it's even worse: Like any good Vim user, I run my terminal in full-screen mode. If there are no line breaks, a line can really go from the very left edge of my screen to the very right. There are no side bars or padding. And maybe because I read much more code than emails in my terminal, I also find long lines in emails unpleasant like I do in code.

So as I mentioned, usually I don't use the flowed format. I just go with hard line breaks, no matter which language I'm writing in. In some cultures this even seems to be the standard practice, for example almost all Japanese emails I get use hard line breaks.

What you need to ask your users: What is their goal, when they provide an input text with soft line breaks?

So yeah, I'm not sure how well I can represent EER's users (feedback welcome!), but it looks like that we've got a bunch who should just be content with hard line breaks. Why bother with flowed format?

Well, here's my personal answer: Mobile!

Smartphone screens are usually narrower, and hard wrapped emails look absolutely awful on them. For example, here are two screenshots of a random email I just found: image 2024-05-22 21 08 10

So the ideal setup for me, especially when sending emails to non-tech people, is:

To achieve this goal, I'll need

  1. PLAINTEXT_FLOWED exposed a. TB/EER can do body.replaceAll(" \n", " ") when flowed format is enabled b. TB/EER doesn't do this when flowed format is off, to cater for the audience who absolutely don't want EER or TB to tamper with their emails, e.g. people who send patches c. Additionally, it'd be really, really great if PLAINTEXT_FLOWED can be used in setComposeDetails() too, so that users can toggle this in external editors. It doesn't have to be a per-message setting. A global switch would be good enough. It'll help me with #50 and potentially also the 827 Toggle Line Wrap users d. Additionally, instead of a boolean flag, it'd be nice if TB can expose the wrap length instead. It'll allow users to adjust their editor settings accordingly, or change it via setComposeDetails()
  2. Flowed email bodies exposed a. So that users can see their emails in the same layout in TB composer and external editors b. Yes, theoretically EER can do the wrapping if the wrap length is available, but practically it's quite difficult due to things like multi-code-point emojis, multi-language contents (languages with & without spaces together specifically), URLs especially with non-ASCII characters, etc. I did a quick test and TB did an excellent job on this (thanks to Firefox?), so it'd be great if EER can borrow this superpower of TB's

Btw I was a little confused by 'With option 3, the composer will correctly reflow any flowed input, and keep the soft line breaks (without the extra space), if the user has disabled flowed output.'

What does 'reflow' mean? And why can't we keep the extra spaces? Shouldn't emails just be sent as they are when flowed output is off?

jobisoft commented 3 months ago

So the ideal setup for me, especially when sending emails to non-tech people, is:

In Thunderbird: send out emails in the flowed format, but enable mailnews.display.disable_format_flowed_support

This will display the messages you receive without a reflow, you see the soft line breaks just like they are in the source of the message.

In Neovim: edit emails with line breaks, and rely on set fo+=w to add the trailing > spaces

Trailing spaces (soft line breaks) is a concept interpreted by the receiving client, not by the sending editor. The sending client will apply soft line breaks according to its settings when it prepares the message to be sent. While you type the message, Thunderbirds plain text editor enforces a wrap limit, where the text is forced onto the next line, but that does not introduce line breaks. Copy such a wrapped line into a different editor, and you will see, it is just one line. Only when the message is sent, these line breaks will become real line breaks, either hard or soft, depending on your mailnews.send_plaintext_flowed setting.

Apparently, you want to use an external editor AND you do not want to type from one side of the screen to the other. If you really cannot tell Neovim to wrap the content at a specific width (without adding real line breaks), which matches your visual preference when typing messages (and I cannot believe that neovim does not support soft wrapping) then you have to continue to manually add soft line breaks to your message, but you need to programmatically remove them before adding them to Thunderbirds composer.

It is not possible to change the output flowed setting on a per-message basis. I do not think there is a real benefit of exposing PLAINTEXT_FLOWED. Either it is supported, and your clean data will get the correct soft line breaks, or it is not supported and your clean data will get the correct hard line breaks.

jobisoft commented 3 months ago

b. TB/EER doesn't do this when flowed format is off, to cater for the audience who absolutely don't want EER or TB to tamper with their emails, e.g. people who send patches

The issue with patches is, they collide with SMTP enforced line breaks. So it is impossible to send patches with longer lines without those lines getting line breaks in the source of the message. A receiving email client will of course always display the correct patch.

c. Additionally, it'd be really, really great if PLAINTEXT_FLOWED can be used in setComposeDetails() too, so that users can toggle this in external editors. It doesn't have to be a per-message setting. A global switch would be good enough. It'll help me with https://github.com/Frederick888/external-editor-revived/issues/50 and potentially also the 827 Toggle Line Wrap users

A global switch will have global consequences. That may have unwanted side effect, when add-ons can change this setting. I will think about it.

jobisoft commented 3 months ago

Btw I was a little confused by 'With option 3, the composer will correctly reflow any flowed input, and keep the soft line breaks (without the extra space), if the user has disabled flowed output.'

I did some tests with this.

If flowed output is on, ´setComposeDetails()` could remove all soft line breaks, which will reflow the input data according to Thunderbirds wrap settings. If flowed output is off and we do not remove the soft line breaks, these breaks will become hard breaks and will collide with the auto wrapping of Thunderbirds editor. This will produce ridged text. So I no longer suggest this.

My suggestion is to always remove soft line breaks before using setComposeDetails().

Frederick888 commented 3 months ago

In Thunderbird: send out emails in the flowed format, but enable mailnews.display.disable_format_flowed_support

This will display the messages you receive without a reflow, you see the soft line breaks just like they are in the source of the message.

Yup that's exactly what I want.

Trailing spaces (soft line breaks) is a concept interpreted by the receiving client, not by the sending editor. The sending client will apply soft line breaks according to its settings when it prepares the message to be sent.

Ok I probably got trapped in the Mutt mindset, where IIRC set fo+=w to do trailing spaces in Neovim is the standard practice. (Don't quote me on this as I barely ever used Mutt/NeoMutt, but a quick search seemed to confirm this.)

If you really cannot tell Neovim to wrap the content at a specific width (without adding real line breaks), which matches your visual preference when typing messages

In Neovim I can probably set wrap & set column=72 or leverage plugins like https://github.com/rickhowe/wrapwidth, but I can't speak for other editors.

I do not think there is a real benefit of exposing PLAINTEXT_FLOWED. Either it is supported, and your clean data will get the correct soft line breaks, or it is not supported and your clean data will get the correct hard line breaks.

I'd like to offer a 'Flowed format support' setting in EER which does body.replaceAll(" \n", " ") if and only if flowed format is enabled in TB.

Otherwise if a user has switched off flowed format to use hard line breaks, and there just happens to be one line that ends with a space for some reason, EER will incorrectly remove the following line break. This is especially important for the frequent-flowed-format-flippers who use EER and Toggle Line Wrap together, since if EER doesn't automatically stop doing body.replaceAll(" \n", " "), they will have to manually turn EER's own flowed format support off after disabling TB's flowed format with Toggle Line Wrap's keyboard shortcut, which obviously is not great.

c. Additionally, it'd be really, really great if PLAINTEXT_FLOWED can be used in setComposeDetails() too, so that users can toggle this in external editors. It doesn't have to be a per-message setting. A global switch would be good enough.

A global switch will have global consequences. That may have unwanted side effect, when add-ons can change this setting. I will think about it.

Of course a per-message setting is better, but requires more effort too. Users right now have to use Toggle Line Wrap which injects an experimental API into TB that works on the global level, so an upgrade is an upgrade ¯\_(ツ)_/¯ (Btw if you do go down this path, maybe it's worth looping TLW's author https://github.com/jan-kiszka in.)