On Wed, Oct 20, 2010 at 4:51 PM, Yitzchak Gale
Michael Snoyman wrote:
Great news! This is an important package.
It's obviously very preliminary, though. This is not trivial to get right - look at the long and colorful history of the Python email library, detailed on the first page of the library documentation.
Here are some initial suggestions for improvement:
Thanks for all of these comments, you're bringing up a lot of good points. I'll go point-by-point below.
1. The module name Network.Mail.Mime is too generic. There will also be a parser someday. We should have Types, Parse, and Render in separate modules. I expect each of those to grow gradually as more features are added.
If it's all going to live in the same package, I would imagine we'd still want a single module that exports everything anyway. We can worry more about this when we actually have the extra code, but for now I think it would just add another module to maintain, plus an extra few characters people type.
On the other hand, I think I like the decision to re-implement just the features of RFC 2822 message format needed for everyday MIME use rather than building this on top of a more generic message type, as is done in Python. It simplifies things.
But are multi-line headers supported properly? That's trickier than it looks, there have historically been many wrong implementations out there. That itself could be a reason to build this on top of a proper RFC 2822 implementation.
No, multi-line headers are *not* supported. For that matter, I don't believe non-ASCII characters in headers are handled properly either. These would all be welcome patches ;).
2. mailHeaders should have an Ord instance that compares case-insensitively, though the underlying Strings should remain Strings.
The main purpose of this package is simply rendering the messages, not inspecting messages about to be rendered. As such, I'd like to optimize for that case: adding a newtype wrapper here will simply require more typing for users. Someone who wants to do a lookup can always apply a newtype wrapper themselves. By the way, this is the opposite answer I gave regarding the WAI, where inspecting headers is very important. If this is something to be reopened in the future, we should consider the solution used over there (suggested by Gregory Collins), which stores both a regular and lower-cased version of the header.
3. It should be possible to control whether text parts get quoted-printable encoded. Perhaps add QuotedPrintable to Encoding?
Agree, I just didn't have time or need to implement it.
4. I don't like having those sendmail things here in the same module and package. It's convenient, but messy in several ways - creates a spurious dependency on process, only works on certain platforms and even then with possible weird platform dependencies, etc. I think this should be in a separate package. Once there is a nice easy-to-use SMTP companion package, I don't think the sendmail things will be used that much anyway. Put them in a separate package, but mention it in prominent places so people will find it.
I'm not sure that having a process dependency is a problem for anyone. Isn't it included with GHC? Having it there can be *incredibly* useful for development systems, where you may not have a full SMTP server but want to use something like ssmtp, or even a simple shell script that spits the data into a text file (which is what I do). Actually, on this topic, it may be convenient to provide such a "sendmailDebug" function...
5. Is the blaze-builder dependency necessary now that Bryan has built those techniques into Data.Text?
Yes. The rendered mail body isn't text, it's binary. While it's true that actual binary data (like image attachments) should be base64 encoded, there's still the case of the message, which could be encoded any way the user wishes.
6. Are I18N text-encoded headers supported? That's very important, even for simple everyday usage. It's a bit complicated, though. That might be another reason to build on top of a full RFC 2822 implementation.
Agree, just like multi-line headers. But I'm not sure quite how complicated it really is; shouldn't it be a single function that checks for special characters (eg, non-ASCII, newlines) and then escapes them?
7. This very simple interface is great for everyday use. Think about how to add the less common options for more general use, without cluttering up the everyday interface - custom parameters for Content-Type and Content-Disposition, specifying the boundary rather than allowing it to be generated automatically, etc. It's important to think about those kinds of things now, before the interface gets set in cement.
I'm ambivalent about specifying the boundary manually; I can't really think of a use case outside of testing where it's useful. Keep in mind that there could possibly be many boundaries needed for a message. One possibility may be to provide two parameters to the render function, along the lines of "seed" and "seed -> (Boundary, seed)". Michael