Re: [Evolution-hackers] Couple new functions for ECal/EBook
On Tue, 2010-07-20 at 11:21 +0530, chen wrote: > On Mon, 2010-07-19 at 17:27 -0400, Matthew Barnes wrote: > > Backends using these classes can easily construct a suitable cache file > > or directory name from e_cal_backend_get_cache_dir(). > get_cache_dir can simply return e_cal_backend_store_get_path. I just realized ECalBackend -- which is where the "cache-dir" property lives -- doesn't have access to ECalBackendStore. The derived backend classes all keep their own private store objects. So without an API break to make ECalBackendStore just take a filename, I'm back to square one. In lieu of an API break I think I'll have to just replicate the path building logic in ECalBackend, ECalBackendStore and ECalBackendCache. Not ideal, but still better than all the backends doing it themselves. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Couple new functions for ECal/EBook
On Tue, 2010-07-20 at 06:34 -0400, Matthew Barnes wrote: > On Tue, 2010-07-20 at 11:21 +0530, chen wrote: > > Is it required ? Having it in ECalBackendStore simplifies the backend > > code to form the path based on the type (calendar,tasks,memos) at a > > single place rather than every backend doing it.. > > Forming the path at a single place instead of all the backends doing it > is definitely the goal. My thought was just that ECalBackend can do it > as easily as ECalBackendStore, since ECalBackendStore does nothing else > with the URI + type it's given and the path still has to be shared with > ECalBackendCache somehow. > > Moving it to ECalBackend seemed cleaner to me, but I can try to keep > things as is if you'd prefer to avoid the API break. I am ok with having it in ECalBackend. > > > > Btw we could deprecate ECalBackendCache and update just > > ECalBackendStore. > > I thought of that, but evolution-mapi still uses ECalBackendCache pretty > heavily. Should we give Bharath and Milan some time to migrate off it > first? I hope they are doing it. Had a chat with Bharath a week back iirc as I realized just then that mapi was not migrated. It should take not more than half a day to migrate at the maximum. It should be more or less s/cache/store work :) - Chenthill. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Couple new functions for ECal/EBook
On Tue, 2010-07-20 at 11:21 +0530, chen wrote: > Is it required ? Having it in ECalBackendStore simplifies the backend > code to form the path based on the type (calendar,tasks,memos) at a > single place rather than every backend doing it.. Forming the path at a single place instead of all the backends doing it is definitely the goal. My thought was just that ECalBackend can do it as easily as ECalBackendStore, since ECalBackendStore does nothing else with the URI + type it's given and the path still has to be shared with ECalBackendCache somehow. Moving it to ECalBackend seemed cleaner to me, but I can try to keep things as is if you'd prefer to avoid the API break. > Btw we could deprecate ECalBackendCache and update just > ECalBackendStore. I thought of that, but evolution-mapi still uses ECalBackendCache pretty heavily. Should we give Bharath and Milan some time to migrate off it first? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Couple new functions for ECal/EBook
On Mon, 2010-07-19 at 17:27 -0400, Matthew Barnes wrote: > On Mon, 2010-07-19 at 09:10 +0200, Milan Crha wrote: > > that's a part which should be changed. The ideal way, as I understand > > it, is to have an ECalBackend function to retrieve a path for > > attachments, and ECal should ask backend for it, instead of duplicating > > the code (it sort of blocks extendibility of ECalBackend, because the > > client side depends on the code in evolution-data-server itself, which > > is kinda wrong). > > > > It will be good to fix this and move functions to libedata-book/cal, > > where they, as you said, belong anyway. > > I took Milan's advice and attempted to do this The Right Way... I think. > > Here's what's changed: > > Instead of adding standalone functions, I added a "cache-dir" property > along with the usual get/set functions to EBookBackend and ECalBackend. > The property is read/write, but the default value should rarely be > overwritten (currently only the file backends override it). > > const gchar * e_book_backend_get_cache_dir (EBookBackend *backend); > void e_book_backend_set_cache_dir (EBookBackend *backend, > const gchar *cache_dir); > > const gchar * e_cal_backend_get_cache_dir (ECalBackend *backend); > void e_cal_backend_set_cache_dir (ECalBackend *backend, > const gchar *cache_dir); > > Also I slightly broke the API of ECalBackendCache and ECalBackendStore > to take a cache file name or cache directory name (respectively) instead > of a URI and ECalSourceType, since the URI and ECalSourceType are only > used to build a cache file name or cache directory name. > > Backends using these classes can easily construct a suitable cache file > or directory name from e_cal_backend_get_cache_dir(). get_cache_dir can simply return e_cal_backend_store_get_path. > > ECalBackendCache * > e_cal_backend_cache_new (const gchar *filename); > > ECalBackendFileStore * > e_cal_backend_file_store_new (const gchar *path); Is it required ? Having it in ECalBackendStore simplifies the backend code to form the path based on the type (calendar,tasks,memos) at a single place rather than every backend doing it.. > > So now the name of a backend's base cache directory is centralized in > its base class. That takes care of the server side. > > For the client side I added a simple ECal D-Bus method getCacheDir(), > which does what it says. ECal still caches the directory internally, so > the method will only be called once per ECal instance. > > I did not add a similar D-Bus method to EBook because there's currently > no need for it, but we could easily add it later if desired. > > How does this sound? Any objections to the minor API break in the > backend libraries? Btw we could deprecate ECalBackendCache and update just ECalBackendStore. - Chenthill. > > ___ > evolution-hackers mailing list > evolution-hackers@gnome.org > To change your list options or unsubscribe, visit ... > http://mail.gnome.org/mailman/listinfo/evolution-hackers ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Couple new functions for ECal/EBook
On Mon, 2010-07-19 at 09:10 +0200, Milan Crha wrote: > that's a part which should be changed. The ideal way, as I understand > it, is to have an ECalBackend function to retrieve a path for > attachments, and ECal should ask backend for it, instead of duplicating > the code (it sort of blocks extendibility of ECalBackend, because the > client side depends on the code in evolution-data-server itself, which > is kinda wrong). > > It will be good to fix this and move functions to libedata-book/cal, > where they, as you said, belong anyway. I took Milan's advice and attempted to do this The Right Way... I think. Here's what's changed: Instead of adding standalone functions, I added a "cache-dir" property along with the usual get/set functions to EBookBackend and ECalBackend. The property is read/write, but the default value should rarely be overwritten (currently only the file backends override it). const gchar * e_book_backend_get_cache_dir (EBookBackend *backend); void e_book_backend_set_cache_dir (EBookBackend *backend, const gchar *cache_dir); const gchar * e_cal_backend_get_cache_dir (ECalBackend *backend); void e_cal_backend_set_cache_dir (ECalBackend *backend, const gchar *cache_dir); Also I slightly broke the API of ECalBackendCache and ECalBackendStore to take a cache file name or cache directory name (respectively) instead of a URI and ECalSourceType, since the URI and ECalSourceType are only used to build a cache file name or cache directory name. Backends using these classes can easily construct a suitable cache file or directory name from e_cal_backend_get_cache_dir(). ECalBackendCache * e_cal_backend_cache_new (const gchar *filename); ECalBackendFileStore * e_cal_backend_file_store_new (const gchar *path); So now the name of a backend's base cache directory is centralized in its base class. That takes care of the server side. For the client side I added a simple ECal D-Bus method getCacheDir(), which does what it says. ECal still caches the directory internally, so the method will only be called once per ECal instance. I did not add a similar D-Bus method to EBook because there's currently no need for it, but we could easily add it later if desired. How does this sound? Any objections to the minor API break in the backend libraries? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Couple new functions for ECal/EBook
On Sun, 2010-07-18 at 20:34 -0400, Matthew Barnes wrote: > Only reason I wrote them for the client-side libraries is > because e_cal_get_user_cache_dir() is used to figure out where to > cache ECal attachments in set_local_attachment_store(). Hi, that's a part which should be changed. The ideal way, as I understand it, is to have an ECalBackend function to retrieve a path for attachments, and ECal should ask backend for it, instead of duplicating the code (it sort of blocks extendibility of ECalBackend, because the client side depends on the code in evolution-data-server itself, which is kinda wrong). It will be good to fix this and move functions to libedata-book/cal, where they, as you said, belong anyway. And because there were API changes for ECal/BookBackend anyway, then this one shouldn't hurt more. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
[Evolution-hackers] Couple new functions for ECal/EBook
As part of the XDG base directory effort, I've written a couple simple functions for libebook and libecal: gchar * e_book_get_user_cache_dir (const gchar *source_uri); gchar * e_cal_get_user_cache_dir (const gchar *source_uri, ECalSourceType source_type); These functions return the cache directory path for the given URI, using e_get_user_cache_dir(). They also centralize the URI-to-directory-name mangling that I see repeated in quite a few places. I'm polling for objections because there's a valid argument to be made that these functions really belong in libedata-book and libedata-cal instead. Only reason I wrote them for the client-side libraries is because e_cal_get_user_cache_dir() is used to figure out where to cache ECal attachments in set_local_attachment_store(). Thoughts? If not I'll proceed. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers