Discussion about mail_refactor

[soywod]

Envelope

I'm a bit suspicious about the name "Envelope". Theoretically, the envelope is the container of the message. The struct looks more like headers for me. I checked what you put in the doc (about the Envelope of the crate imap_proto), and it matches well indeed. But imap_proto and himalaya are not on the same level: imap_proto is really low level, whereas himalaya is at the top level. The meaning of Envelope is not really the same. The 2 options I see so far is:

• Rename Envelope (sth like Headers for example)
• Use the imap_proto struct directly instead of creating our own one

Feel free to argue of course, maybe I miss some information.

Signature in envelope

I do not think the signature should be there. Also (it's not fully related to what you propose, it was already the case before), I'm half satisfied with how the signature is processed. It's a bit overwhelming. We never know when to append \n, how many of them etc. I thought about introducing a function like append_signature that takes in parameter a mutable body and appends the signature if exists. Feel free to propose btw.

Signature position in a forward message

I see that you put the signature before the quote in a forward message, what are your motivations? I think it should be after the quote instead. The default (and recommanded) option in Thunderbird is below the quote:

I may add more points in the future.

PS: don't think I'm bashing your work, in fact you did quite well (like the tests based on RFC examples, the try_from etc). I just list here all points that I partially agree with 😉

[soywod]

Once you reply to this message, do you allow me to propose a PR with some changes so we can agree on a compromise?

[TornaxO7]

Once you reply to this message, do you allow me to propose a PR with some changes so we can agree on a compromise?

Of course

Envelope

Rename Envelope (sth like Headers for example)
Use the imap_proto struct directly instead of creating our own one

I'd prefer the first option because in my opinion the envelope of imap_proto doesn't include 'enough' information. For example how do we mnage custom_headers? Should we create a struct which includes the imap_proto-envelope struct + an extra attribute? Also the encoding isn't part of the imap_proto-envelope. Headers sounds nice to me :D

Signature in Envelope

I can understand what you mean. This problem also came up to me, when I started implementing the change_to_reply method. My thoughts were like: "Ok, so first of course, it should use all attributes of Envelope an-... hold on, the signature is in their as well..." You idea with append_signature sounds reasonable to me. I'd suggest to apply this function to Msg.

Signature position in a forward message

I see that you put the signature before the quote in a forward message, what are your motivations?

Let's use this msg-body as an example:

Dear Soywod,
Himalaya is cool!

-- 
Your sincereley
TornaxO7

Now suppose you'd like to forward my message. If I understood it right it should like this (according to thunderbird):

Dear Other Person,
TornaxO7 sent me this!

---------- Forwarded Message ----------
Dear Soywod,
Himalaya is cool!

-- 
Your Sincereley
TornaxO7

-- 
Your Sincereley
Soywod

Now suppose this message would be forwarded again and again. The list at the bottom would be longer and longer.
What my intention is as follows:

Dear Other Person,
TornaxO7 sent me this!

-- 
Your Sincereley
Soywod

---------- Forwarded Message ----------
Dear Soywod,
Himalaya is cool!

-- 
Your Sincereley
TornaxO7

In my opinion you can clearly see the section of your message and my message. That's what my though was.

PS: don't think I'm bashing your work, in fact you did quite well (like the tests based on RFC examples, the try_from etc). I just list here all points that I partially agree with wink

Don't worry! I'm open for constructive criticism! I can't produce "perfect" programs and as you see in my answers, I wasn't quite happy with my "solutions" as well ;) Thank you for your kind words :)

[soywod]

Now suppose this message would be forwarded again and again. The list at the bottom would be longer and longer.

For this, there is a feature in Thunderbird that removes signature from previous messages to avoid this use case. I can't really find if it's a Thunderbird initiative or if it comes from a RFC, but I like it!

[TornaxO7]

For this, there is a feature in Thunderbird that removes signature from previous messages to avoid this use case. I can't really find if it's a Thunderbird initiative or if it comes from a RFC, but I like it!

Oh, I didn't knew that. Hm... but this is gonna be hard to implement since everyone could have their own "style" for a signature so I'm a little bit unsure how to detect the signature in order to remove it. Do you really want to do the same as Thunderbird?

[soywod]

Maybe not so hard as we could think. For sure it will not work for someone who uses a custom signature delimiter. In fact the space after the -- may be for the purpose of finding easily signatures! I will take care of this, I have an idea on how to impl it.

[TornaxO7]

@soywod how far are you? Which points can I pick up? I think after this issue, you could merge my PR mail_refactor unless we don't find anything new xD

[soywod]

I would love to take time to implement the feature, but I have a project to finish before the 30th of August for one of my customer 😞 I was thinking to put this aside (maybe in a dedicated issue), and take time to do this properly after the mail_refactor merge (so I don't block you for the rest), what do you think?

[TornaxO7]

nono, you don't need to put your current issue aside. I can do this as well, so the three of us are benefiting of this ;)

[TornaxO7]

I'll wait for your feedback of the current test-fixer PR. Because I'd like to have my working tests first xD

[soywod]

nono, you don't need to put your current issue aside. I can do this as well, so the three of us are benefiting of this ;)

Alright 😄

[TornaxO7]

Hm.... could you please explain a little bit more about your signature idea? Because I'm stuck at the moment what you mean because currently we are adding the signature to the body anyhow so what's the benefit of an appending-signature function?

[soywod]

My idea was to simplify the signature management. For now, the signature() function returns an Option<String>. Everywhere we need the signature, we need to do some checks (does it exist? do we need to add a \n? how many of them? etc). I was thinking to keep this logic inside a function that takes a body in parameter and return a signed body (if signature exists, with the correct amount of \n etc). But it's not a priority, it works well now. You can skip, I will take care of this refactor once your PR merged!

[soywod]

What's remaining btw from your side?

[TornaxO7]

Nothing at this moment. I think everything has been implemented now! This issue is the last one. I'd go once again through the code and notifiy you if I couldn't find anything special now. After this you could merge it then! c(^-^)c

[soywod]

Great, I'm waiting for your go then. I will also check once again to be sure then we can merge 🎉

[soywod]

I created 2 issues on my repo about the 2 points we mentioned earlier (not to forget them).