[Twisted-Python] Re: IMAP fixes

[Tony Meyer]

>   Eeep :o  This isn't quite what I was going for... The exec
> stage is a big no-no from my point of view.

Fair enough.

> I guess I should give a couple examples.

That helped!

>   The only other thing I wanted to mention was that scary
> regular expression.  I haven't even attempted to decipher it 
> yet, but I was curious if you pulled it out of or based it 
> off of one of the RFCs, or if you came up with it yourself?  

I built it myself from RFC2060, using Kodos (and my server) to test it.

> In my experience expressions that long tend to be wrong in
> unexpected and surprising ways, and I'm not inclined to trust 
> them without careful analysis, or at least the knowledge that 
> they come from an authoritative source (and even then, 
> ensuring the proper meaning survived into the translation to 
> python REs is a pain...)

Well, it started life as a python RE, so there was no translation.  I did
very carefully analyse (and test) it, and you're welcome to as well.  It's
easy enough to do without it, or split it into several smaller re's, but
this seems the best way to do it.

It's really not as complicated as it looks - a lot of the length comes from
having named groups, which makes things more reliable, not less.  There are
also several uncaptured groups, which could probably just be made into
regular groups, but the result is tidier this way.

I've made the re verbose, so maybe that helps?  (the changes also allowed it
to be simplified just a little, plus I've made it more for capturing and
less for checking, in the mime section specifier, particularly).

>   All that said, thanks for the patch!  I'm looking forward
> to the next revision :)

Here it is.  The atts passed should be more-or-less what you described, now.

The mailbox code isn't quite the same; you had (as an example):
>     def fetch(self, msgs, queryParts, uid):
>         r = {}
>         for q in queryParts:
>             r[q] = getattr(self, 'fetch_' + q.name)(msgs, q, uid)

But this seems strange to me - you end up with a dictionary where the keys
are the classes (Body(), UID(), etc).  Whatever the values end up being,
unless the message id is added to the class, then this really doesn't fit
with the existing __cbFetch at all, which wants the keys to be the message
ids (which makes sense to me).  My mailbox code looks like this:
    def fetch(self, msgs, queryParts, uid):
        r = {}
        for (id, msg) in self.messagesIter(msgs, uid):
            r[id] = {}
            for q in parts:
                r[id][q] = getattr(self, 'grab' + q.name.upper())(msg, q)
        return r
Which is pretty similar, except that the key is still the message id.  (I
also do it message by message, whereas yours was part by part, but that
shouldn't matter).

It also wasn't exactly clear how you wanted the values to be returned.  What
is does at the moment is pass back a dict where the values are dicts of
class (BODY(), UID(), etc) to string.  This seems to be pretty close to the
original.

Let me know what you think of this, and I'll keep on with the fixes...

=Tony Meyer
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: fetch_att.py
URL: </pipermail/twisted-python/attachments/20030710/5f4ce7b4/attachment.ksh>