[PATCH 1/2] Re: Another way of locating UID of just saved message

[Vladimir Marek]

> > It works by fetching all headers from new messages from IMAP server an searching
> > for our X-OfflineIMAP marker by using reular expression.
>                                         ^^^^^^
>                                         regular

nice


[...]

> The logic looks quiet good to me but I'm still missing why we would need
> to fetch all headers of new mails. Is there any way to get the first or
> two messages headers of new mails?

Good question. I wanted to describe my motivation, but forget to do so.
Originally I have written the patch so that it was fetching just one
header line of a given type (X-OfflineIMAP). And it worked fine. But I'm
thinking about this method as a last-hope try before re-downloading the
message (which can be big). Fetching whole header should not be too big
and should be hopefully bulletproof.



> assert sucks. I'd rather much raise an OfflineImapError.

ok


> 
> > +        found = 0
> > +        for item in result:
> > +            if found == 0 and type(item) == type( () ):
> > +                # Walk just tuples
> > +                if re.search("(?:^|\\r|\\n)%s:\s*%s(?:\\r|\\n)" % (headername, headervalue), item[1], flags=re.IGNORECASE):
> > +                    found = 1
> > +            elif found == 1:
> > +                if type(item) == type (""):
> 
>                    if type(item) == str:

I thought there's better way, but didn't find it.

> > +            if found == 0 and type(item) == type( () ):

Maybe there's better way of specifying array type too?


> > +                    if uid == 0:
> > +                        self.ui.debug('imap', 'savemessage: second attempt to get new UID failed. Going to try search headers manually')
> 
> Not that I'm against long lines in code (the 80 chars is obsolete IMHO),
> I think you should limit to 120 characters, at least.

Sure


Thanks for the review, I'll eventually post fixed version
-- 
	Vlad