[PATCH] Re: introducing xattrMaildir: a Maildir repository where message files have all IMAP flags in an extended attribute
offlineimap at equaeghe.nospammail.net
Tue Mar 12 09:54:01 GMT 2013
First of all, I must stress that this patch cannot work without my
earlier one “Let OfflineIMAP sync all flags between IMAP servers, not
just Maildir-compatible flags”, which is far more invasive, but—I
think—also useful to a much wider audience; formulated differently: I
think it corrects a historically understandable but wrong design choice
and is the more important of the two, needing close scrutiny.
>> The code uses the xattr module (http://pyxattr.k1024.org/module.html) as
>> an extra dependency. Actually, very little has changed:
> It would make sense to import the library only if required at runtime to
> have the dependency optionnal (this should then be documented in the
> sample configuration file).
Ok, I'll try to do that when preparing a new patch that modifies Maildir
to take an xattr option instead of adding a separate repository type.
>> * the new repository/xattrMaildir.py is also essentially boilerplate;
>> it is identical to repository/Maildir.py except for a replacement of
>> reference to folder.Maildir.Maildir to folder.xattrMaildir.xattrMaildir
> Didn't check: do you use polymorphism?
Um, not knowingly, but xattrMaildir does essentially just derive from
Maildir and then applies some changes to a few functions. The class
__init__ functions are not modified but inherited.
>> - _scanfolder and savemessageflags we actually read and write the
>> extended attributes (look for 'xattr.get' and 'xattr.set'), but these
>> are essentially one-line changes/additions.
> Did you refactored original methods to avoid duplicated code? Such
> "oneline" changes in the code looks like good place for refactoring
> (with an additional optional parameter in the original methods, I guess).
I avoided refactoring to not make the patch too big and the deviation
from the current code as small as possible.
>> Based on the realization that the changes needed are so small, it may be
>> considered that no new repository is introduced, but that the
>> functionality is activated by an option that controls the then three
>> necessary conditionals.
> I think the new repository type approach is good because of the library
> dependency issue.
Would it be ok to add the functionality to Maildir if the extra
dependency is hidden behind an option-controlled dependency, as i had
the impression you suggested above> (I now think this is the cleanest
way to add the functionality, given that xattrMaildir differs so little
>> P.S.: As with my previous patch, I've also added it in attachment to
>> deal with copy-paste introduced linebreaks.
> Good. :-)
I'll continue doing that, because I can't seem to get Thunderbird under
control... and the patch is attached as x-patch mimetype, with all
Thanks for looking at this,
More information about the OfflineIMAP-project