[MERGE] An EmailMessage class for bzrlib (v3)

[Adeodato Simó]

* Martin Pool [Tue, 10 Jul 2007 00:49:48 -0400]:

> Martin Pool has voted -0.
> Status is now: Waiting
> Comment:
> +        self._init_message(self, from_address, to_address, subject, body)
> +
> +    @staticmethod
> +    def _init_message(msg, from_address, to_address, subject, body=None):

> It does seem pretty strange to have a static method that takes (usually)
> an instance of this class as its first parameter, and then switching on
> isinstance within it is also a bad sign.  To me it looks like rather than
> subclassing this type, we should have our own class which acts as a facade
> to one class or the other.

Hm. This seemed the lesser of two evils to me. Let me see if I understand
your facade concept: a class that, depending on whether there's an attachment
or not, is internally a MIMEMultipart object, or a simple Message.

I can think of two ways of doing this:

  * try to behave like a true Message/MIMEMultipart, that is, the
    creator of the object can do msg['From'] = 'Foo', etc., and use all
    the methods available in the standard Message/MIMEMultipart classes.
    For this, I guess one would have to play with __getattr__().

  * lock down the API of EmailMessage, reducing it to the constructor,
    and add_inline_attachment(). The creator cannot play with it. This
    sounds more sensible, an less error-prone.

Is either of this what you prefer, or something different altogether? In
the constructor a Message() instance is created, and there is code in
add_inline_attachment() to move from that instance to a MIMEMultipart()
one the first one that method is called.

> +
> +    @staticmethod
> +    def string_with_encoding(string):
> +        """Return a str object together with an encoding.
> +
> +        :param string: A str or unicode object.
> +        :return: A tuple (str, encoding), where encoding is one of 'ascii',
> +            'utf-8', or '8-bit', in that preferred order.

> Having automatic behaviour depending on whether something is a unicode or
> str object has been a bit problematic in the past.  Would it be feasible
> to have different interfaces and insist on the caller knowing whether
> they're putting in binary, ascii or unicode data?

Well, it was already agreed that add_inline_attachment(), and the body
parameter of the constructor, would accept either unicode or str,
sending as utf-8 and 8-bit, respectively. This function is, as explained
in the comment right below the docstring (copying it below for
convenience), only a step further because of how Python's email module
behaves. I'd rather keep it if possible, makes things more convinient.
(If it goes away, the only thing that changes is that EmailMessage
objects will *always* be base64-encoded.)

> Also it would be good if the docstring explained more about _why_ you
> would call this method - it's meant to guess the right encoding to use for
> sending this file.

Well, since it's a function local to the function, I didn't gave much
thought to that, an anybody messing with an email message would know
what's for. I can put a reduced version of the comment in the docstring
if you wish.

+ # Python's email module base64-encodes the body whenever the charset is
+ # not explicitly set to ascii. Because of this, and because we want to
+ # avoid base64 when it's not necessary in order to be most compatible
+ # with the capabilities of the receiving side, we check with encode()
+ # and decode() whether the body is actually ascii-only.


Just let me know what way to move on to get this hopefully merged some
day. ;-)

Thanks in advance,

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
                           Listening to: José Mercé - Te recuerdo Amanda