On Tue, 2012-05-08 at 11:52 +0200, Christian Hilberg wrote: > Hi everyone! > > It has been a while [0] since the idea of making IMAPX > subclassable / extendable for backends to use. Time to > bring the topic back into the public again. :-) > > There is a bugzilla entry [1] now for the topic, and Chen > bravely started out with preparations to make IMAPX extendable. > > We've been staring at the imapx_untagged() function in IMAPX > for a while. This is the handler function which takes care of > processing the untagged responses from the IMAP server, and it > is the point where IMAP protocol extension code will want to > start hooking into. There may be other hooks needed. > > evolution-kolab uses IMAPX outside Evolution. Back in 2.30 era, > IMAPX was not extendable. We thus had the IMAPX code duped and > extended the hard way. Since I was overwhelmed by a single > function imapx_untagged(), approaching 1k LOC in length, I tore > it into pieces (one per untagged response) and added these into > a function table. This collapsed imapx_untagged() to almost nothing, > looking far more maintainable (and making the function extendable). > For those interested in the approach, see [2]. > > When porting evolution-kolab from 2.30 to 3.4, I decided to try > and subclass IMAPX in a clean way, since the 2.30 approach of > directly hacking my code into an IMAPX dupe would turn it into > a maintenance nightmare in the long run. > > There are two kinds of extensions to IMAPX I'm doing in evolution-kolab: > > * IMAP extensions (ANNOTATEMORE, METADATA (planned for), > IMAP ACL (planned for)) > > * Kolab extensions > > While the latter is Kolab specific, the former is not at all Kolab > specific, but holds IMAP extensions as per RFC. It therefore makes > sense to strive for integration of these extensions into upstream > IMAPX in order to have all users of IMAPX benefit from it. In the > evolution-kolab source tree, the former resides in > > src/camel/providers/imapx > > while the latter resides in > > src/camel > > to make the distinction clear. There are a number of CamelIMAPXExtd* > classes I've added to src/camel/providers/imapx which hold my > extensions over the upstream IMAPX (the upstream files are duped with > just minor changes, mostly exposing internal API for me to hook into). > Please see [3] for the stretches I needed to make to subclass IMAPX. > Main problem was that imapx_untagged() was not exposed, so I had to > re-implement all code paths to that function inside my own extended > classes. To ensure I would get my extended object types in the right > places, I had to override the CamelIMAPXConnManager classes with my > own, mainly because the IMAPX internal classes were not using virtualized > functions (which made it impossible for me to just inject my types here > and there - I had to make sure the right code paths were followed). > The classes in src/camel (thr Kolab-specific ones) look somewhat alike, > for that same reason, see [4]. > > I've not been diving into Chen's proposals very deeply yet. Allowing > subclasses to override imapx_untagged() seems like the first step. I've > not yet grokked all of the proposals, so I have a question: Will the > current proposal (see [1]) allow for a subclassed IMAPX to be subclassed > again? Subclassing IMAPX twice is what I'm doing in evolution-kolab. > Of course, the first subclassing (for adding support for folder metadata > and ACL) can just be dropped, once these extensions make it into > upstream subclassable IMAPX. > > Then, there's my table-based approach to imapx_untagged(). I've been > having a chat with Matt about that (and back in the days when I did the > implementation for 2.30, with David), which both liked the idea. Of course, > my 2.30 implementation was not done with truly subclassing IMAPX in mind, > so it would need some tweaking. I'll try to gather more thoughts as I'm > rovelling through the code of the proposal at BZ. > > What do you think? > > I would invite everyone who is hacking IMAPX to participate in this > discussion. I hope you had sent the mail just before our irc discussion. To summarize, https://bugzilla.gnome.org/show_bug.cgi?id=674310#c9 has the fixes. We now have a hook in imapx_untagged which would allow the subclasses to handle the responses that are not handled in IMAP (imapx provider). I did not find a case where a sub-classed imap provider would be interested in tags which are already processed by the parent imap provider, so i have provided a simple hook to handle, unhandled tags. I hope this would be sufficient to support all the above needs.
Please let me know if you need anything more.. Thanks, Chenthill. > > > [0] > http://mail.gnome.org/archives/evolution-hackers/2010-September/msg00012.html > [1] https://bugzilla.gnome.org/show_bug.cgi?id=674310 > [2] > http://git.gnome.org/browse/evolution-kolab/tree/src/camel/providers/imapx/camel-imapx-server.c?h=gnome-2-30#n1363 > [3] http://git.gnome.org/browse/evolution-kolab/tree/src/camel/providers/imapx > [4] http://git.gnome.org/browse/evolution-kolab/tree/src/camel/ > > _______________________________________________ > evolution-hackers mailing list > [email protected] > To change your list options or unsubscribe, visit ... > http://mail.gnome.org/mailman/listinfo/evolution-hackers _______________________________________________ evolution-hackers mailing list [email protected] To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
