Re: [Evolution-hackers] Couple new functions for ECal/EBook

2010-07-20 Thread Matthew Barnes
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

2010-07-20 Thread chen
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

2010-07-20 Thread Matthew Barnes
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

2010-07-19 Thread Milan Crha
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


Re: [Evolution-hackers] Couple new functions for ECal/EBook

2010-07-19 Thread chen
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


[Evolution-hackers] Couple new functions for ECal/EBook

2010-07-18 Thread Matthew Barnes
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