[RFC] An EmailMessage class for bzrlib

Adeodato Simó dato at net.com.org.es
Wed Jun 20 17:09:26 BST 2007


* 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?

> > +        try:
> > +            self['Subject'] = Header.Header(unicode(subject))
> > +        except UnicodeDecodeError:
> > +            raise NonAsciiString(subject)

> You are relying on the platform's default encoding being ascii.  It
> might be 'iso-8859-1', for example, and if it is, no bytestring will
> cause a UnicodeDecodeError.

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)

> > +    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.

> 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?

> Yes, it simplifies, but I think it handles 90% of the use cases.

What would be missing?

Cheers,

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
And how do you tell an extroverted mathematician? He looks at *your* shoes
while he's talking to you.




More information about the bazaar mailing list