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

[X Ryl]

Thank you for your patch.

Can you provide me a way to test it (what fs should I use, options and so
on) ?
How different is the new xattrMaildir file to the existing one ? Would it
be possible to derive from the initial class and change the method that
needs to be changed ?
What's the purpose of this, compared to the version with the modified
filenames ?
Would it work with an external tool (or the synced flags will be ignored ?)

Best regards,
Cyril

On Mon, Mar 11, 2013 at 7:01 PM, Nicolas Sebrecht <nicolas.s-dev at laposte.net
> wrote:

> On Sun, Mar 10, 2013 at 09:48:25PM +0100, Erik Quaeghebeur wrote:
> > Dear maintainers & other interested subscribers,
> >
> >
> > To be applied on top of the previous patch I sent to this mailing list,
> > this patch adds a new repository type, xattrMaildir, a Maildir
> > repository where message files have all IMAP flags in an extended
> > attribute 'user.org.offlineimap.flags'. (It is perfectly backwards
> > compatible with the Maildir specification.)
> >
> > As before I have done a (successful) test that is minimal: for one
> > message, I synced from an IMAP server to an xattrMaildir repository and
> > verified (using the getfattr command-line tool) that all IMAP flags were
> > indeed synced.
> >
> > The code uses the xattr module (http://pyxattr.k1024.org/module.html) as
> > an extra dependency. Actually, very little has changed:
> >
> > * in  folder/__init__.py  and  repository/__init__.py  only boilerplate
> > needed to be added
>
> 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).
>
> > * 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?
>
> > * the new  folder/xattrMaildir.py  modifies three methods from
> > folder/Maildir.py, and in a minimal fashion:
> >
> >    - in quickchanged the check whether the flags have been changed as
> > reverted to an equality check (back from the subset check introduced in
> > my previous patch)
> >
> >    - _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).
>
> > 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.
>
> > Again, your testing and critique is very much welcome. The code is a
> > proof-of-concept and therefore very rough.
> >
> >
> > Best,
> >
> > Erik
> >
> > P.S.: As with my previous patch, I've also added it in attachment to
> > deal with copy-paste introduced linebreaks.
>
> Good. :-)
>
> Sorry, I don't have time to review the code much.
>
> --
> Nicolas Sebrecht
>
> _______________________________________________
> OfflineIMAP-project mailing list
> OfflineIMAP-project at lists.alioth.debian.org
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/offlineimap-project
>
> OfflineIMAP homepage: http://software.complete.org/offlineimap
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/offlineimap-project/attachments/20130311/1b907211/attachment-0003.html>