[PATCH] Re: introducing xattrMaildir: a Maildir repository where message files have all IMAP flags in an extended attribute

[Erik Quaeghebeur]

Dear Nicolas,


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 
from Maildir.)

>> 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 
formatting preserved.


Thanks for looking at this,

Erik