[RFC] An EmailMessage class for bzrlib

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Adeodato Simó wrote:
> * Aaron Bentley [Wed, 20 Jun 2007 09:20:39 -0400]:
> 
>> Adeodato Simó wrote:
>>> +class EmailMessage(MIMEMultipart.MIMEMultipart, object):
>>> +    """A MIME Multipart email message."""
> 
>> Why are you deriving from object?
> 
> For super(); better not to do it?

Yes.  Also better to not use super unless you actually need it (i.e. you
are doing multiple-inheritance).

> Oh, good point, I'll change that. It'll require a bit of isinstance(),
> because it's not possible to call unicode() over unicode strings with an
> encoding parameter. I guess I'll move it to a separate function:
> 
>     @staticmethod
>     def _basestring_to_unicode(string):
>         """Return an unicode object for string.
> 
>         If string is a non-ASCII byte string, NonAsciiString will be raised.
>         """
>         if isinstance(string, unicode):
>             return string
>         else:
>             try:
>                 return unicode(string, 'ascii')
>             except UnicodeDecodeError:
>                 raise NonAsciiString(string)

This looks like a potential companion to osutils.safe_unicode.  Does
anyone know if we're following this pattern elsewhere?

Maybe it's simpler to just use safe_unicode?

>>> +    def add_text_attachment(self, body, filename=None):
>>> +        """Add a text attachment to the message.
>>> +
>>> +        :param body: A text to attach. Can be an unicode string, and it'll be
>>> +            sent as utf-8, or a byte string, and it'll be sent as 8-bit.
> 
>> It seems surprising to support bytestrings here.  If these are text
>> attachments, shouldn't bytestrings come with an encoding?
> 
> No, I don't think so: diffs are raw 8bit data, with unknown encoding.
> And if the caller knows the encoding, I think it's fair to ask for an
> unicode object.

Well, it's just a bit strange to advertise it as "text", yet accept
arbitrary binary data.  As "add_attachment", it would make sense to me.

diffs aren't really text-- they do not have a guaranteed consistent
encoding.  It's just convenient to present them as if they were.

>> I guess I'd still like to see a one-shot email command, like
> 
>> EmailMessage.send(config, to, from, subject, body, attach=None,
>>                   attach_filename=None)
> 
> I kinda dislike cluttering the constructor, but I can add it. What do
> others think?

I wasn't proposing changing the constructor.  I was proposing a static
method:

    @staticmethod
    def send(config, to, from, subject, body, attach=None,
             attach_filename=None):
        msg = EmailMessage(to, from, subject, body)
        if attach is not None:
            msg.add_text_attachment(attach, attach_filename)
        SMTPConnection(config).send_email(msg)

For me, this is a nice-to-have, not a requirement.

> What would be missing?

I think there might be situations where you'd want to have multiple
attachments, or specify attachment MIME type.  But I am not certain of that.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGeVZt0F+nu1YWqI0RAjb5AJ9AB0aY29U+b32Rh1Rin47NPHSF0QCeL98b
Tvc1YqbMrgWO/KzznieZLUM=
=oaLS
-----END PGP SIGNATURE-----